Lower production code complexity limit to 12 - #1891
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR refactors admin, payment-processing, booking, shared-data, merge, site-assignment, and reservation UI flows into focused helpers. It adds broad integration and unit coverage for payment outcomes, catalog imports, capacity rules, attendee merges, manual check-in, and reservation rendering. ChangesCore refactors and behavioral coverage
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![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: fc7c89b299
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/shared/merge/attendee-merge/apply.test.ts (1)
62-69: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse
saveConflictAnswerChoiceat the initial setup too. The inlinesaveChoicepair at lines 113-114 duplicates the helper already reused later in this file.🤖 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 `@test/shared/merge/attendee-merge/apply.test.ts` around lines 62 - 69, Replace the duplicated inline saveChoice calls in the initial setup with the existing saveConflictAnswerChoice helper, passing the same target, source, and answers values; preserve the setup behavior and reuse the helper already defined in the test file.Source: Coding guidelines
🤖 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 40-44: Resolve the conflict in the equivalent-mutant manifest by
reconciling scripts/mutation/equivalent-mutants.txt with origin/main and
retaining every valid entry from both versions, including the listed ?? → ||
cases. Remove all conflict markers, then rerun merge-check and mutation
validation to confirm the manifest and PR are mergeable.
In `@src/features/admin/entity-pages.ts`:
- Around line 276-289: Update loadPageSections to load all active.sections
concurrently with Promise.all over a preferred curried functional utility from
`#fp`, replacing the sequential imperative loop while preserving section order and
the existing custom panel behavior.
In `@src/shared/site-assignment.ts`:
- Around line 89-105: Update SITE_ASSIGNMENT_FAILURE_REPORTS so each
notification value reuses the corresponding ErrorCode member instead of
duplicating its string literal, keeping code and notification synchronized for
builder_disabled, initial_months, and missing_tier.
- Around line 108-119: Update reportSiteAssignmentFailure to include
failure.listingId in the log detail when it is present, while preserving the
existing reason and skippedCount information and avoiding an empty or undefined
listing identifier for other failure reasons.
---
Outside diff comments:
In `@test/shared/merge/attendee-merge/apply.test.ts`:
- Around line 62-69: Replace the duplicated inline saveChoice calls in the
initial setup with the existing saveConflictAnswerChoice helper, passing the
same target, source, and answers values; preserve the setup behavior and reuse
the helper already defined in the test file.
🪄 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: bd2a8e9a-c2d4-462a-91fd-85941b1a4ce3
📒 Files selected for processing (66)
biome.jsonscripts/mutation/equivalent-mutants.txtsrc/features/admin/attendee-page-data.tssrc/features/admin/catalog-transfer/import.tssrc/features/admin/entity-pages.tssrc/features/api/payment-processing/index.tssrc/features/api/payment-processing/items.tssrc/features/api/payment-processing/package-pricing.tssrc/shared/booking/fold-tree.tssrc/shared/booking/signed-metadata.tssrc/shared/catalog-fields/definition.tssrc/shared/db/attendees/update.tssrc/shared/images/resize.tssrc/shared/listings-actions.tssrc/shared/merge/attendee-merge.tssrc/shared/site-assignment.tssrc/ui/client/admin/manual-checkin.tssrc/ui/templates/public/reservations/form.tsxsrc/ui/templates/public/reservations/ticket-page.tsxtest/features/admin/attendee-page-data/activity-and-questions.test.tstest/features/admin/attendee-page-data/contact-records.test.tstest/features/admin/attendee-page-data/refunds-ui.test.tstest/features/admin/attendee-page-data/template-data.test.tstest/features/admin/attendee-page-data/warnings.test.tstest/features/admin/catalog-transfer/import/fields.test.tstest/features/admin/catalog-transfer/import/packages.test.tstest/features/admin/catalog-transfer/import/persistence.test.tstest/features/admin/catalog-transfer/import/policy.test.tstest/features/admin/catalog-transfer/import/reachability.test.tstest/features/admin/catalog-transfer/import/references.test.tstest/features/admin/entity-pages.test.tstest/features/api/payment-processing/index/balance.test.tstest/features/api/payment-processing/index/booking.test.tstest/features/api/payment-processing/index/conflicts.test.tstest/features/api/payment-processing/index/helpers.tstest/features/api/payment-processing/index/modifiers.test.tstest/features/api/payment-processing/index/refunds.test.tstest/features/api/payment-processing/items.test.tstest/features/api/payment-processing/items/boundaries.test.tstest/features/api/payment-processing/items/helpers.tstest/features/api/payment-processing/package-pricing.test.tstest/features/api/payment-processing/package-pricing/database.test.tstest/shared/booking/fold-tree.test.tstest/shared/booking/signed-metadata.test.tstest/shared/catalog-fields/definition.test.tstest/shared/db/attendees/update.test.tstest/shared/images/resize.test.tstest/shared/listings-actions.test.tstest/shared/listings-actions/helpers.tstest/shared/listings-actions/lifecycle.test.tstest/shared/listings-actions/reachability.test.tstest/shared/listings-actions/validation.test.tstest/shared/merge/attendee-merge/apply-bookings.test.tstest/shared/merge/attendee-merge/apply-money.test.tstest/shared/merge/attendee-merge/apply.test.tstest/shared/merge/attendee-merge/cleanup.test.tstest/shared/merge/attendee-merge/diff.test.tstest/shared/merge/attendee-merge/labels.test.tstest/shared/merge/attendee-merge/repoint.test.tstest/shared/merge/attendee-merge/validate.test.tstest/shared/site-assignment/contracts.test.tstest/ui/client/admin/manual-checkin/combobox.test.tstest/ui/client/admin/manual-checkin/fixture.tstest/ui/client/admin/manual-checkin/submission.test.tstest/ui/templates/public/reservations/form.test.tstest/ui/templates/public/reservations/ticket-page.test.ts
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 `@scripts/mutation/equivalent-mutants.txt`:
- Around line 1084-1085: Remove or relocate the orphaned schema-index note in
the equivalent-mutants manifest, unless the missing schema-index mutant records
can be added immediately beneath it. Keep the note adjacent to the mutant
entries it explains and ensure it does not remain before the payment-processing
section without associated records.
🪄 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: 8b141ef8-d4e4-4820-a19e-0d41635ac234
📒 Files selected for processing (1)
scripts/mutation/equivalent-mutants.txt
|
Addressed the remaining review-body feedback in 27a43cc: the initial answer-conflict setup now reuses |
|
@codex review @coderabbitai review |
|
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
What changed
mainwith the preceding change.Why
Smaller functions make these important flows easier to read and change. The stricter check prevents complexity from growing back.
Verification
deno task precommitSummary by CodeRabbit
New Features
Bug Fixes
Tests