Normalize repeated form choices and table layouts#1866
Merged
Conversation
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:
📝 WalkthroughWalkthroughThis change introduces typed column-layout registries, repeated picklist parsing, updated form validation and rendering APIs, typed test form values, and expanded admin/settings coverage for layouts, recalculation fields, listing defaults, and form behavior. ChangesTyped column layouts
Repeated form values
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: 4
🤖 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 36-37: Remove the suppressions for the validatedColumnLayout
brand-symbol description mutants in equivalent-mutants.txt. Update the
column-layout tests to reflect on the returned layout’s own symbols and assert
the expected brand symbol description, covering both the normal and mutated
descriptions.
In `@src/shared/forms/validation.ts`:
- Around line 74-88: Add a test covering validateCheckboxField when
readRepeatedPicklist produces an invalid selection and the field’s custom
validate function returns an error message. Assert that the result is invalid
and preserves the custom error instead of using invalidFieldError(field).
In `@src/ui/templates/attendee-table.tsx`:
- Around line 75-77: Update the doc comment above columnLayout in the attendee
table props to describe its current AttendeeColumnLayout object contract, not
the removed Liquid string-template format. Mention that it is a pre-parsed
layout, and remove the obsolete template example and historical wording.
In `@test/test-utils/validation.ts`:
- Around line 21-27: Annotate the outer expectInvalid factory with its explicit
curried function return type, specifying the returned function accepts readonly
Field[] and TestFormValues and returns void. Keep the existing validation and
assertion logic 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: 9d459f73-cbd4-4824-a0ac-a398c9a50061
📒 Files selected for processing (43)
scripts/mutation/equivalent-mutants.txtsrc/features/admin/aggregate-recalculation.tssrc/features/admin/dashboard.tssrc/features/admin/settings-general.tssrc/features/admin/settings-listing-defaults.tssrc/shared/column-order.tssrc/shared/columns/attendee-columns.tssrc/shared/columns/listing-columns.tssrc/shared/day-names.tssrc/shared/db/settings.tssrc/shared/forms/definition.tssrc/shared/forms/repeated-picklist.tssrc/shared/forms/saved-data.tssrc/shared/forms/submitted-value.tssrc/shared/forms/validation.tssrc/ui/templates/admin/dashboard.tsxsrc/ui/templates/admin/groups/overview.tsxsrc/ui/templates/admin/guide/operations.tsxsrc/ui/templates/admin/listing-table.tsxsrc/ui/templates/admin/settings/column-order.tsxsrc/ui/templates/attendee-table.tsxtest/features/admin/aggregate-recalculation.test.tstest/features/admin/settings-listing-defaults.test.tstest/integration/server/settings/column-order.test.tstest/integration/server/settings/listing-defaults.test.tstest/lib/server-editor.test.tstest/lib/server-images/attachment-edit.test.tstest/lib/server-images/create.test.tstest/shared/column-order.test.tstest/shared/columns/attendee-columns.test.tstest/shared/db/attendees/api/errors.test.tstest/shared/forms/definition/field-schema.test.tstest/shared/forms/repeated-picklist.test.tstest/shared/forms/validation.test.tstest/test-utils/db-helpers/listing-forms.tstest/test-utils/db-helpers/request.tstest/test-utils/form-values.test.tstest/test-utils/form-values.tstest/test-utils/mocks.tstest/test-utils/validation.tstest/ui/templates/admin/dashboard.test.tstest/ui/templates/attendee-table/template.test.tstest/ui/templates/fields/listing.test.ts
💤 Files with no reviewable changes (1)
- src/shared/forms/submitted-value.ts
stefan-burke
force-pushed
the
normalize/declarative-inputs
branch
from
July 19, 2026 15:46
4af4187 to
997e33a
Compare
stefan-burke
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/column-order.ts (1)
88-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrive column-key validation from the schema
parseColumnTemplatestill does a separateoptions.find(...)membership check, so the allowed keys live in two places and the docstring is stale. Use thev.picklistschema as the single source of truth for validation/error reporting and update the comment to match the current behavior.🤖 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/column-order.ts` around lines 88 - 180, Update parseColumnTemplate and defineColumnLayout so column-key validation uses the existing v.picklist schema rather than a separate options.find membership check, with unknown-key errors derived from schema validation while preserving the current result shape. Pass or reuse the schema where needed, and revise the stale validation doc comment to describe schema-based parsing without claiming column extraction is avoided.
🤖 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/day-names.ts`:
- Around line 19-27: Update VALID_DAY_NAMES to reuse the canonical DAY_NAMES
tuple instead of duplicating weekday literals, and derive its alternate ordering
from that shared source while preserving the existing exported order and
readonly typing.
In `@src/shared/forms/definition.ts`:
- Around line 100-107: Replace the imperative for...of traversal in the
field-validation logic with the repository’s curried collection utility from
`#fp`, preserving the existing select and checkbox-group branches and their
requireChoiceOptions and requireCheckboxOptions calls.
In `@test/shared/forms/definition/field-schema.test.ts`:
- Around line 16-18: Update the checkbox field-definition helper around the
invalidMessage construction to distinguish an omitted value from an explicitly
empty string: use an undefined check rather than truthiness, preserving `""` as
the configured error message. Add coverage for an invalid checkbox submission
configured with an empty invalidMessage.
---
Outside diff comments:
In `@src/shared/column-order.ts`:
- Around line 88-180: Update parseColumnTemplate and defineColumnLayout so
column-key validation uses the existing v.picklist schema rather than a separate
options.find membership check, with unknown-key errors derived from schema
validation while preserving the current result shape. Pass or reuse the schema
where needed, and revise the stale validation doc comment to describe
schema-based parsing without claiming column extraction is avoided.
🪄 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: e1dc01be-0f0e-4134-8a78-1ebd20de0523
📒 Files selected for processing (49)
scripts/mutation/equivalent-mutants.txtsrc/features/admin/aggregate-recalculation.tssrc/features/admin/dashboard.tssrc/features/admin/settings-general.tssrc/features/admin/settings-listing-defaults.tssrc/shared/column-order.tssrc/shared/columns/attendee-columns.tssrc/shared/columns/listing-columns.tssrc/shared/day-names.tssrc/shared/db/settings.tssrc/shared/form-data.tssrc/shared/forms/definition.tssrc/shared/forms/saved-data.tssrc/shared/forms/submitted-value.tssrc/shared/forms/validation.tssrc/ui/templates/admin/dashboard.tsxsrc/ui/templates/admin/groups/overview.tsxsrc/ui/templates/admin/guide/operations.tsxsrc/ui/templates/admin/ledger/entry-pages.tsxsrc/ui/templates/admin/listing-table.tsxsrc/ui/templates/admin/news.tsxsrc/ui/templates/admin/questions.tsxsrc/ui/templates/admin/settings/column-order.tsxsrc/ui/templates/admin/site-pages.tsxsrc/ui/templates/attendee-table.tsxtest/features/admin/aggregate-recalculation.test.tstest/features/admin/settings-listing-defaults.test.tstest/integration/server/settings/column-order.test.tstest/integration/server/settings/listing-defaults.test.tstest/lib/server-editor.test.tstest/lib/server-images/attachment-edit.test.tstest/lib/server-images/create.test.tstest/shared/column-order.test.tstest/shared/columns/attendee-columns.test.tstest/shared/db/attendees/api/errors.test.tstest/shared/form-data.test.tstest/shared/forms/definition/define-form.test.tstest/shared/forms/definition/field-schema.test.tstest/shared/forms/validation.test.tstest/test-utils/db-helpers/listing-forms.tstest/test-utils/db-helpers/request.tstest/test-utils/form-values.test.tstest/test-utils/form-values.tstest/test-utils/mocks.tstest/test-utils/validation.tstest/ui/templates/admin/dashboard.test.tstest/ui/templates/admin/ledger/forms.test.tstest/ui/templates/attendee-table/template.test.tstest/ui/templates/fields/listing.test.ts
💤 Files with no reviewable changes (1)
- src/shared/forms/submitted-value.ts
Member
Author
|
Resolved the outside-diff |
![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: 1384659ed4
ℹ️ 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".
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/column-layout.ts`:
- Around line 39-54: Refactor the tag scan in the column-layout parser to use
the project’s `#fp` collector style with a small curried helper instead of the
current for...of loop. Keep parseTagBody, schema validation, deduplication,
filter storage, and column accumulation equivalent, and preserve the immediate
error result when an unknown column is encountered.
🪄 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: d25ac3ef-c93c-418a-93e6-36e97a0025c1
📒 Files selected for processing (29)
scripts/mutation/equivalent-mutants.txtsrc/features/admin/dashboard.tssrc/features/admin/entity-write-tab.tssrc/features/admin/listings-edit.tssrc/features/admin/logistics-agent-page.tsxsrc/features/admin/modifiers.tssrc/features/admin/settings-general.tssrc/shared/column-layout.tssrc/shared/column-order.tssrc/shared/columns/attendee-columns.tssrc/shared/columns/listing-columns.tssrc/shared/db/settings.tssrc/shared/form-data.tssrc/ui/templates/admin/dashboard.tsxsrc/ui/templates/admin/guide/operations.tsxsrc/ui/templates/admin/settings/column-order.tsxsrc/ui/templates/attendee-table.tsxtest/features/admin/dashboard.test.tstest/features/admin/dashboard/listings-csv.test.tstest/features/admin/dashboard/log.test.tstest/integration/server/listings/create-validation.test.tstest/lib/server-listings/log-and-activity.test.tstest/shared/column-layout.test.tstest/shared/column-order.test.tstest/shared/columns/attendee-columns.test.tstest/shared/form-data.test.tstest/test-utils/session.tstest/ui/templates/admin/dashboard.test.tstest/ui/templates/attendee-table/template.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c5e34572f
ℹ️ 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".
Comment on lines
+42
to
+43
| if (!selection.ok) { | ||
| throw new Error(`Invalid recalculation field: ${selection.error}`); |
There was a problem hiding this comment.
When an authenticated listing/modifier/answer recalculate POST contains a stale or tampered recalculate_fields value, this exception bubbles through runRecalculatePost/withAuth without being converted to a response, so the operator gets a 500 instead of the same 400 re-render used for the empty-selection validation path. Treat the unknown choice as a form validation error at this boundary so bad request data fails closed without crashing the route.
Useful? React with 👍 / 👎.
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
1 participant
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.
Summary
Review fixes
#fpChecks
nix develop -c deno task precommitSummary by CodeRabbit
New Features
Bug Fixes