-
Notifications
You must be signed in to change notification settings - Fork 1
Update feed pagination logic to include aggregated activities #133
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
base: develop
Are you sure you want to change the base?
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 refactors the GetOrCreateInfo data class to flatten its structure by replacing the nested PaginationResult<ActivityData> with separate pagination and activities fields, and reorders the aggregatedActivities field for better logical grouping. It also introduces a new upsertAll utility function for batch upsert operations and improves test readability by using Kotlin's backtick test naming convention.
- Refactors
GetOrCreateInfodata class to flatten the activities structure - Introduces
upsertAllutility function for efficient batch upsert operations - Updates test file to use descriptive Kotlin backtick test names and improve test assertions
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/utils/List.kt | Adds new upsertAll function for batch upsert operations |
| stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/repository/FeedsRepository.kt | Refactors GetOrCreateInfo to use flat structure with separate pagination and activities fields |
| stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/repository/FeedsRepositoryImpl.kt | Updates implementation to construct GetOrCreateInfo with new structure |
| stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/FeedStateImpl.kt | Updates to use flattened GetOrCreateInfo structure and adds aggregated activities merging |
| stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/FeedImpl.kt | Updates onQueryMoreActivities call to pass separate parameters instead of nested result |
| stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/utils/ListUpsertTest.kt | Refactors tests with backtick names and adds test coverage for upsertAll function |
| stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/FeedStateImplTest.kt | Updates test to verify aggregated activities merging behavior |
| stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/FeedImplTest.kt | Updates test data construction to match new GetOrCreateInfo structure |
| stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/repository/FeedsRepositoryImplTest.kt | Updates test data construction to match new GetOrCreateInfo structure |
Comments suppressed due to low confidence (1)
stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/utils/List.kt:50
- The
upsertfunction restricts the idSelector to returnString, while the newupsertAllfunction uses a generic typeRfor the key. This inconsistency creates an API design issue whereupsertis less flexible. Consider makingupsertgeneric as well:internal fun <T, R> List<T>.upsert(element: T, idSelector: (T) -> R): List<T>to maintain consistency and flexibility across the API.
internal fun <T> List<T>.upsert(element: T, idSelector: (T) -> String): List<T> {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...eeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/utils/List.kt
Outdated
Show resolved
Hide resolved
SDK Size Comparison 📏
|
b255b52 to
2fed45b
Compare
|



Goal
AND-854
aggregated_activitiesin a feed are also paginated. They already come in the response whenever we load new pages, so we should just take those and merge them with the local ones.Implementation
I had to change
GetOrCreateInfoto usePaginationDatadirectly instead of thePaginationResultwrapper, because the latter only allows for one list of paginated items, while here the pagination data refers to bothactivitiesandaggregatedActivities.Testing
It's not trivial because you need a feed where there are enough aggregated activities to trigger pagination.
Checklist