Skip to content

feat: transaction review polish#26

Merged
ECWireless merged 2 commits into
mainfrom
codex/transaction-review-polish
Jun 20, 2026
Merged

feat: transaction review polish#26
ECWireless merged 2 commits into
mainfrom
codex/transaction-review-polish

Conversation

@ECWireless

@ECWireless ECWireless commented Jun 20, 2026

Copy link
Copy Markdown
Member

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:

UI/UX Enhancements for Classification:

Manual Ledger Entry and Transaction Explorer Improvements:

Summary by CodeRabbit

  • New Features

    • Added support for Arbitrum and Optimism blockchain explorers
    • Transactions and manual entries now show their position in the review list (e.g., “3/10”)
  • Bug Fixes

    • Added validation to ensure subcontractor payouts use exactly one linkage: either raid or RIP (not both, not neither)
  • Improvements

    • Improved raid/RIP linkage UI behavior with clearer prompts and state handling when categories change
    • Enhanced manual entry explorer links by parsing manual on-chain source IDs as needed

Copilot AI review requested due to automatic review settings June 20, 2026 02:26
@vercel

vercel Bot commented Jun 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
raidguild-accounting Ready Ready Preview, Comment Jun 20, 2026 3:17pm

Request Review

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0a801d72-5d69-4416-9281-0b753168cbcf

📥 Commits

Reviewing files that changed from the base of the PR and between dc82f6d and 3bd1282.

📒 Files selected for processing (2)
  • src/app/admin/quarters/[id]/transactions/actions.ts
  • src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/app/admin/quarters/[id]/transactions/actions.ts
  • src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsx

📝 Walkthrough

Walkthrough

Adds mutual-exclusion enforcement for subcontractor_payout classifications that prevents linking both a raid and a RIP simultaneously, at both the server-action level (throwing on invalid input) and the UI level (controlled state, cross-disabling selects). Exposes sourceExternalId on ManualLedgerEntryClassificationView to enable explorer URL fallback resolution. Adds Arbitrum/Optimism chain support to the explorer URL helper, a manual-onchain: prefix parser, and position/total badges on TransferCard and ManualLedgerEntryCard.

Changes

Subcontractor Payout Mutual Exclusion, sourceExternalId Explorer Fallback, and Card Position Badges

Layer / File(s) Summary
sourceExternalId added to ManualLedgerEntryClassificationView
src/lib/transaction-classification.ts
ManualLedgerEntryClassificationView gains a sourceExternalId: string | null field; listManualLedgerEntryClassifications maps it from the ledger entry.
Raid/RIP mutual exclusion validation for subcontractor_payout (server)
src/app/admin/quarters/[id]/transactions/actions.ts
classifyQuarterTransfer and updateLedgerEntryClassification throw when both raidId and ripId are provided for subcontractor_payout, enforcing exactly-one-or-none invariant.
Raid/RIP controlled state and cross-disabling (UI)
src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsx
ClassificationLinkedFields converts Raid/RIP selects to controlled state, cross-disables each when the other is selected, clears stale selections on category change, and updates placeholder text accordingly.
Position badges and explorer URL fallback on cards
src/app/admin/quarters/[id]/transactions/page.tsx
getTransactionExplorerUrl adds Arbitrum and Optimism chains. A new getManualSourceExternalTransaction helper parses manual-onchain: sourceExternalId strings. TransferCard and ManualLedgerEntryCard accept position/total props and render a header badge. ManualLedgerEntryCard resolves explorer values from entry fields with fallback to parsed sourceExternalId. Review-item render loops pass index-derived position/total into both cards.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • raid-guild/accounting#25: Directly modifies classification-linked-fields.tsx and actions.ts around subcontractor_payout handling and RIP/raid linking, overlapping with this PR's mutual-exclusion logic.
  • raid-guild/accounting#19: Extends classifyQuarterTransfer with category-specific validation branches in the same file and location as this PR's subcontractor_payout guard.
  • raid-guild/accounting#17: Adds the source_external_id DB column for manual ledger entries that this PR now surfaces through ManualLedgerEntryClassificationView and uses for explorer URL resolution.

Poem

🐇 Hop hop, a raid and a RIP
Cannot both be chosen — just pick!
The selects cross-disable with care,
Position badges float through the air,
And explorer links now know every chain — slick! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'feat: transaction review polish' is vague and generic, using the non-descriptive term 'polish' without clarifying the primary changes. Use a more specific title that highlights the main change, such as 'feat: add raid/RIP mutual exclusion validation and ledger entry position tracking' or focus on the most impactful feature.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/transaction-review-polish

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_payout classifications 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 sourceExternalId and (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.

Comment thread src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsx Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between e764889 and dc82f6d.

📒 Files selected for processing (4)
  • src/app/admin/quarters/[id]/transactions/actions.ts
  • src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsx
  • src/app/admin/quarters/[id]/transactions/page.tsx
  • src/lib/transaction-classification.ts

Comment thread src/app/admin/quarters/[id]/transactions/actions.ts Outdated
Comment thread src/app/admin/quarters/[id]/transactions/classification-linked-fields.tsx Outdated
@ECWireless ECWireless merged commit b959310 into main Jun 20, 2026
3 checks passed
@ECWireless ECWireless deleted the codex/transaction-review-polish branch June 20, 2026 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants