You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Making this change would require a change to the API though as returning an Array of the generated notifications will be trickier, additionally, it looks like the NotifyAllJob passes the collection and not the relation.
I'm happy to open a PR if you have some ideas on what might make a good approach.
System configuration
activity_notification gem version: 2.1.3 Rails version: 6.0 ORM (ActiveRecord, Mongoid or Dynamoid): AR
The text was updated successfully, but these errors were encountered:
Steps to reproduce
In NotificationApi#notify the call to targets.blank? (https://github.com/simukappu/activity_notification/blob/master/lib/activity_notification/apis/notification_api.rb#L231) loads all of the targets, for a large collection, this loads all the records into memory and it seems pretty pointless as the subsequent call is to NotificationApi#notify_all which iterates the enumerable.
Additionally, that map call in NotificationApi#notify_all (https://github.com/simukappu/activity_notification/blob/master/lib/activity_notification/apis/notification_api.rb#L293) also loads all of the collection objects into memory, it'd be nicer if we could use something like .in_batches from the relation in order to do this with more performance on a large set.
Making this change would require a change to the API though as returning an Array of the generated notifications will be trickier, additionally, it looks like the NotifyAllJob passes the collection and not the relation.
I'm happy to open a PR if you have some ideas on what might make a good approach.
System configuration
activity_notification gem version: 2.1.3
Rails version: 6.0
ORM (ActiveRecord, Mongoid or Dynamoid): AR
The text was updated successfully, but these errors were encountered: