Fix get() call not resetting state in paginated lists#189
Fix get() call not resetting state in paginated lists#189VelikovPetar merged 3 commits intodevelopfrom
get() call not resetting state in paginated lists#189Conversation
get() call not resetting state across paginated lists
get() call not resetting state across paginated listsget() call not resetting state in paginated lists
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
SDK Size Comparison 📏
|
WalkthroughThis PR refactors state management across 15+ list entities in the Stream Feeds Android client, unifying the data loading pattern. Initial Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/FeedStateImpl.kt (1)
128-141:⚠️ Potential issue | 🟠 Major
onQueryFeedshould passreplace = truewhen updating the member list.
onQueryFeedis the initial feed-load path and already unconditionally replaces_activities,_feed,_followers, etc. viaupdate { result.x }. However the member list is now forwarded withoutreplace, so it defaults tofalseandMemberListStateImpl.onQueryMemberswill merge the new members with any previously held ones viamergeSorted. This re-introduces exactly the stale/duplicated-items bug this PR is fixing wheneveronQueryFeedis invoked a second time (e.g. a re-get()of the feed) — members will persist/merge while every other collection is reset.🛠️ Proposed fix
- // Members are managed by the paginated list - memberListState.onQueryMembers(result.members, QueryConfiguration(null, null)) + // Members are managed by the paginated list + memberListState.onQueryMembers( + result.members, + QueryConfiguration(null, null), + replace = true, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/FeedStateImpl.kt` around lines 128 - 141, onQueryFeed currently forwards members to memberListState.onQueryMembers without forcing a replacement, causing stale/duplicated members to be merged; update the call in FeedStateImpl.onQueryFeed to invoke MemberListStateImpl.onQueryMembers with replace = true (i.e., pass replace=true along with the existing result.members and QueryConfiguration) so the member list is unconditionally replaced on initial feed loads/refreshes.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/MemberListStateImplTest.kt (1)
43-53:⚠️ Potential issue | 🟡 MinorRename stale test name referencing
queryMoreMembers.The method under test is now
onQueryMembers; the backticked test name still readson queryMoreMembers, then update members and pagination.✏️ Proposed fix
- fun `on queryMoreMembers, then update members and pagination`() = runTest { + fun `on onQueryMembers, then update members and pagination`() = runTest {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/MemberListStateImplTest.kt` around lines 43 - 53, The test name text is stale: the function is testing memberListState.onQueryMembers but the backticked test name still says "on queryMoreMembers..."; rename the test's descriptive name to reference onQueryMembers (e.g., change the backticked string in the test function `fun \`on queryMoreMembers, then update members and pagination\`` to something like `fun \`on queryMembers, then update members and pagination\``) so the test name matches the behavior exercised by memberListState.onQueryMembers and stays accurate.
🧹 Nitpick comments (18)
stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkFolderListStateImplTest.kt (1)
40-50: Stale test name after rename.The callback was renamed to
onQueryBookmarkFolders, but the test name still referencesqueryMoreBookmarkFolders. Minor readability nit.✏️ Suggested rename
- fun `on queryMoreBookmarkFolders, then update folders and pagination`() = runTest { + fun `on onQueryBookmarkFolders, then update folders and pagination`() = runTest {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkFolderListStateImplTest.kt` around lines 40 - 50, Rename the test function whose name currently reads `on queryMoreBookmarkFolders, then update folders and pagination` to reflect the renamed callback: change the backticked test name to `onQueryBookmarkFolders, then update folders and pagination` so it accurately matches the call to bookmarkFolderListState.onQueryBookmarkFolders used inside the test.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkFolderListImplTest.kt (1)
107-121: Strengthen assertion on replace semantics.Asserting only
size == 1passes even if the list were accidentally replaced with the wrong folder or kept a single stale item. Asserting the actual content (page2) makes the replace-not-merge intent explicit and catches regressions where merge ordering could coincidentally produce size 1.♻️ Proposed change
- assertEquals(1, bookmarkFolderList.state.folders.value.size) + assertEquals(page2, bookmarkFolderList.state.folders.value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkFolderListImplTest.kt` around lines 107 - 121, The test currently only checks size==1 which can miss wrong content; after calling bookmarkFolderList.get() twice, replace the size assertion with a content assertion that validates the folders match page2 exactly — e.g., assert that bookmarkFolderList.state.folders.value contains the single folder created by bookmarkFolderData("folder-3") or assert the list of folder ids equals listOf("folder-3"); update the assertion to compare the actual folder content (using bookmarkFolderData/page2 or folder.id) so the test verifies replace-not-merge semantics for bookmarkFolderList.get().stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/FeedListImplTest.kt (1)
107-121: Strengthen the replace assertion.Asserting only
size == 1is a weak signal that replacement occurred — it would still pass if the merge logic happened to deduplicate or ifpage2just happened to contain one item that coincidentally collapsed the list. Consider asserting the exact content equalspage2so an accidental regression to merge-semantics (e.g., ifpage1/page2are later changed to have non-overlapping IDs) is caught.🧪 Suggested assertion
- assertEquals(1, feedList.state.feeds.value.size) + assertEquals(page2, feedList.state.feeds.value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/FeedListImplTest.kt` around lines 107 - 121, Replace the weak size-only assertion in the test `on get called twice, feeds are replaced not merged` with a strict equality check that the feed list content equals the second page; after calling `feedList.get()` twice (which triggers `feedsRepository.queryFeeds(query)` returning `page1` then `page2`), assert that `feedList.state.feeds.value` exactly matches the items from `page2` (or their IDs) created via `createPaginationResult(page2)` so the test fails if merging instead of replacing occurs.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/UserListStateImplTest.kt (1)
70-96: Rename stalequeryMoreUserstest names for consistency.These three tests now exercise
onQueryUsers(withoutreplace) but their backticked names still sayqueryMoreUsers. Since the method was renamed and the other tests were already updated to reflect the new name, align these for readability.✏️ Proposed rename
- fun `on queryMoreUsers with non-empty result, then update users and canLoadMore true`() = + fun `on queryUsers with non-empty result, then update users and canLoadMore true`() = ... - fun `on queryMoreUsers with empty result, then canLoadMore false`() = runTest { + fun `on queryUsers with empty result, then canLoadMore false`() = runTest { ... - fun `on queryMoreUsers called twice, then merge users and advance offset`() = runTest { + fun `on queryUsers called twice, then merge users and advance offset`() = runTest {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/UserListStateImplTest.kt` around lines 70 - 96, Rename the three test names that still mention `queryMoreUsers` to reflect the new method name `onQueryUsers`: update the backticked test declarations for the tests that start with "on queryMoreUsers ..." to "on queryUsers ..." (the three tests exercising UserListStateImpl's onQueryUsers behavior), ensuring the test function names (the backticked strings) match the called method `onQueryUsers` used inside the tests.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListStateImplTest.kt (1)
41-55: Nit: test name is stale after the rename.The test name still references
queryMoreActivityReactions, but the call site now invokesonQueryActivityReactions. Consider renaming for consistency with the new API (e.g.,on queryActivityReactions, then update reactions and pagination).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListStateImplTest.kt` around lines 41 - 55, Rename the test function to match the updated API name: change the test named `on queryMoreActivityReactions, then update reactions and pagination` to something like `on queryActivityReactions, then update reactions and pagination`; update the Kotlin test function identifier (the name after `@Test` and fun) and any references in ActivityReactionListStateImplTest that mention `queryMoreActivityReactions` so it reflects calling activityReactionListState.onQueryActivityReactions and keeps the behavior assertions unchanged.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListImplTest.kt (1)
107-121: Strengthen the replace assertion.Asserting only
size == 1would also pass if the implementation erroneously kept a single stale item from page 1. Consider asserting the exact content equalspage2and verifying the repository was invoked twice, which more directly validates replace semantics.♻️ Suggested tightening
- activityReactionList.get() - activityReactionList.get() - - assertEquals(1, activityReactionList.state.reactions.value.size) + activityReactionList.get() + activityReactionList.get() + + assertEquals(page2, activityReactionList.state.reactions.value) + coVerify(exactly = 2) { + activitiesRepository.queryActivityReactions(query.activityId, any()) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListImplTest.kt` around lines 107 - 121, The test onGetCalledTwice should assert replacement semantics more strictly: after calling activityReactionList.get() twice, assert that activityReactionList.state.reactions.value exactly equals the expected page2 list (not just size) and also verify that activitiesRepository.queryActivityReactions(query.activityId, any()) was invoked twice (use coVerify or equivalent) to ensure the second fetch occurred; update the test `on get called twice, reactions are replaced not merged` to compare collection contents to `page2` and to coVerify two calls to `activitiesRepository.queryActivityReactions`.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkListImplTest.kt (1)
107-121: Strengthen the replacement assertion.Asserting only
size == 1will pass even if the implementation accidentally kept the wrong page or merged and happened to produce a single element. Compare the actual contents againstpage2to lock in replace-not-merge semantics explicitly.♻️ Suggested tightening
- assertEquals(1, bookmarkList.state.bookmarks.value.size) + assertEquals(page2, bookmarkList.state.bookmarks.value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkListImplTest.kt` around lines 107 - 121, The test `on get called twice, bookmarks are replaced not merged` currently only asserts size == 1 which doesn't guarantee the second page replaced the first; update the final assertion to compare the actual bookmarkList.state.bookmarks.value contents against `page2` (or their identifiers) to ensure the second result fully replaced the first, using the same helpers (`bookmarkList`, `bookmarksRepository.queryBookmarks`, `createPaginationResult`, `bookmarkData`) to locate and construct expected values.stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkListStateImpl.kt (1)
124-130: Consider dropping the default on the interface.The interface declares
replace: Boolean = falsewhile the impl override omits the default. Since all current callers (BookmarkListImpl.get()/queryMoreBookmarks()) passreplaceexplicitly, the default mainly benefits tests. Makingreplacerequired at the interface level would prevent future callers from silently getting merge-mode when they intended replace-mode — which is precisely the class of bug this PR is fixing. Optional; feel free to keep as-is if the default is convenient for test ergonomics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkListStateImpl.kt` around lines 124 - 130, The interface BookmarkListStateUpdates currently gives onQueryBookmarks a default parameter (replace: Boolean = false) which allows callers to omit replace and silently use merge behavior; remove the default from the interface signature so replace is a required argument (change fun onQueryBookmarks(result: PaginationResult<BookmarkData>, queryConfig: BookmarksQueryConfig, replace: Boolean)), keep the implementation override in BookmarkListImpl as-is (it already omits the default), and update any callers and tests (e.g., BookmarkListImpl.get(), queryMoreBookmarks(), and any unit tests) to pass an explicit replace value.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentListImplTest.kt (1)
108-122: Same assertion suggestion as inActivityCommentListImplTest.Consider
assertEquals(page2, commentList.state.comments.value)for content-level verification instead of just size.Also, pre-existing nit:
createPaginationResult's parameter is namedactivities(line 137) but holdsCommentData— worth renaming while you're here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentListImplTest.kt` around lines 108 - 122, Update the test on get called twice (function `on get called twice, comments are replaced not merged`) to assert content equality instead of just size by comparing the state to the expected page2 list: replace the current size-based assertion with an equality check against `page2` using `commentList.state.comments.value`; also rename the parameter `activities` in `createPaginationResult` to a more accurate name like `items` or `comments` since it holds `CommentData` to avoid confusion.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityCommentListImplTest.kt (1)
110-124: Consider strengthening the assertion.Asserting only
size == 1passes even if the wrong page was retained. Consider also asserting the content equalspage2to lock in replacement semantics (same applies to the analogous test inCommentListImplTest).Proposed tweak
- assertEquals(1, activityCommentList.state.comments.value.size) + assertEquals(page2, activityCommentList.state.comments.value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityCommentListImplTest.kt` around lines 110 - 124, The test ActivityCommentListImplTest::`on get called twice, comments are replaced not merged` only asserts list size; update it to assert the actual contents equal page2 to ensure replacement semantics (e.g., after two calls to activityCommentList.get() assert that activityCommentList.state.comments.value == createPaginationResult(page2).data or compare against the expected list page2/its IDs); locate the fetch stub using commentsRepository.getComments(query) and the test data threadedCommentData("comment-3") to build the expected value. Also apply the same stronger assertion change to the analogous test in CommentListImplTest.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/MemberListImplTest.kt (1)
108-122: Good replacement-semantics test; consider also asserting identity of members.The size assertion is sufficient to catch the merge regression, but asserting
assertEquals(page2, memberList.state.members.value)would additionally guard against cases where the second page happens to share size with the merged result (e.g., ifpage1andpage2had equal sizes but different items).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/MemberListImplTest.kt` around lines 108 - 122, Update the test on get called twice (`fun \`on get called twice, members are replaced not merged\``) to also assert that the members list equals the second page's items, not just its size: after calling `memberList.get()` twice, compare `memberList.state.members.value` to `page2` (or an equivalent expected list) to verify identity/content equality rather than only size; keep the existing setup using `page1`, `page2`, and `coEvery { feedsRepository.queryFeedMembers(...) }` and add this additional assertion to guard against accidental merges.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollVoteListStateImplTest.kt (1)
40-101: LGTM — rename is mechanical and coverage is preserved.Seeding calls rely on the default
replace = false, which merges against an initially empty list — equivalent to replace for these assertions. Consider adding a dedicated test for thereplace = truebranch (e.g., seeding once, then callingonQueryPollVotes(..., replace = true)with a different set and asserting the prior votes are dropped) so the state-level replace path has direct coverage in addition to the list-levelPollVoteListImplTestcase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollVoteListStateImplTest.kt` around lines 40 - 101, Add a test in PollVoteListStateImplTest that explicitly exercises the replace = true path: first seed the state via pollVoteListState.onQueryPollVotes(...) with an initial paginationResult, then call pollVoteListState.onQueryPollVotes(newPaginationResult, queryConfig.copy(replace = true)) with a different set of votes and assert pollVoteListState.votes.value equals only the new set (previous votes are dropped) and pagination/queryConfig updated; this ensures the state-level replace behavior in onQueryPollVotes is covered in addition to existing list-level tests.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollListStateImplTest.kt (1)
42-52: LGTM — rename only, behavior unchanged.Same note as
PollVoteListStateImplTest: an explicit test exercisingonQueryPolls(..., replace = true)on a non-empty seeded state would directly cover the new state-level branch (currently only exercised transitively byPollListImplTest).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollListStateImplTest.kt` around lines 42 - 52, Add an explicit unit test in PollListStateImplTest that seeds pollListState with a non-empty initial polls list, then calls pollListState.onQueryPolls(paginationResult, queryConfig, replace = true) and asserts that the state's polls were replaced (not appended), pagination.next equals "next-cursor", and queryConfig was updated; reference existing helpers like defaultPolls, defaultPaginationResult and the pollListState instance to mirror other tests but set replace = true to exercise the new state branch.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollListImplTest.kt (1)
105-119: Strengthen assertion to verify contents, not just size.
assertEquals(1, pollList.state.polls.value.size)proves the state wasn't merged (merge would yield 3 distinct IDs), but doesn't prove the surviving entry is frompage2. A subtle regression that returnedpage1.take(1)for the second call would still pass. ConsiderassertEquals(page2, pollList.state.polls.value).♻️ Proposed refinement
- assertEquals(1, pollList.state.polls.value.size) + assertEquals(page2, pollList.state.polls.value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollListImplTest.kt` around lines 105 - 119, Test currently only asserts size which can mask wrong replacement; update the assertion to verify the actual polls content after two pollList.get() calls by asserting pollList.state.polls.value equals the expected page2 result (or compare their IDs) so it confirms the second query replaced the list rather than returning a subset of page1; reference pollList.get(), pollList.state.polls.value and page2 when applying the change.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollVoteListImplTest.kt (1)
110-124: Strengthen assertion to verify contents, not just size.As with
PollListImplTest, asserting onlysize == 1discriminates replace vs merge for these specific page fixtures but doesn't confirm the remaining vote is actually frompage2. Prefer comparing the full list.♻️ Proposed refinement
- assertEquals(1, pollVoteList.state.votes.value.size) + assertEquals(page2, pollVoteList.state.votes.value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollVoteListImplTest.kt` around lines 110 - 124, The test currently only asserts size and should verify the actual contents are the latest page's votes: after calling pollVoteList.get() twice, replace the size assertion with a full equality check against the expected page2 result (e.g., compare pollVoteList.state.votes.value to the list built from page2 or to the IDs from page2). Locate the test `on get called twice, votes are replaced not merged` in PollVoteListImplTest, update the assertion to assert the votes equal the page2 payload (or its mapped DTO/IDs) to ensure replacement rather than merge.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListImplTest.kt (1)
128-142: Strengthen the assertion to pin down replacement semantics.Asserting only
size == 1will pass even if the test accidentally dropped page 1 entirely or produced any 1-element list. Asserting the actual contents (page2) makes the replace-vs-merge intent explicit and would have caught the original bug wherepage1 + page2would yield size 3 rather than 1 — which is already a correctness signal, but content equality is stronger.♻️ Proposed assertion
- assertEquals(1, activityList.state.activities.value.size) + assertEquals(page2, activityList.state.activities.value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListImplTest.kt` around lines 128 - 142, Test currently only asserts size==1 which is weak; update the assertion in ActivityListImplTest (`on get called twice, activities are replaced not merged`) to assert that the activities in activityList.state.activities.value exactly equal the second page (page2) returned by activitiesRepository.queryActivities (use the same activityData/createPaginationResult shape or compare by activity IDs) after calling activityList.get() twice, so the test verifies replacement semantics rather than just length.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListStateImplTest.kt (1)
52-63: Rename stale test and consider coveringreplace = true.The call was migrated to
onQueryActivities, but the test name still readson queryMoreActivities, .... Also, this suite only exercises the default (replace = false) merge path; adding a case that callsonQueryActivities(page2, queryConfig, replace = true)after an initial population would pin down the new replacement semantics at the state layer (theActivityListImplTestcovers it at the list layer, but not here).♻️ Proposed rename
- fun `on queryMoreActivities, then update activities and pagination`() = runTest { + fun `on onQueryActivities, then update activities and pagination`() = runTest {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListStateImplTest.kt` around lines 52 - 63, Rename the test to reflect the migrated API (e.g., change the name from "on queryMoreActivities..." to "onQueryActivities, then update activities and pagination") and add a new test in ActivityListStateImplTest that covers the replace=true path: first call activityListState.onQueryActivities(page1, queryConfig) to populate state, assert initial activities/pagination, then call activityListState.onQueryActivities(page2, queryConfig, replace = true) and assert that activities.value equals page2's items and pagination reflects page2; reference the existing helpers activityData and defaultPaginationResult and the subject under test activityListState/onQueryActivities to locate where to add the new assertions.stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentReplyListImplTest.kt (1)
109-123: Assert the replacement content, not just the size.This test can still pass if the final state contains any single reply. Compare against
page2so it verifies that the secondget()replaced state with the latest response.Proposed test assertion tightening
- assertEquals(1, commentReplyList.state.replies.value.size) + assertEquals(page2, commentReplyList.state.replies.value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentReplyListImplTest.kt` around lines 109 - 123, The test currently only asserts the size after two get() calls, which doesn't ensure the replies were replaced; update the assertion to verify the actual content equals page2 (the second response). After calling commentReplyList.get() twice, assert that commentReplyList.state.replies.value (or its mapped identifiers) equals createPaginationResult(page2).data or page2 so the final state matches the second Result from coEvery rather than any single item.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListStateImpl.kt`:
- Around line 246-260: Search for and replace the old method name
onQueryMoreActivities with the new onQueryActivities in the FeedStateImpl class
(rename the method declaration there), update all call sites such as the call in
FeedImpl (where feed-level query handler invokes the method) to call
onQueryActivities, and update the test FeedStateImplTest to use
onQueryActivities; ensure the method signature (parameters:
PaginationResult<ActivityData>, ActivitiesQueryConfig, replace: Boolean = false)
matches the interface ActivityListStateUpdates so compilation succeeds and the
feed-level get() reset behavior triggers correctly.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentReactionListImplTest.kt`:
- Around line 107-121: The test onGetCalledTwice currently only checks final
size so it can pass if fixtures share IDs; update the test `on get called twice,
reactions are replaced not merged` to use distinct IDs for items in `page1` and
`page2` (create two reactions with different ids via `feedsReactionData()` or by
overriding their id fields), stub
`commentsRepository.queryCommentReactions(query.commentId, query)` to return
`createPaginationResult(page1)` then `createPaginationResult(page2)`, call
`commentReactionList.get()` twice, and assert that
`commentReactionList.state.reactions.value` equals the exact contents of `page2`
(not just size) to prove replacement rather than merge.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/FollowListImplTest.kt`:
- Around line 105-117: Test currently only asserts size after two
followList.get() calls which can miss stale-item regressions; update the test
`on get called twice, follows are replaced not merged` to use distinct
FollowData ids for page1 and page2 (create with unique ids via `followData()` or
helper) and assert that `followList.state.follows.value` equals the exact items
from `page2` (compare objects/ids), not just the size; keep the existing
`coEvery { feedsRepository.queryFollows(any()) } returnsMany ...` and calls to
`followList.get()` and use `createPaginationResult` as before to verify
replacement semantics rather than deduplication by id.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ModerationConfigListImplTest.kt`:
- Around line 114-130: The test in ModerationConfigListImplTest currently only
asserts size after calling moderationConfigList.get() twice; update it to assert
the actual contents equal page2 to verify replacement rather than merge: after
the two moderationConfigList.get() calls, compare
moderationConfigList.state.configs.value (or its mapped representation) against
page2 (e.g., by equality of models or keys) instead of asserting size == 1 so
the test ensures the second Result replaced the first.
---
Outside diff comments:
In
`@stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/FeedStateImpl.kt`:
- Around line 128-141: onQueryFeed currently forwards members to
memberListState.onQueryMembers without forcing a replacement, causing
stale/duplicated members to be merged; update the call in
FeedStateImpl.onQueryFeed to invoke MemberListStateImpl.onQueryMembers with
replace = true (i.e., pass replace=true along with the existing result.members
and QueryConfiguration) so the member list is unconditionally replaced on
initial feed loads/refreshes.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/MemberListStateImplTest.kt`:
- Around line 43-53: The test name text is stale: the function is testing
memberListState.onQueryMembers but the backticked test name still says "on
queryMoreMembers..."; rename the test's descriptive name to reference
onQueryMembers (e.g., change the backticked string in the test function `fun
\`on queryMoreMembers, then update members and pagination\`` to something like
`fun \`on queryMembers, then update members and pagination\``) so the test name
matches the behavior exercised by memberListState.onQueryMembers and stays
accurate.
---
Nitpick comments:
In
`@stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkListStateImpl.kt`:
- Around line 124-130: The interface BookmarkListStateUpdates currently gives
onQueryBookmarks a default parameter (replace: Boolean = false) which allows
callers to omit replace and silently use merge behavior; remove the default from
the interface signature so replace is a required argument (change fun
onQueryBookmarks(result: PaginationResult<BookmarkData>, queryConfig:
BookmarksQueryConfig, replace: Boolean)), keep the implementation override in
BookmarkListImpl as-is (it already omits the default), and update any callers
and tests (e.g., BookmarkListImpl.get(), queryMoreBookmarks(), and any unit
tests) to pass an explicit replace value.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityCommentListImplTest.kt`:
- Around line 110-124: The test ActivityCommentListImplTest::`on get called
twice, comments are replaced not merged` only asserts list size; update it to
assert the actual contents equal page2 to ensure replacement semantics (e.g.,
after two calls to activityCommentList.get() assert that
activityCommentList.state.comments.value == createPaginationResult(page2).data
or compare against the expected list page2/its IDs); locate the fetch stub using
commentsRepository.getComments(query) and the test data
threadedCommentData("comment-3") to build the expected value. Also apply the
same stronger assertion change to the analogous test in CommentListImplTest.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListImplTest.kt`:
- Around line 128-142: Test currently only asserts size==1 which is weak; update
the assertion in ActivityListImplTest (`on get called twice, activities are
replaced not merged`) to assert that the activities in
activityList.state.activities.value exactly equal the second page (page2)
returned by activitiesRepository.queryActivities (use the same
activityData/createPaginationResult shape or compare by activity IDs) after
calling activityList.get() twice, so the test verifies replacement semantics
rather than just length.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListStateImplTest.kt`:
- Around line 52-63: Rename the test to reflect the migrated API (e.g., change
the name from "on queryMoreActivities..." to "onQueryActivities, then update
activities and pagination") and add a new test in ActivityListStateImplTest that
covers the replace=true path: first call
activityListState.onQueryActivities(page1, queryConfig) to populate state,
assert initial activities/pagination, then call
activityListState.onQueryActivities(page2, queryConfig, replace = true) and
assert that activities.value equals page2's items and pagination reflects page2;
reference the existing helpers activityData and defaultPaginationResult and the
subject under test activityListState/onQueryActivities to locate where to add
the new assertions.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListImplTest.kt`:
- Around line 107-121: The test onGetCalledTwice should assert replacement
semantics more strictly: after calling activityReactionList.get() twice, assert
that activityReactionList.state.reactions.value exactly equals the expected
page2 list (not just size) and also verify that
activitiesRepository.queryActivityReactions(query.activityId, any()) was invoked
twice (use coVerify or equivalent) to ensure the second fetch occurred; update
the test `on get called twice, reactions are replaced not merged` to compare
collection contents to `page2` and to coVerify two calls to
`activitiesRepository.queryActivityReactions`.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListStateImplTest.kt`:
- Around line 41-55: Rename the test function to match the updated API name:
change the test named `on queryMoreActivityReactions, then update reactions and
pagination` to something like `on queryActivityReactions, then update reactions
and pagination`; update the Kotlin test function identifier (the name after
`@Test` and fun) and any references in ActivityReactionListStateImplTest that
mention `queryMoreActivityReactions` so it reflects calling
activityReactionListState.onQueryActivityReactions and keeps the behavior
assertions unchanged.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkFolderListImplTest.kt`:
- Around line 107-121: The test currently only checks size==1 which can miss
wrong content; after calling bookmarkFolderList.get() twice, replace the size
assertion with a content assertion that validates the folders match page2
exactly — e.g., assert that bookmarkFolderList.state.folders.value contains the
single folder created by bookmarkFolderData("folder-3") or assert the list of
folder ids equals listOf("folder-3"); update the assertion to compare the actual
folder content (using bookmarkFolderData/page2 or folder.id) so the test
verifies replace-not-merge semantics for bookmarkFolderList.get().
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkFolderListStateImplTest.kt`:
- Around line 40-50: Rename the test function whose name currently reads `on
queryMoreBookmarkFolders, then update folders and pagination` to reflect the
renamed callback: change the backticked test name to `onQueryBookmarkFolders,
then update folders and pagination` so it accurately matches the call to
bookmarkFolderListState.onQueryBookmarkFolders used inside the test.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkListImplTest.kt`:
- Around line 107-121: The test `on get called twice, bookmarks are replaced not
merged` currently only asserts size == 1 which doesn't guarantee the second page
replaced the first; update the final assertion to compare the actual
bookmarkList.state.bookmarks.value contents against `page2` (or their
identifiers) to ensure the second result fully replaced the first, using the
same helpers (`bookmarkList`, `bookmarksRepository.queryBookmarks`,
`createPaginationResult`, `bookmarkData`) to locate and construct expected
values.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentListImplTest.kt`:
- Around line 108-122: Update the test on get called twice (function `on get
called twice, comments are replaced not merged`) to assert content equality
instead of just size by comparing the state to the expected page2 list: replace
the current size-based assertion with an equality check against `page2` using
`commentList.state.comments.value`; also rename the parameter `activities` in
`createPaginationResult` to a more accurate name like `items` or `comments`
since it holds `CommentData` to avoid confusion.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentReplyListImplTest.kt`:
- Around line 109-123: The test currently only asserts the size after two get()
calls, which doesn't ensure the replies were replaced; update the assertion to
verify the actual content equals page2 (the second response). After calling
commentReplyList.get() twice, assert that commentReplyList.state.replies.value
(or its mapped identifiers) equals createPaginationResult(page2).data or page2
so the final state matches the second Result from coEvery rather than any single
item.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/FeedListImplTest.kt`:
- Around line 107-121: Replace the weak size-only assertion in the test `on get
called twice, feeds are replaced not merged` with a strict equality check that
the feed list content equals the second page; after calling `feedList.get()`
twice (which triggers `feedsRepository.queryFeeds(query)` returning `page1` then
`page2`), assert that `feedList.state.feeds.value` exactly matches the items
from `page2` (or their IDs) created via `createPaginationResult(page2)` so the
test fails if merging instead of replacing occurs.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/MemberListImplTest.kt`:
- Around line 108-122: Update the test on get called twice (`fun \`on get called
twice, members are replaced not merged\``) to also assert that the members list
equals the second page's items, not just its size: after calling
`memberList.get()` twice, compare `memberList.state.members.value` to `page2`
(or an equivalent expected list) to verify identity/content equality rather than
only size; keep the existing setup using `page1`, `page2`, and `coEvery {
feedsRepository.queryFeedMembers(...) }` and add this additional assertion to
guard against accidental merges.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollListImplTest.kt`:
- Around line 105-119: Test currently only asserts size which can mask wrong
replacement; update the assertion to verify the actual polls content after two
pollList.get() calls by asserting pollList.state.polls.value equals the expected
page2 result (or compare their IDs) so it confirms the second query replaced the
list rather than returning a subset of page1; reference pollList.get(),
pollList.state.polls.value and page2 when applying the change.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollListStateImplTest.kt`:
- Around line 42-52: Add an explicit unit test in PollListStateImplTest that
seeds pollListState with a non-empty initial polls list, then calls
pollListState.onQueryPolls(paginationResult, queryConfig, replace = true) and
asserts that the state's polls were replaced (not appended), pagination.next
equals "next-cursor", and queryConfig was updated; reference existing helpers
like defaultPolls, defaultPaginationResult and the pollListState instance to
mirror other tests but set replace = true to exercise the new state branch.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollVoteListImplTest.kt`:
- Around line 110-124: The test currently only asserts size and should verify
the actual contents are the latest page's votes: after calling
pollVoteList.get() twice, replace the size assertion with a full equality check
against the expected page2 result (e.g., compare pollVoteList.state.votes.value
to the list built from page2 or to the IDs from page2). Locate the test `on get
called twice, votes are replaced not merged` in PollVoteListImplTest, update the
assertion to assert the votes equal the page2 payload (or its mapped DTO/IDs) to
ensure replacement rather than merge.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollVoteListStateImplTest.kt`:
- Around line 40-101: Add a test in PollVoteListStateImplTest that explicitly
exercises the replace = true path: first seed the state via
pollVoteListState.onQueryPollVotes(...) with an initial paginationResult, then
call pollVoteListState.onQueryPollVotes(newPaginationResult,
queryConfig.copy(replace = true)) with a different set of votes and assert
pollVoteListState.votes.value equals only the new set (previous votes are
dropped) and pagination/queryConfig updated; this ensures the state-level
replace behavior in onQueryPollVotes is covered in addition to existing
list-level tests.
In
`@stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/UserListStateImplTest.kt`:
- Around line 70-96: Rename the three test names that still mention
`queryMoreUsers` to reflect the new method name `onQueryUsers`: update the
backticked test declarations for the tests that start with "on queryMoreUsers
..." to "on queryUsers ..." (the three tests exercising UserListStateImpl's
onQueryUsers behavior), ensuring the test function names (the backticked
strings) match the called method `onQueryUsers` used inside the tests.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9c160c70-1344-4383-996f-aaab28d70de5
📒 Files selected for processing (60)
stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ActivityCommentListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ActivityCommentListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkFolderListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkFolderListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/CommentListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/CommentListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/CommentReactionListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/CommentReactionListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/CommentReplyListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/CommentReplyListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/FeedListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/FeedListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/FeedStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/FollowListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/FollowListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/MemberListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/MemberListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ModerationConfigListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ModerationConfigListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/PollListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/PollListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/PollVoteListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/PollVoteListStateImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/UserListImpl.ktstream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/UserListStateImpl.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityCommentListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityCommentListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ActivityReactionListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkFolderListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkFolderListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/BookmarkListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentReactionListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentReactionListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentReplyListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/CommentReplyListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/FeedListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/FeedListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/FollowListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/FollowListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/MemberListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/MemberListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ModerationConfigListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/ModerationConfigListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollVoteListImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/PollVoteListStateImplTest.ktstream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/UserListStateImplTest.kt
|



Goal
Fix
get()not resetting the list state when called multiple times. Previously, callingget()would merge results with the existing state instead of replacing it, potentially resulting in stale or duplicated items.Implementation
replaceparameter to the internalonQuery*methods across all paginated list state implementationsreplace = truefromget()so the state is fully reset, andreplace = false(default) fromqueryMore*()to preserve the existing merge behavioronQueryMore*toonQuery*in all list state interfaces to reflect their broader responsibilityTesting
onQuery*methods and the newreplaceparameterget()callsChecklist
Summary by CodeRabbit