Skip to content

fix: transaction json handling and parsing#442

Merged
im-adithya merged 3 commits into
masterfrom
fix/tx-json-parsing
Jun 18, 2026
Merged

fix: transaction json handling and parsing#442
im-adithya merged 3 commits into
masterfrom
fix/tx-json-parsing

Conversation

@im-adithya

@im-adithya im-adithya commented Jun 17, 2026

Copy link
Copy Markdown
Member

Fixes #433

Summary by CodeRabbit

  • Bug Fixes
    • Improved payment notification deep-link generation with safer query construction and stronger validation
    • Enhanced deeplink handling for connection and payment flows to avoid incorrect decoding and to fail fast when required data is missing
  • Refactor
    • Reorganized transaction rendering with centralized parsing/validation, safer JSON handling, and consistent UI behavior when data is invalid
  • Tests
    • Added Jest coverage for deeplink URL parsing/navigation, including payment notification payload correctness and connect-screen routing

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0f21c4fc-1980-47fc-a451-56264476e688

📥 Commits

Reviewing files that changed from the base of the PR and between 041a122 and 8b59e9f.

📒 Files selected for processing (1)
  • pages/Transaction.tsx

📝 Walkthrough

Walkthrough

Fixes an iOS crash when viewing payment notification details caused by double URL-encoding. NotificationService.m now builds the deep-link using NSURLComponents/queryItems instead of manual percent-encoding. handleLink in lib/link.ts removes decodeURIComponent calls for transaction, callback, and appicon params. Transaction.tsx is refactored with typed route params, safe JSON parsing, and extracted sub-components and helper functions.

Changes

Deep-link encoding fix and Transaction screen refactor

Layer / File(s) Summary
iOS NSURLComponents URL construction
assets/ios/NotificationService.m
Replaces manual percent-encoding string interpolation with NSURLComponents/queryItems assembly; adds nil guards for transactionJSON and the resulting deepLink.
Remove double-decode in handleLink + tests
lib/link.ts, hooks/__tests__/useHandleLinking.ts
callback, appicon, and transaction query params are now read directly from URLSearchParams without decodeURIComponent. Tests verify nostrnwc://connect preserves param values and alby://payment_notification produces transactionJSON without double-decoding.
Transaction screen typed params, error handling, and component split
pages/Transaction.tsx
Adds TransactionRouteParams; Transaction() parses JSON with useMemo+try/catch, redirects to / with errorToast on failure, and sets wallet from appPubkey. Rendering is split into TransactionScreen, TransactionSummary, and TransactionDetails. Helpers getTransactionIcon, getTransactionStatus, getEncodedEventId, and getBoostagram replace prior inline logic with safe error handling.

Sequence Diagram(s)

sequenceDiagram
  participant iOS as iOS NotificationService
  participant handleLink as handleLink
  participant Transaction as Transaction()
  participant Router as expo-router

  iOS->>iOS: NSURLComponents.queryItems = [{transaction, app_pubkey}]
  iOS->>iOS: deepLink = components.URL (properly encoded, no double-encode)
  iOS->>handleLink: alby://payment_notification?transaction=...&app_pubkey=...
  handleLink->>handleLink: URLSearchParams.get("transaction") — raw, no decodeURIComponent
  handleLink->>Router: push("/transaction", { transactionJSON, appPubkey })
  Router->>Transaction: mount with route params
  Transaction->>Transaction: useMemo → JSON.parse(transactionJSON) with try/catch
  alt parse failure
    Transaction->>Router: replace("/") + errorToast
  else parse success
    Transaction->>Transaction: render TransactionSummary + TransactionDetails
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 A URL once tangled and double-decoded,
Sent the payment screen crashing — the path was eroded!
Now NSURLComponents builds the link with great care,
No decodeURIComponent double-dipping in there.
The transaction screen parses with try/catch in place,
And the rabbit hops safely to payment detail space! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change—improving transaction JSON handling and parsing—which aligns with the refactoring and validation improvements across iOS and TypeScript files.
Linked Issues check ✅ Passed The PR addresses the iOS crash in issue #433 by adding proper JSON validation with try/catch in Transaction.tsx, implementing safe URL parameter parsing in link.ts, and using NSURLComponents with proper encoding in NotificationService.m.
Out of Scope Changes check ✅ Passed All changes directly support transaction JSON handling: iOS deep-link construction, URL parameter parsing, transaction validation, screen refactoring with helper functions, and comprehensive test coverage. No unrelated changes detected.

✏️ 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 fix/tx-json-parsing

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.

@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: 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 `@pages/Transaction.tsx`:
- Around line 83-87: The useEffect hook that calls
useAppStore.getState().setSelectedWallet(appPubkey) currently updates the wallet
whenever appPubkey changes, regardless of whether the transaction payload is
valid. Add a condition to check that the transaction has been successfully
parsed and is valid before executing the setSelectedWallet call. This ensures
that a malformed deep-link cannot change the wallet context before the redirect
happens. Include a check for transaction validation status in the conditional
logic alongside the appPubkey check.
🪄 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: 69c286b4-d591-4a6c-96ff-99a31e13f2de

📥 Commits

Reviewing files that changed from the base of the PR and between cb26512 and 041a122.

📒 Files selected for processing (4)
  • assets/ios/NotificationService.m
  • hooks/__tests__/useHandleLinking.ts
  • lib/link.ts
  • pages/Transaction.tsx

Comment thread pages/Transaction.tsx Outdated
@im-adithya im-adithya merged commit 03c8fff into master Jun 18, 2026
2 of 3 checks passed
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.

bug: iOS crash when attempting to view payment details

1 participant