Skip to content

feat(us9): phase 12 — dispute system, admin chat, dispute list - #63

Merged
grunch merged 6 commits into
mainfrom
006-mostro-p2p-client
Mar 30, 2026
Merged

feat(us9): phase 12 — dispute system, admin chat, dispute list#63
grunch merged 6 commits into
mainfrom
006-mostro-p2p-client

Conversation

@grunch

@grunch grunch commented Mar 30, 2026

Copy link
Copy Markdown
Member
  • Add Rust disputes API (open_dispute, submit_evidence, on_dispute_updated,
    handle_admin_took_dispute/settled/canceled) with DisputeStore + 5 unit tests
  • Add Session.set_admin_shared_key for ECDH admin key storage
  • Add Dart SessionState/SessionNotifier with adminSharedKey support
  • Add DisputeItem/DisputeMessage models and DisputeNotifier providers
  • Add DisputesList, DisputeListItem, DisputeMessagesList, DisputeMessageInput widgets
  • Add DisputeChatScreen with header, bubble chat, input, and terminal state banners
  • Wire Disputes tab in ChatRoomsScreen to DisputesList
  • Add disputed state to TradeDetailScreen (CLOSE+CONTACT+CANCEL+RELEASE + VIEW DISPUTE)
  • Register /dispute_details/:disputeId → DisputeChatScreen in app_routes.dart

Summary by CodeRabbit

  • New Features
    • Dispute workflow: interactive disputes list, dispute chat screen with messages, compose/attach UI, unread indicators, resolved banners and locked-chat states, auto-scroll, copy order/dispute IDs, and “View Dispute” from trade details
    • Real‑time dispute updates and admin-assigned indicators; disputes are marked read when viewed
  • Localization
    • Added dispute-related UI strings for English, German, Spanish, French, and Italian

grunch added 2 commits March 30, 2026 14:33
…tail screens

- Add Rust disputes API (open_dispute, submit_evidence, on_dispute_updated,
  handle_admin_took_dispute/settled/canceled) with DisputeStore + 5 unit tests
- Add Session.set_admin_shared_key for ECDH admin key storage
- Add Dart SessionState/SessionNotifier with adminSharedKey support
- Add DisputeItem/DisputeMessage models and DisputeNotifier providers
- Add DisputesList, DisputeListItem, DisputeMessagesList, DisputeMessageInput widgets
- Add DisputeChatScreen with header, bubble chat, input, and terminal state banners
- Wire Disputes tab in ChatRoomsScreen to DisputesList
- Add disputed state to TradeDetailScreen (CLOSE+CONTACT+CANCEL+RELEASE + VIEW DISPUTE)
- Register /dispute_details/:disputeId → DisputeChatScreen in app_routes.dart
…ards, i18n, error sanitization

- disputes.rs: atomic try_insert_if_absent_or_resolved prevents TOCTOU race in open_dispute
- disputes.rs: handle_admin_took_dispute validates Open state before transitioning to InReview
- disputes.rs: resolve_dispute validates not-already-resolved before mutating
- disputes.rs: rename _resolve → resolve_dispute (conventional naming)
- disputes.rs: DisputeStream::next handles RecvError::Lagged gracefully (continue vs. fail)
- trade_detail_screen: gate CANCEL+RELEASE to seller only (!_isBuyer) in disputed state
- trade_detail_screen: CONTACT button navigates to chatRoomPath (was "Coming soon")
- dispute_chat_screen: show "Dispute not found." instead of indefinite spinner for null dispute
- dispute_chat_screen: refund banner text is role-aware (isSelling seller vs. buyer copy)
- dispute_chat_screen: wrap lock text in Flexible to prevent overflow on narrow screens
- dispute_messages_list: remove unused colors param from _AdminAssignedBanner
- disputes_list: sanitize error with debugPrint + generic user message
- disputes_providers: description getter handles all 3 DisputeResolution cases explicitly
- i18n: add disputesEmptyState, disputeAttachFile, disputeWriteMessageHint, disputeSend to all 5 locales
@coderabbitai

coderabbitai Bot commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@grunch has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 24 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 24 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dae39115-8b0c-4f1b-8081-4cbab44974e5

📥 Commits

Reviewing files that changed from the base of the PR and between 6215d84 and 4f127e3.

📒 Files selected for processing (12)
  • lib/features/disputes/screens/dispute_chat_screen.dart
  • lib/l10n/app_de.arb
  • lib/l10n/app_en.arb
  • lib/l10n/app_es.arb
  • lib/l10n/app_fr.arb
  • lib/l10n/app_it.arb
  • lib/l10n/app_localizations.dart
  • lib/l10n/app_localizations_de.dart
  • lib/l10n/app_localizations_en.dart
  • lib/l10n/app_localizations_es.dart
  • lib/l10n/app_localizations_fr.dart
  • lib/l10n/app_localizations_it.dart

Walkthrough

Adds a disputes feature: Riverpod dispute models/providers, list and chat UIs (messages list, input, banners, auto-scroll), route wiring, session/admin shared-key providers, localization entries, and a Rust in-memory disputes API with event streams and admin handlers.

Changes

