Skip to content

Conversation

@AndreaDiazCorreia
Copy link
Member

@AndreaDiazCorreia AndreaDiazCorreia commented Sep 19, 2025

This PR addresses issues in the dispute status display system and implements a complete localization framework for all dispute-related messages. The main problem was that disputes initiated by peers showed "Unknown status" instead of meaningful descriptions due to timestamp filtering preventing proper state management. Beyond fixing this core issue, we've completely redesigned how dispute statuses are handled throughout the application. The solution includes:
(1) Timestamp override mechanism ensuring all dispute actions are processed regardless of message age
(2) Enhanced payload parsing for robust dispute object creation from various message formats
(3) Comprehensive localization system supporting English, Spanish, and Italian with semantic keys for all dispute states (initiated, pending admin, in progress, resolved, etc.)
(4) Removal of all hardcoded status messages in favor of the new getLocalizedDescription(context) method
(5) Consistent status mapping across the entire dispute lifecycle.

This ensures users receive clear, localized feedback about dispute states whether they initiated the dispute or received one, creating a unified and accessible experience across all supported languages.

Summary by CodeRabbit

  • New Features

    • Locale-aware dispute descriptions with a new "Waiting for admin" state and context-aware admin/settled/seller-refunded messages.
    • Disputes view now shows initiator, order ID and counterparty more reliably.
  • Improvements

    • Dispute list/details load live backend data for faster, accurate views.
    • "Seller refunded" treated and styled as resolved.
    • Better handling/logging of incoming dispute events and stale message gating.
  • Localization

    • Added/updated dispute strings in English, Spanish, and Italian.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 19, 2025

Walkthrough

Adds an action field and action-parsing to dispute models; makes dispute description selection admin- and order-aware and locale-dependent; switches dispute providers from mock data to repository-backed data enriched with Session/OrderState; updates notifiers and OrderState to persist and handle admin/peer dispute events with timestamp gating.

Changes

Cohort / File(s) Summary
Dispute model & view-model
lib/data/models/dispute.dart
Add action to Dispute and DisputeData; add DisputeDescriptionKey.initiatedPendingAdmin; new action parsing helpers; replace description with getLocalizedDescription(BuildContext); add DisputeData.fromDispute(…, {OrderState? orderState}) and fromDisputeEvent; add UI getters orderIdDisplay/counterpartyDisplay; description selection now considers hasAdmin and order context.
Dispute providers (repo-backed)
lib/features/disputes/providers/dispute_providers.dart
Replace mock-driven providers with repository calls (getDispute, getUserDisputes); remove local action derivation; add userDisputeDataProvider mapping DisputeDisputeData, enriching with Session and OrderState when available and watching session notifier for invalidation.
Dispute widgets
lib/features/disputes/widgets/dispute_content.dart
lib/features/disputes/widgets/dispute_status_badge.dart
lib/features/disputes/widgets/dispute_status_content.dart
Use dispute.getLocalizedDescription(context); treat seller-refunded as resolved in badge styling/text; add handling for initiatedPendingAdmin; adjust resolved/seller-refunded/admin-settled messaging branches.
Order state & notifiers
lib/features/order/models/order_state.dart
lib/features/order/notfiers/abstract_mostro_notifier.dart
lib/features/order/notfiers/order_notifier.dart
Extend handleEvent(..., {bool bypassTimestampGate = false}); add timestamp gating and debug logging; bypass gating for dispute-init events while still persisting constructed disputes (ensure orderId/status/action defaults); update dispute fields for admin actions.
Localization
lib/l10n/intl_en.arb
lib/l10n/intl_es.arb
lib/l10n/intl_it.arb
Add dispute description keys (initiatedByUser/Peer/InitiatedPendingAdmin/inProgress/resolved/sellerRefunded/unknown), admin outcome messages, and status strings (waitingForAdmin/youOpened/openedAgainstUser/resolved); minor ES wording fix and added dispute UX strings.
Tests / Mocks
test/mocks.mocks.dart
Update mock handleEvent signature to accept bypassTimestampGate named arg and forward it in noSuchMethod invocation.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant UI as Disputes UI
  participant Prov as dispute_providers
  participant Repo as DisputeRepository
  participant Sess as SessionNotifier
  participant Ord as OrderNotifier

  U->>UI: open disputes screen
  UI->>Prov: read(userDisputeDataProvider)
  Prov->>Repo: getUserDisputes()
  Repo-->>Prov: List<Dispute>
  Prov->>Sess: watch(sessionNotifierProvider)
  Prov->>Ord: read(order_notifier_provider)
  Prov->>Prov: map each Dispute -> DisputeData (attach OrderState when found)
  Prov-->>UI: List<DisputeData>
  UI->>UI: render items using getLocalizedDescription(context)
Loading
sequenceDiagram
  autonumber
  participant Net as Network
  participant Notif as AbstractMostroNotifier
  participant State as OrderState
  participant UI as App UI

  Net-->>Notif: MostroMessage(action, ts, payload)
  Notif->>Notif: compute isRecent := now - ts < 60s
  alt recent OR bypass for dispute-init
    Notif->>Notif: handleEvent(..., bypassTimestampGate)
    alt disputeInitiatedByYou/Peer
      Notif->>Notif: ensure Dispute (set orderId/status/action defaults)
      Notif->>State: persist updated dispute
      Notif-->>UI: emit notification(with dispute_id)
    else admin actions (took/settled/canceled)
      Notif->>State: update dispute.status/action/admin fields
    end
  else old and not dispute-init
    Notif->>Notif: log and ignore
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Catrya
  • grunch

Poem

