From 8f1e0d36e82f4fb409cdf8f90b857c309b3062d7 Mon Sep 17 00:00:00 2001 From: Dani Date: Sun, 28 Oct 2018 18:42:50 +0100 Subject: [PATCH 1/2] feat: allow writing properties on creation selectively --- README.md | 19 +++++++++++++++++++ lib/read-only.js | 13 ++++++++++--- .../simple-app/common/models/product.json | 8 +++++++- test/test.js | 16 ++++++++++++++++ 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index e92a406..09e0a0d 100644 --- a/README.md +++ b/README.md @@ -125,6 +125,25 @@ In this example we mark the `status` and `role` fields as readonly. } ``` +Sometimes you'll want to allow some properties to be created. For example, +you might want to allow the user to set a property on sign up and disallow +any future changes. + +You can achieve this by passing an array with the keys in the `__allowCreation` +option. + +```json + { + "mixins": { + "ReadOnly" : { + "status" : true, + "role" : true, + "__allowCreation": [ "role" ] + } + } + } +``` + Any data set by a REST client in ReadOnly properties will be stripped out on the way to the server and will not be saved on the updated model instance. diff --git a/lib/read-only.js b/lib/read-only.js index 15d6ea1..726a513 100644 --- a/lib/read-only.js +++ b/lib/read-only.js @@ -2,8 +2,12 @@ const debug = require('debug')('loopback:mixin:readonly') -function deletePropertiesFrom(properties, data) { +function deletePropertiesFrom(properties, data, except) { Object.keys(properties).forEach(key => { + if (except.indexOf(key) > -1) { + debug('The \'%s\' property is read only, but marked as allowCreation, doing nothing', key) + return + } debug('The \'%s\' property is read only, removing incoming data', key) delete data[key] }) @@ -33,10 +37,13 @@ module.exports = Model => { const AffectedModel = Model.app.loopback.getModel(modelName) const options = AffectedModel.settings.mixins.ReadOnly const properties = (Object.keys(options).length) ? options : null + const allowCreation = properties && properties.__allowCreation && properties.__allowCreation.length ? + properties.__allowCreation : [] const instanceId = ctx.args[AffectedModel.getIdName()] + if (properties) { - debug('Found read only properties for model %s: %o', modelName, properties) + debug('Creating %s : Read only properties are %j', Model.modelName, properties) // Handle the case for updating an existing instance. if (instanceId) { @@ -53,7 +60,7 @@ module.exports = Model => { } // Handle the case creating a new instance. - deletePropertiesFrom(properties, body) + deletePropertiesFrom(properties, body, allowCreation) return next() } diff --git a/test/fixtures/simple-app/common/models/product.json b/test/fixtures/simple-app/common/models/product.json index c696534..8084cb7 100644 --- a/test/fixtures/simple-app/common/models/product.json +++ b/test/fixtures/simple-app/common/models/product.json @@ -17,6 +17,10 @@ "type": "string", "default": "temp" }, + "allowCreateProp": { + "type": "string", + "default": "temp" + }, "personId": { "type": "string" } @@ -33,7 +37,9 @@ "methods": {}, "mixins": { "ReadOnly": { - "status": true + "status": true, + "allowCreateProp": true, + "__allowCreation": ["allowCreateProp"] } } } diff --git a/test/test.js b/test/test.js index 2db2066..72c7a73 100644 --- a/test/test.js +++ b/test/test.js @@ -60,6 +60,22 @@ describe('loopback datasource readonly property (mixin sources.js)', function() expect(res.body.status).to.equal('temp') }) }) + + it('should save readonly properties if allowCreate is set', function() { + return this.post('/api/products') + .send({ + name: 'test product', + status: 'active', + allowCreateProp: 'new value', + }) + .expect(200) + .then(res => { + expect(res.body.name).to.equal('test product') + expect(res.body.status).to.equal('temp') + expect(res.body.allowCreateProp).to.not.equal('temp') + expect(res.body.allowCreateProp).to.equal('new value') + }) + }) }) describe('updateAttributes', function() { From 7f054bcd8c77547b1ad31f4e660d836a9c06f817 Mon Sep 17 00:00:00 2001 From: Dani Date: Sun, 28 Oct 2018 19:27:58 +0100 Subject: [PATCH 2/2] fix: allowCreation now checks the instance ID from ctx --- lib/read-only.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/read-only.js b/lib/read-only.js index 726a513..43722f4 100644 --- a/lib/read-only.js +++ b/lib/read-only.js @@ -26,7 +26,7 @@ module.exports = Model => { debug('ReadOnly mixin for Model %s', Model.modelName) Model.on('attached', () => { - Model.stripReadOnlyProperties = (modelName, ctx, modelInstance, next) => { + Model.stripReadOnlyProperties = (modelName, ctx, modelInstance, next, relationship) => { debug('stripReadOnlyProperties for model %s (via remote method %o)', modelName, ctx.methodString) const { body } = ctx.req @@ -39,7 +39,9 @@ module.exports = Model => { const properties = (Object.keys(options).length) ? options : null const allowCreation = properties && properties.__allowCreation && properties.__allowCreation.length ? properties.__allowCreation : [] - const instanceId = ctx.args[AffectedModel.getIdName()] + const idName = AffectedModel.getIdName() + const instanceId = !relationship && ctx.instance && ctx.instance[idName] ? + ctx.instance[idName] : ctx.args[idName] if (properties) { @@ -109,7 +111,7 @@ module.exports = Model => { Model.beforeRemote(`prototype.__updateById__${relationName}`, (ctx, modelInstance, next) => { if (typeof AffectedModel.stripReadOnlyProperties === 'function') { - return AffectedModel.stripReadOnlyProperties(modelName, ctx, modelInstance, next) + return AffectedModel.stripReadOnlyProperties(modelName, ctx, modelInstance, next, true) } return next() })