Cohort / File(s) Summary
Routing
lib/core/app_routes.dart
Wired dispute routes to DisputeChatScreen and use non-null state.pathParameters['disputeId']!.
Dispute state & providers
lib/features/disputes/providers/disputes_providers.dart
New dispute enums/models (DisputeStatus, DisputeResolution), DisputeItem/DisputeMessage, DisputeNotifier (upsert, markRead), and Riverpod providers (list, unread count, lookup by id/tradeId).
Chat screen
lib/features/disputes/screens/dispute_chat_screen.dart
New DisputeChatScreen widget: marks read on mount, looks up dispute by id, renders header/status, resolved banner variants, messages list stub, and conditional message input.
Messages UI
lib/features/disputes/widgets/dispute_messages_list.dart
New DisputeMessagesList: dedupe/sort messages, auto-scroll on update, info card, message bubble types, admin/chat-closed banners, clipboard copy handling.
Message input
lib/features/disputes/widgets/dispute_message_input.dart
New DisputeMessageInput: attach button/loading, text input, send handling (trims/ignores empty), disposes controller; UI currently shows “coming soon” SnackBars for actions.
List UI & items
lib/features/disputes/widgets/disputes_list.dart, lib/features/disputes/widgets/dispute_list_item.dart
New DisputesList with loading/error/empty states and DisputeListItem (status chip, unread dot, truncated ids, copy, navigation, markRead).
Chat tab integration
lib/features/chat/screens/chat_rooms_screen.dart
Replaced Disputes tab placeholder with const Expanded(child: DisputesList()).
Trade detail integration
lib/features/trades/screens/trade_detail_screen.dart
When trade is disputed, show dispute-specific actions (CLOSE/CONTACT/VIEW DISPUTE, seller CANCEL/RELEASE flows) and navigation to dispute by tradeId (shows snackbar if none).
Session management (Dart)
lib/shared/providers/session_provider.dart
New SessionState and SessionNotifier to hold order/session keys and admin shared key; providers sessionProvider and adminSharedKeyProvider.
Localization (Dart)
lib/l10n/*.arb, lib/l10n/app_localizations*.dart
Added dispute-related i18n keys across en/de/es/fr/it and corresponding getters/overrides in localization classes.
Rust disputes API & types
rust/src/api/disputes.rs, rust/src/api/types.rs, rust/src/api/mod.rs
New in-memory DisputeStore with broadcast updates, async API (open, submit_evidence, get), admin handlers (took/settled/canceled), DisputeStream, tests; added Dispute struct and exported disputes module; renamed resolution variants to FundsToBuyer/FundsToSeller.
Rust session
rust/src/mostro/session.rs
Added SessionManager::set_admin_shared_key to persist admin shared key per order.
Docs / checklist
specs/004-mostro-p2p-client/tasks.md
Marked Phase 12 dispute tasks T085–T093 completed.

Sequence Diagram

sequenceDiagram
    participant User
    participant App as Dispute App (Dart)
    participant Notifier as DisputeNotifier (Dart)
    participant RustAPI as DisputeStore/API (Rust)
    participant Session as SessionManager (Rust)

    User->>App: Open DisputeChatScreen(disputeId)
    App->>Notifier: markRead(disputeId)
    Notifier-->>App: updated dispute state (isRead=true)
    App->>RustAPI: get_dispute(trade_id) or on_dispute_updated(trade_id)
    RustAPI-->>App: emit dispute updates (broadcast)
    App->>App: render header, messages list, banners, input enabled/disabled

    Note over RustAPI,Session: Admin flow
    RustAPI->>RustAPI: handle_admin_took_dispute(trade_id, admin_pubkey)
    RustAPI->>Session: set_admin_shared_key(order_id, key)
    Session-->>App: adminSharedKey available (via provider)
    App->>RustAPI: submit encrypted message / actions
    RustAPI-->>App: broadcast resolved/canceled update
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰
I hopped through code with tiny feet,
lists and banners, messages neat —
keys and streams to stitch the thread,
disputes can chat (for now, in dev),
carrot cheers for features fled.

🚥 Pre-merge checks | ✅ 3
✅ 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 accurately describes the primary changes: implementation of a dispute system, admin chat functionality, and dispute list UI across the application.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 006-mostro-p2p-client

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (6)
lib/features/disputes/widgets/dispute_messages_list.dart (2)

342-405: Consider localizing banner text.

The banner strings ("An administrator has been assigned...", "This dispute has been resolved...") are hardcoded in English. For consistency with the rest of the app's i18n approach, consider adding localization keys for these messages.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/widgets/dispute_messages_list.dart` around lines 342 -
405, The two banner widgets _AdminAssignedBanner and _ChatClosedBanner contain
hardcoded English strings; replace those literals with localized messages by
adding keys (e.g., adminAssignedBanner, disputeChatClosedBanner) to your app's
localization resources (ARB/JSON and generated AppLocalizations), then update
the widgets to fetch the text via
AppLocalizations.of(context).adminAssignedBanner and .disputeChatClosedBanner
(or your project's localization accessor) so the banners use the localized
strings and appear in all supported locales.

285-294: Consider localizing "Copied" snackbar text.

The snackbar message "Copied" is hardcoded. For consistency with other localized strings in this feature, consider using AppLocalizations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/widgets/dispute_messages_list.dart` around lines 285 -
294, The snackbar text is hardcoded in the GestureDetector onLongPress handler
(where Clipboard.setData and ScaffoldMessenger.of(context).showSnackBar are
called); replace the literal 'Copied' with the localized string from
AppLocalizations (e.g. AppLocalizations.of(context)!.<appropriateKey>) so the
SnackBar content uses the app's localization, and ensure you import and use
AppLocalizations in the widget containing this onLongPress.
lib/features/disputes/widgets/disputes_list.dart (1)

33-37: Consider localizing the error message.

The error message "Failed to load disputes. Please try again." is hardcoded in English, while other strings in this widget use AppLocalizations. For consistency across supported locales, consider adding a localization key for this error message.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/widgets/disputes_list.dart` around lines 33 - 37,
Replace the hardcoded error string in the _ErrorState usage with a localized
string from AppLocalizations (use the widget's BuildContext to access
AppLocalizations, e.g. AppLocalizations.of(context).<newKey>), add a new
localization key (e.g. failedToLoadDisputes) to your ARB/intl files with the
message "Failed to load disputes. Please try again.", regenerate the
localization files, and update the call site that currently passes message:
'Failed to load disputes. Please try again.' (inside the _ErrorState
instantiation) to use the generated localized getter; leave onRetry: () =>
ref.invalidate(userDisputeDataProvider) unchanged.
lib/features/disputes/widgets/dispute_list_item.dart (1)

63-70: Consider localizing "Order dispute" title.

The hardcoded string "Order dispute" should be localized for consistency with the rest of the i18n implementation in this feature.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/widgets/dispute_list_item.dart` around lines 63 - 70,
The hardcoded "Order dispute" string in the Text widget should be replaced with
the app's localization lookup; update the DisputeListItem (or the widget
containing the Expanded -> Text) to use the project's localization API (e.g.,
AppLocalizations.of(context).orderDispute or context.l10n.orderDispute) instead
of the literal, keeping the same textTheme.bodyLarge?.copyWith(...) styling and
passing the BuildContext so the localized string is retrieved at runtime.
lib/features/disputes/providers/disputes_providers.dart (2)

74-91: copyWith only supports isRead—consider making it more complete.

The current copyWith method only accepts isRead, but DisputeItem has many other fields (status, resolution, adminPubkey, etc.) that may need updating during the dispute lifecycle. This could force callers to reconstruct the entire object manually.

♻️ More complete copyWith
- DisputeItem copyWith({bool? isRead}) {
+ DisputeItem copyWith({
+   DisputeStatus? status,
+   String? adminPubkey,
+   DisputeResolution? resolution,
+   int? resolvedAt,
+   bool? isRead,
+ }) {
    return DisputeItem(
      id: id,
      tradeId: tradeId,
-     status: status,
+     status: status ?? this.status,
      initiatedByMe: initiatedByMe,
      openedAt: openedAt,
      reason: reason,
-     adminPubkey: adminPubkey,
-     resolution: resolution,
-     resolvedAt: resolvedAt,
+     adminPubkey: adminPubkey ?? this.adminPubkey,
+     resolution: resolution ?? this.resolution,
+     resolvedAt: resolvedAt ?? this.resolvedAt,
      isRead: isRead ?? this.isRead,
      peerHandle: peerHandle,
      peerIconIndex: peerIconIndex,
      peerColorHue: peerColorHue,
      isSelling: isSelling,
    );
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/providers/disputes_providers.dart` around lines 74 -
91, The current DisputeItem.copyWith only accepts isRead which forces callers to
rebuild objects; update DisputeItem.copyWith to accept optional named parameters
for all mutable fields (e.g., status, initiatedByMe, openedAt, reason,
adminPubkey, resolution, resolvedAt, peerHandle, peerIconIndex, peerColorHue,
isSelling, and any others) and return a new DisputeItem using each parameter if
non-null or falling back to the existing property (e.g., status: status ??
this.status). Modify the signature of DisputeItem.copyWith and its return
expression accordingly so callers can partially update any field without
reconstructing the whole object.

147-152: Consider documenting why AsyncValue.data wraps synchronous state.

userDisputeDataProvider always returns AsyncValue.data(...) since the underlying disputeNotifierProvider is synchronous. This works but may confuse future readers expecting AsyncValue to represent async operations. A brief doc comment explaining this is for API uniformity with future async bridge integration would help.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/providers/disputes_providers.dart` around lines 147 -
152, Add a short doc comment above userDisputeDataProvider explaining that it
intentionally returns AsyncValue.data(...) even though disputeNotifierProvider
is synchronous: this is done to provide a uniform AsyncValue-based API surface
(using AsyncValue.data with the sorted list) for consumers and to allow easy
future migration if the source becomes asynchronous; reference the symbols
userDisputeDataProvider, disputeNotifierProvider, and AsyncValue.data in the
comment so reviewers and future maintainers understand the rationale.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/core/app_routes.dart`:
- Around line 193-195: The route for "/dispute_chat/:disputeId" is still
returning the stub instead of the real screen; update the route builder that
currently returns _Stub to instead construct DisputeChatScreen with the
disputeId from state.pathParameters['disputeId'] (same pattern used by the
/dispute_details route), removing the stub reference so both routes behave
consistently and cannot accidentally navigate to a placeholder.

In `@lib/features/disputes/screens/dispute_chat_screen.dart`:
- Around line 210-328: The build method currently only checks dispute.resolution
== DisputeResolution.fundsToMe and treats all other resolutions as a refund; add
an explicit branch for DisputeResolution.cooperativeCancel (reference
dispute.resolution and DisputeResolution.cooperativeCancel) so cooperative
cancels show the correct message/visuals (e.g., "The order was cooperatively
cancelled; no funds were transferred" and the same resolved badge styling used
for the else branch), instead of the misleading refund text; ensure the existing
refund text remains only for the appropriate resolution (e.g.,
fundsToCounterparty) and update the conditional structure in the Widget build
(where the current if and return containers are) to handle fundsToMe,
cooperativeCancel, and the refund case separately.

In `@lib/features/trades/screens/trade_detail_screen.dart`:
- Around line 488-491: The button navigation is passing widget.orderId (trade
ID) to AppRoute.disputeDetailsPath which expects a disputeId; create a way to
resolve the dispute ID from the trade before calling context.push. Implement a
new provider (e.g., disputeByTradeIdProvider) that scans disputes for
DisputeItem.tradeId == widget.orderId (or surface the disputeId from the trade
model when the Rust bridge is wired), then change the FilledButton.icon
onPressed to lookup the disputeId via disputeByTradeIdProvider and pass that
disputeId into AppRoute.disputeDetailsPath (instead of widget.orderId) so
DisputeChatScreen receives the correct ID.

In `@lib/l10n/app_localizations.dart`:
- Around line 155-177: The new localization getters disputesEmptyState,
disputeAttachFile, disputeWriteMessageHint, and disputeSend were added to the
base AppLocalizations interface but the generated locale implementations
(app_localizations_*.dart) are out of sync; regenerate all localized files by
running the project's localization generation step (e.g., the Flutter gen-l10n
command or the repo's l10n generation script) so app_localizations_en.dart,
app_localizations_de.dart, app_localizations_es.dart, app_localizations_fr.dart
etc. are regenerated from the ARB sources and include implementations for those
four getters.

In `@rust/src/api/disputes.rs`:
- Around line 195-212: The resolve_dispute function currently does a non-atomic
get-modify-upsert (using dispute_store().get(...), mutating the returned
dispute, then dispute_store().upsert(...)), which allows a TOCTOU race; change
it to perform the mutation under the store's write lock or an atomic update API
so the check-and-set is one operation: use the dispute_store()'s write/update
method (or acquire its write lock) to load the dispute, verify status !=
DisputeStatus::Resolved, set status = DisputeStatus::Resolved, set resolution,
resolved_at, is_read, and persist in the same locked/atomic operation instead of
separate get and upsert in resolve_dispute.
- Around line 164-183: The current handle_admin_took_dispute does a non-atomic
get-then-upsert (dispute_store().get -> validate -> mutate ->
dispute_store().upsert) which allows TOCTOU races; change it to perform an
atomic read-modify-write using the store's conditional update API (e.g.,
implement/use a compare-and-swap / atomic update method on dispute_store similar
to try_insert_if_absent_or_resolved) or acquire the store's write lock for the
entire operation so the check and mutation are done atomically; specifically
replace the get + manual status check + upsert sequence in
handle_admin_took_dispute with a single atomic operation that verifies
dispute.status == DisputeStatus::Open, sets status = InReview, admin_pubkey =
Some(...), is_read = false, and returns an error if the conditional update
fails.

---

Nitpick comments:
In `@lib/features/disputes/providers/disputes_providers.dart`:
- Around line 74-91: The current DisputeItem.copyWith only accepts isRead which
forces callers to rebuild objects; update DisputeItem.copyWith to accept
optional named parameters for all mutable fields (e.g., status, initiatedByMe,
openedAt, reason, adminPubkey, resolution, resolvedAt, peerHandle,
peerIconIndex, peerColorHue, isSelling, and any others) and return a new
DisputeItem using each parameter if non-null or falling back to the existing
property (e.g., status: status ?? this.status). Modify the signature of
DisputeItem.copyWith and its return expression accordingly so callers can
partially update any field without reconstructing the whole object.
- Around line 147-152: Add a short doc comment above userDisputeDataProvider
explaining that it intentionally returns AsyncValue.data(...) even though
disputeNotifierProvider is synchronous: this is done to provide a uniform
AsyncValue-based API surface (using AsyncValue.data with the sorted list) for
consumers and to allow easy future migration if the source becomes asynchronous;
reference the symbols userDisputeDataProvider, disputeNotifierProvider, and
AsyncValue.data in the comment so reviewers and future maintainers understand
the rationale.

In `@lib/features/disputes/widgets/dispute_list_item.dart`:
- Around line 63-70: The hardcoded "Order dispute" string in the Text widget
should be replaced with the app's localization lookup; update the
DisputeListItem (or the widget containing the Expanded -> Text) to use the
project's localization API (e.g., AppLocalizations.of(context).orderDispute or
context.l10n.orderDispute) instead of the literal, keeping the same
textTheme.bodyLarge?.copyWith(...) styling and passing the BuildContext so the
localized string is retrieved at runtime.

In `@lib/features/disputes/widgets/dispute_messages_list.dart`:
- Around line 342-405: The two banner widgets _AdminAssignedBanner and
_ChatClosedBanner contain hardcoded English strings; replace those literals with
localized messages by adding keys (e.g., adminAssignedBanner,
disputeChatClosedBanner) to your app's localization resources (ARB/JSON and
generated AppLocalizations), then update the widgets to fetch the text via
AppLocalizations.of(context).adminAssignedBanner and .disputeChatClosedBanner
(or your project's localization accessor) so the banners use the localized
strings and appear in all supported locales.
- Around line 285-294: The snackbar text is hardcoded in the GestureDetector
onLongPress handler (where Clipboard.setData and
ScaffoldMessenger.of(context).showSnackBar are called); replace the literal
'Copied' with the localized string from AppLocalizations (e.g.
AppLocalizations.of(context)!.<appropriateKey>) so the SnackBar content uses the
app's localization, and ensure you import and use AppLocalizations in the widget
containing this onLongPress.

In `@lib/features/disputes/widgets/disputes_list.dart`:
- Around line 33-37: Replace the hardcoded error string in the _ErrorState usage
with a localized string from AppLocalizations (use the widget's BuildContext to
access AppLocalizations, e.g. AppLocalizations.of(context).<newKey>), add a new
localization key (e.g. failedToLoadDisputes) to your ARB/intl files with the
message "Failed to load disputes. Please try again.", regenerate the
localization files, and update the call site that currently passes message:
'Failed to load disputes. Please try again.' (inside the _ErrorState
instantiation) to use the generated localized getter; leave onRetry: () =>
ref.invalidate(userDisputeDataProvider) unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 20d52ed0-9415-41ab-b93d-71de86a21317

📥 Commits

Reviewing files that changed from the base of the PR and between 3a3cfd8 and 7a81aa7.

📒 Files selected for processing (26)
  • lib/core/app_routes.dart
  • lib/features/chat/screens/chat_rooms_screen.dart
  • lib/features/disputes/providers/disputes_providers.dart
  • lib/features/disputes/screens/dispute_chat_screen.dart
  • lib/features/disputes/widgets/dispute_list_item.dart
  • lib/features/disputes/widgets/dispute_message_input.dart
  • lib/features/disputes/widgets/dispute_messages_list.dart
  • lib/features/disputes/widgets/disputes_list.dart
  • lib/features/trades/screens/trade_detail_screen.dart
  • lib/l10n/app_de.arb
  • lib/l10n/app_en.arb
  • lib/l10n/app_es.arb
  • lib/l10n/app_fr.arb
  • lib/l10n/app_it.arb
  • lib/l10n/app_localizations.dart
  • lib/l10n/app_localizations_de.dart
  • lib/l10n/app_localizations_en.dart
  • lib/l10n/app_localizations_es.dart
  • lib/l10n/app_localizations_fr.dart
  • lib/l10n/app_localizations_it.dart
  • lib/shared/providers/session_provider.dart
  • rust/src/api/disputes.rs
  • rust/src/api/mod.rs
  • rust/src/api/types.rs
  • rust/src/mostro/session.rs
  • specs/004-mostro-p2p-client/tasks.md

Comment thread lib/core/app_routes.dart
Comment thread lib/features/disputes/screens/dispute_chat_screen.dart
Comment thread lib/features/trades/screens/trade_detail_screen.dart Outdated
Comment on lines +155 to +177
/// Empty state message on the disputes list screen
///
/// In en, this message translates to:
/// **'Your disputes will appear here'**
String get disputesEmptyState;

/// Tooltip for the attach file button in dispute chat
///
/// In en, this message translates to:
/// **'Attach file'**
String get disputeAttachFile;

/// Hint text for the dispute chat message input field
///
/// In en, this message translates to:
/// **'Write a message…'**
String get disputeWriteMessageHint;

/// Tooltip for the send button in dispute chat
///
/// In en, this message translates to:
/// **'Send'**
String get disputeSend;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

required_keys=(
  disputesEmptyState
  disputeAttachFile
  disputeWriteMessageHint
  disputeSend
)

echo "== Checking ARB keys across lib/l10n/app_*.arb =="
missing=0
for f in $(fd '^app_.*\.arb$' lib/l10n); do
  for k in "${required_keys[@]}"; do
    if ! rg -n "\"$k\"\\s*:" "$f" >/dev/null; then
      echo "MISSING key '$k' in $f"
      missing=1
    fi
  done
done

echo "== Checking generated locale classes implement getters =="
for g in $(fd '^app_localizations_.*\.dart$' lib/l10n); do
  for k in "${required_keys[@]}"; do
    if ! rg -n "String get $k;" "$g" >/dev/null; then
      echo "MISSING getter '$k' in $g"
      missing=1
    fi
  done
done

if [[ "$missing" -eq 0 ]]; then
  echo "All required keys/getters are present."
else
  echo "Localization sync check failed."
  exit 1
fi

Repository: MostroP2P/app

Length of output: 1661


Regenerate all localization files before merging.

The new dispute-related getters added to the base AppLocalizations class are missing from all generated locale implementations (app_localizations_de.dart, app_localizations_en.dart, app_localizations_es.dart, app_localizations_fr.dart). This will cause compilation failures. Run the localization generation tool to regenerate all app_localizations_*.dart files from their corresponding ARB sources.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/l10n/app_localizations.dart` around lines 155 - 177, The new localization
getters disputesEmptyState, disputeAttachFile, disputeWriteMessageHint, and
disputeSend were added to the base AppLocalizations interface but the generated
locale implementations (app_localizations_*.dart) are out of sync; regenerate
all localized files by running the project's localization generation step (e.g.,
the Flutter gen-l10n command or the repo's l10n generation script) so
app_localizations_en.dart, app_localizations_de.dart, app_localizations_es.dart,
app_localizations_fr.dart etc. are regenerated from the ARB sources and include
implementations for those four getters.

Comment thread rust/src/api/disputes.rs
Comment thread rust/src/api/disputes.rs
… cancel, i18n, routing

- disputes.rs: add update_conditional atomic read-modify-write to DisputeStore
- disputes.rs: handle_admin_took_dispute uses update_conditional (eliminates TOCTOU)
- disputes.rs: resolve_dispute uses update_conditional (eliminates TOCTOU)
- dispute_chat_screen: explicit cooperativeCancel branch in _ResolvedBanner
- app_routes.dart: /dispute_chat/:disputeId → DisputeChatScreen (was _Stub)
- disputes_providers: add disputeByTradeIdProvider (lookup by tradeId not id)
- disputes_providers: expand DisputeItem.copyWith to cover all mutable fields
- disputes_providers: add explanatory doc comment to userDisputeDataProvider
- trade_detail_screen: VIEW DISPUTE resolves disputeId via disputeByTradeIdProvider
- i18n: add orderDispute, disputeAdminAssigned, disputeChatClosed, messageCopied,
  disputeLoadError to all 5 locales and regenerate
- dispute_list_item, dispute_messages_list, disputes_list: use localized strings

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (1)
lib/features/trades/screens/trade_detail_screen.dart (1)

490-495: ⚠️ Potential issue | 🟠 Major

The null path reintroduces the orderId/disputeId mixup.

When disputeByTradeIdProvider(widget.orderId) is still null, this falls back to widget.orderId, but AppRoute.disputeDetailsPath() and DisputeChatScreen both expect a dispute ID. That sends users into the "Dispute not found." path instead of blocking until the lookup succeeds.

🔧 Suggested fix
               onPressed: () {
                 final dispute = ref.read(
                   disputeByTradeIdProvider(widget.orderId),
                 );
-                final targetId = dispute?.id ?? widget.orderId;
-                context.push(AppRoute.disputeDetailsPath(targetId));
+                if (dispute == null) {
+                  ScaffoldMessenger.of(context).showSnackBar(
+                    const SnackBar(content: Text('Dispute is still loading')),
+                  );
+                  return;
+                }
+                context.push(AppRoute.disputeDetailsPath(dispute.id));
               },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/trades/screens/trade_detail_screen.dart` around lines 490 - 495,
The button currently falls back to widget.orderId when
disputeByTradeIdProvider(widget.orderId) is null which mixes tradeId with
disputeId and navigates to AppRoute.disputeDetailsPath incorrectly; change the
handler to require a non-null dispute id (read disputeByTradeIdProvider and if
dispute == null either disable/return early or show a loading/error state) and
only call AppRoute.disputeDetailsPath(dispute.id) when dispute is present so
DisputeChatScreen receives a real disputeId (references:
disputeByTradeIdProvider, widget.orderId, AppRoute.disputeDetailsPath,
DisputeChatScreen).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/features/disputes/providers/disputes_providers.dart`:
- Around line 149-155: The dispute StateNotifier (disputeNotifierProvider /
DisputeNotifier) is never populated because no producer calls upsert(), so
consumers (userDisputeDataProvider, disputeByIdProvider,
disputeByTradeIdProvider) always see empty data; fix by adding a producer that
populates the notifier—either load initial disputes in DisputeNotifier's
constructor (call a repository/bridge fetch and call upsert() for each item) or
subscribe to bridge events and call upsert() on incoming dispute events; ensure
markRead() users (dispute_chat_screen.dart, dispute_list_item.dart) operate
against the notifier populated by this new fetch/subscribe flow.

In `@lib/features/disputes/screens/dispute_chat_screen.dart`:
- Around line 44-53: The chat UI currently appears live but is non-functional;
update DisputeChatScreen to disable/hide sending and attachment affordances and
surface an unavailable state until the Rust bridge is implemented: change the
hard-coded messages empty list usage to show a disabled/placeholder message
list, disable the composer send button by checking a new boolean (e.g.,
isBridgeAvailable) and make _onSendText and _onAttachFile no-ops that return
early when isBridgeAvailable is false (or show a toast/error UI), and ensure the
attachment spinner logic in _onAttachFile is removed or gated by the same flag;
reference the symbols _onSendText, _onAttachFile, messages and the composer
widget so the UI cannot accept input while rust/src/api/disputes.rs still
returns NotImplemented.

In `@rust/src/api/disputes.rs`:
- Around line 180-201: handle_admin_took_dispute currently only persists
admin_pubkey and skips deriving/storing the admin shared key; after the
dispute_store().update_conditional completes successfully, derive the admin
shared key from the trade key and the provided admin_pubkey (per the planned
ECDH flow) and call SessionManager::set_admin_shared_key for this trade/session
to store it; ensure you obtain the session manager instance (SessionManager) and
handle/propagate any errors from key derivation or set_admin_shared_key so the
handler returns an Err on failure rather than leaving the session without the
adminSharedKey.
- Around line 204-211: The current handlers handle_admin_settled and
handle_admin_canceled persist perspective-dependent enums
(DisputeResolution::FundsToMe / FundsToCounterparty), which can be
misinterpreted by the opposite party; change them to persist absolute outcomes
(e.g., DisputeResolution::FundsToBuyer / DisputeResolution::FundsToSeller) or
perform a role-aware mapping before calling resolve_dispute: fetch the
trade/participant role (buyer vs seller) for the given trade_id, then translate
admin-settled/admin-canceled into the correct absolute enum and call
resolve_dispute(trade_id, <absolute_resolution>) instead of using
FundsToMe/FundsToCounterparty so UI rendering in the Dart layer is correct for
both parties.

---

Duplicate comments:
In `@lib/features/trades/screens/trade_detail_screen.dart`:
- Around line 490-495: The button currently falls back to widget.orderId when
disputeByTradeIdProvider(widget.orderId) is null which mixes tradeId with
disputeId and navigates to AppRoute.disputeDetailsPath incorrectly; change the
handler to require a non-null dispute id (read disputeByTradeIdProvider and if
dispute == null either disable/return early or show a loading/error state) and
only call AppRoute.disputeDetailsPath(dispute.id) when dispute is present so
DisputeChatScreen receives a real disputeId (references:
disputeByTradeIdProvider, widget.orderId, AppRoute.disputeDetailsPath,
DisputeChatScreen).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4328a9b9-c39d-4d70-b5b9-3694548a74e3

📥 Commits

Reviewing files that changed from the base of the PR and between 7a81aa7 and 64ad3f9.

📒 Files selected for processing (19)
  • lib/core/app_routes.dart
  • lib/features/disputes/providers/disputes_providers.dart
  • lib/features/disputes/screens/dispute_chat_screen.dart
  • lib/features/disputes/widgets/dispute_list_item.dart
  • lib/features/disputes/widgets/dispute_messages_list.dart
  • lib/features/disputes/widgets/disputes_list.dart
  • lib/features/trades/screens/trade_detail_screen.dart
  • lib/l10n/app_de.arb
  • lib/l10n/app_en.arb
  • lib/l10n/app_es.arb
  • lib/l10n/app_fr.arb
  • lib/l10n/app_it.arb
  • lib/l10n/app_localizations.dart
  • lib/l10n/app_localizations_de.dart
  • lib/l10n/app_localizations_en.dart
  • lib/l10n/app_localizations_es.dart
  • lib/l10n/app_localizations_fr.dart
  • lib/l10n/app_localizations_it.dart
  • rust/src/api/disputes.rs
✅ Files skipped from review due to trivial changes (7)
  • lib/core/app_routes.dart
  • lib/l10n/app_localizations_en.dart
  • lib/l10n/app_localizations_de.dart
  • lib/l10n/app_de.arb
  • lib/l10n/app_es.arb
  • lib/l10n/app_fr.arb
  • lib/l10n/app_localizations.dart
🚧 Files skipped from review as they are similar to previous changes (6)
  • lib/l10n/app_it.arb
  • lib/l10n/app_localizations_it.dart
  • lib/l10n/app_localizations_es.dart
  • lib/l10n/app_localizations_fr.dart
  • lib/features/disputes/widgets/dispute_list_item.dart
  • lib/features/disputes/widgets/dispute_messages_list.dart

Comment on lines +149 to +155
/// Source-of-truth list of disputes.
///
/// Empty until bridge events are integrated (Phase 12+).
final disputeNotifierProvider =
StateNotifierProvider<DisputeNotifier, List<DisputeItem>>(
(_) => DisputeNotifier(),
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Nothing populates this notifier yet.

In the provided integrations, I only see consumers calling markRead() (lib/features/disputes/screens/dispute_chat_screen.dart:39-41 and lib/features/disputes/widgets/dispute_list_item.dart:1-50). Without any producer calling upsert(), this source of truth stays empty, so userDisputeDataProvider, disputeByIdProvider, and disputeByTradeIdProvider all resolve to no data and the new disputes tab/detail flow cannot surface real disputes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/providers/disputes_providers.dart` around lines 149 -
155, The dispute StateNotifier (disputeNotifierProvider / DisputeNotifier) is
never populated because no producer calls upsert(), so consumers
(userDisputeDataProvider, disputeByIdProvider, disputeByTradeIdProvider) always
see empty data; fix by adding a producer that populates the notifier—either load
initial disputes in DisputeNotifier's constructor (call a repository/bridge
fetch and call upsert() for each item) or subscribe to bridge events and call
upsert() on incoming dispute events; ensure markRead() users
(dispute_chat_screen.dart, dispute_list_item.dart) operate against the notifier
populated by this new fetch/subscribe flow.

Comment thread lib/features/disputes/screens/dispute_chat_screen.dart
Comment thread rust/src/api/disputes.rs
Comment on lines +180 to +201
/// Handle an incoming `adminTookDispute` event.
///
/// Extracts the admin pubkey, marks the dispute as `InReview`, and triggers
/// ECDH admin shared key derivation via the session manager.
///
/// TODO(Phase 12+): Derive `adminSharedKey` from trade key + admin pubkey
/// and store in the session via `SessionManager::set_admin_shared_key`.
pub async fn handle_admin_took_dispute(trade_id: String, admin_pubkey: String) -> Result<()> {
dispute_store()
.update_conditional(&trade_id, move |dispute| {
if dispute.status != DisputeStatus::Open {
return Err(anyhow!(
"InvalidState: dispute is not open (current: {:?})",
dispute.status
));
}
dispute.status = DisputeStatus::InReview;
dispute.admin_pubkey = Some(admin_pubkey);
dispute.is_read = false;
Ok(())
})
.await

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

adminTookDispute still never wires the admin shared key into the session.

This is the first point where the app knows admin_pubkey, but the handler only persists that pubkey on the dispute and leaves the key-derivation step as a TODO. The PR also adds Session.set_admin_shared_key, and the admin chat flow depends on that derived key for encrypted replies, so an InReview dispute is still missing the data it needs to talk to the admin.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rust/src/api/disputes.rs` around lines 180 - 201, handle_admin_took_dispute
currently only persists admin_pubkey and skips deriving/storing the admin shared
key; after the dispute_store().update_conditional completes successfully, derive
the admin shared key from the trade key and the provided admin_pubkey (per the
planned ECDH flow) and call SessionManager::set_admin_shared_key for this
trade/session to store it; ensure you obtain the session manager instance
(SessionManager) and handle/propagate any errors from key derivation or
set_admin_shared_key so the handler returns an Err on failure rather than
leaving the session without the adminSharedKey.

Comment thread rust/src/api/disputes.rs Outdated
…UI feedback, null safety

- types.rs + disputes_providers: rename FundsToMe→FundsToBuyer, FundsToCounterparty→FundsToSeller
  for role-independent semantics; both parties now read the same absolute outcome
- disputes.rs: update handle_admin_settled/canceled to use FundsToBuyer/FundsToSeller
- disputes.rs test assertion updated to match new enum names
- dispute_chat_screen: _ResolvedBanner derives userWon from absolute enum + isSelling
- dispute_chat_screen: add _lostResolutionText helper for role-aware outcome copy
- dispute_chat_screen: _onSendText/_onAttachFile show "coming soon" toast instead of
  silently swallowing input; remove fake _isAttaching spinner
- trade_detail_screen: VIEW DISPUTE shows error snackbar when dispute is null
  instead of incorrectly falling back to orderId

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
lib/features/disputes/providers/disputes_providers.dart (1)

164-168: ⚠️ Potential issue | 🟠 Major

disputeNotifierProvider is still unhydrated, so dispute UI can stay empty.

This provider is initialized empty and explicitly documented as empty until future integration. If no producer currently calls upsert, userDisputeDataProvider/lookup providers will never expose real disputes.

#!/bin/bash
# Verify whether any current Dart code actually populates DisputeNotifier via upsert.
rg -nP --type=dart -C2 'disputeNotifierProvider\.notifier\)\.upsert\s*\('
rg -nP --type=dart -C2 '\bupsert\s*\('

Expected result: at least one concrete producer path (initial load or event subscription) that writes disputes into DisputeNotifier.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/providers/disputes_providers.dart` around lines 164 -
168, The provider disputeNotifierProvider is created empty and never populated,
so add a concrete producer path that calls DisputeNotifier.upsert to
seed/hydrate it: either implement an initial load inside the DisputeNotifier
constructor (e.g., loadFromRepository/loadInitialDisputes) that fetches stored
disputes and calls upsert for each, or wire an event subscription (bridge
events/stream) in the provider factory to call
disputeNotifierProvider.notifier.upsert when disputes arrive; ensure the new
code references DisputeNotifier, its upsert method, and that the provider
factory (the closure passed to StateNotifierProvider) triggers this
loading/subscription so userDisputeDataProvider and lookup providers surface
real disputes.
🧹 Nitpick comments (1)
lib/features/disputes/screens/dispute_chat_screen.dart (1)

42-49: Localize new user-facing strings instead of hardcoding English text.

Snackbars, headers, status labels, and resolved-banner text are currently literal strings. This will bypass your l10n flow and produce mixed-language UX.

Also applies to: 52-59, 72-73, 147-157, 185-199, 245-267, 305-319, 355-416

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/screens/dispute_chat_screen.dart` around lines 42 - 49,
Replace all hardcoded user-facing strings in DisputeChatScreen with calls to the
app's localization API (e.g., use AppLocalizations.of(context).<key> or
equivalent) instead of literals; specifically update the SnackBar message in
_onSendText and the other literal strings at the ranges noted (headers, status
labels, resolved-banner text, etc.) by adding new localization keys to your l10n
resource files and using those keys in the widget code so the UI uses localized
strings throughout this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/features/disputes/providers/disputes_providers.dart`:
- Around line 140-148: The upsert implementation replaces existing DisputeItem
instances wholesale and can reset isRead to its default; modify the upsert in
disputes_providers.dart so that when idx >= 0 you preserve the current read
state by merging the existing state's isRead into the incoming dispute (e.g.,
use the existing item at state[idx] to copy its isRead into the new DisputeItem
via copyWith or by setting dispute.isRead = existing.isRead) before assigning
updated[idx] = dispute, ensuring new inserts still append as before.

In `@lib/features/disputes/screens/dispute_chat_screen.dart`:
- Around line 37-39: The post-frame callback registered with
WidgetsBinding.instance.addPostFrameCallback calls
ref.read(disputeNotifierProvider.notifier).markRead(widget.disputeId) without
checking lifecycle; wrap the body of that callback with a mounted guard (i.e.
check mounted before accessing ref or widget.disputeId) so you only call
markRead when the State (e.g., DisputeChatScreen State) is still mounted to
avoid touching provider state after disposal.
- Around line 209-211: Update the stale doc references in the comment block to
match the current DisputeResolution enum case names: open the DisputeResolution
enum declaration to copy the exact case identifiers and replace the outdated
DisputeResolution.fundsToMe and DisputeResolution.fundsToCounterparty references
(and any other renamed cases) in the three bullet lines (the lines mentioning
fundsToMe, cooperativeCancel, fundsToCounterparty) so the doc exactly matches
the enum names used by the code.

---

Duplicate comments:
In `@lib/features/disputes/providers/disputes_providers.dart`:
- Around line 164-168: The provider disputeNotifierProvider is created empty and
never populated, so add a concrete producer path that calls
DisputeNotifier.upsert to seed/hydrate it: either implement an initial load
inside the DisputeNotifier constructor (e.g.,
loadFromRepository/loadInitialDisputes) that fetches stored disputes and calls
upsert for each, or wire an event subscription (bridge events/stream) in the
provider factory to call disputeNotifierProvider.notifier.upsert when disputes
arrive; ensure the new code references DisputeNotifier, its upsert method, and
that the provider factory (the closure passed to StateNotifierProvider) triggers
this loading/subscription so userDisputeDataProvider and lookup providers
surface real disputes.

---

Nitpick comments:
In `@lib/features/disputes/screens/dispute_chat_screen.dart`:
- Around line 42-49: Replace all hardcoded user-facing strings in
DisputeChatScreen with calls to the app's localization API (e.g., use
AppLocalizations.of(context).<key> or equivalent) instead of literals;
specifically update the SnackBar message in _onSendText and the other literal
strings at the ranges noted (headers, status labels, resolved-banner text, etc.)
by adding new localization keys to your l10n resource files and using those keys
in the widget code so the UI uses localized strings throughout this file.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 903a9540-aedd-416e-8d30-a69e34d42b7d

📥 Commits

Reviewing files that changed from the base of the PR and between 64ad3f9 and da8c208.

📒 Files selected for processing (5)
  • lib/features/disputes/providers/disputes_providers.dart
  • lib/features/disputes/screens/dispute_chat_screen.dart
  • lib/features/trades/screens/trade_detail_screen.dart
  • rust/src/api/disputes.rs
  • rust/src/api/types.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • rust/src/api/types.rs
  • rust/src/api/disputes.rs
  • lib/features/trades/screens/trade_detail_screen.dart

Comment thread lib/features/disputes/providers/disputes_providers.dart
Comment thread lib/features/disputes/screens/dispute_chat_screen.dart
Comment thread lib/features/disputes/screens/dispute_chat_screen.dart Outdated
…ute strings

- Add mounted guard inside addPostFrameCallback in DisputeChatScreen.initState
- Update stale _ResolvedBanner doc comment to use fundsToBuyer/fundsToSeller
- Localize SnackBar strings in dispute_chat_screen and trade_detail_screen
- Preserve isRead across bridge-driven upserts in DisputeNotifier
- Add 4 new l10n keys (disputeMessagingComingSoon, disputeAttachmentsComingSoon,
  disputeNotFound, disputeNotFoundForOrder) across all 5 locale ARBs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (4)
lib/features/disputes/screens/dispute_chat_screen.dart (4)

226-287: Resolved banner text should use l10n keys.

The banner contains multiple hardcoded English strings:

  • Line 253: 'Resolved'
  • Line 263: 'The order was cooperatively cancelled. No funds were transferred.'
  • Line 275: 'This dispute has been resolved. The chat is closed.'

Note that disputeChatClosed already exists in the ARB files and could be used for line 275.

♻️ Suggested fix for line 275
+                  final l10n = AppLocalizations.of(context);
                   Text(
-                    'This dispute has been resolved. The chat is closed.',
+                    l10n.disputeChatClosed,
                     style: textTheme.bodySmall?.copyWith(
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/screens/dispute_chat_screen.dart` around lines 226 -
287, The cooperative-cancel banner in DisputeChatScreen's build (inside the
DisputeResolution.cooperativeCancel branch) contains hardcoded English strings
for the title and two messages; replace those Text literals ('Resolved', 'The
order was cooperatively cancelled. No funds were transferred.', 'This dispute
has been resolved. The chat is closed.') with localized strings from the app
l10n (use the existing disputeChatClosed key for the third message and
add/choose appropriate ARB keys for the title and second sentence), calling the
localization accessor (e.g., AppLocalizations.of(context).<keyName>) wherever
those Text widgets are created so the banner uses l10n instead of hardcoded
text.

137-165: Hardcoded English strings in header should use l10n.

The header contains several hardcoded English strings that should be localized for consistency with the l10n effort in this PR:

  • Line 141: 'Buyer' / 'Seller'
  • Line 153: 'Dispute with $role: $handle'
  • Line 162: 'Order ${...}'
♻️ Suggested approach

Add corresponding keys to the ARB files (e.g., disputeWithBuyer, disputeWithSeller, orderLabel) and use them here:

-    final role = dispute.isSelling ? 'Buyer' : 'Seller';
+    final l10n = AppLocalizations.of(context);
+    final role = dispute.isSelling ? l10n.buyer : l10n.seller;
     ...
     Text(
-      'Dispute with $role: $handle',
+      l10n.disputeWithRole(role, handle),
       ...
     ),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/screens/dispute_chat_screen.dart` around lines 137 -
165, Replace hardcoded English strings in the header with localized strings:
instead of setting role = dispute.isSelling ? 'Buyer' : 'Seller' and using
literal 'Dispute with $role: $handle' and 'Order ${...}', add keys to the ARB
files (e.g., disputeWithBuyer, disputeWithSeller, orderLabel) and call the
localization API in the build method (e.g., AppLocalizations.of(context) or
existing l10n helper) to compute role and the two Text widgets; update the Text
widgets to use the localized disputeWithBuyer/disputeWithSeller (including
$handle) and orderLabel (including the tradeId truncation) so all three
hardcoded strings are replaced by l10n lookups while preserving the existing
string interpolation and truncation logic.

406-425: _lostResolutionText returns hardcoded English strings.

This helper returns role-aware outcome messages that should be localized. Additionally, note that the branches for fundsToBuyer && !isSelling (lines 413-414) and fundsToSeller && isSelling (lines 418-420) are unreachable dead code given the userWon check that guards the call site — consider simplifying to only the reachable cases.

♻️ Simplified logic (reachable cases only)
  static String _lostResolutionText(DisputeItem dispute) {
+   // Called only when user lost: fundsToBuyer+isSelling OR fundsToSeller+!isSelling
    if (dispute.resolution == DisputeResolution.fundsToBuyer) {
-     return dispute.isSelling
-         ? 'The administrator settled the dispute in the buyer\'s favour. '
-           'The sats were released to the buyer.'
-         : 'The administrator settled the dispute in your favour. '
-           'You received the sats.';
+     // User is seller (lost) — buyer received funds
+     return 'The administrator settled the dispute in the buyer\'s favour. '
+            'The sats were released to the buyer.';
    }
    if (dispute.resolution == DisputeResolution.fundsToSeller) {
-     return dispute.isSelling
-         ? 'The administrator canceled the order. '
-           'The sats were returned to you.'
-         : 'The administrator canceled the order and returned the sats to the seller. '
-           'You did not receive the sats.';
+     // User is buyer (lost) — seller kept funds
+     return 'The administrator canceled the order and returned the sats to the seller. '
+            'You did not receive the sats.';
    }
    return 'The dispute has been resolved.';
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/screens/dispute_chat_screen.dart` around lines 406 -
425, _lostResolutionText currently returns hardcoded English strings and
contains branches that are unreachable due to the userWon guard at the call
site; update _lostResolutionText(DisputeItem dispute) to use the app's
localization mechanism (e.g., call the appropriate l10n getters or translate
function) for every returned message and remove/simplify the unreachable
branches so only the reachable cases remain (handle
DisputeResolution.fundsToBuyer and DisputeResolution.fundsToSeller for the
actual reachable isSelling values), keeping references to
DisputeResolution.fundsToBuyer, DisputeResolution.fundsToSeller, and
DisputeItem.isSelling so you can locate and update the logic consistently with
the existing localization patterns used elsewhere in the codebase.

189-207: Status chip labels should be localized.

The status labels 'Initiated', 'In progress', and 'Closed' are hardcoded English strings. Consider adding l10n keys for these.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/features/disputes/screens/dispute_chat_screen.dart` around lines 189 -
207, The _statusChip function currently returns hardcoded English labels
('Initiated', 'In progress', 'Closed'); change it to use localized strings by
either (a) adding a BuildContext parameter to _statusChip and replacing the
literal strings with AppLocalizations.of(context).disputeInitiated /
.disputeInProgress / .disputeClosed, or (b) return a localization key enum/value
from _statusChip and resolve it to AppLocalizations in the widget caller; also
add the corresponding l10n keys (disputeInitiated, disputeInProgress,
disputeClosed) to your ARB/localization files and update translations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@lib/features/disputes/screens/dispute_chat_screen.dart`:
- Around line 226-287: The cooperative-cancel banner in DisputeChatScreen's
build (inside the DisputeResolution.cooperativeCancel branch) contains hardcoded
English strings for the title and two messages; replace those Text literals
('Resolved', 'The order was cooperatively cancelled. No funds were
transferred.', 'This dispute has been resolved. The chat is closed.') with
localized strings from the app l10n (use the existing disputeChatClosed key for
the third message and add/choose appropriate ARB keys for the title and second
sentence), calling the localization accessor (e.g.,
AppLocalizations.of(context).<keyName>) wherever those Text widgets are created
so the banner uses l10n instead of hardcoded text.
- Around line 137-165: Replace hardcoded English strings in the header with
localized strings: instead of setting role = dispute.isSelling ? 'Buyer' :
'Seller' and using literal 'Dispute with $role: $handle' and 'Order ${...}', add
keys to the ARB files (e.g., disputeWithBuyer, disputeWithSeller, orderLabel)
and call the localization API in the build method (e.g.,
AppLocalizations.of(context) or existing l10n helper) to compute role and the
two Text widgets; update the Text widgets to use the localized
disputeWithBuyer/disputeWithSeller (including $handle) and orderLabel (including
the tradeId truncation) so all three hardcoded strings are replaced by l10n
lookups while preserving the existing string interpolation and truncation logic.
- Around line 406-425: _lostResolutionText currently returns hardcoded English
strings and contains branches that are unreachable due to the userWon guard at
the call site; update _lostResolutionText(DisputeItem dispute) to use the app's
localization mechanism (e.g., call the appropriate l10n getters or translate
function) for every returned message and remove/simplify the unreachable
branches so only the reachable cases remain (handle
DisputeResolution.fundsToBuyer and DisputeResolution.fundsToSeller for the
actual reachable isSelling values), keeping references to
DisputeResolution.fundsToBuyer, DisputeResolution.fundsToSeller, and
DisputeItem.isSelling so you can locate and update the logic consistently with
the existing localization patterns used elsewhere in the codebase.
- Around line 189-207: The _statusChip function currently returns hardcoded
English labels ('Initiated', 'In progress', 'Closed'); change it to use
localized strings by either (a) adding a BuildContext parameter to _statusChip
and replacing the literal strings with
AppLocalizations.of(context).disputeInitiated / .disputeInProgress /
.disputeClosed, or (b) return a localization key enum/value from _statusChip and
resolve it to AppLocalizations in the widget caller; also add the corresponding
l10n keys (disputeInitiated, disputeInProgress, disputeClosed) to your
ARB/localization files and update translations.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5467ae8c-9e32-4499-b74c-0e706bce7e3c

📥 Commits

Reviewing files that changed from the base of the PR and between da8c208 and 6215d84.

📒 Files selected for processing (14)
  • lib/features/disputes/providers/disputes_providers.dart
  • lib/features/disputes/screens/dispute_chat_screen.dart
  • lib/features/trades/screens/trade_detail_screen.dart
  • lib/l10n/app_de.arb
  • lib/l10n/app_en.arb
  • lib/l10n/app_es.arb
  • lib/l10n/app_fr.arb
  • lib/l10n/app_it.arb
  • lib/l10n/app_localizations.dart
  • lib/l10n/app_localizations_de.dart
  • lib/l10n/app_localizations_en.dart
  • lib/l10n/app_localizations_es.dart
  • lib/l10n/app_localizations_fr.dart
  • lib/l10n/app_localizations_it.dart
✅ Files skipped from review due to trivial changes (3)
  • lib/l10n/app_it.arb
  • lib/l10n/app_localizations_en.dart
  • lib/l10n/app_de.arb
🚧 Files skipped from review as they are similar to previous changes (6)
  • lib/features/trades/screens/trade_detail_screen.dart
  • lib/l10n/app_es.arb
  • lib/l10n/app_localizations_it.dart
  • lib/l10n/app_localizations_es.dart
  • lib/l10n/app_fr.arb
  • lib/l10n/app_localizations.dart

- Localize header title (disputeWithBuyer/Seller with handle placeholder)
- Localize order sub-title (orderLabel with orderId placeholder)
- Localize status chip labels (disputeInitiated/InProgress/StatusClosed)
- Localize cooperative-cancel banner using disputeResolved, disputeCoopCancelMessage,
  disputeChatClosed
- Localize won-dispute banner (disputeSuccessfullyCompleted, disputeChatClosed)
- Localize lost-dispute banner (disputeResolved, disputeChatClosed)
- Replace _lostResolutionText hardcoded strings with l10n keys; remove
  unreachable branches — only isSelling check needed at call site
- Add 11 new ARB keys across all 5 locale files
@grunch
grunch merged commit 4c2b410 into main Mar 30, 2026
1 check passed
@grunch
grunch deleted the 006-mostro-p2p-client branch March 30, 2026 20:34
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.

1 participant