fix(Attachment): Detect references in ingredient values

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.

Author: tvdeyen

app/models/alchemy/attachment.rb

@@ -42,6 +42,33 @@ class Attachment < BaseRecord
     scope :recent, -> { where("#{table_name}.created_at > ?", Time.current - 24.hours).order(:created_at) }
     scope :without_tag, -> { left_outer_joins(:taggings).where(gutentag_taggings: {id: nil}) }
 
+    # Override +Alchemy::RelatableResource#deletable+ to also exclude
+    # attachments referenced from +/attachment/:id/download+ URLs inside
+    # ingredient values (e.g. Richtext markup, Link ingredients, raw Html).
+    # Those URLs are written by the file tab of the link dialog and are
+    # not tracked via the polymorphic +related_object+ association, so the
+    # base scope cannot see them.
+    #
+    # Uses a correlated +NOT EXISTS+ subquery that builds the per-row LIKE
+    # pattern with +Arel::Nodes::Concat+, which compiles to +||+ on
+    # SQLite/PostgreSQL and +CONCAT()+ on MySQL.
+    scope :deletable, -> do
+      ingredients = Alchemy::Ingredient.arel_table
+      pattern = Arel::Nodes::Concat.new(
+        Arel::Nodes::Concat.new(
+          Arel::Nodes.build_quoted("%/attachment/"),
+          arel_table[:id]
+        ),
+        Arel::Nodes.build_quoted("/download%")
+      )
+      referenced = ingredients
+        .project(1)
+        .where(ingredients[:value].matches(pattern))
+
+      where("#{table_name}.id NOT IN (#{RelatableResource::RELATED_INGREDIENTS_SUBQUERY})", type: name)
+        .where.not(referenced.exists)
+    end
+
     # We need to define this method here to have it available in the validations below.
     class << self
       # The class used to generate URLs for attachments
@@ -112,6 +139,13 @@ def slug
       CGI.escape(file_name.gsub(/\.#{extension}$/, "").tr(".", " "))
     end
 
+    # Override +Alchemy::RelatableResource#deletable?+ to also consider
+    # +/attachment/:id/download+ links inside ingredient values (e.g.
+    # Richtext markup, Link ingredients, raw Html).
+    def deletable?
+      super && !referenced_in_ingredient_value?
+    end
+
     # Checks if the attachment is restricted, because it is attached on restricted pages only
     def restricted?
       related_pages.any? && related_pages.not_restricted.blank?
@@ -178,6 +212,12 @@ def file_type_allowed
       end
     end
 
+    def referenced_in_ingredient_value?
+      Alchemy::Ingredient
+        .where("value LIKE ?", "%/attachment/#{id}/download%")
+        .exists?
+    end
+
     def set_name
       self.name ||= Alchemy.storage_adapter.file_basename(self).humanize
     end

spec/models/alchemy/attachment_spec.rb

@@ -21,6 +21,72 @@ module Alchemy
       resource_name: :attachment,
       ingredient_type: :file
 
+    describe ".deletable" do
+      let!(:richtext_linked_attachment) { create(:alchemy_attachment) }
+      let!(:link_linked_attachment) { create(:alchemy_attachment) }
+      let!(:unlinked_attachment) { create(:alchemy_attachment) }
+
+      before do
+        create(
+          :alchemy_ingredient_richtext,
+          value: %(<p>See <a href="/attachment/#{richtext_linked_attachment.id}/download/foo.pdf">this</a></p>)
+        )
+        create(
+          :alchemy_ingredient_link,
+          value: "/attachment/#{link_linked_attachment.id}/download"
+        )
+      end
+
+      it "excludes attachments linked from any ingredient download URL" do
+        expect(described_class.deletable).to include(unlinked_attachment)
+        expect(described_class.deletable).not_to include(richtext_linked_attachment)
+        expect(described_class.deletable).not_to include(link_linked_attachment)
+      end
+    end
+
+    describe "#deletable?" do
+      let(:attachment) { create(:alchemy_attachment) }
+
+      subject { attachment.deletable? }
+
+      context "when referenced by a /attachment/:id/download URL in a Richtext ingredient" do
+        before do
+          create(
+            :alchemy_ingredient_richtext,
+            value: %(<a href="/attachment/#{attachment.id}/download">x</a>)
+          )
+        end
+
+        it { is_expected.to be(false) }
+      end
+
+      context "when referenced by a /attachment/:id/download URL in a Link ingredient" do
+        before do
+          create(
+            :alchemy_ingredient_link,
+            value: "/attachment/#{attachment.id}/download"
+          )
+        end
+
+        it { is_expected.to be(false) }
+      end
+
+      context "when another ingredient links to a different attachment whose id shares a digit prefix" do
+        let(:other_id) { attachment.id * 10 }
+
+        before do
+          create(
+            :alchemy_ingredient_richtext,
+            value: %(<a href="/attachment/#{other_id}/download">x</a>)
+          )
+        end
+
+        it "does not falsely report as referenced" do
+          is_expected.to be(true)
+        end
+      end
+    end
+
     it "has file mime type accessor" do
       expect(attachment.file_mime_type).to eq("image/png")
     end