From 102dd19be563f91addb4a546a9dac02cc97036b1 Mon Sep 17 00:00:00 2001 From: Splines Date: Sun, 1 Dec 2024 17:55:07 +0100 Subject: [PATCH 1/5] Fix unknown user variable after deletion --- .vscode/settings.json | 3 ++- app/mailers/user_cleaner_mailer.rb | 14 ++++++-------- app/models/user_cleaner.rb | 7 ++++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index bb386a29a..842ae0e3f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -107,6 +107,7 @@ "factorybot", "helpdesk", "katex", - "turbolinks" + "turbolinks", + "Unsets" ] } \ No newline at end of file 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..39d930890 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 From e0e066cfaa3aa03af5c7bafda072f49e407e7ef8 Mon Sep 17 00:00:00 2001 From: Splines Date: Sun, 1 Dec 2024 17:59:00 +0100 Subject: [PATCH 2/5] Adjust other occurrence of method --- app/models/user_cleaner.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/user_cleaner.rb b/app/models/user_cleaner.rb index 39d930890..7325a6182 100644 --- a/app/models/user_cleaner.rb +++ b/app/models/user_cleaner.rb @@ -134,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 From 6dd845c38ba4925732acef95403d1d438934b25a Mon Sep 17 00:00:00 2001 From: Splines Date: Sun, 1 Dec 2024 19:01:52 +0100 Subject: [PATCH 3/5] Update failing user cleaner jobs with new method signature --- spec/models/user_cleaner_spec.rb | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/spec/models/user_cleaner_spec.rb b/spec/models/user_cleaner_spec.rb index 1223a9048..5c8a2aa1e 100644 --- a/spec/models/user_cleaner_spec.rb +++ b/spec/models/user_cleaner_spec.rb @@ -196,8 +196,8 @@ def test_non_confirmed_user(confirmation_sent_date, expected_inactive_users_coun 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 { |args| expect(args).to include(40) }) end it "does not enqueue a deletion warning mail (40 days) for non-generic users" do @@ -217,8 +217,8 @@ def test_enqueues_additional_deletion_warning_mails(num_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 { |args| expect(args).to include(num_days) }) end it "enqueues additional deletion warning mails" do @@ -263,7 +263,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, 40, num_days) expect(mailer.subject).to include(num_days.to_s) end @@ -275,11 +276,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 +291,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 +308,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 +321,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 From cf87d8bb37efe244b2c2da12645a3ed3c1bee382 Mon Sep 17 00:00:00 2001 From: Splines Date: Sun, 1 Dec 2024 19:03:13 +0100 Subject: [PATCH 4/5] Expect user email for deletion mail --- spec/models/user_cleaner_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/models/user_cleaner_spec.rb b/spec/models/user_cleaner_spec.rb index 5c8a2aa1e..0ac9d5527 100644 --- a/spec/models/user_cleaner_spec.rb +++ b/spec/models/user_cleaner_spec.rb @@ -252,7 +252,8 @@ def test_enqueues_additional_deletion_warning_mails(num_days) 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 { |args| expect(args).to include(user.email) }) end end end From 8c747b730a92050015992d5821e66e204d7fd7d7 Mon Sep 17 00:00:00 2001 From: Splines Date: Sun, 1 Dec 2024 21:06:22 +0100 Subject: [PATCH 5/5] Fix `with` syntax --- spec/models/user_cleaner_spec.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/models/user_cleaner_spec.rb b/spec/models/user_cleaner_spec.rb index 0ac9d5527..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 { |args| expect(args).to include(40) }) + .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 { |args| expect(args).to include(num_days) }) + .with(user.email, user.locale, num_days)) end it "enqueues additional deletion warning mails" do @@ -248,12 +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) - .with { |args| expect(args).to include(user.email) }) + .with(user.email, user.locale)) end end end @@ -265,7 +265,7 @@ def test_enqueues_additional_deletion_warning_mails(num_days) def test_subject_line(num_days) user = FactoryBot.create(:confirmed_user) mailer = UserCleanerMailer - .pending_deletion_email(user.email, user.locale, 40, num_days) + .pending_deletion_email(user.email, user.locale, num_days) expect(mailer.subject).to include(num_days.to_s) end