Dispute creation from order details in Active orders #289
Conversation
WalkthroughImplements a dependency-injected DisputeRepository (requires NostrService, mostro pubkey, Ref) with createDispute(orderId) that reads a session, builds a MostroMessage (action: dispute), gift-wraps it using NIP‑59 with the session.tradeKey and recipient pubkey, and publishes via NostrService; adds disputeRepositoryProvider, updates TradeDetailScreen to call it and show localized SnackBars, and updates i18n keys. DisputeService remains mocked. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant T as TradeDetailScreen
participant P as disputeRepositoryProvider
participant R as DisputeRepository
participant S as sessionNotifierProvider
participant N as NostrService
Note over U,T: User confirms "Open Dispute"
U->>T: Confirm dispute
T->>P: ref.read(disputeRepositoryProvider)
P-->>T: DisputeRepository instance
T->>R: createDispute(orderId)
R->>S: read session by orderId
alt session found
S-->>R: Session (includes tradeKey)
R->>R: build MostroMessage {action: dispute, id: orderId}
R->>R: gift-wrap via NIP-59 with tradeKey & recipient pubkey
R->>N: publish(wrappedEvent)
N-->>R: publish result
R-->>T: true
T-->>U: show success SnackBar
else session missing / error
R-->>T: false / throws
T-->>U: show failure / error SnackBar
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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: 3
🧹 Nitpick comments (9)
lib/services/dispute_service.dart (2)
10-29: Stabilize mock data for predictabilityConsider extracting status/action strings to consts or an enum to avoid typos and keep them consistent with the rest of the app.
46-47: Surface outcome from sendDisputeMessageReturning void and swallowing errors makes UI handling harder. Consider returning a bool or throwing on failure (even in mock) to match repository patterns.
lib/l10n/intl_en.arb (2)
109-113: Consistent casing and placeholders for new dispute messages
- Prefer “order ID”, not “order Id”.
- Add placeholder metadata for id and user_token so tooling generates correct APIs.
- "disputeInitiatedByYou": "You have initiated a dispute for order Id: {id}. A solver will be assigned soon. Once assigned, I will share their npub with you, and only they will be able to assist you. Your dispute ID is: {user_token}.", - "disputeInitiatedByPeer": "Your counterparty has initiated a dispute for order Id: {id}. A solver will be assigned soon. Once assigned, I will share their npub with you, and only they will be able to assist you. Your dispute ID is: {user_token}.", + "disputeInitiatedByYou": "You have initiated a dispute for order ID: {id}. A solver will be assigned soon. Once assigned, I will share their npub with you, and only they will be able to assist you. Your dispute ID is: {user_token}.", + "@disputeInitiatedByYou": { + "placeholders": { + "id": { "type": "String" }, + "user_token": { "type": "String" } + } + }, + "disputeInitiatedByPeer": "Your counterparty has initiated a dispute for order ID: {id}. A solver will be assigned soon. Once assigned, I will share their npub with you, and only they will be able to assist you. Your dispute ID is: {user_token}.", + "@disputeInitiatedByPeer": { + "placeholders": { + "id": { "type": "String" }, + "user_token": { "type": "String" } + } + },
112-112: Add placeholders for adminTookDisputeUsersadmin_npub is used but not declared.
"adminTookDisputeUsers": "The solver {admin_npub} will handle your dispute. You can contact them directly, but if they reach out to you first, make sure to ask them for your dispute ID.", + "@adminTookDisputeUsers": { + "placeholders": { + "admin_npub": { "type": "String" } + } + },lib/l10n/intl_it.arb (1)
109-113: Fix punctuation and add placeholders
- Remove extra period in adminTookDisputeUsers.
- Add placeholder metadata for id, user_token, and admin_npub.
- "disputeInitiatedByYou": "Hai iniziato una disputa per l'ordine ID: {id}. ... L'ID di questa disputa è: {user_token}.", + "disputeInitiatedByYou": "Hai iniziato una disputa per l'ordine ID: {id}. ... L'ID di questa disputa è: {user_token}.", + "@disputeInitiatedByYou": { + "placeholders": { + "id": { "type": "String" }, + "user_token": { "type": "String" } + } + }, - "disputeInitiatedByPeer": "La tua controparte ha iniziato una disputa per l'ordine ID: {id}. ... L'ID di questa disputa è: {user_token}.", + "disputeInitiatedByPeer": "La tua controparte ha iniziato una disputa per l'ordine ID: {id}. ... L'ID di questa disputa è: {user_token}.", + "@disputeInitiatedByPeer": { + "placeholders": { + "id": { "type": "String" }, + "user_token": { "type": "String" } + } + }, - "adminTookDisputeUsers": "L'amministratore {admin_npub} gestirà la tua disputa. Devi contattare l'amministratore direttamente, ma se qualcuno ti contatta prima, assicurati di chiedergli di fornirti l'ID per la tua disputa..", + "adminTookDisputeUsers": "L'amministratore {admin_npub} gestirà la tua disputa. Devi contattare l'amministratore direttamente, ma se qualcuno ti contatta prima, assicurati di chiedergli di fornirti l'ID per la tua disputa.", + "@adminTookDisputeUsers": { + "placeholders": { + "admin_npub": { "type": "String" } + } + },lib/l10n/intl_es.arb (1)
109-113: Casing and placeholders for dispute messages (ES)
- Use “orden ID”, not “orden Id”.
- Add placeholder metadata for id, user_token, and admin_npub.
- "disputeInitiatedByYou": "Has iniciado una disputa para la orden Id: {id}. ... Tu ID de disputa es: {user_token}.", + "disputeInitiatedByYou": "Has iniciado una disputa para la orden ID: {id}. ... Tu ID de disputa es: {user_token}.", + "@disputeInitiatedByYou": { + "placeholders": { + "id": { "type": "String" }, + "user_token": { "type": "String" } + } + }, - "disputeInitiatedByPeer": "Tu contraparte ha iniciado una disputa para la orden Id: {id}. ... Tu ID de disputa es: {user_token}.", + "disputeInitiatedByPeer": "Tu contraparte ha iniciado una disputa para la orden ID: {id}. ... Tu ID de disputa es: {user_token}.", + "@disputeInitiatedByPeer": { + "placeholders": { + "id": { "type": "String" }, + "user_token": { "type": "String" } + } + }, "adminTookDisputeUsers": "El resolutor {admin_npub} manejará tu disputa. Puedes contactarlos directamente, pero si te contactan primero, asegúrate de pedirles tu ID de disputa.", + "@adminTookDisputeUsers": { + "placeholders": { + "admin_npub": { "type": "String" } + } + },lib/features/disputes/providers/dispute_providers.dart (1)
10-17: Harden repository provider (null/empty pubkey, selective watch)
- Guard against missing Mostro pubkey to fail fast.
- Watch only the needed field to avoid unnecessary rebuilds.
-final disputeRepositoryProvider = Provider.autoDispose<DisputeRepository>((ref) { - final nostrService = ref.watch(nostrServiceProvider); - final settings = ref.watch(settingsProvider); - final mostroPubkey = settings.mostroPublicKey; - - return DisputeRepository(nostrService, mostroPubkey, ref); -}); +final disputeRepositoryProvider = Provider.autoDispose<DisputeRepository>((ref) { + final nostrService = ref.watch(nostrServiceProvider); + final mostroPubkey = ref.watch(settingsProvider.select((s) => s.mostroPublicKey)) ?? ''; + if (mostroPubkey.isEmpty) { + throw StateError('Mostro pubkey not configured'); + } + return DisputeRepository(nostrService, mostroPubkey, ref); +});lib/features/trades/screens/trade_detail_screen.dart (1)
650-752: Prevent duplicate dispute sends; consider using MostroReactiveButton with controller or temporary disabling.Right now the ElevatedButton can be tapped again after the dialog. Using the existing MostroReactiveButton pattern (as with other actions) would give built-in loading/disable and success indicator. If you prefer to keep ElevatedButton, set a local loading flag via a StateProvider scoped to this widget.
lib/data/repositories/dispute_repository.dart (1)
19-57: Validate trade key and recipient pubkey before wrapping
- Verify
_mostroPubkeyis non‐empty- Verify
session.tradeKey.publicandsession.tradeKey.privateare non‐empty
Log a warning and returnfalseif any prerequisite is missing to preventwrap()from throwing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
lib/data/repositories/dispute_repository.dart(1 hunks)lib/features/disputes/providers/dispute_providers.dart(1 hunks)lib/features/trades/screens/trade_detail_screen.dart(2 hunks)lib/l10n/intl_en.arb(2 hunks)lib/l10n/intl_es.arb(2 hunks)lib/l10n/intl_it.arb(2 hunks)lib/services/dispute_service.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
{lib/*.dart,lib/!(generated)/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart}: Use localized strings; replace hardcoded user-facing text withS.of(context).keyName
Preferconstconstructors andconstwhere possible
Use latest Flutter/Dart APIs (e.g.,withValues()instead of deprecatedwithOpacity())
Files:
lib/features/disputes/providers/dispute_providers.dartlib/services/dispute_service.dartlib/features/trades/screens/trade_detail_screen.dartlib/data/repositories/dispute_repository.dart
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}: Checkmountedbefore usingBuildContextafterawait(async gaps)
Remove unused imports and dependencies
Files:
lib/features/disputes/providers/dispute_providers.dartlib/services/dispute_service.dartlib/features/trades/screens/trade_detail_screen.dartlib/data/repositories/dispute_repository.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
All code comments, variable names, and function names must be in English
Files:
lib/features/disputes/providers/dispute_providers.dartlib/services/dispute_service.dartlib/features/trades/screens/trade_detail_screen.dartlib/data/repositories/dispute_repository.dart
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Add new localization keys to all three ARB files (en, es, it)
Files:
lib/l10n/intl_en.arblib/l10n/intl_it.arblib/l10n/intl_es.arb
🧠 Learnings (3)
📚 Learning: 2025-08-28T20:32:34.625Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T20:32:34.625Z
Learning: Use Riverpod for state management with Notifiers encapsulating business logic; access data only through repositories
Applied to files:
lib/features/disputes/providers/dispute_providers.dart
📚 Learning: 2025-08-28T20:32:34.625Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T20:32:34.625Z
Learning: Applies to test/mocks.mocks.dart : Do not manually modify `test/mocks.mocks.dart` (auto-generated by Mockito)
Applied to files:
lib/services/dispute_service.dart
📚 Learning: 2025-08-28T20:32:34.625Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T20:32:34.625Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (en, es, it)
Applied to files:
lib/l10n/intl_it.arblib/l10n/intl_es.arb
⏰ 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 (6)
lib/l10n/intl_en.arb (1)
270-279: LGTM: new dispute creation stringsKeys and placeholder metadata look correct and match usage.
lib/l10n/intl_it.arb (1)
652-661: LGTM: new dispute creation stringsKeys and metadata align with EN/ES.
lib/l10n/intl_es.arb (1)
611-620: LGTM: new dispute creation stringsKeys and metadata look good and consistent.
lib/features/disputes/providers/dispute_providers.dart (1)
7-9: Import paths verified and correct
Provider files are located at lib/shared/providers/nostr_service_provider.dart and lib/features/settings/settings_provider.dart, matching the imports. No changes needed.lib/features/trades/screens/trade_detail_screen.dart (1)
26-26: Import looks good.Provider-based wiring is consistent with the rest of the screen.
lib/data/repositories/dispute_repository.dart (1)
12-18: Constructor DI is clean.Good move away from a singleton; easier to test and mock.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
lib/l10n/intl_en.arb (5)
309-309: Deduplicate conflicting keys: “createdOn” defined twice.“createdOn” appears both as "Created on" and "Created on:". Duplicate keys in ARB are error-prone; last one silently overrides the first.
Apply one of:
- Rename the chat label to a unique key (e.g., createdOnWithColon) and update call sites.
- Or standardize on one value and remove the duplicate key.
Verification helper:
#!/bin/bash rg -nP 'AppLocalizations\.of\(.+?\)\.createdOn\b' -C2Also applies to: 696-696
215-215: Duplicate key “retry”.Key is declared twice with identical value. Keep one.
- "retry": "Retry", + "retry": "Retry", @@ - "retry": "Retry",Also applies to: 875-875
370-370: Duplicate key “proofOfWork”.Declared twice. Prefer a single shared key or differentiate (e.g., proofOfWorkLabel).
- "proofOfWork": "Proof of Work", + "proofOfWork": "Proof of Work", @@ - "proofOfWork": "Proof of Work", + "proofOfWorkLabel": "Proof of Work",Note: update references if you rename.
Also applies to: 767-767
259-259: Duplicate key “orderIdLabel”.Declared twice. Remove one or rename to avoid silent overrides.
- "orderIdLabel": "Order ID", + "orderIdLabel": "Order ID", @@ - "orderIdLabel": "Order ID",Also applies to: 885-885
1-1085: Remove duplicated ARB keys and enforce ARB integrity checks
- In lib/l10n/intl_en.arb, consolidate or remove the duplicate top-level keys:
createdOn,retry,orderIdLabel, andproofOfWork(each appears twice).- Add a CI lint step that fails on duplicate top-level keys and on key/placeholder mismatches across intl_{en,es,it}.arb to prevent regressions.
♻️ Duplicate comments (1)
lib/l10n/intl_en.arb (1)
855-858: Remove token-based strings to match new dispute flow.Token-based UX is deprecated; these strings contradict the updated “dispute ID” flow and previous reviewer guidance.
Apply (if unused):
- "tokenVerified": "Token verified", - "awaitingTokenVerification": "Awaiting token verification", - "askAdminQuoteToken": "Ask admin to quote this token:",Verify usage before removal:
#!/bin/bash rg -nP '\b(tokenVerified|awaitingTokenVerification|askAdminQuoteToken)\b' -C2
🧹 Nitpick comments (1)
lib/l10n/intl_en.arb (1)
112-112: Add metadata for admin_npub placeholder.Declare placeholder to keep parity with es/it and avoid generator warnings.
- "adminTookDisputeUsers": "The solver {admin_npub} will handle your dispute. You can contact them directly.", + "adminTookDisputeUsers": "The solver {admin_npub} will handle your dispute. You can contact them directly.", + "@adminTookDisputeUsers": { + "placeholders": { + "admin_npub": { "type": "String" } + } + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
lib/l10n/intl_en.arb(2 hunks)lib/l10n/intl_es.arb(2 hunks)lib/l10n/intl_it.arb(2 hunks)lib/services/dispute_service.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/services/dispute_service.dart
- lib/l10n/intl_it.arb
- lib/l10n/intl_es.arb
🧰 Additional context used
📓 Path-based instructions (1)
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Add new localization keys to all three ARB files (en, es, it)
Files:
lib/l10n/intl_en.arb
🧠 Learnings (1)
📚 Learning: 2025-08-28T20:32:34.625Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T20:32:34.625Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (en, es, it)
Applied to files:
lib/l10n/intl_en.arb
⏰ 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 (1)
lib/l10n/intl_en.arb (1)
270-279: Cross-locale parity confirmed. All six keys and their placeholders are present and identical inintl_es.arbandintl_it.arb.
…glish translations
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lib/l10n/intl_en.arb (2)
109-121: Casing fixed + placeholders added — LGTM“order ID” casing is consistent and the placeholder metadata for id and dispute_id is correctly declared with examples.
122-134: Peer message mirrors ‘you’ message correctly — LGTMText, casing, and placeholder metadata match the “you” variant; no issues.
🧹 Nitpick comments (1)
lib/l10n/intl_en.arb (1)
136-136: Add placeholder metadata for admin_npubThis string uses {admin_npub} but lacks the @metadata block. Add it for tooling parity and stronger type hints.
Apply:
"adminTookDisputeUsers": "The solver {admin_npub} will handle your dispute. You can contact them directly.", + "@adminTookDisputeUsers": { + "placeholders": { + "admin_npub": { + "type": "String", + "description": "Solver's npub" + } + } + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/l10n/intl_en.arb(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Add new localization keys to all three ARB files (en, es, it)
Files:
lib/l10n/intl_en.arb
⏰ 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
| "disputeCreatedSuccessfully": "Dispute created successfully", | ||
| "disputeCreationFailed": "Failed to create dispute", | ||
| "disputeCreationErrorWithMessage": "Error creating dispute: {error}", | ||
| "@disputeCreationErrorWithMessage": { | ||
| "placeholders": { | ||
| "error": { | ||
| "type": "String" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
New dispute-creation snackbar keys — LGTM; ensure locale parity
Strings read well and the error placeholder is typed. Per guidelines, confirm these keys exist in intl_es.arb and intl_it.arb.
Run to verify presence across locales and metadata health:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(lib/l10n/intl_en.arb lib/l10n/intl_es.arb lib/l10n/intl_it.arb)
for f in "${files[@]}"; do
test -f "$f" || { echo "Missing $f"; exit 1; }
done
keys=('disputeCreatedSuccessfully' 'disputeCreationFailed' 'disputeCreationErrorWithMessage' 'disputeInitiatedByYou' 'disputeInitiatedByPeer' 'adminTookDisputeUsers')
for k in "${keys[@]}"; do
for f in "${files[@]}"; do
jq -e --arg k "$k" 'has($k)' "$f" >/dev/null || { echo "$f is missing key: $k"; exit 1; }
done
done
# EN-specific metadata checks
jq -e '.["@disputeCreationErrorWithMessage"].placeholders.error' lib/l10n/intl_en.arb >/dev/null || echo "Note: missing error placeholder metadata in en"
jq -e '.["@adminTookDisputeUsers"].placeholders.admin_npub' lib/l10n/intl_en.arb >/dev/null || echo "Note: add @adminTookDisputeUsers metadata with admin_npub placeholder in en"
for k in disputeInitiatedByYou disputeInitiatedByPeer; do
jq -e --arg k "$k" '.["@"+$k].placeholders.id and .["@"+$k].placeholders.dispute_id' lib/l10n/intl_en.arb >/dev/null \
|| echo "Note: ensure id and dispute_id placeholders are declared for $k in en"
done
echo "OK: localization keys present in en/es/it"Length of output: 3101
Add admin_npub placeholder metadata for adminTookDisputeUsers
In lib/l10n/intl_en.arb, append the following metadata block:
"@adminTookDisputeUsers": {
"placeholders": {
"admin_npub": { "type": "String" }
}
}🤖 Prompt for AI Agents
In lib/l10n/intl_en.arb around lines 294 to 303, the translation key
adminTookDisputeUsers is missing a placeholder metadata block for admin_npub;
add an entry "@adminTookDisputeUsers" with a placeholders object that defines
"admin_npub" of type "String" so the localization tooling recognizes the
placeholder and validates/generated code correctly.
…pub from user notifications
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
lib/data/repositories/dispute_repository.dart (1)
49-55: Add publish timeout and log stacktrace (matches earlier feedback).Prevent indefinite waits and capture stack traces for observability. Also keep the success log only when publish completes.
+import 'dart:async'; @@ - await _nostrService.publishEvent(event); + await _nostrService.publishEvent(event).timeout(const Duration(seconds: 15)); @@ - } catch (e) { - _logger.e('Failed to create dispute: $e'); + } catch (e, s) { + _logger.e('Failed to create dispute', e, s); return false; }
🧹 Nitpick comments (4)
lib/data/repositories/dispute_repository.dart (3)
30-35: Treat missing session as warn, not error (non-exceptional path).Log at warning level to reduce noise in error telemetry.
- _logger - .e('No session found for order: $orderId, cannot create dispute'); + _logger + .w('No session found for order: $orderId, cannot create dispute');
12-18: Inject Logger for testability.Allow passing a mock/spied logger in tests.
- final Logger _logger = Logger(); + final Logger _logger; - DisputeRepository(this._nostrService, this._mostroPubkey, this._ref); + DisputeRepository(this._nostrService, this._mostroPubkey, this._ref, {Logger? logger}) + : _logger = logger ?? Logger();
19-57: Consider returning an error payload (aligns with i18n key that shows error).UI can surface S.disputeCreationErrorWithMessage if repository returns/throws the reason. Right now errors are swallowed and only “false” is returned.
Option A: return a small result object:
class DisputeCreationResult { final bool ok; final String? error; const DisputeCreationResult(this.ok, [this.error]); }Option B: keep return type bool but rethrow after logging; let UI catch and display the error.
lib/l10n/intl_it.arb (1)
112-112: Singular agreement.“un risolutore … Ti contatterà” (singolare).
- "adminTookDisputeUsers": "È stato assegnato un risolutore di controversie per gestire il tuo caso. Ti contatteranno attraverso l'applicazione.", + "adminTookDisputeUsers": "È stato assegnato un risolutore di controversie per gestire il tuo caso. Ti contatterà tramite l'applicazione.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
lib/data/repositories/dispute_repository.dart(1 hunks)lib/l10n/intl_en.arb(2 hunks)lib/l10n/intl_es.arb(2 hunks)lib/l10n/intl_it.arb(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/l10n/intl_es.arb
- lib/l10n/intl_en.arb
🧰 Additional context used
📓 Path-based instructions (4)
{lib/*.dart,lib/!(generated)/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart}: Use localized strings; replace hardcoded user-facing text withS.of(context).keyName
Preferconstconstructors andconstwhere possible
Use latest Flutter/Dart APIs (e.g.,withValues()instead of deprecatedwithOpacity())
Files:
lib/data/repositories/dispute_repository.dart
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}: Checkmountedbefore usingBuildContextafterawait(async gaps)
Remove unused imports and dependencies
Files:
lib/data/repositories/dispute_repository.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
All code comments, variable names, and function names must be in English
Files:
lib/data/repositories/dispute_repository.dart
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Add new localization keys to all three ARB files (en, es, it)
Files:
lib/l10n/intl_it.arb
🧠 Learnings (1)
📚 Learning: 2025-08-28T20:32:34.625Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T20:32:34.625Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (en, es, it)
Applied to files:
lib/l10n/intl_it.arb
⏰ 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 (2)
lib/data/repositories/dispute_repository.dart (1)
24-29: sessionNotifierProvider returns List, so direct read is correct – it’s defined asStateNotifierProvider<SessionNotifier, List<Session>>, soref.read(sessionNotifierProvider)yields aList<Session>, makingfirstWhereOrNullvalid.Likely an incorrect or invalid review comment.
lib/l10n/intl_it.arb (1)
652-661: Verified parity: all three new keys and theirerrorplaceholders exist in intl_en.arb, intl_es.arb, and intl_it.arb.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
lib/features/trades/widgets/mostro_message_detail_widget.dart (5)
4-4: Remove unused imports
nostr_event.dartandmostro_instance.dartaren’t referenced here; keeping them may violate “remove unused imports” and fail lints.-import 'package:mostro_mobile/data/models/nostr_event.dart'; -import 'package:mostro_mobile/features/mostro/mostro_instance.dart';Also applies to: 9-9
24-27: Avoid duplicate provider watch for the same order stateYou watch
orderNotifierProvider(orderId)inbuildand again inside_getActionText. Pass the state down to avoid extra rebuild triggers.@@ - final actionText = _getActionText( - context, - ref, - ); + final actionText = _getActionText( + context, + ref, + orderState, + ); @@ - String _getActionText( - BuildContext context, - WidgetRef ref, - ) { - final tradeState = ref.watch(orderNotifierProvider(orderId)); + String _getActionText( + BuildContext context, + WidgetRef ref, + OrderState tradeState, + ) {Also applies to: 56-63
157-159: Localize the fallback “No payment method”Hardcoded user-facing text violates the localization guideline. Replace with a localized string.
- orderPayload != null - ? orderPayload.paymentMethod - : 'No payment method', + orderPayload != null + ? orderPayload.paymentMethod + : S.of(context)!.noPaymentMethod, // add key in l10n if missing
237-237: Fallback message should be localized or debug-onlyReturning a raw
'No message found for action …'leaks dev text to users. Localize it or gate behindassert/kDebugMode.
119-121: Avoid runtime newline fixes in i18n
.replaceAll('\\n', '\n')hints the ARB contains escaped newlines. Prefer storing real newlines in translations to remove these calls.Also applies to: 141-142
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/features/trades/widgets/mostro_message_detail_widget.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{lib/*.dart,lib/!(generated)/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart}: Use localized strings; replace hardcoded user-facing text withS.of(context).keyName
Preferconstconstructors andconstwhere possible
Use latest Flutter/Dart APIs (e.g.,withValues()instead of deprecatedwithOpacity())
Files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}: Checkmountedbefore usingBuildContextafterawait(async gaps)
Remove unused imports and dependencies
Files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
All code comments, variable names, and function names must be in English
Files:
lib/features/trades/widgets/mostro_message_detail_widget.dart
⏰ 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 (2)
lib/features/trades/widgets/mostro_message_detail_widget.dart (2)
216-216: Confirm i18n API change doesn’t drop the admin handle
adminTookDisputeUsersis now a no-arg getter. If the updated copy still includes a placeholder like{admin_npub}, users will see the literal text. Please confirm the EN/locale strings no longer require interpolation, or wire in the solver/admin handle if it must be shown.
253-254: Verify placeholders are replaced with actual values
outOfRangeFiatAmount('{fiat_min}', '{fiat_max}')passes literal placeholders. Confirm this is intentional and that real min/max are unavailable here; otherwise, pass computed values (and units) from the relevant source.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lib/data/repositories/dispute_repository.dart (2)
1-3: Importdart:asyncto enable publish timeout handling.Needed for
TimeoutExceptionand.timeout(...). This also aligns with prior feedback.+import 'dart:async'; import 'package:collection/collection.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:logger/logger.dart';
55-62: Add timeout and capture stack traces for observability.Avoid indefinite waits on network and log stacks for triage. Mirrors prior bot suggestion.
- await _nostrService.publishEvent(event); + await _nostrService + .publishEvent(event) + .timeout(const Duration(seconds: 15)); @@ - } catch (e) { - _logger.e('Failed to create dispute: $e'); + } on TimeoutException catch (e, s) { + _logger.e('Timed out creating dispute for order: $orderId', e, s); + return false; + } catch (e, s) { + _logger.e('Failed to create dispute for order: $orderId', e, s); return false; }
🧹 Nitpick comments (4)
lib/data/repositories/dispute_repository.dart (4)
12-18: Prefer DI for Logger to improve testability and control.Allow passing a mock/silent logger in tests or different log configs in prod.
- final Logger _logger = Logger(); + final Logger _logger; - DisputeRepository(this._nostrService, this._mostroPubkey, this._ref); + DisputeRepository( + this._nostrService, + this._mostroPubkey, + this._ref, { + Logger? logger, + }) : _logger = logger ?? Logger();Want me to update the provider call sites accordingly?
42-49: Deduplicate adjacent comments about NIP-59.Keep one concise comment to reduce noise.
- // Create dispute message using Gift Wrap protocol (NIP-59) final disputeMessage = MostroMessage( action: Action.dispute, id: orderId, ); - // Wrap message using Gift Wrap protocol (NIP-59) + // Wrap using Gift Wrap protocol (NIP-59)
12-17: Assert non-empty Mostro pubkey at construction.Fail fast if wiring is wrong; safer than discovering at send-time.
- DisputeRepository( + DisputeRepository( this._nostrService, this._mostroPubkey, this._ref, { Logger? logger, - }) : _logger = logger ?? Logger(); + }) : assert(_mostroPubkey.isNotEmpty, 'Mostro pubkey must not be empty'), + _logger = logger ?? Logger();
19-59: Consider idempotency and duplicate-send protection.If users tap twice or reconnect, you might double-send disputes. Consider tagging the event deterministically (e.g., content hash or a unique tag with
orderId) and/or gating on local state before sending; also surface server/Nostr ack when available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/data/repositories/dispute_repository.dart(1 hunks)lib/l10n/intl_it.arb(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/l10n/intl_it.arb
🧰 Additional context used
📓 Path-based instructions (3)
{lib/*.dart,lib/!(generated)/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart}: Use localized strings; replace hardcoded user-facing text withS.of(context).keyName
Preferconstconstructors andconstwhere possible
Use latest Flutter/Dart APIs (e.g.,withValues()instead of deprecatedwithOpacity())
Files:
lib/data/repositories/dispute_repository.dart
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}: Checkmountedbefore usingBuildContextafterawait(async gaps)
Remove unused imports and dependencies
Files:
lib/data/repositories/dispute_repository.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
All code comments, variable names, and function names must be in English
Files:
lib/data/repositories/dispute_repository.dart
🧠 Learnings (1)
📚 Learning: 2025-06-08T23:54:09.987Z
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Applied to files:
lib/data/repositories/dispute_repository.dart
⏰ 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 (2)
lib/data/repositories/dispute_repository.dart (2)
36-41: Good guard on missing trade key.This addresses earlier feedback to avoid wrapping with an empty key. LGTM.
42-53: Correct protocol naming (Gift Wrap, NIP-59).Thanks for fixing the NIP reference; avoids confusion. LGTM.
| /// Create a new dispute for an order | ||
| Future<bool> createDispute(String orderId) async { | ||
| try { | ||
| _logger.d('Creating dispute for order: $orderId'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate input: reject empty orderId upfront.
Prevents constructing/sending malformed events.
Future<bool> createDispute(String orderId) async {
try {
+ if (orderId.trim().isEmpty) {
+ _logger.e('Empty orderId, cannot create dispute');
+ return false;
+ }
_logger.d('Creating dispute for order: $orderId');📝 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.
| /// Create a new dispute for an order | |
| Future<bool> createDispute(String orderId) async { | |
| try { | |
| _logger.d('Creating dispute for order: $orderId'); | |
| /// Create a new dispute for an order | |
| Future<bool> createDispute(String orderId) async { | |
| try { | |
| if (orderId.trim().isEmpty) { | |
| _logger.e('Empty orderId, cannot create dispute'); | |
| return false; | |
| } | |
| _logger.d('Creating dispute for order: $orderId'); | |
| // ... rest of implementation ... | |
| } catch (e, s) { | |
| _logger.e('Failed to create dispute', e, s); | |
| return false; | |
| } | |
| } |
Summary
This PR adds dispute creation functionality for orders in the MostroP2P mobile app. Users can now initiate disputes directly from the order details screen when they encounter issues with a trade. The implementation includes proper state management, error handling, and user feedback through snackbar notifications. The dispute creation process is integrated with the existing Nostr messaging system and follows the application's security model.
Key Changes:
Summary by CodeRabbit
New Features
Refactor
Chores
Tests/Mocks