feat(bond): Phase 3.5 payout confirmation actions on mobile#603
Conversation
- Add and to the Action enum - Add helper (none/pending/acknowledged/completed) with retry support - Bond payout screen: hide form on acknowledged/completed, show info + single CLOSE button - My Trades badge: PAYOUT IN PROGRESS on acknowledged; order's real status on completed - Trade Details: extend the previous non-bond message with a bond-paid line; rate hint only if is in history - : accept default to - : keep tracked order untouched on bond acks; preserve current status (don't fall to default) - Notification extractor: return null for the two new actions (no SnackBar fires) - Exhaustive switches in notification mapper/item updated as no-ops (compiler requirement; functional wiring deferred) - l10n: new keys in en/es/it/de/fr - Tests: bond payout helpers 9→17 cases; regression test for
WalkthroughThis PR implements support for two new bond-related actions— ChangesBond Payout Acknowledgement & Completion
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@lib/features/notifications/utils/notification_message_mapper.dart`:
- Around line 98-100: Replace the "TODO: implement title key if needed"
placeholder returned for actions mostro.Action.bondInvoiceAccepted and
mostro.Action.bondPayoutCompleted with a real localized title lookup (e.g., use
the appropriate S.of(context).<orderUpdateTitle> or similar ARB key) inside the
notification message mapper in notification_message_mapper.dart; update the same
placeholder occurrences referenced around lines 204-206 as well, ensuring you
call the correct S.of(context).<key> accessor used elsewhere in this mapper so
no raw string falls back to the user.
In `@lib/l10n/intl_fr.arb`:
- Around line 1591-1595: The new French ARB entries (keys
bondPayoutInProgressBadge, bondInvoiceAcceptedMessage,
bondPayoutCompletedMessage, bondPayoutCompletedWithRating,
bondPayoutCloseButton) currently contain French text but should follow the repo
convention of using English placeholder text for new feature keys; update each
value to an English placeholder equivalent (e.g., "PAYMENT IN PROGRESS", "Mostro
accepted the Lightning invoice you provided to receive your share of the
anti-abuse bond. Please wait for payment to complete.", "You have received
payment of your share of the anti-abuse bond.", "You have received payment of
your share of the anti-abuse bond. You may rate your counterparty.", "CLOSE") so
translations can be added in a follow-up localization PR.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d945bb9e-5423-4a91-8842-b2bb702fd632
📒 Files selected for processing (17)
lib/data/models/enums/action.dartlib/data/models/order.dartlib/features/notifications/utils/notification_data_extractor.dartlib/features/notifications/utils/notification_message_mapper.dartlib/features/notifications/widgets/notification_item.dartlib/features/order/models/order_state.dartlib/features/order/screens/bond_payout_invoice_screen.dartlib/features/trades/widgets/mostro_message_detail_widget.dartlib/features/trades/widgets/trades_list_item.dartlib/l10n/intl_de.arblib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_fr.arblib/l10n/intl_it.arblib/shared/utils/bond_payout_helpers.darttest/models/order_test.darttest/shared/utils/bond_payout_helpers_test.dart
| case mostro.Action.bondInvoiceAccepted: | ||
| case mostro.Action.bondPayoutCompleted: | ||
| return 'TODO: implement title key if needed'; |
There was a problem hiding this comment.
Replace TODO placeholder keys with real localization keys.
Returning "TODO: ..." here can leak raw placeholder text to users via mapper fallback. Use a valid localized fallback key (e.g., order update) instead.
Suggested fix
case mostro.Action.restore:
case mostro.Action.orders:
case mostro.Action.lastTradeIndex:
case mostro.Action.addBondInvoice:
case mostro.Action.bondInvoiceAccepted:
case mostro.Action.bondPayoutCompleted:
- return 'TODO: implement title key if needed';
+ return 'notification_order_update_title';
@@
case mostro.Action.restore:
case mostro.Action.orders:
case mostro.Action.lastTradeIndex:
case mostro.Action.addBondInvoice:
case mostro.Action.bondInvoiceAccepted:
case mostro.Action.bondPayoutCompleted:
- return 'TODO: implement message key if needed';
+ return 'notification_order_update_message';As per coding guidelines: "Localize all user-facing strings via ARB files and access them with S.of(context) rather than hard-coded literals".
Also applies to: 204-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 `@lib/features/notifications/utils/notification_message_mapper.dart` around
lines 98 - 100, Replace the "TODO: implement title key if needed" placeholder
returned for actions mostro.Action.bondInvoiceAccepted and
mostro.Action.bondPayoutCompleted with a real localized title lookup (e.g., use
the appropriate S.of(context).<orderUpdateTitle> or similar ARB key) inside the
notification message mapper in notification_message_mapper.dart; update the same
placeholder occurrences referenced around lines 204-206 as well, ensuring you
call the correct S.of(context).<key> accessor used elsewhere in this mapper so
no raw string falls back to the user.
There was a problem hiding this comment.
Not blocking. getTitleKey is unreachable for these actions because notification_data_extractor.dart (this PR) returns null for both, which short-circuits both foreground SnackBar and background push paths before any title lokup happens. Same pattern as addBondInvoice (merged in PR #596). The TODO string is dead code; wiring real notification text is explicitly deferred to a follow-up PR.
There was a problem hiding this comment.
@Catrya, thanks for the clear explanation! That makes sense — since notification_data_extractor.dart already returns null for both bondInvoiceAccepted and bondPayoutCompleted, these switch branches in the mapper are indeed unreachable dead code for all notification paths (foreground SnackBar and background push). The TODO strings won't surface to users. Acknowledged, and the pattern is consistent with addBondInvoice from #596.
Would you like me to open a follow-up issue to track wiring the real notification title/message keys for these two actions when the time comes?
✏️ Learnings added
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 603
File: lib/features/notifications/utils/notification_message_mapper.dart:98-100
Timestamp: 2026-05-22T23:04:42.075Z
Learning: In MostroP2P/mobile, `notification_data_extractor.dart` returns `null` for `bondInvoiceAccepted` and `bondPayoutCompleted` actions (same as `addBondInvoice`), which short-circuits both foreground SnackBar and background push notification paths before any title/message key lookup in `notification_message_mapper.dart`. The `TODO: implement title/message key if needed` placeholder strings in the mapper are intentional dead code — real notification text for these bond payout actions is explicitly deferred to a future follow-up PR.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: mostronator
Repo: MostroP2P/mobile PR: 463
File: lib/features/wallet/widgets/wallet_status_card.dart:100-105
Timestamp: 2026-02-14T01:35:16.623Z
Learning: Do not localize the term 'sats' (satoshis) in the Mostro mobile app. Treat 'sats' as a universal Bitcoin unit and keep it as hardcoded text in Dart UI code (e.g., wallet_status_card.dart) where appropriate for consistency. Localization keys may exist in .arb files, but avoid replacing 'sats' with localized strings in UI labels or values. If you need to display different units, prefer explicit conversion or a configurable unit setting rather than localizing the 'sats' string itself.
| "bondPayoutInProgressBadge": "PAIEMENT EN COURS", | ||
| "bondInvoiceAcceptedMessage": "Mostro a accepté la facture Lightning que vous avez fournie pour recevoir votre part du dépôt anti-abus. Veuillez patienter jusqu'à ce que le paiement soit terminé.", | ||
| "bondPayoutCompletedMessage": "Vous avez reçu le paiement de votre part du dépôt anti-abus.", | ||
| "bondPayoutCompletedWithRating": "Vous avez reçu le paiement de votre part du dépôt anti-abus. Vous pouvez évaluer votre contrepartie.", | ||
| "bondPayoutCloseButton": "FERMER", |
There was a problem hiding this comment.
Use English placeholder text for new fr.arb feature keys (per repo convention).
These newly added values are fully translated to French, but this repo’s convention for new features is to keep fr.arb placeholder text in English and translate in a follow-up localization PR.
Based on learnings: “for new features, use English placeholder text in de.arb and fr.arb, with full translations handled in a follow-up PR.”
🤖 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 `@lib/l10n/intl_fr.arb` around lines 1591 - 1595, The new French ARB entries
(keys bondPayoutInProgressBadge, bondInvoiceAcceptedMessage,
bondPayoutCompletedMessage, bondPayoutCompletedWithRating,
bondPayoutCloseButton) currently contain French text but should follow the repo
convention of using English placeholder text for new feature keys; update each
value to an English placeholder equivalent (e.g., "PAYMENT IN PROGRESS", "Mostro
accepted the Lightning invoice you provided to receive your share of the
anti-abuse bond. Please wait for payment to complete.", "You have received
payment of your share of the anti-abuse bond.", "You have received payment of
your share of the anti-abuse bond. You may rate your counterparty.", "CLOSE") so
translations can be added in a follow-up localization PR.
There was a problem hiding this comment.
Not blocking. The convention cited doesn't match repo practice: PR #596 (immediate Phase 3 precedent, same author, same feature family) added 11 bond keys with full fr/de translations. CLAUDE.md guidance is "Always use localized strings" and CI accepts populated values. If we want to switch to placeholder-only fr/de for new features, that's a policy change to discuss separately.
There was a problem hiding this comment.
@Catrya, that's a fair and well-evidenced correction — thank you. The review comment was based on an inaccurate learning, and PR #596's precedent (same author, same feature family, full fr/de translations) clearly shows that fully-translated values in fr.arb and de.arb are the established practice here. I'll update the long-term memory accordingly.
(｡•́︿•̀｡)
✏️ Learnings added
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 603
File: lib/l10n/intl_fr.arb:1591-1595
Timestamp: 2026-05-22T23:05:28.893Z
Learning: In MostroP2P/mobile, Flutter localization ARB files under lib/l10n (e.g., intl_fr.arb, intl_de.arb) should contain fully translated values for new feature keys at the time the feature is added. CLAUDE.md guidance is "Always use localized strings." Do NOT flag fully translated fr/de values as non-conventional — that matches established repo practice (see PR `#596` and PR `#603` as precedents for the bond payout feature family). Placeholder metadata blocks (e.g., key blocks) are not required unless an explicit type or description is needed.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 592
File: lib/l10n/intl_fr.arb:709-709
Timestamp: 2026-05-14T19:41:20.593Z
Learning: For MostroP2P/mobile Flutter localization ARB files under lib/l10n, do not require key metadata blocks for placeholder keys. flutter gen-l10n will generate valid Dart code (e.g., inferred parameter types like `String forAmount(Object amount)`) even when key metadata is omitted. Only require metadata blocks when an explicit type and/or description is needed. Treat “normalizing all placeholder keys to include metadata” as a separate, non-blocking concern. Also, follow the project convention: for new features, use English placeholder text in de.arb and fr.arb, with full translations handled in a follow-up PR.
🧠 Learnings used
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 592
File: lib/l10n/intl_fr.arb:709-709
Timestamp: 2026-05-14T19:41:20.593Z
Learning: For MostroP2P/mobile Flutter localization ARB files under lib/l10n, do not require key metadata blocks for placeholder keys. flutter gen-l10n will generate valid Dart code (e.g., inferred parameter types like `String forAmount(Object amount)`) even when key metadata is omitted. Only require metadata blocks when an explicit type and/or description is needed. Treat “normalizing all placeholder keys to include metadata” as a separate, non-blocking concern. Also, follow the project convention: for new features, use English placeholder text in de.arb and fr.arb, with full translations handled in a follow-up PR.
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests