Phase 2 user-admin chat: unify dispute chat message model with NostrEvent and gift wrap storage#501
Phase 2 user-admin chat: unify dispute chat message model with NostrEvent and gift wrap storage#501
Conversation
…rap storage - Replace DisputeChat model with DisputeChatMessage wrapper around NostrEvent - Store gift wrap events (encrypted) on disk, consistent with P2P chat pattern - Reconstruct and unwrap gift wraps on historical message load - Add getAdminSharedKey() for future multimedia support (Phase 3) - Add isFromUser() method to notifier for sender determination - Update dispute_messages_list, dispute_content, and read status service - Update plan doc: Phase 1 and Phase 2 marked as DONE
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (1)
WalkthroughThis change refactors the dispute chat system by introducing a UI-friendly DisputeChatMessage wrapper containing NostrEvent with pending/error state, replacing raw DisputeChat usage. The implementation switches to single-layer p2pWrap encryption, adds persistent storage for encrypted gift-wrap events, and updates related widgets and services accordingly. Changes
Sequence DiagramsequenceDiagram
participant User as User/Network
participant Notifier as DisputeChatNotifier
participant Storage as Disk Storage
participant Service as DisputeReadStatusService
participant UI as UI Widgets
User->>Notifier: New gift-wrap event received (_onChatEvent)
Notifier->>Notifier: Unwrap & extract inner event
Notifier->>Storage: Persist wrapped event
Storage-->>Notifier: Stored
Notifier->>Notifier: Create DisputeChatMessage(event, isPending=false)
Notifier->>Notifier: Update state.messages
Notifier-->>UI: Notify state change
User->>Notifier: Send message (sendMessage)
Notifier->>Notifier: Create DisputeChatMessage(isPending=true)
Notifier->>Notifier: Update state with pending message
Notifier-->>UI: Show pending indicator
Notifier->>Notifier: Wrap via p2pWrap
Notifier->>Notifier: Publish wrapped event
Notifier->>Storage: Persist wrapped event
Storage-->>Notifier: Stored
Notifier->>Notifier: Update message (isPending=false)
Notifier-->>UI: Message confirmed
UI->>Notifier: Load historical messages (_loadHistoricalMessages)
Notifier->>Storage: Fetch stored wrapped events
Storage-->>Notifier: Wrapped events
Notifier->>Notifier: Unwrap all events
Notifier->>Notifier: Create DisputeChatMessage list
Notifier->>Notifier: Update state.messages
Notifier-->>UI: Render message list
UI->>Service: Check unread status (hasUnreadMessages)
Service->>Service: Filter messages using isFromUser predicate
Service-->>UI: Unread status result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 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/notifiers/dispute_chat_notifier.dart`:
- Line 28: The timestamp getter currently falls back to DateTime.now() which
yields non-deterministic ordering; change the fallback in the timestamp getter
(DateTime get timestamp) to a deterministic value such as
DateTime.fromMillisecondsSinceEpoch(0) or another stable field on event (e.g.,
event.updatedAt or an event ID-derived timestamp) instead of DateTime.now(),
leaving the primary use of event.createdAt intact.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/DISPUTE_CHAT_MULTIMEDIA_PLAN.mdlib/features/disputes/notifiers/dispute_chat_notifier.dartlib/features/disputes/widgets/dispute_content.dartlib/features/disputes/widgets/dispute_messages_list.dartlib/services/dispute_read_status_service.dart
…ring bug - Per-message send errors now stay at message level only - Removed state.error propagation from _updateMessageState - Prevents full-chat error overlay on individual message failures
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean refactor that properly unifies the dispute chat model with the P2P chat pattern. Key improvements:
- Encrypted storage: Storing gift wraps (encrypted) on disk instead of decrypted content is the correct approach — consistent with P2P chat and better for privacy.
- DisputeChatMessage wrapper: Thin, focused wrapper around NostrEvent with UI-only state (pending/error) — good separation of concerns.
isFromUser()via pubkey comparison: Better than storingis_from_userflag — single source of truth._updateMessageState()extraction: Eliminates significant duplication in error handling paths.getAdminSharedKey(): Good forward-thinking for Phase 3 multimedia.
CI passes, no conflicts, no unresolved comments. LGTM ✅
Catrya
requested review from
AndreaDiazCorreia,
BraCR10,
arkanoider and
grunch
arkanoider
left a comment
arkanoider
left a comment
There was a problem hiding this comment.
tACK! I tested with 3 disputes from mobile and cli, really cool! Messages are going very well! Great job @Catrya !
There was a problem hiding this comment.
I tested several disputes, and the expected features planned in this PR are working properly, good job!
However, I would like to point out some details regarding chat restore handling. After testing, I noticed the following:
- When using the same device to switch between buyer and seller accounts, the app keeps the previous user's chat. This might be happening because the dispute ID remains the same and the chat data is not being cleared.
- Once users restores their account, there is no possibility to send or receive messages between the admin and the user. The admin does not receive messages, and vice versa.
I reviewed the implementation plan but did not find a specific phase addressing this scenario. Perhaps this is intended to be handled in a later phase ? or could be treat in other issue
3 participants
Summary by CodeRabbit
New Features
Documentation