refactor: remove dispute token functionality and related translations #294
Conversation
WalkthroughDisputeToken was removed from the Dispute model and its (de)serialization. Notification payloads for dispute events now use Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Notifier as AbstractMostroNotifier
participant System as Notification System
participant UI as NotificationDetails
participant Model as Dispute Model
Note over Model: disputeToken removed from model and (de)serialization
Notifier->>System: sendNotification(eventId, data: { dispute_id: dispute.disputeId })
System-->>UI: deliver payload (includes dispute_id)
UI->>UI: if data.dispute_id present -> render "notificationDisputeId" row
UI-->>User: display dispute_id
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
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.
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. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/features/notifications/widgets/notification_details.dart (1)
88-94: Switch to dispute_id looks correct; consider a legacy-safe fallback to 'dispute'If older backends/clients might still send 'dispute', you can show it without reintroducing tokens.
- if (data.containsKey('dispute_id')) { + final disputeId = data['dispute_id'] ?? data['dispute']; + if (disputeId != null && disputeId.toString().isNotEmpty) { widgets.add(DetailRow( label: S.of(context)!.notificationDisputeId, - value: _formatHashOrId(context, data['dispute_id']), + value: _formatHashOrId(context, disputeId), icon: HeroIcons.exclamationTriangle, )); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/data/models/dispute.dart(1 hunks)lib/features/notifications/widgets/notification_details.dart(1 hunks)lib/features/order/notfiers/abstract_mostro_notifier.dart(2 hunks)lib/l10n/intl_en.arb(0 hunks)lib/l10n/intl_es.arb(0 hunks)lib/l10n/intl_it.arb(0 hunks)
💤 Files with no reviewable changes (3)
- lib/l10n/intl_en.arb
- lib/l10n/intl_it.arb
- lib/l10n/intl_es.arb
🧰 Additional context used
📓 Path-based instructions (3)
{lib/*.dart,lib/!(generated)/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart}: Use localized strings; replace hardcoded user-facing text withS.of(context).keyName
Preferconstconstructors andconstwhere possible
Use latest Flutter/Dart APIs (e.g.,withValues()instead of deprecatedwithOpacity())
Files:
lib/features/notifications/widgets/notification_details.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/data/models/dispute.dart
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}: Checkmountedbefore usingBuildContextafterawait(async gaps)
Remove unused imports and dependencies
Files:
lib/features/notifications/widgets/notification_details.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/data/models/dispute.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
All code comments, variable names, and function names must be in English
Files:
lib/features/notifications/widgets/notification_details.dartlib/features/order/notfiers/abstract_mostro_notifier.dartlib/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 (2)
lib/data/models/dispute.dart (1)
304-305: hashCode/toString updated to drop the token — LGTMBoth now reflect the current fields only; no stale references to the removed token remain.
Also applies to: 307-308
lib/features/order/notfiers/abstract_mostro_notifier.dart (1)
282-284: Approve: notifier emitsdispute_idwith no leftover token or i18n keys
All token-based keys/fields and obsolete ARB entries have been removed;dispute_idusage is consistent.
|
@AndreaDiazCorreia Please update this PR with the latest changes from |
mostrod expect a message with expires_at: null
There was a problem hiding this comment.
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/order.dart (1)
199-217: Data loss risk in DB mapping: trade pubkeys not persisted bytoMap
fromMapreadsbuyerTradePubkey/sellerTradePubkey, buttoMapnever writes them. Round-tripping through storage drops these fields.Map<String, dynamic> toMap() { return { @@ 'masterSellerPubkey': masterSellerPubkey, + 'buyerTradePubkey': buyerTradePubkey, + 'sellerTradePubkey': sellerTradePubkey, 'buyerInvoice': buyerInvoice, 'createdAt': createdAt, 'expiresAt': expiresAt, }; }Also applies to: 219-238
🧹 Nitpick comments (4)
lib/data/models/order.dart (4)
67-67: Out-of-scope wire-format change: avoid emittingexpires_at: nullunless API requires itThis PR is about dispute token removal; always including
expires_at(possibly null) changes the payload. If the backend rejects nulls or treats them differently, this can break requests. Prefer the prior conditional emit unless you’ve validated the contract.- data[type]['expires_at'] = expiresAt; + if (expiresAt != null) { + data[type]['expires_at'] = expiresAt; + }
48-62: Round-trip mismatch:toJsonnests undertype("order"),fromJsonreads flat
toJsonreturns{ "order": { ... } }whilefromJsonexpects top-level fields. Objects you serialize withtoJsonwon’t parse withfromJsonas-is.factory Order.fromJson(Map<String, dynamic> json) { try { + // Support both nested `{ "order": { ... } }` and flat payloads + final Map<String, dynamic> payload = + (json['order'] is Map<String, dynamic>) + ? (json['order'] as Map<String, dynamic>) + : json; // Validate required fields - void validateField(String field) { - if (!json.containsKey(field) || json[field] == null) { + void validateField(String field) { + if (!payload.containsKey(field) || payload[field] == null) { throw FormatException('Missing required field: $field'); } } @@ - int parseIntField(String field, {int defaultValue = 0}) { - final value = json[field]; + int parseIntField(String field, {int defaultValue = 0}) { + final value = payload[field]; @@ - String parseStringField(String field) { - final value = json[field]; + String parseStringField(String field) { + final value = payload[field]; @@ - String? parseOptionalStringField(String field) { - final value = json[field]; + String? parseOptionalStringField(String field) { + final value = payload[field]; @@ - int? parseOptionalIntField(String field) { - final value = json[field]; + int? parseOptionalIntField(String field) { + final value = payload[field];Also applies to: 78-83
1-1: Remove unused import
dart_nostr/nostr/model/event/event.dartisn’t referenced in this file.-import 'package:dart_nostr/nostr/model/event/event.dart';
189-196: Safer numeric casts fromNostrEventCast via
numto avoidTypeErrorwhen values aredoubleor other numeric types.- amount: event.amount as int, + amount: (event.amount as num).toInt(), @@ - premium: event.premium as int, + premium: (event.premium as num).toInt(), @@ - createdAt: event.createdAt as int, - expiresAt: event.expiration as int?, + createdAt: (event.createdAt as num).toInt(), + expiresAt: (event.expiration as num?)?.toInt(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/data/models/order.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{lib/*.dart,lib/!(generated)/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart}: Use localized strings; replace hardcoded user-facing text withS.of(context).keyName
Preferconstconstructors andconstwhere possible
Use latest Flutter/Dart APIs (e.g.,withValues()instead of deprecatedwithOpacity())
Files:
lib/data/models/order.dart
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
{lib/*.dart,lib/!(generated)/**/*.dart,test/**/*.dart}: Checkmountedbefore usingBuildContextafterawait(async gaps)
Remove unused imports and dependencies
Files:
lib/data/models/order.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
All code comments, variable names, and function names must be in English
Files:
lib/data/models/order.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
All references to the dispute token were removed because it is no longer used.
Summary by CodeRabbit