diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 13d8b5ffe44..ee765793ec0 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -231,14 +231,14 @@ class UserNotifications < ActionMailer::Base limit: SiteSetting.digest_topics + SiteSetting.digest_other_topics, top_order: true, } - topics_for_digest = Topic.for_digest(user, min_date, digest_opts).to_a + topics_for_digest = Topic.for_digest(user, min_date, digest_opts) if topics_for_digest.empty? && !user.user_option.try(:include_tl0_in_digests) # Find some topics from new users that are at least 24 hours old topics_for_digest = - Topic - .for_digest(user, min_date, digest_opts.merge(include_tl0: true)) - .where("topics.created_at < ?", 24.hours.ago) - .to_a + Topic.for_digest(user, min_date, digest_opts.merge(include_tl0: true)).where( + "topics.created_at < ?", + 24.hours.ago, + ) end @popular_topics = topics_for_digest[0, SiteSetting.digest_topics] @@ -349,6 +349,9 @@ class UserNotifications < ActionMailer::Base ), add_unsubscribe_link: true, unsubscribe_url: "#{Discourse.base_url}/email/unsubscribe/#{@unsubscribe_key}", + topic_ids: topics_for_digest.pluck(:id), + post_ids: + topics_for_digest.joins(:posts).where(posts: { post_number: 1 }).pluck("posts.id"), } build_email(user.email, opts) diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb index 396085955a4..0cb15ef0db0 100644 --- a/lib/email/message_builder.rb +++ b/lib/email/message_builder.rb @@ -218,7 +218,9 @@ module Email end result["X-Discourse-Post-Id"] = @opts[:post_id].to_s if @opts[:post_id] + result["X-Discourse-Post-Ids"] = @opts[:post_ids].join(",") if @opts[:post_ids].present? result["X-Discourse-Topic-Id"] = @opts[:topic_id].to_s if @opts[:topic_id] + result["X-Discourse-Topic-Ids"] = @opts[:topic_ids].join(",") if @opts[:topic_ids].present? # at this point these have been filtered by the recipient's guardian for visibility, # see UserNotifications#send_notification_email diff --git a/lib/email/sender.rb b/lib/email/sender.rb index abf39dc919b..02738081e86 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -253,6 +253,9 @@ module Email if post.present? @stripped_secure_upload_shas = style.stripped_upload_sha_map.values add_attachments(post) + elsif @email_type == :digest + @stripped_secure_upload_shas = style.stripped_upload_sha_map.values + digest_posts.each { |p| add_attachments(p) } end # Suppress images from short emails @@ -347,6 +350,10 @@ module Email private + def digest_posts + Post.where(id: header_value("X-Discourse-Post-Ids")&.split(",")) + end + def add_attachments(post) max_email_size = SiteSetting.email_total_attachment_size_limit_kb.kilobytes return if max_email_size == 0 diff --git a/spec/lib/email/sender_spec.rb b/spec/lib/email/sender_spec.rb index 91299ca43b1..91a633dc259 100644 --- a/spec/lib/email/sender_spec.rb +++ b/spec/lib/email/sender_spec.rb @@ -640,6 +640,31 @@ 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 + other_post = Fabricate(:post) + @secure_image_2 = + UploadCreator.new( + file_from_fixtures("logo-dev.png", "images"), + "something-cool.png", + ).create_for(Discourse.system_user.id) + @secure_image_2.update_secure_status(override: true) + @secure_image_2.update(access_control_post_id: other_post.id) + + Jobs::PullHotlinkedImages.any_instance.expects(:execute) + other_post.update( + raw: + "#{UploadMarkdown.new(@secure_image).image_markdown}\n#{UploadMarkdown.new(@secure_image_2).image_markdown}", + ) + other_post.rebake! + + message.header["X-Discourse-Post-Id"] = nil + message.header["X-Discourse-Post-Ids"] = "#{reply.id},#{other_post.id}" + Email::Sender.new(message, :digest).send + expect(message.attachments.map(&:filename)).to include( + *[image, @secure_image, @secure_image_2].map(&:original_filename), + ) + end + it "does not attach images that are not marked as secure, in the case of a non-secure upload copied to a PM" do SiteSetting.login_required = false @secure_image.update_secure_status(override: false) diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 9593524be26..adc4f2137b2 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -181,10 +181,18 @@ RSpec.describe UserNotifications do end context "with new topics" do - let!(:popular_topic) do - Fabricate(:topic, user: Fabricate(:coding_horror), created_at: 1.hour.ago) + fab!(:coding_horror) + + let!(:popular_topic) { Fabricate(:topic, user: coding_horror, created_at: 1.hour.ago) } + + let!(:another_popular_topic) do + Fabricate(:topic, user: coding_horror, created_at: 1.hour.ago) end + let!(:post) { Fabricate(:post, topic: popular_topic, post_number: 1) } + + let!(:another_post) { Fabricate(:post, topic: another_popular_topic, post_number: 1) } + it "works" do expect(email.to).to eq([user.email]) expect(email.subject).to be_present @@ -192,6 +200,10 @@ RSpec.describe UserNotifications do expect(email.html_part.body.to_s).to be_present expect(email.text_part.body.to_s).to be_present expect(email.header["List-Unsubscribe"].to_s).to match(/\/email\/unsubscribe\/\h{64}/) + expect(email.header["X-Discourse-Topic-Ids"].to_s).to eq( + "#{another_popular_topic.id},#{popular_topic.id}", + ) + expect(email.header["X-Discourse-Post-Ids"].to_s).to eq("#{another_post.id},#{post.id}") expect(email.html_part.body.to_s).to include("New Users") end