feat(llc): add hideHistoryBefore to addMembers #2459
Conversation
Adds the `hideHistoryBefore` parameter to the `addMembers` and `addChannelMembers` methods. This allows specifying a timestamp before which channel history should be hidden for newly added members. When `hideHistoryBefore` is provided, it takes precedence over the existing `hideHistory` boolean flag. The change is propagated through the client, channel, and API layers, and new tests are added to verify the functionality.
WalkthroughAdds an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/stream_chat/test/src/core/api/channel_api_test.dart (1)
412-492: addMembers payload and precedence semantics are well coveredThe two new tests correctly assert that:
hide_history_beforeis included whenhideHistoryBeforeis provided, and- when both
hideHistoryandhideHistoryBeforeare passed, the payload only carrieshide_history_before, enforcing the documented precedence.Structure aligns with existing tests; nothing blocking here.
If you touch this area again, you could factor the common setup (
channelId,channelType,memberIds,message,hideHistoryBefore) into a small helper to reduce duplication across the two tests, but it's not necessary for this PR.packages/stream_chat/test/src/client/client_test.dart (1)
1918-1960: LGTM! Test properly validates the hideHistoryBefore parameter.The test is well-structured and follows the existing patterns in the file. It correctly:
- Mocks the API call with the
hideHistoryBeforeparameter- Invokes the client method with the parameter
- Verifies the response
- Confirms the API was called exactly once with the correct arguments
Optional: Consider using a relative date like
DateTime.now().subtract(const Duration(days: 30))instead of the hardcoded'2024-01-01T00:00:00Z'for slightly more flexibility. However, hardcoded dates are perfectly acceptable for tests as they ensure reproducibility.
📜 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 (7)
packages/stream_chat/CHANGELOG.md(1 hunks)packages/stream_chat/lib/src/client/channel.dart(2 hunks)packages/stream_chat/lib/src/client/client.dart(1 hunks)packages/stream_chat/lib/src/core/api/channel_api.dart(1 hunks)packages/stream_chat/test/src/client/channel_test.dart(1 hunks)packages/stream_chat/test/src/client/client_test.dart(1 hunks)packages/stream_chat/test/src/core/api/channel_api_test.dart(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). (2)
- GitHub Check: stream_chat_flutter
- GitHub Check: analyze_legacy_versions
🔇 Additional comments (5)
packages/stream_chat/lib/src/client/client.dart (1)
1263-1278:addChannelMemberspropagation ofhideHistoryBeforeis correct and backward‑compatibleThe new optional
hideHistoryBeforeparameter is added in a backward‑compatible way and correctly forwarded to_chatApi.channel.addMemberswhile preserving existinghideHistorybehavior when it’s null. No issues found.packages/stream_chat/lib/src/core/api/channel_api.dart (1)
205-227:ChannelApi.addMemberscorrectly encodeshideHistoryBeforeprecedence overhideHistoryThe updated signature and payload logic cleanly ensure that when
hideHistoryBeforeis non‑null, onlyhide_history_beforeis sent; otherwisehide_historyis used, matching the documented precedence without breaking existing callers. Looks solid.packages/stream_chat/CHANGELOG.md (1)
1-7: Changelog entry accurately documents newhideHistoryBeforebehaviorThe “Upcoming” section clearly explains the new parameter, its purpose, and its precedence over
hideHistory, in line with the implementation.packages/stream_chat/lib/src/client/channel.dart (1)
1573-1588:Channel.addMemberssurface mirrors client/API changes correctlyThe channel‑level
addMembersnow exposeshideHistoryBeforeand correctly forwards it toaddChannelMembers, keeping behavior unchanged when null. Public API remains backward‑compatible while enabling the new capability.packages/stream_chat/test/src/client/channel_test.dart (1)
2689-2734: Channel.addMembers forwards hideHistoryBefore correctlyThis test cleanly exercises the Channel wrapper, ensuring
hideHistoryBeforeis passed through toclient.addChannelMembersand the response is wired as expected. It mirrors the existinghideHistorytest and looks good.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2459 +/- ##
=======================================
Coverage 64.61% 64.62%
=======================================
Files 420 420
Lines 26210 26215 +5
=======================================
+ Hits 16936 16941 +5
Misses 9274 9274 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/stream_chat/lib/src/core/api/channel_api.dart (1)
206-223: hideHistoryBefore handling and precedence look correct; minor consistency/readability nits only
- The new
hideHistoryBeforeparameter and theswitchexpression correctly ensure that only one ofhide_history_before(UTC ISO) orhide_historyis sent, with the former taking precedence. This matches the stated API contract.- Conditional inclusion of
messageis nice and avoids sending explicit nulls. One small consistency nit: other methods in this class (e.g.inviteChannelMembers,acceptChannelInvite) still send'message': messageunconditionally. Consider standardizing either on conditional inclusion or always-present keys across these APIs for predictability, but this is non-blocking.- If the team tends to prefer simpler conditionals over pattern
switchfor map literals, you could also express this as a straightforwardif (hideHistoryBefore != null) ... else ...; current form is fine if everyone is comfortable with pattern-switch style.
📜 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 (2)
packages/stream_chat/lib/src/core/api/channel_api.dart(1 hunks)packages/stream_chat/test/src/core/api/channel_api_test.dart(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). (9)
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_persistence
- GitHub Check: stream_chat_flutter
- GitHub Check: test
- GitHub Check: build (android)
- GitHub Check: build (ios)
- GitHub Check: analyze_legacy_versions
🔇 Additional comments (2)
packages/stream_chat/test/src/core/api/channel_api_test.dart (2)
412-448: Good coverage for hideHistoryBefore payload shapeThis test cleanly verifies that when
hideHistoryBeforeis provided, the request data containsadd_members,message, andhide_history_beforein UTC ISO format, and nothing extra. It mirrors the production conversion logic and keeps the expectations tight around the payload and response.
450-489: Nice precedence test for hideHistoryBefore over hideHistoryThis test correctly exercises the case where both
hideHistoryandhideHistoryBeforeare passed and asserts that onlyhide_history_beforeappears in the payload. Because the stubbeddatamap excludeshide_history, the test will fail if the implementation ever includes both keys, so it robustly protects the intended precedence behavior.
Submit a pull request
Fixes: FLU-340
Docs: https://github.com/GetStream/docs-content/pull/836
Description of the pull request
Adds the
hideHistoryBeforeparameter to theaddMembersandaddChannelMembersmethods.This allows specifying a timestamp before which channel history should be hidden for newly added members. When
hideHistoryBeforeis provided, it takes precedence over the existinghideHistoryboolean flag.Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.