feat: transaction review polish#26
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds mutual-exclusion enforcement for ChangesSubcontractor Payout Mutual Exclusion, sourceExternalId Explorer Fallback, and Card Position Badges
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR polishes quarter transaction review by tightening subcontractor payout linking rules, improving classification UX to prevent invalid selections, and enhancing manual ledger entry display (including explorer links derived from sourceExternalId) plus review-list navigation context.
Changes:
- Added server-side validation to prevent
subcontractor_payoutclassifications from linking both a raid and a RIP simultaneously. - Updated classification linked-field UI to track raid/RIP selections in component state and disable conflicting selectors.
- Enhanced the transactions review page to (a) derive explorer links for manual entries from
sourceExternalIdand (b) show item position (n / total) on cards.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/lib/transaction-classification.ts | Extends manual ledger entry classification view to include sourceExternalId. |
| src/app/admin/quarters/[id]/transactions/page.tsx | Adds more chain explorer URLs, derives manual-entry explorer links from sourceExternalId, and displays review position counters. |
| src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsx | Adds stateful raid/RIP selection and UI logic to prevent invalid raid/RIP combinations. |
| src/app/admin/quarters/[id]/transactions/actions.ts | Adds server-side validation to reject subcontractor_payout entries that link both a raid and a RIP. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
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 `@src/app/admin/quarters/`[id]/transactions/actions.ts:
- Around line 1249-1252: The validation check for subcontractor_payout in both
classifyQuarterTransfer and updateLedgerEntryClassification functions only
rejects when both raidId and ripId are provided, but fails to enforce that
exactly one must be selected - currently allowing neither to be selected. Update
the conditional logic to also reject the case where category is
"subcontractor_payout" and both raidId and ripId are empty or undefined,
ensuring exactly one linkage is required rather than just preventing both from
being selected simultaneously.
In `@src/app/admin/quarters/`[id]/transactions/classification-linked-fields.tsx:
- Around line 104-107: The disable logic for raidDisabled and ripDisabled at
lines 104-107 unconditionally disables each control when its counterpart ID is
selected, which locks both controls when both IDs are already pre-populated,
causing the hidden values at lines 229 and 269 to submit empty values and
silently clear both links. Modify the disable conditions so that a control is
only disabled when its counterpart ID is selected AND that control's own ID is
not already selected, allowing both controls to remain accessible when both IDs
are pre-populated while still preventing selection of both in normal usage.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6006ca7-68a6-4ccc-9cc0-3bd248df75c9
📒 Files selected for processing (4)
src/app/admin/quarters/[id]/transactions/actions.tssrc/app/admin/quarters/[id]/transactions/classification-linked-fields.tsxsrc/app/admin/quarters/[id]/transactions/page.tsxsrc/lib/transaction-classification.ts
This pull request improves the classification and UI logic for quarter transactions, especially around linking raids and RIPs, and enhances manual ledger entry handling. The main changes enforce stricter validation for subcontractor payouts, update the UI to prevent invalid raid/RIP combinations, and improve the display and linking of transaction data.
Validation and Business Logic Improvements:
subcontractor_payouttransactions, users must choose either a raid or a RIP, but not both, in bothclassifyQuarterTransferandupdateLedgerEntryClassificationfunctions. (src/app/admin/quarters/[id]/transactions/actions.tsR1249-R1252, src/app/admin/quarters/[id]/transactions/actions.tsR1401-R1404)UI/UX Enhancements for Classification:
ClassificationLinkedFieldsto:subcontractor_payout, preventing both from being selected at once.Manual Ledger Entry and Transaction Explorer Improvements:
sourceExternalIdfield, including support for multiple chains (Arbitrum, Optimism, etc.). (src/app/admin/quarters/[id]/transactions/page.tsxR113-R140, src/app/admin/quarters/[id]/transactions/page.tsxR1251-R1270, src/app/admin/quarters/[id]/transactions/page.tsxL1316-R1370, [1] [2]Summary by CodeRabbit
New Features
Bug Fixes
Improvements