Import/export a listing or group as JSON, with cross-entity name uniqueness - #1511
Conversation
Add a name-registry that treats listing and group display names as one namespace: a listing may not share a name with another listing or with a group, and vice versa. Names are the stable key the coming catalog import/export will reference, so uniqueness is what keeps that resolution unambiguous. - New src/shared/db/name-registry.ts: cached, decrypt-in-memory name index (no schema column needed) providing isNameTakenAnywhere plus name→id resolution (matchName) with missing/ambiguous outcomes. - Wire the check into validateListingInput and validateGroupWithPackage (covering HTML forms and the admin JSON API for both entities), and add the missing validate to the group create resource so creation is guarded too. - New error.name_in_use message. - Tests for the registry and both validators; update the old "duplicate slug auto-uniquifies" listing test to assert the new name-uniqueness rejection. - Drive-by: fix a pre-existing noUselessFragments lint error in entity-pages.tsx that blocks lint:ci. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
Introduce src/features/admin/catalog-transfer with a versioned, id-free JSON format that captures a single listing or group and all its facets — prices, group memberships (with package price/quantity/day overrides), and parent references — cross-referenced by name so a blob is portable across installs. - schema.ts: valibot discriminated union (listing/group) as the single source of truth; validates an incoming blob at the boundary and types the exporter's output. formatTransferIssues renders per-field messages. - export.ts: build the blob from the decrypted stored row and its facets, resolving every reference to a name. Images/attachments, ledger, and attendees are deliberately excluded. - import.ts: parse → check name uniqueness → resolve parents/groups/ members by name (intelligible missing/ambiguous errors) → reuse the shared listing/group validators (type compatibility, package rules, parent-edge compatibility) → write in one transaction. Never throws for bad input. Supporting db helpers: getListingGroupMemberships + addGroupMembershipTx (groups), listingGroupDayInsertStatements (listing-prices, targeted so importing into a populated package can't disturb other members), addParentEdgesTx (listing-parents), and an exported allPackageableMembers. Round-trip and validation-failure tests cover both entity kinds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
Add content-gated admin routes and UI for the transfer engine: - GET /admin/listing/:id/export.json and /admin/groups/:id/export.json download the entity as a JSON attachment (named from the entity). - GET/POST /admin/catalog/import: an upload form that parses the JSON, runs importCatalog, and redirects with a success flash to the created entity's list — or an intelligible error flash on any failure (invalid JSON, missing file, or a validation/resolution error). - Export links on the listing and group detail pages; an "Import from file" button on the listings and groups list pages. - New catalog-transfer i18n namespace. Route- and template-level tests cover download, the round-trip upload, and every error path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
Strengthen tests so the transfer schema/export/import/routes reach a 100% mutation kill rate, and record the genuinely-equivalent survivors: - Cover a package member priced explicitly free (0 vs null), closesAt preservation and the post-import price re-sync, an incompatible parent edge, a package-member-with-parent conflict, root-type and nested-field parse messages, and activity logging on import. - Simplify formatTransferIssues to root+nested only (the object-variant schemas never produce a pathless "other" issue), removing dead branches. - Record array/object `?? → ||` equivalents in equivalent-mutants.txt. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
Add rendering assertions for the listing/group export links and the import-from-file buttons, and test the entity-pages activity "view all" link — bringing every template/module the feature touched under a changed test so the mutation gate has its killers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
Extract the catalog-import group-membership read/write helpers into src/features/admin/catalog-transfer/membership.ts, reverting the additions to src/shared/db/groups.ts and src/shared/db/listing-prices.ts so those heavily-shared modules leave the change set entirely (addGroupMembershipTx now reuses the existing groupDayPriceStatements, dropping its leading full-group DELETE). Bring the covering tests for the remaining touched modules into the change set with real assertions: addParentEdgesTx, the group add-listings activity log, an API-level group name-uniqueness rejection, and the sold-hidden-package delete guard. Refresh the shifted listing-parents equivalent-mutant line numbers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
Add editor-role tests: an editor can open the import page and the listing/group export routes (all content-gated), and can import a listing from an uploaded JSON file. The export links themselves live on the staff-only detail pages, but the export routes are content-gated so editors reach them directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
Touching validateListingInput / validateGroupWithPackage pulls their whole files into the mutation gate, surfacing latent coverage gaps in code the feature does not itself change. Fill them with direct tests and record the provably-equivalent survivors: - listings-actions: unit tests for toggleListingActive (no-op, deactivate, reactivate + activity log), performListingDelete (row + storage-file cleanup + activity log), package-membership edge rules (child-of-another, hidden vs visible package gating children), renewal config (mixed purchase-only/hidden), maxPrice arithmetic, and the create-ignores-slug path. Record the optional-field `?? →||` defaults as equivalents. - groups: a regular group with a sold-out but visible member stays shareable (kills the share-computation `!` mutant); record the object/ Map/0-fallback `?? →||` equivalents. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7799f3ce42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Apply the review fixes (bounded batched membership writes, day-count key validation, builder-disabled and editor policy handling, duplicate-reference rejection, nested-parent guard) and bring the new modules to 100% line and branch coverage: drop the unreachable name-lookup guards in the exporter, pass resolved group ids into the parent-edge check to remove a dead nullish branch, and cover the empty-name, punctuation-only-slug, and package-member-as-child edges. Dedupe the two builder-site import tests behind a shared helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
- schema.ts: define PriceSchema via intAtLeast(0) instead of aliasing NonNegativeIntSchema, which the code-quality no-aliasing rule forbids. - admin-api-security: give the two content-type-case POSTs distinct listing names so the second is not rejected by the new name-uniqueness rule (the test is about content-type handling, not uniqueness). - server-agent-deliveries: the run-sheet tie-break fixture created two listings with the same name; names are now unique, so the second uses a distinct name and the tie-break resolves deterministically by name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef824820df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Hide webhook_url from listing exports for editors (P1): the edit form already hides this PII-sink URL from editors, so an editor's export must not reveal it. exportListing takes the admin level and excludes the column. - Clear uses_logistics on import when logistics is disabled, mirroring the form which forces it off (assign_built_site already did this). - Reject non-date closesAt/date blobs with a field error instead of letting the datetime normaliser silently store an empty value. - Constrain bookableDays to real weekday names so a typo is a field error rather than a daily listing whose dates never match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a04e8affb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…cation Codex #2: a valid export with >25 parents tripped the request N+1 guard (one getParentIds query per parent) and the transaction round-trip cap (one INSERT per parent). Batch the nested-parent check via getChildListingIds and make addParentEdgesTx a single multi-row INSERT. Codex #3: the group-duplicate flow cloned listing names verbatim via raw insertStatement, bypassing the new uniqueness validator and leaving duplicate names that make later name-based imports ambiguous. Validate the new group name and every clone name (against the catalog and within the batch) before writing, rejecting with an operator-facing message. Existing duplicate tests updated to supply a find/replace that keeps clone names unique. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 449223fa97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ): Promise<boolean> => { | ||
| const key = normalizeEntityName(name); | ||
| if (key === "") return false; | ||
| const { group, listing } = await loadCatalogNameIndex(); |
There was a problem hiding this comment.
When two listing/group creates or imports with the same normalized name overlap, both can pass this decrypted cache scan before either insert commits; there is no DB-level unique key on the encrypted names and the write transaction does not re-check the namespace. That leaves duplicate names created by new writes, making later catalog imports ambiguous despite the new invariant; enforce this with a normalized blind index/unique constraint or re-check under the serialized write path before inserting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Deferring to a dedicated follow-up. This is a pre-existing property of the name registry, not introduced by this PR: the create/import paths scan the decrypted name cache and there's no DB-level unique key on the encrypted names. Closing the TOCTOU window properly needs a normalized blind-index UNIQUE constraint — a schema migration plus a backfill of every existing listing/group name plus a re-check under the serialized write across all create paths — which is materially broader than this import/export change. Tracking it separately rather than bolting a partial guard on here.
Generated by Claude Code
Codex kept finding fields where a raw import/API build accepts a value the listing form would reject (over-cap duration silently clamped, invalid contact fields dropped, bad weekday names, impossible datetimes rolled over). Root cause: the form's per-field validators live only in the form's field defs, so the JSON API and the catalog import bypass them. - Extract those validators (contact fields, weekday names, duration cap, plus a combined validateListingFieldValues) into #shared/listing-field-validators; the form re-exports them so its behaviour is unchanged, and the JSON API now runs them too (validateListingApiInput). - The catalog transfer schema validates the same field values at its wire boundary, including a self-contained strict datetime check that rejects impossible calendar dates (e.g. 2026-02-30) rather than letting the storage layer roll them over. Kept self-contained (no Temporal/timezone import) so this early-loaded schema module stays out of the settings-loading graph. - Field-value import tests live in their own file to keep the main catalog-transfer suite's per-request read count under the N+1 guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 434c693b03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The previous commit ran the form's field validators through the JSON API too,
but the API deliberately CLAMPS out-of-range values (no form layer — see the
duration-days e2e), so rejecting there broke intended behaviour and a
code-quality gate. Revert the API and the fields.ts extraction; keep the actual
fix — the catalog transfer schema validates datetime, duration cap, contact
fields, and weekday names at its wire boundary, which is where the import
findings apply.
Also anchor the datetime regex so a valid prefix with trailing junk
("2030-01-01T00:00not-a-zone") is a field error rather than being silently
emptied by the storage normaliser.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
…isable in demo Three more Codex import-boundary fixes: - Clear package price/quantity/day-price overrides for any membership whose group isn't a package (listing import, per group) or whose imported group blob isn't a package — matching the normal group save, so a blob can't plant a hidden free price that activates if the group is later converted. - Reject a parent that is a hidden-package member (batched via getGroupIdsByListingIds + the cached group set, no N+1), mirroring the edge editor's packageChildEdgeConflict rule. - Disable catalog import in demo mode, since a raw blob bypasses the form's demo-field scrubbing and webhook clearing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
…1 guard Importing a listing that belongs to many groups routed through validateListingInput, whose group check ran one sibling SELECT per group via validateGroupListingType. Past ~25 groups the per-request N+1 read guard tripped before any write, so a valid export of a group-heavy listing could not be imported. validateListingGroup now batches those reads: one cached getAllGroups() plus one getListingsByGroupIds() for every referenced group, with the homogeneity check run in memory per group via a new groupListingTypeError helper (the pure core validateGroupListingType now delegates to). getListingsByGroupIds is the batched, inactive-inclusive form of getListingsByGroupId; getActiveListingsByGroupIds becomes a thin wrapper over it. Reads are now constant regardless of group count, and the change is behaviour-preserving for the form and API create/edit paths. Covered by an "imports a listing that belongs to many groups" test (30 groups). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7f359f8b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const [memberships, groupNames, parentIds] = await Promise.all([ | ||
| getListingGroupMemberships(id), | ||
| getAllGroupNames(), | ||
| getParentIds(id), |
There was a problem hiding this comment.
When exporting a listing that is itself a parent, the blob only records the listings this row is offered under, not the required children it offers. Re-importing that parent therefore creates a standalone listing with no child selector, so buyers can book the parent alone and the add-on structure is lost; include child references in the transfer or reject/export-warn for parent listings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is by design. The transfer format records a listing's parents (what it's offered under), not the children it offers, because children are themselves separate listings — the format is single-entity, and every cross-reference (parents, group memberships, group members) is a by-name reference to an entity that already exists, never an embedded subtree. The add-on structure is preserved by exporting/importing each child, which references this parent by name on its own import (parents must already exist, which is exactly why the import resolves parent references rather than creating them). Exporting a lone parent yielding a standalone listing until its children are imported is the intended boundary, consistent with how memberships and parents are all handled.
Happy to revisit if you'd rather the format embed child references, but that's a format-scope change I'd want your call on rather than make unilaterally.
Generated by Claude Code
…e-group check Three follow-ups from review on the prior commit: - The "imports a listing that belongs to many groups" test created its 30 groups via createTestGroup, whose trailing getAllGroups() re-reads the cache (invalidated by each create) 30 times in one test context and trips the N+1 read guard during setup. Build the groups with a direct groupsTable.insert instead, mirroring the many-parents test. - isStorableDatetime accepted out-of-range timezone offsets like "+99:99" because the offset digits were never range-checked; the storage normaliser then treats the whole value as invalid and silently empties the column. Capture the offset hours/minutes and reject > 23 / > 59, and cover it with an "out-of-range offset" import test. - firstPackageGroup still did one groupsTable.findById per group, so a child listing that joins many groups could trip the N+1 guard before the batched parent validation. Resolve against the request-cached group set via a new shared getGroupsById() helper (also used by validateListingGroup, replacing its inline map). Covered by an "imports a child listing that also belongs to a regular group" test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c062e72f8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The datetime regex placed the optional fractional-seconds group outside the optional seconds group, so "2030-01-01T00:00.123Z" (a fraction with no seconds) passed validation; the storage normaliser then treats the whole value as invalid and silently empties the date/closesAt column. Tie the fractional part to the seconds group so a fraction is only accepted after seconds. Covered by "rejects fractional seconds without a seconds component" and a positive "accepts fractional seconds after a seconds component" test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c945835cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
intAtLeast only checked integer-ness and the lower bound, so a price like 1e100 (an "integer" to Number.isInteger but outside the safe range) passed and would be rounded or throw a raw error at the storage layer. Require v.safeInteger() in the shared intAtLeast helper (and DurationDaysSchema), which covers prices, quantities, counts, and durations at once, matching the form's money parser. Covered by a "rejects a price above the safe-integer range" test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
…n import The package editor only renders day-price override inputs for a member's available day counts (a customisable listing's priced spans within its duration). A raw import bypassed that, persisting a group_day override for any day count in the global 1..90 range — a hidden row that could activate after a later duration/day-price edit. Add memberDayOverrideError, reused on both membership sides: a group import validates each existing member's overrides, and a listing import validates the new listing's own group overrides, against availableDayCounts. Out-of- range overrides are now a field-level error. The tests live in a new catalog-transfer-packages.test.ts (creating customisable members is read-heavy; folding them into catalog-transfer.test.ts tips that file past the per-request N+1 read guard). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
An imported listing created as a child that also joins a group could inherit a group-scoped opt-in add-on. If that add-on's would-be scope reached only the (suppressed) child and not the parent's booking page, the add-on became unbookable — the dead-end the interactive child-edge editor already rejects, but which the import skipped (it stopped at field compatibility). validateParentEdges now builds the would-be listing set (the new child appended at placeholder id 0 with its would-be group memberships) and, per named parent, runs the same reachability core the edge editor uses. To keep it under the request N+1 guard for a many-parent import, add childOnlyAddOnCheckerForListings, which resolves every add-on's scope once and returns a reusable per-parent checker. Covered by rejects/accepts tests in a new catalog-transfer-reachability.test.ts (its modifier setup is read-heavy, so it lives outside catalog-transfer.test.ts). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a60d936bc4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rices Two linked findings on the export/import schema: - Export no longer 500s on a stored row the transfer schema can't represent. A listing created through the JSON API can hold values the API accepts but the transfer format rejects (e.g. bookable_days ["Funday"], an invalid fields string), and export ran them through a strict v.parse that threw. exportListing/exportGroup now return a CatalogExportError (via a safeParse helper) which the download route surfaces as an operator-facing 422, never a raw 500. - A listing's own dayPrices beyond its durationDays are now filtered on import, matching the form which only reads day_price_1..durationDays. A "5"-day price on a 2-day listing was persisted and would activate if the duration were later raised; it's dropped instead. (This is the listing's own field; package-member overrides for unoffered spans are still rejected, mirroring the package editor.) Covered by "returns 422 …" listing and group export tests and a "filters a listing's own day prices beyond its duration" import test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
The dayPrices filter's `durationDays ?? 1` right operand was never exercised (every test set durationDays), so deno flagged schema.ts:209. Add an import test with dayPrices and no durationDays. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
The new default-duration test duplicated the import+unwrap+getListing block, tripping the test cpd gate. Extract importStoredListing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
…import-export-l0d4b8 # Conflicts: # scripts/mutation/equivalent-mutants.txt # src/shared/db/groups.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d2c463907
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The transfer schema predated the bookable_alone column, so rowToInput supplied bookableAlone but ListingFieldsSchema never declared it and parseExport dropped it. A re-imported child then defaulted to false, losing its standalone /ticket page, catalog/API eligibility, and any add-on reachability that depended on the child's own page. Declare the field so it round-trips. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
The merge unified getListingsByGroupIds to seed an entry for every id it is asked about, so siblingsByGroup.get(groupId) always resolves for the ids validateListingGroup iterates. The '?? []' fallback was therefore unreachable — a branch the 100% coverage gate could never cover. Assert the invariant instead so the branch is gone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccba2f5662
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Now that imports carry bookableAlone, a child imported with bookable_alone=true kept its own /ticket page but was still run through the child-only add-on reachability guard, so a valid exported child in an add-on-scoped group could be rejected when its parent wasn't in scope. The edge editor exempts a bookable_alone child (its own page still offers the add-on); mirror that by skipping the guard here too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX
…import-export-l0d4b8 # Conflicts: # src/ui/templates/admin/listings.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b91c00349
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| // A JSON export download (see catalog-transfer). Content-gated like the tab, | ||
| // and a read, so — unlike duplicate — it stays available in read-only mode. | ||
| href: (entity) => actionUrl(entity, "export.json"), |
There was a problem hiding this comment.
For editor sessions, this export action never renders because LISTING_ACTIONS is only displayed inside the Actions tab, and that tab is visible: staffOnly; the only group export link has the same problem on the staff-only group detail page. Since the new export routes themselves are content-gated and the import buttons are visible to editors, editors can import catalog files but cannot discover/export one without hand-crafting the URL, so the export links need to live on an editor-visible edit/list surface or the tab/page gate needs to change.
Useful? React with 👍 / 👎.
| const listingData = parseExport( | ||
| ListingDataSchema, | ||
| { | ||
| ...listingsTable.rowToInput( |
There was a problem hiding this comment.
When a listing has use_defaults enabled, this exports the stored row values plus useDefaults: true, but the transfer file does not include the source site's listing defaults. If an operator exports a default-inheriting listing whose effective hidden, bookableDays, webhookUrl, or thankYouUrl now comes from defaults, importing it into another site with different or no defaults silently changes that listing's behavior; staff exports should either materialize the effective defaulted fields and clear useDefaults, or carry enough default data to preserve the source behavior.
Useful? React with 👍 / 👎.
| export const ListingTransferSchema = v.object({ | ||
| groups: v.optional(v.array(ListingMembershipSchema), []), | ||
| kind: v.literal("listing"), | ||
| listing: ListingDataSchema, | ||
| parents: v.optional(v.array(NameRefSchema), []), |
There was a problem hiding this comment.
With these relationship arrays optional and defaulting to [], a same-version file that misspells a key such as parent/group instead of parents/groups is accepted as if it intentionally had no parent edges or group memberships, creating a standalone listing and silently dropping structure. Because the format is versioned and exact-version gated, unknown keys in the transfer objects should be rejected (for example with strict object schemas) rather than treated as omitted optional fields.
Useful? React with 👍 / 👎.
…ity (chobbledotcom#1516) Follow-up to the three Codex P2 findings on chobbledotcom#1511: - schema.ts: the transfer schemas are now strictObject, so an unknown or misspelled key (e.g. "parent" for "parents") in a versioned, exact-version-gated file is a field error rather than being silently dropped — which would otherwise import a listing with no parents/groups or a missing column. - Export links now also render on the editor-visible edit surfaces (the listing Edit tab and the group edit form), not only the staff-only Actions tab / group detail page. Editors, who can already import, can now discover and export via the UI (the export routes are content-gated). - Lock in that export/import mirror raw database cells, never the read-time default overlay: a use_defaults listing round-trips its own stored cells (imported even when they conflict with a site default) and the export reflects those cells, not the resolved values. No source change — a test guards the intended behaviour. Claude-Session: https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX Co-authored-by: Claude <noreply@anthropic.com>
2 participants
What
Adds the ability to export a single listing or group to a JSON blob and import it back — into this install or another — capturing all of the entity's facets, and enforces name uniqueness so those blobs can reference the catalog by name.
Name uniqueness (new invariant)
A listing or group display name is now unique across both tables: a listing may not share a name with another listing or with a group, and vice versa. Enforced on add and edit, for HTML forms and the admin JSON API, for both entity kinds.
src/shared/db/name-registry.ts: a cached, decrypt-in-memory name index (no schema column needed) —isNameTakenAnywhereplus name→id resolution withmissing/ambiguousoutcomes.validateListingInputandvalidateGroupWithPackage(the shared validators both paths funnel through); the group create resource gains the missingvalidate.error.name_in_usemessage.Catalog transfer (
src/features/admin/catalog-transfer/)A versioned, id-free JSON format — every cross-reference (a listing's parents and group memberships, a group's member listings) is by name.
schema.ts— a valibot discriminated union (kind: "listing" | "group") that is the single source of truth: validates an uploaded blob at the boundary with per-field messages, and types the exporter's output. Day-count keys and capacities are range-checked so a typo or zero-capacity blob is a field error, not a silent drop.export.ts— builds the blob from the decrypted stored row and its facets: prices (unit_price/day_prices), group memberships with package price/quantity/per-day overrides, and parent references. Images/attachments, ledger connections, and attendees are deliberately excluded.import.ts— parse → check name uniqueness → resolve parents/groups/members by name (intelligible errors on a missing/ambiguous/duplicate name) → reuse the existing listing/group validators (type compatibility, package rules, single-level parent-edge compatibility) → write in one transaction. Role-aware policy is re-applied so an editor import can't set a webhook URL /use_defaultsand a built-site assignment is cleared where the builder is disabled. Never throws for bad input; every failure returns an operator-facing message.membership.ts— the import membership writers, batched into at most two multi-row INSERTs so a large group stays under the interactive-transaction round-trip cap.Admin UI & routes (content-gated — owner/manager/editor)
GET /admin/listing/:id/export.jsonand/admin/groups/:id/export.jsondownload the entity as a JSON attachment.GET/POST /admin/catalog/import— an upload form that applies a blob and redirects with a success or error flash. Editors can reach the import page and the export routes.catalog-transferi18n namespace.Notes
membership.ts) rather than in#shared/db/groups.ts/listing-prices.ts, keeping those heavily-shared modules out of the change set.Testing
test.yml: lint:ci, typecheck, cpd, build:edge, and the full suite under the 100%-coverage gate) passes on the head commit.🤖 Generated with Claude Code
https://claude.ai/code/session_013zHSr7TjvPW4WVciygozHX