Explain why a listing can't join a package (and a few other blocked combinations)

[stefan-burke]

Why

I went looking for every place the site refuses a combination someone might reasonably expect to work — the "why can't I select this?" moments that turn into support queries — and checked whether each one has a genuinely unavoidable reason or is just poorly explained. This PR ships the clearest wins where the rule is fine but the message left people guessing. The rest are written up in TODO.md.

What changed

Packages now tell you which listing is the problem, and why.
Before, adding an incompatible listing to a package showed one catch-all message that listed every possible reason at once and never said which of your listings tripped it:

Packages cannot contain pay-what-you-want listings or listings that are add-ons (children) of another listing, and a hidden package's listings cannot require their own add-ons. Remove those listings first.

Now you get the one reason that actually applies, naming the listing and the exact fix — for example:

Packages cannot contain 'Balloon Ride' — it lets the buyer choose their own price, and a package needs a set price for each item. Remove it from the package, or turn off Allow Pay More.

The two "add child listings" conflicts (a hidden package's listing gaining its own add-ons, or picking a package's listing as an add-on elsewhere) shared that same vague message and now have their own clear ones too.

A daily add-on now explains why it needs a daily parent — it takes its date and length from the listing it's booked under, and only a daily listing has them.

The payment-provider chooser now says that switching providers keeps your other provider's saved key (it just stops using it), so entering Square doesn't look like it wiped your Stripe setup.

The free-text question note now says a free-text question can't change the price, since a price change attaches to a chosen answer and a typed answer has none.

Under the hood

The package-membership rule had been written out three separate times (the group save, the listing save, and the catalog importer). This pulls it into one shared, tested helper (src/shared/package-membership.ts) so the rule and its wording can't drift, and removes the now-dead duplicate paths.

Not in this PR (recorded in TODO.md)

