Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unknown user variable after deletion #717

Merged
merged 5 commits into from
Dec 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
"factorybot",
"helpdesk",
"katex",
"turbolinks"
"turbolinks",
"Unsets"
]
}
14 changes: 6 additions & 8 deletions app/mailers/user_cleaner_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 7 additions & 6 deletions app/models/user_cleaner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
41 changes: 24 additions & 17 deletions spec/models/user_cleaner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down