TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-11-27 10:42:45 +03:00
Code Issues Projects Releases Wiki Activity

Fixed route update error on attach/detach events

Browse Source 
refs https://github.com/TryGhost/Toolbox/issues/503

- There was an error thrown due to empty "model._changed" field
- When attached or detached events (e.g. tag.attached) are sent through, their models do not contain any _changed properties. This was taken into account when checking for route related resource changes
This commit is contained in:
Naz 2023-01-24 12:14:05 +08:00
parent d394ce6638
commit 62e3caba2c
No known key found for this signature in database
2 changed files with 77 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
ghost/core/core/server/services/url/Resources.js
View File

@ -274,6 +274,21 @@ class Resources {
}
}
/**
*
* @param {Object} model resource model
* @returns
*/
_containsRoutingAffectingChanges(model, ignoredProperties) {
if (model._changed && Object.keys(model._changed).length) {
return _.difference(Object.keys(model._changed), ignoredProperties).length !== 0;
}
// NOTE: returning true here as "_changed" property might not be available on attached/detached events
// assuming there were route affecting changes by default
return true;
}
/**
* @description Listener for "model updated" event.
*
@ -299,23 +314,20 @@ class Resources {
const resourceConfig = _.find(this.resourcesConfig, {type: type});
if (resourceConfig && Object.keys(model._changed).length) {
const ignoredProperties = [...resourceConfig.modelOptions.exclude, 'updated_at'];
const containsRouteAffectingChanges = _.difference(Object.keys(model._changed), ignoredProperties).length !== 0;
// NOTE: check if any of the route-related fields were changed and only proceed if so
const ignoredProperties = [...resourceConfig.modelOptions.exclude, 'updated_at'];
if (!this._containsRoutingAffectingChanges(model, ignoredProperties)) {
const cachedResource = this.getByIdAndType(type, model.id);
if (!containsRouteAffectingChanges) {
const cachedResource = this.getByIdAndType(type, model.id);
if (cachedResource && Object.keys(model._changed).includes('updated_at')) {
DomainEvents.dispatch(URLResourceUpdatedEvent.create(Object.assign(cachedResource.data, {
resourceType: cachedResource.config.type,
updated_at: model._changed.updated_at
})));
}
debug('skipping _onResourceUpdated because only non-route-related properties changed');
return false;
if (cachedResource && model._changed && Object.keys(model._changed).includes('updated_at')) {
DomainEvents.dispatch(URLResourceUpdatedEvent.create(Object.assign(cachedResource.data, {
resourceType: cachedResource.config.type,
updated_at: model._changed.updated_at
})));
}
debug('skipping _onResourceUpdated because only non-route-related properties changed');
return false;
}
// NOTE: synchronous handling for post and pages so that their URL is available without a delay

67
ghost/core/test/unit/server/services/url/Resources.test.js
View File

@ -3,24 +3,21 @@ const assert = require('assert');
const Resources = require('../../../../../core/server/services/url/Resources');
describe('Unit: services/url/Resources', function () {
const resources = new Resources({
resourcesConfig: [{
type: 'posts',
modelOptions: {
modelName: 'Post',
exclude: [
'title',
'plaintext'
]
}
}]
});
describe('_onResourceUpdated', function () {
it('does not start the queue when non-routing properties were changed', async function () {
const resources = new Resources({
resourcesConfig: [{
type: 'post',
modelOptions: {
modelName: 'Post',
exclude: [
'title',
'mobiledoc',
'lexical',
'html',
'plaintext'
]
}
}]
});
const postModelMock = {
_changed: {
title: 'New Title',
@ -28,9 +25,45 @@ describe('Unit: services/url/Resources', function () {
}
};
const updated = await resources._onResourceUpdated('post', postModelMock);
const updated = await resources._onResourceUpdated('posts', postModelMock);
assert.equal(updated, false);
});
});
describe('_containsRoutingAffectingChanges', function (){
it('returns true when routing affecting properties were changed', async function () {
const postModelMock = {
_changed: {
title: 'New Title',
updated_at: new Date()
}
};
const containsRoutingAffectingChanges = resources._containsRoutingAffectingChanges(postModelMock, ['title', 'updated_at']);
assert.equal(containsRoutingAffectingChanges, false);
});
it('returns false when routing affecting properties were not changed', async function () {
const postModelMock = {
_changed: {
title: 'New Title',
slug: 'new-slug'
}
};
const containsRoutingAffectingChanges = resources._containsRoutingAffectingChanges(postModelMock, ['title', 'updated_at']);
assert.equal(containsRoutingAffectingChanges, true);
});
it('returns true when model does not contain changes', async function () {
const postModelMock = {};
const containsRoutingAffectingChanges = resources._containsRoutingAffectingChanges(postModelMock, ['title', 'updated_at']);
assert.equal(containsRoutingAffectingChanges, true);
});
});
});