Add disputes #280
Add disputes #280
Conversation
…nd add i18n strings
…dispute creation logic
WalkthroughAdds a disputes feature: routing to a dispute details/chat screen, expanded dispute models and chat model, repository/service for dispute events and messaging via Mostro/Nostr, Riverpod providers/notifiers, multiple dispute UI widgets (list/detail/communication/input), i18n keys (EN/ES/IT), Mostro parsing hardening, logging dependency, and minor config/localization refactors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as DisputeChatScreen
participant Prov as dispute_providers
participant Repo as DisputeRepository
participant Nostr as NostrService
User->>UI: Open /dispute_details/:disputeId
UI->>Prov: disputeDetailsProvider(disputeId)
Prov->>Repo: getDisputeDetails(disputeId)
Repo->>Nostr: Query events (kind 38383) / storage
Nostr-->>Repo: Events / decrypted messages
Repo-->>Prov: Dispute
Prov-->>UI: AsyncValue<Dispute?>
UI->>Prov: disputeChatProvider(disputeId)
Prov->>Repo: subscribeToDisputeEvents()
Repo->>Nostr: Subscribe (kind 38383)
Nostr-->>Repo: Event stream
Repo-->>Prov: Stream<Dispute>
Prov-->>UI: Updates (admin assignment / token)
User->>UI: Send message
UI->>Prov: notifier.sendMessage(content)
Prov->>Nostr: Encrypted DM (NIP-17) to admin
Nostr-->>Prov: Ack / Error
Prov-->>UI: Optimistic update / rollback on error
sequenceDiagram
autonumber
participant DisputeSvc as DisputeService
participant Nostr as NostrService
participant OrderN as OrderNotifier
DisputeSvc->>Nostr: Subscribe kind 38383 (z=dispute)
Nostr-->>DisputeSvc: DisputeEvent
DisputeSvc->>OrderN: handleEvent(MostroMessage{action, payload: Dispute})
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
lib/features/chat/widgets/trade_information_tab.dart (1)
26-37: Don’t fabricate dates; surface “Unknown date” instead
_getOrderCreationDate()always returns a non-null DateTime (falls back to “now - 1 hour”), which makes theunknownDatebranch unreachable and displays a misleading date.Apply this to return null when unknown so the UI can show the localized “Unknown date”:
- DateTime _getOrderCreationDate() { + DateTime? _getOrderCreationDate() { if (order?.createdAt != null && order!.createdAt! > 0) { // Convert Unix timestamp to DateTime return DateTime.fromMillisecondsSinceEpoch(order!.createdAt! * 1000); } - - // Fallback: use current time minus a reasonable amount - // This is better than showing "Unknown date" - return DateTime.now().subtract(const Duration(hours: 1)); + // Unknown creation time + return null; }No call-site change needed since
_formatOrderDate(DateTime?)already handles null:- _formatOrderDate(_getOrderCreationDate(), context), + _formatOrderDate(_getOrderCreationDate(), context),Also applies to: 199-200
lib/services/mostro_service.dart (1)
228-229: Don’t log full outgoing payloads
order.toJson()can include invoices and other sensitive fields. Avoid logging the full payload.Prefer minimal, non-sensitive metadata:
- _logger.i('Sending DM, Event ID: ${event.id} with payload: ${order.toJson()}'); + _logger.i('Sending DM, Event ID: ${event.id}, action: ${order.action}, hasPayload: ${order.payload != null}');lib/shared/widgets/order_cards.dart (1)
58-74: Bug: currency appears twice after the switch to forAmountWithCurrencyYou’re passing
amountString(which already contains "$amount $currency $flag") as the first argument andcurrencyas the second. This renders duplicated currency, e.g., “por 100 USD 🇺🇸 USD”.Apply this diff to build the string correctly and keep the flag as a separate span:
- text: S.of(context)!.forAmountWithCurrency(amountString, currency), + text: S.of(context)!.forAmountWithCurrency(amount, currency), style: const TextStyle( color: Colors.white70, fontSize: 16, ), children: [ + if (currencyFlag != null && currencyFlag.isNotEmpty) + TextSpan( + text: ' $currencyFlag', + style: const TextStyle( + color: Colors.white60, + fontSize: 15, + ), + ), if (priceText != null && priceText!.isNotEmpty) TextSpan( text: ' $priceText', style: const TextStyle( color: Colors.white60, fontSize: 15, ), ), ],And outside this hunk (for cleanliness), remove
amountStringand keepcurrencyFlag:final currencyFlag = CurrencyUtils.getFlagFromCurrencyData(currency, currencyData);This avoids duplicate text and preserves the flag.
lib/features/order/screens/take_order_screen.dart (1)
156-166: Duplicate currency in “for amount with currency” text
amountStringalready includes the currency (and flag), so passingorder.currencyagain duplicates it (e.g., “for 100 USD 🇺🇸 USD”). Pass only the numeric amount to the i18n string and, if desired, append the flag separately.Apply this diff:
- text: S.of(context)!.forAmountWithCurrency(amountString, order.currency ?? ''), + text: S.of(context)!.forAmountWithCurrency( + order.fiatAmount.toString(), + order.currency ?? '', + ), style: const TextStyle( color: Colors.white70, fontSize: 16, ), - children: [ + children: [ + TextSpan(text: ' $currencyFlag'), if (priceText.isNotEmpty) TextSpan( text: ' $priceText', style: const TextStyle( color: Colors.white60, fontSize: 15, ), ),lib/l10n/intl_it.arb (1)
730-741: Duplicate key:forAmountreintroduced while also addingforAmountWithCurrency.
forAmountalready exists earlier in the file. Keep one definition and preferforAmountWithCurrencyfor clarity; remove this duplicate.- "forAmount": "per {amount}",Also audit callsites as suggested for EN.
🧹 Nitpick comments (40)
lib/shared/notifiers/session_notifier.dart (1)
181-183: Review Sensitive Logger Calls for ClarityI found two remaining logger invocations that output key material—though they’re only logging the public halves, it’s worth confirming they’re intentional and renaming them for clarity:
• lib/features/chat/notifiers/chat_room_notifier.dart:233
_logger.i('Session found with shared key: ${session.sharedKey?.public}');• lib/data/repositories/dispute_repository.dart:31
_logger.d(' Session: orderId=${session.orderId}, tradeKey=${session.tradeKey.public}');These log the public components (
.public) of the shared/trade keys, which aren’t secrets—but the messages imply they’re full “shared key” or “trade key.” To avoid confusion, consider renaming them to explicitly reference “public key,” e.g.:- _logger.i('Session found with shared key: ${session.sharedKey?.public}'); + _logger.i('Session public key for session ${session.id}: ${session.sharedKey?.public}');And similarly for
tradeKey.public.Aside from these, I didn’t find any other logger calls exposing sensitive fields. Great work sanitizing the derived key log!
lib/features/chat/widgets/trade_information_tab.dart (2)
135-135: LGTM: updated i18n key usage for fiat amountSwitching to
forAmountWithCurrency(amount, currency)aligns with the new ARB keys and improves clarity. One minor improvement: avoid.toString()to preserve locale-specific formatting (thousands separators/decimals).Example:
- S.of(context)!.forAmountWithCurrency(order!.fiatAmount.toString(), order!.fiatCode), + S.of(context)!.forAmountWithCurrency( + NumberFormat.decimalPattern().format(order!.fiatAmount), + order!.fiatCode, + ),
115-126: Localize status labels instead of raw.toUpperCase()
order!.status.value.toUpperCase()is user-facing text and should be localized. Consider mapping known statuses to i18n keys (e.g.,orderStatusActive,orderStatusPending, etc.) for consistency with the disputes status badges.If you add keys, the usage could be:
- order!.status.value.toUpperCase(), + { + 'active': S.of(context)!.orderStatusActive, + 'pending': S.of(context)!.orderStatusPending, + 'completed': S.of(context)!.orderStatusCompleted, + }[order!.status.value] ?? order!.status.value.toUpperCase(),This preserves a fallback while providing localized strings for known statuses.
Also applies to: 121-129
lib/services/mostro_service.dart (3)
45-52: Mark events as processed only after successful parse/persistThe event is marked stored before decryption/parsing. If parsing fails, the event is permanently skipped on retries, potentially dropping messages.
Move the
putItemcall to after the message is successfully persisted:if (await eventStore.hasItem(event.id!)) return; - await eventStore.putItem( - event.id!, - { - 'id': event.id, - 'created_at': event.createdAt!.millisecondsSinceEpoch ~/ 1000, - }, - );…and after
addMessage(...)succeeds:await messageStorage.addMessage(decryptedEvent.id!, msg); - + await eventStore.putItem( + event.id!, + { + 'id': event.id, + 'created_at': event.createdAt!.millisecondsSinceEpoch ~/ 1000, + }, + );If idempotency is critical and duplicates are a concern, consider a two-phase marker (
processing→processed) with retry-safe semantics.Also applies to: 100-105
75-87: Be resilient to alternate DM shapesCurrent parsing assumes
jsonDecode(content)yields a non-emptyListwhose first element is aMap. If upstream sends a directMap(single-object), this will be dropped.Either confirm upstream always wraps content in a single-element array, or support both shapes:
- final result = jsonDecode(decryptedEvent.content!); - if (result is! List || result.isEmpty) { + final result = jsonDecode(decryptedEvent.content!); + if (result is List && result.isNotEmpty) { + // ok + } else if (result is Map<String, dynamic>) { + // normalize to list-of-one + result = [result]; + } else { _logger.w('Invalid message format: Expected non-empty list, got $result'); return; }Adjust types accordingly.
55-63: Recipient Tag Is Spec-Mandated for Direct MessagesAccording to both NIP-04 (kind 4) and NIP-17 (kind 14), every encrypted direct‐message event MUST include a “p” tag identifying the recipient’s public key (nostr-nips.com, e2encrypted.com). In our code,
event.recipientis implemented as pulling the first “p” tag from the event’stagsinNostrEventExtensions.recipient(). The gift-wrap wrapper we create inmostroWrapalso always emits a “p” tag for the shared key, so any incoming wrapper event will haverecipientpopulated.Thus, filtering on
event.recipientto find the matching session is consistent with the Nostr spec and safe for all properly formed DM events—no change is required to make the lookup work under normal conditions.• Optional robustness improvement: if you ever process events from untrusted or non-standard sources, you could catch cases where
event.recipientisnulland instead loop through each session’s private key to attemptNostrUtils.decryptNIP59Event, matching on the resulting inner event’spubkeyor tags. This fallback is purely defensive and not strictly necessary.lib/data/models/dispute_event.dart (3)
44-51: Normalize status and restrict to allowed setToday, upstream may send variants like "in-progress", "in_progress", or "solved". Normalizing to a controlled set makes downstream UI more robust and aligns with the status badge system.
Proposed helper (outside selected range) to add below the factory:
static String _normalizeStatus(String raw) { final v = raw.trim().toLowerCase(); if (v == 'initiated' || v == 'started') return 'initiated'; if (v == 'in-progress' || v == 'in_progress' || v == 'ongoing') return 'in-progress'; if (v == 'resolved' || v == 'solved') return 'resolved'; if (v == 'closed') return 'closed'; return 'unknown'; }Then update the factory’s status assignment:
- final statusValue = (sTag != null && sTag.length > 1 && sTag[1].toString().isNotEmpty) - ? sTag[1] - : 'unknown'; + final statusValueRaw = (sTag != null && sTag.length > 1 && sTag[1].toString().isNotEmpty) + ? sTag[1].toString() + : 'unknown'; + final statusValue = _normalizeStatus(statusValueRaw);
52-56: Optionally enforce z=dispute to drop misclassified events earlyPR objective states we fetch events with
z=dispute. Enforcing this in the parser prevents accidental ingestion if upstream filters regress.If you want to enforce:
- // Optionally verify 'z' tag indicates dispute; do not throw if absent - event.tags?.firstWhere( - (tag) => tag.isNotEmpty && tag[0] == 'z' && tag.length > 1 && tag[1] == 'dispute', - orElse: () => [], - ); + final isDispute = (event.tags ?? const <List<dynamic>>[]) + .any((tag) => tag.isNotEmpty && tag[0] == 'z' && tag.length > 1 && tag[1] == 'dispute'); + if (!isDispute) { + throw ArgumentError('DisputeEvent requires z=dispute tag'); + }Would you like this stricter behavior, or keep it permissive?
107-110: Minor: include orderId in toString for easier diagnosticsThis helps during logs while resolving "Unknown Order ID" cases from the repository.
- return 'DisputeEvent(id: $id, disputeId: $disputeId, status: $status, createdAt: $createdAt)'; + return 'DisputeEvent(id: $id, disputeId: $disputeId, status: $status, createdAt: $createdAt, orderId: ${orderId ?? 'null'})';lib/features/disputes/widgets/dispute_icon.dart (1)
11-15: Use const where possible
Iconcan beconstfor better performance.- child: Icon( + child: const Icon( Icons.warning_amber, color: Colors.amber, size: 32, ),lib/features/disputes/widgets/dispute_order_id.dart (1)
11-16: Make order IDs resilient in tight layouts and improve accessibilityLong IDs can overflow on narrow screens, and screen readers benefit from a labeled value. Below is the pattern you should apply across the codebase—not just in
dispute_order_id.dart—to ensure consistency.import 'package:flutter/material.dart'; +import 'package:mostro_mobile/generated/l10n.dart'; @override Widget build(BuildContext context) { - return Text( - orderId, - style: Theme.of(context).textTheme.bodyMedium?.copyWith( - fontWeight: FontWeight.w500, - ), - ); + return Semantics( + label: S.of(context).disputeOrderIdSemantics(orderId), + child: Text( + orderId, + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: Theme.of(context).textTheme.bodyMedium?.copyWith( + fontWeight: FontWeight.w500, + ), + ), + ); }You’ll need to add this ARB entry:
- disputeOrderIdSemantics:
"Order ID: {id}"(with placeholder{id}).Please apply the same pattern wherever an order ID is displayed without overflow handling:
• lib/shared/widgets/order_cards.dart
Expanded(child: Text(orderId, …))→ wrap inSemantics, addmaxLines: 1andoverflow: TextOverflow.ellipsis.• lib/features/chat/widgets/trade_information_tab.dart (line 74)
Text(orderId, …)→ wrap inSemanticsand add truncation props.• lib/features/disputes/widgets/dispute_order_id.dart
As shown above.• lib/features/trades/widgets/mostro_message_detail_widget.dart
We didn’t find a directText(orderId)in a quick scan—but please double-check any place this ID is rendered in aTextwidget and apply the same treatment.lib/features/disputes/widgets/dispute_list_item.dart (1)
18-43: Use Material+InkWell for ripple, focus, and better a11y; make DisputeIcon constGestureDetector works but lacks Material semantics/feedback. InkWell inside a Material improves UX and accessibility, and const on DisputeIcon reduces rebuild cost.
- return GestureDetector( - onTap: onTap, - child: Container( - decoration: BoxDecoration( - border: Border( - bottom: BorderSide( - color: Colors.white.withValues(alpha: 0.05), - width: 1.0, - ), - ), - ), - child: Padding( - padding: const EdgeInsets.symmetric(vertical: 16, horizontal: 16), - child: Row( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - DisputeIcon(), - const SizedBox(width: 16), - Expanded( - child: DisputeContent(dispute: dispute), - ), - ], - ), - ), - ), - ); + return Material( + color: Colors.transparent, + child: InkWell( + onTap: onTap, + child: Container( + decoration: BoxDecoration( + border: Border( + bottom: BorderSide( + color: Colors.white.withValues(alpha: 0.05), + width: 1.0, + ), + ), + ), + padding: const EdgeInsets.symmetric(vertical: 16, horizontal: 16), + child: Row( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + const DisputeIcon(), + const SizedBox(width: 16), + Expanded( + child: DisputeContent(dispute: dispute), + ), + ], + ), + ), + ), + );Nit: if AppTheme exposes a divider token, prefer that over hardcoded Colors.white.withValues(alpha: 0.05).
lib/features/disputes/widgets/dispute_status_content.dart (2)
16-16: Broaden resolved-status detection and avoid repeated toLowerCase() calls.Support all known synonyms from the PR (“resolved”, “solved”) and compute lowercase once to avoid duplicating calls.
Apply:
- bool isResolved = dispute.status.toLowerCase() == 'resolved' || dispute.status.toLowerCase() == 'closed'; + final status = dispute.status.toLowerCase(); + const resolvedStatuses = {'resolved', 'solved', 'closed', 'settled'}; + final isResolved = resolvedStatuses.contains(status);
20-29: Align resolved-state colors with the badge’s settled palette.The PR maps resolved/solved to the “settled” (blue) palette. This container uses Mostro green, which may read as “success” instead of “settled”. Prefer the same tokens the badge uses (e.g., statusSettledBackground/Text) for consistency.
If available in AppTheme, consider:
- color: AppTheme.mostroGreen.withValues(alpha: 0.1), + color: AppTheme.statusSettledBackground.withValues(alpha: 0.1), ... - color: AppTheme.mostroGreen.withValues(alpha: 0.3), + color: AppTheme.statusSettledBackground.withValues(alpha: 0.3), ... - color: AppTheme.mostroGreen, + color: AppTheme.statusSettledText, ... - style: TextStyle( - color: AppTheme.mostroGreen, + style: TextStyle( + color: AppTheme.statusSettledText,Please confirm the exact token names in AppTheme.
Also applies to: 45-50, 53-59
lib/features/disputes/widgets/disputes_list.dart (1)
97-103: Avoid exposing raw error details to end users.Rendering error.toString() can leak internal details. Show a friendly message and log the error instead.
Apply:
- Text( - error.toString(), - style: TextStyle( - color: AppTheme.textSecondary, - fontSize: 14, - ), - textAlign: TextAlign.center, - ), + // Consider logging `error` & `stack` via your logger instead of showing it. + const SizedBox.shrink(),If you still want context, consider a “Show details” expandable panel gated behind a debug flag.
lib/features/disputes/widgets/dispute_input_section.dart (2)
29-57: Good async handling and mounted checks. Consider dismissing the keyboard after send.The loading guard and mounted checks look solid. UX nit: dismiss the keyboard after a successful send.
Apply:
await notifier.sendMessage(message); _messageController.clear(); + if (mounted) { + FocusScope.of(context).unfocus(); + }
63-71: Prefer theme tokens over raw white for dividers.Minor consistency: if AppTheme defines a divider or surface-variant color, prefer that over Colors.white.withValues(alpha: 0.05).
Example:
- top: BorderSide( - color: Colors.white.withValues(alpha: 0.05), + top: BorderSide( + color: AppTheme.divider, // verify or pick the closest token width: 1.0, ),lib/services/dispute_service.dart (2)
118-146: Handle additional status synonyms (“solved”, “closed”, “settled”).PR notes include “resolved/solved”. Add these to map to adminSettled to keep behavior consistent across the app.
Apply:
- case 'resolved': + case 'resolved': + case 'solved': + case 'closed': + case 'settled': action = mostro.Action.adminSettled; _logger.info('Admin settled dispute for order $orderId'); break;
64-77: Minor logging fix: pass error once; avoid duplicating it in the message and the error field.Logger.severe(message, error, stackTrace) already captures error; including
$ein the message is redundant.Apply:
- } catch (e, stackTrace) { - _logger.severe('Error handling dispute event: $e', e, stackTrace); + } catch (e, stackTrace) { + _logger.severe('Error handling dispute event', e, stackTrace); }Repeat similarly for other severe() calls if desired.
lib/features/disputes/providers/dispute_providers.dart (5)
17-20: Use ref.watch to propagate repository re-creation to consumers.Using read will not rebuild if the repository changes. Prefer watch for dependencies within providers.
-final userDisputesProvider = FutureProvider<List<Dispute>>((ref) async { - final repository = ref.read(disputeRepositoryProvider); +final userDisputesProvider = FutureProvider<List<Dispute>>((ref) async { + final repository = ref.watch(disputeRepositoryProvider); return repository.fetchUserDisputes(); });
23-26: Same here: prefer watch over read.-final disputeDetailsProvider = FutureProvider.family<Dispute?, String>((ref, disputeId) async { - final repository = ref.read(disputeRepositoryProvider); +final disputeDetailsProvider = FutureProvider.family<Dispute?, String>((ref, disputeId) async { + final repository = ref.watch(disputeRepositoryProvider); return repository.getDisputeDetails(disputeId); });
28-33: Stream should probably be scoped per dispute to avoid over-subscribing.If subscribeToDisputeEvents() emits all disputes, make this a StreamProvider.family(disputeId) with filtering to reduce bandwidth and rebuilds. If the repository already scopes by user, consider documenting it.
35-38: watch for repository here too.-final createDisputeProvider = FutureProvider.family<bool, String>((ref, orderId) async { - final repository = ref.read(disputeRepositoryProvider); +final createDisputeProvider = FutureProvider.family<bool, String>((ref, orderId) async { + final repository = ref.watch(disputeRepositoryProvider); return repository.createDispute(orderId); });
8-14: Avoid passing Ref into repositories to keep data layer UI-framework-agnostic.Inject concrete dependencies (nostrService, mostroPubkey, any callbacks) instead of ref. This improves testability and reusability across DI frameworks.
lib/features/disputes/widgets/dispute_communication_section.dart (2)
226-233: Avoid leaking raw error details to end-users.Showing error.toString() can expose internal info. Prefer a generic localized hint and log details instead.
- Text( - error.toString(), - style: TextStyle( - color: AppTheme.textSecondary, - fontSize: 14, - ), - textAlign: TextAlign.center, - ), + // Consider logging the error instead of showing internals + Text( + S.of(context)!.tryAgainLater, + style: TextStyle( + color: AppTheme.textSecondary, + fontSize: 14, + ), + textAlign: TextAlign.center, + ),
271-285: Theme consistency: prefer AppTheme over hard-coded Colors.amber/white.To keep visual consistency and theming flexibility, consider moving amber/white colors to AppTheme (e.g., a warning color) and reference them here. No change required if AppTheme lacks equivalents.
lib/features/disputes/notifiers/dispute_chat_notifier.dart (2)
258-263: Add a private helper to cancel subscriptions; reuse in init and dispose.@override void dispose() { _logger.i('Disposing dispute chat notifier for dispute: $disputeId'); - _chatSubscription?.cancel(); - _disputeSubscription?.cancel(); + _cancelSubscriptions(); super.dispose(); } + + Future<void> _cancelSubscriptions() async { + try { + await _chatSubscription?.cancel(); + await _disputeSubscription?.cancel(); + } finally { + _chatSubscription = null; + _disputeSubscription = null; + } + }
145-159: Deduplicate incoming messages by event id to avoid duplicates from multiple relays.void _handleChatMessage(NostrEvent event) { try { _logger.i('Received chat message for dispute: $disputeId'); final currentState = state.value; if (currentState == null) return; - // TODO: Decrypt gift wrap message - // For now, we'll add the raw event to messages - final updatedMessages = [...currentState.messages, event]; + // TODO: Decrypt gift wrap message + // Basic dedupe by event id + final alreadyExists = currentState.messages.any((m) => m.id == event.id); + if (alreadyExists) return; + final updatedMessages = [...currentState.messages, event];lib/data/models/dispute_chat.dart (3)
73-83: Fallback lastMessageAt when message timestamp is missing.Guarantees recency sorting even when an incoming event lacks createdAt.
DisputeChat addMessage(NostrEvent message) { final updatedMessages = [...messages, message]; final messageTime = message.createdAt; - final newLastMessageAt = messageTime; + final newLastMessageAt = messageTime ?? DateTime.now(); return copyWith( messages: updatedMessages, lastMessageAt: newLastMessageAt, ); }
106-123: Equality ignores messages and timestamps; this can mask state changes.Overriding == without considering messages can lead to subtle bugs in collections/tests. Either remove ==/hashCode or incorporate fields that change with new messages (e.g., messages.length, lastMessageAt).
@override bool operator ==(Object other) { if (identical(this, other)) return true; return other is DisputeChat && other.disputeId == disputeId && other.adminPubkey == adminPubkey && other.userPubkey == userPubkey && - other.disputeToken == disputeToken; + other.disputeToken == disputeToken && + other.messages.length == messages.length && + other.lastMessageAt == lastMessageAt; } @override int get hashCode => Object.hash( disputeId, adminPubkey, userPubkey, - disputeToken, + disputeToken, + messages.length, + lastMessageAt, );
23-44: Token verification heuristic is OK; consider strict token matching.If tokens can appear as substrings of other content, prefer strict matching (e.g., word boundary or a dedicated tag/format).
lib/features/disputes/screens/dispute_chat_screen.dart (2)
70-102: Avoid re-watching the same disputeDetailsProvider in the enhanced lookup.You already have
disputefrom the top-level watch ofdisputeDetailsProvider(widget.disputeId). Re-watching the same provider here can cause unnecessary rebuilds. Prefer computing the “enhanced” data in the repository or expose a single provider that resolves orderId internally, or just use the first value and let it refresh when available.
70-121: Nested scrolling can be heavy with the chat ListView inside a SingleChildScrollView.Given DisputeCommunicationSection contains a ListView, consider:
- Removing the outer SingleChildScrollView and using a CustomScrollView with slivers.
- Or making the inner ListView non-scrollable (shrinkWrap + NeverScrollableScrollPhysics) and letting the outer scroll handle it.
This avoids expensive nested scroll computations and constraints issues.
lib/data/repositories/dispute_repository.dart (5)
73-85: Avoid per-dispute network calls (N+1) when resolving status.Calling
_getDisputeStatusinside the loop issues one fetch per dispute. This will not scale and will slow down the list for users with many disputes. Prefer a single query that fetches all 38383 events for the set of disputeIds and then reduce in-memory.Apply this sketch to batch the fetch:
- for (final message in allMessages) { + final toResolve = <String>{}; + for (final message in allMessages) { ... - if (message.payload is Dispute) { - final dispute = message.payload as Dispute; - final status = await _getDisputeStatus(dispute.disputeId); + if (message.payload is Dispute) { + final dispute = message.payload as Dispute; + toResolve.add(dispute.disputeId); ... } - } + } + + // Batch-fetch latest statuses for all disputes + final latestStatuses = await _getLatestStatuses(toResolve); + + // then when building disputeMap: + final status = latestStatuses[dispute.disputeId] ?? 'initiated';Add a private
_getLatestStatuses(Set<String> ids)that performs one filter with multiple'#d'values and returns aMap<String,String>.
85-101: Use event timestamps instead ofDateTime.now()for admin assignment.
adminTookAtshould reflect the event time that Mostro emitted, not the local device time. If the message object carries a timestamp, prefer it; otherwise, derive from the source event.- adminTookAt: DateTime.now(), + adminTookAt: message.createdAt ?? DateTime.now(),If
message.createdAtisn’t available, consider looking up the corresponding 38383 event for that dispute and use itscreated_at.
339-349: Heuristic fallback can mis-assign disputes when multiple orders exist.Defaulting to the single active order is risky. At minimum, guard behind a feature flag or log a structured warning that includes the disputeId and the candidate order IDs.
Consider returning null instead of making an assumption, and let the UI show “Unknown Order ID” with a help CTA.
409-454: Don’t rethrow inside the stream map; drop unparseable events instead.
rethrowwill surface asStream.error, potentially collapsing the UI stream. Prefer filtering out bad events and logging.- return _nostrService.subscribeToEvents(request).map((event) { + return _nostrService.subscribeToEvents(request).map((event) { try { ... - } catch (e) { - _logger.w('Failed to parse DisputeEvent for ${event.id}: $e'); - // Fallback... - try { - ... - } catch (e2) { - _logger.w('Fallback parse also failed for ${event.id}: $e2'); - rethrow; - } - } - }).handleError((error) { - _logger.e('Error in dispute events stream: $error'); - }); + } catch (e) { + _logger.w('Dropping unparsable dispute event ${event.id}: $e'); + return null; + } + }).where((d) => d != null).cast<Dispute>();Optionally, take a
sinceargument so callers can resume from the last known timestamp instead ofDateTime.now().
23-50: Debug logging prints entire event content.Public 38383 events are fine, but keep logs lean; avoid logging full
contentarrays if they may later include user-provided text.Replace with IDs, kind, created_at, and the ‘d’ and ‘s’ tags only.
lib/data/models/dispute.dart (2)
155-161: Handle seconds vs milliseconds increated_at.Some payloads encode seconds. Guard for both.
- if (timestamp is int) { - createdAt = DateTime.fromMillisecondsSinceEpoch(timestamp); - } + if (timestamp is int) { + final ms = timestamp > 1000000000000 ? timestamp : timestamp * 1000; + createdAt = DateTime.fromMillisecondsSinceEpoch(ms); + }Same comment applies to
admin_took_at.
300-318: RobustcreatedAtconversion in legacy factory.
disputeEvent.createdAtmay be aDateTime. Current code treats non-int as “now”, losing fidelity.createdAt: DateTime.fromMillisecondsSinceEpoch( - disputeEvent.createdAt is int - ? disputeEvent.createdAt * 1000 - : DateTime.now().millisecondsSinceEpoch + disputeEvent.createdAt is int + ? disputeEvent.createdAt * 1000 + : (disputeEvent.createdAt is DateTime + ? (disputeEvent.createdAt as DateTime).millisecondsSinceEpoch + : DateTime.now().millisecondsSinceEpoch) ),
📜 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 ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
lib/core/app_routes.dart(2 hunks)lib/core/config.dart(1 hunks)lib/data/models/dispute.dart(2 hunks)lib/data/models/dispute_chat.dart(1 hunks)lib/data/models/dispute_event.dart(1 hunks)lib/data/repositories/dispute_repository.dart(1 hunks)lib/features/chat/screens/chat_rooms_list.dart(5 hunks)lib/features/chat/widgets/trade_information_tab.dart(1 hunks)lib/features/disputes/notifiers/dispute_chat_notifier.dart(1 hunks)lib/features/disputes/providers/dispute_providers.dart(1 hunks)lib/features/disputes/screens/dispute_chat_screen.dart(1 hunks)lib/features/disputes/widgets/dispute_communication_section.dart(1 hunks)lib/features/disputes/widgets/dispute_content.dart(1 hunks)lib/features/disputes/widgets/dispute_description.dart(1 hunks)lib/features/disputes/widgets/dispute_header.dart(1 hunks)lib/features/disputes/widgets/dispute_icon.dart(1 hunks)lib/features/disputes/widgets/dispute_info_card.dart(1 hunks)lib/features/disputes/widgets/dispute_input_section.dart(1 hunks)lib/features/disputes/widgets/dispute_list_item.dart(1 hunks)lib/features/disputes/widgets/dispute_order_id.dart(1 hunks)lib/features/disputes/widgets/dispute_status_badge.dart(1 hunks)lib/features/disputes/widgets/dispute_status_content.dart(1 hunks)lib/features/disputes/widgets/disputes_list.dart(1 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(1 hunks)lib/features/order/screens/take_order_screen.dart(1 hunks)lib/features/trades/screens/trade_detail_screen.dart(3 hunks)lib/l10n/intl_en.arb(3 hunks)lib/l10n/intl_es.arb(3 hunks)lib/l10n/intl_it.arb(3 hunks)lib/services/dispute_service.dart(1 hunks)lib/services/mostro_service.dart(1 hunks)lib/shared/notifiers/session_notifier.dart(1 hunks)lib/shared/widgets/order_cards.dart(1 hunks)pubspec.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use S.of(context) for all user-facing strings (no hardcoded UI text)
Target zero flutter analyze issues; remove unused imports and use latest non-deprecated APIs (e.g., withValues() over withOpacity())
Check mounted before using BuildContext after async gaps
Prefer const constructors and const widgets where possible
Files:
lib/features/disputes/widgets/dispute_icon.dartlib/features/disputes/widgets/dispute_content.dartlib/shared/notifiers/session_notifier.dartlib/services/mostro_service.dartlib/features/order/screens/take_order_screen.dartlib/shared/widgets/order_cards.dartlib/features/disputes/providers/dispute_providers.dartlib/features/chat/screens/chat_rooms_list.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/features/disputes/widgets/disputes_list.dartlib/features/disputes/widgets/dispute_list_item.dartlib/features/disputes/widgets/dispute_communication_section.dartlib/features/disputes/widgets/dispute_description.dartlib/core/app_routes.dartlib/features/disputes/widgets/dispute_info_card.dartlib/features/disputes/notifiers/dispute_chat_notifier.dartlib/features/disputes/widgets/dispute_header.dartlib/features/disputes/widgets/dispute_status_badge.dartlib/features/disputes/widgets/dispute_order_id.dartlib/features/chat/widgets/trade_information_tab.dartlib/features/trades/screens/trade_detail_screen.dartlib/features/disputes/widgets/dispute_status_content.dartlib/data/models/dispute_event.dartlib/data/repositories/dispute_repository.dartlib/features/disputes/widgets/dispute_input_section.dartlib/services/dispute_service.dartlib/core/config.dartlib/data/models/dispute_chat.dartlib/data/models/dispute.dartlib/features/disputes/screens/dispute_chat_screen.dart
pubspec.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Remove unused dependencies from pubspec.yaml
Files:
pubspec.yaml
lib/features/**/providers/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under features/{feature}/providers/
Files:
lib/features/disputes/providers/dispute_providers.dart
lib/core/app_routes.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Configure app navigation using GoRouter in core/app_routes.dart
Files:
lib/core/app_routes.dart
lib/data/repositories/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Use the Repository pattern for all data access in data/repositories/
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 ARB files (en, es, it)
Files:
lib/l10n/intl_es.arblib/l10n/intl_it.arblib/l10n/intl_en.arb
lib/l10n/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
After modifying ARB files, regenerate localization (flutter gen-l10n or build_runner as configured)
Files:
lib/l10n/intl_es.arblib/l10n/intl_it.arblib/l10n/intl_en.arb
🧠 Learnings (6)
📚 Learning: 2025-08-24T16:01:53.107Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T16:01:53.107Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature under features/{feature}/providers/
Applied to files:
lib/features/disputes/providers/dispute_providers.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Applied to files:
lib/features/disputes/providers/dispute_providers.dart
📚 Learning: 2025-08-24T16:01:53.107Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T16:01:53.107Z
Learning: Applies to lib/core/app_routes.dart : Configure app navigation using GoRouter in core/app_routes.dart
Applied to files:
lib/core/app_routes.dart
📚 Learning: 2025-08-24T16:01:53.107Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T16:01:53.107Z
Learning: Applies to lib/**/*.dart : Use S.of(context) for all user-facing strings (no hardcoded UI text)
Applied to files:
lib/features/chat/widgets/trade_information_tab.dart
📚 Learning: 2025-08-24T16:01:53.107Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T16:01:53.107Z
Learning: Applies to lib/data/repositories/**/*.dart : Use the Repository pattern for all data access in data/repositories/
Applied to files:
lib/data/repositories/dispute_repository.dart
📚 Learning: 2025-08-24T16:01:53.107Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T16:01:53.107Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all ARB files (en, es, it)
Applied to files:
lib/l10n/intl_it.arb
🪛 RuboCop (1.76.1)
lib/l10n/intl_es.arb
[warning] 867-867: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 878-878: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_it.arb
[warning] 730-730: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 901-901: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 912-912: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_en.arb
[warning] 828-828: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 839-839: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
🔇 Additional comments (25)
pubspec.yaml (1)
54-54: Thepackage:loggingimport inlib/services/dispute_service.dartis indeed used, so we cannot simply drop theloggingdependency without breaking that file. Before removing it frompubspec.yaml, address the existing usage:• Either refactor
dispute_service.dartto use your preferredloggerpackage (e.g., replacepackage:loggingimports,Loggerinstantiation, and calls tolog.*withlogger.*), then safely removelogging: ^1.2.0frompubspec.yaml.
• Or, if that refactor isn’t intended now, keep theloggingdependency to satisfy the existing code.Next steps:
- Review
lib/services/dispute_service.dartand decide if it should be migrated to theloggerpackage.- If migrating, update imports and usage in that file, verify no other files depend on
logging, then remove the entry frompubspec.yaml.- If not migrating immediately, leave
loggingin place until the refactor can be done.lib/core/config.dart (1)
12-15: I've requested review of the settings persistence logic to confirm that outdated persistedmostroPublicKeyvalues won’t override the new default at runtime. This will ensure safe rollout of the updated key without requiring manual user resets.I’ll examine
settings_notifier.dartand any storage-related code (e.g.,SharedPreferencesor equivalent) to verify how persisted settings are loaded and whether they merge or replace defaults correctly.lib/data/models/dispute_event.dart (1)
75-82: Defensive: don’t bangevent.id!; throw a precise error if missingA few Nostr libraries treat
idas nullable during construction or parsing. Prefer an explicit check and error.- return DisputeEvent( - id: event.id!, + final evId = event.id; + if (evId == null || evId.isEmpty) { + throw ArgumentError('Event must have a non-empty id for DisputeEvent'); + } + return DisputeEvent( + id: evId, disputeId: dTag[1], status: statusValue, createdAt: timestamp, pubkey: event.pubkey, orderId: orderId, );⛔ Skipped due to learnings
Learnt from: chebizarro PR: MostroP2P/mobile#74 File: lib/notifications/notification_service.dart:54-59 Timestamp: 2025-05-08T16:31:29.582Z Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.lib/l10n/intl_es.arb (4)
652-681: New dispute strings look consistent and idiomaticWording, placeholders, and tone are consistent with the existing Spanish localization.
838-839: Consistency improvement acknowledgedUsing a simple, user-friendly past-tense message for order cancellation aligns with other timeout/cancel messages.
864-890: Ignore duplicate-key removal—no duplicates detected in intl_es.arbI ran the provided duplicate-detection script across all ARB locales and it reported zero duplicate keys in
lib/l10n/intl_es.arb(and in the other locale files). The keys “retry” and “orderIdLabel” each only appear once in the Spanish file, so there’s nothing to remove here.Likely an incorrect or invalid review comment.
701-711: Key rename verification complete – ensure numeric-only first argumentI’ve confirmed that there are no remaining calls to the old
forAmountkey and all three call sites now useforAmountWithCurrency:
lib/shared/widgets/order_cards.dart(line 58)lib/features/chat/widgets/trade_information_tab.dart(line 135)lib/features/order/screens/take_order_screen.dart(line 158)Please double-check that each
amountString(and similarly passed values in your other two files) contains only the numeric amount (e.g."100.00") and not a composite like"100.00 USD". If any include extra text, split them so the{amount}placeholder stays pure digits and the{currency}placeholder receives the currency code.lib/features/order/notfiers/abstract_mostro_notifier.dart (1)
113-118: Use disputeToken preferentially: LGTM; verify l10n placeholders and PII considerationsGood change using
dispute.disputeToken ?? dispute.disputeId. Confirm:
- l10n strings for both actions include
{user_token}(ES does).- The token is safe to surface in notifications (not a secret beyond dispute participants).
If there’s any locale still using
{dispute_id}in these messages, update them to{user_token}.Also applies to: 120-125
lib/core/app_routes.dart (1)
147-157: New dispute route wiring looks correct
/dispute_details/:disputeIdrendersDisputeChatScreenwith the provided parameter and uses the default transition inside the Shell. This integrates with the listeners as intended.lib/features/disputes/widgets/dispute_info_card.dart (1)
21-23: [msg]lib/features/chat/screens/chat_rooms_list.dart (4)
60-63: LGTM: switched to withValues()Using Color.withValues(alpha: …) aligns with “latest non-deprecated APIs” in the guidelines.
88-91: LGTM: consistent divider stylingRepeating the withValues() pattern keeps visual consistency across sections.
110-112: Good integration: DisputesList is now wired into the Disputes tabUsing a dedicated const DisputesList keeps concerns separated and will simplify future maintenance/testing.
94-99: Centralizing tab descriptions via helper is a good moveThis improves readability (single source for the dynamic header text) and makes it easy to extend when tabs change.
lib/features/disputes/widgets/dispute_content.dart (1)
7-26: Clean composition; spacing and alignment look goodThe widget composes header, order, and description logically with minimal coupling. SizedBox spacers are const. Nothing blocking here.
lib/services/dispute_service.dart (1)
101-106: Verify availability of ref.exists(...) in your Riverpod version.Not all Riverpod versions expose ref.exists for this check. If unavailable, consider an alternative (e.g., checking against an orders registry provider, or guarding reads with a selector).
If needed, a safe fallback is to skip the existence check and let handleEvent be a no-op for unknown orders after verifying notifier.mounted, or maintain a dedicated provider that tracks known order IDs and consult that instead:
final knownOrders = _ref.read(ordersIndexProvider); // Set<String> if (!knownOrders.contains(orderId)) { _logger.info('No order notifier found for order $orderId'); return; }lib/features/disputes/providers/dispute_providers.dart (1)
1-5: Good placement and separation of providers by feature.Providers are correctly organized under features/disputes/providers, matching the project convention.
lib/data/repositories/dispute_repository.dart (1)
150-206: Repository pattern and error handling look solid.Constructor injection, scoped providers, and catch blocks are consistent with the existing architecture. Nice guardrails around missing sessions and private keys.
lib/data/models/dispute.dart (2)
333-348: Status-to-description coupling is brittle and duplicates i18n.The switch includes
settledandseller-refunded, but your status badges useresolved/solved. Move this mapping to UI with localization and maintain a single canonical mapping of status strings.Would you like me to produce a shared
DisputeStatusenum + mapper used by both badges and descriptions to eliminate drift?
179-202: Model extensions look good and consistent.
copyWith, equality, hashCode, andtoJsonare coherent and cover new fields.lib/l10n/intl_en.arb (3)
621-646: Nice coverage for dispute UI text.The new dispute keys and placeholders look coherent and adequate for the new screens.
801-814: Admin chat copy looks good.Labels and helper strings will map well to the communication section.
670-680: All translation callsites updated; no remainingforAmountusagesNo instances of the old
forAmountkey were found in the codebase, and all existing references now correctly callforAmountWithCurrency. You can consider removing the unusedforAmountentry fromintl_en.arb, but there are no outstanding callsite updates needed.lib/l10n/intl_it.arb (2)
681-706: Good Italian translations for dispute flows.Coverage looks complete for the new screens and instructions.
874-887: Admin chat strings: solid.Wording aligns with the UX expectations for assignment and token verification.
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
Cuando el administrador toma la disputa, aunque aparece en el SnackBar el mensaje admin-took-dispute y la disputa se muestra como in-progress, aún le sale al usuario: Esperando asignación de administrador.
Debería avisarle del admin que la tomó
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
1 hora despues volvi a abrir la app y dice "Disputa no encontrada", estos son los logs
┌───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
│ #0 DisputeRepository.getDisputeDetails (package:mostro_mobile/data/repositories/dispute_repository.dart:211:15)
│ #1 disputeDetailsProvider.<anonymous closure> (package:mostro_mobile/features/disputes/providers/dispute_providers.dart:25:21)
├┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄
│ 🐛 Fetching dispute details for: 8dcf1e97-6516-4bc1-bdbb-426630efeebd
└───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
┌───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
│ #0 DisputeRepository.getDisputeDetails (package:mostro_mobile/data/repositories/dispute_repository.dart:218:15)
│ #1 disputeDetailsProvider.<anonymous closure> (package:mostro_mobile/features/disputes/providers/dispute_providers.dart:25:21)
├┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄
│ 🐛 User sessions: 1, orderIds: {2ee421e0-6013-4379-81c9-a2000e0628cc}, pubkeys: [ddb5dbd6bbda11234ba832f6be1ea0629ca1b6fae9ffc3a9faae06b0a8f639c5]
└───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
┌───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
│ #0 DisputeRepository.getDisputeDetails (package:mostro_mobile/data/repositories/dispute_repository.dart:264:15)
│ #1 <asynchronous suspension>
├┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄
│ 🐛 Searching for disputeId 8dcf1e97-6516-4bc1-bdbb-426630efeebd in 0 DMs and 5 encrypted DMs
└───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
┌───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
│ #0 DisputeRepository.getDisputeDetails (package:mostro_mobile/data/repositories/dispute_repository.dart:307:17)
│ #1 <asynchronous suspension>
├┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄
│ 🐛 Searching encrypted DMs for dispute 8dcf1e97-6516-4bc1-bdbb-426630efeebd
└───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
┌───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
│ #0 DisputeRepository.getDisputeDetails (package:mostro_mobile/data/repositories/dispute_repository.dart:404:15)
│ #1 <asynchronous suspension>
├┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄
│ ⛔ Failed to get dispute details: Concurrent modification during iteration: Instance(length:6) of '_GrowableList'.
└───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
- Si el admin resuelve la disputa, aunqu een la UI se muestra Dispute resolved, sale la etiqueta de
Iniciada
- Al abrir la disputa dice "Esta disputa fue abierta contra ti por
pubkey del admin que tomo la disputa" pero fue el usuario quien abrio la disputa.
- Al final dice "Esperando asignacion del administrador" pero como ya se resolvio, debe decir el veredicto del admin si fua cancelada o completada la orden
…resolved disputes
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
lib/l10n/intl_en.arb (3)
684-687: Fix duplicate key: createdOn appears twice (conflicts with line ~299).Keep the earlier "createdOn": "Created on" entry and remove this later colon-suffixed variant to prevent l10n generation failures.
Apply this diff:
- "createdOn": "Created on:",
756-758: Fix duplicate key: proofOfWork is already defined earlier (line ~360).Remove the later duplicate under “About Screen New Strings”.
- "proofOfWork": "Proof of Work",
932-954: Fix duplicate and malformed entries in intl_en.arb. Duplicate keys found (createdOn,orderIdLabel,proofOfWork,retry,yourSharedKey) and a JSON syntax error must be resolved—remove duplicates, add the missing comma(s) so the file parses, then runflutter gen-l10nand verify thatintl_it.arbandintl_es.arbinclude all the new keys.lib/l10n/intl_it.arb (3)
741-744: Fix duplicate key: createdOn appears twice (earlier at line ~262).Remove this later colon-suffixed variant.
- "createdOn": "Creato il:",
635-639: Remove duplicate action buttons (already defined at lines ~541–546).Keeping both copies breaks ARB parsing.
- "cancelPendingButton": "CANCELLAZIONE IN ATTESA", - "acceptCancelButton": "ANNULLA", - "completePurchaseButton": "COMPLETA ACQUISTO", - "rateButton": "VALUTA", - "contactButton": "CONTATTA",
792-792: Fix duplicate key: proofOfWork is already defined earlier (line ~323).- "proofOfWork": "Prova di Lavoro",lib/services/mostro_service.dart (1)
227-228: Avoid logging full outbound payloads (may include invoices/tokens).Log action/id instead of the full JSON body.
- _logger.i('Sending DM, Event ID: ${event.id} with payload: ${order.toJson()}'); + _logger.i('Sending DM. Event ID: ${event.id}, Action: ${order.action}, Target: ${order.id ?? order.requestId}');lib/l10n/intl_es.arb (1)
1-979: Fix JSON syntax error in intl_es.arb
lib/l10n/intl_es.arb line 956, col 26 – parse error “Expected separator between values.” Correct the malformed JSON (likely a missing comma or bracket), then rerun l10n validation and flutter gen-l10n to ensure metadata entries and critical dispute keys are valid across all locales.
♻️ Duplicate comments (6)
lib/l10n/intl_en.arb (2)
861-861: Remove duplicate key: retry already exists (line ~215).- "retry": "Retry",
872-872: Remove duplicate key: orderIdLabel already exists (line ~259).- "orderIdLabel": "Order ID",lib/l10n/intl_it.arb (2)
964-965: Remove duplicate key: retry already exists (line ~215).- "retry": "Riprova",
975-976: Remove duplicate key: orderIdLabel already exists (line ~641).- "orderIdLabel": "ID Ordine",lib/services/mostro_service.dart (1)
106-111: Stop logging decrypted message content (sensitive data).This prints raw private chat/invoices/tokens to logs on parse errors. Log metadata only.
- _logger.e( - 'Error parsing message content: ${decryptedEvent.content}', - error: e, - stackTrace: stackTrace, - ); + _logger.e( + 'Error parsing message content', + error: e, + stackTrace: stackTrace, + );lib/data/repositories/dispute_repository.dart (1)
290-327: Decrypt kind-4 DMs before parsing; current JSON decode will fail.DMs are encrypted; parse only after decryption with session keys.
- // Search regular DMs - for (final event in dmEventsCopy) { - try { - if (event.content != null && event.content!.isNotEmpty) { - final content = jsonDecode(event.content!); + // Search regular DMs (kind 4) — decrypt first (NIP-44) + for (final event in dmEventsCopy) { + for (final session in sessionsCopy) { + try { + final ciphertext = event.content ?? ''; + if (ciphertext.isEmpty) continue; + final decryptedJson = await NostrUtils.decryptNIP44( + ciphertext, + session.tradeKey.private, + _mostroPubkey, + ); + final content = jsonDecode(decryptedJson); if (content is Map<String, dynamic>) { final order = content['order'] as Map<String, dynamic>?; if (order != null) { final payload = order['payload'] as Map<String, dynamic>?; if (payload != null && payload['dispute'] is List) { final disputeArray = payload['dispute'] as List; _logger.d('Checking DM event: disputeArray=$disputeArray, looking for $disputeId'); if (disputeArray.isNotEmpty && disputeArray[0] == disputeId) { foundOrderId = order['id'] as String?; foundAction = order['action'] as String?; // Extract user token from dispute array [disputeId, userToken, peerToken] if (disputeArray.length > 1 && disputeArray[1] != null) { foundToken = disputeArray[1].toString(); } _logger.d('✅ Found orderId from DM: $foundOrderId, action: $foundAction, token: $foundToken'); break; } } else if (payload != null && payload['dispute'] is String && payload['dispute'] == disputeId) { // Handle case where dispute is a string instead of array foundOrderId = order['id'] as String?; foundAction = order['action'] as String?; _logger.d('✅ Found orderId from DM (string format): $foundOrderId, action: $foundAction'); break; } else { _logger.d('DM event payload structure: ${payload?.keys}'); } } } - } - } catch (e) { - _logger.d('Failed to parse DM event: $e'); - } + // success -> stop trying other session keys for this event + break; + } catch (e) { + // try next session key + continue; + } + } }
🧹 Nitpick comments (10)
lib/services/mostro_service.dart (2)
45-52: Null-guard NostrEvent id/createdAt before storing.Defensive check prevents rare NPEs from malformed events.
- if (await eventStore.hasItem(event.id!)) return; + if (event.id == null || event.createdAt == null) { + _logger.w('Skipping event with missing id/createdAt.'); + return; + } + if (await eventStore.hasItem(event.id!)) return;
45-53: Mark events as processed only after successful parse (optional).Currently, events are deduped before parsing; a transient parse error causes permanent drop. Consider two-phase bookkeeping (seen vs processed) or putting only after successful decode/store.
Also applies to: 98-105
lib/features/disputes/widgets/dispute_info_card.dart (1)
70-97: Avoid fixed 80px label width for i18n (may truncate in DE/IT/ES).Consider letting the label flex or using a Table with intrinsic sizing to accommodate longer translations.
lib/features/disputes/widgets/dispute_status_content.dart (1)
83-84: Localize counterparty in bullet text.- _buildBulletPoint(S.of(context)!.disputeInstruction4(dispute.counterpartyDisplay)), + _buildBulletPoint(S.of(context)!.disputeInstruction4(dispute.counterparty ?? S.of(context)!.unknown)),lib/features/disputes/providers/dispute_providers.dart (2)
10-13: Narrow rebuilds withselectwhen reading settings.Only rebuild when Mostro pubkey changes.
- final settings = ref.watch(settingsProvider); - final mostroPubkey = settings.mostroPublicKey; + final mostroPubkey = ref.watch(settingsProvider.select((s) => s.mostroPublicKey));
16-19: Remove no-op onDispose block.Keeps provider minimal until cleanup is needed.
- // Register cleanup when provider is disposed - ref.onDispose(() { - // No explicit cleanup method in DisputeRepository, but we can add null safety - // for future implementations - }); + // No explicit cleanup needed at the moment.lib/features/disputes/widgets/dispute_communication_section.dart (1)
304-311: Don’t expose raw error details to users.Show a friendly message; log details elsewhere.
- Text( - error.toString(), - style: TextStyle( - color: AppTheme.textSecondary, - fontSize: 14, - ), - textAlign: TextAlign.center, - ), + Text( + S.of(context)!.tryAgainLater, + style: TextStyle( + color: AppTheme.textSecondary, + fontSize: 14, + ), + textAlign: TextAlign.center, + ),Note: ensure
tryAgainLaterexists in ARB.lib/data/repositories/dispute_repository.dart (1)
115-124: Potential N+1 event fetches per dispute; consider batching by IDs.Collect disputeIds then fetch a single 38383 query with multiple
#dvalues to reduce latency.lib/data/models/dispute.dart (2)
22-26: Avoid sentinel strings that can surface in UI.
DisputeSemanticKeysvalues may render literally; prefer nulls and handle localization in widgets.
470-475: Mark “Display” getters as deprecated to discourage UI use.Widgets should localize unknowns instead of relying on model placeholders.
- /// Convenience getter for orderId with fallback - String get orderIdDisplay => orderId ?? DisputeSemanticKeys.unknownOrderId; + /// Convenience getter for orderId with fallback + @Deprecated('Avoid in UI; prefer (orderId ?? localizedUnknown) in widgets.') + String get orderIdDisplay => orderId ?? DisputeSemanticKeys.unknownOrderId; - /// Convenience getter for counterparty with fallback - String get counterpartyDisplay => counterparty ?? DisputeSemanticKeys.unknownCounterparty; + /// Convenience getter for counterparty with fallback + @Deprecated('Avoid in UI; prefer (counterparty ?? localizedUnknown) in widgets.') + String get counterpartyDisplay => counterparty ?? DisputeSemanticKeys.unknownCounterparty;
📜 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 ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
lib/core/config.dart(0 hunks)lib/data/models/dispute.dart(2 hunks)lib/data/models/dispute_event.dart(1 hunks)lib/data/repositories/dispute_repository.dart(1 hunks)lib/features/disputes/notifiers/dispute_chat_notifier.dart(1 hunks)lib/features/disputes/providers/dispute_providers.dart(1 hunks)lib/features/disputes/widgets/dispute_communication_section.dart(1 hunks)lib/features/disputes/widgets/dispute_content.dart(1 hunks)lib/features/disputes/widgets/dispute_description.dart(1 hunks)lib/features/disputes/widgets/dispute_info_card.dart(1 hunks)lib/features/disputes/widgets/dispute_status_badge.dart(1 hunks)lib/features/disputes/widgets/dispute_status_content.dart(1 hunks)lib/l10n/intl_en.arb(3 hunks)lib/l10n/intl_es.arb(4 hunks)lib/l10n/intl_it.arb(4 hunks)lib/services/dispute_service.dart(1 hunks)lib/services/mostro_service.dart(1 hunks)pubspec.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- lib/core/config.dart
🚧 Files skipped from review as they are similar to previous changes (6)
- pubspec.yaml
- lib/services/dispute_service.dart
- lib/features/disputes/widgets/dispute_description.dart
- lib/features/disputes/widgets/dispute_content.dart
- lib/data/models/dispute_event.dart
- lib/features/disputes/notifiers/dispute_chat_notifier.dart
🧰 Additional context used
📓 Path-based instructions (5)
lib/data/repositories/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Use the Repository pattern for all data access in data/repositories/
Files:
lib/data/repositories/dispute_repository.dart
lib/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.dart: Use S.of(context) for all user-facing strings (no hardcoded UI text)
Target zero flutter analyze issues; remove unused imports and use latest non-deprecated APIs (e.g., withValues() over withOpacity())
Check mounted before using BuildContext after async gaps
Prefer const constructors and const widgets where possible
Files:
lib/data/repositories/dispute_repository.dartlib/features/disputes/widgets/dispute_status_badge.dartlib/features/disputes/widgets/dispute_info_card.dartlib/features/disputes/widgets/dispute_status_content.dartlib/features/disputes/widgets/dispute_communication_section.dartlib/features/disputes/providers/dispute_providers.dartlib/data/models/dispute.dartlib/services/mostro_service.dart
lib/features/**/providers/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under features/{feature}/providers/
Files:
lib/features/disputes/providers/dispute_providers.dart
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Add new localization keys to all ARB files (en, es, it)
Files:
lib/l10n/intl_es.arblib/l10n/intl_it.arblib/l10n/intl_en.arb
lib/l10n/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
After modifying ARB files, regenerate localization (flutter gen-l10n or build_runner as configured)
Files:
lib/l10n/intl_es.arblib/l10n/intl_it.arblib/l10n/intl_en.arb
🧠 Learnings (5)
📚 Learning: 2025-08-24T16:01:53.137Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T16:01:53.137Z
Learning: Applies to lib/data/repositories/**/*.dart : Use the Repository pattern for all data access in data/repositories/
Applied to files:
lib/data/repositories/dispute_repository.dart
📚 Learning: 2025-08-24T16:01:53.137Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T16:01:53.137Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature under features/{feature}/providers/
Applied to files:
lib/features/disputes/providers/dispute_providers.dart
📚 Learning: 2025-08-21T14:45:43.974Z
Learnt from: Catrya
PR: MostroP2P/mobile#272
File: lib/features/relays/widgets/relay_selector.dart:13-15
Timestamp: 2025-08-21T14:45:43.974Z
Learning: In the Mostro mobile app's RelaySelector widget (lib/features/relays/widgets/relay_selector.dart), watching relaysProvider.notifier correctly triggers rebuilds because the relaysProvider itself depends on settingsProvider (line 8 in relays_provider.dart). When blacklist changes via toggleMostroRelayBlacklist(), the settingsProvider updates, causing relaysProvider to rebuild, which then notifies widgets watching the notifier. The UI correctly reflects active/inactive states in real-time through this dependency chain.
Applied to files:
lib/features/disputes/providers/dispute_providers.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Applied to files:
lib/features/disputes/providers/dispute_providers.dart
📚 Learning: 2025-08-24T16:01:53.137Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T16:01:53.137Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all ARB files (en, es, it)
Applied to files:
lib/l10n/intl_it.arb
🪛 GitHub Actions: Flutter
lib/l10n/intl_es.arb
[error] 956-956: Flutter ARB parsing failed: Unexpected character at line 956 (intl_es.arb). The metadata entry '@_comment_relay_status' appears to be a string instead of a metadata object.
🪛 RuboCop (1.76.1)
lib/l10n/intl_en.arb
[warning] 861-861: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 872-872: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
🔇 Additional comments (13)
lib/l10n/intl_en.arb (1)
651-683: LGTM: Dispute translations and placeholders are consistent.New keys and placeholder metadata align with usage in the dispute UI (status labels, info, communication, input, list, and screen). No issues spotted in these blocks.
Also applies to: 703-713, 833-847, 849-857, 859-862, 864-876, 877-884, 949-954, 955-962
lib/l10n/intl_it.arb (2)
711-740: LGTM: Dispute translations (IT) and placeholders look correct.The dispute status set and communication/input/info/screen strings are coherent and match EN.
Also applies to: 936-987
990-1012: Regenerate localization and validate cross-locale parity.After removing duplicates, ensure IT/ES have all EN keys (esp. dispute keys and forAmountWithCurrency).
Use the script provided in the EN comment to:
- detect duplicate keys in EN/IT/ES,
- compare key sets (EN → IT/ES),
- then run flutter gen-l10n locally.
lib/services/mostro_service.dart (1)
64-105: Robust validation improvements — nice work.Empty-content guard, structured JSON checks, and peer pubkey validation reduce parse-time exceptions and bad data ingestion.
lib/features/disputes/widgets/dispute_status_badge.dart (1)
1-90: LGTM: Normalization + “solved” alias handled correctly.Mappings use AppTheme.* with withValues(alpha) and S.of(context) keys. Matches PR’s status plan and guards variants (“in progress”, “in_progress”, etc.).
lib/features/disputes/widgets/dispute_info_card.dart (1)
10-68: LGTM: Clear, localized dispute header and IDs, with proper provider usage.Good use of S.of(context) for all labels and DisputeStatusBadge integration.
lib/features/disputes/widgets/dispute_status_content.dart (1)
128-131: Remove hardcoded English via model’sdescription; use i18n only.
dispute.descriptionis English. Show a localized message instead.- return "${S.of(context)!.disputeStatusInProgress}: ${dispute.description}"; + return S.of(context)!.disputeStatusInProgress;Likely an incorrect or invalid review comment.
lib/features/disputes/providers/dispute_providers.dart (1)
8-22: Repository provider: reactive, scoped, and disposable — LGTM.lib/features/disputes/widgets/dispute_communication_section.dart (1)
215-277: Fixed unbounded layout by using a fixed-height SizedBox — nice.lib/data/models/dispute.dart (1)
145-176: Event parsing and timestamp extraction — solid.Tag extraction and ms conversion logic looks robust.
lib/l10n/intl_es.arb (3)
682-689: Dispute status strings look good.Labels are concise and consistent with the status badge system.
946-953: Nice: dedicated dispute screen error strings.Placeholders and wording LGTM.
731-741: Remove deprecatedforAmountkey in intl_es.arb
No occurrences offorAmountremain in Dart code; all calls now useforAmountWithCurrency. You can safely delete the old key fromlib/l10n/intl_es.arb.
| /// Backward compatibility getter for description | ||
| String get description { | ||
| switch (descriptionKey) { | ||
| case DisputeDescriptionKey.initiatedByUser: | ||
| return 'You opened this dispute'; | ||
| case DisputeDescriptionKey.initiatedByPeer: | ||
| return 'A dispute was opened against you'; | ||
| case DisputeDescriptionKey.inProgress: | ||
| return 'Dispute is being reviewed by an admin'; | ||
| case DisputeDescriptionKey.resolved: | ||
| return 'Dispute has been resolved'; | ||
| case DisputeDescriptionKey.sellerRefunded: | ||
| return 'Dispute resolved - seller refunded'; | ||
| case DisputeDescriptionKey.unknown: | ||
| return 'Unknown status'; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Deprecate hardcoded English description to enforce i18n in UI.
Prevents accidental UI leakage; use descriptionKey in widgets.
- /// Backward compatibility getter for description
- String get description {
+ /// Backward compatibility getter for description
+ @Deprecated('Do not use in UI; resolve `descriptionKey` to localized strings in widgets.')
+ String get description {📝 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.
| /// Backward compatibility getter for description | |
| String get description { | |
| switch (descriptionKey) { | |
| case DisputeDescriptionKey.initiatedByUser: | |
| return 'You opened this dispute'; | |
| case DisputeDescriptionKey.initiatedByPeer: | |
| return 'A dispute was opened against you'; | |
| case DisputeDescriptionKey.inProgress: | |
| return 'Dispute is being reviewed by an admin'; | |
| case DisputeDescriptionKey.resolved: | |
| return 'Dispute has been resolved'; | |
| case DisputeDescriptionKey.sellerRefunded: | |
| return 'Dispute resolved - seller refunded'; | |
| case DisputeDescriptionKey.unknown: | |
| return 'Unknown status'; | |
| } | |
| } | |
| /// Backward compatibility getter for description | |
| @Deprecated('Do not use in UI; resolve `descriptionKey` to localized strings in widgets.') | |
| String get description { | |
| switch (descriptionKey) { | |
| case DisputeDescriptionKey.initiatedByUser: | |
| return 'You opened this dispute'; | |
| case DisputeDescriptionKey.initiatedByPeer: | |
| return 'A dispute was opened against you'; | |
| case DisputeDescriptionKey.inProgress: | |
| return 'Dispute is being reviewed by an admin'; | |
| case DisputeDescriptionKey.resolved: | |
| return 'Dispute has been resolved'; | |
| case DisputeDescriptionKey.sellerRefunded: | |
| return 'Dispute resolved - seller refunded'; | |
| case DisputeDescriptionKey.unknown: | |
| return 'Unknown status'; | |
| } | |
| } |
🤖 Prompt for AI Agents
In lib/data/models/dispute.dart around lines 449 to 465, remove the hardcoded
English descriptions in the description getter and deprecate it so UI code must
use descriptionKey for i18n; replace the switch body with a simple deprecated
implementation that returns a non-localized representation (e.g.
descriptionKey.toString() or descriptionKey.name) and add an @Deprecated
annotation and a short doc comment telling callers to use descriptionKey for
localization in widgets.
| // Look for dispute-related messages in stored decrypted messages | ||
| for (final message in allMessages) { | ||
|
|
||
| // Look for dispute-related actions | ||
| if (message.action.toString().contains('dispute')) { | ||
| final action = message.action.toString(); | ||
| final orderId = message.id; | ||
|
|
||
| if (action == 'dispute-initiated-by-you' || action == 'dispute-initiated-by-peer') { | ||
| // Extract dispute data from payload | ||
| if (message.payload is Dispute) { | ||
| final dispute = message.payload as Dispute; | ||
| final status = await _getDisputeStatus(dispute.disputeId); | ||
|
|
||
| disputeMap[dispute.disputeId] = dispute.copyWith( | ||
| orderId: orderId, | ||
| action: action, | ||
| status: status ?? 'initiated', | ||
| ); | ||
| } | ||
| } else if (action == 'admin-took-dispute') { | ||
| // Update existing disputes with admin info | ||
| if (message.payload is Peer) { | ||
| final peer = message.payload as Peer; | ||
|
|
||
| // Find existing dispute for this order and update with admin info | ||
| disputeMap.forEach((disputeId, dispute) { | ||
| if (dispute.orderId == orderId) { | ||
| disputeMap[disputeId] = dispute.copyWith( | ||
| adminPubkey: peer.publicKey, | ||
| adminTookAt: DateTime.now(), | ||
| status: 'in-progress', | ||
| ); | ||
| } | ||
| }); | ||
| } | ||
| } else if (action == 'admin-settled') { | ||
| // Update existing disputes when admin resolves them | ||
| disputeMap.forEach((disputeId, dispute) { | ||
| if (dispute.orderId == orderId) { | ||
| disputeMap[disputeId] = dispute.copyWith( | ||
| status: 'resolved', | ||
| ); | ||
| } | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
‘admin-settled’ never reached due to outer contains('dispute') guard.
This skips resolved updates. Switch to explicit action handling.
- // Look for dispute-related messages in stored decrypted messages
- for (final message in allMessages) {
-
- // Look for dispute-related actions
- if (message.action.toString().contains('dispute')) {
- final action = message.action.toString();
- final orderId = message.id;
-
- if (action == 'dispute-initiated-by-you' || action == 'dispute-initiated-by-peer') {
+ // Look for dispute-related messages in stored decrypted messages
+ for (final message in allMessages) {
+ final action = message.action.toString();
+ final orderId = message.id;
+
+ if (action == 'dispute-initiated-by-you' || action == 'dispute-initiated-by-peer') {
// Extract dispute data from payload
if (message.payload is Dispute) {
final dispute = message.payload as Dispute;
final status = await _getDisputeStatus(dispute.disputeId);
disputeMap[dispute.disputeId] = dispute.copyWith(
orderId: orderId,
action: action,
status: status ?? 'initiated',
);
}
- } else if (action == 'admin-took-dispute') {
+ } else if (action == 'admin-took-dispute') {
// Update existing disputes with admin info
if (message.payload is Peer) {
final peer = message.payload as Peer;
// Find existing dispute for this order and update with admin info
disputeMap.forEach((disputeId, dispute) {
if (dispute.orderId == orderId) {
disputeMap[disputeId] = dispute.copyWith(
adminPubkey: peer.publicKey,
adminTookAt: DateTime.now(),
status: 'in-progress',
);
}
});
}
- } else if (action == 'admin-settled') {
+ } else if (action == 'admin-settled') {
// Update existing disputes when admin resolves them
disputeMap.forEach((disputeId, dispute) {
if (dispute.orderId == orderId) {
disputeMap[disputeId] = dispute.copyWith(
status: 'resolved',
);
}
});
- }
- }
+ }
}📝 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.
| // Look for dispute-related messages in stored decrypted messages | |
| for (final message in allMessages) { | |
| // Look for dispute-related actions | |
| if (message.action.toString().contains('dispute')) { | |
| final action = message.action.toString(); | |
| final orderId = message.id; | |
| if (action == 'dispute-initiated-by-you' || action == 'dispute-initiated-by-peer') { | |
| // Extract dispute data from payload | |
| if (message.payload is Dispute) { | |
| final dispute = message.payload as Dispute; | |
| final status = await _getDisputeStatus(dispute.disputeId); | |
| disputeMap[dispute.disputeId] = dispute.copyWith( | |
| orderId: orderId, | |
| action: action, | |
| status: status ?? 'initiated', | |
| ); | |
| } | |
| } else if (action == 'admin-took-dispute') { | |
| // Update existing disputes with admin info | |
| if (message.payload is Peer) { | |
| final peer = message.payload as Peer; | |
| // Find existing dispute for this order and update with admin info | |
| disputeMap.forEach((disputeId, dispute) { | |
| if (dispute.orderId == orderId) { | |
| disputeMap[disputeId] = dispute.copyWith( | |
| adminPubkey: peer.publicKey, | |
| adminTookAt: DateTime.now(), | |
| status: 'in-progress', | |
| ); | |
| } | |
| }); | |
| } | |
| } else if (action == 'admin-settled') { | |
| // Update existing disputes when admin resolves them | |
| disputeMap.forEach((disputeId, dispute) { | |
| if (dispute.orderId == orderId) { | |
| disputeMap[disputeId] = dispute.copyWith( | |
| status: 'resolved', | |
| ); | |
| } | |
| }); | |
| } | |
| } | |
| // Look for dispute-related messages in stored decrypted messages | |
| for (final message in allMessages) { | |
| final action = message.action.toString(); | |
| final orderId = message.id; | |
| if (action == 'dispute-initiated-by-you' || action == 'dispute-initiated-by-peer') { | |
| // Extract dispute data from payload | |
| if (message.payload is Dispute) { | |
| final dispute = message.payload as Dispute; | |
| final status = await _getDisputeStatus(dispute.disputeId); | |
| disputeMap[dispute.disputeId] = dispute.copyWith( | |
| orderId: orderId, | |
| action: action, | |
| status: status ?? 'initiated', | |
| ); | |
| } | |
| } else if (action == 'admin-took-dispute') { | |
| // Update existing disputes with admin info | |
| if (message.payload is Peer) { | |
| final peer = message.payload as Peer; | |
| // Find existing dispute for this order and update with admin info | |
| disputeMap.forEach((disputeId, dispute) { | |
| if (dispute.orderId == orderId) { | |
| disputeMap[disputeId] = dispute.copyWith( | |
| adminPubkey: peer.publicKey, | |
| adminTookAt: DateTime.now(), | |
| status: 'in-progress', | |
| ); | |
| } | |
| }); | |
| } | |
| } else if (action == 'admin-settled') { | |
| // Update existing disputes when admin resolves them | |
| disputeMap.forEach((disputeId, dispute) { | |
| if (dispute.orderId == orderId) { | |
| disputeMap[disputeId] = dispute.copyWith( | |
| status: 'resolved', | |
| ); | |
| } | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In lib/data/repositories/dispute_repository.dart around lines 66 to 112, the
outer guard if (message.action.toString().contains('dispute')) prevents branches
like 'admin-settled' from ever running; replace the broad contains check with
explicit action handling (e.g., compute action = message.action.toString() and
either remove the outer guard and use direct if/else if checks for
'dispute-initiated-by-you', 'dispute-initiated-by-peer', 'admin-took-dispute',
'admin-settled' etc., or change the outer condition to check against a set/list
of allowed actions) so the 'admin-settled' branch is reachable and resolved
updates are applied.
| Text( | ||
| '${S.of(context)!.admin}: ${adminPubkey.substring(0, 16)}...', | ||
| style: TextStyle( | ||
| color: AppTheme.textSecondary, | ||
| fontSize: 14, | ||
| ), | ||
| textAlign: TextAlign.center, |
There was a problem hiding this comment.
Avoid substring range error on short pubkeys.
Use a safe truncation.
- Text(
- '${S.of(context)!.admin}: ${adminPubkey.substring(0, 16)}...',
+ Text(
+ '${S.of(context)!.admin}: ${adminPubkey.length <= 16 ? adminPubkey : '${adminPubkey.substring(0, 16)}...'}',📝 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.
| Text( | |
| '${S.of(context)!.admin}: ${adminPubkey.substring(0, 16)}...', | |
| style: TextStyle( | |
| color: AppTheme.textSecondary, | |
| fontSize: 14, | |
| ), | |
| textAlign: TextAlign.center, | |
| Text( | |
| '${S.of(context)!.admin}: ${adminPubkey.length <= 16 ? adminPubkey : '${adminPubkey.substring(0, 16)}...'}', | |
| style: TextStyle( | |
| color: AppTheme.textSecondary, | |
| fontSize: 14, | |
| ), | |
| textAlign: TextAlign.center, |
🤖 Prompt for AI Agents
In lib/features/disputes/widgets/dispute_communication_section.dart around lines
153 to 159 the code uses adminPubkey.substring(0, 16) which will throw if the
pubkey is shorter than 16 characters; replace the direct substring with a safe
truncation that checks the string length (e.g., take up to min(16,
adminPubkey.length)) and append "..." only when truncation occurred, or use a
small helper/extension to ellipsize the pubkey so it never attempts an
out-of-range substring.
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| bool isResolved = dispute.status.toLowerCase() == 'resolved' || dispute.status.toLowerCase() == 'closed'; |
There was a problem hiding this comment.
Fix resolved-status detection (use 'solved', not 'closed').
Current check misses “solved” (used elsewhere in the PR) and includes “closed” which may not be emitted. Guard against both to be safe.
- bool isResolved = dispute.status.toLowerCase() == 'resolved' || dispute.status.toLowerCase() == 'closed';
+ final status = dispute.status.toLowerCase();
+ final isResolved = status == 'resolved' || status == 'solved' || status == 'closed';📝 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.
| bool isResolved = dispute.status.toLowerCase() == 'resolved' || dispute.status.toLowerCase() == 'closed'; | |
| final status = dispute.status.toLowerCase(); | |
| final isResolved = status == 'resolved' || status == 'solved' || status == 'closed'; |
🤖 Prompt for AI Agents
In lib/features/disputes/widgets/dispute_status_content.dart around line 16, the
boolean uses the wrong tokens to detect a resolved dispute: it checks for
'resolved' or 'closed' but the codebase uses 'solved' elsewhere and 'closed' may
not be emitted. Replace the logic to normalize the status once (e.g., lowercased
trimmed string) and set isResolved to true if the normalized status equals
'resolved' or 'solved' (you may optionally also include 'closed' as a fallback)
so all expected variants are handled.
| return S.of(context)!.disputeOpenedByYou(dispute.counterpartyDisplay); | ||
| case DisputeDescriptionKey.initiatedByPeer: | ||
| return S.of(context)!.disputeOpenedAgainstYou(dispute.counterpartyDisplay); | ||
| case DisputeDescriptionKey.inProgress: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Localize counterparty; don’t pass model placeholders to UI.
Avoid using counterpartyDisplay (may be a sentinel like unknownCounterparty). Prefer localized fallback.
- return S.of(context)!.disputeOpenedByYou(dispute.counterpartyDisplay);
+ return S.of(context)!.disputeOpenedByYou(dispute.counterparty ?? S.of(context)!.unknown);- return S.of(context)!.disputeOpenedAgainstYou(dispute.counterpartyDisplay);
+ return S.of(context)!.disputeOpenedAgainstYou(dispute.counterparty ?? S.of(context)!.unknown);📝 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.
| return S.of(context)!.disputeOpenedByYou(dispute.counterpartyDisplay); | |
| case DisputeDescriptionKey.initiatedByPeer: | |
| return S.of(context)!.disputeOpenedAgainstYou(dispute.counterpartyDisplay); | |
| case DisputeDescriptionKey.inProgress: | |
| return S.of(context)!.disputeOpenedByYou(dispute.counterparty ?? S.of(context)!.unknown); | |
| case DisputeDescriptionKey.initiatedByPeer: | |
| return S.of(context)!.disputeOpenedAgainstYou(dispute.counterparty ?? S.of(context)!.unknown); | |
| case DisputeDescriptionKey.inProgress: |
🤖 Prompt for AI Agents
In lib/features/disputes/widgets/dispute_status_content.dart around lines
124-127, the code currently passes dispute.counterpartyDisplay directly into the
localized message which can be a sentinel like "unknownCounterparty"; instead,
detect if dispute.counterpartyDisplay is null/empty or equals the sentinel
constant and in that case pass a localized fallback (e.g.
S.of(context)!.unknownCounterparty or an appropriate localized label), otherwise
pass the actual counterparty name; update both disputeOpenedByYou and
disputeOpenedAgainstYou calls to use this resolved, localized counterparty
string so the UI never receives raw model placeholders.
| "@disputeOpenedByYou": { | ||
| "placeholders": { | ||
| "counterparty": { "type": "String" } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t hardcode the counterparty role in “disputeOpenedByYou”.
It says “contra el comprador …” even when the counterparty is the seller. Make it neutral.
- "disputeOpenedByYou": "Abriste esta disputa contra el comprador {counterparty}, lee atentamente a continuación:",
+ "disputeOpenedByYou": "Abriste esta disputa contra {counterparty}, lee atentamente a continuación:",📝 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.
| "@disputeOpenedByYou": { | |
| "placeholders": { | |
| "counterparty": { "type": "String" } | |
| } | |
| }, | |
| "disputeOpenedByYou": "Abriste esta disputa contra {counterparty}, lee atentamente a continuación:", | |
| "@disputeOpenedByYou": { | |
| "placeholders": { | |
| "counterparty": { "type": "String" } | |
| } | |
| }, |
🤖 Prompt for AI Agents
In lib/l10n/intl_es.arb around lines 692 to 696, the "disputeOpenedByYou"
message hardcodes "comprador" which incorrectly labels the counterparty when
they are the seller; change the message text to a neutral formulation that
either injects the existing "counterparty" placeholder (e.g., use the
placeholder token where the role should appear) or use a role-neutral phrase
such as "la otra parte" so the string no longer assumes "comprador"; ensure the
placeholder name matches the "counterparty" placeholder defined and adjust
surrounding grammar to remain correct for Spanish.
| "orderCanceled": "La orden fue cancelada", | ||
|
|
||
|
|
||
| "@_comment_dispute_communication": "Texto de Comunicación de Disputas", |
There was a problem hiding this comment.
Prevent future ARB parsing errors: turn newly added “@comment*” strings into metadata objects.
These will be treated as invalid metadata if left as strings. Use objects with a description to keep them as non-translatable notes.
- "@_comment_dispute_communication": "Texto de Comunicación de Disputas",
+ "@_comment_dispute_communication": { "description": "Texto de Comunicación de Disputas" },
- "@_comment_dispute_input": "Texto de Entrada de Disputas",
+ "@_comment_dispute_input": { "description": "Texto de Entrada de Disputas" },
- "@_comment_dispute_list": "Texto de Lista de Disputas",
+ "@_comment_dispute_list": { "description": "Texto de Lista de Disputas" },
- "@_comment_dispute_info": "Texto de Información de Disputas",
+ "@_comment_dispute_info": { "description": "Texto de Información de Disputas" },
- "@_comment_dispute_screen": "Texto de Pantalla de Disputas",
+ "@_comment_dispute_screen": { "description": "Texto de Pantalla de Disputas" },Also applies to: 917-917, 927-927, 932-932, 946-946
🤖 Prompt for AI Agents
In lib/l10n/intl_es.arb around lines 903, 917, 927, 932, and 946, several
entries like "@_comment_dispute_communication" are currently plain strings which
will be treated as invalid metadata by ARB parsers; replace each of these
string-valued keys with proper metadata objects (e.g., change the value from a
string to an object containing a "description" field) so they remain
non-translatable notes and prevent ARB parsing errors.
| "retry": "Reintentar", | ||
|
|
There was a problem hiding this comment.
Remove duplicate keys (“retry”, “orderIdLabel”).
These keys already exist earlier in the file; duplicates can cause surprises during codegen and at runtime (last one wins).
- "retry": "Reintentar",
…
- "orderIdLabel": "ID de Orden",Also applies to: 941-941
🤖 Prompt for AI Agents
In lib/l10n/intl_es.arb around lines 930-931 (and similarly at line 941), there
are duplicate localization keys ("retry" and "orderIdLabel") that override
earlier entries; remove the later duplicate entries so each key appears only
once (keep the original translations defined earlier) and run the
localization/codegen step to ensure no duplicates remain.
| } | ||
|
|
||
|
|
||
| "@_comment_relay_status": "Mensajes de estado de relays", |
There was a problem hiding this comment.
Fix CI: metadata entries starting with @ must be objects (not strings).
Flutter gen-l10n fails on this line. Convert the comment to a metadata object.
- "@_comment_relay_status": "Mensajes de estado de relays",
+ "@_comment_relay_status": { "description": "Mensajes de estado de relays" },📝 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.
| "@_comment_relay_status": "Mensajes de estado de relays", | |
| "@_comment_relay_status": { "description": "Mensajes de estado de relays" }, |
🧰 Tools
🪛 RuboCop (1.76.1)
[fatal] 956-956: unexpected token tSTRING
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)
(Lint/Syntax)
🪛 GitHub Actions: Flutter
[error] 956-956: Flutter ARB parsing failed: Unexpected character at line 956 (intl_es.arb). The metadata entry '@_comment_relay_status' appears to be a string instead of a metadata object.
🤖 Prompt for AI Agents
In lib/l10n/intl_es.arb around line 956, the metadata entry starting with '@' is
currently a string which breaks Flutter gen-l10n; change the value from a string
to a JSON object containing at least a description field (for example:
"@_comment_relay_status": {"description": "Mensajes de estado de relays"}) and
ensure surrounding commas and JSON syntax remain valid so the metadata key is an
object rather than a plain string.
| "forAmount": "per {amount}", | ||
| "forAmountWithCurrency": "per {amount} {currency}", | ||
| "@forAmountWithCurrency": { |
There was a problem hiding this comment.
Remove duplicate key: forAmount defined earlier (line ~619).
This later copy conflicts; keep only one “forAmount” plus the new “forAmountWithCurrency”.
- "forAmount": "per {amount}",📝 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.
| "forAmount": "per {amount}", | |
| "forAmountWithCurrency": "per {amount} {currency}", | |
| "@forAmountWithCurrency": { | |
| "forAmountWithCurrency": "per {amount} {currency}", | |
| "@forAmountWithCurrency": { |
🤖 Prompt for AI Agents
In lib/l10n/intl_it.arb around lines 760 to 762, there is a duplicate
"forAmount" key (already defined earlier ~line 619); remove the duplicated
"forAmount": "per {amount}" entry at these lines and keep the new
"forAmountWithCurrency" and its metadata (@forAmountWithCurrency) intact so the
file contains only one "forAmount" definition plus the new
"forAmountWithCurrency" entry.
PR Description: Implement Mobile App Dispute System
Summary
This PR implements a comprehensive dispute system for the MostroP2P mobile app, allowing users to create, view, and manage disputes with proper order ID tracking and status visualization.
Features Implemented
Fetch kind 38383 events with z=dispute tags from Mostro
Extract disputeId from d tag and status from s tag
Deduplicate by keeping latest event per dispute ID
Created _resolveOrderIdForDispute() method that searches DM events
Looks for payload.dispute array where first element matches disputeId
Returns corresponding order.id from the DM
Handles cases where order ID cannot be resolved with "Unknown Order ID" fallback
"initiated": Yellow (statusPendingBackground/Text)
"in-progress": Green (statusSuccessBackground/Text)
"resolved"/"solved": Blue (statusSettledBackground/Text)
Added "disputeStatusInitiated" strings to all language files (en, es, it)
Full i18n support for dispute status labels
Files Added/Modified
/lib/data/repositories/dispute_repository.dart - Core dispute data handling
/lib/features/disputes/widgets/dispute_status_badge.dart - Status visualization
/lib/features/disputes/widgets/dispute_order_id.dart - Order ID display component
/lib/l10n/intl_*.arb - Localization strings
User Experience
Users can view all their disputes in a dedicated list
Each dispute shows the correct order ID and current status
Status colors provide clear visual feedback
Chat functionality available for in-progress disputes
Graceful handling of edge cases (missing order IDs)
Important
There may be bugs with disputes; for any UI bug or error, please open a new issue because the PR is already very large.
Summary by CodeRabbit
New Features
Improvements
Chores