Skip to content

Conversation

@gpunto
Copy link
Contributor

@gpunto gpunto commented Nov 12, 2025

Goal

Part of the wider AND-904.

This PR tackles the following: bookmark events don't have a fid so we use event.bookmark.activity.feeds, but that doesn't necessarily contain all feed ids that might be displaying the activity, for example, it won't contain "for you" feeds (since they're virtual) or timeline feeds (since the activity belongs to a user feed, not to the timeline itself).

Implementation

  • Remove fid filtering for bookmarks events and match only based on the activity id
  • I also took the opportunity to remove the fid property from poll events -> we stopped using it some time ago

Testing

In the demo app, adding/removing a bookmark to an activity in the feed should work correctly.

Checklist

  • Issue linked (if any)
  • Tests/docs updated
  • I have signed the Stream CLA (required for external contributors)

@gpunto gpunto added the pr:bug Bug fix label Nov 12, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

PR checklist ✅

All required conditions are satisfied:

  • Title length is OK (or ignored by label).
  • At least one pr: label exists.
  • Sections ### Goal, ### Implementation, and ### Testing are filled.

🎉 Great job! This PR is ready for review.

@github-actions
Copy link
Contributor

SDK Size Comparison 📏

SDK Before After Difference Status
stream-feeds-android-client 2.36 MB 2.36 MB 0.00 MB 🟢

@sonarqubecloud
Copy link

Copy link
Contributor

Copilot AI left a 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 feed ID (fid) filtering for bookmark events and removes the fid property from poll-related events. The change addresses an issue where bookmark events were being filtered based on event.bookmark.activity.feeds, which doesn't contain all feed IDs that might display the activity (e.g., "for you" or timeline feeds).

Key changes:

  • Bookmark events now match only on activity ID, removing feed-based filtering
  • Poll event signatures simplified by removing the unused fid parameter
  • Test cases updated to reflect new event signatures and filtering behavior

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
StateUpdateEvent.kt Removed fid parameter from poll event data classes (PollDeleted, PollUpdated, PollVoteCasted, PollVoteChanged, PollVoteRemoved) and their mapping functions
FeedEventHandler.kt Removed feed-based filtering for bookmark events (BookmarkAdded, BookmarkDeleted, BookmarkUpdated)
ActivityEventHandler.kt Removed feed-based filtering from bookmark event handlers, keeping only activity ID matching
ActivityImpl.kt Updated poll event emissions to exclude the fid parameter
FeedEventHandlerTest.kt Updated test cases to remove non-matching feed tests for bookmarks and updated poll event instantiations
ActivityEventHandlerTest.kt Removed non-matching feed bookmark tests and updated poll event test names and instantiations
ActivityListEventHandlerTest.kt Updated poll event instantiations to remove fid parameter
PollListEventHandlerTest.kt Updated poll event instantiations to remove fid parameter
PollVoteListEventHandlerTest.kt Updated poll event instantiations to remove fid parameter
ActivityImplTest.kt Updated all poll event assertions to match new signatures without fid

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gpunto gpunto marked this pull request as ready for review November 12, 2025 10:32
@VelikovPetar VelikovPetar merged commit 4800b3b into develop Nov 13, 2025
14 checks passed
@VelikovPetar VelikovPetar deleted the and-904/bookmark-event-filtering branch November 13, 2025 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants