fix: update json parsing for CTX staging environment #1443
Conversation
WalkthroughUpdates to build/versioning and ExploreDash: RemoteDataSource now uses WalletDataProvider to choose BASE_URL vs DEV_BASE_URL based on network; GetMerchantResponse adds userDiscount and discount computed property; repositories, DI, constants, UI, and a layout height were adjusted accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller/ViewModel
participant Remote as RemoteDataSource
participant Wallet as WalletDataProvider
participant Config as CTXSpendConfig
participant API as CTX REST API
Caller->>Remote: request buildApi/buildTokenApi()
Remote->>Wallet: read networkParameters.id
Wallet-->>Remote: return network id (MAINNET or other)
alt MAINNET
Remote->>Config: use BASE_URL
else non-MAINNET
Remote->>Config: use DEV_BASE_URL
end
Remote->>API: construct Retrofit client with selected base URL
Remote-->>Caller: return API client
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
features/exploredash/src/main/java/org/dash/wallet/features/exploredash/data/dashspend/ctx/model/GetMerchantResponse.kt (1)
39-52: Newdiscountaccessor cleanly encapsulates discount precedenceUsing
discountto preferuserDiscountoversavingsPercentagesimplifies call sites and matches the staged JSON changes. For API clarity, consider making the backing fieldsInt? = nulland declaringval discount: Intexplicitly so the nullability and “0 means no discount” semantics are clearer, without changing runtime behavior.features/exploredash/src/main/java/org/dash/wallet/features/exploredash/utils/CTXSpendConstants.kt (1)
19-27: Staging URL constant is fine; name could be clearerAdding
DEV_BASE_URLforhttps://staging.spend.ctx.com/matches the new environment switching logic; in a future cleanup you might consider renaming it to something likeSTAGING_BASE_URLto better reflect the actual host.features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/CTXSpendRepository.kt (1)
246-250: getGiftCardDiscount correctly switches to cacheddiscount(denomination still unused)Reading
giftCardMap[merchantId]?.discountkeeps repository logic consistent with the new model; note that thedenominationparameter is still unused (as before), so at some point it may be worth either wiring it into the calculation or dropping it to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
build.gradle(1 hunks)features/exploredash/src/main/java/org/dash/wallet/features/exploredash/data/dashspend/ctx/model/GetMerchantResponse.kt(1 hunks)features/exploredash/src/main/java/org/dash/wallet/features/exploredash/di/ExploreDashModule.kt(1 hunks)features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/RemoteDataSource.kt(3 hunks)features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/CTXSpendRepository.kt(2 hunks)features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/DashSpendRepositoryFactory.kt(1 hunks)features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/dashspend/dialogs/GiftCardDetailsViewModel.kt(2 hunks)features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/explore/dialogs/ExploreDashInfoDialog.kt(1 hunks)features/exploredash/src/main/java/org/dash/wallet/features/exploredash/utils/CTXSpendConstants.kt(1 hunks)features/exploredash/src/main/res/layout/explore_dash_main_info.xml(1 hunks)wallet/build.gradle(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1417
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/data/explore/MerchantDao.kt:0-0
Timestamp: 2025-08-08T16:48:49.964Z
Learning: In PR dashpay/dash-wallet#1417, HashEngineering chose to defer adding Room indexes for gift_card_providers (provider, denominationsType, merchantId) to a follow-up PR; do not block the current PR on this optimization. Files: features/exploredash/.../data/explore/MerchantDao.kt and features/exploredash/.../data/dashspend/GiftCardProvider.kt.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: wallet/src/de/schildbach/wallet/data/PaymentIntent.java:476-481
Timestamp: 2025-08-25T15:04:18.461Z
Learning: HashEngineering prefers to keep payeeUsername and payeeUserId in PaymentIntent.toString() logs unredacted for debugging purposes, prioritizing troubleshooting capability over PII protection concerns in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
📚 Learning: 2025-08-08T16:48:49.964Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1417
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/data/explore/MerchantDao.kt:0-0
Timestamp: 2025-08-08T16:48:49.964Z
Learning: In PR dashpay/dash-wallet#1417, HashEngineering chose to defer adding Room indexes for gift_card_providers (provider, denominationsType, merchantId) to a follow-up PR; do not block the current PR on this optimization. Files: features/exploredash/.../data/explore/MerchantDao.kt and features/exploredash/.../data/dashspend/GiftCardProvider.kt.
Applied to files:
features/exploredash/src/main/java/org/dash/wallet/features/exploredash/di/ExploreDashModule.ktfeatures/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/DashSpendRepositoryFactory.ktfeatures/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/dashspend/dialogs/GiftCardDetailsViewModel.ktfeatures/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/CTXSpendRepository.kt
📚 Learning: 2025-05-08T18:11:40.249Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1390
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/ctxspend/CTXSpendViewModel.kt:129-145
Timestamp: 2025-05-08T18:11:40.249Z
Learning: The hardcoded test data in the purchaseGiftCard() function of CTXSpendViewModel is intentionally left in place for testing the error handling for limit mismatch, and will be fixed later before release.
Applied to files:
features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/dashspend/dialogs/GiftCardDetailsViewModel.ktfeatures/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/CTXSpendRepository.kt
📚 Learning: 2025-08-25T15:02:39.634Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/ctxspend/dialogs/GiftCardDetailsViewModel.kt:154-158
Timestamp: 2025-08-25T15:02:39.634Z
Learning: The CTXSpend getGiftCardByTxid API in CTXSpendRepository guarantees a non-null GiftCardResponse payload when returning ResponseResource.Success, so using response.value!! is safe and doesn't risk NPE despite the nullable type signature.
Applied to files:
features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/dashspend/dialogs/GiftCardDetailsViewModel.ktfeatures/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/CTXSpendRepository.kt
📚 Learning: 2025-04-19T07:01:17.535Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1362
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:107-115
Timestamp: 2025-04-19T07:01:17.535Z
Learning: In the Dash Wallet app, DashPayConfig methods like isTransactionMetadataInfoShown() already use withContext(Dispatchers.IO) internally, so wrapping these calls with another withContext(Dispatchers.IO) in ViewModels is redundant.
Applied to files:
features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/dashspend/dialogs/GiftCardDetailsViewModel.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (11)
features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/explore/dialogs/ExploreDashInfoDialog.kt (1)
31-34: Force-expanded dialog looks appropriate for this info sheetSetting
forceExpand = trueensures the bottom sheet opens fully, which makes sense for this one-shot informational dialog and pairs well with the updatedwrap_contentlayout height.wallet/build.gradle (1)
218-221: Versioning change is consistent with MMmmppbb scheme
11040210correctly encodes 11.4.2 build 10, and the11.4.2-ctx-stagingname clearly distinguishes this build as CTX staging–focused; just ensure your release/distribution channels expect this staging suffix.features/exploredash/src/main/res/layout/explore_dash_main_info.xml (1)
21-24: Changing root towrap_contentshould improve dialog sizingUsing
wrap_contentfor the root here should prevent the info dialog from filling the whole screen with empty space, whileforceExpandensures all content is visible; please sanity-check on smaller screens and landscape so the continue button remains fully accessible.build.gradle (1)
3-8: SwitchingdppVersionto a released version improves build stabilityUsing
2.0.2instead of a SNAPSHOT should make builds more reproducible; just confirm this exact version is published and resolvable from your Maven repositories before merging.features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/dashspend/dialogs/GiftCardDetailsViewModel.kt (1)
500-529: Preserving gift card notes on metadata updates makes senseStopping the explicit
note = nullin bothupdateGiftCardoverloads means existing notes (or provider order IDs) will no longer be cleared when card numbers or URLs are saved, which is safer for user/context data while leaving other behavior unchanged. Based on learnings, this aligns with how notes are used for gift card metadata.features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/CTXSpendRepository.kt (1)
205-217: Usingresponse.discountingetMerchantaligns with new JSON schemaRouting the stored
savingsPercentagethroughresponse.discountensures per-user discounts are honored when present while preserving existing behavior when only the legacy field is set; please validate this against a couple of real CTX staging responses (with and withoutuserDiscount) to confirm the precedence matches backend expectations.features/exploredash/src/main/java/org/dash/wallet/features/exploredash/repository/DashSpendRepositoryFactory.kt (1)
46-53: LGTM! Constructor parameter properly propagated.The updated
RemoteDataSourceinstantiation correctly passes the injectedwalletDataProvider, enabling network-aware URL resolution for the CTX staging environment.features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/RemoteDataSource.kt (3)
38-41: Constructor updated to support network-aware configuration.The addition of
WalletDataProviderenables dynamic URL resolution based on the wallet's network context.
61-74: Consistent URL resolution in token API.Good consistency applying the same network-aware URL logic to the token API. The same verification and null safety considerations from
buildApiapply here.
48-54: The original review comment's concerns about null safety are unfounded; both constants are properly defined and the code is correct.The verification confirms:
- DEV_BASE_URL exists and is correct: Points to
https://staging.spend.ctx.com/in CTXSpendConstants.kt- BASE_URL exists and is correct: Points to
https://spend.ctx.com/in CTXSpendConstants.kt- No null safety issue:
networkParametersis declared as a non-nullable property (NetworkParameters, notNetworkParameters?) in the WalletDataProvider interfaceThe network-aware URL selection logic at lines 48-54 is properly implemented with the correct constants and no defensive checks are needed.
features/exploredash/src/main/java/org/dash/wallet/features/exploredash/di/ExploreDashModule.kt (1)
58-61: DI wiring correctly updated.The provider method signature and
RemoteDataSourceinstantiation properly reflect the constructor changes, enabling network-aware URL resolution through dependency injection.
Issue being fixed or feature implemented
Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.