Add handling for bookmark updated event #94
Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds missing handling for the BookmarkUpdated event and simplifies bookmark change logic by treating both additions and updates as upsert operations.
- Add handling for
BookmarkUpdatedevents across all relevant event handlers - Unify bookmark addition and update logic into a single
onBookmarkUpsertedmethod - Simplify bookmark state updates by using backend data as source of truth, only computing
ownBookmarksseparately
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TestData.kt | Add createdAt parameter to test bookmark factory method |
| FeedEventHandlerTest.kt | Add test for BookmarkUpdated event handling |
| BookmarkListEventHandlerTest.kt | Refactor tests to use parameterized approach and update bookmark handlers |
| ActivityListEventHandlerTest.kt | Add test for BookmarkUpdated event and update existing handlers |
| ActivityEventHandlerTest.kt | Add test for BookmarkUpdated event handling |
| FeedStateImplTest.kt | Update tests to use onBookmarkUpserted method |
| FeedImplTest.kt | Simplify bookmark addition test logic |
| BookmarkListStateImplTest.kt | Replace onBookmarkUpdated with onBookmarkUpserted and improve test structure |
| ActivityStateImplTest.kt | Update tests to use onBookmarkUpserted method |
| ActivityListStateImplTest.kt | Update tests to use onBookmarkUpserted method |
| FeedEventHandler.kt | Add handling for BookmarkUpdated events |
| BookmarkListEventHandler.kt | Add BookmarkAdded and BookmarkUpdated handling with upsert logic |
| ActivityListEventHandler.kt | Add BookmarkUpdated handling and update existing handlers |
| ActivityEventHandler.kt | Add handling for BookmarkUpdated events |
| FeedStateImpl.kt | Replace separate add/update methods with unified onBookmarkUpserted |
| BookmarkListStateImpl.kt | Replace onBookmarkUpdated with onBookmarkUpserted using sorted upsert |
| ActivityStateImpl.kt | Replace separate add/update methods with unified onBookmarkUpserted |
| ActivityListStateImpl.kt | Replace separate add/update methods with unified onBookmarkUpserted |
| ActivityData.kt | Simplify bookmark change logic with unified changeBookmarks helper |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
gpunto
changed the title
gpunto
changed the title
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
.../getstream/feeds/android/client/internal/state/event/handler/BookmarkListEventHandlerTest.kt
Show resolved
Hide resolved
SDK Size Comparison 📏
|
| internal fun List<ActivityData>.deleteBookmark( | ||
| bookmark: BookmarkData, | ||
| currentUserId: String, | ||
| ): List<ActivityData> = | ||
| updateIf({ it.id == bookmark.activity.id }) { activity -> | ||
| activity.deleteBookmark(bookmark, currentUserId) | ||
| } | ||
|
|
||
| internal fun List<ActivityData>.upsertBookmark( | ||
| bookmark: BookmarkData, | ||
| currentUserId: String, | ||
| ): List<ActivityData> = | ||
| updateIf({ it.id == bookmark.activity.id }) { activity -> | ||
| activity.upsertBookmark(bookmark, currentUserId) | ||
| } |
There was a problem hiding this comment.
I introduced these two on List to address the duplication found by Sonar and be able to merge. However, the duplication was scheduled to disappear because of some changes in a future PR, so I'll remove them at that point.
|
Goal
Part of AND-796
I'm adding missing handling for some events. This PR does that for
BookmarkUpdated. I also took the chance to simplify the logic to update bookmarks on events.Implementation
BookmarkUpdatedwhere neededownBookmarks, so we only compute thoseTesting
For testing that adding/deleting bookmarks still works, it's just a matter of launching the sample and testing it manually. For updates it's not easy, as we don't have anything in the sample that would trigger it. It would require adding code that changes an existing bookmark, e.g. changing its folder.
Checklist