Make attendee reads faster and load them all one way - #1790
Merged
Conversation
…ns so far) Introduce src/shared/db/attendees/select.ts: one place that assembles the attendee SELECT from a per-field opt-in list. It always emits the cheap identity and per-listing columns and adds an expensive ledger subquery (remaining_balance, refunded, price_paid) only when the caller asks for it. The three subquery fragments move here from queries.ts (their true home now); queries.ts, tokens.ts and balance.ts import them. decryptAttendees/decryptAttendeeFields become generic over the selected row shape, coercing price_paid/refunded only when those columns are actually present so a narrowed read never fabricates a value. Every caller of the old ATTENDEE_JOIN_SELECT / ATTENDEE_LEFT_JOIN_SELECT constants now goes through the builder with the full field set, so this commit changes no SQL output — it only puts the shared mechanism in place. The two fat constants are deleted. Narrowing individual reads to the fields they actually display comes next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G24MzgR46DN4ju1srifKaq
…bqueries The admin dashboard's recent-bookings table and the /admin/servicing summary list show only a booking's quantity, date, check-in state and (dashboard) refunded badge — never price_paid or remaining_balance. They were paying for those correlated ledger subqueries on every line regardless. Introduce DisplayAttendee — the exact attendee fields the shared attendee table reads — and type AttendeeTableRow/the row builders/CheckinButton against it. A full Attendee still satisfies it, and so does a field-selected read that skipped the money columns. getNewestAttendeesRaw now asks for just `refunded` (4 price_paid subqueries + remaining_balance dropped per line), and the servicing summary read asks for no money fields at all. The attendees-browser page query stays on the full field set for now because the same query also backs the CSV export, which sums price_paid — the next step centralises attendee reads behind one getAttendees(filter, fields) so the table and the CSV can ask for different fields over the same filter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G24MzgR46DN4ju1srifKaq
…, fields)
Instead of each reader hand-writing its own SELECT/FROM/WHERE/ORDER, they now
declare WHICH attendees they want and WHICH fields to project, and one façade
builds the minimal query. `AttendeeWhere` covers every filter the old readers
used (by listing, by attendee id or id-list, by package group, real-lines-only,
upcoming-from-date, a daily-listing date range, and a "pick these attendee ids"
subquery for the newest/paged reads); `AttendeeOrder` names the orderings; and
`getAttendees` / `getAttendeeRow` / `attendeeBatchStatement` share one resolver.
Passing a list of ids is now first-class: `where: { attendeeIds }` and
`where: { listingIds }`. getAttendeesRaw, getAttendeePackageRowsRaw,
getNewestAttendeesRaw, getAttendeesPage, getAttendeeRaw, getAttendeesByIds,
getDailyListingAttendeesByDate, getAttendeesByListingIds, the two batch
listing+attendee readers, and both servicing readers all now go through it.
Because the filter is declared once, the browsing table and the CSV export can
ask for different fields over the same filter — the whole point.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G24MzgR46DN4ju1srifKaq
…ws kind-agnostic Add exact-string assertions for the generated column list and FROM/WHERE/ORDER so a mutation to any column, separator, COALESCE wrapper, ledger subquery, WHERE clause, join keyword or ORDER BY is caught — the builder's output is its contract. The two defaulted `kind`/`join` operands are string unions that can never be "", so `?? → ||` on them is provably equivalent and is recorded in equivalent-mutants.txt. select.ts now mutation-scores 100% (90 killed, 2 equivalents). Also restore getAttendeePackageRowsRaw's original behaviour: it never filtered by kind (the attendee id already pins one attendee), so it now passes `attendee-or-servicing` (every kind the CHECK allows) instead of the default regular-attendee filter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G24MzgR46DN4ju1srifKaq
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe attendee data-access layer now provides typed, field-selective SQL builders and shared filtering, ordering, and batch-query helpers. Attendee, servicing, listing, administrative, decryption, and display paths use these helpers, with generated-SQL tests and updated row types. ChangesAttendee query and projection refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AttendeeReader
participant getAttendees
participant attendeeFromWhere
participant Database
participant decryptAttendees
AttendeeReader->>getAttendees: fields, join, filters, and order
getAttendees->>attendeeFromWhere: build projection and SQL tail
getAttendees->>Database: execute SQL with bound arguments
Database-->>getAttendees: raw selected attendee rows
getAttendees-->>AttendeeReader: typed attendee rows
AttendeeReader->>decryptAttendees: decrypt selected rows
decryptAttendees-->>AttendeeReader: decrypted attendee rows
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…-queries-el4dr0 # Conflicts: # scripts/mutation/equivalent-mutants.txt
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/mutation/equivalent-mutants.txt`:
- Around line 899-903: Resolve the merge conflict in
scripts/mutation/equivalent-mutants.txt against origin/main, preserving the
equivalent-mutant suppression entries for getAttendees at
src/shared/db/attendees/select.ts lines 325 and 414. Remove all conflict
markers, verify the file contains the correct combined entries, and recommit the
resolved result.
In `@src/features/admin/attendees-edit.ts`:
- Around line 19-24: Replace the duplicated getAttendeeRow query construction in
both handlers with the existing getAttendeeRaw(attendeeId) helper from
`#shared/db/attendees/queries.ts`. Update imports and preserve each handler’s
existing result handling, removing the now-unused ATTENDEE_FIELDS and
getAttendeeRow imports.
In `@src/features/admin/attendees-merge.ts`:
- Around line 16-23: Update loadMergeTarget to reuse getAttendeeRaw(attendeeId)
from `#shared/db/attendees/queries.ts` instead of rebuilding the getAttendeeRow
call with ATTENDEE_FIELDS, the left join, and attendeeId filter. Add or adjust
the import accordingly and remove now-unused query-construction imports.
In `@src/shared/db/attendees/queries.ts`:
- Around line 388-395: Update getAttendeesByIds to explicitly disable the
default kind filter when calling getAttendees, keeping the attendee ID lookup
kind-agnostic so servicing run-sheet rows are included and loadLegLookups
receives all matching entries.
In `@src/shared/db/attendees/select.ts`:
- Around line 230-235: Update attendeeBatchStatement to reuse attendeeSql with
the { args, fields, from, join } result from resolveAttendeeQuery instead of
re-inlining the SELECT template. Preserve the existing query arguments and
ensure batch readers use the exact SQL columns and clauses produced by
attendeeSql.
In `@test/lib/db/attendees/select.test.ts`:
- Around line 124-144: Update the singular-id tests and their callers to use
only attendeeIds and listingIds, passing a one-element array for single lookups.
Remove expectations and setup for attendeeId/listingId, and ensure
attendeeFromWhere continues producing one placeholder with the same argument
value for single-item arrays.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b668e185-17e8-4180-83d5-ea1dea720511
📒 Files selected for processing (19)
scripts/mutation/equivalent-mutants.txtsrc/features/admin/attendees-edit.tssrc/features/admin/attendees-merge.tssrc/shared/attendee-table-rows.tssrc/shared/db/attendees/balance.tssrc/shared/db/attendees/pii.tssrc/shared/db/attendees/queries.tssrc/shared/db/attendees/select.tssrc/shared/db/attendees/servicing.tssrc/shared/db/attendees/tokens.tssrc/shared/db/listings.tssrc/shared/types.tssrc/ui/templates/admin/dashboard.tsxsrc/ui/templates/attendee-table.tsxtest/lib/db/attendees/select.test.tstest/lib/server-reservation/deposit-basics.test.tstest/lib/server-reservation/helpers.tstest/shared/db/attendees/servicing/code-quality.test.tstest/test-utils/servicing.ts
…isfy stricter cpd Main's tightened jscpd threshold (#1787) flagged the paired thin wrappers and the duplicated attendee-by-id load once merged in. Restructure the reader layer so there's a single SQL builder: - attendeeBatchStatement is now the one place a declared query becomes SQL; getAttendees runs it, and the batch listing readers embed it. Removes the selectAttendees/selectAttendeeOrNull pair and getAttendeeRow. - getAttendeeRaw is the single attendee-by-id read; attendees-edit and attendees-merge reuse it instead of re-inlining the same getAttendees call. - Collapse the singular attendeeId/listingId filters into attendeeIds/listingIds (a single lookup is a one-element array), matching the repo's "one path for one-or-many" rule and dropping the duplicate WHERE branches. getAttendeesByIds keeps its kind='attendee' filter — that's the exact behaviour the pre-existing query had; whether the logistics run sheet should include servicing events is a separate product decision, recorded in TODO.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G24MzgR46DN4ju1srifKaq
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/db/attendees/select.ts (1)
291-306: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard empty
attendeeIds/listingIdshere.inList()still emitsIN ()for[], which is invalid SQL. Handle the empty case explicitly in this helper instead of relying on every caller to prefilter it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/db/attendees/select.ts` around lines 291 - 306, Update the local inList helper to handle empty ID arrays explicitly, preventing it from adding an IN () clause while preserving normal filtering for non-empty arrays. Apply this behavior to both attendeeIds and listingIds through the existing inList calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/shared/db/attendees/select.ts`:
- Around line 291-306: Update the local inList helper to handle empty ID arrays
explicitly, preventing it from adding an IN () clause while preserving normal
filtering for non-empty arrays. Apply this behavior to both attendeeIds and
listingIds through the existing inList calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6a1b19bc-7627-4766-9a81-88d6b3dac8b1
📒 Files selected for processing (9)
TODO.mdscripts/mutation/equivalent-mutants.txtsrc/features/admin/attendees-edit.tssrc/features/admin/attendees-merge.tssrc/shared/db/attendees/queries.tssrc/shared/db/attendees/select.tssrc/shared/db/attendees/servicing.tssrc/shared/db/listings.tstest/lib/db/attendees/select.test.ts
… invalid SQL whereClauses' inList emitted `IN ()` for an empty attendeeIds/listingIds, which is invalid SQL. No caller triggers it today (they pass one-element arrays or prefilter empties), but the filter accepts any number[], so the builder should stay total: an empty id set now emits `IN (NULL)` — always NULL, so no row passes — matching the "empty filter matches nothing" behaviour the callers already rely on. Covered by a direct builder test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G24MzgR46DN4ju1srifKaq
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shared/db/attendees/select.ts`:
- Around line 293-295: Reword the comment explaining the empty ID set handling
to replace “stays total” with plain language describing that the query builder
remains valid and safe when callers provide an empty list. Keep the existing IN
(NULL) behavior and surrounding explanation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 24558843-8f85-460c-af12-b8757e6f184c
📒 Files selected for processing (2)
src/shared/db/attendees/select.tstest/lib/db/attendees/select.test.ts
"stays total" was CS jargon (total function); say "still produces valid SQL" instead, per the repo's plain-language rule for code comments. Comment-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G24MzgR46DN4ju1srifKaq
stefan-burke
pushed a commit
that referenced
this pull request
Jul 12, 2026
Main's #1785 (ledger/servicing split) and #1790 (attendee/listing reads) added new code plus reorganised files my dedups touched. Restore/re-home the fixes so the whole tree is clone-free at the tightened threshold: - IdParam type in entity.ts, shared by site-pages idHandler and the relocated ledger postedTransferRoute. - nameMapByIdsFor factory (config-object param, distinct from allNamesById) for the modifier/listing name lookups. - decryptNameSlug helper for the news-summary and listing-catalog projections. - Namespace-import groups in webhooks to break the checkin import-block match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTrbBujCcBu8CUC56dy4tQ
This was referenced Jul 23, 2026
This was referenced Aug 17, 2026
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
2 participants
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.
What this changes
The admin homepage was doing a lot of database work it didn't need. Every list of bookings — the dashboard's recent bookings, the attendees page, the calendar, the service-event list — ran the same heavy database query, even when the page only showed a booking's name, date and quantity. The most expensive parts of that query work out how much each person paid and what they still owe; those numbers take several extra lookups per booking and most pages never show them.
This change lets each page ask for only the booking details it actually shows, so the database skips the work behind anything the page won't display.
How it works now
There is now one shared way to load bookings. A page says which bookings it wants (by listing, by person, by package, upcoming ones, a date range, or a specific set of ids) and which details it needs, and the system builds the smallest query that answers that. Before, each page hand-wrote its own database query; there were lots of near-copies that could quietly drift apart. Now they all go through the same place.
Concretely:
Safety
Nothing a page displays has changed — where a page does show payment or balance figures, it still asks for them and still shows the same values. The one query that had no filter on the kind of booking keeps that behaviour. Every existing test passes, coverage stays at 100%, and the new shared query builder is covered by its own tests with a full mutation score.
🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit