Skip to content

[8.2-stable] Fix Attachment#deletable? false positives#3859

Merged
tvdeyen merged 4 commits into8.2-stablefrom
backport/8.2-stable/pr-3855
Apr 28, 2026
Merged

[8.2-stable] Fix Attachment#deletable? false positives#3859
tvdeyen merged 4 commits into8.2-stablefrom
backport/8.2-stable/pr-3855

Conversation

@alchemycms-bot
Copy link
Copy Markdown

Backport

This will backport the following commits from main to 8.2-stable:

Questions ?

Please refer to the Backport tool documentation

tvdeyen added 4 commits April 28, 2026 08:43
Extracts the shared polymorphic-reference subquery from
RelatableResource into a constant so including classes
can reuse it if they oeverride the .deletable scope, because
you can't call super in scopes.

(cherry picked from commit d33e09c)
The deletable scope only considered polymorphic related_object
references, which missed attachments linked from /attachment/:id/download
URLs written into ingredient values by the file tab of the link dialog.
On sites where attachments are linked from Richtext, Link, or Html
ingredients, every attachment was reported as deletable, risking
accidental data loss.

The scope now adds a correlated NOT EXISTS against ingredient value
columns. The per-row LIKE pattern is built with Arel::Nodes::Concat so
it compiles to || on SQLite and PostgreSQL and to CONCAT() on MySQL
without adapter-specific SQL. The /download suffix in the pattern
delimits the id, so matching attachment 1 does not incorrectly pull in
URLs pointing at attachment 10.

(cherry picked from commit 3ed90b4)
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.

(cherry picked from commit f425112)
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.

(cherry picked from commit f32d773)
@alchemycms-bot alchemycms-bot Bot requested a review from a team as a code owner April 28, 2026 08:43
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.06%. Comparing base (7462271) to head (0722860).
⚠️ Report is 6 commits behind head on 8.2-stable.

Additional details and impacted files
@@             Coverage Diff             @@
##           8.2-stable    #3859   +/-   ##
===========================================
  Coverage       98.06%   98.06%           
===========================================
  Files             322      322           
  Lines            8530     8547   +17     
===========================================
+ Hits             8365     8382   +17     
  Misses            165      165           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tvdeyen tvdeyen merged commit c100d10 into 8.2-stable Apr 28, 2026
24 checks passed
@tvdeyen tvdeyen deleted the backport/8.2-stable/pr-3855 branch April 28, 2026 10:02
@alchemycms-bot alchemycms-bot Bot mentioned this pull request Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant