-
Notifications
You must be signed in to change notification settings - Fork 368
feat(llc, persistence): add messageCount to channel model and event #2403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This commit introduces a new `messageCount` field to the `ChannelModel` and `Event` classes. This field represents the total number of messages in a channel and is populated when the `count_messages` option is enabled for the app. The channel state is now updated to reflect changes in `messageCount` by listening to channel events.
This commit introduces two new properties to the `Channel` class: - `messageCount`: An integer representing the number of messages in the channel. - `messageCountStream`: A stream that emits the message count whenever it changes. These properties are only populated if the `count_messages` option is enabled for the app.
This commit introduces the `messageCount` field to the `ChannelEntity` in the persistence layer. This change includes: - Adding the `messageCount` column to the `Channels` table definition. - Incrementing the database schema version to 24. - Updating the `ChannelMapper` to include `messageCount` in mappings between `Channel` and `ChannelEntity`. - Regenerating the `drift_chat_database.g.dart` file to reflect the schema changes. - Updating the changelog. - Updating tests to include `messageCount`.
WalkthroughAdds channel-level message count support: new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor WS as WebSocket
participant CCS as ChannelClientState
participant CS as Channel State
participant C as Channel
participant S as Subscribers
WS->>CCS: event(channel_message_count = N)
note right of CCS #DDEEDF: _listenChannelMessageCount invoked during init
CCS->>CS: updateChannelState(messageCount: N)
CS->>C: copyWith(messageCount: N)
C-->>S: messageCountStream emits N
sequenceDiagram
autonumber
participant DB as Drift (schema v24)
participant Entity as ChannelEntity
participant Mapper as ChannelMapper
participant Model as ChannelModel
DB-->>Entity: read row (message_count -> messageCount)
Entity->>Mapper: toChannelModel()
Mapper-->>Model: set messageCount
Model->>Mapper: toEntity()
Mapper-->>Entity: include messageCount
Entity-->>DB: write row (message_count)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
|
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart (1)
58-73
: Schema bump triggers destructive migration — prefer additive migrationBumping to 24 currently wipes all tables on upgrade. This will drop persisted channels/messages for apps upgrading. Add a targeted migration for
message_count
instead of full rebuild.@override MigrationStrategy get migration => MigrationStrategy( beforeOpen: (details) async { await customStatement('PRAGMA foreign_keys = ON'); }, onUpgrade: (migrator, from, to) async { - if (from != to) { - for (final table in allTables) { - await migrator.deleteTable(table.actualTableName); - } - await migrator.createAll(); - } + // Additive migrations first + if (from < 24) { + // Add `message_count` to Channels without data loss + await migrator.addColumn(channels, channels.messageCount); + } + // Keep the destructive fallback only for future breaking changes + // where additive migration isn't feasible. + // if (from != to) { + // for (final table in allTables) { + // await migrator.deleteTable(table.actualTableName); + // } + // await migrator.createAll(); + // } }, );Also document this in the CHANGELOG/upgrade notes if you intentionally keep the destructive path.
🧹 Nitpick comments (3)
packages/stream_chat_persistence/CHANGELOG.md (1)
1-3
: Add note about the Drift schema bump.Persistence consumers will need to know that the Drift schema version jumped to 24 for the new
message_count
column. Please mention the schema bump (and migration implications, if any) in this Upcoming entry so upgrade steps are clear.packages/stream_chat/lib/src/core/models/channel_model.g.dart (1)
45-52
: Serialization asymmetry is intentional — verify tests cover it
toJson
omitsmessage_count
. Ensure tests assert thatmessage_count
is not emitted when serializingChannelModel
.packages/stream_chat/test/src/client/channel_test.dart (1)
5773-5799
: Reactive stream assertions (LGTM) — add one more case?Nice sequence check. Consider adding a case for
notification.message_new
carryingchannelMessageCount
to ensure parity.
📜 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 (12)
packages/stream_chat/lib/src/client/channel.dart
(3 hunks)packages/stream_chat/lib/src/core/models/channel_model.dart
(6 hunks)packages/stream_chat/lib/src/core/models/channel_model.g.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/test/src/client/channel_test.dart
(1 hunks)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
(33 hunks)packages/stream_chat_persistence/lib/src/entity/channels.dart
(1 hunks)packages/stream_chat_persistence/lib/src/mapper/channel_mapper.dart
(2 hunks)packages/stream_chat_persistence/test/src/mapper/channel_mapper_test.dart
(5 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
📚 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: stream_chat_persistence
- GitHub Check: stream_chat
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_flutter
- GitHub Check: analyze_legacy_versions
- GitHub Check: build (android)
- GitHub Check: build (ios)
- GitHub Check: analyze
- GitHub Check: test
🔇 Additional comments (12)
packages/stream_chat/lib/src/core/models/channel_model.g.dart (1)
42-43
: Read-only JSON ingestion for messageCount (LGTM) — confirm annotations
messageCount
is parsed frommessage_count
and excluded fromtoJson
(expected, server-managed). Please confirm the sourceChannelModel.messageCount
has@JsonKey(includeToJson: false)
and thatChannelModel.topLevelFields
includes'message_count'
inchannel_model.dart
.packages/stream_chat_persistence/lib/src/mapper/channel_mapper.dart (2)
18-19
: Entity → Model propagation (LGTM)Correctly maps
messageCount
toChannelModel
.
59-60
: Model → Entity propagation (LGTM)Round‑trip mapping preserved.
packages/stream_chat/test/src/client/channel_test.dart (3)
5656-5676
: Event-sourced update from channelMessageCount (LGTM)Covers the primary flow.
5680-5729
: Increment/decrement via message.new/deleted (LGTM)Good coverage of both directions.
5733-5769
: Property preservation while updating count (LGTM)Validates non-regression on unrelated fields.
packages/stream_chat/lib/src/core/models/event.g.dart (1)
79-129
: Serialization round-trip updated cleanly.Great to see
channel_message_count
wired through both fromJson and toJson—this will keep the field flowing without disturbing existing payloads.packages/stream_chat/lib/src/core/models/event.dart (1)
166-211
: New event surface area is consistent.Adding
channelMessageCount
to the model, top-level whitelist, andcopyWith
keeps the API coherent and ensures serializers don’t drop the new value.packages/stream_chat/lib/src/client/channel.dart (2)
419-435
: Channel accessors stay in sync.Exposing
messageCount
and its stream alongside the existing getters keeps the public API uniform—nice touch documenting thecount_messages
prerequisite.
2370-2385
: Listener cleanly propagates count updates.Subscribing to all channel events and short-circuiting on null keeps the handler cheap while guaranteeing we persist the new count through
updateChannelState
.packages/stream_chat_persistence/test/src/mapper/channel_mapper_test.dart (1)
25-123
: Persistence coverage looks solid.The added expectations for
messageCount
across entity↔model↔state mappings give good confidence the new column won’t regress silently.packages/stream_chat/lib/src/core/models/channel_model.dart (1)
34-264
: Channel model integration is well-rounded.Constructor, JSON surface,
copyWith
, andmerge
all account formessageCount
, so downstream consumers get the field without leaking it back to writes.
There was a problem hiding this 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/CHANGELOG.md (1)
5-5
: Document all new public surfacesPR adds
Channel.messageCountStream
andEvent.channelMessageCount
, but the changelog only mentionsChannel.messageCount
. Please list the other new entry points so downstream integrators know about them when scanning release notes.-- Added support for `Channel.messageCount` field. +- Added support for `Channel.messageCount` and `Channel.messageCountStream`. +- Added `Event.channelMessageCount` to surface channel-level counts in event payloads.
📜 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 (1)
packages/stream_chat/CHANGELOG.md
(1 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). (4)
- GitHub Check: build (android)
- GitHub Check: build (ios)
- GitHub Check: test
- GitHub Check: analyze_legacy_versions
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2403 +/- ##
==========================================
+ Coverage 63.84% 63.91% +0.06%
==========================================
Files 413 413
Lines 25862 25881 +19
==========================================
+ Hits 16511 16541 +30
+ Misses 9351 9340 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Submit a pull request
Fixes: FLU-241
Description of the pull request
This commit introduces a new
messageCount
field to theChannelModel
andEvent
classes.This field represents the total number of messages in a channel and is populated when the
count_messages
option is enabled for the app.The channel state is now updated to reflect changes in
messageCount
by listening to channel events.Summary by CodeRabbit