From 8b08b9a7636c2ac7d400c0fa663372e6d70a885b Mon Sep 17 00:00:00 2001
From: Blake Erickson <o.blakeerickson@gmail.com>
Date: Wed, 27 Jul 2022 07:28:44 +0100
Subject: [PATCH] FIX: Rejected emails should not be cleaned up before their
 logs (#17648)

* FIX: Rejected emails should not be cleaned up before their logs

If we delete the rejected emails before we delete their associated logs
we will receive 404 errors trying to inspect an email message for that
log.

* don't add a blank line

* test for max value as well

* pr cleanup and add migration

* Fix failing test
---
 config/locales/server.en.yml                  |  1 +
 config/site_settings.yml                      |  2 +-
 ..._rejected_email_after_days_site_setting.rb | 26 +++++++++++++++++++
 ...ete_rejected_email_after_days_validator.rb | 17 ++++++++++++
 .../delete_rejected_email_after_days_spec.rb  | 15 +++++++++++
 5 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20220726164831_fix_delete_rejected_email_after_days_site_setting.rb
 create mode 100644 lib/validators/delete_rejected_email_after_days_validator.rb
 create mode 100644 spec/lib/validators/delete_rejected_email_after_days_spec.rb

diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 32ae6be552c..cf7c5f8a517 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -2432,6 +2432,7 @@ en:
       search_tokenize_chinese_enabled: "You must disable 'search_tokenize_chinese' before enabling this setting."
       search_tokenize_japanese_enabled: "You must disable 'search_tokenize_japanese' before enabling this setting."
       discourse_connect_cannot_be_enabled_if_second_factor_enforced: "You cannot enable DiscourseConnect if 2FA is enforced."
+      delete_rejected_email_after_days: "This setting cannot be set lower than the delete_email_logs_after_days setting or greater than %{max}"
 
     placeholder:
       discourse_connect_provider_secrets:
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 5f05a26a650..293edff401b 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1266,7 +1266,7 @@ email:
   raw_rejected_email_max_length: 4000
   delete_rejected_email_after_days:
     default: 90
-    max: 36500
+    validator: "DeleteRejectedEmailAfterDaysValidator"
   enable_secondary_emails:
     client: true
     default: true
diff --git a/db/migrate/20220726164831_fix_delete_rejected_email_after_days_site_setting.rb b/db/migrate/20220726164831_fix_delete_rejected_email_after_days_site_setting.rb
new file mode 100644
index 00000000000..89ce57c1d8c
--- /dev/null
+++ b/db/migrate/20220726164831_fix_delete_rejected_email_after_days_site_setting.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+class FixDeleteRejectedEmailAfterDaysSiteSetting < ActiveRecord::Migration[6.1]
+  def up
+    delete_rejected_email_after_days = DB.query_single("SELECT value FROM site_settings WHERE name = 'delete_rejected_email_after_days'").first
+    delete_email_logs_after_days = DB.query_single("SELECT value FROM site_settings WHERE name = 'delete_email_logs_after_days'").first
+
+    # These settings via the sql query return nil if they are using their default values
+    unless delete_email_logs_after_days
+      delete_email_logs_after_days = DeleteRejectedEmailAfterDaysValidator::MAX
+    end
+
+    # Only update if the setting is not using the default and it is lower than 'delete_email_logs_after_days'
+    if delete_rejected_email_after_days != nil && delete_rejected_email_after_days.to_i < delete_email_logs_after_days.to_i
+      execute <<~SQL
+      UPDATE site_settings
+      SET value = #{delete_email_logs_after_days.to_i}
+      WHERE name = 'delete_rejected_email_after_days'
+      SQL
+    end
+
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end
diff --git a/lib/validators/delete_rejected_email_after_days_validator.rb b/lib/validators/delete_rejected_email_after_days_validator.rb
new file mode 100644
index 00000000000..0ad322e7064
--- /dev/null
+++ b/lib/validators/delete_rejected_email_after_days_validator.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class DeleteRejectedEmailAfterDaysValidator
+  MAX = 36500
+
+  def initialize(opts = {})
+    @opts = opts
+  end
+
+  def valid_value?(value)
+    @valid = value.to_i >= SiteSetting.delete_email_logs_after_days && value.to_i <= MAX
+  end
+
+  def error_message
+    I18n.t("site_settings.errors.delete_rejected_email_after_days", max: MAX) if !@valid
+  end
+end
diff --git a/spec/lib/validators/delete_rejected_email_after_days_spec.rb b/spec/lib/validators/delete_rejected_email_after_days_spec.rb
new file mode 100644
index 00000000000..4a410983ad3
--- /dev/null
+++ b/spec/lib/validators/delete_rejected_email_after_days_spec.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+describe DeleteRejectedEmailAfterDaysValidator do
+
+  it 'is not valid if value is smaller than the value of SiteSetting.delete_email_logs_after_days' do
+    SiteSetting.delete_email_logs_after_days = 90
+
+    expect { SiteSetting.delete_rejected_email_after_days = 89 }.to raise_error(Discourse::InvalidParameters)
+  end
+
+  it "is not valid if value is greater than #{DeleteRejectedEmailAfterDaysValidator::MAX}" do
+    expect { SiteSetting.delete_rejected_email_after_days = DeleteRejectedEmailAfterDaysValidator::MAX + 1 }.to raise_error(Discourse::InvalidParameters)
+  end
+
+end