0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-10 23:36:14 -05:00

Fetch relations by default when insert/updating posts (#9568)

no issue

- required for model events
- otherwise you won't receive a full data set
  - in worst case you have to re-fetch the post
- required for the url service
  - the url service always needs relations (authors,tags) to be able to generate the url properly

@IMPORTANT
- no API change, we still return what you are asking for
  - we first edit/add the resource
  - then we fetch the data with the API options
  - @TODO: this can be optimised and will improve performance
    	   picking/selecting it from the insert/update response
- this is an internal change
This commit is contained in:
Katharina Irrgang 2018-04-15 12:12:20 +02:00 committed by GitHub
parent 5d21ce0644
commit 7273786459
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 459 additions and 3 deletions

View file

@ -604,6 +604,10 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
permittedOptions = _.union(permittedOptions, extraAllowedProperties);
options = _.pick(options, permittedOptions);
if (this.defaultRelations) {
options = this.defaultRelations(methodName, options);
}
return options;
},
@ -643,6 +647,8 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
* information about the request (page, limit), along with the
* info needed for pagination (pages, total).
*
* @TODO: This model function does return JSON O_O.
*
* **response:**
*
* {

View file

@ -563,6 +563,20 @@ Post = ghostBookshelf.Model.extend({
return options;
},
/**
* We have to ensure consistency. If you listen on model events (e.g. `post.published`), you can expect that you always
* receive all fields including relations. Otherwise you can't rely on a consistent flow. And we want to avoid
* that event listeners have to re-fetch a resource. This function is used in the context of inserting
* and updating resources. We won't return the relations by default for now.
*/
defaultRelations: function defaultRelations(methodName, options) {
if (['edit', 'add'].indexOf(methodName) !== -1) {
options.withRelated = _.union(this.prototype.relationships, options.withRelated || []);
}
return options;
},
/**
* Manually add 'tags' attribute since it's not in the schema and call parent.
*
@ -611,7 +625,10 @@ Post = ghostBookshelf.Model.extend({
return ghostBookshelf.Model.edit.call(this, data, options)
.then((post) => {
return this.findOne({status: 'all', id: options.id}, options)
return this.findOne({
status: 'all',
id: options.id
}, _.merge({transacting: options.transacting}, unfilteredOptions))
.then((found) => {
if (found) {
// Pass along the updated attributes for checking status changes
@ -643,7 +660,10 @@ Post = ghostBookshelf.Model.extend({
const addPost = (() => {
return ghostBookshelf.Model.add.call(this, data, options)
.then((post) => {
return this.findOne({status: 'all', id: post.id}, options);
return this.findOne({
status: 'all',
id: post.id
}, _.merge({transacting: options.transacting}, unfilteredOptions));
});
});

View file

@ -1,4 +1,5 @@
'use strict';
/* eslint no-invalid-this:0 */
const should = require('should'), // jshint ignore:line
sinon = require('sinon'),
@ -6,6 +7,7 @@ const should = require('should'), // jshint ignore:line
testUtils = require('../../utils'),
knex = require('../../../server/data/db').knex,
settingsCache = require('../../../server/services/settings/cache'),
schema = require('../../../server/data/schema'),
models = require('../../../server/models'),
common = require('../../../server/lib/common'),
security = require('../../../server/lib/security'),
@ -56,7 +58,227 @@ describe('Unit: models/post', function () {
});
});
describe('Edit', function () {
describe('add', function () {
describe('ensure full set of data for model events', function () {
it('default', function () {
const events = {
post: []
};
sandbox.stub(models.Post.prototype, 'emitChange').callsFake(function (event) {
events.post.push({event: event, data: this.toJSON()});
});
return models.Post.add({
title: 'My beautiful title.',
tags: [{
name: 'my-tag'
}]
}, testUtils.context.editor)
.then((post) => {
post.get('title').should.eql('My beautiful title.');
post = post.toJSON();
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(post.hasOwnProperty(key));
});
should.not.exist(post.authors);
should.not.exist(post.primary_author);
should.not.exist(post.tags);
should.not.exist(post.primary_tag);
events.post[0].event.should.eql('added');
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(events.post[0].data.hasOwnProperty(key));
});
should.exist(events.post[0].data.authors);
should.exist(events.post[0].data.primary_author);
should.exist(events.post[0].data.tags);
should.exist(events.post[0].data.primary_tag);
});
});
it('use `withRelated=tags`', function () {
const events = {
post: []
};
sandbox.stub(models.Post.prototype, 'emitChange').callsFake(function (event) {
events.post.push({event: event, data: this.toJSON()});
});
return models.Post.add({
title: 'My beautiful title.',
tags: [{
name: 'my-tag'
}]
}, _.merge({
withRelated: ['tags']
}, testUtils.context.editor))
.then((post) => {
post.get('title').should.eql('My beautiful title.');
post = post.toJSON();
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(post.hasOwnProperty(key));
});
should.not.exist(post.authors);
should.not.exist(post.primary_author);
should.exist(post.tags);
should.exist(post.primary_tag);
events.post[0].event.should.eql('added');
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(events.post[0].data.hasOwnProperty(key));
});
should.exist(events.post[0].data.authors);
should.exist(events.post[0].data.primary_author);
should.exist(events.post[0].data.tags);
should.exist(events.post[0].data.primary_tag);
});
});
it('use `withRelated=tags,authors`', function () {
const events = {
post: []
};
sandbox.stub(models.Post.prototype, 'emitChange').callsFake(function (event) {
events.post.push({event: event, data: this.toJSON()});
});
return models.Post.add({
title: 'My beautiful title.',
tags: [{
name: 'my-tag'
}]
}, _.merge({
withRelated: ['tags', 'authors']
}, testUtils.context.editor))
.then((post) => {
post.get('title').should.eql('My beautiful title.');
post = post.toJSON();
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(post.hasOwnProperty(key));
});
should.exist(post.authors);
should.exist(post.primary_author);
should.exist(post.tags);
should.exist(post.primary_tag);
events.post[0].event.should.eql('added');
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(events.post[0].data.hasOwnProperty(key));
});
should.exist(events.post[0].data.authors);
should.exist(events.post[0].data.primary_author);
should.exist(events.post[0].data.tags);
should.exist(events.post[0].data.primary_tag);
});
});
it('use `columns=title`', function () {
const events = {
post: []
};
sandbox.stub(models.Post.prototype, 'emitChange').callsFake(function (event) {
events.post.push({event: event, data: this.toJSON()});
});
return models.Post.add({
title: 'My beautiful title.',
tags: [{
name: 'my-tag'
}]
}, _.merge({
columns: ['title']
}, testUtils.context.editor))
.then((post) => {
post.get('title').should.eql('My beautiful title.');
post = post.toJSON();
_.each(_.keys(_.omit(schema.tables.posts, ['title', 'id'])), (key) => {
should.not.exist(post[key]);
});
should.exist(post.id);
should.exist(post.title);
should.not.exist(post.authors);
should.not.exist(post.primary_author);
should.not.exist(post.tags);
should.not.exist(post.primary_tag);
events.post[0].event.should.eql('added');
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(events.post[0].data.hasOwnProperty(key));
});
should.exist(events.post[0].data.authors);
should.exist(events.post[0].data.primary_author);
should.exist(events.post[0].data.tags);
should.exist(events.post[0].data.primary_tag);
});
});
it('use `formats=mobiledoc`', function () {
const events = {
post: []
};
sandbox.stub(models.Post.prototype, 'emitChange').callsFake(function (event) {
events.post.push({event: event, data: this.toJSON()});
});
return models.Post.add({
title: 'My beautiful title.',
tags: [{
name: 'my-tag'
}]
}, _.merge({
formats: ['mobiledoc']
}, testUtils.context.editor))
.then((post) => {
post.get('title').should.eql('My beautiful title.');
post = post.toJSON();
_.each(_.keys(_.omit(schema.tables.posts, ['html', 'amp', 'plaintext'])), (key) => {
should.exist(post.hasOwnProperty(key));
});
should.not.exist(post.authors);
should.not.exist(post.primary_author);
should.not.exist(post.tags);
should.not.exist(post.primary_tag);
events.post[0].event.should.eql('added');
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(events.post[0].data.hasOwnProperty(key));
});
should.exist(events.post[0].data.authors);
should.exist(events.post[0].data.primary_author);
should.exist(events.post[0].data.tags);
should.exist(events.post[0].data.primary_tag);
});
});
});
});
describe('edit', function () {
it('update post, relation has not changed', function () {
const events = {
post: [],
@ -133,6 +355,213 @@ describe('Unit: models/post', function () {
post.get('custom_excerpt').should.eql('');
});
});
describe('ensure full set of data for model events', function () {
it('default', function () {
const events = {
post: []
};
sandbox.stub(models.Post.prototype, 'emitChange').callsFake(function (event) {
events.post.push({event: event, data: this.toJSON()});
});
return models.Post.edit({
title: 'My beautiful title.'
}, _.merge({id: testUtils.DataGenerator.forKnex.posts[3].id}, testUtils.context.editor))
.then((post) => {
post.get('title').should.eql('My beautiful title.');
post = post.toJSON();
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(post.hasOwnProperty(key));
});
should.not.exist(post.authors);
should.not.exist(post.primary_author);
should.not.exist(post.tags);
should.not.exist(post.primary_tag);
events.post[0].event.should.eql('edited');
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(events.post[0].data.hasOwnProperty(key));
});
should.exist(events.post[0].data.authors);
should.exist(events.post[0].data.primary_author);
should.exist(events.post[0].data.tags);
should.exist(events.post[0].data.primary_tag);
});
});
it('use `withRelated=tags`', function () {
const events = {
post: []
};
sandbox.stub(models.Post.prototype, 'emitChange').callsFake(function (event) {
events.post.push({event: event, data: this.toJSON()});
});
return models.Post.edit({
title: 'My beautiful title.'
}, _.merge({
id: testUtils.DataGenerator.forKnex.posts[3].id,
withRelated: ['tags']
}, testUtils.context.editor))
.then((post) => {
post.get('title').should.eql('My beautiful title.');
post = post.toJSON();
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(post.hasOwnProperty(key));
});
should.not.exist(post.authors);
should.not.exist(post.primary_author);
should.exist(post.tags);
should.exist(post.primary_tag);
events.post[0].event.should.eql('edited');
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(events.post[0].data.hasOwnProperty(key));
});
should.exist(events.post[0].data.authors);
should.exist(events.post[0].data.primary_author);
should.exist(events.post[0].data.tags);
should.exist(events.post[0].data.primary_tag);
});
});
it('use `withRelated=tags,authors`', function () {
const events = {
post: []
};
sandbox.stub(models.Post.prototype, 'emitChange').callsFake(function (event) {
events.post.push({event: event, data: this.toJSON()});
});
return models.Post.edit({
title: 'My beautiful title.'
}, _.merge({
id: testUtils.DataGenerator.forKnex.posts[3].id,
withRelated: ['tags', 'authors']
}, testUtils.context.editor))
.then((post) => {
post.get('title').should.eql('My beautiful title.');
post = post.toJSON();
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(post.hasOwnProperty(key));
});
should.exist(post.authors);
should.exist(post.primary_author);
should.exist(post.tags);
should.exist(post.primary_tag);
events.post[0].event.should.eql('edited');
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(events.post[0].data.hasOwnProperty(key));
});
should.exist(events.post[0].data.authors);
should.exist(events.post[0].data.primary_author);
should.exist(events.post[0].data.tags);
should.exist(events.post[0].data.primary_tag);
});
});
it('use `columns=title`', function () {
const events = {
post: []
};
sandbox.stub(models.Post.prototype, 'emitChange').callsFake(function (event) {
events.post.push({event: event, data: this.toJSON()});
});
return models.Post.edit({
title: 'My beautiful title.'
}, _.merge({
id: testUtils.DataGenerator.forKnex.posts[3].id,
columns: ['title']
}, testUtils.context.editor))
.then((post) => {
post.get('title').should.eql('My beautiful title.');
post = post.toJSON();
_.each(_.keys(_.omit(schema.tables.posts, ['title', 'id'])), (key) => {
should.not.exist(post[key]);
});
should.exist(post.id);
should.exist(post.title);
should.not.exist(post.authors);
should.not.exist(post.primary_author);
should.not.exist(post.tags);
should.not.exist(post.primary_tag);
events.post[0].event.should.eql('edited');
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(events.post[0].data.hasOwnProperty(key));
});
should.exist(events.post[0].data.authors);
should.exist(events.post[0].data.primary_author);
should.exist(events.post[0].data.tags);
should.exist(events.post[0].data.primary_tag);
});
});
it('use `formats=mobiledoc`', function () {
const events = {
post: []
};
sandbox.stub(models.Post.prototype, 'emitChange').callsFake(function (event) {
events.post.push({event: event, data: this.toJSON()});
});
return models.Post.edit({
title: 'My beautiful title.'
}, _.merge({
id: testUtils.DataGenerator.forKnex.posts[3].id,
formats: ['mobiledoc']
}, testUtils.context.editor))
.then((post) => {
post.get('title').should.eql('My beautiful title.');
post = post.toJSON();
_.each(_.keys(_.omit(schema.tables.posts, ['html', 'amp', 'plaintext'])), (key) => {
should.exist(post.hasOwnProperty(key));
});
should.not.exist(post.authors);
should.not.exist(post.primary_author);
should.not.exist(post.tags);
should.not.exist(post.primary_tag);
events.post[0].event.should.eql('edited');
_.each(_.keys(_.omit(schema.tables.posts, ['mobiledoc', 'amp', 'plaintext'])), (key) => {
should.exist(events.post[0].data.hasOwnProperty(key));
});
should.exist(events.post[0].data.authors);
should.exist(events.post[0].data.primary_author);
should.exist(events.post[0].data.tags);
should.exist(events.post[0].data.primary_tag);
});
});
});
});
describe('Relations', function () {

View file

@ -432,6 +432,7 @@ DataGenerator.forKnex = (function () {
uuid: uuid.v4(),
title: 'title',
status: 'published',
feature_image: null,
featured: false,
page: false,
author_id: DataGenerator.Content.users[0].id,