FIX: only attach images in digests (#30844)

When secure uploads are enabled, we have to attach the images in the
digest so they can show up in the email.

However, we send attaching all the attachments, including "files" and
"media".

This ensures we only attach images when sending a digest.

Internal t/144542

lib/email/sender.rb

@@ -257,7 +257,7 @@ module Email
         add_attachments(post)
       elsif @email_type.to_s == "digest"
         @stripped_secure_upload_shas = style.stripped_upload_sha_map.values
-        add_attachments(*digest_posts)
+        add_attachments(*digest_posts, is_digest: true)
       end
 
       # Suppress images from short emails
@@ -356,7 +356,7 @@ module Email
       Post.where(id: header_value("X-Discourse-Post-Ids")&.split(","))
     end
 
-    def add_attachments(*posts)
+    def add_attachments(*posts, is_digest: false)
       max_email_size = SiteSetting.email_total_attachment_size_limit_kb.kilobytes
       return if max_email_size == 0
 
@@ -367,6 +367,9 @@ module Email
         post.uploads.each do |original_upload|
           optimized_1X = original_upload.optimized_images.first
 
+          # only attach images in digests
+          next if is_digest && !FileHelper.is_supported_image?(original_upload.original_filename)
+
           if FileHelper.is_supported_image?(original_upload.original_filename) &&
                !should_attach_image?(original_upload, optimized_1X)
             next

spec/lib/email/sender_spec.rb

@@ -599,7 +599,7 @@ RSpec.describe Email::Sender do
     context "when secure uploads enabled" do
       before do
         setup_s3
-        store = stub_s3_store
+        stub_s3_store
 
         SiteSetting.secure_uploads = true
         SiteSetting.login_required = true
@@ -654,10 +654,12 @@ RSpec.describe Email::Sender do
           expect(message.to_s.scan(/cid:[\w\-@.]+/).uniq.length).to eq(2)
         end
 
-        it "attaches allowed images from multiple posts in the activity summary" do
+        it "attaches only allowed images from multiple posts in the activity summary" do
           digest_post = Fabricate(:post)
           other_digest_post = Fabricate(:post)
 
+          SiteSetting.authorized_extensions = "*"
+
           Topic.stubs(:for_digest).returns(
             Topic.where(id: [digest_post.topic_id, other_digest_post.topic_id]),
           )
@@ -680,16 +682,53 @@ RSpec.describe Email::Sender do
           @secure_image_3.update_secure_status(override: true)
           @secure_image_3.update(access_control_post_id: other_digest_post.id)
 
+          @secure_attachment =
+            UploadCreator.new(
+              file_from_fixtures("small.pdf", "pdf"),
+              "cool-attachment.pdf",
+            ).create_for(Discourse.system_user.id)
+          @secure_attachment.update_secure_status(override: true)
+          @secure_attachment.update(access_control_post_id: other_digest_post.id)
+
+          @secure_video =
+            UploadCreator.new(
+              file_from_fixtures("small.mp4", "media"),
+              "cool-video.mp4",
+            ).create_for(Discourse.system_user.id)
+          @secure_video.update_secure_status(override: true)
+          @secure_video.update(access_control_post_id: other_digest_post.id)
+
           Jobs::PullHotlinkedImages.any_instance.expects(:execute)
-          digest_post.update(
-            raw:
-              "#{UploadMarkdown.new(@secure_image).image_markdown}\n#{UploadMarkdown.new(@secure_image_2).image_markdown}",
-          )
+
+          raw = <<~MD
+            IMAGE #1
+            #{UploadMarkdown.new(@secure_image).image_markdown}
+            
+            IMAGE #2
+            #{UploadMarkdown.new(@secure_image_2).image_markdown}
+          MD
+
+          digest_post.update(raw:)
           digest_post.rebake!
 
-          other_digest_post.update(raw: "#{UploadMarkdown.new(@secure_image_3).image_markdown}")
+          expect(digest_post.upload_references.size).to eq(2)
+
+          raw = <<~MD
+            IMAGE #3
+            #{UploadMarkdown.new(@secure_image_3).image_markdown}
+            
+            ATTACHMENT
+            #{UploadMarkdown.new(@secure_attachment).attachment_markdown}
+
+            VIDEO
+            #{UploadMarkdown.new(@secure_video).playable_media_markdown}
+          MD
+
+          other_digest_post.update(raw:)
           other_digest_post.rebake!
 
+          expect(other_digest_post.upload_references.size).to eq(3)
+
           summary.header["X-Discourse-Post-Id"] = nil
           summary.header["X-Discourse-Post-Ids"] = "#{digest_post.id},#{other_digest_post.id}"
 
@@ -698,7 +737,7 @@ RSpec.describe Email::Sender do
           expect(summary.content_type).to eq(
             "multipart/mixed; boundary=\"#{summary.body.boundary}\"",
           )
-          expect(summary.attachments.map(&:filename)).to include(
+          expect(summary.attachments.map(&:filename)).to contain_exactly(
             *[@secure_image, @secure_image_2, @secure_image_3].map(&:original_filename),
           )
           expect(summary.attachments.size).to eq(3)