fix(wallet): hide unavailable account entry points#776
Conversation
Why: - CrowdNode sign-up and deposit flows are temporarily unavailable in several entry points. What: - Hide Explore staking unless the user has a linked CrowdNode account. - Remove CrowdNode from customizable and restored shortcuts when no account exists. - Filter portal items and adjust the portal table layout to the reduced item set. - Hide sign-up and link actions on the getting started screen. Validation: - Not run.
Why: - CrowdNode deposit and online account entry points should stay unavailable in the portal. What: - Always filter deposit and online account out of portalItems. Validation: - Not run.
|
Warning Review limit reached
More reviews will be available in 36 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR gates CrowdNode features based on signup state. Portal items are filtered to hide deposit and account-linking actions. The portal UI displays an unavailable message and hides signup buttons. Portal view tables resize dynamically based on filtered item count. Explore menu and home shortcuts conditionally display CrowdNode features only when signup is complete. ChangesCrowdNode Feature Availability
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
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)
DashWallet/Sources/UI/CrowdNode/Portal/CrowdNodePortalViewController.swift (1)
151-157:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDynamic section count now makes fixed reload index paths crash-prone.
Line 201 can produce only one section, but Line 156 still reloads
section: 1. That will trigger an out-of-bounds crash whenonlineAccountStatepublishes.Suggested fix
- viewModel.$crowdNodeBalance + viewModel.$crowdNodeBalance .receive(on: DispatchQueue.main) .removeDuplicates() .sink(receiveValue: { [weak self] _ in self?.balanceView.dataSource = self - self?.tableView.reloadRows(at: [ - IndexPath(item: 0, section: 0), - IndexPath(item: 1, section: 0), - ], - with: .none) + self?.tableView.reloadData() }) .store(in: &cancellableBag) viewModel.$walletBalance .receive(on: DispatchQueue.main) .removeDuplicates() .sink(receiveValue: { [weak self] _ in - self?.tableView.reloadRows(at: [ - IndexPath(item: 0, section: 0), - ], - with: .none) + self?.tableView.reloadData() }) .store(in: &cancellableBag) viewModel.$onlineAccountState .receive(on: DispatchQueue.main) .sink { [weak self] _ in - self?.tableView.reloadRows(at: [ - IndexPath(item: 1, section: 0), - IndexPath(item: 0, section: 1), - ], - with: .none) + self?.tableView.reloadData()Also applies to: 201-206
🤖 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/CrowdNode/Portal/CrowdNodePortalViewController.swift` around lines 151 - 157, The current sink on viewModel.$onlineAccountState calls tableView.reloadRows with hard-coded IndexPath(section: 1) which can be out-of-bounds when the table has only one section; update the closure in the viewModel.$onlineAccountState sink to compute valid index paths before calling tableView.reloadRows — e.g., query tableView.numberOfSections and tableView.numberOfRows(inSection:) (or consult the viewModel’s current sections) and only include IndexPath(item: 1, section: 0) and IndexPath(item: 0, section: 1) if those sections/rows exist, otherwise skip or reload available rows/sections to avoid the crash. Ensure the change is applied to the same closure where tableView.reloadRows(at:...) is invoked.
🤖 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/Sources/UI/Explore` Dash/ExploreMenuScreen.swift:
- Around line 34-36: The visibility check for Staking is too strict: update
hasCrowdNodeAccount to treat linked accounts as valid by including the
.linkedOnline state in the predicate (e.g. CrowdNode.shared.signUpState ==
.finished || CrowdNode.shared.signUpState == .linkedOnline) so linked existing
CrowdNode users see Staking; apply the same change to the other occurrence(s)
that reference CrowdNode.shared.signUpState (the logic around lines 91–100 in
ExploreMenuScreen) so both places use the relaxed check.
In `@DashWallet/Sources/UI/Home/Views/HomeViewModel.swift`:
- Around line 719-724: In the HomeViewModel filter closure that currently checks
action.type == .crowdNode and then only allows CrowdNode.shared.signUpState ==
.finished, update the condition to also permit when CrowdNode.shared.signUpState
== .linkedOnline so saved CrowdNode shortcuts for linked-online users are not
filtered out; locate the anonymous closure inside the .filter call (the one
testing action.type == .crowdNode) and add a second allowed state
(.linkedOnline) alongside .finished when deciding to return true.
In `@DashWallet/Sources/UI/Home/Views/Shortcuts/Models/ShortcutAction.swift`:
- Around line 55-57: The gating only allows CrowdNode when
CrowdNode.shared.signUpState == .finished, excluding accounts with the
.linkedOnline state; update the condition in ShortcutAction (where
CrowdNode.shared.signUpState is checked before calling
actions.append(.crowdNode)) to also permit .linkedOnline (e.g., check for
.finished || .linkedOnline or use a set/contains on allowed states) so linked
online users see the CrowdNode action.
---
Outside diff comments:
In `@DashWallet/Sources/UI/CrowdNode/Portal/CrowdNodePortalViewController.swift`:
- Around line 151-157: The current sink on viewModel.$onlineAccountState calls
tableView.reloadRows with hard-coded IndexPath(section: 1) which can be
out-of-bounds when the table has only one section; update the closure in the
viewModel.$onlineAccountState sink to compute valid index paths before calling
tableView.reloadRows — e.g., query tableView.numberOfSections and
tableView.numberOfRows(inSection:) (or consult the viewModel’s current sections)
and only include IndexPath(item: 1, section: 0) and IndexPath(item: 0, section:
1) if those sections/rows exist, otherwise skip or reload available
rows/sections to avoid the crash. Ensure the change is applied to the same
closure where tableView.reloadRows(at:...) is invoked.
🪄 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: 4854c6f4-2857-4492-9dee-c46e461dc988
📒 Files selected for processing (6)
DashWallet/Sources/UI/CrowdNode/CrowdNodeModel.swiftDashWallet/Sources/UI/CrowdNode/Getting Started/GettingStartedViewController.swiftDashWallet/Sources/UI/CrowdNode/Portal/CrowdNodePortalViewController.swiftDashWallet/Sources/UI/Explore Dash/ExploreMenuScreen.swiftDashWallet/Sources/UI/Home/Views/HomeViewModel.swiftDashWallet/Sources/UI/Home/Views/Shortcuts/Models/ShortcutAction.swift
Issue being fixed or feature implemented
CrowdNode sign-up, deposit, and related entry points should no longer be exposed in parts of the app where that functionality is temporarily unavailable. This change removes or hides those entry points so users only see actions that are currently supported.
What was done?
How Has This Been Tested?
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Bug Fixes