Skip to content

Commit 3f0bc74

Browse files
committed
perf(Admin::PicturesController): Preload deletable ids
Rendering the picture archive used to call Picture#deletable? per row to decide which delete buttons to show. Each call issued up to two queries (the polymorphic related_ingredients check and the LIKE scan for ingredient-value references), so a page of N pictures cost up to 2N extra queries in addition to the main fetch. The controller now runs the deletable scope once for the ids on the current page and stores the result in a Set. Because the scope relation is chained as the subquery source of an IN clause, Rails applies the same Ransack filter, sort, and pagination on the database side without materializing the ids Ruby-side, keeping the check tight to the rows the view actually renders. The same preload is wired to the update action since it re-renders the picture partial via turbo stream.
1 parent bad0ff8 commit 3f0bc74

3 files changed

Lines changed: 42 additions & 3 deletions

File tree

app/controllers/alchemy/admin/pictures_controller.rb

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ class PicturesController < Alchemy::Admin::ResourcesController
88
include CurrentLanguage
99
include PictureDescriptionsFormHelper
1010

11+
before_action :load_pictures, only: :index
12+
1113
before_action :load_resource,
1214
only: [:edit, :update, :url, :destroy]
1315

@@ -19,6 +21,12 @@ class PicturesController < Alchemy::Admin::ResourcesController
1921
@picture = Picture.find(params[:id])
2022
end
2123

24+
# Preload deletable ids for the current page (index) or the current
25+
# resource (update, which re-renders the +_picture+ partial via a
26+
# turbo stream). One query replaces the per-row +deletable?+ check
27+
# that would otherwise fire for every picture the view renders.
28+
before_action :load_deletable_picture_ids, only: [:index, :update]
29+
2230
add_alchemy_filter :by_file_format, type: :select, options: ->(query) do
2331
Alchemy::Picture.file_formats(query.result)
2432
end
@@ -30,8 +38,6 @@ class PicturesController < Alchemy::Admin::ResourcesController
3038
helper_method :picture_offset
3139

3240
def index
33-
@pictures = filtered_pictures(page: params[:page])
34-
3541
if in_overlay?
3642
archive_overlay
3743
end
@@ -159,6 +165,18 @@ def items_per_page_options
159165

160166
private
161167

168+
def load_pictures
169+
@pictures = filtered_pictures(page: params[:page])
170+
end
171+
172+
def load_deletable_picture_ids
173+
@deletable_picture_ids = Picture
174+
.where(id: @pictures || @picture)
175+
.deletable
176+
.pluck(:id)
177+
.to_set
178+
end
179+
162180
def picture_offset
163181
((params[:page] || 1).to_i - 1) * items_per_page
164182
end

app/views/alchemy/admin/pictures/_picture.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<span class="picture_tool select">
33
<%= check_box_tag "picture_ids[]", picture.id %>
44
</span>
5-
<% if picture.deletable? && can?(:destroy, picture) %>
5+
<% if @deletable_picture_ids.include?(picture.id) && can?(:destroy, picture) %>
66
<div class="picture_tool delete">
77
<sl-tooltip content="<%= Alchemy.t('Delete image') %>">
88
<%= link_to_confirm_dialog(

spec/controllers/alchemy/admin/pictures_controller_spec.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,27 @@ module Alchemy
136136
end
137137
end
138138
end
139+
140+
describe "@deletable_picture_ids" do
141+
let!(:deletable_picture) { create(:alchemy_picture) }
142+
let!(:referenced_picture) { create(:alchemy_picture) }
143+
144+
before do
145+
create(:alchemy_ingredient_picture, related_object: referenced_picture)
146+
end
147+
148+
it "is a Set of ids of pictures on the current page that are deletable" do
149+
get :index
150+
expect(assigns(:deletable_picture_ids)).to be_a(Set)
151+
expect(assigns(:deletable_picture_ids)).to include(deletable_picture.id)
152+
expect(assigns(:deletable_picture_ids)).not_to include(referenced_picture.id)
153+
end
154+
155+
it "is scoped to the current page only" do
156+
get :index, params: {per_page: 1}
157+
expect(assigns(:deletable_picture_ids).size).to be <= 1
158+
end
159+
end
139160
end
140161

141162
describe "#create" do

0 commit comments

Comments
 (0)