From a60c4cc723fe49227f15a7a434594238bb3e031f Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Mon, 29 Jun 2026 12:18:04 -0400 Subject: [PATCH] fix(events): re-run page-1 pipeline on load-more so filters don't drift (om-5iiw) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit loadMoreEvents always paginated via the unfiltered listRecords pipeline, so page 2+ dropped the filters page 1 applied: - /events home: non-discoverable events leaked (page 1 = listDiscoverable) - /p/[actor]/hosting: conference talks leaked (page 1 = listAuthored) - /p/[actor]/past-events: same authored filter was dropped Thread a `pipeline` selector ('discoverable' | 'authored') through each page's fetchParams; load-more re-runs the matching contrail pipeline (absent => plain listRecords). Also carry rsvpsCountMin through the schema + EventList coercion so the home "Popular" filter survives load-more (it was silently stripped). Extract the schema + handler into events-load-more.ts (plain module) so it is unit-testable — SvelteKit's plugin rejects non-remote exports from *.remote.ts; events.remote.ts is now a thin command() adapter. Add events-load-more.test.ts pinning the routing contract. --- apps/web/src/lib/components/EventList.svelte | 2 +- .../src/lib/contrail/events-load-more.test.ts | 96 +++++++++++++++++++ apps/web/src/lib/contrail/events-load-more.ts | 94 ++++++++++++++++++ apps/web/src/lib/contrail/events.remote.ts | 59 +----------- apps/web/src/routes/(app)/events/+page.svelte | 3 + .../(app)/p/[actor]/hosting/+page.svelte | 3 + .../(app)/p/[actor]/past-events/+page.svelte | 3 + 7 files changed, 202 insertions(+), 58 deletions(-) create mode 100644 apps/web/src/lib/contrail/events-load-more.test.ts create mode 100644 apps/web/src/lib/contrail/events-load-more.ts diff --git a/apps/web/src/lib/components/EventList.svelte b/apps/web/src/lib/components/EventList.svelte index 48bbd03..8c6853b 100644 --- a/apps/web/src/lib/components/EventList.svelte +++ b/apps/web/src/lib/components/EventList.svelte @@ -45,7 +45,7 @@ try { const params: Record = {}; for (const [key, value] of Object.entries(fetchParams)) { - if (key === 'limit' || key === 'rsvpsGoingCountMin') { + if (key === 'limit' || key === 'rsvpsGoingCountMin' || key === 'rsvpsCountMin') { params[key] = Number(value); } else if (key === 'profiles') { params[key] = value === 'true'; diff --git a/apps/web/src/lib/contrail/events-load-more.test.ts b/apps/web/src/lib/contrail/events-load-more.test.ts new file mode 100644 index 0000000..0689679 --- /dev/null +++ b/apps/web/src/lib/contrail/events-load-more.test.ts @@ -0,0 +1,96 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +// runLoadMoreEvents must re-run the SAME read pipeline page 1 used. The bug +// (om-5iiw) was that it always called listRecords, so the discoverable filter +// (home) and the authored filter (profile hosting/past) were dropped on page +// 2+, leaking unlisted events and conference talks. These tests pin the +// routing: the `pipeline` selector picks the matching contrail fn and is +// stripped from the params handed to it (it is our selector, not an xrpc param). +vi.mock('./index', () => ({ + getServerClient: vi.fn(() => ({})) +})); +vi.mock('$lib/contrail', () => ({ + flattenEventRecords: vi.fn((records: unknown[]) => records), + listEventRecordsFromContrail: vi.fn(), + listDiscoverableEventsFromContrail: vi.fn(), + listAuthoredEventsFromContrail: vi.fn() +})); +vi.mock('$lib/search/server/query', () => ({ + searchBackendFromEnv: vi.fn(() => null), + runEventSearchPage: vi.fn() +})); + +import { runLoadMoreEvents, type LoadMoreEventsInput } from './events-load-more'; +import { + listAuthoredEventsFromContrail, + listDiscoverableEventsFromContrail, + listEventRecordsFromContrail +} from '$lib/contrail'; +import { searchBackendFromEnv } from '$lib/search/server/query'; + +const mockRecords = vi.mocked(listEventRecordsFromContrail); +const mockDiscoverable = vi.mocked(listDiscoverableEventsFromContrail); +const mockAuthored = vi.mocked(listAuthoredEventsFromContrail); +const mockSearchBackend = vi.mocked(searchBackendFromEnv); + +const emptyPage = { records: [], profiles: [], cursor: 'next' } as unknown as Awaited< + ReturnType +>; + +// env is opaque here — getServerClient is mocked, and the search backend is +// resolved via the (mocked) searchBackendFromEnv. +const env = { DB: {} } as unknown as App.Platform['env']; +const call = (input: Partial) => + runLoadMoreEvents(env, input as LoadMoreEventsInput); + +afterEach(() => vi.clearAllMocks()); + +describe('runLoadMoreEvents pipeline routing', () => { + it("routes pipeline:'discoverable' to listDiscoverable, never listRecords", async () => { + mockDiscoverable.mockResolvedValue(emptyPage); + + await call({ pipeline: 'discoverable', startsAtMin: '2026-01-01T00:00:00Z', cursor: 'c' }); + + expect(mockDiscoverable).toHaveBeenCalledTimes(1); + expect(mockRecords).not.toHaveBeenCalled(); + expect(mockAuthored).not.toHaveBeenCalled(); + }); + + it("routes pipeline:'authored' to listAuthored, never listRecords", async () => { + mockAuthored.mockResolvedValue(emptyPage); + + await call({ pipeline: 'authored', actor: 'did:plc:alice', cursor: 'c' }); + + expect(mockAuthored).toHaveBeenCalledTimes(1); + expect(mockRecords).not.toHaveBeenCalled(); + expect(mockDiscoverable).not.toHaveBeenCalled(); + }); + + it('falls back to plain listRecords when no pipeline is given', async () => { + mockRecords.mockResolvedValue(emptyPage); + + await call({ cursor: 'c' }); + + expect(mockRecords).toHaveBeenCalledTimes(1); + expect(mockDiscoverable).not.toHaveBeenCalled(); + expect(mockAuthored).not.toHaveBeenCalled(); + }); + + it('strips the pipeline selector but forwards the real filters', async () => { + mockDiscoverable.mockResolvedValue(emptyPage); + + await call({ pipeline: 'discoverable', rsvpsCountMin: 2, cursor: 'c' }); + + const params = mockDiscoverable.mock.calls[0][1]; + expect(params).not.toHaveProperty('pipeline'); + expect(params).toMatchObject({ rsvpsCountMin: 2, cursor: 'c' }); + }); + + it('does not consult the search backend for a non-search load', async () => { + mockDiscoverable.mockResolvedValue(emptyPage); + + await call({ pipeline: 'discoverable', cursor: 'c' }); + + expect(mockSearchBackend).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/lib/contrail/events-load-more.ts b/apps/web/src/lib/contrail/events-load-more.ts new file mode 100644 index 0000000..bd8abc5 --- /dev/null +++ b/apps/web/src/lib/contrail/events-load-more.ts @@ -0,0 +1,94 @@ +import * as v from 'valibot'; +import { getServerClient } from './index'; +import { + flattenEventRecords, + listAuthoredEventsFromContrail, + listDiscoverableEventsFromContrail, + listEventRecordsFromContrail +} from '$lib/contrail'; +import { runEventSearchPage, searchBackendFromEnv } from '$lib/search/server/query'; +import type { ActorIdentifier } from '@atcute/lexicons'; + +export const listEventsInput = v.object({ + actor: v.optional(v.string()), + search: v.optional(v.string()), + startsAtMin: v.optional(v.string()), + startsAtMax: v.optional(v.string()), + endsAtMin: v.optional(v.string()), + endsAtMax: v.optional(v.string()), + rsvpsCountMin: v.optional(v.number()), + rsvpsGoingCountMin: v.optional(v.number()), + profiles: v.optional(v.boolean()), + sort: v.optional(v.string()), + order: v.optional(v.picklist(['asc', 'desc'])), + limit: v.optional(v.number()), + cursor: v.optional(v.string()), + // Which page-1 read pipeline this list came from. load-more MUST re-run the + // same pipeline or it drifts: 'discoverable' (home) drops the unlisted-event + // filter, 'authored' (profile hosting/past) drops the conference-talk filter, + // and either leaks records page 1 excluded. Absent => plain listRecords. + pipeline: v.optional(v.picklist(['discoverable', 'authored'])) +}); + +export type LoadMoreEventsInput = v.InferOutput; + +export type LoadMoreEventsResult = { + events: ReturnType; + handles: Record; + cursor: string | null; +}; + +/** + * Shared load-more handler. Kept out of the `.remote.ts` adapter so it is a + * plain function the SvelteKit remote-functions plugin won't wrap — that lets it + * be unit-tested directly (the plugin rejects non-remote exports from + * `*.remote.ts`, so a test there can't mock `$app/server`). + */ +export async function runLoadMoreEvents( + env: App.Platform['env'], + input: LoadMoreEventsInput +): Promise { + const client = getServerClient(env.DB); + + // Text-search pagination goes through Meilisearch when configured, matching + // the search page's first-page path — its cursor is a Meili offset, which + // the D1 path below cannot consume (and vice versa). Errors propagate to + // EventList's catch so the user can retry with the cursor intact. + const searchBackend = input.search?.trim() ? searchBackendFromEnv(env) : null; + if (searchBackend && input.search) { + const page = await runEventSearchPage(searchBackend, client, { + q: input.search.trim(), + cursor: input.cursor ?? null + }); + return { events: page.events, handles: page.handles, cursor: page.cursor }; + } + + // Re-run the SAME page-1 pipeline so load-more inherits its filters. `pipeline` + // is our selector, not an xrpc param, so strip it before the call. + const { pipeline, ...rest } = input; + const params = { ...rest, actor: rest.actor as ActorIdentifier | undefined }; + + const response = + pipeline === 'discoverable' + ? await listDiscoverableEventsFromContrail(client, params) + : pipeline === 'authored' + ? await listAuthoredEventsFromContrail(client, params) + : await listEventRecordsFromContrail(client, params); + + if (!response) { + return { events: [], handles: {}, cursor: null }; + } + + const events = flattenEventRecords(response.records ?? []); + + const handles: Record = {}; + for (const p of response.profiles ?? []) { + if (p.handle) handles[p.did] = p.handle; + } + + return { + events, + handles, + cursor: response.cursor ?? null + }; +} diff --git a/apps/web/src/lib/contrail/events.remote.ts b/apps/web/src/lib/contrail/events.remote.ts index cc6cace..95cfb9b 100644 --- a/apps/web/src/lib/contrail/events.remote.ts +++ b/apps/web/src/lib/contrail/events.remote.ts @@ -1,62 +1,7 @@ import { command, getRequestEvent } from '$app/server'; -import * as v from 'valibot'; -import { getServerClient } from './index'; -import { flattenEventRecords, listEventRecordsFromContrail } from '$lib/contrail'; -import { runEventSearchPage, searchBackendFromEnv } from '$lib/search/server/query'; -import type { ActorIdentifier } from '@atcute/lexicons'; - -const listEventsInput = v.object({ - actor: v.optional(v.string()), - search: v.optional(v.string()), - startsAtMin: v.optional(v.string()), - startsAtMax: v.optional(v.string()), - endsAtMin: v.optional(v.string()), - endsAtMax: v.optional(v.string()), - rsvpsGoingCountMin: v.optional(v.number()), - profiles: v.optional(v.boolean()), - sort: v.optional(v.string()), - order: v.optional(v.picklist(['asc', 'desc'])), - limit: v.optional(v.number()), - cursor: v.optional(v.string()) -}); +import { listEventsInput, runLoadMoreEvents } from './events-load-more'; export const loadMoreEvents = command(listEventsInput, async (input) => { const { platform } = getRequestEvent(); - - const client = getServerClient(platform!.env.DB); - - // Text-search pagination goes through Meilisearch when configured, matching - // the search page's first-page path — its cursor is a Meili offset, which - // the D1 path below cannot consume (and vice versa). Errors propagate to - // EventList's catch so the user can retry with the cursor intact. - const searchBackend = input.search?.trim() ? searchBackendFromEnv(platform?.env) : null; - if (searchBackend && input.search) { - const page = await runEventSearchPage(searchBackend, client, { - q: input.search.trim(), - cursor: input.cursor ?? null - }); - return { events: page.events, handles: page.handles, cursor: page.cursor }; - } - - const response = await listEventRecordsFromContrail(client, { - ...input, - actor: input.actor as ActorIdentifier | undefined - }); - - if (!response) { - return { events: [] as ReturnType, handles: {} as Record, cursor: null as string | null }; - } - - const events = flattenEventRecords(response.records ?? []); - - const handles: Record = {}; - for (const p of response.profiles ?? []) { - if (p.handle) handles[p.did] = p.handle; - } - - return { - events, - handles, - cursor: response.cursor ?? null - }; + return runLoadMoreEvents(platform!.env, input); }); diff --git a/apps/web/src/routes/(app)/events/+page.svelte b/apps/web/src/routes/(app)/events/+page.svelte index 9dcd70f..35b20f6 100644 --- a/apps/web/src/routes/(app)/events/+page.svelte +++ b/apps/web/src/routes/(app)/events/+page.svelte @@ -9,6 +9,9 @@ let filter = $derived(page.url.searchParams.get('filter') === 'all' ? 'all' : 'popular'); let fetchParams = $derived({ + // load-more must re-run the discoverable pipeline + popular filter page 1 + // used, or unlisted / non-popular events leak onto later pages. + pipeline: 'discoverable', startsAtMin: new Date().toISOString(), profiles: 'true', sort: 'startsAt', diff --git a/apps/web/src/routes/(app)/p/[actor]/hosting/+page.svelte b/apps/web/src/routes/(app)/p/[actor]/hosting/+page.svelte index 81233fe..dbd98b3 100644 --- a/apps/web/src/routes/(app)/p/[actor]/hosting/+page.svelte +++ b/apps/web/src/routes/(app)/p/[actor]/hosting/+page.svelte @@ -12,6 +12,9 @@ ); let fetchParams: Record = $derived({ + // load-more must re-run the authored pipeline page 1 used, or conference + // talks (excluded by listAuthored) leak onto later pages. + pipeline: 'authored', profiles: 'true', sort: 'startsAt', order: 'asc', diff --git a/apps/web/src/routes/(app)/p/[actor]/past-events/+page.svelte b/apps/web/src/routes/(app)/p/[actor]/past-events/+page.svelte index e3a08d2..3b73860 100644 --- a/apps/web/src/routes/(app)/p/[actor]/past-events/+page.svelte +++ b/apps/web/src/routes/(app)/p/[actor]/past-events/+page.svelte @@ -12,6 +12,9 @@ ); let fetchParams: Record = $derived({ + // load-more must re-run the authored pipeline page 1 used, or conference + // talks (excluded by listAuthored) leak onto later pages. + pipeline: 'authored', profiles: 'true', sort: 'startsAt', order: 'desc', -- 2.51.2