Skip to content

Commit

Permalink
🐛 Fixed default recipients setting not showing label filters
Browse files Browse the repository at this point in the history
fixes https://linear.app/tryghost/issue/SLO-171

- when default recipients are filtered by a label, we store the filtered label in the `label:slug` format
- however, when loading existing label filters, we were fetching labels by ID, instead of slug
- with this change, we allow fetching by the relevant key
  • Loading branch information
sagzy committed Jun 27, 2024
1 parent 430a2ca commit 40332d9
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 21 deletions.
33 changes: 17 additions & 16 deletions apps/admin-x-framework/src/hooks/useFilterableApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}: {
Expand Down Expand Up @@ -41,26 +41,27 @@ const useFilterableApi = <
return response[responseKey];
};

return {
loadData,
const loadInitialValues = async (values: string[], key: string) => {
await loadData('');

loadInitialValues: async (ids: string[]) => {
await loadData('');
const data = [...(result.current.data || [])];
const missingValues = values.filter(value => !result.current.data?.find(item => item[key] === value));

const data = [...(result.current.data || [])];
const missingIds = ids.filter(id => !result.current.data?.find(({id: dataId}) => dataId === id));
if (missingValues.length) {
const additionalData = await fetchApi<{meta?: Meta} & {[k in ResponseKey]: Data[]}>(apiUrl(path, {
filter: `${key}:[${missingValues.join(',')}]`,
limit: 'all'
}));

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]);
}

data.push(...additionalData[responseKey]);
}
return values.map(value => data.find(item => item[key] === value)!);
};

return ids.map(id => data.find(({id: dataId}) => dataId === id)!);
}
return {
loadData,
loadInitialValues
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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)!));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

0 comments on commit 40332d9

Please sign in to comment.