Skip to content

Commit bad0ff8

Browse files
committed
perf(Admin::AttachmentsController): Preload deletable ids
Rendering the attachment index used to call Attachment#deletable? per row to decide which delete buttons to enable. 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 attachments 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.
1 parent 3ed90b4 commit bad0ff8

3 files changed

Lines changed: 33 additions & 1 deletion

File tree

app/controllers/alchemy/admin/attachments_controller.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ def index
3939
.page(params[:page] || 1)
4040
.per(items_per_page)
4141

42+
# Preload deletable ids for the current page in a single query so the
43+
# view can decide which delete buttons to enable without calling
44+
# +deletable?+ (a two-query check) per row.
45+
@deletable_attachment_ids =
46+
Attachment.where(id: @attachments).deletable.pluck(:id).to_set
47+
4248
if in_overlay?
4349
archive_overlay
4450
end

app/views/alchemy/admin/attachments/_files_list.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
file_attribute: 'file' %>
6262
<% end %>
6363
<% table.with_action(:destroy) do |attachment| %>
64-
<% if attachment.deletable? %>
64+
<% if @deletable_attachment_ids.include?(attachment.id) %>
6565
<sl-tooltip content="<%= Alchemy.t(:delete_file) %>">
6666
<%= link_to_confirm_dialog render_icon(:minus),
6767
Alchemy.t(:confirm_to_delete_file),

spec/controllers/alchemy/admin/attachments_controller_spec.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,32 @@ module Alchemy
7070
end
7171
end
7272

73+
describe "@deletable_attachment_ids" do
74+
let!(:deletable_attachment) { create(:alchemy_attachment) }
75+
let!(:referenced_attachment) { create(:alchemy_attachment) }
76+
77+
before do
78+
create(
79+
:alchemy_ingredient_richtext,
80+
value: %(<a href="/attachment/#{referenced_attachment.id}/download">x</a>)
81+
)
82+
end
83+
84+
it "is a Set of ids of attachments on the current page that are deletable" do
85+
get :index
86+
expect(assigns(:deletable_attachment_ids)).to be_a(Set)
87+
expect(assigns(:deletable_attachment_ids)).to include(deletable_attachment.id)
88+
expect(assigns(:deletable_attachment_ids)).not_to include(referenced_attachment.id)
89+
end
90+
91+
it "is scoped to the current page only" do
92+
get :index, params: {per_page: 1}
93+
# With per_page: 1 only one attachment is on the current page,
94+
# so the set must have at most one element.
95+
expect(assigns(:deletable_attachment_ids).size).to be <= 1
96+
end
97+
end
98+
7399
describe "by_file_type filter" do
74100
let!(:png) { create(:alchemy_attachment) }
75101

0 commit comments

Comments
 (0)