[openSUSE/open-build-service] 1c5b38: Don't initialize `NotificationsFinder` twice
Branch: refs/heads/master Home: https://github.com/openSUSE/open-build-service Commit: 1c5b38a26f556f77215b2987d4178d27b39d6cf1 https://github.com/openSUSE/open-build-service/commit/1c5b38a26f556f77215b29... Author: Lukas Krause <lkrause@suse.de> Date: 2022-02-18 (Fri, 18 Feb 2022) Changed paths: M src/api/app/controllers/person/notifications_controller.rb M src/api/app/controllers/webui/users/notifications_controller.rb M src/api/app/policies/notification_policy.rb M src/api/app/queries/notifications_finder.rb Log Message: ----------- Don't initialize `NotificationsFinder` twice Currently we initialize the `NotificationsFinder` once in the pundit notification policy scope and then use the returned notifications to initialize another `NotificationsFinder` object. This is confusing and makes it harder to track down unefficient queries and other issues. We should move the logic that fetches the notifications for the subscribed user from the finder to the policy since it is related to authorization. Commit: 83405d354c1d6882e194ddcd45449b66f88d27ca https://github.com/openSUSE/open-build-service/commit/83405d354c1d6882e194dd... Author: Lukas Krause <lkrause@suse.de> Date: 2022-02-18 (Fri, 18 Feb 2022) Changed paths: M src/api/app/controllers/webui/users/notifications_controller.rb Log Message: ----------- Eager load records associated with notifications Currently the notifications view cause a lot of n+1 queries Commit: dc5db00af5f6525d859168035b1c7178816d8d2b https://github.com/openSUSE/open-build-service/commit/dc5db00af5f6525d859168... Author: Lukas Krause <lkrause@suse.de> Date: 2022-02-18 (Fri, 18 Feb 2022) Changed paths: M src/api/app/components/notification_avatars_component.rb M src/api/app/models/review.rb Log Message: ----------- Remove `where` clause for comments and reviews in notification avatar component We eager loaded the comments and reviews when querying the notifications from the DB. The `where` clause cause another query to the db, so it ignores the by activerecord eager loaded objects. Commit: 8b6f26795cf793138a54ab574664d2454f2fb244 https://github.com/openSUSE/open-build-service/commit/8b6f26795cf793138a54ab... Author: Lukas Krause <lkrause@suse.de> Date: 2022-02-18 (Fri, 18 Feb 2022) Changed paths: M src/api/app/controllers/webui/package_controller.rb M src/api/app/controllers/webui/project_controller.rb M src/api/app/controllers/webui/request_controller.rb M src/api/app/queries/notifications_finder.rb Log Message: ----------- Check for logged in user in controller instead of finder class Checking if the user is logged in should happen on the controller level, not inside the notifications finder class. Commit: 30b0a3a613e741732c3759ebbcc52a982c9ad1bc https://github.com/openSUSE/open-build-service/commit/30b0a3a613e741732c3759... Author: Lukas Krause <lkrause@suse.de> Date: 2022-02-18 (Fri, 18 Feb 2022) Changed paths: M src/api/app/controllers/webui/package_controller.rb M src/api/app/controllers/webui/project_controller.rb M src/api/app/controllers/webui/request_controller.rb M src/api/app/queries/notifications_finder.rb Log Message: ----------- Use notification pundit policy in order to authorize and fetch a single notification Currently we query a whole scope in order to receive and authorize a single notification. Also having a method in a finder class to simply query a notification by an Id creates a bit of overhead. Lets simply use the `find` method to fetch the record and a pundit method to authorize. Commit: bf78a42d208d768ffcf855b6d6bec3ac05287ffd https://github.com/openSUSE/open-build-service/commit/bf78a42d208d768ffcf855... Author: Lukas Krause <lkrause@suse.de> Date: 2022-02-21 (Mon, 21 Feb 2022) Changed paths: M src/api/app/components/notification_avatars_component.rb M src/api/app/controllers/person/notifications_controller.rb M src/api/app/controllers/webui/package_controller.rb M src/api/app/controllers/webui/project_controller.rb M src/api/app/controllers/webui/request_controller.rb M src/api/app/controllers/webui/users/notifications_controller.rb M src/api/app/models/review.rb M src/api/app/policies/notification_policy.rb M src/api/app/queries/notifications_finder.rb Log Message: ----------- Merge pull request #12223 from krauselukas/improve_notification_queries02 Avoid n+1 queries for notification index view Compare: https://github.com/openSUSE/open-build-service/compare/ee129e3e0890...bf78a4...
participants (1)
-
Lukas Krause