refactor(events): consolidate wallet event list query logic (#1271) - #1383
Open
Rafiat30 wants to merge 1 commit into
Open
refactor(events): consolidate wallet event list query logic (#1271)#1383Rafiat30 wants to merge 1 commit into
Rafiat30 wants to merge 1 commit into
Conversation
…t#1271) Extract the duplicated "paginated stream events by sender/recipient wallet" query logic from GET /v1/events (events.routes.ts) and GET /v1/users/:publicKey/events (user.controller.ts) into a single listEventsForWallet helper in the new backend/src/repositories/streamEvent.repository.ts. The shared repository also exposes parseEventTypeFilter, resolveEventsPageSize, and resolveEventsOffset, and now supports the union of both endpoints' prior feature sets: comma-separated `type` filtering (previously only on /v1/events) and an optional includeStream flag that embeds the related stream (previously always-on and only on /v1/users/:publicKey/events). Each endpoint keeps its historical includeStream default (false for /v1/events, true for /v1/users/:publicKey/events) so existing consumers see no response-shape change. stream.controller.ts is updated to import DEFAULT_EVENTS_PAGE_SIZE / MAX_EVENTS_PAGE_SIZE from the new repository module instead of from events.routes.ts, where they no longer live. Adds streamEvent.repository.test.ts covering the new helpers, and extends user.controller.test.ts and events-list.test.ts with includeStream/type-filter coverage plus parity tests asserting both endpoints build identical where-clauses via the shared helper.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1271
Summary
Two near-identical implementations of "paginated stream events by sender/recipient wallet" existed with different feature sets:
GET /v1/events(backend/src/routes/v1/events.routes.ts) supported comma-separatedtypefiltering but never included the relatedstream.GET /v1/users/:publicKey/events(backend/src/controllers/user.controller.ts) always included the relatedstreambut had notypefilter.Both duplicated the same sender/recipient
ORwhere-clause, pagination (limit/offset/page) logic, and event-type regex/validation.This PR extracts a single shared
listEventsForWallethelper (plus supportingparseEventTypeFilter,resolveEventsPageSize,resolveEventsOffset) into a new repository module, and updates both endpoints to delegate to it, so both now support the union of the two original feature sets.New files
backend/src/repositories/streamEvent.repository.ts— shared query logic:EVENT_TYPES,MAX_EVENTS_PAGE_SIZE,DEFAULT_EVENTS_PAGE_SIZEparseEventTypeFilter(rawType)— parses/validates a comma-separatedtypequery paramresolveEventsPageSize(rawLimit)— clamps/defaultslimitresolveEventsOffset({ rawOffset, rawPage, limit })— resolvesoffsetfromoffsetorpagelistEventsForWallet({ address, types, limit, offset, includeStream })— builds the sender/recipientORwhere-clause, applies the optionaleventType infilter, paginates, optionally includes the relatedstream, and returns{ events, total, limit, offset, hasMore }backend/tests/streamEvent.repository.test.ts— unit tests for the new repository moduleModified files
backend/src/routes/v1/events.routes.ts—GET /v1/eventsnow delegates tolistEventsForWallet; gained a newincludeStreamquery param (defaultfalse, matching its historical behavior of never embedding the stream)backend/src/controllers/user.controller.ts—GET /v1/users/:publicKey/eventsnow delegates tolistEventsForWallet; gained comma-separatedtypefiltering (previously unsupported);includeStreamdefaults totruehere to preserve this endpoint's historical behavior of always embedding the streambackend/src/controllers/stream.controller.ts— updated to importDEFAULT_EVENTS_PAGE_SIZE/MAX_EVENTS_PAGE_SIZEfrom the new repository module instead ofevents.routes.ts, since those constants movedbackend/tests/integration/events-list.test.ts— addedincludeStreamcoverage forGET /v1/events, plus a new "filtering parity" describe block asserting both endpoints build identicalwhereclauses (sender/recipient OR, andeventType in) via the shared helper, and that both reject an all-invalidtypefilter with 400backend/tests/user.controller.test.ts— added coverage forincludeStreamdefault/opt-out and the newtypefilter (including the 400 rejection path) onGET /v1/users/:publicKey/eventsImplementation details
listEventsForWalletacceptstypes(comma-separated event type filter) andincludeStream(whether toinclude: { stream: true }), so either caller can opt into either feature.includeStream(falsefor/v1/events,truefor/v1/users/:publicKey/events), so existing consumers (e.g. the frontend Activity page, which readsdata.eventsfrom/v1/eventsand never asked forincludeStream) see no response-shape change. Response envelopes ({ events, total, limit, offset, hasMore }vs{ data, total, hasMore, limit, offset }) are unchanged — only the underlying query logic is shared.parseEventTypeFilter(case-insensitive, comma-separated, unknown values dropped from the applied filter but tracked separately so an all-invalid filter still returns 400) and the same pagination resolution (resolveEventsPageSize,resolveEventsOffset).Tests added
backend/tests/streamEvent.repository.test.ts(new): unit tests forparseEventTypeFilter,resolveEventsPageSize,resolveEventsOffset, andlistEventsForWallet(where-clause construction, type filter application/omission, pagination,includeStreamon/off,hasMorecomputation).backend/tests/user.controller.test.ts:includeStreamdefault-on /includeStream=falseopt-out, comma-separatedtypefilter forwarding, and 400 on an all-invalidtypefilter.backend/tests/integration/events-list.test.ts:includeStreamdefault-off /includeStream=trueopt-in forGET /v1/events, plus a new parity suite that hits bothGET /v1/eventsandGET /v1/users/:publicKey/eventswith the same wallet/type filter and asserts the Prismawhereclauses passed tofindMany/countare identical, and that both return 400 for an all-invalidtypefilter.How to test manually
Start the backend (
npm run devinbackend/) with a valid session token, then:Expect: the
typefilter behaves identically on both endpoints (same accepted values, same 400 on an all-invalid filter), andincludeStreamtoggles the embeddedstreamobject on both endpoints identically, with each endpoint's response envelope unchanged from before ({ events, total, limit, offset, hasMore }for/v1/events,{ data, total, hasMore, limit, offset }for/v1/users/:publicKey/events).Test/build status
npx vitest run --no-coverage tests/streamEvent.repository.test.ts tests/user.controller.test.ts tests/integration/events-list.test.ts→ 50/50 passednpx vitest run --no-coverage --exclude='tests/integration/**'(full unit suite) → 339 passed, 3 skipped (pre-existing, unrelated), 0 failednpx tsc --noEmit→ no errorsbackend/tests/integration/(e.g.stream-lifecycle.test.ts) were not run in this environment since noDATABASE_URL/Postgres was available; those tests self-skip gracefully by design (seebackend/tests/integration/_db.ts, issue [BUG] Backend integration tests fail on main due to PostgreSQL connection error #760) rather than failing.events-list.test.ts, which is what covers this change, fully mocks Prisma and ran/passed normally.