Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-03-11 02:12:21 -05:00
Code Issues Activity

Fixed collection ownership if a published post status changes to featured

Browse source 
refs #9601

- implementation of resource listener updated
- if you define two collections: `featured:true` (1) and `page:false` (2) you can run into the following bug:
  - you create a published post (owned by (2))
  - you change the status to featured
  - still owned by (2), because the filter still matches (it's still not a static page)
- this adaption fixes the behaviour
- less smart logic, but less error prone
This commit is contained in:
kirrg001 2018-06-17 10:41:05 +02:00
parent 960ee2f20e
commit 1a79aac673
3 changed files with 25 additions and 63 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
core/server/services/url/UrlGenerator.js
View file

@ -143,6 +143,13 @@ class UrlGenerator {
/**
* I want to know if my resources changes.
* Register events of resource.
*
* If the owned resource get's updated, we simply release/free the resource and push it back to the queue.
* This is the easiest, less error prone implementation.
*
* Imagine you have two collections: `featured:true` and `page:false`.
* If a published post status get's featured and you have not explicitly defined `featured:false`, we wouldn't
* be able to figure out if this resource still belongs to me, because the filter still matches.
*/
_resourceListeners(resource) {
const onUpdate = (updatedResource) => {
@ -152,25 +159,17 @@ class UrlGenerator {
// 2. free resource, the url <-> resource connection no longer exists
updatedResource.release();
// 3. try to own the resource again
// Imagine you change `featured` to true and your filter excludes featured posts.
const isMine = this._try(updatedResource);
// 3. post has the change to get owned from a different collection again
debug('put back in queue', updatedResource.data.id);
// 4. if the resource is no longer mine, tell the others
// e.g. post -> page
// e.g. post is featured now
if (!isMine) {
debug('free, this is not mine anymore', updatedResource.data.id);
this.queue.start({
event: 'added',
action: 'added:' + resource.data.id,
eventData: {
id: resource.data.id,
type: this.router.getType()
}
});
}
this.queue.start({
event: 'added',
action: 'added:' + resource.data.id,
eventData: {
id: resource.data.id,
type: this.router.getType()
}
});
};
const onRemoved = (removedResource) => {

10
core/test/integration/services/url/UrlService_spec.js
View file

@ -360,19 +360,19 @@ describe('Unit: services/url/UrlService', function () {
}
};
router1.getFilter.returns('featured:false');
router1.getFilter.returns('featured:true');
router1.getType.returns('posts');
router1.getPermalinks.returns({
getValue: function () {
return '/collection/:year/:slug/';
return '/podcast/:slug/';
}
});
router2.getFilter.returns('featured:true');
router2.getFilter.returns('page:false');
router2.getType.returns('posts');
router2.getPermalinks.returns({
getValue: function () {
return '/podcast/:slug/';
return '/collection/:year/:slug/';
}
});
@ -435,7 +435,7 @@ describe('Unit: services/url/UrlService', function () {
it('getUrl', function () {
urlService.urlGenerators.forEach(function (generator) {
if (generator.router.getType() === 'posts' && generator.router.getFilter() === 'featured:false') {
if (generator.router.getType() === 'posts' && generator.router.getFilter() === 'page:false') {
generator.getUrls().length.should.eql(2);
}

43
core/test/unit/services/url/UrlGenerator_spec.js
View file

@ -273,7 +273,7 @@ describe('Unit: services/url/UrlGenerator', function () {
resource.addListener.calledTwice.should.be.true();
});
it('url has not changed', function () {
it('resource was updated', function () {
const urlGenerator = new UrlGenerator(router, queue, resources, urls);
sandbox.stub(urlGenerator, '_generateUrl').returns('/welcome/');
sandbox.stub(urlGenerator, '_try').returns(true);
@ -285,47 +285,10 @@ describe('Unit: services/url/UrlGenerator', function () {
urlGenerator._resourceListeners(resource);
resource.addListener.args[0][1](resource);
urlGenerator._try.called.should.be.true();
urlGenerator._try.called.should.be.false();
urls.removeResourceId.called.should.be.true();
resource.release.called.should.be.true();
queue.start.called.should.be.false();
});
it('url has changed, but is still mine', function () {
const urlGenerator = new UrlGenerator(router, queue, resources, urls);
sandbox.stub(urlGenerator, '_generateUrl').returns('/salute/');
sandbox.stub(urlGenerator, '_try').returns(true);
resource.data = {
id: 'object-id'
};
urlGenerator._resourceListeners(resource);
resource.addListener.args[0][1](resource);
urlGenerator._try.calledOnce.should.be.true();
urls.removeResourceId.calledOnce.should.be.true();
resource.release.calledOnce.should.be.true();
queue.start.called.should.be.false();
});
it('url has changed and is no longer mine (e.g. filter does not match anymore)', function () {
const urlGenerator = new UrlGenerator(router, queue, resources, urls);
sandbox.stub(urlGenerator, '_generateUrl').returns('/salute/');
sandbox.stub(urlGenerator, '_try').returns(false);
urlGenerator._resourceListeners(resource);
resource.data = {
id: 'object-id'
};
resource.addListener.args[0][1](resource);
urlGenerator._try.calledOnce.should.be.true();
urls.removeResourceId.calledOnce.should.be.true();
resource.release.calledOnce.should.be.true();
queue.start.calledOnce.should.be.true();
queue.start.called.should.be.true();
});
it('resource got removed', function () {