Skip to content

fix: return home after transfer to savings - #1280

Open
jvsena42 wants to merge 12 commits into
masterfrom
fix/809-transfer-savings-home
Open

jvsena42 wants to merge 12 commits into
masterfrom
fix/809-transfer-savings-home

Conversation

@jvsena42

@jvsena42 jvsena42 commented Sep 16, 2026

Copy link
Copy Markdown
Member

Fixes #809
Related to #1275

This PR sends the user to Home after a transfer from Spending to Savings, instead of back to the Spending wallet screen.

Description

  • Fixes OK on the transfer to savings success screen returning to the Spending wallet, because the flow popped only the transfer routes and the flow is entered from Spending; it now navigates to Home, matching iOS and the other transfer exits.
  • Applies the same Home navigation when the transfer becomes unavailable or the swap fails on the transfer to savings progress screen.
  • Ignores the exit unless the savings progress screen that owns the callback is the one on screen (currentDestination?.hasRoute<Routes.SavingsProgress>(), public nav API), so the coop-close retry job cannot navigate to Home up to 30 minutes later from an unrelated screen, or from a transfer to spending the user started since. On the progress screen itself nothing changes.
  • Adds unit tests that walk Spending → transfer to savings → progress and check the exit route lands on Home with Spending removed from the back stack, and that a late exit call leaves an unrelated screen, a screen stacked on the flow, or a later transfer to spending alone.
  • Adds a changelog fragment.

Companion e2e PR: synonymdev/bitkit-e2e-tests#225 removes the Android-only back/close taps after TransferSuccess-button, which fail once this lands. Merge both together.

Out of Scope

  • Design: the Figma prototype wires OK on "Transfer to savings successful" to the Savings detail screen. This PR follows iOS and the issue (Home) instead; the prototype link should be updated or confirmed by design.
  • journeys/transfer: the journey is committed at journeys/transfer/transfer-to-savings-returns-home.xml; each run closes the channel, so reopen one afterwards before any journey that needs a Spending balance.
  • iOS: no change needed, SavingsProgressView already resets navigation to Home.

Design

Preview

Full flow recorded on the emulator (Pixel_9, dev build): Spending -> Transfer To Savings -> Funds availability -> Please confirm -> swipe -> Transfer Successful -> OK -> Home -> system back to the launcher.

809-transfer-to-savings-home.mp4
Home before Spending (flow entry) Funds availability Please confirm
Home before transfer Spending wallet screen Funds availability screen Please confirm screen
Success screen After OK (Home) System back from Home Figma success
Transfer Successful screen Home after OK Launcher after system back Figma transfer to savings successful

QA Notes

Manual Tests

  • 1. Open channel with Spending balance > 0 → Home → Spending → Transfer To Savings → Savings Intro (if shown) → Savings Availability → Continue → Savings Confirm → swipe to transfer → Savings Progress: "Transfer Successful" shows.
    • 1b. Tap OK: lands on Home (Spending 0, pending "Transfer From Spending" row), no back button or Spending title.
    • 1c. System back from Home: app goes to the launcher, not to Spending or the transfer flow.
  • 2. regression: Savings Availability → Cancel: still lands on Home.

A journey covering steps 1-1c was written and run by hand on the emulator, and is committed at journeys/transfer/transfer-to-savings-returns-home.xml (suite and cross-platform rows added to journeys/README.md).

journeys/transfer/transfer-to-savings-returns-home.xml
diff --git a/journeys/README.md b/journeys/README.md
index 403c5e230..fdff33ec5 100644
--- a/journeys/README.md
+++ b/journeys/README.md
@@ -121,6 +121,7 @@ fixtures, push notifications) live in each suite's README.
 | [notification-permission](notification-permission) | 4 | Background-setup toggles |
 | [payment-requests](payment-requests) | 2 | Requires a linked fixture issuer; rejected shapes are unit fixtures |
 | [pubky-marketplace](pubky-marketplace) | 1 | Two-wallet Paykit marketplace payment; integration fixture required |
+| [transfer](transfer) | 1 | Spending to Savings exit route; needs an open channel and closes it; no README |
 | [widgets](widgets) | 2 | Needs no backend — the quickest way to see the loop work; no README |
 
 ## Cross-platform
