🐛 Fixed updated_at not being updated (#9532)

closes #9520

- it contains a dependency bump of the latest Bookshelf release
- Bookshelf introduced a bug in the last release
  - see https://github.com/bookshelf/bookshelf/pull/1583
  - see https://github.com/bookshelf/bookshelf/pull/1798
- this has caused trouble in Ghost
  - the `updated_at` attribute was not automatically set anymore

---

The bookshelf added one breaking change: it's allow to pass custom `updated_at` and `created_at`.
We already have a protection for not being able to override the `created_at` date on update.
We had to add another protection to now allow to only change the `updated_at` property.
You can only change `updated_at` if you actually change something else e.g. the title of a post.

To be able to implement this check i discovered that Bookshelfs `model.changed` object has a tricky behaviour.
It remembers **all** attributes, which where changed, doesn't matter if they are valid or invalid model properties.
We had to add a line of code to avoid remembering none valid model attributes in this object.

e.g. you change `tag.parent` (no valid model attribute). The valid property is `tag.parent_id`.
     If you pass `tag.parent` but the value has **not** changed (`tag.parent` === `tag.parent_id`), it will output you `tag.changed.parent`. But this is wrong.
     Bookshelf detects `changed` attributes too early. Or if you think the other way around, Ghost detects valid attributes too late.
     But the current earliest possible stage is the `onSaving` event, there is no earlier way to pick valid attributes (except of `.forge`, but we don't use this fn ATM).
     Later: the API should transform `tag.parent` into `tag.parent_id`, but we are not using it ATM, so no need to pre-optimise.
     The API already transforms `post.author` into `post.author_id`.

core/server/models/base/index.js

@@ -171,6 +171,18 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
         this.attributes = this.pick(this.permittedAttributes());
         // Store the previous attributes so we can tell what was updated later
         this._updatedAttributes = newObj.previousAttributes();
+
+        /**
+         * Bookshelf keeps none valid model attributes in `model.changed`. This causes problems
+         * when detecting if a model has changed. Bookshelf detects changed attributes too early.
+         * So we have to manually remove invalid model attributes from this object.
+         *
+         * e.g. if you pass `tag.parent` into the model layer, but the value has not changed,
+         * the attribute (`tag.parent`) is still kept in the `changed` object. This is wrong.
+         *
+         * TLDR; only keep valid model attributes in the changed object
+         */
+        this.changed = _.pick(this.changed, Object.keys(this.attributes));
     },
 
     /**
@@ -243,6 +255,11 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
             }
         }
 
+        // CASE: you only change the `updated_at` property. This is not allowed.
+        if (newObj.hasChanged() && Object.keys(newObj.changed).length === 1 && newObj.changed.hasOwnProperty('updated_at')) {
+            newObj.set('updated_at', this.previous('updated_at'));
+        }
+
         return Promise.resolve(this.onValidate(newObj, attr, options));
     },
 

core/test/integration/model/model_posts_spec.js

@@ -1811,17 +1811,12 @@ describe('Post Model', function () {
         });
 
         it('can\'t edit dates and authors of existing tag', function () {
-            var newJSON = _.cloneDeep(postJSON), updatedAtFormat;
+            var newJSON = _.cloneDeep(postJSON), updatedAtFormat, createdAtFormat;
 
             // Add an existing tag to the beginning of the array
-            newJSON.tags = [{
-                id: postJSON.tags[0].id,
-                slug: 'eins',
-                created_at: moment().add(2, 'days').format('YYYY-MM-DD HH:mm:ss'),
-                updated_at: moment().add(2, 'days').format('YYYY-MM-DD HH:mm:ss'),
-                created_by: 2,
-                updated_by: 2
-            }];
+            newJSON.tags = [_.cloneDeep(postJSON.tags[0])];
+            newJSON.tags[0].created_at = moment().add(2, 'days').format('YYYY-MM-DD HH:mm:ss');
+            newJSON.tags[0].updated_at = moment().add(2, 'days').format('YYYY-MM-DD HH:mm:ss');
 
             // Edit the post
             return Promise.delay(1000)
@@ -1834,16 +1829,19 @@ describe('Post Model', function () {
                     updatedPost.tags.should.have.lengthOf(1);
                     updatedPost.tags[0].should.have.properties({
                         name: postJSON.tags[0].name,
-                        slug: 'eins',
+                        slug: postJSON.tags[0].slug,
                         id: postJSON.tags[0].id,
-                        created_at: postJSON.tags[0].created_at,
-                        created_by: postJSON.created_by,
-                        updated_by: postJSON.updated_by
+                        created_by: postJSON.tags[0].created_by,
+                        updated_by: postJSON.tags[0].updated_by
                     });
 
                     updatedAtFormat = moment(updatedPost.tags[0].updated_at).format('YYYY-MM-DD HH:mm:ss');
-                    updatedAtFormat.should.not.eql(moment(postJSON.updated_at).format('YYYY-MM-DD HH:mm:ss'));
+                    updatedAtFormat.should.eql(moment(postJSON.tags[0].updated_at).format('YYYY-MM-DD HH:mm:ss'));
                     updatedAtFormat.should.not.eql(moment(newJSON.tags[0].updated_at).format('YYYY-MM-DD HH:mm:ss'));
+
+                    createdAtFormat = moment(updatedPost.tags[0].created_at).format('YYYY-MM-DD HH:mm:ss');
+                    createdAtFormat.should.eql(moment(postJSON.tags[0].created_at).format('YYYY-MM-DD HH:mm:ss'));
+                    createdAtFormat.should.not.eql(moment(newJSON.tags[0].created_at).format('YYYY-MM-DD HH:mm:ss'));
                 });
         });
 

package.json

@@ -34,7 +34,7 @@
     "bcryptjs": "2.4.3",
     "bluebird": "3.5.1",
     "body-parser": "1.18.2",
-    "bookshelf": "0.13.0",
+    "bookshelf": "0.13.3",
     "bookshelf-relations": "0.2.0",
     "brute-knex": "https://github.com/cobbspur/brute-knex/tarball/4feff38ad2e4ccd8d9de05f04a2ad7a5eb3e0ac1",
     "bson-objectid": "1.2.2",

yarn.lock

@@ -535,9 +535,9 @@ bookshelf-relations@0.2.0:
     ghost-ignition "^2.8.16"
     lodash "^4.17.4"
 
-bookshelf@0.13.0:
-  version "0.13.0"
-  resolved "https://registry.yarnpkg.com/bookshelf/-/bookshelf-0.13.0.tgz#dec282886c7653436a43c1e55fcb873c5822cb4a"
+bookshelf@0.13.3:
+  version "0.13.3"
+  resolved "https://registry.yarnpkg.com/bookshelf/-/bookshelf-0.13.3.tgz#aa73d9159b6cac92830dc1ff37325490c3c6dfba"
   dependencies:
     babel-runtime "^6.26.0"
     bluebird "^3.4.3"