diff --git a/core/server/api/utils.js b/core/server/api/utils.js index 9c6a683328..69f4f2c48e 100644 --- a/core/server/api/utils.js +++ b/core/server/api/utils.js @@ -305,6 +305,16 @@ utils = { * ### Check Object * Check an object passed to the API is in the correct format * + * @TODO: + * The weird thing about this function is.. + * - that the API converts properties back to model notation + * - post.author -> post.author_id + * - and the model layer implementation of `toJSON` knows about these transformations as well + * - post.author_id -> post.author + * - this must live in one place + * - API IN <-> API OUT + * - this should be unrelated to the model layer + * * @param {Object} object * @param {String} docName * @returns {Promise(Object)} resolves to the original object if it checks out @@ -352,6 +362,51 @@ utils = { message: common.i18n.t('errors.api.utils.invalidStructure', {key: 'posts[*].authors'}) })); } + + /** + * CASE: we don't support updating nested-nested relations e.g. `post.authors[*].roles` yet. + * + * Bookshelf-relations supports this feature, BUT bookshelf's `hasChanged` fn will currently + * clash with this, because `hasChanged` won't be able to tell if relations have changed or not. + * It would always return `changed.roles = [....]`. It would always throw a model event that relations + * were updated, which is not true. + * + * Bookshelf-relations can tell us if a relation has changed, it knows that. + * But the connection between our model layer, Bookshelf's `hasChanged` fn and Bookshelf-relations + * is not present. As long as we don't support this case, we have to ignore this. + */ + if (object.posts[0].authors && object.posts[0].authors.length) { + _.each(object.posts[0].authors, (author, index) => { + if (author.hasOwnProperty('roles')) { + delete object.posts[0].authors[index].roles; + } + + if (author.hasOwnProperty('permissions')) { + delete object.posts[0].authors[index].permissions; + } + }); + } + } + + /** + * Model notation is: `tag.parent_id`. + * The API notation is `tag.parent`. + * + * See @TODO on the fn description. This information lives in two places. Not nice. + */ + if (object.posts[0].hasOwnProperty('tags')) { + if (_.isArray(object.posts[0].tags) && object.posts[0].tags.length) { + _.each(object.posts[0].tags, (tag, index) => { + if (tag.hasOwnProperty('parent')) { + object.posts[0].tags[index].parent_id = tag.parent; + delete object.posts[0].tags[index].parent; + } + + if (tag.hasOwnProperty('posts')) { + delete object.posts[0].tags[index].posts; + } + }); + } } } diff --git a/core/test/integration/model/model_posts_spec.js b/core/test/integration/model/model_posts_spec.js index b970e2e187..9d1b0ca176 100644 --- a/core/test/integration/model/model_posts_spec.js +++ b/core/test/integration/model/model_posts_spec.js @@ -1835,6 +1835,10 @@ describe('Post Model', function () { 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'); + // NOTE: this is currently only removed in the API layer + newJSON.tags[0].parent_id = newJSON.tags[0].parent; + delete newJSON.tags[0].parent; + // Edit the post return Promise.delay(1000) .then(function () { diff --git a/core/test/unit/api/utils_spec.js b/core/test/unit/api/utils_spec.js index 4ed25ed096..860b015881 100644 --- a/core/test/unit/api/utils_spec.js +++ b/core/test/unit/api/utils_spec.js @@ -351,6 +351,16 @@ describe('API Utils', function () { }).catch(done); }); + it('passed through a simple, correct object', function (done) { + var object = {test: [{id: 1}]}; + apiUtils.checkObject(_.cloneDeep(object), 'test').then(function (data) { + should.exist(data); + data.should.have.ownProperty('test'); + object.should.eql(data); + done(); + }).catch(done); + }); + it('[DEPRECATED] should do author_id to author conversion for posts', function (done) { var object = {posts: [{id: 1, author: 4}]}; apiUtils.checkObject(_.cloneDeep(object), 'posts').then(function (data) { @@ -425,7 +435,76 @@ describe('API Utils', function () { }).catch(done); }); - describe('invalid post.authors structure', function () { + describe('post.tags structure', function () { + it('post.tags is empty', function (done) { + var object = { + posts: [{ + id: 1, + tags: [] + }] + }; + + apiUtils.checkObject(_.cloneDeep(object), 'posts') + .then(function (object) { + object.posts[0].tags.length.should.eql(0); + done(); + }) + .catch(done); + }); + + it('post.tags contains `parent`', function (done) { + var object = { + posts: [{ + id: 1, + tags: [{id: 'objectid', parent: null}] + }] + }; + + apiUtils.checkObject(_.cloneDeep(object), 'posts') + .then(function (object) { + object.posts[0].tags[0].hasOwnProperty('parent').should.be.false(); + object.posts[0].tags[0].hasOwnProperty('parent_id').should.be.true(); + done(); + }) + .catch(done); + }); + + it('post.tags contains `parent_id`', function (done) { + var object = { + posts: [{ + id: 1, + tags: [{id: 'objectid', parent_id: null}] + }] + }; + + apiUtils.checkObject(_.cloneDeep(object), 'posts') + .then(function (object) { + object.posts[0].tags[0].hasOwnProperty('parent').should.be.false(); + object.posts[0].tags[0].hasOwnProperty('parent_id').should.be.true(); + done(); + }) + .catch(done); + }); + + it('post.tags contains no parent', function (done) { + var object = { + posts: [{ + id: 1, + tags: [{id: 'objectid'}] + }] + }; + + apiUtils.checkObject(_.cloneDeep(object), 'posts') + .then(function (object) { + object.posts[0].tags[0].hasOwnProperty('parent').should.be.false(); + object.posts[0].tags[0].hasOwnProperty('parent_id').should.be.false(); + done(); + }) + .catch(done); + }); + }); + + describe('post.authors structure', function () { it('post.authors is not present', function (done) { var object = {posts: [{id: 1}]}; @@ -488,6 +567,23 @@ describe('API Utils', function () { done(); }); }); + + it('post.authors contains nested relations', function (done) { + var object = { + posts: [{ + id: 1, + authors: [{id: 'objectid', name: 'Kate', roles: [{id: 'something'}], permissions: []}] + }] + }; + + apiUtils.checkObject(_.cloneDeep(object), 'posts') + .then(function (object) { + should.not.exist(object.posts[0].authors[0].roles); + should.not.exist(object.posts[0].authors[0].permissions); + done(); + }) + .catch(done); + }); }); }); diff --git a/core/test/unit/models/user_spec.js b/core/test/unit/models/user_spec.js index 71f2ba23ee..75735e74fb 100644 --- a/core/test/unit/models/user_spec.js +++ b/core/test/unit/models/user_spec.js @@ -309,6 +309,37 @@ describe('Unit: models/user', function () { }); }); + describe('Fetch', function () { + let knexMock; + + before(function () { + models.init(); + }); + + after(function () { + sandbox.restore(); + }); + + before(function () { + knexMock = new testUtils.mocks.knex(); + knexMock.mock(); + }); + + after(function () { + knexMock.unmock(); + }); + + it('ensure data type', function () { + return models.User.findOne({slug: 'joe-bloggs'}, testUtils.context.internal) + .then((user) => { + user.get('updated_by').should.be.a.String(); + user.get('created_by').should.be.a.String(); + user.get('created_at').should.be.a.Date(); + user.get('updated_at').should.be.a.Date(); + }); + }); + }); + describe('Edit', function () { let knexMock;