FEATURE: Clean up PostReplyKey records.

* Default retention of 90 days.
This commit is contained in:
Guo Xiang Tan 2018-08-21 16:17:08 +08:00
parent 8da2d8df3d
commit 36a7028f19
9 changed files with 97 additions and 7 deletions

View File

@ -0,0 +1,14 @@
module Jobs
class CleanUpPostReplyKeys < Jobs::Scheduled
every 1.day
def execute(_)
return if SiteSetting.disallow_reply_by_email_after_days <= 0
PostReplyKey.where(
"created_at < ?",
SiteSetting.disallow_reply_by_email_after_days.days.ago
).delete_all
end
end
end

View File

@ -1565,6 +1565,7 @@ en:
unsubscribe_via_email_footer: "Attach an unsubscribe via email mailto: link to the footer of sent emails"
delete_email_logs_after_days: "Delete email logs after (N) days. 0 to keep indefinitely"
disallow_reply_by_email_after_days: "Disallow reply by email after (N) days. 0 to keep indefinitely"
max_emails_per_day_per_user: "Maximum number of emails to send users per day. 0 to disable the limit"
enable_staged_users: "Automatically create staged users when processing incoming emails."
maximum_staged_users_per_email: "Maximum number of staged users created when processing an incoming email."
@ -2554,7 +2555,7 @@ en:
text_body_template: |
We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work.
For security reasons we only accept replies to original notifications for 90 days. Please [visit the topic](%{short_url}) to continue the conversation.
For security reasons we only accept replies to original notifications for %{number_of_days} days. Please [visit the topic](%{short_url}) to continue the conversation.
email_reject_topic_not_found:
title: "Email Reject Topic Not Found"

View File

@ -816,6 +816,9 @@ email:
default: true
unsubscribe_via_email_footer:
default: false
disallow_reply_by_email_after_days:
default: 90
shadowed_by_global: true
delete_email_logs_after_days:
default: 90
shadowed_by_global: true

View File

@ -85,6 +85,7 @@ module Email
if message_template == :email_reject_old_destination
template_args[:short_url] = e.message
template_args[:number_of_days] = SiteSetting.disallow_reply_by_email_after_days
end
if message_template

View File

@ -171,8 +171,12 @@ module Email
raise first_exception if first_exception
if post = find_related_post(force: true)
if Guardian.new(user).can_see_post?(post) && post.created_at < 90.days.ago
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
raise OldDestinationError.new("#{Discourse.base_url}/p/#{post.id}")
end
end

View File

@ -161,4 +161,27 @@ describe Email::Processor do
expect { Email::Processor.process!(email) }.to_not change { EmailLog.count }
end
end
describe 'when replying to a post that is too old' do
let(:mail) { file_from_fixtures("old_destination.eml", "emails").read }
it 'rejects the email with the right response' do
SiteSetting.disallow_reply_by_email_after_days = 2
topic = Fabricate(:topic, id: 424242)
post = Fabricate(:post, topic: topic, id: 123456, created_at: 3.days.ago)
processor = Email::Processor.new(mail)
processor.process!
rejection_raw = ActionMailer::Base.deliveries.first.body.to_s
expect(rejection_raw).to eq(I18n.t("system_messages.email_reject_old_destination.text_body_template",
destination: '["reply+4f97315cc828096c9cb34c6f1a0d6fe8@bar.com"]',
former_title: 'Some Old Post',
short_url: "#{Discourse.base_url}/p/#{post.id}",
number_of_days: 2
))
end
end
end

View File

@ -79,10 +79,28 @@ describe Email::Receiver do
end
it "raises an OldDestinationError when notification is too old" do
topic = Fabricate(:topic, id: 424242)
post = Fabricate(:post, topic: topic, id: 123456, created_at: 1.year.ago)
SiteSetting.disallow_reply_by_email_after_days = 2
expect { process(:old_destination) }.to raise_error(Email::Receiver::OldDestinationError)
topic = Fabricate(:topic, id: 424242)
post = Fabricate(:post, topic: topic, id: 123456)
expect { process(:old_destination) }.to raise_error(
Email::Receiver::BadDestinationAddress
)
IncomingEmail.destroy_all
post.update!(created_at: 3.days.ago)
expect { process(:old_destination) }.to raise_error(
Email::Receiver::OldDestinationError
)
SiteSetting.disallow_reply_by_email_after_days = 0
IncomingEmail.destroy_all
expect { process(:old_destination) }.to raise_error(
Email::Receiver::BadDestinationAddress
)
end
it "raises a BouncerEmailError when email is a bounced email" do

View File

@ -1,6 +1,6 @@
Return-Path: <staged@bar.com>
From: Foo Bar <staged@bar.com>
To: wat@bar.com
To: reply+4f97315cc828096c9cb34c6f1a0d6fe8@bar.com
Cc: foofoo@bar.com
Date: Fri, 15 Jan 2018 00:12:43 +0100
Message-ID: <999@foo.bar.mail>
@ -8,5 +8,6 @@ In-Reply-To: <topic/424242/123456@test.localhost>
Mime-Version: 1.0
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Subject: Some Old Post
Lorem ipsum dolor sit amet, consectetur adipiscing elit.

View File

@ -0,0 +1,25 @@
require 'rails_helper'
RSpec.describe Jobs::CleanUpPostReplyKeys do
it 'removes old post_reply_keys' do
freeze_time
reply_key1 = Fabricate(:post_reply_key, created_at: 1.day.ago)
reply_key2 = Fabricate(:post_reply_key, created_at: 2.days.ago)
Fabricate(:post_reply_key, created_at: 3.days.ago)
SiteSetting.disallow_reply_by_email_after_days = 0
expect { Jobs::CleanUpPostReplyKeys.new.execute({}) }
.to change { PostReplyKey.count }.by(0)
SiteSetting.disallow_reply_by_email_after_days = 2
expect { Jobs::CleanUpPostReplyKeys.new.execute({}) }
.to change { PostReplyKey.count }.by(-1)
expect(PostReplyKey.all).to contain_exactly(
reply_key1, reply_key2
)
end
end