fix(dashpay): persist Upgrade-to-DashPay banner dismissal (Hide now works) - #900
Conversation
Why: - Hide was ignored before identity creation because banner gating only honored dismissal for wallets that already had an identity - the legacy global preference could not safely survive network switches What: - scope dismissal storage to the active wallet and network - apply dismissal independently of identity existence - cover pre-identity hiding and scope isolation with regression tests Validation: - dashwallet-dashpay arm64 iOS Simulator build succeeds
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
llbartekll
left a comment
llbartekll
left a comment
There was a problem hiding this comment.
The Home banner now honors the scoped dismissal, but the same Upgrade-to-DashPay banner in More/Menu still does not. MainMenuViewController's Hide action only calls markAsDismissed(), while CurrentUserProfileModel.updateShowJoinDashpay() never reads UsernamePrefs.shared.joinDashPayDismissed (and no dismissal event triggers a recomputation there). As a result, tapping Hide in the menu still leaves the banner visible. Please apply the same scoped dismissal policy to the menu/profile model and refresh it after Hide.
Apply the wallet-and-network-scoped dismissal policy to the menu profile model and refresh its published visibility immediately after Hide. Validation: dashwallet-dashpay arm64 iOS Simulator build succeeds.
3 participants
Issue being fixed or feature implemented
Tapping Hide on the "Upgrade to DashPay — Your Shielded balance is ready" banner did nothing — the banner stayed (reported as SB-8 during shielded-balance testing). Two causes:
UsernamePrefs.joinDashPayDismissedcached the value in an in-memory scalar and stored it under a global key — stale after a network round-trip and leaking between Mainnet/Testnet.HomeViewModelonly honored the dismissal when the wallet already had an identity (identityScopedDismissal = hasIdentity && dismissed). But the banner shows precisely when the user has no identity yet ("register your username"), so Hide was ignored in exactly the case it's offered.What was done?
JoinDashPayDismissalScope→ keyjoinDashPayDismissed.v2.<network>.<wallet>), reading/writingUserDefaultsdirectly (no stale in-memory scalar). This keeps the choice attached to the wallet/network where it was made and survives readiness re-evaluation and leaving/returning to the network.JoinDashPayBannerPolicy.shouldShow(contextReady:syncDone:dismissed:hasRegisteredUsername:hasRegistrationInProgress:)— no longer gated onhasIdentity, so Hide works before an identity exists.SwiftDashSDKCoreLifecycleTestscases for the banner policy.How Has This Been Tested?
Manual, testnet: tapping Hide on the Upgrade-to-DashPay banner now dismisses it immediately and it stays hidden across shielded-readiness re-evaluations and network round-trips within the session. Upgrade still works; Hide and Upgrade are independent. (Unit-test target is pre-existing broken; the added tests are compile-ready.)
Breaking Changes
None. New scoped key (
.v2.) — a prior global dismissal simply isn't read; the banner reappears once and can be re-hidden, now persistently.Checklist:
For repository code-owners and collaborators only