feat(llc, ui): add delete message for me #2394
Conversation
This commit introduces the ability to delete a message only for the current user. The following changes are included: - Added `MessageDeleteScope` to represent the scope of deletion for a message. - Added `deleteMessageForMe` method to `StreamChatClient` and `Channel` to delete a message only for the current user. - Updated `Message` model to include `deletedOnlyForMe` field. - Updated `Event` model to include `deletedForMe` field. - Updated `Member` model to include `deletedMessages` field. - Updated `MessageState` to include `deletingForMe`, `deletedForMe`, and `deletingForMeFailed` states. - Updated `MessageApi` to include `delete_for_me` parameter in `deleteMessage` method. - Updated sample app to include "Delete Message for Me" option in message actions.
This commit introduces a new `sentenceCase` extension for strings and replaces all usages of the deprecated `capitalize` extension with it. The `sentenceCase` extension converts a string to sentence case, where the first letter is capitalized and the rest of the string is in lowercase. This provides more accurate capitalization for various UI elements. The `capitalize` extension has been deprecated and will be removed in a future release.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds per-user ("for me") deletion scope: introduces MessageDeleteScope, makes message state and API scope-aware, adds Channel/Client delete-for-me methods, updates models/serialization/tests, adjusts attachment and local-delete flows, and updates Flutter UI casing utilities and sample app action. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as App UI
participant Channel as Channel
participant Client as Client
participant API as MessageApi
participant Server
rect rgba(220,240,255,0.5)
User->>UI: Tap "Delete for me"
UI->>Channel: deleteMessageForMe(message)
Channel->>Client: deleteMessageForMe(message.id)
Client->>API: deleteMessage(id, deleteForMe=true)
API->>Server: DELETE /messages/{id}?delete_for_me=true
Server-->>API: 200 EmptyResponse
API-->>Client: EmptyResponse
Client-->>Channel: EmptyResponse
Channel->>Channel: update message -> Deleted(scope: DeleteForMe)
Channel-->>UI: notify updated message
end
alt Delete for all (soft/hard)
UI->>Channel: deleteMessage(message, hard=?)
Channel->>Client: deleteMessage(message.id, hard)
Client->>API: deleteMessage(id, hard=flag)
API->>Server: DELETE /messages/{id}?hard=flag
Server-->>API: 200
API-->>Client: EmptyResponse
Channel->>Channel: update message -> Deleted(scope: DeleteForAll{hard})
opt hard delete
Channel->>Server: delete attachments
Server-->>Channel: 2xx
end
Channel-->>UI: notify updated message
else Local-only / bounced
UI->>Channel: deleteMessage(local)
Channel->>Channel: remove local message (no API)
Channel-->>UI: notify removed message
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sample_app/lib/pages/channel_page.dart (1)
396-402: Hide messages deleted only for the current user.After “Delete for me,” the message likely remains visible because
defaultFilterdoesn’t checkdeletedOnlyForMe. Update it:bool defaultFilter(Message m) { final currentUser = StreamChat.of(context).currentUser; final isMyMessage = m.user?.id == currentUser?.id; if (m.deletedOnlyForMe == true) return false; final isDeletedOrShadowed = m.isDeleted == true || m.shadowed == true; if (isDeletedOrShadowed && !isMyMessage) return false; return true; }
🧹 Nitpick comments (10)
packages/stream_chat_flutter/lib/src/attachment/builder/url_attachment_builder.dart (1)
68-71: Consistent casing LGTM; consider safer host parsing (optional).Switch to sentenceCase improves UI consistency. As an optional hardening, avoid a crash if
titleLinkis null/invalid by usingUri.tryParseand guarding empty hosts.Apply:
- final host = Uri.parse(urlPreview.titleLink!).host; - final splitList = host.split('.'); - final hostName = splitList.length == 3 ? splitList[1] : splitList[0]; + final host = Uri.tryParse(urlPreview.titleLink ?? '')?.host ?? ''; + final splitList = host.split('.'); + final hostName = splitList.length == 3 + ? splitList[1] + : (splitList.isNotEmpty ? splitList.first : ''); final hostDisplayName = urlPreview.authorName?.sentenceCase ?? getWebsiteName(hostName.toLowerCase()) ?? hostName.sentenceCase;packages/stream_chat_flutter/lib/src/utils/extensions.dart (1)
47-60: Deprecation + new sentenceCase: good; handle grapheme clusters to avoid splitting emojis/combined chars.Current logic uses code units; multi-codepoint graphemes (e.g., emoji, accented chars) may get split. Prefer Characters API (already used elsewhere in this file).
Apply:
@Deprecated('Use sentenceCase instead') - String capitalize() => sentenceCase; + String capitalize() => sentenceCase; /// Returns the string in sentence case. /// /// Example: 'hello WORLD' -> 'Hello world' String get sentenceCase { - if (isEmpty) return this; - - final firstChar = this[0].toUpperCase(); - final restOfString = substring(1).toLowerCase(); - - return '$firstChar$restOfString'; + if (isEmpty) return this; + final chars = characters; + final first = chars.first.toUpperCase(); + final rest = chars.skip(1).toString().toLowerCase(); + return '$first$rest'; }packages/stream_chat/lib/src/core/models/event.dart (1)
126-128: Naming consistency: align withMessage.deletedOnlyForMe.
Event usesdeletedForMewhile Message usesdeletedOnlyForMe. Consider unifying names across models to avoid mental mapping.packages/stream_chat/lib/src/client/client.dart (1)
1760-1770: Verify backend support & document side-effects fordeleteMessageForMe
ConfirmedMessageApi.deleteMessageaccepts and sends thedelete_for_meflag. Recommend updating the Dart docs to note that callingdeleteMessageForMeemits anEvent.deletedForMe(updatingMember.deletedMessagesandMessageState.isDeletedForMe) and that channels apply an optimistic UI update.packages/stream_chat/test/src/client/channel_test.dart (1)
2310-2381: New deleteMessageForMe flow tests are on point.Covers happy path and the “sending/failed” early-hard-delete (local removal) behavior. Consider adding a thread-message case to ensure thread state updates mirror channel state.
packages/stream_chat/lib/src/core/models/message_delete_scope.dart (1)
6-11: Docstring fixes for accuracy (rename references and a typo).Update references to match actual factory names and fix a minor type name typo.
-/// - [forMe]: The message is deleted only for the current user. -/// - [forAll]: The message is deleted for all users. The [hard] -/// parameter indicates whether the deletion is permanent (hard) or soft. +/// - [deleteForMe]: The message is deleted only for the current user. +/// - [deleteForAll]: The message is deleted for all users. +/// Use [hard] to indicate whether the deletion is permanent (hard) or soft. @@ - /// Creates a MessageDeletionScope from a JSON map. + /// Creates a MessageDeleteScope from a JSON map.Also applies to: 30-33
packages/stream_chat/lib/src/core/models/message_state.freezed.dart (1)
1107-1149: DeletingFailed now carries scope — OK.Equality/hash and copyWith updated accordingly. Confirm error-path telemetry/logs include the scope to aid debugging.
Also applies to: 1152-1194
packages/stream_chat/test/src/core/models/member_test.dart (1)
15-15: LGTM: Asserting deletedMessages parsing.Good coverage for the new field; consider adding a small test for copyWith(deletedMessages) later.
sample_app/lib/pages/channel_page.dart (1)
378-394: Handle errors when deleting for me.Currently the call is ignored; surface failures to users.
Apply:
- final channel = StreamChannel.of(context).channel; - return channel.deleteMessageForMe(message).ignore(); + final channel = StreamChannel.of(context).channel; + try { + await channel.deleteMessageForMe(message); + } catch (e) { + if (!mounted) return; + ScaffoldMessenger.of(context).showSnackBar( + const SnackBar(content: Text('Failed to delete message for you')), + ); + }packages/stream_chat/lib/src/core/models/member.dart (1)
29-31: New deletedMessages field — OK, but use immutable view and deep equality.
- Good: top-level field, default to empty list, copyWith wiring.
- Concern: Equatable’s default List equality is by identity, not by contents. Also, exposing a mutable List can cause accidental mutation.
Recommended changes (non-generated):
- Wrap
deletedMessagesin an unmodifiable view.- Use deep equality in props (or ensure stable identity by wrapping).
Example:
import 'package:collection/collection.dart'; final List<String> deletedMessages; Member({ ... List<String> deletedMessages = const [], ... }) : deletedMessages = List.unmodifiable(deletedMessages), ... @override List<Object?> get props => [ ..., const ListEquality().equals, // or just keep props but ensure unmodifiable identity ];Alternatively, keep
propsas-is but storeEqualUnmodifiableListView(deletedMessages)to stabilize identity.Also applies to: 57-59, 103-108, 129-129, 147-148, 172-173
📜 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.
📒 Files selected for processing (28)
packages/stream_chat/lib/src/client/channel.dart(6 hunks)packages/stream_chat/lib/src/client/client.dart(1 hunks)packages/stream_chat/lib/src/core/api/message_api.dart(1 hunks)packages/stream_chat/lib/src/core/models/event.dart(5 hunks)packages/stream_chat/lib/src/core/models/event.g.dart(2 hunks)packages/stream_chat/lib/src/core/models/member.dart(6 hunks)packages/stream_chat/lib/src/core/models/member.g.dart(2 hunks)packages/stream_chat/lib/src/core/models/message.dart(8 hunks)packages/stream_chat/lib/src/core/models/message.g.dart(1 hunks)packages/stream_chat/lib/src/core/models/message_delete_scope.dart(1 hunks)packages/stream_chat/lib/src/core/models/message_delete_scope.freezed.dart(1 hunks)packages/stream_chat/lib/src/core/models/message_delete_scope.g.dart(1 hunks)packages/stream_chat/lib/src/core/models/message_state.dart(30 hunks)packages/stream_chat/lib/src/core/models/message_state.freezed.dart(12 hunks)packages/stream_chat/lib/src/core/models/message_state.g.dart(3 hunks)packages/stream_chat/lib/stream_chat.dart(1 hunks)packages/stream_chat/test/fixtures/member.json(1 hunks)packages/stream_chat/test/src/client/channel_test.dart(6 hunks)packages/stream_chat/test/src/client/client_test.dart(2 hunks)packages/stream_chat/test/src/core/models/member_test.dart(1 hunks)packages/stream_chat/test/src/core/models/message_state_test.dart(7 hunks)packages/stream_chat_flutter/lib/src/attachment/builder/url_attachment_builder.dart(1 hunks)packages/stream_chat_flutter/lib/src/attachment_actions_modal/attachment_actions_modal.dart(1 hunks)packages/stream_chat_flutter/lib/src/autocomplete/stream_command_autocomplete_options.dart(1 hunks)packages/stream_chat_flutter/lib/src/message_widget/giphy_ephemeral_message.dart(3 hunks)packages/stream_chat_flutter/lib/src/message_widget/message_widget.dart(0 hunks)packages/stream_chat_flutter/lib/src/utils/extensions.dart(1 hunks)sample_app/lib/pages/channel_page.dart(5 hunks)
💤 Files with no reviewable changes (1)
- packages/stream_chat_flutter/lib/src/message_widget/message_widget.dart
🧰 Additional context used
🪛 Gitleaks (8.28.0)
packages/stream_chat/test/src/client/client_test.dart
[high] 28-28: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
⏰ 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). (2)
- GitHub Check: build (ios)
- GitHub Check: build (android)
🔇 Additional comments (52)
packages/stream_chat_flutter/lib/src/message_widget/giphy_ephemeral_message.dart (1)
162-162: UI copy casing alignment LGTM.Updated labels to
sentenceCaseare consistent with the new extension.Also applies to: 176-176, 190-190
packages/stream_chat_flutter/lib/src/autocomplete/stream_command_autocomplete_options.dart (1)
68-69: Command title casing LGTM.Switch to
sentenceCasematches the shared extension and avoids deprecatedcapitalize().packages/stream_chat_flutter/lib/src/attachment_actions_modal/attachment_actions_modal.dart (1)
198-199: Delete action label casing LGTM.Adopts
sentenceCaseand stays consistent with other UI labels.packages/stream_chat_flutter/lib/src/utils/extensions.dart (1)
47-49: No lingering.capitalize()usages found. All call sites now usesentenceCase, so deprecation warnings will be clean.packages/stream_chat/lib/src/core/models/event.dart (3)
33-33: Good addition:deletedForMesurfaced in constructor.
Wires the new flag cleanly into the model.Please confirm which event types include
deleted_for_mefrom the API so downstream consumers can rely on it (e.g., message.deleted vs. custom events).
196-197: Top-level field mapping looks correct.
deleted_for_meadded to known fields keeps the serializer behavior consistent.
241-242: CopyWith wiring is complete.
deletedForMeis included in both the signature and the initializer; no gaps spotted.Also applies to: 279-279
packages/stream_chat/lib/src/core/models/event.g.dart (1)
51-51: JSON mapping fordeleted_for_meis correct.
Round-trips the new field as expected.Also applies to: 109-109
packages/stream_chat/lib/src/core/models/message_delete_scope.g.dart (1)
18-27: Scope (de)serialization is solid.
Defaults and discriminators look correct.packages/stream_chat/test/fixtures/member.json (1)
15-19: Approve fixture update and mapping
Member.deletedMessagesmaps todeleted_messagesand is asserted in member_test.dart.packages/stream_chat/lib/src/core/models/message_delete_scope.freezed.dart (1)
88-107: No action required:MessageDeleteScope.softDeleteForAllis already defined as a static constant in message_delete_scope.dartpackages/stream_chat/lib/src/core/models/message_state.g.dart (1)
74-82: Drop migration suggestion: no legacy top-levelhardkey in message_state JSONLikely an incorrect or invalid review comment.
packages/stream_chat/lib/src/client/client.dart (1)
1747-1758: No action needed: local state and persistence are already handled
Channel.deleteMessage updates in-memory state via ChannelState.deleteMessage and invokes chatPersistenceClient.deleteMessageById, so offline data won’t remain stale.packages/stream_chat/lib/src/core/models/message.dart (3)
86-101: Per-user delete state/type override order looks correct.Good precedence: deletedOnlyForMe → state: deletedForMe and type: deleted; else softDeleted via deletedAt; else updated via timestamps. Keep this until backend enriches type; please track and remove the TODO once server sends enriched types.
323-327: JSON contract for deleted_for_me is consistent.
- Field mapped from JSON: name deleted_for_me, default false.
- Excluded from toJson and added to topLevelFields for correct extraction.
LGTM.Also applies to: 376-377
436-437: Propagation into copyWith/merge/props is complete.deletedOnlyForMe correctly flows through copyWith/merge and participates in equality.
Also applies to: 515-516, 629-630
packages/stream_chat/test/src/core/models/message_state_test.dart (1)
78-111: Solid coverage for scope-aware deleting/deleted/failed states.The tests clearly validate soft/hard DeleteForAll and DeleteForMe across Outgoing/Completed/Failed. Nicely done.
Also applies to: 140-175, 204-238, 261-299, 318-356, 395-433
packages/stream_chat/test/src/client/channel_test.dart (2)
2243-2273: Hard-delete cleans up attachments as expected.Verifies image/file deletion counts correctly. Looks good.
7285-7343: Retry preserves delete scope correctly.Retrying for hard/soft delete-for-all and delete-for-me routes to the right client methods. LGTM.
packages/stream_chat/lib/src/core/models/message_delete_scope.dart (1)
11-29: Union design and ergonomics look good.Freezed union, predefined constants, and hard getter are clean and intuitive. No issues.
Also applies to: 34-47, 49-61
packages/stream_chat/lib/src/core/api/message_api.dart (1)
180-191: Rejecting incompatible query params is correct, but we still allowdelete_for_me=false.Nice guard on the mutually-exclusive flags and thanks for wiring the new query parameter.
packages/stream_chat/lib/src/core/models/message_state.dart (22)
4-4: LGTM: Proper import for new MessageDeleteScope model.The import statement correctly references the new
MessageDeleteScopemodel that enables scope-based message deletion.
46-50: LGTM: Scope-based soft deleting logic is correct.The implementation correctly checks for
DeleteForAllscope and ensures it's not a hard delete by checking!deletingScope.hard. The logic properly differentiates between soft and hard deletion within theDeleteForAllscope.
60-64: LGTM: Scope-based hard deleting logic is correct.The implementation correctly identifies hard deleting by checking for
DeleteForAllscope withdeletingScope.hardset to true. This properly distinguishes hard deletion from soft deletion.
66-76: LGTM: New isDeletingForMe getter implementation.The new getter correctly identifies when a message is being deleted only for the current user by checking for the
DeleteForMescope type. The logic is consistent with the pattern used for other deletion state checks.
91-91: LGTM: Comprehensive deleted state logic.The updated
isDeletedgetter now correctly encompasses all deleted states: soft deleted, hard deleted, and deleted for me. This provides complete coverage of deletion completion states.
101-105: LGTM: Scope-based soft deleted logic is correct.The implementation correctly identifies soft deleted messages by checking for
DeleteForAllscope with!deletingScope.hard. This maintains consistency with the deleting state logic.
115-119: LGTM: Scope-based hard deleted logic is correct.The implementation correctly identifies hard deleted messages by checking for
DeleteForAllscope withdeletingScope.hardset to true, maintaining consistency with the deleting state logic.
121-131: LGTM: New isDeletedForMe getter implementation.The new getter correctly identifies when a message has been deleted only for the current user by checking for the
DeleteForMescope type. The implementation follows the established pattern and is consistent with other state checks.
150-151: LGTM: Comprehensive deleting failed state logic.The updated
isDeletingFailedgetter now correctly encompasses all failed deletion states: soft deleting failed, hard deleting failed, and deleting for me failed. This provides complete coverage of deletion failure states.
161-165: LGTM: Scope-based soft deleting failed logic is correct.The implementation correctly identifies soft deleting failures by checking for
DeleteForAllscope with!deletingScope.hard, maintaining consistency with other soft deletion state checks.
175-179: LGTM: Scope-based hard deleting failed logic is correct.The implementation correctly identifies hard deleting failures by checking for
DeleteForAllscope withdeletingScope.hardset to true, maintaining consistency with other hard deletion state checks.
181-191: LGTM: New isDeletingForMeFailed getter implementation.The new getter correctly identifies when deleting a message for the current user has failed by checking for the
DeleteForMescope type. The implementation follows the established pattern and maintains consistency.
221-290: LGTM: Factory constructors updated to use MessageDeleteScope.The factory constructors have been properly updated to require
MessageDeleteScopeinstead of a booleanhardparameter. This provides better type safety and clearer intent. The constructors correctly pass the scope to the underlying state objects.
292-372: LGTM: Comprehensive static instances for all deletion scopes.The static instances have been properly expanded to cover all deletion scenarios:
- Hard deletion:
hardDeleting,hardDeleted,hardDeletingFailed- Delete for me:
deletingForMe,deletedForMe,deletingForMeFailedThe implementations correctly use the appropriate
MessageDeleteScopevalues and maintain consistency across all deletion types.
385-387: LGTM: OutgoingState.deleting updated with proper defaults.The
deletingfactory constructor now usesMessageDeleteScopewith a sensible default ofMessageDeleteScope.softDeleteForAll. This maintains backward compatibility while enabling the new scope-based functionality.
404-406: LGTM: CompletedState.deleted updated with proper defaults.The
deletedfactory constructor now usesMessageDeleteScopewith a sensible default ofMessageDeleteScope.softDeleteForAll, maintaining consistency with the outgoing state.
435-437: LGTM: FailedState.deletingFailed updated with proper defaults.The
deletingFailedfactory constructor now usesMessageDeleteScopewith a sensible default ofMessageDeleteScope.softDeleteForAll, maintaining consistency across all deletion-related states.
564-564: LGTM: OutgoingStatePatternMatching updated for scope parameter.The pattern matching methods for
OutgoingStatehave been correctly updated to pass theMessageDeleteScope scopeparameter instead of a booleanhardparameter. This maintains the pattern matching API while supporting the new scope-based deletion model.Also applies to: 579-579, 594-594
663-663: LGTM: CompletedStatePatternMatching updated for scope parameter.The pattern matching methods for
CompletedStatehave been correctly updated to pass theMessageDeleteScope scopeparameter, maintaining consistency with theOutgoingStatepattern matching updates.Also applies to: 678-678, 693-693
765-765: LGTM: FailedStatePatternMatching updated for scope parameter.The pattern matching methods for
FailedStatehave been correctly updated to pass theMessageDeleteScope scopeparameter, completing the consistent update of all pattern matching interfaces to support the new scope-based deletion model.Also applies to: 787-787, 809-809
36-36: Approve deletion state logic.isDeletingnow covers soft, hard, and “for me” deletion states, and all related getters and factory methods are implemented and fully tested.
570-570: No pattern matching calls to update
Searches for.when(deleting…),.whenOrNull(deleting…)and.maybeWhen(deleting…)returned no occurrences; all pattern-matching methods now useMessageDeleteScopeand no caller updates are needed.packages/stream_chat/lib/src/core/models/message_state.freezed.dart (2)
476-517: Scope-based deleting state looks correct; verify JSON wiring.Adding
scopewith defaultMessageDeleteScope.softDeleteForAlland updating equality/hash is consistent. Please re-run build and confirmmessage_state.g.dartincludesscopein (de)serialization forDeleting.Also applies to: 521-561
672-714: Deleted state migrated to scope — OK; double-check server payload compatibility.
CompletedState.deleted(scope: ...)reads well. Ensure downstream consumers expectscopeinstead of the previous boolean (if any) to avoid deserialization errors.Also applies to: 717-757
packages/stream_chat/lib/stream_chat.dart (1)
50-50: Publicly exporting MessageDeleteScope — OK.This aligns the API surface with the new scope-based delete flow.
packages/stream_chat/lib/src/core/models/member.g.dart (2)
40-44: Deserialization of deleted_messages — OK.Defaults to empty list; types are correct.
62-62: Serialization includes deleted_messages — OK.Matches the new Member field and top-level schema.
packages/stream_chat/lib/src/core/models/message.g.dart (1)
57-57: New Message field deletedOnlyForMe (from deleted_for_me) — OK.Not serializing it back is appropriate if server controls this flag.
Please confirm message.dart declares
@JsonKey(name: 'deleted_for_me') final bool deletedOnlyForMe;(default false).sample_app/lib/pages/channel_page.dart (3)
1-1: Ignore rules update — OK.No concerns.
215-219: Permission gating variables — OK.Clear separation of author check and permissions.
232-232: Icon updates — OK.Consistent use of StreamSvgIcon/Icon variants.
Also applies to: 238-238, 244-244, 249-249
This commit renames the `DeleteForMe` class and its usages to `DeleteMessageForMe` for clarity.
The documentation for MessageDeleteScope was updated to reflect the new names for the enum values. `forMe` was changed to `deleteForMe` and `forAll` was changed to `deleteForAll`.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
sample_app/lib/pages/channel_page.dart (1)
441-443: Make the action classfinallike the others.The other custom action types in this file are declared
final class. MarkingDeleteMessageForMeasfinalkeeps the API consistent and prevents accidental extension.Apply this diff:
-class DeleteMessageForMe extends CustomMessageAction { +final class DeleteMessageForMe extends CustomMessageAction { const DeleteMessageForMe({required super.message}); }
📜 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.
📒 Files selected for processing (3)
packages/stream_chat/lib/src/core/models/message_delete_scope.dart(1 hunks)packages/stream_chat/test/src/client/client_test.dart(1 hunks)sample_app/lib/pages/channel_page.dart(5 hunks)
🔇 Additional comments (1)
packages/stream_chat/test/src/client/client_test.dart (1)
3539-3551: Nice coverage for delete-for-me.This test cleanly stubs the API, exercises the new client method, and asserts that we invoke
deleteMessagewithdeleteForMe: trueexactly once—perfect for guarding the new flow.
packages/stream_chat/lib/src/core/models/message_delete_scope.dart
Outdated
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This commit introduces a new message action that allows users to delete a message that failed to send. The new action is added to the `MessageActionsBuilder` and is only displayed when the message state is `isSendingFailed`.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/stream_chat/lib/src/core/api/message_api.dart (1)
188-191: Send delete_for_me only when true (avoid sending false).Avoid emitting delete_for_me: false; include the flag only when true to reduce ambiguity and align with common API patterns.
Apply this diff:
- queryParameters: { - if (hard != null) 'hard': hard, - if (deleteForMe != null) 'delete_for_me': deleteForMe, - }, + queryParameters: { + if (hard != null) 'hard': hard, + if (deleteForMe == true) 'delete_for_me': true, + },Also confirm the backend expects boolean query params as true/false strings (not 1/0).
packages/stream_chat/lib/src/core/models/message.dart (2)
86-101: Reconsider forcing type to deleted for delete‑for‑me (optional).Overriding type to MessageType.deleted will make isDeleted true for per‑user deletions, which may ripple into UI/logic that key off type instead of state. Since you already set state = MessageState.deletedForMe, consider leaving type unchanged and letting state drive visibility, at least once backend enriches types.
If you want to minimize behavior changes now, keep this as a temporary shim but audit any isDeleted usages to ensure they behave as intended for per‑user deletes.
323-326: Confirm JSON key mapping for deletedForMe.@jsonkey(includeToJson: false) assumes global snake_case mapping is enabled. If fieldRename: snake isn’t configured, add an explicit name to avoid deserialization gaps.
Optionally enforce explicitly:
- @JsonKey(includeToJson: false) + @JsonKey(name: 'deleted_for_me', includeToJson: false) final bool? deletedForMe;packages/stream_chat/test/src/core/api/message_api_test.dart (1)
208-219: Test covers delete_for_me flag; add guard and regression tests.Add:
- A test asserting deleteMessage throws ArgumentError when hard and deleteForMe are both true.
- A regression test that still verifies the hard delete path works (query hard: true).
Apply this diff to extend coverage:
test('deleteMessage', () async { const messageId = 'test-message-id'; const path = '/messages/$messageId'; const params = {'delete_for_me': true}; when(() => client.delete(path, queryParameters: params)).thenAnswer( (_) async => successResponse(path, data: <String, dynamic>{}), ); final res = await messageApi.deleteMessage(messageId, deleteForMe: true); expect(res, isNotNull); verify(() => client.delete(path, queryParameters: params)).called(1); verifyNoMoreInteractions(client); }); + + test('deleteMessage throws when both hard and deleteForMe are true', () async { + const messageId = 'test-message-id'; + expect( + () => messageApi.deleteMessage( + messageId, + hard: true, + deleteForMe: true, + ), + throwsArgumentError, + ); + }); + + test('deleteMessage hard delete still works', () async { + const messageId = 'test-message-id'; + const path = '/messages/$messageId'; + const params = {'hard': true}; + + when(() => client.delete(path, queryParameters: params)).thenAnswer( + (_) async => successResponse(path, data: <String, dynamic>{}), + ); + + final res = await messageApi.deleteMessage(messageId, hard: true); + expect(res, isNotNull); + verify(() => client.delete(path, queryParameters: params)).called(1); + verifyNoMoreInteractions(client); + });
📜 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.
📒 Files selected for processing (6)
packages/stream_chat/lib/src/client/channel.dart(6 hunks)packages/stream_chat/lib/src/core/api/message_api.dart(1 hunks)packages/stream_chat/lib/src/core/models/message.dart(8 hunks)packages/stream_chat/lib/src/core/models/message.g.dart(1 hunks)packages/stream_chat/test/src/core/api/message_api_test.dart(1 hunks)packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_chat/lib/src/core/models/message.g.dart
🔇 Additional comments (8)
packages/stream_chat/lib/src/client/channel.dart (1)
1022-1030: Fix_deleteLocalMessageargument ordering
ChannelClientState.deleteMessagetakes the positionalMessagefirst and the namedhardDeleteflag second. Passing the named argument before the positional one is invalid Dart syntax and prevents the code from compiling. Please swap the order so the message comes first.state?.deleteMessage( - hardDelete: true, // Local messages are always hard deleted. - message.copyWith( + message.copyWith( type: MessageType.deleted, localDeletedAt: DateTime.now(), state: MessageState.hardDeleted, ), + hardDelete: true, // Local messages are always hard deleted. );packages/stream_chat/lib/src/core/api/message_api.dart (1)
180-184: Good mutual‑exclusion guard; add test coverage.The runtime check preventing hard and deleteForMe together is correct. Please add a unit test that asserts ArgumentError is thrown when both are true.
packages/stream_chat/lib/src/core/models/message.dart (6)
54-55: New field wiring looks good.Constructor now accepts deletedForMe; consistent with the rest of the changes.
376-377: Top‑level field registration is correct.Including deleted_for_me in topLevelFields keeps extraData handling consistent.
436-437: copyWith API updated appropriately.
515-516: copyWith propagation looks correct.
562-563: merge propagation looks correct.
629-630: Equality surface updated to include deletedForMe.Good catch to keep Equatable props in sync.
packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart
Show resolved
Hide resolved
xsahil03x
changed the title
Submit a pull request
Fixes: FLU-225
Persistence PR: #2395
Description of the pull request
This PR introduces the ability to delete a message only for the current user.
The following changes are included:
MessageDeleteScopeto represent the scope of deletion for a message.deleteMessageForMemethod toStreamChatClientandChannelto delete a message only for the current user.Messagemodel to includedeletedOnlyForMefield.Eventmodel to includedeletedForMefield.Membermodel to includedeletedMessagesfield.MessageStateto includedeletingForMe,deletedForMe, anddeletingForMeFailedstates.MessageApito includedelete_for_meparameter indeleteMessagemethod.Screenshots / Videos
Screen.Recording.2025-09-25.at.11.17.45.mov
Summary by CodeRabbit
New Features
Improvements
Documentation