a2n-seattle / a2n-seattle/rms-app
GetBorrowGroup.ts scans the full items table -- use ScheduleTable.consume() + direct lookup instead
- Dominant language
- TypeScript
- Stars
- 1
- Forks
- 1
- Avg merge
- 27m
- Merged PRs (30d)
- 4
Description
# Why?
`GetBorrowGroup.ts` (`ts-code/src/api/GetBorrowGroup.ts`) does an unbounded, unfiltered-by-`Limit` DynamoDB `Scan+FilterExpression` against the full `items-alpha` table on every load of the batched-return confirmation page (`web/e2e/return-group.spec.ts`'s flow) — the one remaining full-table scan not already addressed by GH-384/#385's `UserTable`-denormalization work (found while auditing the full list of remaining scans post-GH-384).
It filters on `borrowGroupId`, set on each item at borrow time by `BorrowFromSchedule.ts` (via `ItemTable.changeBorrower`'s `borrowGroupId` param) to the id of the `ScheduleTable` reservation consumed to create the borrow. The reason it can't just do a direct `Get` today: `BorrowFromSchedule.execute()` deletes that schedule row immediately after borrowing (`scheduleTable.delete(input.scheduleId)`), so there's no live `ScheduleTable` entry left to look up `itemIds` from by the time someone views/returns the group.
# What?
Stop deleting the schedule row on borrow-consumption, and use it as the lookup instead of scanning:
- Add a new `ScheduleTable.consume(id)` method (refactored out of `delete(id)`'s existing item-back-reference cleanup logic) that removes the schedule's id from each of its items' `schedule[]` array — the same cleanup `delete()` already does, which is what prevents `ListOverdueItems`'s overdue check from later treating a consumed reservation's `endTime` as still relevant — but leaves the `ScheduleTable` row itself intact in DynamoDB.
- `BorrowFromSchedule.ts` calls `consume()` instead of `delete()`.
- `GetBorrowGroup.ts` does a direct `ScheduleTable.get(borrowGroupId)` (the schedule id and borrowGroupId are the same value) to retrieve `itemIds`, then targeted `ItemTable.get(id)` calls for each — bounded by the borrow group's own size, matching the code's existing comment that a borrow group is expected to be small, not table-wide.
- Must preserve existing partial-return semantics: the current Scan-based version only returns items whose `borrowGroupId` still equals the group id (an item already individually returned has `borrowGroupId` cleared by `ItemTable.changeBorrower`'s "return" action) — the new version needs to filter fetched items the same way, not unconditionally return everyone from the schedule's original `itemIds`.
- `DeleteReservation.ts`'s use of `ScheduleTable.delete()` (real cancellation, not borrow-consumption) is unaffected and should keep doing a full delete.
# Testing
Per root CLAUDE.md's testing policy:
- New `ScheduleTable.test.ts` case for `consume()`.
- `BorrowFromSchedule.test.ts`'s existing assertion that `db.schedule` equals `{}` after borrowing needs to change — the schedule row now persists.
- `GetBorrowGroup.test.ts` needs rewriting for the new lookup mechanism, including a partial-return case (one item in the group already individually returned, confirm it's excluded from the result).
# Additional context
Found during a live debugging session investigating real DynamoDB read-capacity pressure on `alpha` — this scan reads the entire `items-alpha` table on every batched-return page view, unbounded.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.