From b32d3d66e5d846d4728ed12580f58d00f490e1c0 Mon Sep 17 00:00:00 2001
From: Arpit Jalan <arpit@techapj.com>
Date: Thu, 23 Feb 2017 11:18:57 +0530
Subject: [PATCH] FEATURE: log all username and name changes

---
 app/models/user_history.rb             |  3 ++-
 app/services/staff_action_logger.rb    | 10 ++++++++++
 app/services/user_updater.rb           | 10 +++++++++-
 app/services/username_changer.rb       |  2 +-
 spec/services/user_updater_spec.rb     |  5 +++++
 spec/services/username_changer_spec.rb |  4 ++++
 6 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/app/models/user_history.rb b/app/models/user_history.rb
index ee667213c3c..a6677de389d 100644
--- a/app/models/user_history.rb
+++ b/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.
diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb
index 2bd24889fe7..796b8b36798 100644
--- a/app/services/staff_action_logger.rb
+++ b/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({
diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb
index 762a0cd0b91..d3b9f680bf3 100644
--- a/app/services/user_updater.rb
+++ b/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
diff --git a/app/services/username_changer.rb b/app/services/username_changer.rb
index 00a1df62b47..0e35727db9e 100644
--- a/app/services/username_changer.rb
+++ b/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
 
diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb
index 0a1b538ab4a..b494d59d804 100644
--- a/spec/services/user_updater_spec.rb
+++ b/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
diff --git a/spec/services/username_changer_spec.rb b/spec/services/username_changer_spec.rb
index cf79494f261..b89ae46c793 100644
--- a/spec/services/username_changer_spec.rb
+++ b/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