Skip to content

refactor(lane-3): transformer per-kind strategies — 952→553 lines#1915

Closed
Hugo0 wants to merge 2 commits intoqa/post-cutover-fe-fixesfrom
chore/transformer-strategies
Closed

refactor(lane-3): transformer per-kind strategies — 952→553 lines#1915
Hugo0 wants to merge 2 commits intoqa/post-cutover-fe-fixesfrom
chore/transformer-strategies

Conversation

@Hugo0
Copy link
Copy Markdown
Contributor

@Hugo0 Hugo0 commented Apr 29, 2026

Summary

Split the monolithic 51-arm switch in `transactionTransformer.ts` into
21 per-kind strategy files under `strategies/`. Composite-key registry
maps `{legacy entry type, TRANSACTION_INTENT:kind}` → strategy. Public
API (`mapTransactionDataForDrawer`) unchanged; the receipt drawer +
`useReceiptViewModel` keep working without modification.

Why

The old 3D dispatch (`entry.type × intent.kind × userRole`) was nested
3-4 levels deep. Today's playtest revealed 5 missing/wrong branches that
all looked plausible at review. Adding a new kind required editing the
right level + getting all three dimensions right. Per-strategy files
make each kind grep-able in one place — adding a new kind is now "add a
file + register it" instead of "find the right nested arm".

Scope (21 strategies)

Legacy: direct-send, send-link, request, bank-send-link-claim,
withdraw, cashout, bank-offramp (×2 keys), bank-onramp (×2 keys),
deposit, manteca-qr, simplefi-qr, perk-reward
Intent: p2p-send (covers REQUEST_PAY), qr-pay, link-create,
crypto-deposit, crypto-withdraw, fiat-offramp, card-spend, card-refund
Fallback: intentFallback (unknown intent kinds → Sentry + safe default),
legacyFallback (unknown legacy types → safe default)

Post-strategy globals (status mapping, reaper-failed override, derived
fields: explorer URL, token logos, initials) stay in
`mapTransactionDataForDrawer` — those run uniformly regardless of kind.

Verified

  • 33/33 transformer fixtures pass (the contract lock)
  • 961/961 full FE test suite passes
  • ✅ `pnpm typecheck` clean
  • ✅ `mapTransactionDataForDrawer` signature unchanged

Notes for review

  • Registry uses string-literal keys (not `EHistoryEntryType.X`) — some
    tests mock `useTransactionHistory`, which breaks the enum re-export at
    module-init time. Inlined string values match the enum.
  • The intent fallback inlines the legacy default-arm Sentry call shape.
    Lane 4 FE PR refactor(lane-4): pipelineAlert() helper for FE Sentry signals #1914 consolidates that onto `pipelineAlert`; two-line
    conflict resolution at merge.

Base

