From 004e4895923120b9cac9589ebbc0978b16c65ff5 Mon Sep 17 00:00:00 2001 From: vineyardbovines Date: Tue, 1 Sep 2026 18:39:28 -0400 Subject: [PATCH] Test and harden starter pack opt-outs --- src/screens/StarterPack/StarterPackScreen.tsx | 4 +- .../queries/__tests__/starter-packs.test.tsx | 170 ++++++++++++++++++ src/state/queries/starter-packs.ts | 35 +++- 3 files changed, 199 insertions(+), 10 deletions(-) create mode 100644 src/state/queries/__tests__/starter-packs.test.tsx diff --git a/src/screens/StarterPack/StarterPackScreen.tsx b/src/screens/StarterPack/StarterPackScreen.tsx index 111272c3b3..7d9a11d358 100644 --- a/src/screens/StarterPack/StarterPackScreen.tsx +++ b/src/screens/StarterPack/StarterPackScreen.tsx @@ -753,7 +753,9 @@ function OverflowMenu({ {referenceListOptOut ? ( - You will appear in this starter pack again. + + You will be eligible to appear in this starter pack again. + ) : ( You will no longer appear in this starter pack. The creator can diff --git a/src/state/queries/__tests__/starter-packs.test.tsx b/src/state/queries/__tests__/starter-packs.test.tsx new file mode 100644 index 0000000000..5f958f7ed2 --- /dev/null +++ b/src/state/queries/__tests__/starter-packs.test.tsx @@ -0,0 +1,170 @@ +import {type PropsWithChildren} from 'react' +import { + notifyManager, + QueryClient, + QueryClientProvider, +} from '@tanstack/react-query' +import {act, renderHook, waitFor} from '@testing-library/react-native' + +import {until} from '#/lib/async/until' +import {useAppviewClient, usePdsClient} from '#/state/session' +import {type app} from '#/lexicons' +import {useReferenceListOptOutMutation} from '../starter-packs' + +jest.mock('#/lib/async/until', () => ({ + until: jest.fn(), +})) + +jest.mock('#/state/session', () => ({ + useAppviewClient: jest.fn(), + usePdsClient: jest.fn(), +})) + +const starterPack = { + uri: 'at://did:plc:creator/app.bsky.graph.starterpack/pack', + list: { + uri: 'at://did:plc:creator/app.bsky.graph.list/list', + viewer: {}, + }, +} as app.bsky.graph.defs.StarterPackView + +const queryKey = ['starter-pack', 'did:plc:creator', 'pack'] +const createdOptOut = + 'at://did:plc:viewer/app.bsky.graph.referencelistoptout/created' +const indexedOptOut = + 'at://did:plc:viewer/app.bsky.graph.referencelistoptout/indexed' + +function setup() { + const queryClient = new QueryClient({ + defaultOptions: { + queries: {gcTime: Infinity, retry: false}, + mutations: {gcTime: Infinity, retry: false}, + }, + }) + const pdsClient = { + assertDid: 'did:plc:viewer', + create: jest.fn(), + delete: jest.fn(), + } + const appviewClient = {call: jest.fn()} + const onError = jest.fn() + + jest.mocked(usePdsClient).mockReturnValue(pdsClient as never) + jest.mocked(useAppviewClient).mockReturnValue(appviewClient as never) + queryClient.setQueryData(queryKey, starterPack) + + const wrapper = ({children}: PropsWithChildren) => ( + {children} + ) + const hook = renderHook( + () => useReferenceListOptOutMutation({starterPack, onError}), + {wrapper}, + ) + + return {appviewClient, hook, onError, pdsClient, queryClient} +} + +beforeEach(() => { + jest.clearAllMocks() +}) + +beforeAll(() => { + notifyManager.setNotifyFunction(callback => { + act(callback) + }) +}) + +describe('useReferenceListOptOutMutation', () => { + it('keeps the successful PDS write optimistic when AppView has not caught up', async () => { + const {hook, pdsClient, queryClient} = setup() + pdsClient.create.mockResolvedValue({uri: createdOptOut}) + jest.mocked(until).mockResolvedValue(false) + + await act(() => + hook.result.current.mutateAsync({referenceListOptOut: undefined}), + ) + + expect(pdsClient.create).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({subject: starterPack.list!.uri}), + ) + expect( + queryClient.getQueryData(queryKey) + ?.list?.viewer?.referenceListOptOut, + ).toBe(createdOptOut) + }) + + it('uses the indexed viewer-state URI when AppView reports a duplicate', async () => { + const {hook, pdsClient, queryClient} = setup() + pdsClient.create.mockResolvedValue({uri: createdOptOut}) + jest.mocked(until).mockImplementation((_retries, _delay, cond) => + Promise.resolve( + cond( + { + starterPack: { + list: {viewer: {referenceListOptOut: indexedOptOut}}, + }, + }, + undefined, + ), + ), + ) + + await act(() => + hook.result.current.mutateAsync({referenceListOptOut: undefined}), + ) + + expect( + queryClient.getQueryData(queryKey) + ?.list?.viewer?.referenceListOptOut, + ).toBe(indexedOptOut) + }) + + it('deletes the viewer-state record URI when undoing', async () => { + const {hook, pdsClient, queryClient} = setup() + queryClient.setQueryData(queryKey, { + ...starterPack, + list: { + ...starterPack.list, + viewer: {referenceListOptOut: indexedOptOut}, + }, + }) + pdsClient.delete.mockResolvedValue(undefined) + jest + .mocked(until) + .mockImplementation((_retries, _delay, cond) => + Promise.resolve(cond({starterPack: {list: {viewer: {}}}}, undefined)), + ) + + await act(() => + hook.result.current.mutateAsync({referenceListOptOut: indexedOptOut}), + ) + + expect(pdsClient.delete).toHaveBeenCalledWith(expect.anything(), { + repo: 'did:plc:viewer', + rkey: 'indexed', + }) + expect( + queryClient.getQueryData(queryKey) + ?.list?.viewer?.referenceListOptOut, + ).toBeUndefined() + }) + + it('restores viewer state and surfaces PDS write failures', async () => { + const {hook, onError, pdsClient, queryClient} = setup() + const error = new Error('write failed') + pdsClient.create.mockRejectedValue(error) + + await act(async () => { + await expect( + hook.result.current.mutateAsync({referenceListOptOut: undefined}), + ).rejects.toThrow('write failed') + }) + + await waitFor(() => expect(onError).toHaveBeenCalledWith(error)) + expect( + queryClient.getQueryData(queryKey) + ?.list?.viewer?.referenceListOptOut, + ).toBeUndefined() + }) +}) diff --git a/src/state/queries/starter-packs.ts b/src/state/queries/starter-packs.ts index 7ba7f8fe5e..674fe7336d 100644 --- a/src/state/queries/starter-packs.ts +++ b/src/state/queries/starter-packs.ts @@ -85,7 +85,10 @@ export function useReferenceListOptOutMutation({ const queryKey = RQKEY({did: parsed.name, rkey: parsed.rkey}) return useMutation< - AtUriString | undefined, + { + referenceListOptOut: AtUriString | undefined + didObserveRequestedState: boolean + }, Error, {referenceListOptOut?: string}, {previous?: app.bsky.graph.defs.StarterPackView} @@ -119,12 +122,19 @@ export function useReferenceListOptOutMutation({ (value, error) => { if (error) return false - nextOptOut = value.starterPack.list?.viewer?.referenceListOptOut + const observedOptOut = + value.starterPack.list?.viewer?.referenceListOptOut // AppView ignores duplicate records and continues to expose the URI // of the record it indexed first. Treat that viewer state as the // source of truth instead of waiting for the newly-created URI. - return referenceListOptOut ? !nextOptOut : Boolean(nextOptOut) + const didObserveRequestedState = referenceListOptOut + ? !observedOptOut + : Boolean(observedOptOut) + if (didObserveRequestedState) { + nextOptOut = observedOptOut + } + return didObserveRequestedState }, async () => await appviewClient.call(app.bsky.graph.getStarterPack, { @@ -132,10 +142,10 @@ export function useReferenceListOptOutMutation({ }), ) - if (!didObserveRequestedState) { - throw new Error('Timed out waiting for starter pack opt-out state') + return { + referenceListOptOut: nextOptOut, + didObserveRequestedState, } - return nextOptOut }, onMutate: async ({referenceListOptOut}) => { await queryClient.cancelQueries({queryKey}) @@ -161,7 +171,7 @@ export function useReferenceListOptOutMutation({ ) return {previous} }, - onSuccess: referenceListOptOut => { + onSuccess: ({referenceListOptOut}) => { queryClient.setQueryData( queryKey, current => @@ -189,8 +199,15 @@ export function useReferenceListOptOutMutation({ } onError(error) }, - onSettled: () => { - void queryClient.invalidateQueries({queryKey}) + onSettled: data => { + void queryClient.invalidateQueries({ + queryKey, + // If AppView has not indexed the PDS write yet, keep the committed + // optimistic state visible. Mark it stale so a later mount/focus can + // refetch once AppView has caught up without replacing it immediately + // with the known-outdated value. + refetchType: data && !data.didObserveRequestedState ? 'none' : 'active', + }) }, }) }