The audit turned up more than a quick copy tweak could cover. TODO.md now has a "Restrictions audit" section splitting the rest into message/UX fixes (flag SumUp as unavailable for your currency before you paste a key; a hint on the answer's price-modifier dropdown; moving the hardcoded group-homogeneity messages into the translatable copy; mutual-disable for the paired either/or checkboxes; naming the items when a multi-item cart shares no common date; linking the manual-adjustment page from the "refund not recorded" notice) and rule relaxations (multiple payment providers, percentage surcharges above 100%, deleting an in-use status by reassigning its holders, packages containing pay-what-you-want listings, two-level nesting, and more) — each with a starting point.

Testing

• New unit test for the shared package-membership helper (all branches).
• Updated the package and daily-add-on tests to assert the new, more specific messages (and that they explain the why).
• Full suite, typecheck (including test files), strict lint, and the 0% duplication check all pass locally.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MD8aVv9TPPMx1rH3NgkhqN

---

Generated by Claude Code

Summary by CodeRabbit

• Improvements
  • Package and child-listing compatibility now surfaces more precise, localized errors for specific invalid combinations (including pay-more/add-on/hidden-gating and package-parenting conflicts), with consistent messaging across relevant admin flows.
  • Clarified daily listings guidance (date/length inheritance from the parent) for better children/add-on understanding.
  • Updated payment-provider settings help to explain saved key retention and temporary provider switching behavior.
  • Expanded free-text question UI copy to confirm free-text answers can’t change pricing.
• Documentation
  • Added “Restrictions audit — ‘why can’t I combine X with Y?’ follow-ups” to track remaining restriction and messaging work.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 437f63c3-395b-469d-bfb7-9d4ea5dfa9b8

📥 Commits

Reviewing files that changed from the base of the PR and between a5196bd and d8fdb65.

📒 Files selected for processing (1)

• TODO.md

---

📝 Walkthrough

Walkthrough

Package restriction validation now reports specific blocking reasons across shared and admin flows. English error messages, tests, restriction TODOs, and explanatory UI copy were updated; no other functional areas changed.

Changes

Package restriction validation

Layer / File(s)	Summary
Shared package blocking rules src/shared/package-membership.ts, src/shared/db/groups.ts, src/shared/listings-actions.ts	Typed membership and child-edge conflicts now produce specific localized errors used by listing validation.
Admin and import validation integration src/features/admin/...	Admin joins, child management, group validation, and catalog imports now use the specific package restriction errors.
Localized messages and validation coverage src/locales/en/..., test/...	Generic package errors were replaced with specific messages, and tests assert the new wording and blocking reasons.
Restriction follow-ups and explanatory copy TODO.md, src/ui/templates/admin/questions.tsx, src/locales/en/..., test/ui/...	Restriction audit notes and explanatory copy for listing, payment-provider, and free-text question behavior were expanded. User interface tests were updated accordingly.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AdminRequest
  participant GroupValidation
  participant PackageMembership
  participant ErrorLocalization
  AdminRequest->>GroupValidation: validate package or child relationship
  GroupValidation->>PackageMembership: determine blocking reason
  PackageMembership->>ErrorLocalization: format reason-specific message
  ErrorLocalization-->>AdminRequest: return validation error

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: more specific explanations for package/listing blocking rules and related combinations.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/feature-restrictions-audit-gu66aa

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@test/shared/package-membership.test.ts`:
- Around line 79-116: Update the tests for packageMemberBlockError and
packageChildEdgeError to assert each complete expected localized message with
exact equality instead of multiple toContain checks. Preserve the existing
reason-specific inputs and expected user-facing wording for all five cases.

In `@test/ui/templates/admin/questions.test.ts`:
- Around line 212-216: Strengthen the assertions in the admin question rendering
test by checking the complete explanatory text, including both the absence of
answer options and the selected-answer rationale, rather than asserting only the
price conclusion. Replace the broad presence checks near the existing html
assertions with an exact or sufficiently specific assertion that covers the full
explanation while preserving the existing no-form assertion.

🪄 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: dc4b189e-3645-4c06-acdb-16abe628c142

📥 Commits

Reviewing files that changed from the base of the PR and between 9c67da9 and 0877ce8.

📒 Files selected for processing (17)

• TODO.md
• src/features/admin/api.ts
• src/features/admin/catalog-transfer/import.ts
• src/features/admin/groups.ts
• src/features/admin/listings-parents.ts
• src/locales/en/errors.json
• src/locales/en/listings-table.json
• src/locales/en/settings.json
• src/shared/db/groups.ts
• src/shared/listings-actions.ts
• src/shared/package-membership.ts
• src/ui/templates/admin/questions.tsx
• test/lib/server-group-packages.test.ts
• test/shared/db/listing-parents.test.ts
• test/shared/listings-actions.test.ts
• test/shared/package-membership.test.ts
• test/ui/templates/admin/questions.test.ts

[coderabbitai]

Actionable comments posted: 5

🤖 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/features/admin/catalog-transfer/import.ts`:
- Line 20: Move the reusable DB-backed packageMembersError validation helper out
of the groups route module into a neutral validation module. Update both the
groups route and the catalog import flow to import it directly from the new
module, preserving its current behavior and avoiding route-to-route coupling.

In `@src/features/admin/groups.ts`:
- Around line 97-104: Replace the imperative for...of scan in the
listing-selection flow with the repository’s curried `#fp` find or reduce
composition. Preserve packageMemberBlock evaluation order, return the first
truthy block with its listing, and retain the existing null result when no
listing matches.

In `@test/lib/server-group-packages.test.ts`:
- Line 600: Replace the partial body.error assertions in the server-group
package tests, including the corresponding checks around lines 624, 644, and
677, with exact comparisons against the appropriate localized error or flash
message. Use the existing localization symbols or complete structured response
expected by each scenario, while preserving the current test setup and behavior.

In `@test/shared/db/listing-parents.test.ts`:
- Around line 302-303: Strengthen the assertion in the daily-direction error
test by comparing the complete localized error message instead of checking only
the substring “takes its date and length from the parent.” Preserve the existing
error setup and assert the full stable message, or all stable structured fields
if the error is structured.

In `@TODO.md`:
- Around line 639-644: Update the TODO entry describing validateCalcValue so its
classification matches the document’s stated policy: either explicitly mark the
>100% surcharge rejection as an intentional restriction and remove the
bug-fix/workaround language, or classify it consistently as a known correctness
bug. Keep the behavior recommendation aligned with that classification.

🪄 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: bef247c2-8940-4d4a-8ab2-1e1ee171253f

📥 Commits

Reviewing files that changed from the base of the PR and between 0877ce8 and eca223d.

📒 Files selected for processing (17)

• TODO.md
• src/features/admin/api.ts
• src/features/admin/catalog-transfer/import.ts
• src/features/admin/groups.ts
• src/features/admin/listings-parents.ts
• src/locales/en/errors.json
• src/locales/en/listings-table.json
• src/locales/en/settings.json
• src/shared/db/groups.ts
• src/shared/listings-actions.ts
• src/shared/package-membership.ts
• src/ui/templates/admin/questions.tsx
• test/lib/server-group-packages.test.ts
• test/shared/db/listing-parents.test.ts
• test/shared/listings-actions.test.ts
• test/shared/package-membership.test.ts
• test/ui/templates/admin/questions.test.ts