Stacked on `qa/post-cutover-fe-fixes` (PR #1912). Independent of Lanes 1, 2a, 4.

Split the monolithic 51-arm switch in transactionTransformer.ts into
21 per-kind strategies under src/components/TransactionDetails/strategies/.
Composite-key registry maps {legacy entry type, TRANSACTION_INTENT:kind}
→ strategy. Public API (mapTransactionDataForDrawer) unchanged.

## Why
The old 3D dispatch (entry.type × intent.kind × userRole) was nested 3-4
levels deep; today's playtest revealed 5 missing/wrong branches that all
looked plausible at review. Adding a new kind required editing the right
level + getting all three dimensions right. Per-strategy files make each
kind grep-able in one place, so a new kind is "add a file + register it"
not "find the right nested arm".

## Scope
21 strategies, no behavioral change:
- legacy: direct-send, send-link, request, bank-send-link-claim,
  withdraw, cashout, bank-offramp×2, bank-onramp×2, deposit,
  manteca-qr, simplefi-qr, perk-reward
- intent: p2p-send (covers REQUEST_PAY), qr-pay, link-create,
  crypto-deposit, crypto-withdraw, fiat-offramp, card-spend, card-refund
- fallback: intentFallback (unknown intent kinds → Sentry log + safe
  default), legacyFallback (unknown legacy types → safe default)

Post-strategy globals (status mapping, reaper-failed override, derived
fields like explorer URL / token logos / initials) stay in
mapTransactionDataForDrawer — those run uniformly regardless of kind.

## Verified
- 33/33 transformer fixtures pass (locks the contract)
- 961/961 FE tests pass
- pnpm typecheck clean
- mapTransactionDataForDrawer signature unchanged → useReceiptViewModel
  + drawer rendering unaffected

## Notes
- Registry uses string-literal keys (not EHistoryEntryType.X) so tests
  that mock useTransactionHistory still load the registry module.
- The intent fallback inlines the legacy default-arm Sentry call shape;
  Lane 4 FE PR (#1914) consolidates it onto pipelineAlert. Two-line
  conflict resolution at merge.
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 29, 2026

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

Project Deployment Actions Updated (UTC)
peanut-wallet Ready Ready Preview, Comment Apr 29, 2026 5:50pm

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Walkthrough

Adds a strategy pattern for transaction-detail rendering: new strategy types, many intent and legacy strategy implementations, a registry dispatcher with fallbacks, and refactors the main transformer to use dispatched strategies instead of an inline switch.

Changes

Cohort / File(s) Summary
Types & Registry
src/components/TransactionDetails/strategies/types.ts, src/components/TransactionDetails/strategies/registry.ts
Introduce TransactionStrategyOutput and TransactionStrategy types; add dispatchStrategy(entry) registry that maps legacy entry.type and TRANSACTION_INTENT:<kind> keys to strategy functions and routes to fallbacks when missing.
Fallbacks
src/components/TransactionDetails/strategies/fallback.ts
Add intentFallback and legacyFallback. intentFallback conditionally delegates to cardRefund for refund-like intents or lazy-logs unhandled intents to Sentry (browser-only) and returns defensive defaults. legacyFallback returns legacy-compatible default output.
Intent strategies
src/components/TransactionDetails/strategies/intent/...
card.ts, crypto.ts, fiat-offramp.ts, link-create.ts, p2p-send.ts
Add multiple intent-based strategy implementations: card (qrPay, cardSpend, cardRefund), crypto (cryptoDeposit, cryptoWithdraw), fiat off-ramp (fiatOfframp), link creation (linkCreate), and P2P/request handling (p2pSendOrRequestPay). Each produces normalized TransactionStrategyOutput based on role, accounts, and extraData.
Legacy strategies
src/components/TransactionDetails/strategies/legacy/...
bank-send-link-claim.ts, direct-send.ts, external-account.ts, request.ts, send-link.ts
Add legacy strategy implementations covering bank-send link claims, direct sends, external-account flows (withdraw/cashout/bank on/offramp/QR/deposit/perkReward), requests, and send-link flows. Each derives display fields, role/link flags, and optional uiStatus.
Transformer refactor
src/components/TransactionDetails/transactionTransformer.ts
Replace inline switch routing with dispatchStrategy(entry)(entry) to obtain classification fields (direction, card type, name, fullName, showFullName, isPeerActuallyUser, isLinkTx, optional uiStatus); preserve existing post-mapping adjustments (status→pill, failed-row tweaks, etc.).
Tests & minor formatting
src/components/TransactionDetails/__tests__/transactionTransformer.test.ts, src/context/PeanutDebug.tsx, src/utils/account-mask.utils.ts
Reformat test imports and object literals; small formatting tweaks in debug constant and account-mask function signature (no semantic changes).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main refactoring work: converting a monolithic switch statement into per-kind strategy files, with quantified improvements (952→553 lines).
Description check ✅ Passed The description comprehensively covers the changeset, explaining the motivation for the refactor, the 21 strategies added, scope, verification status, and implementation notes.
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

Review rate limit: 3/5 reviews remaining, refill in 12 minutes and 15 seconds.

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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/components/TransactionDetails/transactionTransformer.ts (1)

269-375: ⚠️ Potential issue | 🟡 Minor

uiStatus from strategies is currently ignored.

Line [269] applies out.uiStatus, but Lines [291-375] always recompute uiStatus, so strategy overrides never survive to the returned payload.

Suggested fix
-    if (out.uiStatus) uiStatus = out.uiStatus
+    const strategyUiStatus = out.uiStatus
@@
     if (
         entry.type === EHistoryEntryType.BRIDGE_OFFRAMP ||
@@
         }
     }
+
+    if (strategyUiStatus) uiStatus = strategyUiStatus
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/TransactionDetails/transactionTransformer.ts` around lines 269
- 375, The code sets uiStatus from out.uiStatus but then always recomputes it,
ignoring strategy overrides; update transactionTransformer.ts so that after
assigning uiStatus = out.uiStatus you short-circuit the recompute logic (or wrap
the two big status-switch blocks in a guard) and only run the
EHistoryEntryType/EHistoryEntryType.BRIDGE_* and default entry.status switches
when uiStatus is falsy/undefined; reference the uiStatus local, the out.uiStatus
source, and the status-switch logic that checks entry.type and entry.status so
strategy-provided uiStatus survives to the returned payload.
🧹 Nitpick comments (1)
src/components/TransactionDetails/strategies/legacy/request.ts (1)

39-42: Simplify nameForDetails fallback to avoid awkward text.

Line 39–Line 42 reuses the same fields in all fallback branches, which can degrade to unclear strings on bad/missing account data. Prefer a stable terminal fallback.

Proposed cleanup
-            nameForDetails:
-                entry.recipientAccount?.username ||
-                entry.recipientAccount?.identifier ||
-                `Request From ${entry.recipientAccount?.username || entry.recipientAccount?.identifier}`,
+            nameForDetails:
+                entry.recipientAccount?.username ||
+                entry.recipientAccount?.identifier ||
+                'Request',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/TransactionDetails/strategies/legacy/request.ts` around lines
39 - 42, The current nameForDetails expression repeats the same fields and can
produce awkward strings; replace it with a stable fallback like
entry.recipientAccount?.username || entry.recipientAccount?.identifier ||
'Unknown Sender' (i.e., remove the template `Request From ${...}` fallback and
use a single final constant). Update the nameForDetails assignment in request.ts
so it references entry.recipientAccount?.username, then
entry.recipientAccount?.identifier, then the stable terminal fallback 'Unknown
Sender'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/TransactionDetails/strategies/fallback.ts`:
- Around line 17-23: The fallback predicate currently routes card refunds based
only on kind and parentRainTxId; update the conditional that returns
cardRefund(entry) to also require the provider === 'RAIN' check (i.e., ensure
the if tests (kind === 'OTHER' || kind === 'REFUND') &&
entry.extraData?.parentRainTxId && provider === 'RAIN') so the logic matches the
comment and avoids misrouting.

In `@src/components/TransactionDetails/strategies/intent/p2p-send.ts`:
- Around line 15-23: In the REQUEST_PAY && entry.extraData?.fulfillmentType ===
'bridge' && entry.userRole === 'SENDER' branch inside p2p-send.ts, the
isPeerActuallyUser flag is using entry.senderAccount?.isUser which is the wrong
side and can yield false positives; change the computation to rely on the
recipient side (entry.recipientAccount?.isUser) so isPeerActuallyUser becomes
!!entry.recipientAccount?.isUser || !!entry.senderAccount?.isUser (or simply
!!entry.recipientAccount?.isUser if only recipient matters) in the object
returned by this branch.

In `@src/components/TransactionDetails/strategies/legacy/send-link.ts`:
- Around line 15-16: The mapping in the send-link strategy is incorrectly
assigning recipientAccount.username to the UI fullName field; update the mapping
inside the send-link strategy (e.g., where you construct the recipient object in
send-link.ts) to use entry.recipientAccount?.fullName for fullName (and likewise
replace the username usage at the other occurrence around lines 60-61) while
leaving showFullName as entry.recipientAccount?.showFullName so the actual full
name is displayed.

---

Outside diff comments:
In `@src/components/TransactionDetails/transactionTransformer.ts`:
- Around line 269-375: The code sets uiStatus from out.uiStatus but then always
recomputes it, ignoring strategy overrides; update transactionTransformer.ts so
that after assigning uiStatus = out.uiStatus you short-circuit the recompute
logic (or wrap the two big status-switch blocks in a guard) and only run the
EHistoryEntryType/EHistoryEntryType.BRIDGE_* and default entry.status switches
when uiStatus is falsy/undefined; reference the uiStatus local, the out.uiStatus
source, and the status-switch logic that checks entry.type and entry.status so
strategy-provided uiStatus survives to the returned payload.

---

Nitpick comments:
In `@src/components/TransactionDetails/strategies/legacy/request.ts`:
- Around line 39-42: The current nameForDetails expression repeats the same
fields and can produce awkward strings; replace it with a stable fallback like
entry.recipientAccount?.username || entry.recipientAccount?.identifier ||
'Unknown Sender' (i.e., remove the template `Request From ${...}` fallback and
use a single final constant). Update the nameForDetails assignment in request.ts
so it references entry.recipientAccount?.username, then
entry.recipientAccount?.identifier, then the stable terminal fallback 'Unknown
Sender'.
🪄 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: CHILL

Plan: Pro

Run ID: e3471e4a-b086-43c3-9902-3907e3ce11ca

📥 Commits

Reviewing files that changed from the base of the PR and between b4c974c and 6499575.

📒 Files selected for processing (14)
  • src/components/TransactionDetails/strategies/fallback.ts
  • src/components/TransactionDetails/strategies/intent/card.ts
  • src/components/TransactionDetails/strategies/intent/crypto.ts
  • src/components/TransactionDetails/strategies/intent/fiat-offramp.ts
  • src/components/TransactionDetails/strategies/intent/link-create.ts
  • src/components/TransactionDetails/strategies/intent/p2p-send.ts
  • src/components/TransactionDetails/strategies/legacy/bank-send-link-claim.ts
  • src/components/TransactionDetails/strategies/legacy/direct-send.ts
  • src/components/TransactionDetails/strategies/legacy/external-account.ts
  • src/components/TransactionDetails/strategies/legacy/request.ts
  • src/components/TransactionDetails/strategies/legacy/send-link.ts
  • src/components/TransactionDetails/strategies/registry.ts
  • src/components/TransactionDetails/strategies/types.ts
  • src/components/TransactionDetails/transactionTransformer.ts

Comment on lines +17 to +23
// Card refunds come back with kind === 'REFUND' or kind === 'OTHER'
// alongside provider === RAIN. Scope strictly to these two kinds —
// guarding only on parentRainTxId would misroute any future intent
// that happens to carry the linkage.
if ((kind === 'OTHER' || kind === 'REFUND') && entry.extraData?.parentRainTxId) {
return cardRefund(entry)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Card-refund fallback predicate is missing the provider === 'RAIN' guard.

Line [21] currently routes by kind + parent linkage only, despite the comment/scoping intent. Add the provider check to avoid future misrouting.

Suggested fix
-    if ((kind === 'OTHER' || kind === 'REFUND') && entry.extraData?.parentRainTxId) {
+    if (
+        (kind === 'OTHER' || kind === 'REFUND') &&
+        entry.extraData?.provider === 'RAIN' &&
+        entry.extraData?.parentRainTxId
+    ) {
         return cardRefund(entry)
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Card refunds come back with kind === 'REFUND' or kind === 'OTHER'
// alongside provider === RAIN. Scope strictly to these two kinds —
// guarding only on parentRainTxId would misroute any future intent
// that happens to carry the linkage.
if ((kind === 'OTHER' || kind === 'REFUND') && entry.extraData?.parentRainTxId) {
return cardRefund(entry)
}
// Card refunds come back with kind === 'REFUND' or kind === 'OTHER'
// alongside provider === RAIN. Scope strictly to these two kinds —
// guarding only on parentRainTxId would misroute any future intent
// that happens to carry the linkage.
if (
(kind === 'OTHER' || kind === 'REFUND') &&
entry.extraData?.provider === 'RAIN' &&
entry.extraData?.parentRainTxId
) {
return cardRefund(entry)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/TransactionDetails/strategies/fallback.ts` around lines 17 -
23, The fallback predicate currently routes card refunds based only on kind and
parentRainTxId; update the conditional that returns cardRefund(entry) to also
require the provider === 'RAIN' check (i.e., ensure the if tests (kind ===
'OTHER' || kind === 'REFUND') && entry.extraData?.parentRainTxId && provider ===
'RAIN') so the logic matches the comment and avoids misrouting.

Comment on lines +15 to +23
if (kind === 'REQUEST_PAY' && entry.extraData?.fulfillmentType === 'bridge' && entry.userRole === 'SENDER') {
return {
direction: 'bank_request_fulfillment',
transactionCardType: 'bank_request_fulfillment',
nameForDetails: entry.recipientAccount?.username ?? entry.recipientAccount?.identifier ?? 'Recipient',
fullName: entry.recipientAccount?.fullName ?? '',
showFullName: entry.recipientAccount?.showFullName,
isPeerActuallyUser: !!entry.recipientAccount?.isUser || !!entry.senderAccount?.isUser,
isLinkTx: false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

isPeerActuallyUser is computed from the wrong side in REQUEST_PAY bridge sender flow.

Line [22] includes senderAccount?.isUser, which can produce false positives for peer-user badges. This should be based on the recipient in this branch.

Suggested fix
-            isPeerActuallyUser: !!entry.recipientAccount?.isUser || !!entry.senderAccount?.isUser,
+            isPeerActuallyUser: !!entry.recipientAccount?.isUser,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (kind === 'REQUEST_PAY' && entry.extraData?.fulfillmentType === 'bridge' && entry.userRole === 'SENDER') {
return {
direction: 'bank_request_fulfillment',
transactionCardType: 'bank_request_fulfillment',
nameForDetails: entry.recipientAccount?.username ?? entry.recipientAccount?.identifier ?? 'Recipient',
fullName: entry.recipientAccount?.fullName ?? '',
showFullName: entry.recipientAccount?.showFullName,
isPeerActuallyUser: !!entry.recipientAccount?.isUser || !!entry.senderAccount?.isUser,
isLinkTx: false,
if (kind === 'REQUEST_PAY' && entry.extraData?.fulfillmentType === 'bridge' && entry.userRole === 'SENDER') {
return {
direction: 'bank_request_fulfillment',
transactionCardType: 'bank_request_fulfillment',
nameForDetails: entry.recipientAccount?.username ?? entry.recipientAccount?.identifier ?? 'Recipient',
fullName: entry.recipientAccount?.fullName ?? '',
showFullName: entry.recipientAccount?.showFullName,
isPeerActuallyUser: !!entry.recipientAccount?.isUser,
isLinkTx: false,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/TransactionDetails/strategies/intent/p2p-send.ts` around lines
15 - 23, In the REQUEST_PAY && entry.extraData?.fulfillmentType === 'bridge' &&
entry.userRole === 'SENDER' branch inside p2p-send.ts, the isPeerActuallyUser
flag is using entry.senderAccount?.isUser which is the wrong side and can yield
false positives; change the computation to rely on the recipient side
(entry.recipientAccount?.isUser) so isPeerActuallyUser becomes
!!entry.recipientAccount?.isUser || !!entry.senderAccount?.isUser (or simply
!!entry.recipientAccount?.isUser if only recipient matters) in the object
returned by this branch.

Comment on lines +15 to +16
fullName: entry.recipientAccount?.username ?? '',
showFullName: entry.recipientAccount?.showFullName,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

fullName is mapped from username instead of fullName.

Line 15 and Line 60 should pull recipientAccount.fullName; using username can suppress the intended full-name display.

Proposed fix
-            fullName: entry.recipientAccount?.username ?? '',
+            fullName: entry.recipientAccount?.fullName ?? '',
@@
-        fullName: entry.recipientAccount?.username ?? '',
+        fullName: entry.recipientAccount?.fullName ?? '',

Also applies to: 60-61

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/TransactionDetails/strategies/legacy/send-link.ts` around
lines 15 - 16, The mapping in the send-link strategy is incorrectly assigning
recipientAccount.username to the UI fullName field; update the mapping inside
the send-link strategy (e.g., where you construct the recipient object in
send-link.ts) to use entry.recipientAccount?.fullName for fullName (and likewise
replace the username usage at the other occurrence around lines 60-61) while
leaving showFullName as entry.recipientAccount?.showFullName so the actual full
name is displayed.

…format

- Replace stringly-typed userRole compares ('SENDER'/'RECIPIENT') with
  EHistoryUserRole enum (intent strategies were the stragglers).
- Hoist BANK_CLAIM constant in bank-send-link-claim.ts — guest and
  claimant branches were byte-identical.
- Drop dead default initializers / narrative comments in
  transactionTransformer.ts and fallback.ts.
- Apply prettier to 3 inherited unformatted files (PeanutDebug.tsx,
  account-mask.utils.ts, transactionTransformer.test.ts) so the lint
  gate passes on this stack.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (2)
src/components/TransactionDetails/strategies/intent/p2p-send.ts (1)

13-25: ⚠️ Potential issue | 🟠 Major

Use recipient-only peer flag in the bridge REQUEST_PAY sender branch.

Line 23 still mixes in entry.senderAccount?.isUser; in this branch the sender is the viewer, so this can produce false-positive peer badges.

Suggested fix
-            isPeerActuallyUser: !!entry.recipientAccount?.isUser || !!entry.senderAccount?.isUser,
+            isPeerActuallyUser: !!entry.recipientAccount?.isUser,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/TransactionDetails/strategies/intent/p2p-send.ts` around lines
13 - 25, In the REQUEST_PAY branch of the p2p-send intent (the branch where kind
=== 'REQUEST_PAY' and entry.extraData?.fulfillmentType === 'bridge' and
entry.userRole === EHistoryUserRole.SENDER), change the isPeerActuallyUser
computation to use only the recipient account flag
(entry.recipientAccount?.isUser) instead of OR-ing with
entry.senderAccount?.isUser; update the isPeerActuallyUser property to reflect
recipient-only peer detection so the viewer/sender doesn't cause false-positive
peer badges.
src/components/TransactionDetails/strategies/fallback.ts (1)

11-13: ⚠️ Potential issue | 🟠 Major

Add the missing provider === 'RAIN' guard in refund fallback routing.

At Line [11], the predicate still routes by kind + parentRainTxId only. Because unmatched intents are funneled into this fallback (src/components/TransactionDetails/strategies/registry.ts:76-81), this can misroute future non-Rain intents that also carry parentRainTxId.

Suggested fix
-    if ((kind === 'OTHER' || kind === 'REFUND') && entry.extraData?.parentRainTxId) {
+    if (
+        (kind === 'OTHER' || kind === 'REFUND') &&
+        entry.extraData?.provider === 'RAIN' &&
+        entry.extraData?.parentRainTxId
+    ) {
         return cardRefund(entry)
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/TransactionDetails/strategies/fallback.ts` around lines 11 -
13, The fallback routing condition in fallback.ts currently checks only kind and
entry.extraData.parentRainTxId and can misroute non-Rain intents; update the
predicate so it also requires entry.provider === 'RAIN' before calling
cardRefund(entry). Locate the if in the fallback strategy that returns
cardRefund(entry) (referencing symbols: entry, kind, cardRefund) and change the
condition to include the provider === 'RAIN' guard (e.g., && entry.provider ===
'RAIN') so only Rain-provider refunds use this route.
🧹 Nitpick comments (1)
src/components/TransactionDetails/strategies/intent/link-create.ts (1)

22-22: Harden nameForDetails against empty-string account fields.

Line 22 can still resolve to an empty label ('') when account fields are empty strings. A non-empty fallback avoids blank UI in drawer headers.

Suggested patch
-            nameForDetails: entry.recipientAccount?.username ?? entry.recipientAccount?.identifier ?? '',
+            nameForDetails:
+                entry.recipientAccount?.username ||
+                entry.recipientAccount?.identifier ||
+                'Sent to user',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/TransactionDetails/strategies/intent/link-create.ts` at line
22, The nameForDetails assignment can still become an empty string if username
or identifier are empty; change it to pick the first non-empty trimmed value
from entry.recipientAccount?.username and entry.recipientAccount?.identifier and
fall back to a non-empty label (e.g. 'Unknown account' or similar). Locate the
nameForDetails usage in link-create.ts (the line using
entry.recipientAccount?.username ?? entry.recipientAccount?.identifier ?? '')
and replace the expression with logic that trims and checks length (or a small
helper like firstNonEmpty) so blank strings are ignored before applying the
final fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/components/TransactionDetails/strategies/fallback.ts`:
- Around line 11-13: The fallback routing condition in fallback.ts currently
checks only kind and entry.extraData.parentRainTxId and can misroute non-Rain
intents; update the predicate so it also requires entry.provider === 'RAIN'
before calling cardRefund(entry). Locate the if in the fallback strategy that
returns cardRefund(entry) (referencing symbols: entry, kind, cardRefund) and
change the condition to include the provider === 'RAIN' guard (e.g., &&
entry.provider === 'RAIN') so only Rain-provider refunds use this route.

In `@src/components/TransactionDetails/strategies/intent/p2p-send.ts`:
- Around line 13-25: In the REQUEST_PAY branch of the p2p-send intent (the
branch where kind === 'REQUEST_PAY' and entry.extraData?.fulfillmentType ===
'bridge' and entry.userRole === EHistoryUserRole.SENDER), change the
isPeerActuallyUser computation to use only the recipient account flag
(entry.recipientAccount?.isUser) instead of OR-ing with
entry.senderAccount?.isUser; update the isPeerActuallyUser property to reflect
recipient-only peer detection so the viewer/sender doesn't cause false-positive
peer badges.

---

Nitpick comments:
In `@src/components/TransactionDetails/strategies/intent/link-create.ts`:
- Line 22: The nameForDetails assignment can still become an empty string if
username or identifier are empty; change it to pick the first non-empty trimmed
value from entry.recipientAccount?.username and
entry.recipientAccount?.identifier and fall back to a non-empty label (e.g.
'Unknown account' or similar). Locate the nameForDetails usage in link-create.ts
(the line using entry.recipientAccount?.username ??
entry.recipientAccount?.identifier ?? '') and replace the expression with logic
that trims and checks length (or a small helper like firstNonEmpty) so blank
strings are ignored before applying the final fallback.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0e1033a3-92dd-498e-9815-744cb53ac347

📥 Commits

Reviewing files that changed from the base of the PR and between 6499575 and 081fa90.

📒 Files selected for processing (10)
  • src/components/TransactionDetails/__tests__/transactionTransformer.test.ts
  • src/components/TransactionDetails/strategies/fallback.ts
  • src/components/TransactionDetails/strategies/intent/crypto.ts
  • src/components/TransactionDetails/strategies/intent/fiat-offramp.ts
  • src/components/TransactionDetails/strategies/intent/link-create.ts
  • src/components/TransactionDetails/strategies/intent/p2p-send.ts
  • src/components/TransactionDetails/strategies/legacy/bank-send-link-claim.ts
  • src/components/TransactionDetails/transactionTransformer.ts
  • src/context/PeanutDebug.tsx
  • src/utils/account-mask.utils.ts
✅ Files skipped from review due to trivial changes (4)
  • src/context/PeanutDebug.tsx
  • src/utils/account-mask.utils.ts
  • src/components/TransactionDetails/strategies/intent/crypto.ts
  • src/components/TransactionDetails/tests/transactionTransformer.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/components/TransactionDetails/strategies/legacy/bank-send-link-claim.ts
  • src/components/TransactionDetails/transactionTransformer.ts
  • src/components/TransactionDetails/strategies/intent/fiat-offramp.ts

@Hugo0
Copy link
Copy Markdown
Contributor Author

Hugo0 commented Apr 29, 2026

Superseded by consolidated PR #1916. CR comments triaged: send-link fullName (addressed in #1916); fallback provider===RAIN guard + p2p-send isPeer flag tracked in mono Tier 4 #7 (legacy-port issues, defensive).

@Hugo0 Hugo0 closed this Apr 29, 2026
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.

1 participant