Skip to content

Commit 9b585d8

Browse files
committed
🐛 Fixed bookmark filtering in Welcome Emails editor
closes https://linear.app/ghost/issue/NY-1143 `useFilterableApi` filtered the response if data was already loaded, but not if it wasn't. This fixes that bug, which fixes the end-user bug.
1 parent 5bf370b commit 9b585d8

File tree

2 files changed

+49
-9
lines changed

2 files changed

+49
-9
lines changed

apps/admin-x-framework/src/hooks/use-filterable-api.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,16 @@ import {useRef} from 'react';
22
import {apiUrl, useFetchApi} from '../utils/api/fetch-api';
33
import {Meta} from '../utils/api/hooks';
44

5+
const filterData = <
6+
Data extends {[k in FilterKey]: string},
7+
FilterKey extends keyof Data
8+
>(data: Data[] = [], filterKey: FilterKey, input: string): Data[] => {
9+
if (!data || !input) {
10+
return data;
11+
}
12+
return data.filter(item => item[filterKey]?.toLowerCase().includes(input.toLowerCase()));
13+
};
14+
515
const escapeNqlString = (value: string) => {
616
return '\'' + value.replace(/'/g, '\\\'') + '\'';
717
};
@@ -26,7 +36,7 @@ const useFilterableApi = <
2636

2737
const loadData = async (input: string) => {
2838
if ((result.current.allLoaded || result.current.lastInput === input) && result.current.data) {
29-
return result.current.data.filter(item => item[filterKey]?.toLowerCase().includes(input.toLowerCase()));
39+
return filterData(result.current.data, filterKey, input);
3040
}
3141

3242
const response = await fetchApi<{meta?: Meta} & {[k in ResponseKey]: Data[]}>(apiUrl(path, {
@@ -38,7 +48,7 @@ const useFilterableApi = <
3848
result.current.allLoaded = !input && !response.meta?.pagination.next;
3949
result.current.lastInput = input;
4050

41-
return response[responseKey];
51+
return filterData(response[responseKey], filterKey, input);
4252
};
4353

4454
const loadInitialValues = async (values: string[], key: string) => {

apps/admin-x-framework/test/unit/hooks/use-filterable-api.test.ts

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,36 @@ describe('useFilterableApi', () => {
9999
});
100100
});
101101

102+
it('filters unfiltered API responses for non-empty search input', async () => {
103+
const mockData: TestData[] = [
104+
{id: '1', name: 'John Doe', email: 'john@example.com'},
105+
{id: '2', name: 'Jane Smith', email: 'jane@example.com'},
106+
{id: '3', name: 'johnny Appleseed', email: 'johnny@example.com'}
107+
];
108+
109+
mockFetchApi.mockResolvedValueOnce({
110+
users: mockData,
111+
meta: {pagination: {next: null}}
112+
});
113+
114+
const {result} = renderHook(() => useFilterableApi<TestData, 'users', 'name'>({
115+
path: '/users',
116+
filterKey: 'name',
117+
responseKey: 'users'
118+
}));
119+
120+
const data = await result.current.loadData('john');
121+
122+
expect(data).toEqual([
123+
{id: '1', name: 'John Doe', email: 'john@example.com'},
124+
{id: '3', name: 'johnny Appleseed', email: 'johnny@example.com'}
125+
]);
126+
expect(mockApiUrl).toHaveBeenCalledWith('/users', {
127+
filter: 'name:~\'john\'',
128+
limit: '20'
129+
});
130+
});
131+
102132
it('escapes single quotes in filter values', async () => {
103133
const mockData: TestData[] = [];
104134

@@ -314,7 +344,7 @@ describe('useFilterableApi', () => {
314344
const firstResult = await result.current.loadData('');
315345
expect(firstResult).toEqual(mockData);
316346

317-
const secondResult = await result.current.loadData('test');
347+
const secondResult = await result.current.loadData('john');
318348
expect(secondResult).toEqual(mockData);
319349
});
320350

@@ -425,7 +455,9 @@ describe('useFilterableApi', () => {
425455

426456
it('handles rapid filter changes correctly', async () => {
427457
const mockData: TestData[] = [
428-
{id: '1', name: 'Test Result'}
458+
{id: '1', name: 'test1'},
459+
{id: '2', name: 'test2'},
460+
{id: '3', name: 'test3'}
429461
];
430462

431463
mockFetchApi.mockResolvedValue({
@@ -444,12 +476,10 @@ describe('useFilterableApi', () => {
444476
const promise2 = result.current.loadData('test2');
445477
const promise3 = result.current.loadData('test3');
446478

447-
const results = await Promise.all([promise1, promise2, promise3]);
448-
449479
// All calls should return data
450-
results.forEach((resultItem) => {
451-
expect(resultItem).toEqual(mockData);
452-
});
480+
await expect(promise1).resolves.toEqual([{id: '1', name: 'test1'}]);
481+
await expect(promise2).resolves.toEqual([{id: '2', name: 'test2'}]);
482+
await expect(promise3).resolves.toEqual([{id: '3', name: 'test3'}]);
453483
});
454484
});
455485

0 commit comments

Comments
 (0)