Avoid N+1 SQL queries with CanCanCan accessibility checks

This commit is contained in:
Christoph Haas 2021-03-01 00:49:25 +01:00
parent f154c1838c
commit b36a49a6cd
6 changed files with 14 additions and 12 deletions

View file

@ -288,6 +288,7 @@ class PackagesController < ApplicationController
# Return packages matching the criteria given by parameters # Return packages matching the criteria given by parameters
def query_packages def query_packages
# Eager load the screenshots to avoid N+1 queries to check Screenshot.accessibly_by later
packages = Package.includes(:screenshots).order(visits: :desc) packages = Package.includes(:screenshots).order(visits: :desc)
# text search # text search
@ -298,7 +299,8 @@ class PackagesController < ApplicationController
case params[:show] case params[:show]
when 'with' when 'with'
packages = packages.with_screenshots # Enrich the result with the screenshots readable by the current user (CanCanCan)
packages = packages.where(screenshots: Screenshot.accessible_by(current_ability, :view))
logger.debug 'Limiting packages to those with screenshots' logger.debug 'Limiting packages to those with screenshots'
when 'without' when 'without'
packages = packages.without_screenshots packages = packages.without_screenshots

View file

@ -1,7 +1,7 @@
class WelcomeController < ApplicationController class WelcomeController < ApplicationController
def home def home
# TODO: Only approved screenshots! # TODO: Only approved screenshots!
@newest_upload = Screenshot.newest_upload @newest_upload = Screenshot.newest.accessible_by(current_ability, :view).first
@most_popular_package = Package.with_screenshots.order(visits: :desc).first @most_popular_package = Package.with_screenshots.order(visits: :desc).first

View file

@ -48,7 +48,7 @@ class Package < ApplicationRecord
def self.with_screenshots def self.with_screenshots
# Query for all packages who's ID appears in a screenshot's "package_id" field # Query for all packages who's ID appears in a screenshot's "package_id" field
subselect = Screenshot.select(:package_id) subselect = Screenshot.select(:package_id)
where(id: subselect ) where(id: subselect)
end end
# Return a list of packages that have screenshots to be moderated # Return a list of packages that have screenshots to be moderated
@ -71,7 +71,7 @@ class Package < ApplicationRecord
# Return a query of all approved/public screenshots of this package # Return a query of all approved/public screenshots of this package
def screenshots_pending def screenshots_pending
self.screenshots.where('approved=false') self.screenshots.where(approved: false)
end end
# Return a list of packages that have unapproved screenshots # Return a list of packages that have unapproved screenshots

View file

@ -63,9 +63,9 @@ class Screenshot < ApplicationRecord
self.save! self.save!
end end
# Get the newest screenshot regardless of the package it belongs to # Get the newest screenshots regardless of the package it belongs to
def self.newest_upload def self.newest
self.order(created_at: :desc).where(approved: true).first self.order(created_at: :desc)
end end
# Return the part of the version up to the first - or + # Return the part of the version up to the first - or +

View file

@ -18,7 +18,7 @@
.cell.pkgcard data-equalizer-watch=true .cell.pkgcard data-equalizer-watch=true
a.black href=package_path(name: pkg.name) a.black href=package_path(name: pkg.name)
.image .image
= small_img(pkg.screenshots.accessible_by(current_ability, :view).first, cls: '') = small_img(pkg.screenshots.first, cls: '')
.text.pkgname .text.pkgname
= pkg.name = pkg.name
.text .text
@ -29,7 +29,7 @@
.small-12.medium-4.cell.pkgcard .small-12.medium-4.cell.pkgcard
a.black href=package_path(name: pkg.name) a.black href=package_path(name: pkg.name)
.image .image
= small_img(pkg.screenshots.accessible_by(current_ability, :view).first, cls: '') = small_img(pkg.screenshots.first, cls: '')
.small-12.medium-8.cell .small-12.medium-8.cell
h2 h2
a href=package_path(name: pkg.name) a href=package_path(name: pkg.name)

View file

@ -25,7 +25,7 @@
.text .text
= screenshot.caption = screenshot.caption
// Has the unmoderated screenshot been uploaded by the current user? // Has the unmoderated screenshot been uploaded by the current user?
- if current_user.screenshots.include?(screenshot) - if current_user && current_user.screenshots.include?(screenshot)
.text .text
span.label.secondary span.label.secondary
' Uploaded by you ' Uploaded by you