fix(persistence): persist channel team field#2700
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a nullable ChangesChannel Team Field & Schema
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
VelikovPetar
left a comment
There was a problem hiding this comment.
It would be great if you can also update the relevant CHANGELOG.md file (from the stream_chat_persistence package).
Additionally, please make sure to sign the Stream CLA - We cannot merge the PR otherwise.
|
Thanks for the review! I've updated the CHANGELOG, added team field coverage to the relevant unit tests (channel_mapper_test.dart and channel_query_dao_test.dart), and re-generated the drift files with melos run generate:dart. The CLA has been signed as well. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2700 +/- ##
=======================================
Coverage 65.64% 65.64%
=======================================
Files 423 423
Lines 26708 26710 +2
=======================================
+ Hits 17533 17535 +2
Misses 9175 9175 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
VelikovPetar
left a comment
There was a problem hiding this comment.
We are looking into the failing CI job, we'll merge the PR as soon as we resolve it!
The check_db_entities workflow used `on: pull_request`, which gives GITHUB_TOKEN read-only perms on fork PRs and made peter-evans/create-or-update-comment fail with HTTP 403. Switch to pull_request_target with least-privilege permissions (contents: read, pull-requests: write). Avoid the usual pull_request_target footgun by never checking out the PR head into a working tree: fetch the PR head SHA as a ref and diff between SHAs, so attacker-controlled file contents never reach checkout/smudge filters/hooks while the write token is in scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `team` field of `ChannelModel` was not included in the `Channels` table schema, causing it to be lost on every app session. The field was always read as null from the local cache, even after the network response had populated it correctly. - Add `team` column to the `Channels` drift table - Map `team` in both `toChannelModel()` and `toEntity()` - Bump schema version to 29 (migration is destructive by design)
…per and query dao tests
fc99f16 to
166a000
Compare
c9b9232 to
4efdc45
Compare
|
Regenerating with drift_dev 2.22.1 had removed VerificationMeta entries for columns using type converters (ownCapabilities, config, filterTags, extraData). Restore them to match master and avoid unintended diff noise unrelated to the team field addition.
Summary
The `team` field of `ChannelModel` was missing from the `Channels` table schema in `stream_chat_persistence`. As a result, `channel.team` was always `null` when read from the local SQLite cache, even when the value had been correctly set server-side.
Root cause
`channels.dart` defines the Drift table for channel data. While most scalar fields of `ChannelModel` are stored as dedicated columns (e.g. `frozen`, `memberCount`, `lastMessageAt`), `team` was never added. It is listed in `ChannelModel.topLevelFields` and serialised correctly over the network, but was silently dropped during persistence.
Changes
Impact
Without this fix, any logic depending on `channel.team` behaves incorrectly on first render of each session (cache returns `null`), requiring a network round-trip before the correct value is available. This is particularly visible in multi-tenant setups where `team` determines channel routing or UI visibility.
Summary by CodeRabbit