Merge pull request #4727 from techAPJ/log-name-changes

FEATURE: log all username and name changes

app/models/user_history.rb

@@ -62,7 +62,8 @@ class UserHistory < ActiveRecord::Base
                           change_readonly_mode: 44,
                           backup_download: 45,
                           backup_destroy: 46,
-                          notified_about_get_a_room: 47)
+                          notified_about_get_a_room: 47,
+                          change_name: 48)
   end
 
   # Staff actions is a subset of all actions, used to audit actions taken by staff users.

app/services/staff_action_logger.rb

@@ -164,6 +164,16 @@ class StaffActionLogger
     }))
   end
 
+  def log_name_change(user_id, old_name, new_name, opts={})
+    raise Discourse::InvalidParameters.new(:user) unless user_id
+    UserHistory.create( params(opts).merge({
+      action: UserHistory.actions[:change_name],
+      target_user_id: user_id,
+      previous_value: old_name,
+      new_value: new_name
+    }))
+  end
+
   def log_user_suspend(user, reason, opts={})
     raise Discourse::InvalidParameters.new(:user) unless user
     UserHistory.create( params(opts).merge({

app/services/user_updater.rb

@@ -39,6 +39,7 @@ class UserUpdater
   def initialize(actor, user)
     @user = user
     @guardian = Guardian.new(actor)
+    @actor = actor
   end
 
   def update(attributes = {})
@@ -52,7 +53,6 @@ class UserUpdater
     user_profile.profile_background = attributes.fetch(:profile_background) { user_profile.profile_background }
     user_profile.card_background = attributes.fetch(:card_background) { user_profile.card_background }
 
-    user.name = attributes.fetch(:name) { user.name }
     user.locale = attributes.fetch(:locale) { user.locale }
     user.date_of_birth = attributes.fetch(:date_of_birth) { user.date_of_birth }
 
@@ -94,6 +94,14 @@ class UserUpdater
     end
 
     User.transaction do
+      # log name changes
+      if attributes[:name].present? && user.name.downcase != attributes.fetch(:name).downcase
+        StaffActionLogger.new(@actor).log_name_change(user.id, user.name, attributes.fetch(:name))
+      elsif attributes[:name].blank? && user.name.present?
+        StaffActionLogger.new(@actor).log_name_change(user.id, user.name, "")
+      end
+      user.name = attributes.fetch(:name) { user.name }
+
       if attributes.key?(:muted_usernames)
         update_muted_users(attributes[:muted_usernames])
       end

app/services/username_changer.rb

@@ -11,7 +11,7 @@ class UsernameChanger
   end
 
   def change
-    if @actor && @actor != @user
+    if @actor
       StaffActionLogger.new(@actor).log_username_change(@user, @user.username, @new_username)
     end
 

spec/services/user_updater_spec.rb

@@ -190,5 +190,10 @@ describe UserUpdater do
         expect(user.reload.custom_fields).to eq({'import_username' => 'my_old_username'})
       end
     end
+
+    it "logs the action" do
+      user = Fabricate(:user, name: 'Billy Bob')
+      expect { described_class.new(acting_user, user).update(name: 'Jim Tom') }.to change { UserHistory.count }.by(1)
+    end
   end
 end

spec/services/username_changer_spec.rb

@@ -62,6 +62,10 @@ describe UsernameChanger do
         described_class.change(myself, "HanSolo")
         expect(myself.reload.username).to eq('HanSolo')
       end
+
+      it "logs the action" do
+        expect { described_class.change(myself, "HanSolo", myself) }.to change { UserHistory.count }.by(1)
+      end
     end
 
     describe 'allow custom minimum username length from site settings' do