[issue-4351] add logic to notify old user email of email change (#4374)

* [issue-4351] add logic to notify old user email of email change

* [issue-4351] hound cleanups

* [issue-4351] move UserInfoChangedMailer call outside super to prevent race condition

* [issue-4351] edit test case to reflect old mail sent

* [issue-4351] add test cases to both UserInfoChangedMailerWorker and UserInfoChangedMailer

* [issue-4351] hound cleanups

* [issue-4351] hound cleanups

* [issue-4351] renaming test variables

app/controllers/api/v1/users_controller.rb

@@ -47,18 +47,15 @@ def me
   end
 
   def update
-    [].tap do |update_email_user_ids|
-
-      super do |user|
-        if user.email_changed?
-          update_email_user_ids << user.id
-          user.update_attribute(:valid_email, true)
-        end
-      end
+    prev_email = user.email
+    email_changed = false
+    super do |user|
+      email_changed = user.email_changed?
+    end
 
-      update_email_user_ids.each do |user_id|
-        UserInfoChangedMailerWorker.perform_async(user_id, "email")
-      end
+    if email_changed
+      user.update(valid_email: true)
+      UserInfoChangedMailerWorker.perform_async(user.id, 'email', prev_email)
     end
   end
 

app/mailers/user_info_changed_mailer.rb

@@ -1,19 +1,20 @@
 class UserInfoChangedMailer < ApplicationMailer
   layout false
 
-  def user_info_changed(user, info)
+  def user_info_changed(user, info, previous_email=nil)
     @user = user
-    @email_to = user.email
+    @recipient_emails = [user.email]
 
     case info
     when "email"
       subject = "Your Zooniverse email address has been changed"
       template = "email_changed"
+      @recipient_emails << previous_email if previous_email
     when "password"
       subject = "Your Zooniverse password has been changed"
       template = "password_changed"
     end
 
-    mail(to: @email_to, subject: subject, :template_name => template )
+    mail(to: @recipient_emails, subject: subject, template_name: template)
   end
 end

app/workers/user_info_changed_mailer_worker.rb

@@ -5,10 +5,10 @@ class UserInfoChangedMailerWorker
 
   attr_reader :user, :project
 
-  def perform(user_id, info)
+  def perform(user_id, info, previous_email=nil)
     @user = User.find(user_id)
     if @user && @user.receives_email?
-      UserInfoChangedMailer.user_info_changed(user, info).deliver
+      UserInfoChangedMailer.user_info_changed(user, info, previous_email).deliver
     end
   rescue ActiveRecord::RecordNotFound
     nil

spec/controllers/api/v1/users_controller_spec.rb

@@ -473,13 +473,14 @@ def update_request
 
     context "when changing email" do
       let(:put_operations) { {users: {email: "[email protected]"}} }
+      let!(:user_email) { user.email }
 
       after(:each) do
         update_request
       end
 
-      it "sends an email to the new address if user is valid" do
-        expect(UserInfoChangedMailerWorker).to receive(:perform_async).with(user.id, "email")
+      it "sends an email to the new and old address if user is valid" do
+        expect(UserInfoChangedMailerWorker).to receive(:perform_async).with(user.id, "email", user_email)
       end
 
       it "sets valid_email parameter to true" do

spec/mailers/user_info_changed_mailer_spec.rb

@@ -21,10 +21,16 @@
     end
 
     context "email address was changed" do
-      let(:mail) { UserInfoChangedMailer.user_info_changed(user, "email")}
+      prev_email = '[email protected]'
+      let(:mail) { UserInfoChangedMailer.user_info_changed(user, 'email', prev_email)}
+
       it 'should have the correct subject' do
         expect(mail.subject).to eq("Your Zooniverse email address has been changed")
       end
+
+      it 'notifies the previous mail' do
+        expect(mail.to).to include(prev_email)
+      end
     end
   end
 end

spec/workers/user_info_changed_mailer_worker_spec.rb

@@ -3,9 +3,18 @@
 RSpec.describe UserInfoChangedMailerWorker do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
+  let(:prev_email) { '[email protected]' }
 
-  it 'should deliver the mail' do
-    expect{ subject.perform(user.id, "password") }.to change{ ActionMailer::Base.deliveries.count }.by(1)
+  context 'when delivering an email' do
+    it 'delivers the mail' do
+      expect { subject.perform(user.id, 'password') }.to change { ActionMailer::Base.deliveries.count }.by(1)
+    end
+
+    it 'delivers to the right recipients' do
+      subject.perform(user.id, 'email', prev_email)
+      mail = ActionMailer::Base.deliveries.last
+      expect(mail.to).to eq([user.email, prev_email])
+    end
   end
 
   context "without a user" do