-
Notifications
You must be signed in to change notification settings - Fork 1
Remove client-side sorting of activities in a feed #148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes client-side sorting of activities in feeds to support different feed types (e.g., "for you" feeds) that may not follow chronological ordering. The change removes the activitiesQueryConfig parameter that was used to maintain sorting information.
Key changes:
- Removed
ActivitiesQueryConfigand related sorting logic from feed state management - Replaced
mergeSorted/upsertSortedcalls withupsertAll/upsertto preserve server-provided order - Removed
activityFilterfrom pagination queries to avoid carrying over filters
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| FeedStateImpl.kt | Removed sorting configuration tracking and replaced sorted merge/upsert operations with order-preserving variants |
| FeedImpl.kt | Removed activityFilter from pagination queries to avoid filtering subsequent pages |
| FeedsRepositoryImpl.kt | Removed manual sorting of activities in repository layer while keeping import for initial sort |
| FeedsRepository.kt | Updated GetOrCreateInfo data class documentation and removed activitiesQueryConfig parameter |
| FeedStateImplTest.kt | Removed test setup for query configuration and sorting-related test code |
| FeedImplTest.kt | Removed query configuration from test data setup |
| FeedsRepositoryImplTest.kt | Removed query configuration assertions from repository tests |
Comments suppressed due to low confidence (1)
stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/repository/FeedsRepositoryImpl.kt:28
- The import for
ActivitiesSorton line 28 is now unused since the sorting logic has been removed. This import should be removed.
import io.getstream.feeds.android.client.api.state.query.ActivitiesSort
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...src/main/kotlin/io/getstream/feeds/android/client/internal/repository/FeedsRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
SDK Size Comparison 📏
|
d2c1088 to
731ea42
Compare
| val query = | ||
| FeedQuery( | ||
| fid = fid, | ||
| activityFilter = _state.activitiesQueryConfig?.filter, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we not supposed to pass the original filter argument from the original query when loading the next page of activities?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦♂️ fixing it and adding a test
031e02e to
970fbb3
Compare
|



Goal
Part of AND-893.
Removing client-side sorting of activities in a feed as we can't assume a sort order for all feeds (e.g. "for you" feeds aren't necessarily sorted chronologically).
Implementation
Remove
activitiesQueryConfigfrom different places, as the only thing we need to care about is the activity filter (query config contains filter + sorting).Testing
Activities in feeds should be sorted in the order they came in.
Checklist