fix; retain existing values on update

Author: Tom Kirkpatrick

lib/read-only.js

@@ -2,11 +2,27 @@
 
 const debug = require('debug')('loopback:mixin:readonly')
 
+function deletePropertiesFrom(properties, data) {
+  Object.keys(properties).forEach(key => {
+    debug('The \'%s\' property is read only, removing incoming data', key)
+    delete data[key]
+  })
+}
+
+function replacePropertiesWithValuesFrom(properties, from, to) {
+  Object.keys(properties).forEach(key => {
+    const value = from[key]
+
+    debug('The \'%s\' property is read only, replacing incoming data with existing value: %o', key, value)
+    to[key] = value
+  })
+}
+
 module.exports = Model => {
   debug('ReadOnly mixin for Model %s', Model.modelName)
 
   Model.on('attached', () => {
-    Model.stripReadOnlyProperties = (modelName, ctx, next) => {
+    Model.stripReadOnlyProperties = (modelName, ctx, modelInstance, next) => {
       debug('stripReadOnlyProperties for model %s (via remote method %o)', modelName, ctx.methodString)
       const { body } = ctx.req
 
@@ -17,14 +33,29 @@ 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 instanceId = ctx.args[AffectedModel.getIdName()]
 
       if (properties) {
         debug('Found read only properties for model %s: %o', modelName, properties)
-        Object.keys(properties).forEach(key => {
-          debug('The \'%s\' property is read only, removing incoming data', key)
-          delete body[key]
-        })
+
+        // Handle the case for updating an existing instance.
+        if (instanceId) {
+          return AffectedModel.findById(instanceId)
+            .then(instance => {
+              if (instance) {
+                replacePropertiesWithValuesFrom(properties, instance, body)
+              }
+              else {
+                deletePropertiesFrom(properties, body)
+              }
+              return next()
+            })
+        }
+
+        // Handle the case creating a new instance.
+        deletePropertiesFrom(properties, body)
         return next()
+
       }
       const err = new Error(`Unable to update: ${modelName} is read only.`)
 
@@ -34,31 +65,31 @@ module.exports = Model => {
 
     // Handle native model methods.
     Model.beforeRemote('create', (ctx, modelInstance, next) => {
-      Model.stripReadOnlyProperties(Model.modelName, ctx, next)
+      Model.stripReadOnlyProperties(Model.modelName, ctx, modelInstance, next)
     })
     Model.beforeRemote('upsert', (ctx, modelInstance, next) => {
-      Model.stripReadOnlyProperties(Model.modelName, ctx, next)
+      Model.stripReadOnlyProperties(Model.modelName, ctx, modelInstance, next)
     })
     Model.beforeRemote('replaceOrCreate', (ctx, modelInstance, next) => {
-      Model.stripReadOnlyProperties(Model.modelName, ctx, next)
+      Model.stripReadOnlyProperties(Model.modelName, ctx, modelInstance, next)
     })
     Model.beforeRemote('patchOrCreate', (ctx, modelInstance, next) => {
-      Model.stripReadOnlyProperties(Model.modelName, ctx, next)
+      Model.stripReadOnlyProperties(Model.modelName, ctx, modelInstance, next)
     })
     Model.beforeRemote('prototype.updateAttributes', (ctx, modelInstance, next) => {
-      Model.stripReadOnlyProperties(Model.modelName, ctx, next)
+      Model.stripReadOnlyProperties(Model.modelName, ctx, modelInstance, next)
     })
     Model.beforeRemote('prototype.patchAttributes', (ctx, modelInstance, next) => {
-      Model.stripReadOnlyProperties(Model.modelName, ctx, next)
+      Model.stripReadOnlyProperties(Model.modelName, ctx, modelInstance, next)
     })
     Model.beforeRemote('updateAll', (ctx, modelInstance, next) => {
-      Model.stripReadOnlyProperties(Model.modelName, ctx, next)
+      Model.stripReadOnlyProperties(Model.modelName, ctx, modelInstance, next)
     })
     Model.beforeRemote('upsertWithWhere', (ctx, modelInstance, next) => {
-      Model.stripReadOnlyProperties(Model.modelName, ctx, next)
+      Model.stripReadOnlyProperties(Model.modelName, ctx, modelInstance, next)
     })
     Model.beforeRemote('replaceById', (ctx, modelInstance, next) => {
-      Model.stripReadOnlyProperties(Model.modelName, ctx, next)
+      Model.stripReadOnlyProperties(Model.modelName, ctx, modelInstance, next)
     })
 
     // Handle updates via relationship.
@@ -71,7 +102,7 @@ module.exports = Model => {
 
         Model.beforeRemote(`prototype.__updateById__${relationName}`, (ctx, modelInstance, next) => {
           if (typeof AffectedModel.stripReadOnlyProperties === 'function') {
-            return AffectedModel.stripReadOnlyProperties(modelName, ctx, next)
+            return AffectedModel.stripReadOnlyProperties(modelName, ctx, modelInstance, next)
           }
           return next()
         })

test/test.js

@@ -88,8 +88,8 @@ describe('loopback datasource readonly property (mixin sources.js)', function()
           .expect(200)
           .then(res => {
             expect(res.body.name).to.equal('Tom (edited)')
-            expect(res.body.status).to.be.undefined()
-            expect(res.body.role).to.be.undefined()
+            expect(res.body.status).to.equal('disabled')
+            expect(res.body.role).to.equal('user')
           })
       })
     })
@@ -171,11 +171,11 @@ describe('loopback datasource readonly property (mixin sources.js)', function()
     describe('replaceById', function() {
       lt.beforeEach.givenModel('Product', { name: 'book 1', type: 'book', status: 'pending' }, 'product')
       it('should not change readonly properties', function() {
-        return json('post', `/api/products/${this.product.id}/replace`)
+        return json('put', `/api/products/${this.product.id}`)
           .send({ id: this.product.id, status: 'disabled' })
           .expect(200)
           .then(() => app.models.Product.findById(this.product.id))
-          .then(product => expect(product.status).to.equal('temp'))
+          .then(product => expect(product.status).to.equal('pending'))
       })
     })