feat(persistence): add deletedForMe and deletedMessages fields #2395
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.
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`.
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`.
WalkthroughAdds per-user deletion support: new Messages.deletedForMe (bool) and Members.deletedMessages (List) columns, schema version bumped to 1000 + 25, mappers updated to propagate these fields, generated drift code refreshed, and tests updated. CHANGELOG notes upcoming beta support. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant Model as Domain Model
participant Mapper
participant DB as Drift DB
rect rgba(227,242,253,0.6)
note right of App: User marks message "deleted for me"
App->>Model: set Message.deletedForMe = true
Model->>Mapper: MessageX.toEntity()
Mapper->>DB: upsert MessageEntity{ deletedForMe: true }
end
rect rgba(232,245,233,0.6)
note right of App: Member deletes messages locally
App->>Model: update Member.deletedMessages = [...]
Model->>Mapper: MemberX.toEntity()
Mapper->>DB: upsert MemberEntity{ deletedMessages: [...] }
end
rect rgba(255,243,224,0.6)
note right of DB: Read path
DB-->>Mapper: fetch entities (Messages/Members/PinnedMessages)
Mapper-->>Model: toMessage()/toMember() includes deletedForMe / deletedMessages
Model-->>App: domain objects with new fields
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/stream_chat_persistence/lib/src/db/drift_chat_database.g.dart (3)
1371-1405: GuarddeletedForMeagainst missing JSON fields.
serializer.fromJson<bool>(json['deletedForMe'])will throw when the key is absent or null in persisted JSON (older caches / payloads). We need to default tofalseto remain backwards-compatible.- deletedForMe: serializer.fromJson<bool>(json['deletedForMe']), + deletedForMe: serializer.fromJson<bool?>(json['deletedForMe']) ?? false,Please adjust the generator/source so the generated code handles the missing field gracefully.
4006-4042: Same fix needed for pinned message deserialization.Persisted pinned-message JSON can lack
deletedForMe. Deserialising with a non-null cast will blow up; default it tofalse.- deletedForMe: serializer.fromJson<bool>(json['deletedForMe']), + deletedForMe: serializer.fromJson<bool?>(json['deletedForMe']) ?? false,
7790-7808: DefaultdeletedMessagesto an empty list when missing.Older JSON blobs won’t carry
deletedMessages. Callingserializer.fromJson<List<String>>(null)will throw. Default it toconst <String>[]to keep things robust.- deletedMessages: - serializer.fromJson<List<String>>(json['deletedMessages']), + deletedMessages: + serializer.fromJson<List<String>?>(json['deletedMessages']) ?? + const <String>[],
🧹 Nitpick comments (6)
packages/stream_chat_persistence/lib/src/mapper/member_mapper.dart (1)
21-23: Ensure deletedMessages is non-null when mapping to MemberEntityThe DB column is non-nullable. Guard against nulls from older payloads or partially constructed Member objects.
Apply this diff:
- deletedMessages: deletedMessages, + deletedMessages: deletedMessages ?? const [],Also confirm that Member.deletedMessages in stream_chat is non-nullable with a default empty list. If nullable in any codepath, consider a similar null-coalesce on the toMember side as well.
Also applies to: 43-45
packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart (1)
60-60: Schema bump will trigger destructive migration; prefer additive migration for new columnsThe current onUpgrade deletes and recreates all tables, causing data loss on upgrade (even for additive changes). For these additive fields, consider targeted migrations.
Example approach:
@override MigrationStrategy get migration => MigrationStrategy( beforeOpen: (_) async => await customStatement('PRAGMA foreign_keys = ON'), onUpgrade: (migrator, from, to) async { // Example: Additive migrations when upgrading to 1025 if (from < 1025) { await migrator.addColumn(messages, messages.deletedForMe); await migrator.addColumn(pinnedMessages, pinnedMessages.deletedForMe); await migrator.addColumn(members, members.deletedMessages); } }, );If destructive migration is intentional during beta, please confirm and document the data-loss expectation in release notes.
packages/stream_chat_persistence/lib/src/entity/members.dart (1)
3-3: Default deletedMessages to [] and minor doc tweak
- Without a default, inserts must always provide a value; defaulting to [] improves resilience.
- Minor wording tweak for inclusivity.
Apply this diff:
-/// List of message ids deleted by the member only for himself. +/// List of message IDs deleted by the member only for themselves. /// -/// These messages are now marked deleted for this member, but are still -/// kept as regular to other channel members. -TextColumn get deletedMessages => text().map(ListConverter<String>())(); +/// These messages are marked deleted for this member, but remain visible +/// to other channel members. +TextColumn get deletedMessages => + text().withDefault(const Constant('[]')).map(ListConverter<String>())();Note: If Drift requires withDefault before/after map in a specific order, adjust accordingly (some versions require calling withDefault before map()).
Also applies to: 54-59
packages/stream_chat_persistence/lib/src/mapper/message_mapper.dart (2)
34-34: Null-safety for deletedOnlyForMe mappingIf
deletedForMecan be null (older rows/migration gaps), default tofalseto avoid propagating nulls intoMessage.deletedOnlyForMe.- deletedOnlyForMe: deletedForMe, + deletedOnlyForMe: deletedForMe ?? false,
89-89: Null-safety for deletedForMe persistenceMirror the read-path safeguard; default to
falsewhen persisting ifdeletedOnlyForMecan be null. Also ensure the Drift column has a non-null default offalse.- deletedForMe: deletedOnlyForMe, + deletedForMe: deletedOnlyForMe ?? false,packages/stream_chat_persistence/test/src/mapper/member_mapper_test.dart (1)
82-84: LGTM: assertion validates round-trip for deletedMessagesConsider adding an extra test for empty lists (and, if applicable, null) to harden edge cases.
📜 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 (11)
packages/stream_chat_persistence/CHANGELOG.md(1 hunks)packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart(1 hunks)packages/stream_chat_persistence/lib/src/db/drift_chat_database.g.dart(99 hunks)packages/stream_chat_persistence/lib/src/entity/members.dart(2 hunks)packages/stream_chat_persistence/lib/src/entity/messages.dart(1 hunks)packages/stream_chat_persistence/lib/src/mapper/member_mapper.dart(2 hunks)packages/stream_chat_persistence/lib/src/mapper/message_mapper.dart(2 hunks)packages/stream_chat_persistence/lib/src/mapper/pinned_message_mapper.dart(2 hunks)packages/stream_chat_persistence/test/src/mapper/member_mapper_test.dart(4 hunks)packages/stream_chat_persistence/test/src/mapper/message_mapper_test.dart(4 hunks)packages/stream_chat_persistence/test/src/mapper/pinned_message_mapper_test.dart(4 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). (3)
- GitHub Check: build (android)
- GitHub Check: build (ios)
- GitHub Check: test
🔇 Additional comments (8)
packages/stream_chat_persistence/lib/src/entity/messages.dart (1)
102-104: LGTM: added per-user deletion flag with safe defaultThe new column and default(false) look correct and migration-friendly.
packages/stream_chat_persistence/test/src/mapper/pinned_message_mapper_test.dart (1)
83-85: LGTM: tests cover deletedForMe ↔ deletedOnlyForMe mapping both waysGood coverage for entity→model and model→entity propagation.
Also applies to: 134-135
packages/stream_chat_persistence/lib/src/mapper/pinned_message_mapper.dart (1)
34-36: LGTM: consistent mapping between deletedForMe and deletedOnlyForMeSymmetric propagation in both directions looks correct.
Also applies to: 90-92
packages/stream_chat_persistence/test/src/mapper/message_mapper_test.dart (1)
83-84: LGTM: tests validate deletedForMe ↔ deletedOnlyForMe mappingAssertions on both directions are accurate and consistent with the mappers.
Also applies to: 134-135, 223-224, 262-263
packages/stream_chat_persistence/test/src/mapper/member_mapper_test.dart (3)
27-29: LGTM: entity → model mapping covers deletedMessagesGood coverage for the new field on the entity-to-model path.
44-46: LGTM: assertion validates deletedMessages propagationDeep equality via
expect(actual, expected)works for lists here.
64-66: LGTM: model → entity mapping covers deletedMessagesConst list usage is fine and verifies immutability expectations.
packages/stream_chat_persistence/lib/src/mapper/message_mapper.dart (1)
82-82: Verify schema v1025 migration and backward‐compatibility for message state
No migration rewritingmessages.statefor v1025 was found; confirm or add a Drift migration to normalize existing rows to the new JSON shape and makeMessageState.fromJsonresilient to both legacy string enums and JSON map inputs.
xsahil03x
force-pushed
the
feat/delete-for-me-persistence
branch
from
d20f7e2 to
c41a842
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/delete-for-me #2395 +/- ##
======================================================
- Coverage 65.05% 65.05% -0.01%
======================================================
Files 421 421
Lines 26054 26066 +12
======================================================
+ Hits 16950 16957 +7
- Misses 9104 9109 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
xsahil03x
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/stream_chat_persistence/lib/src/db/drift_chat_database.g.dart (1)
7529-7534: Modeling note: consider normalizing per‑user deletions instead of a growing JSON list.Storing all deleted message IDs in one TEXT column can bloat rows and complicate targeted queries. A join table (user_id, channel_cid, message_id) with indexes scales better if this list grows.
📜 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 (11)
packages/stream_chat_persistence/CHANGELOG.md(1 hunks)packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart(1 hunks)packages/stream_chat_persistence/lib/src/db/drift_chat_database.g.dart(99 hunks)packages/stream_chat_persistence/lib/src/entity/members.dart(2 hunks)packages/stream_chat_persistence/lib/src/entity/messages.dart(1 hunks)packages/stream_chat_persistence/lib/src/mapper/member_mapper.dart(2 hunks)packages/stream_chat_persistence/lib/src/mapper/message_mapper.dart(2 hunks)packages/stream_chat_persistence/lib/src/mapper/pinned_message_mapper.dart(2 hunks)packages/stream_chat_persistence/test/src/mapper/member_mapper_test.dart(4 hunks)packages/stream_chat_persistence/test/src/mapper/message_mapper_test.dart(4 hunks)packages/stream_chat_persistence/test/src/mapper/pinned_message_mapper_test.dart(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/stream_chat_persistence/test/src/mapper/pinned_message_mapper_test.dart
- packages/stream_chat_persistence/test/src/mapper/member_mapper_test.dart
- packages/stream_chat_persistence/lib/src/mapper/message_mapper.dart
- packages/stream_chat_persistence/CHANGELOG.md
- packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart
- packages/stream_chat_persistence/lib/src/entity/messages.dart
- packages/stream_chat_persistence/lib/src/entity/members.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). (4)
- GitHub Check: analyze
- GitHub Check: build (android)
- GitHub Check: test
- GitHub Check: build (ios)
🔇 Additional comments (4)
packages/stream_chat_persistence/lib/src/mapper/member_mapper.dart (1)
21-44: Good symmetry on deletedMessages mapping.
Both directions now persist the new field, so the entity and model will stay in sync without extra work.packages/stream_chat_persistence/lib/src/mapper/pinned_message_mapper.dart (1)
34-34: Great job propagatingdeletedForMe.Both directions now keep the flag in sync, so pinned-message hydration and persistence remain consistent with the new per-user deletion semantics.
Also applies to: 90-90
packages/stream_chat_persistence/test/src/mapper/message_mapper_test.dart (1)
83-84: Tests cover the new field thoroughly.I appreciate the explicit round-trip assertions for
deletedForMe; they’ll catch regressions during future mapper tweaks.Also applies to: 223-224, 134-135, 262-263
packages/stream_chat_persistence/lib/src/db/drift_chat_database.g.dart (1)
7529-7534: Members.deletedMessages column is safe as-is
DB upgrade strategy drops and recreates all tables on schemaVersion bump, and every insert via the generated Companion.insert requires a non-null deletedMessages list (the mapper always supplies a default List), so no backfill or default is needed.
This commit introduces two new fields to the persistence layer: - `deletedForMe` (boolean) to `MessageEntity` and `PinnedMessageEntity`: Indicates if a message was deleted only for the current user. - `deletedMessages` (list of strings) to `MemberEntity`: Stores a list of message IDs that a member has deleted only for themselves. These changes allow for more granular control over message deletion states within the local database. The database schema version has been incremented to `1025` to reflect these changes. Mapper and test files have been updated accordingly.
The `deletedOnlyForMe` field in `MessageEntity` has been renamed to `deletedForMe` to match the API. The `deletedForMe` field in `MessageEntity` is now nullable.
xsahil03x
force-pushed
the
feat/delete-for-me-persistence
branch
from
3882e65 to
c3d39ce
Compare
xsahil03x
force-pushed
the
feat/delete-for-me-persistence
branch
from
848de27 to
c3d39ce
Compare
Submit a pull request
Fixes: FLU-225
LLC PR: #2394
Description of the pull request
This PR introduces two new fields to the persistence layer:
deletedForMe(boolean) toMessageEntityandPinnedMessageEntity: Indicates if a message was deleted only for the current user.deletedMessages(list of strings) toMemberEntity: Stores a list of message IDs that a member has deleted only for themselves.These changes allow for more granular control over message deletion states within the local database.
The database schema version has been incremented to
1025to reflect these changes.Mapper and test files have been updated accordingly.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests