-
Notifications
You must be signed in to change notification settings - Fork 1
Remove fid filtering from ActivityEventHandler
#146
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
Remove fid filtering from ActivityEventHandler
#146
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 fid-based filtering from ActivityEventHandler in favor of relying solely on activity ID for event filtering. This change addresses reliability issues with feed ID (fid) filtering while maintaining correct event handling for activity-related updates.
Key Changes:
- Removed
FeedIdparameter fromActivityEventHandlerconstructor and all fid-based event filtering logic - Updated event handlers to filter events based only on activity ID
- Removed test cases that verified fid-based filtering behavior
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/event/handler/ActivityEventHandler.kt | Removed fid parameter from constructor and all fid-based filtering checks in event handlers |
| stream-feeds-android-client/src/main/kotlin/io/getstream/feeds/android/client/internal/state/ActivityImpl.kt | Updated ActivityEventHandler instantiation to remove fid argument |
| stream-feeds-android-client/src/test/kotlin/io/getstream/feeds/android/client/internal/state/event/handler/ActivityEventHandlerTest.kt | Removed fid-based test cases and updated test data to reflect simplified filtering logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...otlin/io/getstream/feeds/android/client/internal/state/event/handler/ActivityEventHandler.kt
Outdated
Show resolved
Hide resolved
...otlin/io/getstream/feeds/android/client/internal/state/event/handler/ActivityEventHandler.kt
Show resolved
Hide resolved
SDK Size Comparison 📏
|
eafb462 to
e24d525
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
e24d525 to
2a5c4af
Compare
|



Goal
Part of the wider AND-904.
This PR removes fid filtering from
ActivityEventHandlersince it's not reliable in all cases and we can just filter based on activity id.Implementation
ActivityEventHandlerand rely on activity idTesting
Updates to an
Activitystate should work as expected, regardless whether we're receiving WS events or not. This is partially testable in the demo for example by opening the comments for an activity in one device and adding/removing comments in that activity in another device.Checklist