diff --git a/.vscode/settings.json b/.vscode/settings.json index 4b4d1cfa7..40c9d2aeb 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -122,6 +122,7 @@ "selectize", "Timecop", "turbolinks", + "Unsets", "uncached", "whitespaces" ], diff --git a/app/mailers/user_cleaner_mailer.rb b/app/mailers/user_cleaner_mailer.rb index f8535ccc2..0ee8a6931 100644 --- a/app/mailers/user_cleaner_mailer.rb +++ b/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 diff --git a/app/models/user_cleaner.rb b/app/models/user_cleaner.rb index 566d0c8cd..7325a6182 100644 --- a/app/models/user_cleaner.rb +++ b/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 diff --git a/app/workers/user_cleaner_job.rb b/app/workers/user_cleaner_job.rb index 184a21f5d..d87fb2ee7 100644 --- a/app/workers/user_cleaner_job.rb +++ b/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. diff --git a/spec/models/user_cleaner_spec.rb b/spec/models/user_cleaner_spec.rb index 1223a9048..eb00e07ad 100644 --- a/spec/models/user_cleaner_spec.rb +++ b/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