diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 796b8b36798..71734ab2e20 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -146,7 +146,7 @@ class StaffActionLogger def log_site_text_change(subject, new_text=nil, old_text=nil, opts={}) raise Discourse::InvalidParameters.new(:subject) unless subject.present? - UserHistory.create( params(opts).merge({ + UserHistory.create!( params(opts).merge({ action: UserHistory.actions[:change_site_text], subject: subject, previous_value: old_text, diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index d3b9f680bf3..4d1f6f44984 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -53,6 +53,9 @@ 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 } + old_user_name = user.name.present? ? user.name : "" + 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,20 +97,21 @@ 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 saved = (!save_options || user.user_option.save) && user_profile.save && user.save - DiscourseEvent.trigger(:user_updated, user) if saved + if saved + DiscourseEvent.trigger(:user_updated, user) + + # 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 + end saved end end diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index 544ca55c759..3305f67842d 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -192,9 +192,12 @@ describe UserUpdater do end 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) end end end