REFACTOR: DRY up code and add better test coverage.

app/services/user_updater.rb

@@ -110,15 +110,15 @@ class UserUpdater
         update_muted_users(attributes[:muted_usernames])
       end
 
-      saved = (!save_options || user.user_option.save) && user_profile.save && user.save
+      if (saved = (!save_options || user.user_option.save) && user_profile.save && user.save) &&
+         (attributes[:name].present? && old_user_name.casecmp(attributes.fetch(:name)) != 0) ||
+         (attributes[:name].blank? && old_user_name.present?)
 
-      if saved
-        # log name changes
-        if attributes[:name].present? && old_user_name.downcase != attributes.fetch(:name).downcase
-          StaffActionLogger.new(@actor).log_name_change(user.id, old_user_name, attributes.fetch(:name))
-        elsif attributes[:name].blank? && old_user_name.present?
-          StaffActionLogger.new(@actor).log_name_change(user.id, old_user_name, "")
-        end
+        StaffActionLogger.new(@actor).log_name_change(
+          user.id,
+          old_user_name,
+          attributes.fetch(:name) { '' }
+        )
       end
     end
 

spec/services/user_updater_spec.rb

@@ -200,10 +200,25 @@ describe UserUpdater do
     it "logs the action" do
       user_without_name = Fabricate(:user, name: nil)
       user = Fabricate(:user, name: 'Billy Bob')
-      expect { UserUpdater.new(acting_user, user).update(name: 'Jim Tom') }.to change { UserHistory.count }.by(1)
-      expect { UserUpdater.new(acting_user, user).update(name: 'Jim Tom') }.to change { UserHistory.count }.by(0) # make sure it does not log a dupe
-      expect { UserUpdater.new(acting_user, user_without_name).update(bio_raw: 'foo bar') }.to change { UserHistory.count }.by(0) # make sure user without name (name = nil) does not raise an error
-      expect { UserUpdater.new(acting_user, user_without_name).update(name: 'Jim Tom') }.to change { UserHistory.count }.by(1)
+      expect do
+        UserUpdater.new(acting_user, user).update(name: 'Jim Tom')
+      end.to change { UserHistory.count }.by(1)
+
+      expect do
+        UserUpdater.new(acting_user, user).update(name: 'JiM TOm')
+      end.to_not change { UserHistory.count }
+
+      expect do
+        UserUpdater.new(acting_user, user_without_name).update(bio_raw: 'foo bar')
+      end.to_not change { UserHistory.count }
+
+      expect do
+        UserUpdater.new(acting_user, user_without_name).update(name: 'Jim Tom')
+      end.to change { UserHistory.count }.by(1)
+
+      expect do
+        UserUpdater.new(acting_user, user).update(name: '')
+      end.to change { UserHistory.count }.by(1)
     end
   end
 end