From 5dfee47fca11c8fb8eba4ffa0457d234ce71bd34 Mon Sep 17 00:00:00 2001 From: Sag Date: Thu, 27 Jun 2024 17:47:26 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20default=20recipients=20s?= =?UTF-8?q?etting=20not=20showing=20label=20filters=20(#20480)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes https://linear.app/tryghost/issue/SLO-171 - problem: when the Default Recipient setting is set to "Specific people" and is filtered by a label, we were not able to render the label correctly - cause: during the rendering, we look for labels by `id`, but they're stored by `slug` in the database setting `editor_default_email_recipients_filter` - solution: allow to look by the relevant key, by introducing a programmatic `key` to search for Before the fix: https://github.com/TryGhost/Ghost/assets/6225080/aed5fc31-6409-4986-aafe-557073c7f355 After the fix: https://github.com/TryGhost/Ghost/assets/6225080/f35b2607-5f22-42be-b1bb-92f35ccc9ab7 --- .../src/hooks/useFilterableApi.ts | 39 ++++++++++--------- .../email/useDefaultRecipientsOptions.tsx | 10 ++--- .../email/defaultRecipients.test.ts | 29 ++++++++++++++ 3 files changed, 54 insertions(+), 24 deletions(-) diff --git a/apps/admin-x-framework/src/hooks/useFilterableApi.ts b/apps/admin-x-framework/src/hooks/useFilterableApi.ts index 6b738e8cd9..61b568916a 100644 --- a/apps/admin-x-framework/src/hooks/useFilterableApi.ts +++ b/apps/admin-x-framework/src/hooks/useFilterableApi.ts @@ -7,7 +7,7 @@ const escapeNqlString = (value: string) => { }; const useFilterableApi = < - Data extends {id: string} & {[Key in FilterKey]: string}, + Data extends {id: string} & {[k in FilterKey]: string} & {[k: string]: unknown}, ResponseKey extends string = string, FilterKey extends string = string >({path, filterKey, responseKey, limit = 20}: { @@ -41,26 +41,27 @@ const useFilterableApi = < return response[responseKey]; }; + const loadInitialValues = async (values: string[], key: string) => { + await loadData(''); + + const data = [...(result.current.data || [])]; + const missingValues = values.filter(value => !result.current.data?.find(item => item[key] === value)); + + if (missingValues.length) { + const additionalData = await fetchApi<{meta?: Meta} & {[k in ResponseKey]: Data[]}>(apiUrl(path, { + filter: `${key}:[${missingValues.join(',')}]`, + limit: 'all' + })); + + data.push(...additionalData[responseKey]); + } + + return values.map(value => data.find(item => item[key] === value)!); + }; + return { loadData, - - loadInitialValues: async (ids: string[]) => { - await loadData(''); - - const data = [...(result.current.data || [])]; - const missingIds = ids.filter(id => !result.current.data?.find(({id: dataId}) => dataId === id)); - - if (missingIds.length) { - const additionalData = await fetchApi<{meta?: Meta} & {[k in ResponseKey]: Data[]}>(apiUrl(path, { - filter: `id:[${missingIds.join(',')}]`, - limit: 'all' - })); - - data.push(...additionalData[responseKey]); - } - - return ids.map(id => data.find(({id: dataId}) => dataId === id)!); - } + loadInitialValues }; }; diff --git a/apps/admin-x-settings/src/components/settings/email/useDefaultRecipientsOptions.tsx b/apps/admin-x-settings/src/components/settings/email/useDefaultRecipientsOptions.tsx index 731b8dc645..8ee3e4af38 100644 --- a/apps/admin-x-settings/src/components/settings/email/useDefaultRecipientsOptions.tsx +++ b/apps/admin-x-settings/src/components/settings/email/useDefaultRecipientsOptions.tsx @@ -62,11 +62,11 @@ const useDefaultRecipientsOptions = (selectedOption: string, defaultEmailRecipie const initSelectedSegments = async () => { const filters = defaultEmailRecipientsFilter?.split(',') || []; - const tierIds: string[] = [], labelIds: string[] = [], offerIds: string[] = []; + const tierIds: string[] = [], labelSlugs: string[] = [], offerIds: string[] = []; for (const filter of filters) { if (filter.startsWith('label:')) { - labelIds.push(filter.replace('label:', '')); + labelSlugs.push(filter.replace('label:', '')); } else if (filter.startsWith('offer_redemptions:')) { offerIds.push(filter.replace('offer_redemptions:', '')); } else if (isObjectId(filter)) { @@ -75,9 +75,9 @@ const useDefaultRecipientsOptions = (selectedOption: string, defaultEmailRecipie } const options = await Promise.all([ - tiers.loadInitialValues(tierIds).then(data => data.map(tierOption)), - labels.loadInitialValues(labelIds).then(data => data.map(labelOption)), - offers.loadInitialValues(offerIds).then(data => data.map(offerOption)) + tiers.loadInitialValues(tierIds, 'id').then(data => data.map(tierOption)), + labels.loadInitialValues(labelSlugs, 'slug').then(data => data.map(labelOption)), + offers.loadInitialValues(offerIds, 'id').then(data => data.map(offerOption)) ]).then(results => results.flat()); setSelectedSegments(filters.map(filter => options.find(option => option.value === filter)!)); diff --git a/apps/admin-x-settings/test/acceptance/email/defaultRecipients.test.ts b/apps/admin-x-settings/test/acceptance/email/defaultRecipients.test.ts index 26751488e2..565c3dd9d8 100644 --- a/apps/admin-x-settings/test/acceptance/email/defaultRecipients.test.ts +++ b/apps/admin-x-settings/test/acceptance/email/defaultRecipients.test.ts @@ -104,4 +104,33 @@ test.describe('Default recipient settings', async () => { ] }); }); + + test('renders existing default recipients filters correctly', async ({page}) => { + await mockApi({page, requests: { + ...globalDataRequests, + browseTiers: {method: 'GET', path: '/tiers/?filter=&limit=20', response: responseFixtures.tiers}, + browseLabels: {method: 'GET', path: '/labels/?filter=&limit=20', response: responseFixtures.labels}, + browseOffers: {method: 'GET', path: '/offers/?filter=&limit=20', response: responseFixtures.offers}, + browseSettings: {...globalDataRequests.browseSettings, response: updatedSettingsResponse([ + { + key: 'editor_default_email_recipients', + value: 'filter' + }, + { + key: 'editor_default_email_recipients_filter', + value: '645453f4d254799990dd0e22,label:first-label,offer_redemptions:6487ea6464fca78ec2fff5fe' + } + ])} + }}); + + await page.goto('/'); + + const section = page.getByTestId('default-recipients'); + await section.getByRole('button', {name: 'Edit'}).click(); + + await expect(section.getByText('Specific people')).toHaveCount(1); + await expect(section.getByText('Basic Supporter')).toHaveCount(1); + await expect(section.getByText('first-label')).toHaveCount(1); + await expect(section.getByText('First offer')).toHaveCount(1); + }); });