fix(dashpay): repair the invitation-claim aftermath and rebuild the banner as a menu row - #999
Merged
Merged
Conversation
…anner as a menu row Claiming an invitation and registering a non-contested username left the app in three wrong states, all with the same tell: a relaunch fixed them. 1. The user landed back on "Claim your invitation" — the screen they had just completed and cannot repeat. `CreateUsernameViewController` popped one level, and the invitation entry pushes the form ON TOP of the redeem screen. It now unwinds to the tab's root. 2. The DashPay tabs stayed at three instead of five, and the More screen kept offering "Join DashPay". Both follow from the same break: `CreateUsernameViewModel`'s invitation branch calls the coordinator directly and never touches `DWIdentityRegistrationBridge.shared`. That singleton is the only poster of the canonical registration notification, and it wires itself up in its own lazy `init` — so on an invitation-first launch (the normal case: invitations exist to onboard new users) it is never constructed and the notification is never posted at all. Nothing told the tab bar, the banner or `DWCurrentUserIdentityInfo` that registration had happened. The branch now announces it explicitly, the same way `reconcileRecoveredIdentity()` does for identities that arrive outside the bridge's flow. `MainTabbarController.applyPendingDashPayTabReconfiguration` additionally gated on a flag that `reconfigureDashPayTabsIfNeeded` only raises after it has already seen an identity — so when the notification landed before `DWCurrentUserIdentityInfo` could see the new row, the flag stayed false and the tabs were never rebuilt. It no longer depends on that flag; the rebuild is idempotent. The Join DashPay banner is now a standard menu row on both Home and More. The Hide / Upgrade buttons are gone: the row itself is the action, and a trailing close control appears only on Home, where the banner is an interruption. On More it is a standing menu entry, so it is no longer hidden by the dismissal flag (one tap on Home's close used to remove it there permanently, with no way to bring it back) and no longer gated on sync — it stays put while the chain catches up, presenting itself as unavailable instead of vanishing. State-to-copy mapping moved into `JoinDashPayCopy` so the two surfaces cannot drift. The row's tap needed its own handler rather than the old action button's: `.callToAction` reached `editProfile()`, which guards on an existing identity and returns silently for exactly the users that state describes — a dead row.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDashPay now exposes synchronization state, uses a shared state-driven menu row in Home and the main menu, and updates invitation registration completion to refresh identity, publish status, unwind navigation, and reconfigure tabs. ChangesDashPay flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SyncModelImpl
participant CurrentUserProfileModel
participant JoinDashPayMenuItem
participant HomeView
participant MainMenuViewController
SyncModelImpl->>CurrentUserProfileModel: emit synchronization state
CurrentUserProfileModel->>JoinDashPayMenuItem: provide syncing and availability state
JoinDashPayMenuItem->>HomeView: render shared Home row
JoinDashPayMenuItem->>MainMenuViewController: route row tap
MainMenuViewController->>MainMenuViewController: open join, profile, or request-status flow
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@DashWallet/en.lproj/Localizable.strings`:
- Around line 455-457: Propagate the “Available after sync finishes” key from
the English Localizable.strings catalog through Transifex by running tx push -s
followed by tx pull -a. Ensure the resulting main-app locale catalogs remain
UTF-8 encoded without a BOM.
In `@DashWallet/Sources/UI/DashPay/Setup/CreateUsername/JoinDashPayView.swift`:
- Line 182: Resolve the missing menu-send-account-disabled asset used by the
syncing row in JoinDashPayView by updating and pinning DashUIKit to a released
version that includes it, or remove the disabledLeadingIcon reference if the
asset cannot be made available.
- Line 161: Remove the redundant “= nil” initializer from the optional onDismiss
property in JoinDashPayView, leaving it as an implicitly initialized optional
declaration.
- Around line 262-273: Update the JoinDashPayState declaration to conform to
Hashable so the state array used by ForEach with id: \.self provides valid
hashable identifiers.
🪄 Autofix
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 Plus
Run ID: d53a54a3-d712-4325-9b3e-fb606dc40768
📒 Files selected for processing (9)
DashWallet/Sources/Models/Usernames/CurrentUserProfileModel.swiftDashWallet/Sources/UI/DashPay/Setup/CreateUsername/CreateUsernameViewController.swiftDashWallet/Sources/UI/DashPay/Setup/CreateUsername/CreateUsernameViewModel.swiftDashWallet/Sources/UI/DashPay/Setup/CreateUsername/JoinDashPayView.swiftDashWallet/Sources/UI/Home/Views/HomeView.swiftDashWallet/Sources/UI/Main/MainTabbarController.swiftDashWallet/Sources/UI/Menu/Main/MainMenuViewController.swiftDashWallet/Sources/UI/Menu/Main/MainMenuViewModel.swiftDashWallet/en.lproj/Localizable.strings
Comment on lines
+455
to
+457
| /* DashPay */ | ||
| "Available after sync finishes" = "Available after sync finishes"; | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
key='Available after sync finishes'
fd -t f 'Localizable.strings' DashWallet | while IFS= read -r file; do
if ! rg -qF "\"$key\"" "$file"; then
echo "Missing key: $file"
fi
done
fd -HI '^\.tx$|^transifex' . -t fRepository: dashpay/dashwallet-ios
Length of output: 4877
🏁 Script executed:
#!/bin/bash
set -euo pipefail
key='Available after sync finishes'
printf '%s\n' '--- localization files containing the key ---'
find DashWallet -type f -path '*/Localizable.strings' -print0 |
while IFS= read -r -d '' file; do
if grep -qF "\"$key\"" "$file"; then
printf 'FOUND %s\n' "$file"
else
printf 'MISSING %s\n' "$file"
fi
done
printf '%s\n' '--- source key call sites ---'
rg -n -F "$key" --glob '!DashWallet/**/Localizable.strings' --glob '!DashWallet/**/Localizable.stringsdict' . || true
printf '%s\n' '--- Transifex-related files and references ---'
find . -type f \( -name '.tx' -o -iname '*transifex*' \) -print
rg -n -i 'transifex|localizable\.strings' .github .tx transifex* 2>/dev/null || true
printf '%s\n' '--- encoding of main-app localization files ---'
python3 - <<'PY'
from pathlib import Path
for path in sorted(Path("DashWallet").glob("*.lproj/Localizable.strings")):
data = path.read_bytes()
bom = data.startswith(b"\xef\xbb\xbf") or data.startswith(b"\xff\xfe") or data.startswith(b"\xfe\xff")
print(f"{path}: {'BOM' if bom else 'no BOM'}")
PYRepository: dashpay/dashwallet-ios
Length of output: 5009
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- locale catalog inventory ---'
find DashWallet -type f -path '*/Localizable.strings' -printf '%p\n' | sort
printf '%s\n' '--- exact key declarations ---'
rg -n -F '"Available after sync finishes" =' DashWallet --glob '*/Localizable.strings' || true
printf '%s\n' '--- recent repository state ---'
git status --short
git diff --statRepository: dashpay/dashwallet-ios
Length of output: 1996
🏁 Script executed:
#!/bin/bash
set -euo pipefail
key='Available after sync finishes'
find DashWallet -type f -path '*/Localizable.strings' -print0 |
while IFS= read -r -d '' file; do
if grep -qF "\"$key\"" "$file"; then
echo "FOUND $file"
else
echo "MISSING $file"
fi
done
echo '--- call sites outside catalogs ---'
rg -n -F "$key" --glob '!**/Localizable.strings' --glob '!**/Localizable.stringsdict' . || true
echo '--- Transifex files and references ---'
find . -type f \( -name '.tx' -o -iname '*transifex*' \) -print
rg -n -i 'transifex' . --glob '!.git/**' || true
echo '--- encoding ---'
python3 - <<'PY'
from pathlib import Path
for path in sorted(Path("DashWallet").glob("*.lproj/Localizable.strings")):
data = path.read_bytes()
print(f"{path}: {'UTF-8 BOM' if data.startswith(b'\\xef\\xbb\\xbf') else 'no UTF-8 BOM'}")
PYRepository: dashpay/dashwallet-ios
Length of output: 4032
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- all matching catalog entries, excluding stringsdict ---'
find DashWallet -type f -name 'Localizable.strings' -print0 |
xargs -0 grep -nF '"Available after sync finishes" =' || true
echo '--- all source references ---'
rg -n -F 'Available after sync finishes' . \
--glob '!DashWallet/**/Localizable.strings' \
--glob '!DashWallet/**/Localizable.stringsdict' || true
echo '--- localization and translation configuration ---'
git ls-files | grep -Ei '(^|/)(\.tx|transifex|localiz|translation)' | head -200Repository: dashpay/dashwallet-ios
Length of output: 4271
Propagate the key through Transifex.
JoinDashPayView.swift uses this key, but only the English catalog defines it. Run tx push -s and tx pull -a so the key reaches the locale catalogs. Keep main-app catalogs in UTF-8 without a BOM.
🤖 Prompt for 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.
In `@DashWallet/en.lproj/Localizable.strings` around lines 455 - 457, Propagate
the “Available after sync finishes” key from the English Localizable.strings
catalog through Transifex by running tx push -s followed by tx pull -a. Ensure
the resulting main-app locale catalogs remain UTF-8 encoded without a BOM.
Source: Learnings
| DashUIKit.MenuItem( | ||
| leadingIcon: .custom(copy.iconName, bundle: .main), | ||
| isEnabled: !isSyncing, | ||
| disabledLeadingIcon: .custom("menu-send-account-disabled", bundle: .dashUIKit), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Resolve the disabled icon dependency before merge.
menu-send-account-disabled is not available in the current DashUIKit dependency. The syncing row uses this asset, so its disabled icon can fail to render. Release and pin the required DashUIKit version, or remove this asset reference.
The PR objective identifies this asset as requiring a release and dependency update before merge.
🤖 Prompt for 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.
In `@DashWallet/Sources/UI/DashPay/Setup/CreateUsername/JoinDashPayView.swift` at
line 182, Resolve the missing menu-send-account-disabled asset used by the
syncing row in JoinDashPayView by updating and pinning DashUIKit to a released
version that includes it, or remove the disabledLeadingIcon reference if the
asset cannot be made available.
Comment on lines
+262
to
+273
| ForEach( | ||
| [ | ||
| JoinDashPayState.callToAction, | ||
| .voting, | ||
| .approved, | ||
| .failed, | ||
| .blocked, | ||
| .contested, | ||
| .registered | ||
| ], | ||
| id: \.self | ||
| ) { state in |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline DashWallet/Sources/UI/DashPay/Setup/CreateUsername/JoinDashPayView.swift \
--items all --match JoinDashPayState
rg -nUP --glob '*.swift' \
'enum\s+JoinDashPayState\s*:\s*[^{]*\bHashable\b|extension\s+JoinDashPayState\s*:\s*[^{]*\bHashable\b' \
DashWalletRepository: dashpay/dashwallet-ios
Length of output: 261
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,40p;250,285p' DashWallet/Sources/UI/DashPay/Setup/CreateUsername/JoinDashPayView.swift
rg -n --glob '*.swift' 'JoinDashPayState' DashWallet
rg -n --glob '*.swift' 'ForEach\(' DashWallet/Sources/UI/DashPay/Setup/CreateUsername/JoinDashPayView.swiftRepository: dashpay/dashwallet-ios
Length of output: 4293
Declare JoinDashPayState: Hashable. ForEach(..., id: \.self) requires a Hashable element ID, and no conformance exists.
🤖 Prompt for 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.
In `@DashWallet/Sources/UI/DashPay/Setup/CreateUsername/JoinDashPayView.swift`
around lines 262 - 273, Update the JoinDashPayState declaration to conform to
Hashable so the state array used by ForEach with id: \.self provides valid
hashable identifiers.
…t-disabled `JoinDashPayMenuItem` passes `menu-send-account-disabled` as the row's `disabledLeadingIcon`. The asset landed in DashUIKit#12; without this bump a fresh checkout resolves to a master revision that predates it and the disabled icon renders as nothing. The pin is a branch (`master`), so this only fixes the recorded revision — Xcode also rewrote the file's key spacing, which accounts for the rest of the diff.
The Home banner was gated on `syncDone`, so it did not exist for the whole of a long sync. That hid the only surface in the app carrying the "Have an invitation?" entry from exactly the user who needs it — someone who was just invited and is opening the wallet for the first time, which is also when the chain is furthest behind. It now behaves like the More entry: the row stays put and renders as unavailable (greyed leading icon and text, "Available after sync finishes", inert tap) while the chain catches up. `HomeViewModel` publishes `isSyncing` from the `syncModel.$state` subscription it already had. The close control stays live throughout — dismissing an interruption should not depend on the chain, even while the action behind it is unavailable.
SwiftLint's `implicit_optional_initialization` — an optional var is already nil. No behaviour change: the memberwise initializer still defaults the parameter, so `JoinDashPayMenuItem` can be built without an `onDismiss`.
Contributor
Author
|
Going through the CodeRabbit findings — one fixed, two do not hold, one is outside this PR. Fixed — Does not hold — Already resolved — Left for the team's flow — For context on lint: |
QuantumExplorer
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Claiming a DashPay invitation and registering a non-contested username left the app in three wrong states, each of which a relaunch silently fixed:
(2) and (3) share one root cause.
CreateUsernameViewModel's invitation branch callsDWIdentityRegistrationCoordinatordirectly and never touchesDWIdentityRegistrationBridge.shared. That singleton is the only poster ofDWDashPayRegistrationStatusUpdatedNotification, and it subscribes to the coordinator inside its own lazyinit—.sharedand.stateChangedNotificationare independently lazy statics, so referencing the notification name does not construct the bridge. On an invitation-first launch — the normal case, since invitations exist to onboard new users — the bridge is never built and the canonical notification is never posted at all. Nothing informs the tab bar, the banner, orDWCurrentUserIdentityInfo. A relaunch works becauseCurrentUserProfileModel.init()recomputes from disk, which the coordinator had already written correctly.What was done?
Landing (1) —
CreateUsernameViewControllerpopped a single level; the invitation entry pushes the form on top of the redeem screen. It now unwinds to the tab's root, which is the correct destination for all three push sites.Notification (2, 3) — the invitation branch announces registration explicitly after a successful claim, mirroring what
DWCurrentUserIdentityInfo.reconcileRecoveredIdentity()already does for identities that arrive outside the bridge's flow.Tabs (2) —
applyPendingDashPayTabReconfigurationgated on a flag thatreconfigureDashPayTabsIfNeededonly raises after it has seen an identity. When the notification landed beforeDWCurrentUserIdentityInfocould observe the new row, the flag stayed false and the tabs were never rebuilt. The gate is gone; the rebuild is idempotent.Banner redesign — Join DashPay is now a standard menu row (
JoinDashPayMenuItem) on both Home and More, replacing the card with Hide/Upgrade buttons:JoinDashPayCopy, shared by both surfaces so they cannot drift.The row also needed its own tap handler rather than reusing the old action button's:
.callToActionreachededitProfile(), which guards on an existing identity and returns silently for exactly the users that state describes — a row that looked dead.Screenshots
Home — row with the close control
More — row without the close control
Syncing — greyed icon, "Available after sync finishes", inert tap
(pending)
Dependencies
Requires DashUIKit#12 (merged) for the
menu-send-account-disabledasset used as the row'sdisabledLeadingIcon.Package.resolvedis bumped to that revision in this PR.How Has This Been Tested?
Clean
dashpaybuild (xcodebuild -workspace DashWallet.xcworkspace -scheme dashpay -sdk iphonesimulator ARCHS=arm64) against DashUIKit master. SwiftUI previews added for each presentation:Menu row — More (no close),Menu row — Home (with close),Menu row — syncing.Not yet smoke-tested end to end on testnet: the invitation-claim run that produced these findings predates the fixes. The three symptoms should be re-checked by hand — claim an invitation with a non-contested name, then confirm the landing screen, five tabs, and the banner retiring without a relaunch.
Breaking Changes
None.
Checklist: