From ad62cd48220759243445df4e56aeff7bf89d6e3b Mon Sep 17 00:00:00 2001 From: "Fabien \"egg\" O'Carroll" Date: Mon, 7 Aug 2023 16:05:33 +0100 Subject: [PATCH] Removed `edit` method from Collection We want to keep Entities as basic as possible, particularly when setting property, so that each property can handle its own validation. --- ghost/collections/src/Collection.ts | 59 ++++++------- ghost/collections/src/CollectionsService.ts | 21 ++++- ghost/collections/test/Collection.test.ts | 97 +++++++++++---------- 3 files changed, 97 insertions(+), 80 deletions(-) diff --git a/ghost/collections/src/Collection.ts b/ghost/collections/src/Collection.ts index ea44ee5a79..155dda09b1 100644 --- a/ghost/collections/src/Collection.ts +++ b/ghost/collections/src/Collection.ts @@ -40,7 +40,31 @@ export class Collection { } description: string; type: 'manual' | 'automatic'; - filter: string | null; + _filter: string | null; + get filter() { + return this._filter; + } + set filter(value) { + // Cannot change the filter of these collections + if (this.slug === 'latest' || this.slug === 'featured') { + return; + } + if (this.type === 'manual') { + if (value !== null) { + throw new ValidationError({ + message: tpl(messages.invalidFilterProvided.message), + context: tpl(messages.invalidFilterProvided.context) + }); + } + } else { + if (value === null || value === '') { + throw new ValidationError({ + message: tpl(messages.invalidFilterProvided.message), + context: tpl(messages.invalidFilterProvided.context) + }); + } + } + } featureImage: string | null; createdAt: Date; updatedAt: Date; @@ -64,37 +88,6 @@ export class Collection { } } - public async edit(data: Partial, uniqueChecker: UniqueChecker) { - if (this.type === 'automatic' && this.slug !== 'latest' && (data.filter === null || data.filter === '')) { - throw new ValidationError({ - message: tpl(messages.invalidFilterProvided.message), - context: tpl(messages.invalidFilterProvided.context) - }); - } - - if (data.title !== undefined) { - this.title = data.title; - } - - if (data.slug !== undefined) { - await this.setSlug(data.slug, uniqueChecker); - } - - if (data.description !== undefined) { - this.description = data.description; - } - - if (data.filter !== undefined) { - this.filter = data.filter; - } - - if (data.featureImage !== undefined) { - this.featureImage = data.featureImage; - } - - return this; - } - postMatchesFilter(post: CollectionPost) { const filterNql = nql(this.filter, { expansions: postExpansions @@ -148,7 +141,7 @@ export class Collection { this._slug = data.slug; this.description = data.description; this.type = data.type; - this.filter = data.filter; + this._filter = data.filter; this.featureImage = data.featureImage; this.createdAt = data.createdAt; this.updatedAt = data.updatedAt; diff --git a/ghost/collections/src/CollectionsService.ts b/ghost/collections/src/CollectionsService.ts index 20705a96c8..a126836c0f 100644 --- a/ghost/collections/src/CollectionsService.ts +++ b/ghost/collections/src/CollectionsService.ts @@ -421,7 +421,26 @@ export class CollectionsService { } const collectionData = this.fromDTO(data); - await collection.edit(collectionData, this.uniqueChecker); + + if (collectionData.title) { + collection.title = collectionData.title; + } + + if (data.slug !== undefined) { + await collection.setSlug(data.slug, this.uniqueChecker); + } + + if (data.description !== undefined) { + collection.description = data.description; + } + + if (data.filter !== undefined) { + collection.filter = data.filter; + } + + if (data.feature_image !== undefined) { + collection.featureImage = data.feature_image; + } if (collection.type === 'manual' && data.posts) { for (const post of data.posts) { diff --git a/ghost/collections/test/Collection.test.ts b/ghost/collections/test/Collection.test.ts index ee633f5c9f..053064babb 100644 --- a/ghost/collections/test/Collection.test.ts +++ b/ghost/collections/test/Collection.test.ts @@ -172,59 +172,64 @@ describe('Collection', function () { }); }); - describe('edit', function () { - it('Can edit Collection values', async function () { - const collection = await Collection.create({ - slug: 'test-collection', - title: 'Testing edits', - type: 'automatic', - filter: 'featured:true' - }); - - assert.equal(collection.title, 'Testing edits'); - - await collection.edit({ - title: 'Edited title', - slug: 'edited-slug' - }, uniqueChecker); - - assert.equal(collection.title, 'Edited title'); - assert.equal(collection.slug, 'edited-slug'); + it('Can edit Collection values', async function () { + const collection = await Collection.create({ + slug: 'test-collection', + title: 'Testing edits', + type: 'automatic', + filter: 'featured:true' }); - it('Throws when the collection filter is empty', async function () { - const collection = await Collection.create({ - title: 'Testing edits', - type: 'automatic', - filter: 'featured:true' - }); + assert.equal(collection.title, 'Testing edits'); - assert.rejects(async () => { - await collection.edit({ - filter: null - }, uniqueChecker); - }, (err: any) => { - assert.equal(err.message, 'Invalid filter provided for automatic Collection', 'Error message should match'); - assert.equal(err.context, 'Automatic type of collection should always have a filter value', 'Error message should match'); - return true; - }); + collection.title = 'Edited title'; + await collection.setSlug('edited-slug', uniqueChecker); + + assert.equal(collection.title, 'Edited title'); + assert.equal(collection.slug, 'edited-slug'); + }); + + it('Throws when the collection filter is empty', async function () { + const collection = await Collection.create({ + title: 'Testing edits', + type: 'automatic', + filter: 'featured:true' }); - it('Does not throw when collection filter is empty for automatic "latest" collection', async function (){ - const collection = await Collection.create({ - title: 'Latest', - slug: 'latest', - type: 'automatic', - filter: '' - }); + assert.throws(() => { + collection.filter = null; + }, (err: any) => { + assert.equal(err.message, 'Invalid filter provided for automatic Collection', 'Error message should match'); + assert.equal(err.context, 'Automatic type of collection should always have a filter value', 'Error message should match'); + return true; + }); + }); - const editedCollection = await collection.edit({ - title: 'Edited latest', - filter: '' - }, uniqueChecker); + it('Does not throw when collection filter is empty for automatic "latest" collection', async function (){ + const collection = await Collection.create({ + title: 'Latest', + slug: 'latest', + type: 'automatic', + filter: '' + }); - assert.equal(editedCollection.title, 'Edited latest'); - assert.equal(editedCollection.filter, ''); + collection.filter = ''; + }); + + it('throws when trying to set filter on a manual collection', async function () { + const collection = await Collection.create({ + title: 'Testing Manual Filter', + slug: 'testing-manual-filter', + type: 'manual', + filter: null + }); + + assert.throws(() => { + collection.filter = 'awesome:true'; + }, (err: any) => { + assert.equal(err.message, 'Invalid filter provided for automatic Collection', 'Error message should match'); + assert.equal(err.context, 'Automatic type of collection should always have a filter value', 'Error message should match'); + return true; }); });