I thump my nose and hop around,
From mocks to repo truth I'm bound.
Admins arrive, messages hum,
Badges glow and strings become.
A rabbit cheers — disputes well found! 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately captures the PR’s primary intent: a broad overhaul of dispute status handling paired with full localization support. It directly reflects the main changes in the diff (status mapping, timestamp handling, dispute parsing, and added localized strings/getLocalizedDescription). The wording is clear, specific, and reviewer-friendly without extraneous details.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch andrea/dispute-status

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
lib/data/models/dispute.dart (2)

343-354: Creator detection compares enum.toString() to a hyphenated string — likely always false

orderState.action.toString() will be like 'Action.disputeInitiatedByYou', not 'dispute-initiated-by-you'. This will misclassify creator and skew description keys.

Apply this diff to reliably detect creator from Action enums and fall back to string value:

-    if (orderState != null) {
-      // Use OrderState action which has the correct dispute initiation info
-      final actionString = orderState.action.toString();
-      isUserCreator = actionString == 'dispute-initiated-by-you';
-    } else if (dispute.action != null) {
+    if (orderState != null) {
+      final a = orderState.action;
+      // Support both enum and string forms
+      final actionName = a is Enum ? a.name : a.toString();
+      isUserCreator = actionName == 'disputeInitiatedByYou' || actionName == 'dispute-initiated-by-you';
+    } else if (dispute.action != null) {
       // Fallback to dispute action - this should now be set correctly
       isUserCreator = dispute.action == 'dispute-initiated-by-you';
     } else {
       // If no action info is available, leave as null (unknown state)
       // This removes the assumption that user is creator by default
       isUserCreator = null;
     }

438-462: Bug: peer‑initiated disputes mapped to “pending admin” instead of “initiated by peer”

When status == 'initiated' and isUserCreator == false, this should return initiatedByPeer, not initiatedPendingAdmin.

Apply this diff:

   static DisputeDescriptionKey _getDescriptionKeyForStatus(String status, bool? isUserCreator, {bool hasAdmin = false}) {
     switch (status.toLowerCase()) {
       case 'initiated':
         // If admin has been assigned, it's in progress even if status says initiated
         if (hasAdmin) {
           return DisputeDescriptionKey.inProgress;
         }
         if (isUserCreator == null) {
           return DisputeDescriptionKey.initiatedPendingAdmin; // Waiting for admin assignment
         }
-        return isUserCreator
-          ? DisputeDescriptionKey.initiatedByUser
-          : DisputeDescriptionKey.initiatedPendingAdmin;
+        return isUserCreator
+          ? DisputeDescriptionKey.initiatedByUser
+          : DisputeDescriptionKey.initiatedByPeer;
lib/features/order/notfiers/abstract_mostro_notifier.dart (2)

103-115: Double notification for disputes.

You notify once via NotificationDataExtractor and again inside dispute cases, causing duplicate toasts/pushes.

-        sendNotification(event.action, values: {
-          'dispute_id': dispute.disputeId,
-        }, eventId: event.id);
+        // Notification already handled by extractor (gated as recent).

Apply the same removal for the peer-initiated branch.

Also applies to: 243-246, 294-297


103-105: Fix potential LateInitializationError — session can be uninitialized

late Session session; is only assigned when oldSession != null (lib/features/order/notfiers/abstract_mostro_notifier.dart:18, 34–36), but handleEvent unconditionally passes session to NotificationDataExtractor.extractFromMostroMessage(...) — reading the uninitialized late var will throw at runtime.

  • Fix: make session nullable (Session?) and pass it optionally to the extractor, or ensure session is initialized on every code path (assign a default/minimal Session or fail early).
🧹 Nitpick comments (20)
lib/services/dispute_service.dart (1)

53-54: Duplicate mock delay adds unnecessary latency

Two back-to-back 200ms delays double the artificial latency without value. Keep a single delay.

Apply this diff:

   Future<void> sendDisputeMessage(String disputeId, String message) async {
     // Mock implementation
     await Future.delayed(const Duration(milliseconds: 200));
-    // Mock implementation
-    await Future.delayed(const Duration(milliseconds: 200));
   }
lib/features/chat/screens/chat_rooms_list.dart (2)

55-60: Hiding Disputes tab outside debug: confirm product intent or gate with a feature flag

In release builds Disputes become unreachable from this screen. If this is intentional for the current release, fine; otherwise consider a remote/feature flag to enable without code changes.


67-69: Color.withValues may require recent Flutter

withValues(alpha: ...) is relatively new; if your minimum Flutter SDK is older, prefer withOpacity(0.1) for broader compatibility.

Apply this diff if you need broader SDK support:

-                          color: Colors.white.withValues(alpha: 0.1),
+                          color: Colors.white.withOpacity(0.1),
lib/data/models/dispute.dart (1)

365-369: Counterparty fallback should not show admin for any resolved state

Currently only excludes 'resolved'. Also exclude 'solved' and 'seller-refunded'.

Apply this diff:

-    } else if (dispute.adminPubkey != null && dispute.status != 'resolved') {
+    } else if (dispute.adminPubkey != null &&
+        !({'resolved', 'solved', 'seller-refunded'}.contains((dispute.status ?? '').toLowerCase()))) {
lib/features/order/notfiers/abstract_mostro_notifier.dart (4)

92-100: Event de-dup key includes timestamp; duplicates can slip through.

If the same event replays with differing timestamps, de-duping fails. Prefer id+action.

-  final eventKey = '${event.id}_${event.action}_${event.timestamp}';
+  final eventKey = '${event.id}_${event.action}';

230-246: Normalize Dispute fields for “by you” to match “by peer”.

Mirror the peer branch: set status/action defaults to keep UI logic consistent.

-        final disputeWithOrderId = dispute.copyWith(orderId: orderId);
+        final disputeWithOrderId = dispute.copyWith(
+          orderId: orderId,
+          status: dispute.status ?? 'initiated',
+          action: dispute.action ?? 'dispute-initiated-by-you',
+        );

254-275: Unsafe cast of payload; use pattern matching to avoid TypeError.

-          try {
-            // Try to create Dispute from payload map (e.g., {dispute: "id"})
-            final payloadMap = event.payload as Map<String, dynamic>;
+          try {
+            // Try to create Dispute from payload map (e.g., {dispute: "id"})
+            if (event.payload is! Map) {
+              throw StateError('Unexpected payload type: ${event.payload.runtimeType}');
+            }
+            final payloadMap = (event.payload as Map).cast<String, dynamic>();

Alternatively (Dart 3):

if (event.payload case final Map<String, dynamic> payloadMap) {
  // ...
}

264-270: createdAt should come from event timestamp, not local clock.

Using DateTime.now() skews ordering. Prefer event.timestamp.

-                createdAt: DateTime.now(),
+                createdAt: DateTime.fromMillisecondsSinceEpoch(
+                  event.timestamp ?? DateTime.now().millisecondsSinceEpoch,
+                ),
lib/data/repositories/dispute_repository.dart (2)

66-95: Repository now sources disputes from order notifiers; consider de-duping and sync return.

  • De-duplicate by disputeId across sessions to avoid duplicates.
  • Method is async but contains no await; keep for interface, but add a brief comment or make it sync if callers allow.
-  Future<List<Dispute>> getUserDisputes() async {
+  Future<List<Dispute>> getUserDisputes() async {
     try {
       // ...
-      return disputes;
+      // De-duplicate by disputeId
+      final unique = {
+        for (final d in disputes) d.disputeId: d,
+      }.values.toList();
+      return unique;

97-118: Add a fast-path lookup by orderId when available

Dispute contains an orderId (lib/data/models/dispute.dart) and callers often populate it — in lib/data/repositories/dispute_repository.dart#getDispute short‑circuit by resolving the order (order provider/state or event payload) and returning that order's dispute when orderId is known; fall back to the current firstWhere scan otherwise.

lib/features/disputes/widgets/dispute_status_badge.dart (2)

53-55: Unknown statuses default to “initiated”; prefer explicit “unknown”.

Improves UX when backend sends unseen states.

-      default:
-        return AppTheme.statusPendingBackground.withValues(alpha: 0.3);
+      default:
+        return AppTheme.statusInactiveBackground.withValues(alpha: 0.3);
-      default:
-        return AppTheme.statusPendingText;
+      default:
+        return AppTheme.statusInactiveText;
-      default:
-        return S.of(context)!.disputeStatusInitiated;
+      default:
+        return S.of(context)!.unknown;

Also applies to: 71-73, 89-91


33-38: Reduce duplication across three switches.

Consider a single map from normalized status -> {bg, fg, textKey} to keep styling/text in sync.

Also applies to: 40-56, 58-74, 76-92

lib/features/disputes/widgets/dispute_status_content.dart (2)

16-16: Treat seller-refunded/solved as resolved for the green panel.

Currently only resolved/closed trigger the success box; align with badge mapping.

-    bool isResolved = dispute.status.toLowerCase() == 'resolved' || dispute.status.toLowerCase() == 'closed';
+    final s = dispute.status.toLowerCase();
+    bool isResolved = s == 'resolved' || s == 'closed' || s == 'solved' || s == 'seller-refunded';

140-142: “Unknown … Resolved” string reads incorrectly.

Show the raw status or a generic unknown message.

-        return "${S.of(context)!.unknown} ${S.of(context)!.disputeStatusResolved}";
+        return "${S.of(context)!.unknown}: ${dispute.status}";
lib/features/disputes/widgets/disputes_list.dart (1)

37-37: Consider adding error tracking

While the retry functionality is good, consider logging the error for debugging purposes before displaying it to the user.

       error: (error, stackTrace) => Center(
+        // Log error for debugging
+        // Consider: debugPrint('Failed to load disputes: $error');
         child: Column(
lib/features/disputes/providers/dispute_providers.dart (4)

36-54: Complex state watching pattern may cause performance issues

The provider watches all sessions and their order states, which could lead to unnecessary rebuilds when any order state changes, even if unrelated to disputes. This O(n) watching pattern might impact performance with many active sessions.

Consider using a more targeted approach by only watching order states that have associated disputes:

 /// Provider for user disputes list - uses real data from repository
 final userDisputesProvider = FutureProvider<List<Dispute>>((ref) async {
   final repository = ref.watch(disputeRepositoryProvider);
   
-  // Watch all sessions to invalidate when order states change
-  final sessions = ref.watch(sessionNotifierProvider);
-  
-  // Watch order states for all sessions to trigger refresh when disputes update
-  for (final session in sessions) {
-    if (session.orderId != null) {
-      try {
-        ref.watch(orderNotifierProvider(session.orderId!));
-      } catch (e) {
-        // Continue if order state doesn't exist yet
-      }
-    }
-  }
+  // Only watch session changes, not all order states
+  ref.watch(sessionNotifierProvider);
   
   return repository.getUserDisputes();
 });

Alternatively, consider creating a dedicated dispute state notifier that only updates when dispute-related changes occur.


45-49: Avoid silently catching exceptions

The empty catch block silently swallows all exceptions, which could hide real errors beyond just missing order states.

Be more specific about the expected exceptions:

       try {
         ref.watch(orderNotifierProvider(session.orderId!));
       } catch (e) {
-        // Continue if order state doesn't exist yet
+        // Only continue for expected provider not found errors
+        // Log unexpected errors for debugging
+        if (e is! ProviderNotFoundException) {
+          // Consider logging: debugPrint('Unexpected error watching order: $e');
+        }
       }

69-82: Optimize the nested loop for finding matching sessions

The nested try-catch within a loop (O(n) complexity) could be optimized. Additionally, using ref.read inside an async provider that already watches the sessions could lead to stale data.

Consider using a map for O(1) lookup and consistent state reading:

 final userDisputeDataProvider = FutureProvider<List<DisputeData>>((ref) async {
   final disputes = await ref.watch(userDisputesProvider.future);
-  final sessions = ref.read(sessionNotifierProvider);
+  final sessions = ref.watch(sessionNotifierProvider);
+  
+  // Build a map for O(1) lookup
+  final orderStateMap = <String, dynamic>{};
+  for (final session in sessions) {
+    if (session.orderId != null) {
+      try {
+        final orderState = ref.watch(orderNotifierProvider(session.orderId!));
+        if (orderState.dispute != null) {
+          orderStateMap[orderState.dispute!.disputeId] = orderState;
+        }
+      } catch (e) {
+        continue;
+      }
+    }
+  }
 
   return disputes.map((dispute) {
-    // Find the specific session for this dispute's order
-    Session? matchingSession;
-    dynamic matchingOrderState;
-
-    // Try to find the session and order state that contains this dispute
-    for (final session in sessions) {
-      if (session.orderId != null) {
-        try {
-          final orderState = ref.read(orderNotifierProvider(session.orderId!));
-
-          // Check if this order state contains our dispute
-          if (orderState.dispute?.disputeId == dispute.disputeId) {
-            matchingSession = session;
-            matchingOrderState = orderState;
-            break;
-          }
-        } catch (e) {
-          // Continue checking other sessions
-          continue;
-        }
-      }
-    }
-
-    // If we found matching order state, use it for context
-    if (matchingSession != null && matchingOrderState != null) {
+    final matchingOrderState = orderStateMap[dispute.disputeId];
+    if (matchingOrderState != null) {
       return DisputeData.fromDispute(dispute, orderState: matchingOrderState);
     }

70-70: Inconsistent state reading pattern

Using ref.read here while using ref.watch for sessions creates an inconsistent pattern and could lead to stale data issues.

Use consistent state reading:

-          final orderState = ref.read(orderNotifierProvider(session.orderId!));
+          final orderState = ref.watch(orderNotifierProvider(session.orderId!));
lib/features/order/models/order_state.dart (1)

138-141: Avoid hardcoded 'admin' fallback for adminPubkey

Setting adminPubkey: dispute!.adminPubkey ?? 'admin' makes the dispute appear assigned (the model's hasAdmin checks non-null/non-empty) and can misrepresent state. Replace with the actual admin pubkey (fetch from the message/config) or leave it null when unknown; if a placeholder is required use a clear sentinel (e.g. '') and update consumers accordingly.

Location: lib/features/order/models/order_state.dart:141. Verify lib/data/models/dispute.dart (hasAdmin) and existing mock keys in lib/services/dispute_service.dart and lib/features/disputes/data/dispute_mock_data.dart.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7fbf3fb and 8c54966.

📒 Files selected for processing (14)
  • lib/data/models/dispute.dart (7 hunks)
  • lib/data/repositories/dispute_repository.dart (2 hunks)
  • lib/features/chat/screens/chat_rooms_list.dart (5 hunks)
  • lib/features/disputes/providers/dispute_providers.dart (3 hunks)
  • lib/features/disputes/widgets/dispute_content.dart (1 hunks)
  • lib/features/disputes/widgets/dispute_status_badge.dart (3 hunks)
  • lib/features/disputes/widgets/dispute_status_content.dart (1 hunks)
  • lib/features/disputes/widgets/disputes_list.dart (1 hunks)
  • lib/features/order/models/order_state.dart (2 hunks)
  • lib/features/order/notfiers/abstract_mostro_notifier.dart (3 hunks)
  • lib/l10n/intl_en.arb (2 hunks)
  • lib/l10n/intl_es.arb (2 hunks)
  • lib/l10n/intl_it.arb (2 hunks)
  • lib/services/dispute_service.dart (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Run flutter format and keep code formatted

Files:

  • lib/services/dispute_service.dart
  • lib/features/disputes/widgets/dispute_content.dart
  • lib/data/repositories/dispute_repository.dart
  • lib/features/order/models/order_state.dart
  • lib/features/disputes/widgets/dispute_status_badge.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/features/disputes/widgets/disputes_list.dart
  • lib/features/disputes/widgets/dispute_status_content.dart
  • lib/features/disputes/providers/dispute_providers.dart
  • lib/data/models/dispute.dart
  • lib/features/chat/screens/chat_rooms_list.dart
lib/l10n/intl_{en,es,it}.arb

📄 CodeRabbit inference engine (CLAUDE.md)

When adding a localization key, add it to all three ARB files: intl_en.arb, intl_es.arb, intl_it.arb

Files:

  • lib/l10n/intl_es.arb
  • lib/l10n/intl_it.arb
  • lib/l10n/intl_en.arb
lib/l10n/*.arb

📄 CodeRabbit inference engine (CLAUDE.md)

After modifying ARB files, regenerate localization (e.g., dart run build_runner build -d or flutter gen-l10n)

Files:

  • lib/l10n/intl_es.arb
  • lib/l10n/intl_it.arb
  • lib/l10n/intl_en.arb
🧠 Learnings (3)
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-09-18T17:17:38.944Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T17:17:38.944Z
Learning: Applies to lib/services/nostr_service.dart : All Nostr protocol interactions must go through NostrService

Applied to files:

  • lib/features/chat/screens/chat_rooms_list.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/chat/screens/chat_rooms_list.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (24)
lib/features/chat/screens/chat_rooms_list.dart (4)

3-3: Importing foundation for kDebugMode is fine

No concerns.


73-76: Good: localized description fallback

Using S.of(context)?.conversationsDescription with a sensible fallback looks correct.


84-107: Gesture-only navigation in debug builds

The swipe handler is debug-only; release has no way to reach Disputes from here. Confirm UX is acceptable and there’s another entry point in release.


150-183: Messages header LGTM

Header styling and localization look good.

lib/l10n/intl_it.arb (2)

758-768: Added dispute description/status keys: looks consistent

The new keys (disputeDescription*, dispute* messages) align with the model changes.


780-784: Regenerate l10n — keys present in intl_en.arb, intl_es.arb, intl_it.arb
Provided diff script returned no differences; run flutter gen-l10n or dart run build_runner build -d to regenerate localization code.

lib/l10n/intl_en.arb (3)

697-708: Added dispute description messages: OK

Keys and values align with new DisputeDescriptionKey variants.


720-724: Top-level dispute status strings added: OK

These support the new UI badges/labels.


664-666: Reminder: regenerate localization after ARB changes

Run flutter gen-l10n (or build_runner) to update generated S class.

lib/l10n/intl_es.arb (4)

701-701: Minor copy tweak for disputeSellerRefunded is fine

Wording change reads well in Spanish.


704-714: Added dispute description messages: OK

Keys mirror en/it.


726-729: Top-level dispute status strings added: OK

Consistent with the model/UI.


622-627: Reminder: regenerate localization after ARB changes

Run flutter gen-l10n (or build_runner) to update generated S class.

lib/data/models/dispute.dart (3)

12-17: New enum value for pending-admin state: good addition

initiatedPendingAdmin captures the “waiting for admin” state cleanly.


380-385: Description key mapping: good use of hasAdmin and isUserCreator

This centralizes the logic in one place.


464-483: Localized description getter: OK

Switch cleanly maps keys to S.* strings.

lib/features/disputes/widgets/dispute_content.dart (1)

22-22: LGTM: moved to localized description source.

Switching to getLocalizedDescription(context) aligns with i18n and removes hardcoded strings.

lib/features/disputes/widgets/dispute_status_badge.dart (1)

49-50: LGTM: seller-refunded mapped as resolved.

Consistent color/text with resolved/solved; normalization handles input variants.

Also applies to: 67-68, 85-86

lib/features/disputes/widgets/dispute_status_content.dart (1)

127-131: Approve — i18n keys added and present in locales.

Verified disputeWaitingForAdmin, disputeAdminSettledMessage, disputeSellerRefundedMessage, and disputeStatusResolved exist in lib/l10n/intl_en.arb, lib/l10n/intl_es.arb, and lib/l10n/intl_it.arb.

lib/features/disputes/widgets/disputes_list.dart (3)

1-7: LGTM! Clean migration to Riverpod

The migration from StatelessWidget to ConsumerWidget is properly implemented with correct imports and provider usage.


16-42: Well-structured async state handling

Good implementation of the loading, error, and data states using AsyncValue.when. The error state includes proper user feedback with a retry button.


76-89: LGTM! Clean list implementation

The ListView.builder implementation is efficient and properly uses the DisputeData view model with navigation handling.

lib/features/disputes/providers/dispute_providers.dart (1)

22-25: Good transition from mock to repository data

The provider now correctly fetches real dispute data from the repository instead of using mock data.

lib/features/order/models/order_state.dart (1)

151-159: Fix inconsistent null checking and redundant logging

The same null checking issue exists here. Additionally, line 158 logs both the original and updated dispute status, but it references dispute!.status which could be null if dispute is null (even though it's checked).

Apply this diff to fix the issues:

-    } else if (message.action == Action.adminCanceled && dispute != null) {
+    } else if (message.action == Action.adminCanceled && updatedDispute != null) {
       // When admin cancels order, update dispute status to seller-refunded
-      updatedDispute = dispute!.copyWith(
+      updatedDispute = updatedDispute.copyWith(
         status: 'seller-refunded',
         action: 'admin-canceled', // Store the resolution type
       );
       _logger.i('Updated dispute status to seller-refunded for adminCanceled action');
-      _logger.i('Dispute before update: ${dispute!.status}, after update: ${updatedDispute.status}');
     }

Likely an incorrect or invalid review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
lib/data/models/dispute.dart (1)

443-453: Consider simplifying the nested conditional logic

The nested conditions for determining the description key when status is 'initiated' could be made more readable.

Consider this refactor for better readability:

 case 'initiated':
-  // If admin has been assigned, it's in progress even if status says initiated
-  if (hasAdmin) {
-    return DisputeDescriptionKey.inProgress;
-  }
-  if (isUserCreator == null) {
-    return DisputeDescriptionKey.initiatedPendingAdmin; // Waiting for admin assignment
-  }
-  return isUserCreator
-    ? DisputeDescriptionKey.initiatedByUser
-    : DisputeDescriptionKey.initiatedPendingAdmin;
+  // If admin has been assigned, it's in progress even if status says initiated
+  if (hasAdmin) {
+    return DisputeDescriptionKey.inProgress;
+  }
+  
+  // Handle cases where admin is not yet assigned
+  if (isUserCreator == null || !isUserCreator) {
+    return DisputeDescriptionKey.initiatedPendingAdmin;
+  }
+  
+  return DisputeDescriptionKey.initiatedByUser;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c54966 and e178c70.

📒 Files selected for processing (3)
  • lib/data/models/dispute.dart (8 hunks)
  • lib/features/disputes/providers/dispute_providers.dart (1 hunks)
  • lib/features/order/notfiers/abstract_mostro_notifier.dart (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • lib/features/disputes/providers/dispute_providers.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Run flutter format and keep code formatted

Files:

  • lib/data/models/dispute.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (13)
lib/data/models/dispute.dart (13)

1-5: LGTM - clean imports organization

The imports are well-organized and include all necessary dependencies for the localization support and enhanced dispute functionality.


9-17: LGTM - comprehensive dispute state enumeration

The DisputeDescriptionKey enum provides good semantic coverage of all dispute states, including the new initiatedPendingAdmin state that addresses the core bug mentioned in the PR objectives.


46-57: LGTM - action field addition

The action field addition to the Dispute class aligns with the PR's objective to track dispute actions for proper state management. The constructor and field declaration are properly implemented.


97-99: LGTM - JSON serialization updated

The toJson method correctly includes the new action field when present, maintaining backward compatibility by only adding it when not null.


174-174: LGTM - action field properly parsed

Both factory constructors (fromNostrEvent and fromJson) correctly extract and assign the action field from their respective data sources.

Also applies to: 259-259


276-287: LGTM - copyWith method updated

The copyWith method correctly includes the new action parameter using the sentinel pattern for proper null handling.


304-308: LGTM - equality and hash methods updated

Both operator == and hashCode have been properly updated to include the new action field, maintaining object equality contract.


324-336: LGTM - DisputeData action field addition

The action field has been properly added to the DisputeData class with appropriate constructor parameter and field declaration.


339-397: LGTM - enhanced DisputeData.fromDispute factory

The factory method has been significantly improved with:

  • Better OrderState integration for determining user creator status
  • More robust counterparty resolution logic
  • Proper user role determination based on order type
  • Clean fallback mechanisms when OrderState is not available

This addresses the core issue mentioned in the PR objectives where disputes initiated by peers were showing "Unknown status".


422-423: LGTM - legacy path handling

The legacy fromDisputeEvent method correctly sets action to null, maintaining compatibility while new paths use the enhanced action tracking.


439-464: LGTM - enhanced status mapping logic

The _getDescriptionKeyForStatus method now properly handles the hasAdmin parameter to distinguish between pending admin assignment and active admin involvement, which aligns with the PR's goal of providing clear dispute state feedback.


466-488: LGTM - localization implementation

The getLocalizedDescription method properly implements the localization framework mentioned in the PR objectives, replacing hardcoded status messages with localized descriptions using semantic keys.


468-469: No action required — localization delegates configured in MaterialApp

lib/core/app.dart registers S.delegate and uses S.supportedLocales on MaterialApp, and getLocalizedDescription(context) is called from widget code (e.g. lib/features/disputes/widgets/dispute_status_content.dart), so using S.of(context)! here is acceptable.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)

47-51: Good: raw payload logged only in debug builds

This addresses prior PII concerns in release logs.

🧹 Nitpick comments (2)
lib/data/models/dispute.dart (1)

448-470: Comment contradicts logic in 'initiated' mapping

The comment says “always show pending admin,” but code returns initiatedByUser when isUserCreator == true. Either update the comment or align logic.

Suggested comment tweak:

-        // For 'initiated' status, always show "admin will take this dispute soon"
-        // regardless of who created it, since the key info is that it's waiting for admin
+        // For 'initiated' without admin, show:
+        //  - initiatedByUser if the user opened it
+        //  - initiatedPendingAdmin otherwise (waiting for admin)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)

278-296: Safer payload fallback: avoid blind cast and support non-string IDs

event.payload may not be Map<String, dynamic>, and dispute may be non-string-ish. Current cast can throw; use type checks and toString().

-        if (dispute == null && event.payload != null) {
+        if (dispute == null && event.payload != null) {
           try {
-            // Try to create Dispute from payload map (e.g., {dispute: "id"})
-            final payloadMap = event.payload as Map<String, dynamic>;
+            // Try to create Dispute from payload map (e.g., {dispute: "id"})
+            final payload = event.payload;
+            if (payload is Map) {
-            if (kDebugMode) {
-              logger.i('disputeInitiatedByPeer: Payload map: $payloadMap');
-            }
-            
-            if (payloadMap.containsKey('dispute')) {
-              final disputeId = payloadMap['dispute'] as String;
-              dispute = Dispute(
-                disputeId: disputeId,
+              if (kDebugMode) {
+                logger.i('disputeInitiatedByPeer: Payload map: $payload');
+              }
+              final idRaw = payload['dispute'];
+              final disputeId = idRaw?.toString();
+              if (disputeId != null && disputeId.isNotEmpty) {
+                dispute = Dispute(
+                  disputeId: disputeId,
                   orderId: orderId,
                   status: 'initiated',
                   action: 'dispute-initiated-by-peer',
                   createdAt: DateTime.now(),
-              );
-              logger.i('disputeInitiatedByPeer: Created dispute from ID: $disputeId');
-            }
+                );
+                logger.i('disputeInitiatedByPeer: Created dispute from ID: $disputeId');
+              }
+            }
           } catch (e) {
             logger.e('disputeInitiatedByPeer: Failed to create dispute from payload: $e');
           }
         }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e178c70 and 571d527.

📒 Files selected for processing (4)
  • lib/data/models/dispute.dart (8 hunks)
  • lib/features/order/models/order_state.dart (2 hunks)
  • lib/features/order/notfiers/abstract_mostro_notifier.dart (7 hunks)
  • lib/features/order/notfiers/order_notifier.dart (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/features/order/models/order_state.dart
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Run flutter format and keep code formatted

Files:

  • lib/features/order/notfiers/order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/data/models/dispute.dart
🧠 Learnings (6)
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.

Applied to files:

  • lib/features/order/notfiers/order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.

Applied to files:

  • lib/features/order/notfiers/order_notifier.dart
  • lib/features/order/notfiers/abstract_mostro_notifier.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/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-09-18T17:17:38.944Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T17:17:38.944Z
Learning: Use Riverpod for all state management and encapsulate business logic in Notifiers

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-08-15T01:37:12.243Z
Learnt from: Catrya
PR: MostroP2P/mobile#270
File: lib/shared/widgets/order_filter.dart:133-135
Timestamp: 2025-08-15T01:37:12.243Z
Learning: The MostroP2P/mobile project requires Flutter >=3.27.0 as specified in pubspec.yaml, which supports the Color.withValues() method, so usage of withValues() throughout the codebase is valid and should not be flagged as a compatibility issue.

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-09-18T17:17:38.944Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T17:17:38.944Z
Learning: Applies to core/mostro_fsm.dart : Manage order state transitions via MostroFSM

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (2)
lib/data/models/dispute.dart (1)

486-507: Verify S.of(context)! null-assertion

If S.of(context) is non-nullable in your generator, the “!” is unnecessary and may not compile. If nullable, keep it. Please confirm generation settings.

lib/features/order/notfiers/order_notifier.dart (1)

26-32: Good: handleEvent now async and forwards bypass flag

Awaiting super and threading through bypassTimestampGate aligns with the abstract notifier changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/data/models/dispute.dart (1)

383-389: Order role detection likely incorrect: use enum, not .value string

Order.kind is an enum elsewhere in the codebase; comparing kind.value to 'buy' may not compile or may be brittle.

- userRole = orderState.order!.kind.value == 'buy' ? UserRole.buyer : UserRole.seller;
+ userRole = orderState.order!.kind == enums.OrderType.buy
+     ? UserRole.buyer
+     : UserRole.seller;
♻️ Duplicate comments (3)
lib/features/order/notfiers/abstract_mostro_notifier.dart (2)

61-71: Old-message handling for disputes looks correct

You preserve state hydration while suppressing side effects via bypassTimestampGate. This aligns with the “hydrate always, side‑effect only for recent” learning.


322-325: Good: duplicate dispute notifications removed

Relying on NotificationDataExtractor avoids bypassing the timestamp gate and prevents double notifies.

lib/data/models/dispute.dart (1)

345-359: Creator detection fixed to use enums.Action — LGTM

Switching from string slug comparisons to enums resolves the previous correctness bug.

🧹 Nitpick comments (8)
lib/features/order/notfiers/abstract_mostro_notifier.dart (5)

47-51: Avoid logging raw payloads; redact even in debug builds

Full toJson() can include PII. Keep the action/id/timestamps and strip or hash fields like invoices, amounts, pubkeys.

Example:

- if (kDebugMode) {
-   logger.i('Received message: ${msg?.toJson()}');
- } else {
+ if (kDebugMode) {
+   logger.i('Received: ${{
+     'id': msg?.id,
+     'action': msg?.action,
+     'ts': msg?.timestamp,
+   }}');
+ } else {
   logger.i('Received message with action: ${msg?.action}');
 }

100-106: Event de-dup key risks misses when timestamps differ across retries

Including timestamp in the key can treat the same event as “new.” Prefer id when present; fall back only if id is null.

- final eventKey = '${event.id}_${event.action}_${event.timestamp}';
+ final eventKey = event.id ??
+     // Fall back to action+ts only if id is missing
+     '${event.action}_${event.timestamp}';

119-129: Tighten notification gate condition

To guarantee we never notify for stale events (even if someone accidentally calls handleEvent with bypassTimestampGate=false), gate strictly on isRecent.

- if (notificationData != null && (isRecent || !bypassTimestampGate)) {
+ if (notificationData != null && isRecent) {
     sendNotification(
       notificationData.action,
       values: notificationData.values,
       isTemporary: notificationData.isTemporary,
       eventId: notificationData.eventId,
     );
- } else if (notificationData != null && bypassTimestampGate) {
+ } else if (notificationData != null) {
     logger.i('Skipping notification for old event: ${event.action} (timestamp: ${event.timestamp})');
 }

252-257: Avoid hardcoded action slugs; centralize mapping

"dispute-initiated-by-you" is duplicated across files. Define a single mapper or constants to prevent drift.

- action: 'dispute-initiated-by-you', // Store the action for UI logic
+ action: DisputeActionSlugs.initiatedByYou, // e.g., central constants

If you prefer enums, store enums.Action and serialize in toJson.


270-301: Defensive cast before map access to avoid catching unrelated errors

Prefer an explicit type check to keep try/catch for truly exceptional paths.

- final payloadMap = event.payload as Map<String, dynamic>;
+ if (event.payload is! Map<String, dynamic>) {
+   logger.w('disputeInitiatedByPeer: Non-map payload, skipping map-based parse');
+ } else {
+   final payloadMap = event.payload as Map<String, dynamic>;
+   ...
+ }

Also handle non-string dispute IDs safely:

- final disputeId = payloadMap['dispute'] as String;
+ final disputeId = payloadMap['dispute']?.toString();
+ if (disputeId == null || disputeId.isEmpty) {
+   throw FormatException('Missing dispute id in payload');
+ }
lib/data/models/dispute.dart (3)

451-473: Initiated mapping is inconsistent; return initiatedByPeer when peer opened

Comment says “always show waiting for admin,” but code returns initiatedByUser vs initiatedPendingAdmin and never uses initiatedByPeer. Map precisely:

   case 'initiated':
-    // If admin has been assigned, it's in progress even if status says initiated
+    // Admin assigned → in progress regardless of raw status
     if (hasAdmin) {
       ...
       return DisputeDescriptionKey.inProgress;
     }
-    // For 'initiated' status, always show "admin will take this dispute soon"
-    // regardless of who created it, since the key info is that it's waiting for admin
-    ...
-    return isUserCreator == true
-      ? DisputeDescriptionKey.initiatedByUser
-      : DisputeDescriptionKey.initiatedPendingAdmin;
+    // No admin yet: reflect initiator when known; fall back to generic pending-admin
+    if (isUserCreator == true) {
+      return DisputeDescriptionKey.initiatedByUser;
+    } else if (isUserCreator == false) {
+      return DisputeDescriptionKey.initiatedByPeer;
+    } else {
+      return DisputeDescriptionKey.initiatedPendingAdmin;
+    }

Ensure l10n keys exist for initiatedByPeer (they do per PR notes).


521-540: Centralize action string ↔ enum mapping

good: helper exists. Consider moving this to a shared utils/enums adapter to keep notifier/model in sync and avoid drift.


399-408: Minor: comment mismatch (“resolved”)

The comment says “Store the action that resolved the dispute,” but this field also carries initiation actions. Reword to “Store the action that triggered the dispute event.”

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 394b579 and 667d942.

📒 Files selected for processing (2)
  • lib/data/models/dispute.dart (9 hunks)
  • lib/features/order/notfiers/abstract_mostro_notifier.dart (6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Run flutter format and keep code formatted

Files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/data/models/dispute.dart
🧠 Learnings (6)
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.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/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-09-18T17:17:38.944Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T17:17:38.944Z
Learning: Use Riverpod for all state management and encapsulate business logic in Notifiers

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-08-15T01:37:12.243Z
Learnt from: Catrya
PR: MostroP2P/mobile#270
File: lib/shared/widgets/order_filter.dart:133-135
Timestamp: 2025-08-15T01:37:12.243Z
Learning: The MostroP2P/mobile project requires Flutter >=3.27.0 as specified in pubspec.yaml, which supports the Color.withValues() method, so usage of withValues() throughout the codebase is valid and should not be flagged as a compatibility issue.

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
📚 Learning: 2025-09-18T17:17:38.944Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T17:17:38.944Z
Learning: Applies to core/mostro_fsm.dart : Manage order state transitions via MostroFSM

Applied to files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (2)
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)

98-106: All handleEvent overrides and mocks updated to Future<void>
No remaining void handleEvent(…) declarations found across the codebase.

lib/data/models/dispute.dart (1)

489-508: Confirm S.of(context) nullability in generated l10n

Generated l10n source (e.g. lib/generated/l10n.dart) is not in the repo; searches show many S.of(context)! usages and some S.of(context)? usages, so the signature can't be determined. Inspect the generated S.of(Context) signature and remove the unnecessary '!' here if S.of returns non-nullable, otherwise keep or convert to null-aware access.
Location: lib/data/models/dispute.dart:489-508.

Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Desde que la disputa está en status Initiated, dentro se muestra como si estuviera en status In-progress, y aunque luego pasa a Resolved también se sigue mostrando solo In-progress

  • En Order ID siempre sale order_unknown

  • Independientemente del status y si el usuario es seller/buyer siempre siempre dice Dispute with Seller: loose-sats

image
  • Si la orden fue cancelada dice: Order was canceled -seller refunded, pero si se le liberaron los sats al comprador dice: This dispute has been resolved, deberían ser similares los mensajes, por ejemplo: Order was completed: buyer received the sats
image

@AndreaDiazCorreia
Copy link
Member Author

@Catrya el detalle de la disputa es el proximo pr, por eso no lo ves acorde, no esta implementado en esta iteracion

@Catrya
Copy link
Member

Catrya commented Sep 20, 2025

Ah ok, en este PR qué miro entonces? Los status de la disputa?

@AndreaDiazCorreia
Copy link
Member Author

Ah ok, en este PR qué miro entonces? Los status de la disputa?

si, aunque lo de los mensajes tienes razon deberia cambiar ese resuelto por algo mas descriptivo como el otro que esta, ya hago ese cambio

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
lib/l10n/intl_es.arb (2)

701-701: Spanish phrasing polish for seller-refunded status

More natural and consistent wording:

-  "disputeSellerRefunded": "Esta disputa se ha resuelto con el vendedor siendo reembolsado.",
+  "disputeSellerRefunded": "Esta disputa se ha resuelto a favor del vendedor; se le ha reembolsado.",

704-714: lib/l10n/intl_es.arb — parity OK; two minor Spanish copy tweaks suggested

JSON validated and all dispute keys are present in intl_en.arb and intl_it.arb.

-  "disputeDescriptionInProgress": "Aún no hay mensajes",
+  "disputeDescriptionInProgress": "Un mediador está revisando tu caso",
-  "disputeDescriptionSellerRefunded": "Orden cancelada - vendedor reembolsado",
+  "disputeDescriptionSellerRefunded": "Orden cancelada - el vendedor fue reembolsado",
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 667d942 and 4a931c8.

📒 Files selected for processing (3)
  • lib/l10n/intl_en.arb (1 hunks)
  • lib/l10n/intl_es.arb (1 hunks)
  • lib/l10n/intl_it.arb (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • lib/l10n/intl_it.arb
  • lib/l10n/intl_en.arb
🧰 Additional context used
📓 Path-based instructions (3)
lib/l10n/*.arb

📄 CodeRabbit inference engine (CLAUDE.md)

Maintain ARB localization files in lib/l10n/ (en, es, it)

Files:

  • lib/l10n/intl_es.arb
lib/l10n/intl_{en,es,it}.arb

📄 CodeRabbit inference engine (CLAUDE.md)

Add new localization keys to all ARB files: intl_en.arb, intl_es.arb, intl_it.arb

Files:

  • lib/l10n/intl_es.arb
**/*.arb

📄 CodeRabbit inference engine (AGENTS.md)

Keep user-facing strings in localization ARB files; do not hard-code copy in Dart code

Files:

  • lib/l10n/intl_es.arb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@grunch grunch merged commit 399ae2a into main Sep 24, 2025
2 of 3 checks passed
@grunch grunch deleted the andrea/dispute-status branch September 24, 2025 13:27
@coderabbitai coderabbitai bot mentioned this pull request Oct 13, 2025
3 tasks
@coderabbitai coderabbitai bot mentioned this pull request Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants