Skip to content

Conversation

xsahil03x
Copy link
Member

@xsahil03x xsahil03x commented Oct 13, 2025

Submit a pull request

Fixes: FLU-266

Description of the pull request

Adds support for the user.messages.deleted WebSocket event.

This event is now handled at the client level and propagated to all active channels. Each channel will then proceed to either soft or hard delete all messages from the specified user, depending on the event payload.

Refactors message update and removal logic to support bulk operations, improving performance and maintainability.

Summary by CodeRabbit

  • New Features

    • Support for user-level message deletions via a new user.messages.deleted event that is applied across channels.
  • Refactor

    • Centralized, batched message update/remove flows for more consistent thread and channel state handling.
  • Tests

    • Added tests covering user-based soft/hard deletions across channels and threads.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 13, 2025

Walkthrough

Adds EventType.userMessagesDeleted, wires a global listener that redispatches user-scoped deletions to each cached channel, and refactors channel state to perform batched message updates/removals and to handle user deletions via centralized internal helpers. (33 words)

Changes

Cohort / File(s) Summary
Event wiring for global user deletions
packages/stream_chat/lib/src/client/client.dart
Adds _listenUserMessagesDeleted and invokes it from subscribeToEvents; when user.messages.deleted arrives without a cid, iterates cached channels, clones the event with each channel's cid, and dispatches it via _client.handleEvent.
Channel state: batched message ops & user-deletion handling
packages/stream_chat/lib/src/client/channel.dart
Refactors channel internals to centralize batched updates/removals: adds _updateMessages*, _removeMessages*, _deleteMessagesFromUser, _mergeMessagesIntoExisting, unified thread/channel update flows, persistence delete calls, and initializes user-deletion listener in state setup.
Event type constant
packages/stream_chat/lib/src/event_type.dart
Adds EventType.userMessagesDeleted = 'user.messages.deleted'.
Tests: client & channel
packages/stream_chat/test/src/client/channel_test.dart, packages/stream_chat/test/src/client/client_test.dart
New tests covering user.messages.deleted scenarios: soft deletes (marking messages deleted), hard deletes (removing messages and persisting deletions), thread handling, no-op cases, empty lists, and global broadcast across cached channels.
Changelog
packages/stream_chat/CHANGELOG.md
Documents upcoming support for the user.messages.deleted event.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Server
  participant ClientState
  participant CachedChannels
  participant ChannelState
  participant Storage

  note over Server,ClientState: Server emits user.messages.deleted (no cid)
  Server->>ClientState: user.messages.deleted
  ClientState->>ClientState: _listenUserMessagesDeleted
  ClientState->>CachedChannels: list cached channel CIDs
  loop per cached channel
    ClientState->>ClientState: clone event with channel.cid
    ClientState->>ChannelState: _client.handleEvent(event with cid)
    activate ChannelState
    ChannelState->>ChannelState: _deleteMessagesFromUser(userId)
    ChannelState->>ChannelState: batch _removeChannelMessages / _removeThreadMessages / _updateMessagesIntoOriginal
    ChannelState->>Storage: persist removals/updates (deleteMessageByIds / deletePinnedMessageByIds)
    deactivate ChannelState
  end

  note over ChannelState: Batched helpers centralize update/remove flows and quotedMessage handling
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

  • renefloor
  • Brazol

Poem

I twitched my whiskers at the drum,
Hopped through channels, tidy and hum —
Softly flagged or gone outright,
Threads and quotes put back to right.
A happy hop, the meadow's bright. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely describes the main feature being introduced, namely handling the user.messages.deleted event, without extraneous details or vague terminology, matching the primary change in the pull request.
Linked Issues Check ✅ Passed The changes implement the new user.messages.deleted event per FLU-266 by adding the event constant, wiring up client‐level listening and propagation to channels, extending channel logic for soft and hard deletions with persistence calls, refactoring bulk update/removal flows, and including comprehensive tests for these scenarios.
Out of Scope Changes Check ✅ Passed All modified files and logic relate directly to introducing and handling the user.messages.deleted event and its bulk update/removal support; there are no unrelated or out‐of‐scope changes present.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/handle-user-message-deleted-event

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.

@codecov
Copy link

codecov bot commented Oct 13, 2025

Codecov Report

❌ Patch coverage is 90.37037% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.06%. Comparing base (1f501c0) to head (695db6c).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
packages/stream_chat/lib/src/client/channel.dart 89.76% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2412      +/-   ##
==========================================
+ Coverage   63.93%   64.06%   +0.12%     
==========================================
  Files         413      413              
  Lines       25898    25961      +63     
==========================================
+ Hits        16559    16632      +73     
+ Misses       9339     9329      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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: 1

🧹 Nitpick comments (4)
packages/stream_chat/lib/src/client/client.dart (1)

2284-2301: Snapshot channel keys to avoid concurrent modification during redispatch

If channel map mutates during handling, iterating channels.keys can throw. Snapshot before looping.

Apply this diff:

   _eventsSubscription?.add(
     _client.on(EventType.userMessagesDeleted).listen((event) async {
       final cid = event.cid;
       // Only handle message deletions that are not channel specific
       // (i.e. user banned globally from the app)
       if (cid != null) return;

-      // Iterate through all the available channels and send the event
-      // to be handled by the respective channel instances.
-      for (final cid in channels.keys) {
+      // Iterate through a snapshot of available channels and send the event
+      // to be handled by the respective channel instances.
+      final cids = [...channels.keys];
+      for (final cid in cids) {
         final channelEvent = event.copyWith(cid: cid);
         _client.handleEvent(channelEvent);
       }
     }),
   );
packages/stream_chat/lib/src/client/channel.dart (3)

3490-3511: Thread updates are correct; minor naming nit

Method name _updatedThreadMessages reads past-tense; consider _updateThreadMessages for consistency.


3512-3553: Avoid using deleted/invalid messages to advance lastMessageAt

When soft-deleting, the last element may be a deleted/system/non-visible message. Pick the last eligible message instead.

Apply this diff:

-    var lastMessageAt = _channelState.channel?.lastMessageAt;
-    final lastMessage = updatedChannelMessages.lastOrNull;
-    if (lastMessage != null && _shouldUpdateChannelLastMessageAt(lastMessage)) {
-      lastMessageAt = [lastMessageAt, lastMessage.createdAt].nonNulls.max;
-    }
+    var lastMessageAt = _channelState.channel?.lastMessageAt;
+    final lastEligible = updatedChannelMessages
+        .lastWhereOrNull(_shouldUpdateChannelLastMessageAt);
+    if (lastEligible != null) {
+      lastMessageAt = [lastMessageAt, lastEligible.createdAt].nonNulls.max;
+    }

3726-3735: Exclude deleted messages from pinned list

A deleted message should not remain “validly pinned”.

Apply this diff:

 bool _pinIsValid(Message message) {
   // If the message is not pinned, it's not valid.
   if (message.pinned != true) return false;
+  // Deleted messages should not be considered valid pins.
+  if (message.type == MessageType.deleted) return false;
 
   // If there's no expiration, the pin is valid.
   final pinExpires = message.pinExpires;
   if (pinExpires == null) return true;
 
   // If there's an expiration, check if it's still valid.
   return pinExpires.isAfter(DateTime.now());
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7291e67 and b49d80d.

📒 Files selected for processing (4)
  • packages/stream_chat/CHANGELOG.md (1 hunks)
  • packages/stream_chat/lib/src/client/channel.dart (4 hunks)
  • packages/stream_chat/lib/src/client/client.dart (2 hunks)
  • packages/stream_chat/lib/src/event_type.dart (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
PR: GetStream/stream-chat-flutter#2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message with MessageSendingStatus.failed or MessageSendingStatus.failed_update status, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.

Applied to files:

  • packages/stream_chat/lib/src/client/channel.dart
  • packages/stream_chat/lib/src/client/client.dart
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
PR: GetStream/stream-chat-flutter#2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message that hasn't been sent to the server yet (message.remoteCreatedAt == null) or is bounced with error, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.

Applied to files:

  • packages/stream_chat/lib/src/client/channel.dart
  • packages/stream_chat/lib/src/client/client.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). (10)
  • GitHub Check: build (ios)
  • GitHub Check: stream_chat_flutter
  • GitHub Check: analyze
  • GitHub Check: analyze_legacy_versions
  • GitHub Check: stream_chat_localizations
  • GitHub Check: build (android)
  • GitHub Check: test
  • GitHub Check: stream_chat_flutter_core
  • GitHub Check: stream_chat_persistence
  • GitHub Check: stream_chat
🔇 Additional comments (15)
packages/stream_chat/CHANGELOG.md (1)

11-11: Changelog entry looks good

Accurately reflects added support for the event.

packages/stream_chat/lib/src/event_type.dart (1)

180-182: New event type is well-defined

Constant name and value are clear and consistent.

packages/stream_chat/lib/src/client/client.dart (1)

2180-2182: Hook registration is correct

Subscribing to userMessagesDeleted at client state init is appropriate.

packages/stream_chat/lib/src/client/channel.dart (12)

2212-2213: Channel now listens for user-level deletions

Good addition; aligns channel with client redispatch.


2938-2938: Unified update entry point

Delegating to batched updater is a solid refactor.


2942-2948: Stale error cleanup via batched remove

Looks good; reduces noise in state.


2951-2951: Unified remove entry point

Consistent with new batched flow.


3472-3480: Delete dispatcher reads cleanly

Soft vs hard path separation is clear.


3482-3488: Batched update entry-point is fine

Keeps thread/channel updates in sync.


3555-3590: Merge logic preserves quoted references—nice

Sync and quotedMessage preservation are correct.


3591-3603: Batched removal integrates persistence—good

Deletes from storage and in-memory consistently.


3604-3636: Thread removal path looks solid

Handles empty-thread cleanup and quote nulling.


3638-3671: Channel removal path is correct

Respects showInChannel for thread replies.


3673-3691: Removal merge correctly clears quotes

Efficient and clear.


3693-3708: Per-channel user-level delete handling is correct

Uses event flags and createdAt appropriately.

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: 1

🧹 Nitpick comments (1)
packages/stream_chat/lib/src/client/channel.dart (1)

3472-3546: Consider extracting channel message filtering logic.

The logic at lines 3505-3514 (filtering messages by parentId and showInChannel) is duplicated in _removeChannelMessages (lines 3634-3643). While not critical, extracting this into a helper like _filterChannelVisibleMessages would reduce duplication.

Example helper:

List<Message> _filterChannelVisibleMessages(List<Message> messages) {
  return [
    ...messages.map((it) {
      if (it.parentId == null) return it;
      if (it.showInChannel == true) return it;
      return null;
    }).nonNulls,
  ];
}

Then use it in both methods:

void _updateChannelMessages(List<Message> messages) {
  if (messages.isEmpty) return;
  final affectedMessages = _filterChannelVisibleMessages(messages);
  if (affectedMessages.isEmpty) return;
  // ... rest of the logic
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e196980 and 5472bf6.

📒 Files selected for processing (2)
  • packages/stream_chat/lib/src/client/channel.dart (6 hunks)
  • packages/stream_chat/lib/src/client/client.dart (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/stream_chat/lib/src/client/client.dart
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
PR: GetStream/stream-chat-flutter#2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message with MessageSendingStatus.failed or MessageSendingStatus.failed_update status, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.

Applied to files:

  • packages/stream_chat/lib/src/client/channel.dart
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
PR: GetStream/stream-chat-flutter#2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message that hasn't been sent to the server yet (message.remoteCreatedAt == null) or is bounced with error, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.

Applied to files:

  • packages/stream_chat/lib/src/client/channel.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). (9)
  • GitHub Check: build (ios)
  • GitHub Check: test
  • GitHub Check: analyze
  • GitHub Check: build (android)
  • GitHub Check: stream_chat_flutter_core
  • GitHub Check: stream_chat_localizations
  • GitHub Check: stream_chat_flutter
  • GitHub Check: stream_chat_persistence
  • GitHub Check: stream_chat
🔇 Additional comments (8)
packages/stream_chat/lib/src/client/channel.dart (8)

2212-2212: LGTM: User message deletion listener correctly initialized.

The _listenUserMessagesDeleted() listener is properly initialized during ChannelClientState construction, enabling the channel to respond to global user message deletion events.


2938-2938: LGTM: Clean delegation to batch operations.

The refactor of updateMessage and removeMessage to delegate to batch methods (_updateMessages and _removeMessages) is a good architectural improvement that consolidates message processing logic.

Also applies to: 2951-2951


3443-3460: LGTM: User deletion correctly processes both channel and thread messages.

The implementation correctly iterates through both messages and threads.values.flattened (line 3449), ensuring thread replies from the deleted user are included. The use of a map keyed by message ID avoids duplicates for thread messages shown in the channel.


3462-3470: LGTM: Clean separation of soft vs hard delete flows.

The _deleteMessages method properly delegates to _removeMessages for hard deletes and _updateMessages for soft deletes, maintaining consistency with the rest of the codebase.


3548-3582: LGTM: Comprehensive merge logic with quoted message handling.

The _mergeMessagesIntoExisting helper correctly:

  • Merges messages using syncWith to preserve state
  • Preserves quotedMessage references when updates omit them (lines 3561-3563)
  • Updates quotedMessage references in messages that quote the updated messages (lines 3569-3579)

This ensures quoted message integrity throughout the state updates.


3666-3684: LGTM: Clean quoted message reference cleanup on removal.

The _removeMessagesFromExisting method correctly clears both quotedMessageId and quotedMessage (line 3679) when the quoted message is hard-deleted, preventing dangling references.


3688-3701: LGTM: User messages deleted event handler correctly wired.

The event listener properly extracts user ID and hard delete flag, with appropriate null-safety checks and default values.


3718-3728: LGTM: Pin validation improvement.

The explicit check for pins without expiration (line 3724) improves code clarity and correctly allows indefinite pins to remain valid.

@xsahil03x xsahil03x requested a review from renefloor October 13, 2025 16:26
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: 1

🧹 Nitpick comments (2)
packages/stream_chat/test/src/client/client_test.dart (1)

3914-3982: Consider clarifying test intent with assertion message.

The test correctly verifies that channel-specific events aren't broadcast to other channels. Consider adding assertion messages to make the intent clearer:

// Verify only channel1 is affected (by channel's own handler)
expect(
  channel1.state?.messages.any((m) => m.user?.id == 'banned-user'),
  isFalse,
  reason: 'Channel1 should handle its own event and delete banned user messages',
);

// Verify channel2 is unaffected (client state didn't broadcast it)
expect(
  channel2.state?.messages.length,
  equals(1),
  reason: 'Channel2 should not receive channel1-specific events',
);
expect(
  channel2.state?.messages.any((m) => m.user?.id == 'banned-user'),
  isTrue,
  reason: 'Channel2 should still contain banned user message',
);

This makes the test's verification logic more explicit and easier to debug if it fails.

packages/stream_chat/test/src/client/channel_test.dart (1)

5199-5471: Good test coverage for user message deletion events.

The test group provides solid coverage of the main scenarios:

  • ✅ Soft delete behavior
  • ✅ Hard delete behavior
  • ✅ Thread message handling
  • ✅ Null user edge case
  • ✅ Empty list edge case

However, consider these optional improvements:

  1. Verify timestamp in soft delete (line 5252): The deletedAt variable is created but not used in assertions. Consider verifying that message.deletedAt equals the event's createdAt:
expect(message.deletedAt?.isAtSameMomentAs(deletedAt), isTrue);
  1. Parent message deletion: Add a test case where the parent message itself is from the deleted user to verify it's handled correctly alongside its thread messages.

  2. Already-deleted messages: Consider testing what happens when some messages are already deleted before the event arrives.

  3. Stream emissions: Consider verifying that messagesStream and threadsStream properly emit the updated state after deletion.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ca304f7 and 7e82150.

📒 Files selected for processing (2)
  • packages/stream_chat/test/src/client/channel_test.dart (1 hunks)
  • packages/stream_chat/test/src/client/client_test.dart (2 hunks)
⏰ 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). (9)
  • GitHub Check: analyze_legacy_versions
  • GitHub Check: build (ios)
  • GitHub Check: build (android)
  • GitHub Check: test
  • GitHub Check: stream_chat_localizations
  • GitHub Check: stream_chat_flutter
  • GitHub Check: stream_chat_persistence
  • GitHub Check: stream_chat
  • GitHub Check: stream_chat_flutter_core
🔇 Additional comments (1)
packages/stream_chat/test/src/client/client_test.dart (1)

3823-3824: Excellent cleanup pattern.

All tests properly dispose channel instances after completion, preventing resource leaks and ensuring test isolation. This follows best practices for test cleanup.

Also applies to: 3909-3910, 3979-3980

@xsahil03x xsahil03x force-pushed the feat/handle-user-message-deleted-event branch from 5c1e40a to 81825c7 Compare October 14, 2025 14:08
Adds support for the `user.messages.deleted` WebSocket event.

This event is now handled at the client level and propagated to all active channels. Each channel will then proceed to either soft or hard delete all messages from the specified user, depending on the event payload.

Refactors message update and removal logic to support bulk operations, improving performance and maintainability.
The `clearThread` method is simplified to call `updateThreadInfo`.
The logic inside `updateThreadInfo` is refactored for clarity, replacing the `merge` and `sorted` chain with a more explicit update process.
The method `_updatedThreadMessages` is renamed to `_updateThreadMessages`.
When a user is banned, their messages in threads are now also marked as deleted, in addition to their messages directly in the channel.
The `channels.keys` iterable is now cloned before iteration to prevent concurrent modification exceptions.

Additionally, the logic for updating a channel's `lastMessageAt` timestamp has been corrected to consider all messages, not just the very last one.
Refactors the tests for the `user.messages.deleted` WebSocket event to improve reliability and correctness.

- Connects the client and waits for a connected status in `setUp`, ensuring a realistic test environment.
- Simplifies event dispatching by sending a single global event instead of one per channel.
- Removes an unnecessary test for channel-specific `user.messages.deleted` events, as these are handled by the channel itself.
- Adds tests to verify that messages are correctly deleted from the persistence client when `hardDelete` is true.
@xsahil03x xsahil03x force-pushed the feat/handle-user-message-deleted-event branch from 81825c7 to 695db6c Compare October 14, 2025 14:10
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)
packages/stream_chat/test/src/client/channel_test.dart (1)

5199-5581: LGTM! Comprehensive test coverage for user message deletion.

The test group thoroughly validates the user.messages.deleted event handling with well-structured test cases covering:

✅ Soft delete behavior with proper state marking
✅ Hard delete with complete removal from state
✅ Thread message handling independent of parent messages
✅ Edge cases (null user, empty message list)
✅ Persistence integration for both soft and hard deletes

The tests correctly verify:

  • State updates (message type, deletedAt timestamps, state flags)
  • Selective user filtering (only target user's messages affected)
  • Persistence method calls with correct message IDs
  • No persistence calls for soft deletes

Optional enhancements to consider:

  1. Pinned message scenario: Add a test case where deleted messages include pinned messages to verify the deletePinnedMessageByIds call includes the correct subset.
test(
  'should delete pinned messages from persistence when hardDelete is true',
  () async {
    final user1 = User(id: 'user-1');
    final pinnedMessage = Message(
      id: 'msg-1',
      text: 'Pinned message',
      user: user1,
      pinned: true,
    );
    final unpinnedMessage = Message(
      id: 'msg-2', 
      text: 'Regular message',
      user: user1,
      pinned: false,
    );
    
    channel.state?.updateMessage(pinnedMessage);
    channel.state?.updateMessage(unpinnedMessage);
    
    final event = Event(
      cid: channel.cid,
      type: EventType.userMessagesDeleted,
      user: user1,
      hardDelete: true,
    );
    
    client.addEvent(event);
    await Future.delayed(Duration.zero);
    
    // Verify only pinned message ID passed to deletePinnedMessageByIds
    verify(() => persistenceClient.deletePinnedMessageByIds(['msg-1'])).called(1);
    verify(() => persistenceClient.deleteMessageByIds(['msg-1', 'msg-2'])).called(1);
  },
);
  1. Persistence error handling: Consider adding a test to verify graceful handling if persistence operations fail during hard delete.

  2. State update ordering: Add an assertion to verify state is updated before persistence calls are made (if order matters for consistency).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 81825c7 and 695db6c.

📒 Files selected for processing (6)
  • packages/stream_chat/CHANGELOG.md (1 hunks)
  • packages/stream_chat/lib/src/client/channel.dart (6 hunks)
  • packages/stream_chat/lib/src/client/client.dart (2 hunks)
  • packages/stream_chat/lib/src/event_type.dart (1 hunks)
  • packages/stream_chat/test/src/client/channel_test.dart (1 hunks)
  • packages/stream_chat/test/src/client/client_test.dart (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/stream_chat/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/stream_chat/lib/src/client/client.dart
  • packages/stream_chat/lib/src/event_type.dart
  • packages/stream_chat/test/src/client/client_test.dart
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
PR: GetStream/stream-chat-flutter#2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message with MessageSendingStatus.failed or MessageSendingStatus.failed_update status, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.

Applied to files:

  • packages/stream_chat/lib/src/client/channel.dart
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
PR: GetStream/stream-chat-flutter#2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message that hasn't been sent to the server yet (message.remoteCreatedAt == null) or is bounced with error, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.

Applied to files:

  • packages/stream_chat/lib/src/client/channel.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). (10)
  • GitHub Check: build (ios)
  • GitHub Check: build (android)
  • GitHub Check: analyze
  • GitHub Check: test
  • GitHub Check: stream_chat_localizations
  • GitHub Check: stream_chat_flutter
  • GitHub Check: stream_chat_flutter_core
  • GitHub Check: stream_chat_persistence
  • GitHub Check: stream_chat
  • GitHub Check: analyze_legacy_versions
🔇 Additional comments (7)
packages/stream_chat/lib/src/client/channel.dart (7)

2212-2212: LGTM: User messages deleted listener initialized.

The initialization properly wires up the event listener during channel state construction, enabling automatic handling of user.messages.deleted events.


2938-2938: LGTM: Refactored to use centralized batch operations.

Delegating to _updateMessages and _removeMessages improves maintainability by consolidating message update/removal logic, ensuring consistent handling across all code paths.

Also applies to: 2951-2951


3450-3477: LGTM: User message deletion implementation is comprehensive.

The implementation correctly:

  • Includes both channel and thread messages via threads.values.flattened (line 3456), addressing the past review concern
  • Deduplicates by message ID using a Map (line 3455), preventing duplicate processing
  • Marks messages with appropriate deleted state (lines 3458-3462)
  • Routes to either soft or hard deletion via _deleteMessages (line 3466)

3479-3551: LGTM: Centralized batch update operations improve consistency.

The refactored update flow:

  • Efficiently filters affected messages (lines 3489-3491, 3512-3524)
  • Maintains thread and channel message separation correctly
  • Updates pinned messages with proper validation (lines 3532-3536)
  • Preserves lastMessageAt consistency (lines 3539-3544)

3553-3587: LGTM: Comprehensive quoted message handling.

The merge implementation properly:

  • Preserves quotedMessage objects when merging (lines 3566-3568)
  • Updates quotedMessage references in messages quoting updated messages (lines 3574-3584)
  • Uses efficient Map-based lookups (line 3574)

This ensures quoted message data remains consistent across updates.


3589-3689: LGTM: Centralized batch remove operations with proper cleanup.

The removal flow correctly:

  • Cleans up persistence via both deleteMessageByIds and deletePinnedMessageByIds (lines 3593-3596)
  • Removes empty thread entries to prevent stale data (lines 3622-3626)
  • Clears quotedMessage references when removing quoted messages (lines 3680-3684)
  • Uses Sets for efficient ID lookups (lines 3592, 3677)

3693-3706: LGTM: Event listener properly handles user message deletions.

The listener correctly:

  • Validates event data before processing (line 3697)
  • Delegates to _deleteMessagesFromUser with appropriate parameters
  • Defaults hardDelete to false when not specified (line 3701)
  • Uses the event's createdAt as the deletion timestamp (line 3702)

@xsahil03x
Copy link
Member Author

Closing in favor of #2423

@xsahil03x xsahil03x closed this Oct 21, 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.

1 participant