FIX: : trigger user_updated event only if email changed after user creation.

Follow-up to 1460d7957c

app/controllers/users_controller.rb

@@ -230,6 +230,7 @@ class UsersController < ApplicationController
     User.transaction do
       old_primary.update!(primary: false)
       new_primary.update!(primary: true)
+      DiscourseEvent.trigger(:user_updated, user)
 
       if current_user.staff? && current_user != user
         StaffActionLogger.new(current_user).log_update_email(user)
@@ -259,6 +260,7 @@ class UsersController < ApplicationController
     ActiveRecord::Base.transaction do
       if user_email
         user_email.destroy
+        DiscourseEvent.trigger(:user_updated, user)
       elsif
         user.email_change_requests.where(new_email: params[:email]).destroy_all
       end

app/models/user_email.rb

@@ -19,10 +19,6 @@ class UserEmail < ActiveRecord::Base
 
   scope :secondary, -> { where(primary: false) }
 
-  after_commit(on: [:create, :update, :destroy]) do
-    DiscourseEvent.trigger(:user_updated, user)
-  end
-
   private
 
   def strip_downcase_email

lib/email_updater.rb

@@ -121,6 +121,7 @@ class EmailUpdater
       @user.user_emails.create!(email: new_email)
     end
 
+    DiscourseEvent.trigger(:user_updated, @user)
     @user.set_automatic_groups
   end
 

spec/components/email_updater_spec.rb

@@ -140,10 +140,17 @@ describe EmailUpdater do
       context 'confirming a valid token' do
         it "updates the user's email" do
           Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :notify_old_email, to_address: old_email))
-          updater.confirm(@change_req.new_email_token.token)
+
+          event = DiscourseEvent.track_events {
+            updater.confirm(@change_req.new_email_token.token)
+          }.last
+
           expect(updater.errors).to be_blank
           expect(user.reload.email).to eq(new_email)
 
+          expect(event[:event_name]).to eq(:user_updated)
+          expect(event[:params].first).to eq(user)
+
           @change_req.reload
           expect(@change_req.change_state).to eq(EmailChangeRequest.states[:complete])
         end
@@ -162,10 +169,17 @@ describe EmailUpdater do
           expect(UserHistory.where(action: UserHistory.actions[:add_email], acting_user_id: user.id).last).to be_present
 
           Jobs.expects(:enqueue).once.with(:critical_user_email, has_entries(type: :notify_old_email_add, to_address: old_email))
-          updater.confirm(@change_req.new_email_token.token)
+
+          event = DiscourseEvent.track_events {
+            updater.confirm(@change_req.new_email_token.token)
+          }.last
+
           expect(updater.errors).to be_blank
           expect(UserEmail.where(user_id: user.id).pluck(:email)).to contain_exactly(user.email, new_email)
 
+          expect(event[:event_name]).to eq(:user_updated)
+          expect(event[:params].first).to eq(user)
+
           @change_req.reload
           expect(@change_req.change_state).to eq(EmailChangeRequest.states[:complete])
         end

spec/models/user_email_spec.rb

@@ -13,17 +13,10 @@ describe UserEmail do
     end
 
     it "allows multiple secondary emails" do
-      events = DiscourseEvent.track_events {
+      Fabricate(:secondary_email, user: user, primary: false)
-        Fabricate(:secondary_email, user: user, primary: false)
+      Fabricate(:secondary_email, user: user, primary: false)
-        Fabricate(:secondary_email, user: user, primary: false)
-      }
 
       expect(user.user_emails.count).to eq 3
-      expect(events.count).to eq 2
-
-      event = events.first
-      expect(event[:event_name]).to eq(:user_updated)
-      expect(event[:params].first).to eq(user)
     end
 
     it "does not allow an invalid email" do

spec/requests/users_controller_spec.rb

@@ -2652,11 +2652,17 @@ describe UsersController do
       expect(user_email.reload.primary).to eq(true)
       expect(other_email.reload.primary).to eq(false)
 
-      expect { put "/u/#{user.username}/preferences/primary-email.json", params: { email: other_email.email } }
+      event = DiscourseEvent.track_events {
-        .to change { UserHistory.where(action: UserHistory.actions[:update_email], acting_user_id: user.id).count }.by(1)
+        expect { put "/u/#{user.username}/preferences/primary-email.json", params: { email: other_email.email } }
+          .to change { UserHistory.where(action: UserHistory.actions[:update_email], acting_user_id: user.id).count }.by(1)
+      }.last
+
       expect(response.status).to eq(200)
       expect(user_email.reload.primary).to eq(false)
       expect(other_email.reload.primary).to eq(true)
+
+      expect(event[:event_name]).to eq(:user_updated)
+      expect(event[:params].first).to eq(user)
     end
   end
 
@@ -2676,10 +2682,16 @@ describe UsersController do
       expect(response.status).to eq(428)
       expect(user.reload.user_emails.pluck(:email)).to contain_exactly(user_email.email, other_email.email)
 
-      expect { delete "/u/#{user.username}/preferences/email.json", params: { email: other_email.email } }
+      event = DiscourseEvent.track_events {
-        .to change { UserHistory.where(action: UserHistory.actions[:destroy_email], acting_user_id: user.id).count }.by(1)
+        expect { delete "/u/#{user.username}/preferences/email.json", params: { email: other_email.email } }
+          .to change { UserHistory.where(action: UserHistory.actions[:destroy_email], acting_user_id: user.id).count }.by(1)
+      }.last
+
       expect(response.status).to eq(200)
       expect(user.reload.user_emails.pluck(:email)).to contain_exactly(user_email.email)
+
+      expect(event[:event_name]).to eq(:user_updated)
+      expect(event[:params].first).to eq(user)
     end
   end
 

spec/requests/users_email_controller_spec.rb

@@ -63,14 +63,9 @@ describe UsersEmailController do
       it 'confirms with a correct token' do
         user.user_stat.update_columns(bounce_score: 42, reset_bounce_score_after: 1.week.from_now)
 
-        event = DiscourseEvent.track_events {
+        put "/u/confirm-new-email", params: {
-          put "/u/confirm-new-email", params: {
+          token: "#{user.email_tokens.last.token}"
-            token: "#{user.email_tokens.last.token}"
+        }
-          }
-        }.last
-
-        expect(event[:event_name]).to eq(:user_updated)
-        expect(event[:params].first).to eq(user)
 
         expect(response.status).to eq(302)
         expect(response.redirect_url).to include("done")