Keep a standalone parent's child selector when its package is sold out - #1732
Conversation
`buildPageListingRows` derived its child-selector suppression set from every package's members, then dropped the selector on any standalone row in that set (the member's own package row carries the one selector). But a sold-out package (`limit < 1`) renders a bare sold-out card with no member rows — so a standalone parent that is also a member of a sold-out package lost its child selector on both paths, leaving a multi-choice parent unbookable. Build the suppression set from only the packages that actually render member rows (`limit >= 1`). Verified the `hideListings` case the TODO flagged is a no-op here: a hidden package's members are never standalone rows, so they need no handling. Adds an in-process regression test (a standalone parent whose sibling-capacity sold-out package hid its member row still renders the child selector) — confirmed it fails on the old code and passes on the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jDnHypgnzR8SWzcoasw6S
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPackage rendering now suppresses standalone child selectors only for members rendered by available packages. A regression test confirms that sold-out packages do not suppress a standalone parent selector, and the corresponding TODO is removed. ChangesPackage rendering behavior
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/ui/templates/public/reservations/packages.ts`:
- Around line 164-176: Replace the nested imperative loops building
renderedMemberIds with the repository’s `#fp` functional composition, following
the nearby pattern in the same template: filter opts.packages by
packageLimits.get(pkg.groupId)! >= 1, flatMap each package’s memberListingIds,
then construct the Set from the resulting IDs. Preserve the existing suppression
behavior and invariant.
🪄 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: 01eb6a76-c72f-4636-bbb3-4daecc385790
📒 Files selected for processing (3)
TODO.mdsrc/ui/templates/public/reservations/packages.tstest/templates/public/ticket-page-packages.test.ts
💤 Files with no reviewable changes (1)
- TODO.md
Per review: replace the nested imperative loops with the file's functional style. Behaviourally identical (the test is unchanged and still red on the old bug, green here). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jDnHypgnzR8SWzcoasw6S
…yle, equivalent-mutant entry Addresses 5 actionable inline review comments from CodeRabbit and Codex on PR #1721 (2 additional Codex comments about stale imports were already fixed by the merge commit 0e553fe). 1. listing-rows.ts (Major/functional — CodeRabbit + Codex): Real bug: a standalone parent that is also a member of a sold-out or hideListings package lost its child selector entirely. The code used 'memberIds = packageMemberIds(opts.packages)' which includes ALL package members, even those in packages that never render member rows (sold-out / hidden). Changed to 'claimedChildParents' — the set that claimChildCtx actually populated — so only rows whose child selector a package section ACTUALLY rendered are suppressed on the standalone path. The unused packageMemberIds import is removed. The regression test (ticket-page-packages.test.ts:169, from main's #1732) passes. 2. controls.ts (Major/security — CodeRabbit): XSS: date values were interpolated into the option value attribute without escaping. A malformed date string could break out of the attribute and inject markup. Now wrapped in escapeHtml(d). Regression test added (test/lib/render-date-selector.test.ts) that verifies a date containing a double-quote is properly escaped. 3. availability.ts (nitpick — CodeRabbit): Replaced the imperative for...of loop in buildPageTree with a curried reduce from #fp, preserving the BUYER_CHOICE-wins precedence. 4. child-pricing.ts (nitpick — CodeRabbit): Replaced the mutable Map + nested loops in foldReserveByChildId with a flatMap + reduce pipeline from #fp: flatMap to (childId, reserve) pairs, then reduce to sum them. 5. equivalent-mutants.txt (P2 — Codex): Moved the '?? → ||' equivalent-mutant entry from page-meta.ts:66:55 (a #1693-only file, now deleted) to contact-fields.ts:107:51 (where the same 'addOns?.some(...) ?? false' expression now lives). Checks after fixes: - deno check: PASS - test:files (render-date-selector + ticket-page-packages): 15/15 PASS - jscpd src: 0 clones, exit 0 - jscpd test: 0 clones, exit 0 - biome lint: PASS
2 participants
The bug
On a multi-item booking page,
buildPageListingRows(reservations/packages.ts) suppressed a standalone row's child selector whenever that listing was a member of any package — on the assumption the member's package row carries the one selector.But a sold-out package (
limit < 1) renders a bare sold-out card with no member rows. So a standalone parent that is also a member of a sold-out package lost its child selector on both paths — the package section (no rows) and its own standalone row (suppressed) — leaving a multi-choice parent unbookable.(Flagged by CodeRabbit on PR #1693; pre-existing behaviour carried verbatim from the original monolith, out of scope for that mechanical split.)
The fix
Build the suppression set from only the packages that actually render member rows — i.e.
packageLimits.get(groupId) >= 1. A standalone parent whose package section was omitted now keepsopts.childCtxand renders its selector.I also verified the
hideListingshalf the TODO flagged: a hidden package's members are never standalone rows (confirmed by rendering one — the member gets no standalone row), so the condition needs nohideListingsterm. Keeping it would only add an unreachable branch.Tests
Adds an in-process regression to
ticket-page-packages.test.ts: a standalone parent whose sibling-capacity sold-out package hid its member row still renders its child selector (data-parent-id). Confirmed it fails on the old code (selector suppressed) and passes on the fix. Coverage of the new lines verified; lint, typecheck, and duplication (0 clones) clean.Removes the corresponding item from
TODO.md.🤖 Generated with Claude Code
https://claude.ai/code/session_013jDnHypgnzR8SWzcoasw6S
Generated by Claude Code
Summary by CodeRabbit