diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index cf7534a92f9..dd99be5d7fc 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -111,7 +111,10 @@ en: maximum_staged_user_per_email_reached: "Reached maximum number of staged users created per email." no_subject: "(no subject)" no_body: "(no body)" - missing_attachment: "(Attachment %{filename} is missing)" + missing_attachment: "(Attachment %{filename} is missing) ago." + continuing_old_discussion: + one: "Continuing the discussion from [%{title}](%{url}), because it was created more than %{count} day ago." + other: "Continuing the discussion from [%{title}](%{url}), because it was created more than %{count} days ago." errors: empty_email_error: "Happens when the raw mail we received was blank." no_message_id_error: "Happens when the mail has no 'Message-Id' header." diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index da699c08bcd..a6da67cbaa1 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -224,12 +224,12 @@ module Email # We don't stage new users for emails to reply addresses, exit if user is nil raise BadDestinationAddress if user.blank? + # We only get here if there are no destinations (the email is not going to + # a Category, Group, or PostReplyKey) post = find_related_post(force: true) if post && Guardian.new(user).can_see_post?(post) - num_of_days = SiteSetting.disallow_reply_by_email_after_days - - if num_of_days > 0 && post.created_at < num_of_days.days.ago + if destination_too_old?(post) raise OldDestinationError.new("#{Discourse.base_url}/p/#{post.id}") end end @@ -765,37 +765,47 @@ module Email .order(created_at: :desc) .limit(1) .pluck(:post_id).last - post_ids << post_id_from_email_log + post_ids << post_id_from_email_log if post_id_from_email_log end - if post_ids.any? && post = Post.where(id: post_ids).order(:created_at).last - - # this must be done for the unknown user (who is staged) to - # be allowed to post a reply in the topic - if group.allow_unknown_sender_topic_replies - post.topic.topic_allowed_users.find_or_create_by!(user_id: user.id) - end - - create_reply(user: user, - raw: body, - elided: elided, - post: post, - topic: post.topic, - skip_validations: true) - else - enable_email_pm_setting(user) + target_post = post_ids.any? && Post.where(id: post_ids).order(:created_at).last + too_old_for_group_smtp = (destination_too_old?(target_post) && group.smtp_enabled) + if target_post.blank? || too_old_for_group_smtp create_topic(user: user, - raw: body, + raw: new_group_topic_body(body, target_post, too_old_for_group_smtp), elided: elided, title: subject, archetype: Archetype.private_message, target_group_names: [group.name], is_group_message: true, skip_validations: true) + else + # This must be done for the unknown user (who is staged) to + # be allowed to post a reply in the topic. + if group.allow_unknown_sender_topic_replies + target_post.topic.topic_allowed_users.find_or_create_by!(user_id: user.id) + end + + create_reply(user: user, + raw: body, + elided: elided, + post: target_post, + topic: target_post.topic, + skip_validations: true) end end + def new_group_topic_body(body, target_post, too_old_for_group_smtp) + return body if !too_old_for_group_smtp + body + "\n\n----\n\n" + I18n.t( + "emails.incoming.continuing_old_discussion", + url: target_post.topic.url, + title: target_post.topic.title, + count: SiteSetting.disallow_reply_by_email_after_days + ) + end + def forwarded_reply_key?(post_reply_key, user) incoming_emails = IncomingEmail .joins(:post) @@ -1042,6 +1052,10 @@ module Email end def create_topic(options = {}) + if options[:archetype] == Archetype.private_message + enable_email_pm_setting(options[:user]) + end + create_post_with_attachments(options) end @@ -1327,5 +1341,11 @@ module Email user.user_option.update!(email_messages_level: UserOption.email_level_types[:always]) end end + + def destination_too_old?(post) + return false if post.blank? + num_of_days = SiteSetting.disallow_reply_by_email_after_days + num_of_days > 0 && post.created_at < num_of_days.days.ago + end end end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 11a87b4e632..79e14445bcb 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -986,8 +986,14 @@ describe Email::Receiver do end context "emailing a group by email_username and following reply flow" do - let!(:topic) do - group.update!(email_username: "team@somesmtpaddress.com") + let!(:original_inbound_email_topic) do + group.update!( + email_username: "team@somesmtpaddress.com", + smtp_server: "smtp.test.com", + smtp_port: 587, + smtp_ssl: true, + smtp_enabled: true + ) process(:email_to_group_email_username_1) Topic.last end @@ -999,6 +1005,7 @@ describe Email::Receiver do before do NotificationEmailer.enable + SiteSetting.disallow_reply_by_email_after_days = 10000 Jobs.run_immediately! end @@ -1006,17 +1013,17 @@ describe Email::Receiver do group_post = PostCreator.create( user_in_group, raw: "Thanks for your request. Please try to restart.", - topic_id: topic.id + topic_id: original_inbound_email_topic.id ) email_log = EmailLog.last [email_log, group_post] end it "the inbound processed email creates an incoming email and topic record correctly, and adds the group to the topic" do - incoming = IncomingEmail.find_by(topic: topic) + incoming = IncomingEmail.find_by(topic: original_inbound_email_topic) user = User.find_by_email("two@foo.com") - expect(topic.topic_allowed_users.first.user_id).to eq(user.id) - expect(topic.topic_allowed_groups.first.group_id).to eq(group.id) + expect(original_inbound_email_topic.topic_allowed_users.first.user_id).to eq(user.id) + expect(original_inbound_email_topic.topic_allowed_groups.first.group_id).to eq(group.id) expect(incoming.to_addresses).to eq("team@somesmtpaddress.com") expect(incoming.from_address).to eq("two@foo.com") expect(incoming.message_id).to eq("u4w8c9r4y984yh98r3h69873@foo.bar.mail") @@ -1024,7 +1031,7 @@ describe Email::Receiver do it "creates an EmailLog when someone from the group replies, and does not create an IncomingEmail record for the reply" do email_log, group_post = reply_as_group_user - expect(email_log.message_id).to eq("topic/#{topic.id}/#{group_post.id}@test.localhost") + expect(email_log.message_id).to eq("topic/#{original_inbound_email_topic.id}/#{group_post.id}@test.localhost") expect(email_log.to_address).to eq("two@foo.com") expect(email_log.email_type).to eq("user_private_message") expect(email_log.post_id).to eq(group_post.id) @@ -1056,7 +1063,7 @@ describe Email::Receiver do end.to change { Topic.count }.by(1).and change { Post.count }.by(1) reply_post = Post.last - expect(reply_post.topic_id).not_to eq(topic.id) + expect(reply_post.topic_id).not_to eq(original_inbound_email_topic.id) end it "processes the reply from the user as a reply if they have replied from a different address (e.g. auto forward) and allow_unknown_sender_topic_replies is enabled" do @@ -1070,7 +1077,32 @@ describe Email::Receiver do end.to change { Topic.count }.by(0).and change { Post.count }.by(1) reply_post = Post.last - expect(reply_post.topic_id).to eq(topic.id) + expect(reply_post.topic_id).to eq(original_inbound_email_topic.id) + end + + it "creates a new topic with a reference back to the original if replying to a too old topic" do + SiteSetting.disallow_reply_by_email_after_days = 2 + email_log, group_post = reply_as_group_user + + group_post.update(created_at: 10.days.ago) + group_post.topic.update(created_at: 10.days.ago) + + reply_email = email(:email_to_group_email_username_2) + reply_email.gsub!("MESSAGE_ID_REPLY_TO", email_log.message_id) + expect do + Email::Receiver.new(reply_email).process! + end.to change { Topic.count }.by(1).and change { Post.count }.by(1) + + reply_post = Post.last + new_topic = Topic.last + + expect(reply_post.topic).to eq(new_topic) + expect(reply_post.raw).to include( + I18n.t( + "emails.incoming.continuing_old_discussion", + url: group_post.topic.url, title: group_post.topic.title, count: SiteSetting.disallow_reply_by_email_after_days + ) + ) end end