SQL optimisation rolled back. It lead to access problems.

This commit is contained in:
Christoph Haas 2021-03-01 02:07:55 +01:00
parent 7227f769c0
commit 2b24083653
5 changed files with 15 additions and 19 deletions

View file

@ -288,9 +288,8 @@ 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.accessible_by later packages = Package.order(visits: :desc)
packages = Package.includes(:screenshots).order(visits: :desc)
# text search # text search
if params[:search].present? if params[:search].present?
logger.debug "Searching for #{params[:search]}" logger.debug "Searching for #{params[:search]}"
@ -300,15 +299,11 @@ class PackagesController < ApplicationController
case params[:show] case params[:show]
when 'with' when 'with'
# Enrich the result with the screenshots readable by the current user (CanCanCan) # Enrich the result with the screenshots readable by the current user (CanCanCan)
packages = packages.where(screenshots: Screenshot.accessible_by(current_ability, :view)) packages = packages.with_screenshots
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
logger.debug 'Limiting packages to those without screenshots' logger.debug 'Limiting packages to those without screenshots'
else
packages = packages.where(screenshots: Screenshot.accessible_by(current_ability, :view)).or(
packages.without_screenshots)
end end
return packages return packages

View file

@ -1,7 +1,6 @@
class WelcomeController < ApplicationController class WelcomeController < ApplicationController
def home def home
# TODO: Only approved screenshots! @newest_upload = Screenshot.where(approved: true).first
@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

@ -33,6 +33,12 @@ class Ability
# https://github.com/CanCanCommunity/cancancan/wiki/Defining-Abilities # https://github.com/CanCanCommunity/cancancan/wiki/Defining-Abilities
if user.present? # Logged-in users if user.present? # Logged-in users
# Allow to view all public/approved screenshots
can :view, Screenshot, approved: true
# Allow to view any own uploads (even not-yet-approved)
can :view, Screenshot, user_id: user.id
can :destroy, Screenshot, user_id: user.id
if user.admin_role? if user.admin_role?
can :approve, Screenshot can :approve, Screenshot
can :destroy, Screenshot can :destroy, Screenshot
@ -45,12 +51,6 @@ class Ability
can :view, Screenshot can :view, Screenshot
can :destroy, Screenshot can :destroy, Screenshot
end end
if user.pseudo?
# Allow to view all public/approved screenshots
can :view, Screenshot, approved: true
# Allow to view any own uploads (even not-yet-approved)
can :view, Screenshot, user_id: user.id
end
else # Nobody logged in else # Nobody logged in
can :view, Screenshot, approved: true can :view, Screenshot, approved: true
end end

View file

@ -57,7 +57,7 @@ class Package < ApplicationRecord
end end
# Return a query of all packages that have screenshots # Return a query of all packages that do not have screenshots
def self.without_screenshots def self.without_screenshots
# Query for all packages who's ID does not appear in a screenshot's "package_id" field # Query for all packages who's ID does not appear in a screenshot's "package_id" field
subselect = Screenshot.select(:package_id) subselect = Screenshot.select(:package_id)

View file

@ -18,7 +18,8 @@
.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.first, cls: '') / This leads to an N+1 SQL query for each image. Ideas for optimization welcome.
= small_img(pkg.screenshots.accessible_by(current_ability, :view).first, cls: '')
.text.pkgname .text.pkgname
= pkg.name = pkg.name
.text .text
@ -29,7 +30,8 @@
.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.first, cls: '') / This leads to an N+1 SQL query for each image. Ideas for optimization welcome.
= small_img(pkg.screenshots.accessible_by(current_ability, :view).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)