From 8b60a1873545c125b258e4a1d433598e8617ebb9 Mon Sep 17 00:00:00 2001 From: rhymes Date: Thu, 13 Aug 2020 18:31:56 +0200 Subject: [PATCH] Account deletion email: serialize only user's details (#9752) * Account deletion email: serialize only user's details * Fix specs and mailer preview --- app/mailers/notify_mailer.rb | 5 ++--- app/workers/users/delete_worker.rb | 5 ++++- spec/mailers/notify_mailer_spec.rb | 2 +- spec/mailers/previews/notify_mailer_preview.rb | 3 ++- spec/workers/users/delete_worker_spec.rb | 15 +++++++++------ 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/app/mailers/notify_mailer.rb b/app/mailers/notify_mailer.rb index 96a1c8ae8..07e905eef 100644 --- a/app/mailers/notify_mailer.rb +++ b/app/mailers/notify_mailer.rb @@ -104,11 +104,10 @@ class NotifyMailer < ApplicationMailer end def account_deleted_email - user = params[:user] - @name = user.name + @name = params[:name] subject = "#{ApplicationConfig['COMMUNITY_NAME']} - Account Deletion Confirmation" - mail(to: user.email, subject: subject) + mail(to: params[:email], subject: subject) end def account_deletion_requested_email diff --git a/app/workers/users/delete_worker.rb b/app/workers/users/delete_worker.rb index 6d0c30e87..19c1ff939 100644 --- a/app/workers/users/delete_worker.rb +++ b/app/workers/users/delete_worker.rb @@ -11,7 +11,10 @@ module Users Users::Delete.call(user) return if admin_delete || user.email.blank? - NotifyMailer.with(user: user).account_deleted_email.deliver_now + # at this point the user object is already destroyed on the DB, + # thus we pass the data we need to render to deliver the email, not the + # whole object + NotifyMailer.with(name: user.name, email: user.email).account_deleted_email.deliver_now rescue StandardError => e DatadogStatsClient.count("users.delete", 1, tags: ["action:failed", "user_id:#{user.id}"]) Honeybadger.context({ user_id: user.id }) diff --git a/spec/mailers/notify_mailer_spec.rb b/spec/mailers/notify_mailer_spec.rb index 8cda27de4..97ffd1c67 100644 --- a/spec/mailers/notify_mailer_spec.rb +++ b/spec/mailers/notify_mailer_spec.rb @@ -373,7 +373,7 @@ RSpec.describe NotifyMailer, type: :mailer do end describe "#account_deleted_email" do - let(:email) { described_class.with(user: user).account_deleted_email } + let(:email) { described_class.with(name: user.name, email: user.email).account_deleted_email } it "renders proper subject" do expect(email.subject).to eq("#{ApplicationConfig['COMMUNITY_NAME']} - Account Deletion Confirmation") diff --git a/spec/mailers/previews/notify_mailer_preview.rb b/spec/mailers/previews/notify_mailer_preview.rb index 8318402f6..bec38380a 100644 --- a/spec/mailers/previews/notify_mailer_preview.rb +++ b/spec/mailers/previews/notify_mailer_preview.rb @@ -77,7 +77,8 @@ class NotifyMailerPreview < ActionMailer::Preview end def account_deleted_email - NotifyMailer.with(user: User.last).account_deleted_email + user = User.last + NotifyMailer.with(name: user.name, email: user.email).account_deleted_email end def export_email diff --git a/spec/workers/users/delete_worker_spec.rb b/spec/workers/users/delete_worker_spec.rb index 6cef8c61c..08b294329 100644 --- a/spec/workers/users/delete_worker_spec.rb +++ b/spec/workers/users/delete_worker_spec.rb @@ -7,15 +7,18 @@ RSpec.describe Users::DeleteWorker, type: :worker do let(:message_delivery) { double } describe "#perform" do - let(:user) { create(:user) } + let!(:user) { create(:user) } let(:delete) { Users::Delete } - before do - allow(delete).to receive(:call) - end - context "when user is found" do + it "deletes the user correctly" do + worker.perform(user.id) + + expect(User.exists?(id: user.id)).to be(false) + end + it "calls the service when a user is found" do + allow(delete).to receive(:call) worker.perform(user.id) expect(delete).to have_received(:call).with(user) end @@ -39,7 +42,7 @@ RSpec.describe Users::DeleteWorker, type: :worker do worker.perform(user.id) - expect(mailer_class).to have_received(:with).with(user: user) + expect(mailer_class).to have_received(:with).with(name: user.name, email: user.email) expect(mailer).to have_received(:account_deleted_email) expect(message_delivery).to have_received(:deliver_now) end