@@ -142,6 +143,7 @@ Known differences in the corpus, as of the iOS port (synonymdev/bitkit-ios#691):
 | `hardware-wallet/receive-onchain.xml`, `hardware-wallet/send-onchain.xml` | not ported |
 | `payment-requests/requested-resolution-failure.xml` | not ported |
 | `deeplinks/*` | not ported — iOS registers the `bitkit` scheme but has no screen or sheet router |
+| `transfer/transfer-to-savings-returns-home.xml` | not ported — iOS already resets navigation to home on the same OK, so the journey has no iOS counterpart yet |
 | — | `hardware-wallet/transfer-to-spending-over-max.xml` exists only on iOS |
 
 ### Running one on iOS
diff --git a/journeys/transfer/transfer-to-savings-returns-home.xml b/journeys/transfer/transfer-to-savings-returns-home.xml
new file mode 100644
index 000000000..8c567f7b9
--- /dev/null
+++ b/journeys/transfer/transfer-to-savings-returns-home.xml
@@ -0,0 +1,28 @@
+<journey name="transfer to savings returns home">
+  <description>
+    Verifies that confirming a Spending to Savings transfer and tapping OK on the success screen
+    lands on the wallet home screen, not on the Spending wallet screen the flow was started from,
+    and that system back from there does not reopen the Spending screen or the transfer flow
+    (synonymdev/bitkit-android#809). iOS resets navigation to home on the same OK.
+
+    Precondition: onboarded dev wallet with an open, usable channel to the LSP and a POSITIVE
+    Spending balance. The transfer closes that channel, so reopen one (Blocktank order through the
+    lsp helper, then mine blocks) afterwards if later work needs a Spending balance. Start on the
+    wallet home screen.
+  </description>
+  <actions>
+    <action>Verify the Spending balance card (testTag "ActivitySpending") shows a positive amount</action>
+    <action>Tap the Spending balance card (testTag "ActivitySpending")</action>
+    <action>Tap "Transfer To Savings" (testTag "TransferToSavings")</action>
+    <action>If the transfer to savings intro appears, tap "Get Started" (testTag "SavingsIntro-button")</action>
+    <action>Tap "Continue" (testTag "AvailabilityContinue") on the funds availability screen</action>
+    <action>On the confirm screen, swipe the "Swipe To Transfer" handle (testTag "GRAB") fully to the right</action>
+    <action>Wait up to 60 seconds for the "Transfer Successful" screen (testTag "TransferSuccess") with the OK button (testTag "TransferSuccess-button")</action>
+    <action>Tap OK (testTag "TransferSuccess-button")</action>
+    <action>Verify the home screen is visible with the Savings and Spending cards (testTags "ActivitySavings" and "ActivitySpending")</action>
+    <action>Verify there is no back button (testTag "NavigationBack") and no "Transfer To Savings" button (testTag "TransferToSavings")</action>
+    <action>Verify the Spending balance card (testTag "ActivitySpending") shows 0</action>
+    <action>Press the system back button once</action>
+    <action>Verify the app is backgrounded to the launcher, and neither the Spending screen nor any transfer screen is shown</action>
+  </actions>
+</journey>

Automated Checks

  • Unit tests added: ContentViewTest.kt builds a Home → Spending → TransferRoot nav graph with Robolectric and asserts the savings transfer exit lands on Home with nothing left above it; it fails if the exit is reverted to popping the transfer routes. Three more cases assert a late exit call leaves the current screen alone: after navigating away from the flow, with a screen pushed on top of it, and with a transfer to spending started afterwards inside the same TransferRoot graph.
  • E2E: fix: expect home after android transfer to savings bitkit-e2e-tests#225 updates transfer.e2e.ts and actions.ts for the new Android behaviour.
  • Local: just compile, just test, just lint — all green (detekt reports baseline findings only, none in the changed files).

jvsena42 and others added 3 commits September 16, 2026 15:39
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

This PR should not merge until the delayed unavailable callback can no longer redirect users after they leave the transfer flow.

Findings

  1. P1 Delayed callback redirects users
  2. P2 Test bypasses changed callbacks

Summary

This PR changes savings-transfer success and unavailable exits to return to Home and adds navigation-stack regression coverage.

  • Replaces transfer-graph popping with the shared Home-navigation helper.
  • Verifies the standard Home → Spending → savings-transfer stack removes Spending and progress destinations.
  • Adds a changelog fragment for the corrected exit behavior.
  • The unavailable path still needs a lifecycle guard because its callback can outlive the progress screen.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    H[Home] --> S[Spending]
    S --> T[Savings transfer]
    T --> P[Savings progress]
    P -->|Success / unavailable| H
    P -. activity-scoped retry survives screen exit .-> R[Unrelated destination]
    R -->|Stale unavailable callback| H
Loading

Reviews (1) · Last reviewed commit: "chore: reword savings transfer changelog..."

Comment thread app/src/main/java/to/bitkit/ui/ContentView.kt
Comment thread app/src/test/java/to/bitkit/ui/ContentViewTest.kt
@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Regtest APK

Built from f8bfd79 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Approved — no HIGH/MEDIUM findings at head b0207069f. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Traced every exit of SavingsProgressScreen at the PR head: OK on SUCCESS, SETTLING and INTERRUPTED, the trusted-only onUnavailable branch of runChannelClose, SavingsSwapResult.Failure, and the give-up onTransferUnavailable callback that startCoopCloseRetries keeps in viewModelScope for GIVE_UP_MS. All go through navigateOnSavingsTransferExit (ContentView.kt:945-946, 2086-2090), whose TransferRoot guard reads the nested-graph entry that NavController keeps on currentBackStack, so on-screen exits reach navigateToHome and the late callback is a no-op once the flow is gone; the toast it also fires is unchanged from master. Home is always beneath the transfer flow: the only in-app entry is Home -> Spending -> TransferRoot (HomeScreen.kt:392 uses the root controller), the dev-only screen deep links go through handleDeepLink with CLEAR_TASK which rebuilds the stack from the root start destination, and the two clearStack Navigate effects target RecoveryMode and CriticalUpdate only, so popBackStack(inclusive = false) in navigateToHome never hits the push-on-top fallback. Back and cancel paths are untouched: SavingsAvailability Cancel already used navigateToHome, system back on the progress screen still pops to SavingsConfirm as on master, and drawer navigation off the Interrupted screen leaves TransferRoot on the stack so a late callback there navigates Home where master popped the same entries to Spending. The Robolectric test builds a Home -> Spending -> TransferRoot stack with a real NavHostController and would fail on master's popBackStack (which lands on Spending). Note only: currentBackStack is @get:RestrictTo(LIBRARY_GROUP) in navigation 2.9.6; it compiles and CI runs detekt not Android lint, so no functional impact.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

✅ Manually checked

ovi-reviewer[bot]

This comment was marked as resolved.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ovi-reviewer[bot]

This comment was marked as resolved.

jvsena42 and others added 3 commits September 18, 2026 10:47
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Approved — no blocking HIGH/MEDIUM findings at head 75a7bdc99. (Posted as a comment because GitHub does not allow approving your own PR.)

Inline: LOW — Late give-up callback still exits a sibling transfer flow started afterwards; LOW — 'no Spending screen title' assertion matches text that Home also shows

Checked: Round 4 (d745972 refactor + 096bf76 journey): Round 4 delta is d745972 (guard rewritten on public API) and 096bf76 (journey). The guard at ContentView.kt:2099-2102 now walks currentDestination.hierarchy for the TransferRoot graph built by navigationWithDefaultTransitions -> navigation<Routes.TransferRoot> (Transitions.kt:76-95), so hasRoute on the parent NavGraph entry is what makes it true; ContentViewTest's completion case proves that on a Home -> Spending -> TransferRoot/SavingsProgress stack and CI's testDevDebugUnitTest passed at head. Traced every caller: OK on SUCCESS/SETTLING/INTERRUPTED (SavingsProgressScreen.kt:121 -> ContentView.kt:957), the trusted-only immediate onUnavailable (SavingsProgressScreen.kt:150), SavingsSwapResult.Failure (SavingsProgressScreen.kt:106-112) and the give-up onTransferUnavailable that startCoopCloseRetries keeps in viewModelScope (TransferViewModel.kt:1699-1727, activity-scoped via MainActivity.kt:92) all fire on SavingsProgress when prompt, where the hierarchy contains TransferRoot and navigateToHome pops to Home exactly as at the reviewed sha. Late give-up after the user left: from Home, Settings, Savings, Spending or any screen pushed over the flow via the DrawerNavIcon, the current destination's hierarchy has no TransferRoot, so the exit is a no-op and the user is not moved; this fully answers greptile 4030447639 and is stricter than the old currentBackStack check for the pushed-on-top case, which the author already documented as an accepted trade in thread 4046976234. Process recreation destroys the TransferViewModel and its retry job, so no stale callback survives it, and the restored SavingsProgress entry re-wires a fresh lambda. Journey: every testTag named (ActivitySpending, ActivitySavings, TransferToSavings, SavingsIntro-button, AvailabilityContinue, GRAB, TransferSuccess, TransferSuccess-button, NavigationBack) exists at head, the intro step matches ContentView.kt:1229-1233, swap mode is off by default (SettingsStore.kt:175) so the CLOSE path and 'closes that channel' precondition hold, and DeriveBalanceStateUseCase.kt:57-59 subtracts the coop-close transfer so the Spending card reads 0 after OK; README suite and cross-platform rows are present and no bitkit-ios twin PR exists.

Comment thread app/src/main/java/to/bitkit/ui/ContentView.kt Outdated
Comment thread journeys/transfer/transfer-to-savings-returns-home.xml Outdated
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Approved — no blocking HIGH/MEDIUM findings at head 0a2320e22. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Follow-up on 0a2320e: the earlier give-up-callback LOW is fixed. Commit 0a2320e fixes the earlier LOW: navigateOnSavingsTransferExit (ContentView.kt:2098-2102) now navigates home only when currentDestination is Routes.SavingsProgress, so a coop-close give-up/unavailable callback firing 30 min later inside a transfer-to-spending flow (same TransferRoot graph) is a no-op; the new test 'savings transfer exit leaves a later transfer to spending alone' pins this and the three earlier tests still hold. Every prompt exit still reaches Home: OK on SUCCESS/SETTLING/INTERRUPTED (Content onContinueClick, SavingsProgressScreen.kt:121), trusted-only unavailable from runChannelClose (:149-150), swap Failure (:106-112) and post-retry trusted-only give-up (TransferViewModel.kt:1727) all run while the user is on SavingsProgress; the screen has no back button and no BackHandler. Nothing legitimately on top of it changes currentDestination: sheets (Sheet.ForceTransfer, toasts) render through SheetHost (ContentView.kt:504), the PIN lock is a MainActivity overlay (MainActivity.kt:176), and Routes.AuthCheck is only pushed from Settings. hasRoute on the leaf destination works for the nested-graph data object route as in existing code. Journey change is correct: testTag TransferToSavings exists on SpendingWalletScreen.kt:191, so asserting its absence on Home is a valid check that Spending is not the visible screen.

ovi-reviewer[bot]

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

@jvsena42 conflicts & ci e2e red

@jvsena42

Copy link
Copy Markdown
Member Author

Both are in hand.

Conflicts: same one-line collision in journeys/README.md that hits every PR in this batch whenever one of them merges. #1279 and #1284 landed in the last half hour, so eleven PRs re-conflicted at once. A merge of current master is running across all of them (plus the two stack bases, so #1288 and #1310 come along), resolved by keeping every row and validated with compile and unit tests before each push. No rebases, no force-pushes.

e2e: I will report on the red shard here once the merge lands and CI re-runs on the new head, rather than reading a result from the pre-merge commit.

ovi-reviewer[bot]

This comment was marked as resolved.

ovi-reviewer[bot]

This comment was marked as resolved.

@ovi-reviewer ovi-reviewer 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.

Verdict: ✅ Approve


Reaudit: diff 1 file.

Findings:
N/A

Audit:
Audited - no findings.

QA: emu-1 redroid container on Android 15, dev build 2.4.1 (188)

  1. passed: Transfer Successful appeared after moving the funded Spending balance to Savings.

    Test 1:
    Open channel with Spending balance > 0 → Home → Spending → Transfer To Savings → Savings Intro (if shown) →…
    1.mp4
  2. passed: Cancel returned Home; separately, OK returned Home with Spending 0 and no transfer or back controls.

    Test 2:
    `regression:` Savings Availability → Cancel: still lands on Home.
    2.mp4
  3. passed: System back from Home opened the launcher without exposing Spending or transfer screens.

    Test 3:
    **1c.** System back from Home: app goes to the launcher, not to Spending or the transfer flow.
    3.mp4

Tip

Test 1 worth a journey:

  • Verify the Spending balance is positive
  • Open Spending
  • Tap Transfer To Savings
  • Tap Continue on Funds Availability
  • Swipe fully to transfer
  • Verify Transfer Successful appears

Test 2 worth a journey:

  • Open Spending and tap Transfer To Savings
  • Tap Cancel on Funds Availability
  • Verify Home appears with Savings and Spending cards
  • Complete a separate transfer to the Transfer Successful screen
  • Tap OK
  • Verify Home appears with Spending 0 and no transfer or back control

Test 3 worth a journey:

  • Start on Home after tapping OK
  • Verify Savings and Spending cards are visible
  • Press system back once
  • Verify the launcher appears and no Bitkit transfer screen remains

Coverage:
QA: 3 of 3 manual tests passed


Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)

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.

android: transfer to savings completes on spending details screen

2 participants