fix(llc, ui): Issues found by the e2e suites - #2874
Open
testableapple wants to merge 6 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR filters parent messages from online thread replies, localizes thread reply labels, and aligns channel-list dates and text with the resolved preview message. ChangesThread reply state
Localized reply labels
Channel-list preview rendering
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 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 |
3 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2874 +/- ##
==========================================
+ Coverage 73.18% 73.24% +0.06%
==========================================
Files 429 429
Lines 27724 27735 +11
==========================================
+ Hits 20289 20314 +25
+ Misses 7435 7421 -14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
A backend that returns the root alongside the replies made ChannelClientState.threads hold it too, so StreamMessageListView rendered it twice. Aligns the online path with the offline one, which already filtered on parent_id. Linear: FLU-668
The footer hardcoded '$replyCount replies', so a single reply read "1 replies" and the label was never translated. It now uses threadReplyCountText, like the two sibling call sites. Covered by widget tests that inject a translation returning sentinels, so both the singular and the plural case prove the label goes through the translations table. Linear: FLU-669
The tile read its subtitle from channelState.messages but its timestamp from channel.lastMessageAt, which truncate() never clears, so a truncated channel showed a date next to "No messages yet". ChannelLastMessageDate now derives the date from the message the preview actually shows, via a _LastMessageResolver mixin shared by all three preview widgets so they cannot drift apart again. Linear: FLU-685
Channel-list rows are unkeyed, so a reorder rebinds the same State to a different channel. The preserved last-known message (added in #2774 to keep the preview alive across a channel-state reload) was still used as a fallback, so an empty, not-yet-up-to-date channel previewed the previous channel's message. The resolver now drops the cache when it sees a different ChannelClientState. Linear: FLU-686
testableapple
force-pushed
the
fix/e2e-tests-related-issues
branch
from
August 7, 2026 16:31
6ee0e3b to
f6e1faa
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/stream_chat/test/src/client/channel_test.dart (1)
4121-4124: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert every retained reply.
The current assertions check only the count and parent absence. They pass if one reply is dropped and another reply is duplicated. Assert the exact reply ID set to prove that filtering removes only the parent.
Proposed test assertion
expect(threadMessages, isNotNull); expect(threadMessages!.length, messages.length - 1); - expect(threadMessages.any((it) => it.id == parentId), isFalse); + final replyIds = threadMessages.map((it) => it.id).toSet(); + expect(replyIds, hasLength(3)); + expect( + replyIds.containsAll([ + 'test-message-id-0', + 'test-message-id-1', + 'test-message-id-2', + ]), + isTrue, + );🤖 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/test/src/client/channel_test.dart` around lines 4121 - 4124, Strengthen the assertions in the threadMessages test by verifying the exact set of retained reply IDs, not just the count and parent exclusion. Derive the expected IDs from messages after removing parentId, then compare them with the IDs in threadMessages while preserving the existing parent-absence check.
🤖 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.
Inline comments:
In
`@packages/stream_chat_flutter/lib/src/scroll_view/channel_scroll_view/stream_channel_list_item.dart`:
- Around line 493-499: Update the documentation for ChannelLastMessageDate to
state that it uses the latest previewable sent message, excluding drafts, and
renders no date when the preview is a draft or no eligible message exists;
remove the claim that it always reads the same message as the preview subtitle.
- Around line 805-806: Validate the cached _currentLastMessage against the
active predicate before using it in the !channelState.isUpToDate fallback within
the surrounding message-selection logic. Clear or exclude the cached value when
it fails predicate, while preserving the latest-message selection for valid
cached messages; add a widget test covering a predicate change while isUpToDate
remains false.
In
`@packages/stream_chat_flutter/test/src/message_widget/stream_message_item_test.dart`:
- Around line 99-100: Update the test assertions near the existing
singular/plural checks to also verify that
DefaultTranslations.instance.threadReplyCountText(0) is not rendered. Keep the
fake translation absence assertions, and specifically assert findsNothing for
the default zero-count reply label.
In `@packages/stream_chat/lib/src/client/channel.dart`:
- Around line 3927-3931: Update the thread merge flow around
_mergeMessagesIntoExisting to remove any cached parent message from
threadMessages before merging. Continue filtering parentId from incoming
messages, and preserve all non-parent cached and incoming thread messages.
---
Nitpick comments:
In `@packages/stream_chat/test/src/client/channel_test.dart`:
- Around line 4121-4124: Strengthen the assertions in the threadMessages test by
verifying the exact set of retained reply IDs, not just the count and parent
exclusion. Derive the expected IDs from messages after removing parentId, then
compare them with the IDs in threadMessages while preserving the existing
parent-absence check.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5be7224a-e90d-4eb5-85ff-7ec457170cc8
📒 Files selected for processing (8)
packages/stream_chat/CHANGELOG.mdpackages/stream_chat/lib/src/client/channel.dartpackages/stream_chat/test/src/client/channel_test.dartpackages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/lib/src/message_widget/stream_message_item.dartpackages/stream_chat_flutter/lib/src/scroll_view/channel_scroll_view/stream_channel_list_item.dartpackages/stream_chat_flutter/test/src/message_widget/stream_message_item_test.dartpackages/stream_chat_flutter/test/src/scroll_view/channel_scroll_view/stream_channel_list_item_test.dart
- Drop the cached preview message when the active predicate stops accepting it, so a changed filter cannot resurrect it as the not-up-to-date fallback. - Correct the ChannelLastMessageDate doc: it dates the latest sent message and ignores drafts, so it does not always match the subtitle. - Assert the default "0 replies" label is absent in the no-replies test.
The summary still said it dates "the message shown in the channel preview", which is the claim the body paragraph was corrected for: with a draft, the preview shows the draft and this dates the last sent message.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Goal
Split out of #2869 — these are the SDK fixes the e2e suites uncovered, with their unit tests and changelog entries. #2869 keeps only the e2e suites and CI.
Based on
masterand reviewable on its own. #2869 picks these up by mergingmasteronce this lands; until then the e2e tests covering them fail there by design.🛠 Implementation details
🟢 FLU-668 —
ChannelClientState.updateThreadInfono longer merges the parent message into a thread's reply list. A backend that returns the root alongside the replies madeStreamMessageListViewrender it twice, since it also rendersparentMessagein its own slot. This aligns the online path with the offline one, which already filtered onparent_id.🟢 FLU-669 — the thread-replies footer under a message in the channel hardcoded
'$replyCount replies', so a single reply read "1 replies" and the label was never localized. It now usesthreadReplyCountText, which is translated in 11 locales and is what the two sibling call sites already used.🟢 FLU-685 — after a channel was truncated, its row in the channel list still showed a timestamp next to "No messages yet". The tile read its subtitle from
channelState.messagesbut its timestamp fromchannel.lastMessageAt, whichtruncate()never clears — and clearing it locally would not have held, sinceonChannelTruncatedre-queries and the server keeps the field.ChannelLastMessageDatenow derives the date from the message the preview actually shows, through a_LastMessageResolvermixin shared by all three preview widgets so they cannot drift apart again.🟢 FLU-686 — surfaced while reviewing the fix above, not by a test: a channel-list row could preview another channel's last message. Rows are unkeyed, so a reorder rebinds the same
Stateto a different channel, and the preserved last-known message that #2774 added (to keep the preview alive across a channel-state reload) was still used as a fallback — so an empty, not-yet-up-to-date channel showed the previous one's message. The resolver now drops the cache when it is rebound to a differentChannelClientState. Shipped in 10.2.0, so it is fixed here rather than introduced here.🧪 Testing
Unit tests moved across with the fixes:
channel_test.dart—.getReplieskeeps the parent message out of the thread. 383 tests green instream_chat.stream_message_item_test.dart(new) — the thread-reply label is read from an injectedTranslationsreturning sentinels, so both the singular and the plural case prove the widget goes through the translations table. A hardcoded label passes a plural-only assertion, which is why the default-English variant of this test was not enough. A separate plaintest()pinsDefaultTranslations' own pluralization.stream_channel_list_item_test.dart—ChannelLastMessageDatefollows the previewed message (notlastMessageAt) and renders nothing when there is none; plus the rebind regression for FLU-686, which only catches the bug because the replacement channel is stubbedisUpToDate == false.21 tests green in the two
stream_chat_flutterfiles. Each fix was mutation-checked — reverting the production line fails the matching test.The e2e tests that cover these live in #2869 and fail there until this lands and
masteris merged in; that is expected and called out in that PR. Nothing here depends on #2869.☑️ Contributor Checklist
stream_chat,stream_chat_flutter)dart analyze --fatal-infosanddart formatcleanSummary by CodeRabbit