feat: txmetadata #1439
feat: txmetadata #1439
Conversation
* feat: update txmetadata handling * fix: save restore state to blockchain identity data * fix: do not include CoinJoin transactions * fix: log errors with more detail
…feature-txmetadata
…feature-txmetadata
* feat: convert SettingsActivity to a Fragment * feat: add nav graph for settings and add Tx Metadata dialogs and fragments * feat: format tx metadata settings fragment and link to DashPayConfig * feat: finish TransactionMetadataCostDialog * feat: update UI * feat: use TransactionMetadataSettings * fix: make ExchangeRatesConfig a singleton * fix: remove withContext for DataStore * style: update format * feat: add saveGiftcardInfo * fix: visuals * fix: change observing of the localCurrencySymbol * fix: add a tx metadata saving job * refactor: redo nav-graphs to make navigation easier * fix: add shouldSaveToCache * feat: add debugging, logs and adjust when tx-metadata is saved
…feature-txmetadata
* feat: add DashCheckBox and DashRadioButton * feat: refactor TransactionMetadataSettingsFragment as Composable * feat: reset wallet dialogs * feat: update metadata setting defaults * feat: always hide tx metadata settings by default
…feature-txmetadata
…feature-txmetadata
* fix: use Flow instead of LiveData when observing Work * fix: updates for components * remove duplicate setupRadioGroup method * feat: add exists method * feat: enable transaction metadata saving on prod flavor * feat: build out saving historical data * fix: improve logging * fix: enable reset wallet * fix: make TransactionMetadataDocument a data class * fix: improve some functions for metadata * fix: improve publish transaction metadata worker * fix: improve publish transaction metadata functions * fix: update should save methods in DashPayConfig * fix: use dpp 2.0.0-TX-SNAPSHOT * fix: update save status and progress * fix: last fixes for QA found issues * fix: coderabbit issues * fix: compile issues * fix: remove commented code * fix: remove unneeded argument * fix: remove unneeded nav graphs * fix: remove unneeded nav graphs
…feature-txmetadata
* fix: support Transaction Metadata in the new design for Settings * fix: align with txmetadata spec (txid stored in little endian) * fix: use smaller push periods on Testnet or debug * fix: fix the encryptionKey index calculation * fix: minor improvements
WalkthroughIntroduces a comprehensive transaction metadata feature enabling users to save and manage payment categories, tax information, exchange rates, and gift card data for wallet transactions. Includes new UI screens, database entities, service orchestration for platform synchronization, background work scheduling, and configuration management. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as TransactionMetadataSettings<br/>Screen
participant VM as TransactionMetadataSettings<br/>ViewModel
participant Config as DashPayConfig
participant DAO as Database DAOs
participant Sync as PlatformSync<br/>Service
participant Worker as PublishTxMetadata<br/>Worker
participant Platform as Platform
User->>UI: Opens Settings
UI->>VM: loadLastWorkId()
VM->>Config: getTransactionMetadataSettings()
Config-->>VM: Settings state
User->>UI: Clicks "Save to Network"
UI->>VM: saveToNetwork(forceSave)
VM->>DAO: getUnsavedTransactions()
DAO-->>VM: [Transactions, Timestamp]
alt Has Unsaved Transactions
UI->>User: Show confirmation dialog
User->>UI: Confirm save
else No Unsaved
VM->>VM: Proceed directly
end
VM->>VM: saveToNetworkNow()
VM->>Sync: publishPastTxMetadata(progress)
activate Sync
Sync->>DAO: getCachedItemsBefore()
DAO-->>Sync: Cached metadata
Sync->>Platform: Publish metadata batch
Platform-->>Sync: Ack
Sync-->>VM: Progress updates
deactivate Sync
VM->>Worker: Queue PublishTxMetadataWorker
activate Worker
Worker->>Sync: publishPastTxMetadata(callback)
Worker->>Config: Update LAST_PAST_SAVE
Worker-->>VM: Success/Result
deactivate Worker
VM->>UI: Emit completion status
UI->>User: Show success + navigate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (7)
wallet/src/de/schildbach/wallet/service/platform/TxMetadataSaveInfo.kt (1)
20-27: LGTM! Clean and straightforward implementation.The data class structure is appropriate for tracking transaction metadata save progress. The immutable properties and the
NONEconstant follow good Kotlin conventions.As an optional enhancement, consider adding KDoc to document the purpose of this class and its properties:
+/** + * Tracks the progress of saving transaction metadata items. + * + * @property itemsSaved The number of items that have been successfully saved + * @property itemsToSave The total number of items that need to be saved + */ data class TxMetadataSaveInfo( val itemsSaved: Int, val itemsToSave: Int ) { companion object { + /** Represents no save progress (zero items saved and zero items to save) */ val NONE = TxMetadataSaveInfo(0, 0) } }wallet/src/de/schildbach/wallet/database/dao/TransactionMetadataDocumentDao.kt (1)
178-179: Consider adding ORDER BY for deterministic results.Unlike other queries in this DAO that use
ORDER BY timestamp DESC, this query doesn't specify an order. If multiple documents exist for a txId, the order will be non-deterministic. Consider adding an ORDER BY clause for consistency.- @Query("SELECT * FROM transaction_metadata_platform WHERE txId = :txId") + @Query("SELECT * FROM transaction_metadata_platform WHERE txId = :txId ORDER BY timestamp DESC") suspend fun getTransactionMetadata(txId: Sha256Hash): List<TransactionMetadataDocument>common/build.gradle (1)
86-86: Update androidx.activity:activity-compose to latest stable version 1.11.0.The latest stable version of androidx.activity:activity-compose is 1.11.0 (released September 10, 2025), which is significantly newer than the current 1.8.0 and likely includes important bug fixes and improvements.
wallet/src/de/schildbach/wallet/ui/more/ContactSupportViewModel.kt (1)
236-262: Consider BuildConfig gate for development-only code.The comment "only for development" suggests this attachment should not be included in production builds. Consider wrapping this block with a BuildConfig.DEBUG check to avoid exposing internal metadata documents in production support emails.
if (BuildConfig.DEBUG) { try { val txMetadataEntries = File.createTempFile("tx-metadata-documents.", ".txt", reportDir) // ... rest of the block } catch (x: IOException) { log.info("problem writing attachment", x) } }wallet/src/de/schildbach/wallet/service/platform/work/PublishTransactionMetadataWorker.kt (1)
62-62: Remove or document commented code.The commented
//KEY_IDENTITY to identity,line should either be removed if unused or uncommented with proper implementation if needed.wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt (1)
931-949: Consider using IO dispatcher for wallet operations.The
metadataReminder()function iterates through all transactions usingwallet.getTransactions(true), which may be a blocking database operation. Consider wrapping the transaction counting logic inwithContext(Dispatchers.IO)to avoid blocking the main thread.fun metadataReminder() { viewModelScope.launch { if (hasIdentity && !dashPayConfig.isTransactionMetadataInfoShown()) { // have there been 10 transactions since the last update? val installedDate = dashPayConfig.getMetadataFeatureInstalled() walletData.wallet?.let { wallet: Wallet -> - var count = 0 - wallet.getTransactions(true).forEach { tx -> - if (tx.updateTime.time > installedDate) { - count++ + withContext(Dispatchers.IO) { + var count = 0 + wallet.getTransactions(true).forEach { tx -> + if (tx.updateTime.time > installedDate) { + count++ + } } + if (count >= 10) { + _remindMetadata.value = true + } } - if (count >= 10) { - _remindMetadata.value = true - } } } } }common/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.kt (1)
68-68: Consider using theme colors instead of hardcoded values.Several Color values are hardcoded (e.g.,
Color.White,Color(0xFF008DE4),Color(0xFF191C1F), etc.) instead of usingMyTheme.Colors.*. For consistency and easier theming, consider using theme color references.Examples:
- Line 68:
Color.White→MyTheme.Colors.backgroundPrimary- Line 83:
Color(0xFF008DE4)→MyTheme.Colors.dashBlue- Line 115, 125, 179:
Color(0xFF525C66)→MyTheme.Colors.textSecondaryAlso applies to: 83-83, 115-115, 125-125, 150-150, 156-156, 166-166, 179-179
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
build.gradle(0 hunks)common/build.gradle(1 hunks)common/src/main/java/org/dash/wallet/common/services/TransactionMetadataProvider.kt(1 hunks)common/src/main/java/org/dash/wallet/common/ui/components/DashCheckBox.kt(1 hunks)common/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.kt(1 hunks)common/src/main/java/org/dash/wallet/common/ui/components/DashRadioButton.kt(1 hunks)common/src/main/java/org/dash/wallet/common/ui/components/MyTheme.kt(2 hunks)common/src/main/java/org/dash/wallet/common/ui/components/System.kt(1 hunks)common/src/main/java/org/dash/wallet/common/ui/dialogs/AdaptiveDialog.kt(3 hunks)common/src/main/java/org/dash/wallet/common/ui/dialogs/ExtraActionDialog.kt(2 hunks)common/src/main/java/org/dash/wallet/common/ui/radio_group/RadioGroupAdapter.kt(1 hunks)common/src/main/res/drawable/ic_radio_button_circle.xml(1 hunks)wallet/res/drawable/ic_file.xml(1 hunks)wallet/res/layout/dialog_transaction_metadata.xml(1 hunks)wallet/res/layout/dialog_transaction_metadata_costs.xml(1 hunks)wallet/res/layout/fragment_settings.xml(2 hunks)wallet/res/layout/fragment_transaction_metadata_settings.xml(1 hunks)wallet/res/navigation/nav_home.xml(4 hunks)wallet/res/values/strings-dashpay.xml(1 hunks)wallet/res/values/strings-extra.xml(1 hunks)wallet/src/de/schildbach/wallet/Constants.java(3 hunks)wallet/src/de/schildbach/wallet/WalletApplication.java(1 hunks)wallet/src/de/schildbach/wallet/database/dao/TransactionMetadataChangeCacheDao.kt(3 hunks)wallet/src/de/schildbach/wallet/database/dao/TransactionMetadataDocumentDao.kt(1 hunks)wallet/src/de/schildbach/wallet/database/entity/TransactionMetadataCacheItem.kt(1 hunks)wallet/src/de/schildbach/wallet/database/entity/TransactionMetadataDocument.kt(1 hunks)wallet/src/de/schildbach/wallet/service/WalletTransactionMetadataProvider.kt(11 hunks)wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt(23 hunks)wallet/src/de/schildbach/wallet/service/platform/TxMetadataSaveInfo.kt(1 hunks)wallet/src/de/schildbach/wallet/service/platform/work/PublishTransactionMetadataOperation.kt(1 hunks)wallet/src/de/schildbach/wallet/service/platform/work/PublishTransactionMetadataWorker.kt(1 hunks)wallet/src/de/schildbach/wallet/service/work/BaseWorker.kt(1 hunks)wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt(3 hunks)wallet/src/de/schildbach/wallet/ui/dashpay/CreateIdentityService.kt(0 hunks)wallet/src/de/schildbach/wallet/ui/dashpay/utils/DashPayConfig.kt(3 hunks)wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt(4 hunks)wallet/src/de/schildbach/wallet/ui/main/WalletFragment.kt(1 hunks)wallet/src/de/schildbach/wallet/ui/more/ContactSupportViewModel.kt(4 hunks)wallet/src/de/schildbach/wallet/ui/more/MixDashFirstViewModel.kt(0 hunks)wallet/src/de/schildbach/wallet/ui/more/SaveMetadataAndResetDialogFragment.kt(1 hunks)wallet/src/de/schildbach/wallet/ui/more/SecurityFragment.kt(3 hunks)wallet/src/de/schildbach/wallet/ui/more/SecurityViewModel.kt(3 hunks)wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt(2 hunks)wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt(6 hunks)wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataCostDialog.kt(1 hunks)wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataDialog.kt(1 hunks)wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsFragment.kt(1 hunks)wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsScreen.kt(1 hunks)wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsViewModel.kt(1 hunks)
💤 Files with no reviewable changes (3)
- wallet/src/de/schildbach/wallet/ui/more/MixDashFirstViewModel.kt
- wallet/src/de/schildbach/wallet/ui/dashpay/CreateIdentityService.kt
- build.gradle
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
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: 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: 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-05-12T09:15:56.594Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1391
File: integrations/coinbase/src/main/java/org/dash/wallet/integrations/coinbase/ui/convert_currency/ConvertViewFragment.kt:134-155
Timestamp: 2025-05-12T09:15:56.594Z
Learning: The SegmentedPicker composable in Dash Wallet has a safety check for empty options, returning early if options.isEmpty() is true.
Applied to files:
common/src/main/java/org/dash/wallet/common/ui/components/DashCheckBox.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:
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataCostDialog.ktwallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsFragment.ktwallet/src/de/schildbach/wallet/ui/more/SaveMetadataAndResetDialogFragment.ktwallet/src/de/schildbach/wallet/ui/main/WalletFragment.ktwallet/src/de/schildbach/wallet/ui/more/ContactSupportViewModel.ktcommon/src/main/java/org/dash/wallet/common/ui/dialogs/AdaptiveDialog.ktwallet/src/de/schildbach/wallet/ui/dashpay/utils/DashPayConfig.ktwallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsScreen.ktwallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsViewModel.ktwallet/src/de/schildbach/wallet/ui/SettingsFragment.ktwallet/src/de/schildbach/wallet/database/dao/TransactionMetadataChangeCacheDao.ktwallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.ktcommon/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.ktwallet/src/de/schildbach/wallet/ui/more/TransactionMetadataDialog.ktwallet/src/de/schildbach/wallet/service/WalletTransactionMetadataProvider.ktwallet/src/de/schildbach/wallet/ui/more/SecurityViewModel.ktwallet/src/de/schildbach/wallet/service/platform/work/PublishTransactionMetadataWorker.ktwallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt
📚 Learning: 2025-09-23T05:19:47.861Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1428
File: wallet/src/de/schildbach/wallet/ui/dashpay/PlatformRepo.kt:1065-1068
Timestamp: 2025-09-23T05:19:47.861Z
Learning: The Dash wallet app doesn't use non-encrypted wallets, so getWalletEncryptionKey() will never return null in practice. Defensive null-safe programming with ?.let is appropriate for methods like getIdentityFromPublicKeyId() in PlatformRepo.kt.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SecurityFragment.ktwallet/src/de/schildbach/wallet/ui/main/WalletFragment.ktcommon/src/main/java/org/dash/wallet/common/ui/dialogs/AdaptiveDialog.ktwallet/src/de/schildbach/wallet/ui/dashpay/utils/DashPayConfig.ktwallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.ktwallet/src/de/schildbach/wallet/service/WalletTransactionMetadataProvider.ktwallet/src/de/schildbach/wallet/ui/more/SecurityViewModel.ktwallet/src/de/schildbach/wallet/ui/main/MainViewModel.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 Dash Wallet, DataStore operations (like those in DashPayConfig through BaseConfig) already handle background threading internally. Adding another layer of withContext(Dispatchers.IO) around these calls in ViewModels is redundant because the Jetpack DataStore API automatically dispatches its operations to a background thread.
Applied to files:
wallet/src/de/schildbach/wallet/ui/main/WalletFragment.ktwallet/src/de/schildbach/wallet/ui/more/ContactSupportViewModel.ktwallet/src/de/schildbach/wallet/ui/dashpay/utils/DashPayConfig.ktwallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsViewModel.ktwallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.ktwallet/src/de/schildbach/wallet/service/WalletTransactionMetadataProvider.ktwallet/src/de/schildbach/wallet/ui/more/SecurityViewModel.ktwallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt
📚 Learning: 2025-05-07T14:18:11.161Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1389
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/explore/SearchFragment.kt:45-46
Timestamp: 2025-05-07T14:18:11.161Z
Learning: In the ExploreViewModel of the dash-wallet application, `appliedFilters` is a StateFlow (not LiveData), so Flow operators like `distinctUntilChangedBy` can be used with it.
Applied to files:
wallet/src/de/schildbach/wallet/ui/main/WalletFragment.ktwallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsViewModel.ktwallet/src/de/schildbach/wallet/database/dao/TransactionMetadataChangeCacheDao.ktwallet/src/de/schildbach/wallet/ui/more/SecurityViewModel.ktwallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt
📚 Learning: 2025-05-12T09:14:36.656Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1391
File: wallet/src/de/schildbach/wallet/ui/payments/PaymentsFragment.kt:111-119
Timestamp: 2025-05-12T09:14:36.656Z
Learning: In the dashpay/dash-wallet project, explicitly unregistering ViewPager2 callbacks is not considered critical since memory leaks haven't been observed in practice with the memory profiler.
Applied to files:
wallet/src/de/schildbach/wallet/ui/main/WalletFragment.ktwallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/src/de/schildbach/wallet/ui/more/SecurityViewModel.kt
📚 Learning: 2025-05-06T15:46:59.440Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1386
File: wallet/src/de/schildbach/wallet/ui/send/SendCoinsViewModel.kt:108-0
Timestamp: 2025-05-06T15:46:59.440Z
Learning: In UI code where frequent updates to a value might trigger expensive operations (like the SendCoinsViewModel's `executeDryrun` method), it's preferable to use a Flow with debounce rather than launching a new coroutine for each update. This prevents race conditions, reduces resource usage, and provides a more reactive architecture.
Applied to files:
common/src/main/java/org/dash/wallet/common/ui/dialogs/AdaptiveDialog.ktwallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
📚 Learning: 2025-04-16T17:07:27.359Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1377
File: wallet/src/de/schildbach/wallet/service/platform/PlatformService.kt:0-0
Timestamp: 2025-04-16T17:07:27.359Z
Learning: For PlatformService in the Dash Wallet, the implementation should avoid throwing exceptions when Constants.SUPPORTS_PLATFORM is false. The platform should only be initialized when SUPPORTS_PLATFORM is true, and initialization should be done lazily to defer Platform creation until needed.
Applied to files:
wallet/src/de/schildbach/wallet/Constants.javawallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt
📚 Learning: 2025-08-08T13:29:37.702Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1419
File: wallet/src/de/schildbach/wallet/payments/SendCoinsTaskRunner.kt:312-331
Timestamp: 2025-08-08T13:29:37.702Z
Learning: Dash Wallet: For network presence checks, if a transaction is not in the wallet, we generally won’t have broadcast peer data (likely 0) and chain-locks only arrive after the block containing the transaction; thus fallback confidence checks on the local transaction often remain false. It’s safe but usually non-effective to include them; primary detection should rely on the wallet-known transaction.
Applied to files:
wallet/src/de/schildbach/wallet/Constants.javawallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt
📚 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:
wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.ktwallet/src/de/schildbach/wallet/service/WalletTransactionMetadataProvider.kt
🧬 Code graph analysis (13)
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataCostDialog.kt (1)
common/src/main/java/org/dash/wallet/common/ui/FragmentViewBindingDelegate.kt (1)
viewBinding(81-82)
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsFragment.kt (3)
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsScreen.kt (1)
TransactionMetadataSettingsScreen(53-248)common/src/main/java/org/dash/wallet/common/util/NavigationExtensions.kt (1)
safeNavigate(30-40)wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsViewModel.kt (1)
saveToNetwork(255-272)
wallet/src/de/schildbach/wallet/ui/more/SaveMetadataAndResetDialogFragment.kt (2)
common/src/main/java/org/dash/wallet/common/ui/components/ComposeHostFrameLayout.kt (1)
setContent(18-24)common/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.kt (1)
ModalDialog(43-220)
wallet/src/de/schildbach/wallet/ui/main/WalletFragment.kt (1)
common/src/main/java/org/dash/wallet/common/util/NavigationExtensions.kt (1)
safeNavigate(30-40)
wallet/src/de/schildbach/wallet/database/entity/TransactionMetadataCacheItem.kt (1)
common/src/main/java/org/dash/wallet/common/data/entity/TransactionMetadata.kt (1)
isNotEmpty(49-52)
common/src/main/java/org/dash/wallet/common/ui/dialogs/ExtraActionDialog.kt (3)
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataDialog.kt (1)
show(75-78)wallet/src/de/schildbach/wallet/ui/more/SaveMetadataAndResetDialogFragment.kt (1)
show(109-112)common/src/main/java/org/dash/wallet/common/ui/dialogs/AdaptiveDialog.kt (1)
show(209-214)
wallet/src/de/schildbach/wallet/ui/dashpay/utils/DashPayConfig.kt (1)
common/src/main/java/org/dash/wallet/common/data/BaseConfig.kt (4)
get(78-80)set(82-86)setSecuredData(95-97)getSecuredData(92-93)
wallet/src/de/schildbach/wallet/WalletApplication.java (1)
wallet/src/de/schildbach/wallet/util/LogMarkerFilter.kt (1)
addAcceptedMarker(17-19)
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsScreen.kt (3)
common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt (1)
DashButton(40-156)common/src/main/java/org/dash/wallet/common/ui/components/DashCheckBox.kt (1)
DashCheckbox(22-157)common/src/main/java/org/dash/wallet/common/ui/components/DashRadioButton.kt (1)
DashRadioButton(34-117)
common/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.kt (1)
common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt (1)
DashButton(40-156)
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataDialog.kt (5)
common/src/main/java/org/dash/wallet/common/ui/FragmentViewBindingDelegate.kt (1)
viewBinding(81-82)common/src/main/java/org/dash/wallet/common/util/NavigationExtensions.kt (1)
dialogSafeNavigate(42-52)wallet/src/de/schildbach/wallet/ui/more/SaveMetadataAndResetDialogFragment.kt (1)
show(109-112)common/src/main/java/org/dash/wallet/common/ui/dialogs/AdaptiveDialog.kt (1)
show(209-214)common/src/main/java/org/dash/wallet/common/ui/dialogs/ExtraActionDialog.kt (1)
show(93-97)
wallet/src/de/schildbach/wallet/service/platform/work/PublishTransactionMetadataWorker.kt (1)
wallet/src/de/schildbach/wallet/service/work/BaseWorker.kt (2)
setProgress(31-33)formatExceptionMessage(51-62)
wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt (1)
common/src/main/java/org/dash/wallet/common/ui/components/MenuItem.kt (1)
MenuItem(44-263)
🪛 detekt (1.23.8)
wallet/src/de/schildbach/wallet/ui/more/SaveMetadataAndResetDialogFragment.kt
[warning] 165-165: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsScreen.kt
[warning] 307-307: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsViewModel.kt
[warning] 232-232: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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)
- GitHub Check: build
🔇 Additional comments (29)
common/src/main/java/org/dash/wallet/common/ui/radio_group/RadioGroupAdapter.kt (1)
36-40: Trailing comma is idiomatic Kotlin—LGTM!Adding a trailing comma after the last parameter in multi-line parameter lists is a Kotlin best practice that reduces diff noise when adding new parameters in future PRs and aligns with ktlint conventions.
wallet/res/drawable/ic_file.xml (1)
1-12: Valid Android vector drawable resource.The drawable defines a clean file icon with proper dimensions (20×24 dp), consistent viewport sizing, valid path data, and uniform fill color. XML structure and Android drawable conventions are correctly applied.
common/src/main/java/org/dash/wallet/common/ui/components/DashRadioButton.kt (1)
54-65: Whitespace-only change; component implementation looks solid.Line 64 appears to be a whitespace adjustment with no functional impact. The
DashRadioButtoncomponent itself is well-designed: proper use of Compose patterns, good accessibility support viaRole.RadioButton, and flexible parameterization for leading icons, trailing text, and help text. Helper composables are cleanly extracted, and the preview coverage is comprehensive.wallet/res/layout/fragment_settings.xml (2)
2-17: License header properly added.
206-258: All string and drawable resources are properly defined.Verification confirms:
@string/transaction_metadata_titleand@string/transaction_metadata_data_being_savedare defined inwallet/res/values/strings-dashpay.xml@drawable/ic_fileis defined inwallet/res/drawable/ic_file.xmlNo issues found.
wallet/src/de/schildbach/wallet/database/dao/TransactionMetadataChangeCacheDao.kt (3)
52-53: LGTM!The query correctly uses
DISTINCTto ensure unique transaction IDs are returned.
168-225: LGTM! Well-designed existence check.The use of
ISfor NULL comparisons is correct SQL syntax, and the Kotlin overload provides a clean API by accepting the entity object directly. This follows good DAO design patterns.
150-166: No issues found after verification.The
transaction_metadatatable is properly defined as a Room @entity (atcommon/src/main/java/org/dash/wallet/common/data/entity/TransactionMetadata.kt), which auto-generates the table schema. Both required columns—txIdandtimestamp—exist in the entity. The subquery logic correctly filters cached items by txIds from the metadata table wheretimestamp < maxTimestamp, making the cross-table join valid and intentional.wallet/src/de/schildbach/wallet/WalletApplication.java (1)
633-633: LGTM! Logging configuration for publish workflow.Adding the "PUBLISH" marker enables filtering logs related to the transaction metadata publish workflow introduced in this PR.
wallet/src/de/schildbach/wallet/ui/main/WalletFragment.kt (1)
237-241: LGTM! Clean navigation wiring.The observer correctly triggers navigation to transaction metadata settings when the reminder flag is set. Using
safeNavigateensures the navigation only occurs from the correct source destination.wallet/src/de/schildbach/wallet/Constants.java (2)
63-63: LGTM! Helpful documentation.The comment clarifies that 32-bit devices don't support the platform, which helps explain the conditional initialization logic.
100-100: Verify transaction metadata feature readiness for production.Enabling
SUPPORTS_TXMETADATAfor prod and testnet is a significant change. Please ensure:
- All transaction metadata storage and synchronization mechanisms are fully tested
- Platform synchronization handles errors gracefully
- Migration/backward compatibility is handled if users have existing wallets
- Performance impact has been assessed, especially for users with many transactions
Also applies to: 119-119
wallet/res/values/strings-extra.xml (1)
433-433: LGTM! String resource addition.The new
reset_wallet_buttonstring resource is properly defined and follows existing naming conventions.common/src/main/java/org/dash/wallet/common/services/TransactionMetadataProvider.kt (1)
121-124: LGTM! Clean API extension.The new
exists()method provides a lightweight way to check for metadata presence without fetching the full object. The documentation is clear and the signature is consistent with other provider methods.wallet/src/de/schildbach/wallet/service/work/BaseWorker.kt (1)
22-28: Null-safe progress/error helpers look good.Accepting nullable
Dataand defaulting to-1avoids crashes when WorkManager hands back empty payloads.common/src/main/java/org/dash/wallet/common/ui/components/MyTheme.kt (1)
31-32: Thanks for exposing the shared font/color tokens.Promoting
interRegular(and the gray variants below) makes it straightforward for the new components to stay on-brand without redeclaring assets.wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataCostDialog.kt (1)
34-41: Observer wiring looks correct.Hooking the dialog copy to
selectedExchangeRatekeeps the localized cost string current with the latest fiat rate.common/src/main/java/org/dash/wallet/common/ui/dialogs/AdaptiveDialog.kt (1)
195-199: Positive button theming hook fits well.Reading
POS_BUTTON_COLOR_ARGbefore wiring the click listener lets the new red style slot in without disturbing existing dialogs.wallet/res/layout/dialog_transaction_metadata_costs.xml (1)
43-81: Dialog layout stays consistent with the existing modal pattern.The constraints and styles mirror the other metadata dialogs, so nothing jumps out as problematic.
wallet/res/layout/dialog_transaction_metadata.xml (1)
67-278: Nice job structuring the metadata sections.Keeping each icon/title/description pair inside its own constrained block maintains alignment with the rest of the settings UI.
wallet/src/de/schildbach/wallet/ui/more/SecurityFragment.kt (1)
142-198: Reset flow styling looks consistent.Passing the red button style via
POS_BUTTON_COLOR_ARGkeeps the dialog logic intact while matching the new visual spec.wallet/src/de/schildbach/wallet/database/entity/TransactionMetadataCacheItem.kt (3)
72-77: LGTM! Simplified emptiness check.The removal of
sentTimestampfrom the emptiness check is appropriate since the timestamp is metadata about the cache entry itself, not part of the substantive transaction metadata being cached.
82-104: LGTM! Clean delta computation.The minus operator correctly computes a delta by nulling out unchanged fields. The txId validation ensures safety against accidental misuse.
106-123: Potential NPE in giftCard comparison.When
giftCardis non-null butgiftCardEqualsevaluation encounters a null field mismatch, line 122 could evaluategiftCardEquals == truewheregiftCardEqualsmight be implicitly null/false from the let block logic.However, the logic appears safe on closer inspection: the
letblock returns a Boolean, sogiftCardEqualswill be Boolean or null. The final return correctly handles both cases.wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt (1)
220-227: LGTM! Proper visibility gating and string resource usage.The Transaction Metadata menu item correctly:
- Uses both capability flag and UI state for visibility
- Uses localized string resource for the title
- Displays dynamic subtitle from UI state
wallet/src/de/schildbach/wallet/ui/more/SecurityViewModel.kt (2)
61-61: LGTM! Proper base class inheritance.Correctly extends
BaseProfileViewModeland passes required dependencies to the base class.
145-147: LGTM! Defensive IO context usage.The
withContext(Dispatchers.IO)wrapper is appropriate for callingplatformSyncService.hasPendingTxMetadataToSave(), ensuring the operation doesn't block the main thread even if the service method doesn't internally handle threading.wallet/src/de/schildbach/wallet/service/platform/work/PublishTransactionMetadataWorker.kt (1)
48-87: LGTM! Proper worker implementation with comprehensive error handling.The worker correctly:
- Propagates BitcoinJ context
- Reports progress via callback
- Handles success/partial failure/exception cases
- Logs analytics for failures
- Updates last-save timestamp only on complete success
wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt (1)
144-144: LGTM! Proper property declaration.Converting
dashPayConfigto a private property allows it to be referenced throughout the class.
| LimitationItem("0", "text", true), | ||
| LimitationItem("0", "text", true) | ||
| ), | ||
| ButtonData("Learn more", {}), |
There was a problem hiding this comment.
Fix preview parameter syntax.
Line 262 is missing the parameter name for moreInfoButton.
Apply this diff:
- ButtonData("Learn more", {}),
+ moreInfoButton = ButtonData("Learn more", {}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ButtonData("Learn more", {}), | |
| moreInfoButton = ButtonData("Learn more", {}), |
🤖 Prompt for AI Agents
In common/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.kt
around line 262, the preview invocation is missing the parameter name for the
moreInfoButton argument; update the call by prefixing that entry with the
parameter name (e.g. moreInfoButton = ButtonData("Learn more", {})) so it
matches the function signature and named-parameter usage.
| suspend fun showAsync(activity: FragmentActivity, onResult: (Boolean?) -> Unit, onExtraMessageAction: () -> Unit) { | ||
| if (!activity.lifecycle.currentState.isAtLeast(Lifecycle.State.RESUMED)) { | ||
| return | ||
| } | ||
|
|
||
| suspendCancellableCoroutine { coroutine -> | ||
| val onResultClick: (Boolean?) -> Unit = { result -> | ||
| if (coroutine.isActive) { | ||
| onResult.invoke(result) | ||
| coroutine.resume(Unit) | ||
| } | ||
| } | ||
|
|
||
| val onExtraMessageActionClick: () -> Unit = { | ||
| if (coroutine.isActive) { | ||
| onExtraMessageAction.invoke() | ||
| coroutine.resume(Unit) | ||
| } | ||
| } | ||
|
|
||
| try { | ||
| show(activity, onResultClick, onExtraMessageActionClick) | ||
| } catch (ex: Exception) { | ||
| if (coroutine.isActive) { | ||
| coroutine.resumeWithException(ex) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Don’t silently drop showAsync invocations before RESUMED
Right now, if the caller awaits showAsync while the Activity is in STARTED (or earlier), the guard returns immediately, the dialog never appears, and the caller never gets a result. We should wait for RESUMED instead of bailing out. Wrap the coroutine work in whenStateAtLeast (lifecycle-ktx) so the suspension survives configuration changes without losing the request.
-import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.whenStateAtLeast
...
- if (!activity.lifecycle.currentState.isAtLeast(Lifecycle.State.RESUMED)) {
- return
- }
-
- suspendCancellableCoroutine { coroutine ->
+ activity.lifecycle.whenStateAtLeast(Lifecycle.State.RESUMED) {
+ suspendCancellableCoroutine { coroutine ->
val onResultClick: (Boolean?) -> Unit = { result ->
if (coroutine.isActive) {
onResult.invoke(result)
coroutine.resume(Unit)
}
}
...
- try {
- show(activity, onResultClick, onExtraMessageActionClick)
- } catch (ex: Exception) {
- if (coroutine.isActive) {
- coroutine.resumeWithException(ex)
- }
- }
- }
+ try {
+ show(activity, onResultClick, onExtraMessageActionClick)
+ } catch (ex: Exception) {
+ if (coroutine.isActive) {
+ coroutine.resumeWithException(ex)
+ }
+ }
+ }
+ }| <androidx.constraintlayout.widget.ConstraintLayout | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| app:layout_constraintStart_toEndOf="@id/transaction_metadata_icon" | ||
| app:layout_constraintBottom_toBottomOf="parent" | ||
| app:layout_constraintTop_toTopOf="parent"> |
There was a problem hiding this comment.
Add margin to nested ConstraintLayout for proper icon spacing.
The nested ConstraintLayout lacks android:layout_marginStart to create spacing from the icon. Compare this to the coinjoin section (line 142) which uses marginStart="22dp". Without this margin, the title and subtitle will appear cramped against the icon, degrading visual consistency.
Apply this diff:
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="wrap_content"
android:layout_height="wrap_content"
+ android:layout_marginStart="22dp"
app:layout_constraintStart_toEndOf="@id/transaction_metadata_icon"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintTop_toTopOf="parent">🤖 Prompt for AI Agents
In wallet/res/layout/fragment_settings.xml around lines 221 to 226, the nested
ConstraintLayout adjacent to the transaction icon lacks a start margin causing
the title/subtitle to sit too close to the icon; add
android:layout_marginStart="22dp" (matching the coinjoin section) to that
ConstraintLayout so there is consistent horizontal spacing between the icon and
the text.
| android:layout_height="match_parent" | ||
| android:layout_marginHorizontal="20dp" | ||
| android:background="@color/background_primary" | ||
| tools:context="de.schildbach.wallet.ui.username.UsernameRequestsFragment"> |
There was a problem hiding this comment.
Incorrect tools:context attribute.
The tools:context points to UsernameRequestsFragment but this layout is for TransactionMetadataSettingsFragment.
Apply this diff:
- tools:context="de.schildbach.wallet.ui.username.UsernameRequestsFragment">
+ tools:context="de.schildbach.wallet.ui.more.TransactionMetadataSettingsFragment">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tools:context="de.schildbach.wallet.ui.username.UsernameRequestsFragment"> | |
| tools:context="de.schildbach.wallet.ui.more.TransactionMetadataSettingsFragment"> |
🤖 Prompt for AI Agents
In wallet/res/layout/fragment_transaction_metadata_settings.xml around line 24,
the tools:context attribute incorrectly references
de.schildbach.wallet.ui.username.UsernameRequestsFragment; update it to
reference the correct fragment class for this layout,
de.schildbach.wallet.ui.settings.TransactionMetadataSettingsFragment (or the
actual fully-qualified TransactionMetadataSettingsFragment class used in the
project) so the tools namespace points to the proper fragment for layout
previews and tooling.
| <ImageView | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center_vertical" | ||
| android:src="@drawable/ic_info" /> |
There was a problem hiding this comment.
Add contentDescription for accessibility.
The info icon ImageView lacks a contentDescription attribute, which is required for screen reader accessibility.
Apply this diff:
<ImageView
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_vertical"
+ android:contentDescription="@string/transaction_metadata_info"
android:src="@drawable/ic_info" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <ImageView | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:layout_gravity="center_vertical" | |
| android:src="@drawable/ic_info" /> | |
| <ImageView | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:layout_gravity="center_vertical" | |
| android:contentDescription="@string/transaction_metadata_info" | |
| android:src="@drawable/ic_info" /> |
🤖 Prompt for AI Agents
In wallet/res/layout/fragment_transaction_metadata_settings.xml around lines
46-50 the ImageView for the info icon is missing a contentDescription; add an
android:contentDescription attribute referencing a string resource (e.g.
@string/transaction_info_description) to provide a readable label for screen
readers and add that string to wallet/res/values/strings.xml; if the icon is
purely decorative instead set android:contentDescription="@null".
| } else if (lastSaveDate != 0L){ | ||
| stringResource(R.string.transaction_metadata_past_already_saved, dateFormat.format(Date(lastSaveDate))) | ||
| } else { | ||
| stringResource(R.string.transaction_metadata_past_already_saved_none, dateFormat.format(Date(lastSaveDate))) | ||
| }, |
There was a problem hiding this comment.
Avoid showing “Jan 1, 1970” when nothing was saved yet
When lastSaveDate is 0L, Date(0) renders as January 1 1970, so users see a bogus timestamp the first time they open the screen. Let’s treat the zero case separately and surface the “none” text without formatting a date.
} else if (lastSaveDate != 0L){
- stringResource(R.string.transaction_metadata_past_already_saved, dateFormat.format(Date(lastSaveDate)))
+ stringResource(
+ R.string.transaction_metadata_past_already_saved,
+ dateFormat.format(Date(lastSaveDate))
+ )
} else {
- stringResource(R.string.transaction_metadata_past_already_saved_none, dateFormat.format(Date(lastSaveDate)))
+ stringResource(R.string.transaction_metadata_past_already_saved_none)
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if (lastSaveDate != 0L){ | |
| stringResource(R.string.transaction_metadata_past_already_saved, dateFormat.format(Date(lastSaveDate))) | |
| } else { | |
| stringResource(R.string.transaction_metadata_past_already_saved_none, dateFormat.format(Date(lastSaveDate))) | |
| }, | |
| } else if (lastSaveDate != 0L){ | |
| stringResource( | |
| R.string.transaction_metadata_past_already_saved, | |
| dateFormat.format(Date(lastSaveDate)) | |
| ) | |
| } else { | |
| stringResource(R.string.transaction_metadata_past_already_saved_none) | |
| }, |
🤖 Prompt for AI Agents
In wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsScreen.kt
around lines 163 to 167, the code formats Date(0) which renders as "Jan 1, 1970"
when lastSaveDate == 0L; change the conditional so the zero case returns the
"none" stringResource without calling dateFormat.format(Date(lastSaveDate)), and
only call dateFormat.format(Date(lastSaveDate)) when lastSaveDate != 0L; ensure
the stringResource variants match the existing resources (one with formatted
date param and one without) and remove the unused date formatting in the zero
branch.
| dashPayConfig.observe(DashPayConfig.TRANSACTION_METADATA_LAST_PAST_SAVE) | ||
| .flatMapLatest { startTimestamp -> | ||
| transactionMetadataDao.observeByTimestampRange(startTimestamp ?: 0, System.currentTimeMillis()) | ||
| .flatMapConcat { | ||
| _oldUnsavedTransactions | ||
| } | ||
| .flatMapConcat { | ||
| transactionMetadataChangeCacheDao.observeCachedItemsBefore(System.currentTimeMillis()) | ||
| .map { | ||
| it.map { | ||
| // use default values, just need a count of items | ||
| item -> TransactionMetadata( | ||
| item.txId, | ||
| item.sentTimestamp ?: 0, | ||
| Coin.ZERO, | ||
| TransactionCategory.Sent | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| .onEach { | ||
| // are there older transactions not in the database? | ||
| _hasPastTransactionsToSave.value = it.isNotEmpty() | ||
| unsavedTxCount = it.size | ||
| log.info("unsaved count: ${it.size}") | ||
| } | ||
| .launchIn(viewModelWorkerScope) |
There was a problem hiding this comment.
Flow chain drops DAO results. The nested flatMapConcat chain discards both the TransactionMetadataDao.observeByTimestampRange() output and _oldUnsavedTransactions; only the cache flow reaches onEach. As a result _hasPastTransactionsToSave/unsavedTxCount ignore actual unsaved metadata from the DAO, giving false negatives. Please restructure this section (e.g., combine the DAO, _oldUnsavedTransactions, and cache flows explicitly) so all sources contribute to the final list.
🤖 Prompt for AI Agents
In
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsViewModel.kt
around lines 166 to 193, the nested flatMapConcat chain discards earlier DAO and
_oldUnsavedTransactions results so only the cache flow reaches onEach; fix by
explicitly combining the three sources instead of chaining them: obtain the
three flows (transactionMetadataDao.observeByTimestampRange(...),
_oldUnsavedTransactions, and
transactionMetadataChangeCacheDao.observeCachedItemsBefore(...).map{...}) and
then merge or combine them into a single Flow<List<TransactionMetadata>> (e.g.,
map each source to a List<TransactionMetadata> and use combine { a,b,c -> a + b
+ c } or flatten with merge and collect into a list), then feed that combined
flow to onEach so _hasPastTransactionsToSave and unsavedTxCount reflect the
union of all sources; preserve existing null/startTimestamp handling and launch
the resulting flow in viewModelWorkerScope.
| fun hasPastTransactionsToSave(): Boolean { | ||
| // TODO: optimize this | ||
|
|
||
| return false | ||
| } |
There was a problem hiding this comment.
Method always returns false. hasPastTransactionsToSave() is wired to surface whether unsaved metadata exists, yet it currently just returns false, so any caller gets the wrong answer. Return the tracked state (or recompute from current data).
- return false
+ return _hasPastTransactionsToSave.value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun hasPastTransactionsToSave(): Boolean { | |
| // TODO: optimize this | |
| return false | |
| } | |
| fun hasPastTransactionsToSave(): Boolean { | |
| // TODO: optimize this | |
| return _hasPastTransactionsToSave.value | |
| } |
🤖 Prompt for AI Agents
In
wallet/src/de/schildbach/wallet/ui/more/TransactionMetadataSettingsViewModel.kt
around lines 289-293, the hasPastTransactionsToSave() method incorrectly always
returns false; update it to return the actual tracked state or compute from
current data (e.g., return the backing Boolean/flow/state property that tracks
unsaved metadata or evaluate the collection of transactions/metadata to
determine if any unsaved entries exist) so callers receive the correct boolean
indicating whether there are past transactions to save.
| setupTransactionMetadataObservers() | ||
|
|
There was a problem hiding this comment.
Move transaction-metadata observers to onViewCreated.
Calling setupTransactionMetadataObservers() inside onCreateView() hits viewLifecycleOwner before the Fragment actually has a view, which triggers IllegalStateException. Please move the call to onViewCreated() (or gate it behind viewLifecycleOwnerLiveData) so the lifecycle owner is available.
@@
- setupTransactionMetadataObservers()
-
return ComposeView(requireContext()).apply {
@@
}
}
}
+
+ override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
+ super.onViewCreated(view, savedInstanceState)
+ setupTransactionMetadataObservers()
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setupTransactionMetadataObservers() | |
| override fun onCreateView( | |
| inflater: LayoutInflater, | |
| container: ViewGroup?, | |
| savedInstanceState: Bundle? | |
| ): View { | |
| return ComposeView(requireContext()).apply { | |
| setBackgroundColor(ContextCompat.getColor(requireContext(), android.R.color.white)) | |
| setContent { | |
| // ... compose content | |
| } | |
| } | |
| } | |
| override fun onViewCreated(view: View, savedInstanceState: Bundle?) { | |
| super.onViewCreated(view, savedInstanceState) | |
| setupTransactionMetadataObservers() | |
| } |
🤖 Prompt for AI Agents
In wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt around lines 81-82,
move the call to setupTransactionMetadataObservers() out of onCreateView and
into onViewCreated (or alternatively register it only after viewLifecycleOwner
is available via viewLifecycleOwnerLiveData) because calling it in onCreateView
accesses viewLifecycleOwner too early and causes IllegalStateException; update
the fragment to invoke setupTransactionMetadataObservers() inside onViewCreated
with the provided viewLifecycleOwner, or wrap the observer setup in
viewLifecycleOwnerLiveData.observe to defer until the lifecycle owner exists.
| val progress = it.data?.progress?.let { data -> BaseWorker.extractProgress(data) } ?: 0 | ||
| setTransactionMetadataText(progress != 100 && progress != -1, progress) | ||
| } |
There was a problem hiding this comment.
Default to “no progress” instead of 0%.
When observePublishOperation() yields no progress Data, the ?: 0 fallback keeps progress = 0, so the subtitle sticks on “Saving to network (0%)”. Defaulting to -1 preserves the sentinel semantics you added in BaseWorker.extractProgress and lets the UI fall back to the last-saved copy until real progress arrives.
- val progress = it.data?.progress?.let { data -> BaseWorker.extractProgress(data) } ?: 0
+ val progress = it.data?.progress?.let { data -> BaseWorker.extractProgress(data) } ?: -1
setTransactionMetadataText(progress != 100 && progress != -1, progress)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val progress = it.data?.progress?.let { data -> BaseWorker.extractProgress(data) } ?: 0 | |
| setTransactionMetadataText(progress != 100 && progress != -1, progress) | |
| } | |
| val progress = it.data?.progress?.let { data -> BaseWorker.extractProgress(data) } ?: -1 | |
| setTransactionMetadataText(progress != 100 && progress != -1, progress) | |
| } |
🤖 Prompt for AI Agents
In wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt around lines 152 to
154, the progress fallback uses ?: 0 which forces progress to 0 when no Data is
present; change the fallback to ?: -1 so the sentinel value from
BaseWorker.extractProgress is preserved and the UI will treat "no progress"
correctly (e.g., keep last-saved subtitle) and then call
setTransactionMetadataText(progress != 100 && progress != -1, progress) as
before.
Issue being fixed or feature implemented
Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Chores