-
Notifications
You must be signed in to change notification settings - Fork 1
feat(llc): add onNewActivity callback to Feed
#65
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: main
Are you sure you want to change the base?
Conversation
Introduces an `onNewActivity` callback to the `Feed` and `feedFromQuery` methods. This allows for custom logic to determine how new activities from real-time events are inserted into the activity list. The callback returns an `InsertionAction` (`addToStart`, `addToEnd`, or `ignore`). A default implementation is provided that adds activities from the current user to the start of the list if they match the query filter.
…activities to the end
|
Warning Rate limit exceeded@xsahil03x has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds an OnNewActivity callback and InsertionAction enum to control how real-time ActivityAdded events are inserted (ignore / add-to-start / add-to-end). Threads the callback through client APIs and Feed/FeedEventHandler/FeedState, adjusts many import paths for event handlers, and switches stream_core to a git dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant WS as WebSocket Event
participant Handler as FeedEventHandler
participant Callback as onNewActivity Callback
participant State as FeedState
participant List as Activity List
WS->>Handler: ActivityAddedEvent
Handler->>Handler: extract query, activity, currentUserId
Handler->>Callback: onNewActivity(query, activity, currentUserId)
activate Callback
Callback-->>Handler: InsertionAction (ignore | addToStart | addToEnd)
deactivate Callback
alt ignore
Handler->>Handler: skip insertion
else addToStart
Handler->>State: onActivityAdded(activity, insertionAction: addToStart)
State->>List: insert at index 0
else addToEnd
Handler->>State: onActivityAdded(activity, insertionAction: addToEnd)
State->>List: append to end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_feeds/lib/src/feeds_client.dart (1)
800-834:OnNewActivitytypedef anddefaultOnNewActivityfunction are not exported in the public API.The
OnNewActivitytypedef is used in the publicfeed()andfeedFromId()method signatures infeeds_client.dart, but it's not exported throughstate.dart. The filestate/on_activity_added.dartis missing from the export list inpackages/stream_feeds/lib/src/state.dart. Additionally,defaultOnNewActivityis marked@internaland should not be part of the public API. Add the export tostate.dart:export 'state/on_activity_added.dart';
🧹 Nitpick comments (5)
packages/stream_feeds/lib/src/state/insertion_action.dart (1)
1-13: Consider adding barrel file annotation.The enum is well-designed and clearly documented. However, since this is part of the public API (used by the
OnNewActivitycallback), consider marking it with@includeInBarrelFileannotation if it should be exported through the barrel file mechanism.Based on coding guidelines, public APIs should be marked with
@includeInBarrelFileannotation.packages/stream_feeds/lib/src/utils/filter.dart (1)
9-15: Consider narrowing the extension scope.While the extension is well-implemented with proper null-handling, extending all objects (
T extends Object) is quite broad and could lead to API pollution. Consider whether this extension could be more narrowly scoped to specific types that actually need filter matching (e.g.,ActivityData,CommentData) rather than all objects.If the current generic approach is intentional for reusability, this is acceptable. However, more targeted extensions would be clearer and less intrusive to the global namespace.
packages/stream_feeds/lib/src/state/event/on_activity_added.dart (1)
8-71:OnNewActivitycontract and default behavior align with requirementsThe typedef and
defaultOnNewActivityimplement the intended “add-if-from-current-user-and-matching-filter” default, giving consumers full control viaInsertionAction. As a tiny clean‑up, you could flatten the nestedifwithout changing behavior:- if (activity.user.id == currentUserId) { - if (activity.matches(query.activityFilter)) { - return InsertionAction.addToStart; - } - } - - return InsertionAction.ignore; + if (activity.user.id == currentUserId && + activity.matches(query.activityFilter)) { + return InsertionAction.addToStart; + } + + return InsertionAction.ignore;packages/stream_feeds/lib/src/state/feed_state.dart (1)
23-27:onActivityAddednow cleanly supports configurable insertionThe
InsertionActionparameter plusinsertAtcomputation and early return onignoregive a simple, extensible way to control placement while preserving the old default. Usingupsert(..., insertAt: (_) => insertAt)is a good fit to avoid duplicating list manipulation logic.If this method is a primary extension point, consider expanding its doc comment to briefly describe how
insertionActioninfluences ordering and how it relates toOnNewActivity, so future maintainers don’t have to chase the call chain.Also applies to: 111-133
packages/stream_feeds/lib/src/state/event/handler/feed_event_handler.dart (1)
3-18:FeedEventHandlercorrectly delegates ActivityAdded behavior toOnNewActivityWiring
onNewActivityinto the constructor and using it exclusively forActivityAddedEvent(while keepingmatchesQueryFilterfor other events) cleanly hands off insertion decisions to the callback and matches the new tests’ expectations. The sequence of:
- compute
InsertionAction,- call
state.onActivityAdded(...),- optionally enrich via
withUpdatedFeedCapabilities(...)
is straightforward and keeps capability updates orthogonal to insertion policy.One nuance worth keeping in mind (and which your tests already model) is that custom
onNewActivityimplementations are now solely responsible for honoringquery.activityFilterfor added activities; the previous automatic filter check no longer runs for this branch.Also applies to: 22-34, 48-58
Submit a pull request
Closes FLU-348
Description of the pull request
Introduces an
onNewActivitycallback to theFeedandfeedFromQuerymethods. This allows for custom logic to determine how new activities from real-time events are inserted into the activity list.The callback returns an
InsertionAction(addToStart,addToEnd, orignore).A default implementation is provided that adds activities from the current user to the start of the list if they match the query filter.
Important
Switch to pub dependency before merging the PR
Summary by CodeRabbit
New Features
Dependencies
Tests
✏️ Tip: You can customize this high-level summary in your review settings.