Merge hotfixes of main into dev (#722)

* Set retry option to false for UserCleanerJob (#715)

* Deal with registration edge cases in UserCleaner (#693) (#716)

* Deal with registration edge cases in UserCleaner

* Replace `last_sign_in_at` by `current_sign_in_at`

* Simplify inactive_users method via usage of scopes

* Add another scope for recently active users

* Fix unknown user variable after deletion (#717)

* Fix unknown user variable after deletion

* Adjust other occurrence of method

* Update failing user cleaner jobs with new method signature

* Expect user email for deletion mail

* Fix `with` syntax

---------

Co-authored-by: fosterfarrell9 <[email protected]>

.vscode/settings.json

@@ -122,6 +122,7 @@
         "selectize",
         "Timecop",
         "turbolinks",
+        "Unsets",
         "uncached",
         "whitespaces"
     ],

app/mailers/user_cleaner_mailer.rb

@@ -5,24 +5,22 @@ class UserCleanerMailer < ApplicationMailer
   #
   # @param [Integer] num_days_until_deletion:
   #  The number of days until the account will be deleted.
-  def pending_deletion_email(num_days_until_deletion)
-    user = params[:user]
+  def pending_deletion_email(user_email, user_locale, num_days_until_deletion)
     sender = "#{t("mailer.warning")} <#{DefaultSetting::PROJECT_EMAIL}>"
-    I18n.locale = user.locale
+    I18n.locale = user_locale
 
     @num_days_until_deletion = num_days_until_deletion
     subject = t("mailer.pending_deletion_subject",
                 num_days_until_deletion: @num_days_until_deletion)
-    mail(from: sender, to: user.email, subject: subject, priority: "high")
+    mail(from: sender, to: user_email, subject: subject, priority: "high")
   end
 
   # Creates an email to inform a user that their account has been deleted.
-  def deletion_email
-    user = params[:user]
+  def deletion_email(user_email, user_locale)
     sender = "#{t("mailer.warning")} <#{DefaultSetting::PROJECT_EMAIL}>"
-    I18n.locale = user.locale
+    I18n.locale = user_locale
 
     subject = t("mailer.deletion_subject")
-    mail(from: sender, to: user.email, subject: subject, priority: "high")
+    mail(from: sender, to: user_email, subject: subject, priority: "high")
   end
 end

app/models/user_cleaner.rb

@@ -76,8 +76,9 @@ def set_deletion_date_for_inactive_users
                   .find_each do |user|
       user.update(deletion_date: Date.current + 40.days)
 
-      if user.generic? # rubocop:disable Style/IfUnlessModifier
-        UserCleanerMailer.with(user: user).pending_deletion_email(40).deliver_later
+      if user.generic?
+        UserCleanerMailer.pending_deletion_email(user.email, user.locale, 40)
+                         .deliver_later
       end
     end
   end
@@ -114,7 +115,7 @@ def delete_users_according_to_deletion_date!
     User.where(deletion_date: ..Date.current).find_each do |user|
       next unless user.generic?
 
-      UserCleanerMailer.with(user: user).deletion_email.deliver_later
+      UserCleanerMailer.deletion_email(user.email, user.locale).deliver_later
       user.destroy
       num_deleted_users += 1
     end
@@ -133,9 +134,9 @@ def send_additional_warning_mails
       num_days_until_deletion = (user.deletion_date - Date.current).to_i
 
       if [14, 7, 2].include?(num_days_until_deletion)
-        UserCleanerMailer.with(user: user)
-                         .pending_deletion_email(num_days_until_deletion)
-                         .deliver_later
+        UserCleanerMailer
+          .pending_deletion_email(user.email, user.locale, num_days_until_deletion)
+          .deliver_later
       end
     end
   end

app/workers/user_cleaner_job.rb

@@ -1,5 +1,6 @@
 class UserCleanerJob
   include Sidekiq::Worker
+  sidekiq_options retry: false # job will be discarded if it fails
 
   def perform
     # Only run this job in production, not for mampf-experimental or mampf-dev.

spec/models/user_cleaner_spec.rb

@@ -192,12 +192,12 @@ def test_non_confirmed_user(confirmation_sent_date, expected_inactive_users_coun
   describe("mails") do
     context "when setting a deletion date" do
       it "enqueues a deletion warning mail (40 days)" do
-        FactoryBot.create(:confirmed_user, current_sign_in_at: 7.months.ago)
+        user = FactoryBot.create(:confirmed_user, current_sign_in_at: 7.months.ago)
 
         expect do
           UserCleaner.new.set_deletion_date_for_inactive_users
-        end.to have_enqueued_mail(UserCleanerMailer, :pending_deletion_email)
-          .with(a_hash_including(args: [40]))
+        end.to(have_enqueued_mail(UserCleanerMailer, :pending_deletion_email)
+          .with(user.email, user.locale, 40))
       end
 
       it "does not enqueue a deletion warning mail (40 days) for non-generic users" do
@@ -213,12 +213,12 @@ def test_non_confirmed_user(confirmation_sent_date, expected_inactive_users_coun
 
     context "when a deletion date is assigned" do
       def test_enqueues_additional_deletion_warning_mails(num_days)
-        FactoryBot.create(:confirmed_user, deletion_date: Date.current + num_days.days)
+        user = FactoryBot.create(:confirmed_user, deletion_date: Date.current + num_days.days)
 
         expect do
           UserCleaner.new.send_additional_warning_mails
-        end.to have_enqueued_mail(UserCleanerMailer, :pending_deletion_email)
-          .with(a_hash_including(args: [num_days]))
+        end.to(have_enqueued_mail(UserCleanerMailer, :pending_deletion_email)
+        .with(user.email, user.locale, num_days))
       end
 
       it "enqueues additional deletion warning mails" do
@@ -248,11 +248,12 @@ def test_enqueues_additional_deletion_warning_mails(num_days)
 
     context "when a user is finally deleted" do
       it "enqueues a deletion mail" do
-        FactoryBot.create(:confirmed_user, deletion_date: Date.current - 1.day)
+        user = FactoryBot.create(:confirmed_user, deletion_date: Date.current - 1.day)
 
         expect do
           UserCleaner.new.delete_users_according_to_deletion_date!
-        end.to have_enqueued_mail(UserCleanerMailer, :deletion_email)
+        end.to(have_enqueued_mail(UserCleanerMailer, :deletion_email)
+          .with(user.email, user.locale))
       end
     end
   end
@@ -263,7 +264,8 @@ def test_enqueues_additional_deletion_warning_mails(num_days)
 
     def test_subject_line(num_days)
       user = FactoryBot.create(:confirmed_user)
-      mailer = UserCleanerMailer.with(user: user).pending_deletion_email(num_days)
+      mailer = UserCleanerMailer
+               .pending_deletion_email(user.email, user.locale, num_days)
       expect(mailer.subject).to include(num_days.to_s)
     end
 
@@ -275,11 +277,13 @@ def test_subject_line(num_days)
     end
 
     it "has mail subject localized to the user's locale" do
-      mailer = UserCleanerMailer.with(user: user_de).pending_deletion_email(40)
+      mailer = UserCleanerMailer
+               .pending_deletion_email(user_de.email, user_de.locale, 40)
       expect(mailer.subject).to include("Tage")
       expect(mailer.subject).not_to include("days")
 
-      mailer = UserCleanerMailer.with(user: user_en).pending_deletion_email(40)
+      mailer = UserCleanerMailer
+               .pending_deletion_email(user_en.email, user_en.locale, 40)
       expect(mailer.subject).to include("days")
       expect(mailer.subject).not_to include("Tage")
     end
@@ -288,11 +292,13 @@ def test_subject_line(num_days)
       expected_de = "verloren"
       expected_en = "lost"
 
-      mailer = UserCleanerMailer.with(user: user_de).pending_deletion_email(40)
+      mailer = UserCleanerMailer
+               .pending_deletion_email(user_de.email, user_de.locale, 40)
       expect(mailer.html_part.body).to include(expected_de)
       expect(mailer.html_part.body).not_to include(expected_en)
 
-      mailer = UserCleanerMailer.with(user: user_en).pending_deletion_email(40)
+      mailer = UserCleanerMailer
+               .pending_deletion_email(user_en.email, user_en.locale, 40)
       expect(mailer.html_part.body).to include(expected_en)
       expect(mailer.html_part.body).not_to include(expected_de)
     end
@@ -303,11 +309,11 @@ def test_subject_line(num_days)
     let(:user_en) { FactoryBot.create(:confirmed_user, locale: "en") }
 
     it "has mail subject localized to the user's locale" do
-      mailer = UserCleanerMailer.with(user: user_de).deletion_email
+      mailer = UserCleanerMailer.deletion_email(user_de.email, user_de.locale)
       expect(mailer.subject).to include("gelöscht")
       expect(mailer.subject).not_to include("deleted")
 
-      mailer = UserCleanerMailer.with(user: user_en).deletion_email
+      mailer = UserCleanerMailer.deletion_email(user_en.email, user_en.locale)
       expect(mailer.subject).to include("deleted")
       expect(mailer.subject).not_to include("gelöscht")
     end
@@ -316,12 +322,13 @@ def test_subject_line(num_days)
       expected_de = "vollständig gelöscht"
       expected_en = "deleted entirely"
 
-      mailer = UserCleanerMailer.with(user: user_de).deletion_email
+      mailer = UserCleanerMailer.deletion_email(user_de.email, user_de.locale)
 
       expect(mailer.html_part.body).to include(expected_de)
       expect(mailer.html_part.body).not_to include(expected_en)
 
-      mailer = UserCleanerMailer.with(user: user_en).deletion_email
+      mailer = UserCleanerMailer.deletion_email(user_en.email, user_en.locale)
+
       expect(mailer.html_part.body).to include(expected_en)
       expect(mailer.html_part.body).not_to include(expected_de)
     end