Skip to content

Commit

Permalink
Merge pull request #17473 from opf/implementation/60018-ensure-remind…
Browse files Browse the repository at this point in the history
…ers-are-included-in-notifications-eager-loading-wrapper

implementation/60018 Add notification polymorphic resource as part of the eager loading list to avoid isolated query
  • Loading branch information
ulferts authored Dec 17, 2024
2 parents 066b97b + 7116350 commit 6cc7668
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 18 deletions.
17 changes: 0 additions & 17 deletions lib/api/v3/notifications/notification_eager_loading_wrapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,6 @@ def wrap(notifications)
notifications
.includes(API::V3::Notifications::NotificationRepresenter.to_eager_load)
.to_a
.tap { |loaded_notifications| set_resource(loaded_notifications) }
end

private

# The resource cannot be loaded by rails eager loading means (include)
# because it is a polymorphic association. That being as it is, currently only
# work packages are assigned.
def set_resource(notifications)
work_packages_by_id = WorkPackage
.includes(:project)
.where(id: notifications.pluck(:resource_id).uniq)
.index_by(&:id)

notifications.each do |notification|
notification.resource = work_packages_by_id[notification.resource_id]
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/api/v3/notifications/notification_representer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def _type
"Notification"
end

self.to_eager_load = %i[actor journal reminder]
self.to_eager_load = [:actor, :journal, :reminder, { resource: :project }]
end
end
end
Expand Down

0 comments on commit 6cc7668

Please sign in to comment.