fix(core): Fix backwards pagination not working if channel was never opened#2789
fix(core): Fix backwards pagination not working if channel was never opened#2789VelikovPetar wants to merge 4 commits into
Conversation
|
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 zero-read epoch sentinel check in ChangesBackwards pagination fix and tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/stream_chat_flutter_core/test/stream_channel_test.dart (1)
838-862: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the stale-channel zero-sentinel case.
This only locks down the
isUpToDate == truepath. The production fix also depends on reloading the latest page when the zero-time sentinel arrives on a stale channel, so add a companion case withstate.isUpToDate = falseand assert a latest-page query happens with both around-anchors still null.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter_core/test/stream_channel_test.dart` around lines 838 - 862, The current test only covers the zero-time sentinel case when the channel is already up to date; add a companion widget test in stream_channel_test that sets mockChannel.state.isUpToDate to false and keeps lastReadMessageId and the around-anchor inputs null while currentUserRead.lastRead is DateTime.utc(1, 1, 1). In _pumpStreamChannel/_maybeInitChannel coverage, assert that a latest-page query is issued for the stale-channel path, using mockChannel.query to verify the reload behavior rather than verifyNever.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/stream_chat_flutter_core/test/stream_channel_test.dart`:
- Around line 838-862: The current test only covers the zero-time sentinel case
when the channel is already up to date; add a companion widget test in
stream_channel_test that sets mockChannel.state.isUpToDate to false and keeps
lastReadMessageId and the around-anchor inputs null while
currentUserRead.lastRead is DateTime.utc(1, 1, 1). In
_pumpStreamChannel/_maybeInitChannel coverage, assert that a latest-page query
is issued for the stale-channel path, using mockChannel.query to verify the
reload behavior rather than verifyNever.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1f671f2-011a-48d1-b78e-028786108896
📒 Files selected for processing (3)
packages/stream_chat_flutter_core/CHANGELOG.mdpackages/stream_chat_flutter_core/lib/src/stream_channel.dartpackages/stream_chat_flutter_core/test/stream_channel_test.dart
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2789 +/- ##
==========================================
+ Coverage 70.04% 70.09% +0.05%
==========================================
Files 426 426
Lines 25680 25682 +2
==========================================
+ Hits 17987 18003 +16
+ Misses 7693 7679 -14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| // channel tail, after which `_inferBoundariesFromAnchorTimestamp` | ||
| // would mis-conclude `_topPaginationEnded = true`. Skip in that case | ||
| // and fall through to the catch-all "load latest" below. | ||
| if (currentUserRead.lastRead.toUtc().isAfter(DateTime.utc(1970, 1, 1))) { |
There was a problem hiding this comment.
I think the comparison works without converting them to utc. Also maybe good to extract the default somewhere in static or const value in the class.
2 participants
Submit a pull request
Linear: FLU-563
Github Issue: #
CLA
Description of the pull request
When a user enters a channel with
unread_count > 0but the server-side read state haslast_read_message_id = nullandlast_read = 0001-01-01T00:00:00Z(Go'stime.Time{}zero value — the "never explicitly read" sentinel),StreamChannel._maybeInitChannelwas unconditionally callingloadChannelAtTimestamp(currentUserRead.lastRead).The backend silently ignores zero-time
created_at_around(viaIsZero()) and returns the channel tail. The client then mis-infers boundaries via_inferBoundariesFromAnchorTimestamp: because year-1 is before every loaded message, it concludesendOfPrependReached: true, sets_topPaginationEnded = true, and backwards pagination is permanently dead on the channel.The fix guards the timestamp branch on
lastRead.toUtc().isAfter(DateTime.utc(1970, 1, 1)). When it's the zero sentinel, the code falls through to the existing catch-all if(channel.state?.isUpToDate == false) loadChannelAtMessage(null)— i.e. stay on cached latest if up-to-date, otherwise reload latest.Adds a
_maybeInitChanneltest group with three regression guards:idAroundforlastReadMessageId,createdAtAroundfor real timestamps, and no query for the Go zero-time case.Screenshots / Videos
pagin-before.mp4
pagin-after.mp4
Summary by CodeRabbit
Summary by CodeRabbit