FEATURE: staff action logs when creating/updating/deleting badges

app/assets/javascripts/admin/models/staff-action-log.js.es6

@@ -1,54 +1,58 @@
-import { ajax } from 'discourse/lib/ajax';
-import AdminUser from 'admin/models/admin-user';
-import { escapeExpression } from 'discourse/lib/utilities';
+import computed from "ember-addons/ember-computed-decorators";
+import { ajax } from "discourse/lib/ajax";
+import AdminUser from "admin/models/admin-user";
+import { escapeExpression } from "discourse/lib/utilities";
+
+function format(label, value, escape = true) {
+  return value ? `<b>${I18n.t(label)}</b>: ${escape ? escapeExpression(value) : value}` : "";
+};
 
 const StaffActionLog = Discourse.Model.extend({
   showFullDetails: false,
 
-  actionName: function() {
-    return I18n.t("admin.logs.staff_actions.actions." + this.get('action_name'));
-  }.property('action_name'),
-
-  formattedDetails: function() {
-    let formatted = "";
-    formatted += this.format('email', 'email');
-    formatted += this.format('admin.logs.ip_address', 'ip_address');
-    formatted += this.format('admin.logs.topic_id', 'topic_id');
-    formatted += this.format('admin.logs.post_id', 'post_id');
-    formatted += this.format('admin.logs.category_id', 'category_id');
-    if (!this.get('useCustomModalForDetails')) {
-      formatted += this.format('admin.logs.staff_actions.new_value', 'new_value');
-      formatted += this.format('admin.logs.staff_actions.previous_value', 'previous_value');
-    }
-    if (!this.get('useModalForDetails')) {
-      if (this.get('details')) formatted += escapeExpression(this.get('details')) + '<br/>';
-    }
-    return formatted;
-  }.property('ip_address', 'email', 'topic_id', 'post_id', 'category_id'),
-
-  format(label, propertyName) {
-    if (this.get(propertyName)) {
-      let value = escapeExpression(this.get(propertyName));
-      if (propertyName === 'post_id') {
-        value = `<a href data-link-post-id="${value}">${value}</a>`;
-      }
-      return `<b>${I18n.t(label)}:</b> ${value}<br/>`;
-    } else {
-      return '';
-    }
+  @computed("action_name")
+  actionName(actionName) {
+    return I18n.t(`admin.logs.staff_actions.actions.${actionName}`);
   },
 
-  useModalForDetails: function() {
-    return (this.get('details') && this.get('details').length > 100);
-  }.property('action_name'),
+  @computed("email", "ip_address", "topic_id", "post_id", "category_id", "new_value", "previous_value", "details", "useCustomModalForDetails", "useModalForDetails")
+  formattedDetails(email, ipAddress, topicId, postId, categoryId, newValue, previousValue, details, useCustomModalForDetails, useModalForDetails) {
+    const postLink = postId ? `<a href data-link-post-id="${postId}">${postId}</a>` : null;
 
-  useCustomModalForDetails: function() {
-    return _.contains(['change_theme', 'delete_theme'], this.get('action_name'));
-  }.property('action_name')
+    let lines = [
+      format("email", email),
+      format("admin.logs.ip_address", ipAddress),
+      format("admin.logs.topic_id", topicId),
+      format("admin.logs.post_id", postLink, false),
+      format("admin.logs.category_id", categoryId),
+    ];
+
+    if (!useCustomModalForDetails) {
+      lines.push(format("admin.logs.staff_actions.new_value", newValue));
+      lines.push(format("admin.logs.staff_actions.previous_value", previousValue));
+    }
+
+    if (!useModalForDetails && details) {
+      lines = [...lines, ...escapeExpression(details).split("\n")];
+    }
+
+    const formatted = lines.filter(l => l.length > 0).join("<br/>");
+    return formatted.length > 0 ? formatted + "<br/>" : "";
+  },
+
+  @computed("details")
+  useModalForDetails(details) {
+    return details && details.length > 100;
+  },
+
+  @computed("action_name")
+  useCustomModalForDetails(actionName) {
+    return ["change_theme", "delete_theme"].includes(actionName);
+  }
 });
 
 StaffActionLog.reopenClass({
-  create: function(attrs) {
+  create(attrs) {
     attrs = attrs || {};
 
     if (attrs.acting_user) {
@@ -60,13 +64,11 @@ StaffActionLog.reopenClass({
     return this._super(attrs);
   },
 
-  findAll: function(filters) {
-    return ajax("/admin/logs/staff_action_logs.json", { data: filters }).then((data) => {
+  findAll(data) {
+    return ajax("/admin/logs/staff_action_logs.json", { data }).then(result => {
       return {
-        staff_action_logs: data.staff_action_logs.map(function(s) {
-          return StaffActionLog.create(s);
-        }),
-        user_history_actions: data.user_history_actions
+        staff_action_logs: result.staff_action_logs.map(s => StaffActionLog.create(s)),
+        user_history_actions: result.user_history_actions
       };
     });
   }

app/controllers/admin/badges_controller.rb

@@ -15,10 +15,8 @@ class Admin::BadgesController < Admin::AdminController
   end
 
   def preview
-
     unless SiteSetting.enable_badge_sql
-      render json: "preview not allowed", status: 403
-      return
+      return render json: "preview not allowed", status: 403
     end
 
     render json: BadgeGranter.preview(params[:sql],
@@ -39,13 +37,12 @@ class Admin::BadgesController < Admin::AdminController
   end
 
   def save_badge_groupings
-
     badge_groupings = BadgeGrouping.all.order(:position).to_a
     ids = params[:ids].map(&:to_i)
 
     params[:names].each_with_index do |name, index|
       id = ids[index].to_i
-      group = badge_groupings.find { |b| b.id == id } || BadgeGrouping.new()
+      group = badge_groupings.find { |b| b.id == id } || BadgeGrouping.new
       group.name = name
       group.position = index
       group.save
@@ -66,24 +63,27 @@ class Admin::BadgesController < Admin::AdminController
     if errors.present?
       render_json_error errors
     else
+      StaffActionLogger.new(current_user).log_badge_creation(badge)
       render_serialized(badge, AdminBadgeSerializer, root: "badge")
     end
   end
 
   def update
     badge = find_badge
-
     errors = update_badge_from_params(badge)
 
     if errors.present?
       render_json_error errors
     else
+      StaffActionLogger.new(current_user).log_badge_change(badge)
       render_serialized(badge, AdminBadgeSerializer, root: "badge")
     end
   end
 
   def destroy
-    find_badge.destroy
+    badge = find_badge
+    StaffActionLogger.new(current_user).log_badge_deletion(badge)
+    badge.destroy
     render body: nil
   end
 

app/models/user_history.rb

@@ -72,7 +72,10 @@ class UserHistory < ActiveRecord::Base
       post_edit: 53,
       topic_published: 54,
       recover_topic: 55,
-      post_approved: 56
+      post_approved: 56,
+      create_badge: 57,
+      change_badge: 58,
+      delete_badge: 59,
     )
   end
 
@@ -123,7 +126,10 @@ class UserHistory < ActiveRecord::Base
       :post_edit,
       :topic_published,
       :recover_topic,
-      :post_approved
+      :post_approved,
+      :create_badge,
+      :change_badge,
+      :delete_badge,
     ]
   end
 

app/services/staff_action_logger.rb

@@ -12,11 +12,16 @@ class StaffActionLogger
     raise Discourse::InvalidParameters.new(:admin) unless @admin && @admin.is_a?(User)
   end
 
+  USER_FIELDS ||= %i{id username name created_at trust_level last_seen_at last_emailed_at}
+
   def log_user_deletion(deleted_user, opts = {})
     raise Discourse::InvalidParameters.new(:deleted_user) unless deleted_user && deleted_user.is_a?(User)
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:delete_user],
-                                          ip_address: deleted_user.ip_address.to_s,
-                                          details: [:id, :username, :name, :created_at, :trust_level, :last_seen_at, :last_emailed_at].map { |x| "#{x}: #{deleted_user.send(x)}" }.join("\n")))
+    details = USER_FIELDS.map { |x| "#{x}: #{deleted_user.send(x)}" }.join("\n")
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:delete_user],
+      ip_address: deleted_user.ip_address.to_s,
+      details: details
+    ))
   end
 
   def log_custom(custom_type, details = nil)
@@ -33,7 +38,7 @@ class StaffActionLogger
     attrs[:action] = UserHistory.actions[:custom_staff]
     attrs[:custom_type] = custom_type
 
-    UserHistory.create(attrs)
+    UserHistory.create!(attrs)
   end
 
   def log_post_deletion(deleted_post, opts = {})
@@ -54,9 +59,11 @@ class StaffActionLogger
       "raw: #{deleted_post.raw}"
     ]
 
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:delete_post],
-                                          post_id: deleted_post.id,
-                                          details: details.join("\n")))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:delete_post],
+      post_id: deleted_post.id,
+      details: details.join("\n")
+    ))
   end
 
   def log_topic_delete_recover(topic, action = "delete_topic", opts = {})
@@ -75,24 +82,31 @@ class StaffActionLogger
       details << "raw: #{first_post.raw}"
     end
 
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[action.to_sym],
-                                          topic_id: topic.id,
-                                          details: details.join("\n")))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[action.to_sym],
+      topic_id: topic.id,
+      details: details.join("\n")
+    ))
   end
 
   def log_trust_level_change(user, old_trust_level, new_trust_level, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user && user.is_a?(User)
     raise Discourse::InvalidParameters.new(:old_trust_level) unless TrustLevel.valid? old_trust_level
     raise Discourse::InvalidParameters.new(:new_trust_level) unless TrustLevel.valid? new_trust_level
-    UserHistory.create!(params(opts).merge(action: UserHistory.actions[:change_trust_level],
-                                           target_user_id: user.id,
-                                           details: "old trust level: #{old_trust_level}\nnew trust level: #{new_trust_level}"))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:change_trust_level],
+      target_user_id: user.id,
+      details: "old trust level: #{old_trust_level}\nnew trust level: #{new_trust_level}"
+    ))
   end
 
   def log_lock_trust_level(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user && user.is_a?(User)
-    UserHistory.create!(params(opts).merge(action: UserHistory.actions[user.manual_locked_trust_level.nil? ? :unlock_trust_level : :lock_trust_level],
-                                           target_user_id: user.id))
+    action = UserHistory.actions[user.manual_locked_trust_level.nil? ? :unlock_trust_level : :lock_trust_level]
+    UserHistory.create!(params(opts).merge(
+      action: action,
+      target_user_id: user.id
+    ))
   end
 
   def log_topic_published(topic, opts = {})
@@ -122,10 +136,12 @@ class StaffActionLogger
 
   def log_site_setting_change(setting_name, previous_value, new_value, opts = {})
     raise Discourse::InvalidParameters.new(:setting_name) unless setting_name.present? && SiteSetting.respond_to?(setting_name)
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:change_site_setting],
-                                          subject: setting_name,
-                                          previous_value: previous_value,
-                                          new_value: new_value))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:change_site_setting],
+      subject: setting_name,
+      previous_value: previous_value,
+      new_value: new_value
+    ))
   end
 
   def theme_json(theme)
@@ -157,41 +173,51 @@ class StaffActionLogger
 
     old_json, new_json = strip_duplicates(old_json, new_json)
 
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:change_theme],
-                                          subject: new_theme.name,
-                                          previous_value: old_json,
-                                          new_value: new_json))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:change_theme],
+      subject: new_theme.name,
+      previous_value: old_json,
+      new_value: new_json
+    ))
   end
 
   def log_theme_destroy(theme, opts = {})
     raise Discourse::InvalidParameters.new(:theme) unless theme
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:delete_theme],
-                                          subject: theme.name,
-                                          previous_value: theme_json(theme)))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:delete_theme],
+      subject: theme.name,
+      previous_value: theme_json(theme)
+    ))
   end
 
   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(action: UserHistory.actions[:change_site_text],
-                                           subject: subject,
-                                           previous_value: old_text,
-                                           new_value: new_text))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:change_site_text],
+      subject: subject,
+      previous_value: old_text,
+      new_value: new_text
+    ))
   end
 
   def log_username_change(user, old_username, new_username, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:change_username],
-                                          target_user_id: user.id,
-                                          previous_value: old_username,
-                                          new_value: new_username))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:change_username],
+      target_user_id: user.id,
+      previous_value: old_username,
+      new_value: new_username
+    ))
   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))
+    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 = {})
@@ -205,50 +231,96 @@ class StaffActionLogger
       details: details
     )
     args[:post_id] = opts[:post_id] if opts[:post_id]
-    UserHistory.create(args)
+    UserHistory.create!(args)
   end
 
   def log_user_unsuspend(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:unsuspend_user],
-                                          target_user_id: user.id))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:unsuspend_user],
+      target_user_id: user.id
+    ))
+  end
+
+  BADGE_FIELDS ||= %i{id name description long_description icon image badge_type_id
+    badge_grouping_id query allow_title multiple_grant listable target_posts
+    enabled auto_revoke show_posts system}
+
+  def log_badge_creation(badge)
+    raise Discourse::InvalidParameters.new(:badge) unless badge
+    details = BADGE_FIELDS.map { |f| [f, badge.send(f)] }.select { |f, v| v.present? }.map { |f, v| "#{f}: #{v}" }
+    UserHistory.create!(params.merge(
+      action: UserHistory.actions[:create_badge],
+      details: details.join("\n")
+    ))
+  end
+
+  def log_badge_change(badge)
+    raise Discourse::InvalidParameters.new(:badge) unless badge
+    details = ["id: #{badge.id}"]
+    badge.previous_changes.each { |f, values| details << "#{f}: #{values[1]}" if BADGE_FIELDS.include?(f.to_sym) }
+    UserHistory.create!(params.merge(
+      action: UserHistory.actions[:change_badge],
+      details: details.join("\n")
+    ))
+  end
+
+  def log_badge_deletion(badge)
+    raise Discourse::InvalidParameters.new(:badge) unless badge
+    details = BADGE_FIELDS.map { |f| [f, badge.send(f)] }.select { |f, v| v.present? }.map { |f, v| "#{f}: #{v}" }
+    UserHistory.create!(params.merge(
+      action: UserHistory.actions[:delete_badge],
+      details: details.join("\n")
+    ))
   end
 
   def log_badge_grant(user_badge, opts = {})
     raise Discourse::InvalidParameters.new(:user_badge) unless user_badge
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:grant_badge],
-                                          target_user_id: user_badge.user_id,
-                                          details: user_badge.badge.name))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:grant_badge],
+      target_user_id: user_badge.user_id,
+      details: user_badge.badge.name
+    ))
   end
 
   def log_badge_revoke(user_badge, opts = {})
     raise Discourse::InvalidParameters.new(:user_badge) unless user_badge
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:revoke_badge],
-                                          target_user_id: user_badge.user_id,
-                                          details: user_badge.badge.name))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:revoke_badge],
+      target_user_id: user_badge.user_id,
+      details: user_badge.badge.name
+    ))
   end
 
   def log_check_email(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:check_email],
-                                          target_user_id: user.id))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:check_email],
+      target_user_id: user.id
+    ))
   end
 
   def log_show_emails(users, opts = {})
     return if users.blank?
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:check_email],
-                                          details: users.map { |u| "[#{u.id}] #{u.username}" }.join("\n")))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:check_email],
+      details: users.map { |u| "[#{u.id}] #{u.username}" }.join("\n")
+    ))
   end
 
   def log_impersonate(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:impersonate],
-                                          target_user_id: user.id))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:impersonate],
+      target_user_id: user.id
+    ))
   end
 
   def log_roll_up(subnets, opts = {})
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:roll_up],
-                                          details: subnets.join(", ")))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:roll_up],
+      details: subnets.join(", ")
+    ))
   end
 
   def log_category_settings_change(category, category_params, old_permissions = nil)
@@ -261,12 +333,14 @@ class StaffActionLogger
     end
 
     changed_attributes.each do |key, value|
-      UserHistory.create(params.merge(action: UserHistory.actions[:change_category_settings],
-                                      category_id: category.id,
-                                      context: category.url,
-                                      subject: key,
-                                      previous_value: value[0],
-                                      new_value: value[1]))
+      UserHistory.create!(params.merge(
+        action: UserHistory.actions[:change_category_settings],
+        category_id: category.id,
+        context: category.url,
+        subject: key,
+        previous_value: value[0],
+        new_value: value[1]
+      ))
     end
   end
 
@@ -283,10 +357,12 @@ class StaffActionLogger
       details << "parent_category: #{parent_category.name}"
     end
 
-    UserHistory.create(params.merge(action: UserHistory.actions[:delete_category],
-                                    category_id: category.id,
-                                    details: details.join("\n"),
-                                    context: category.url))
+    UserHistory.create!(params.merge(
+      action: UserHistory.actions[:delete_category],
+      category_id: category.id,
+      details: details.join("\n"),
+      context: category.url
+    ))
   end
 
   def log_category_creation(category)
@@ -297,10 +373,12 @@ class StaffActionLogger
       "name: #{category.name}"
     ]
 
-    UserHistory.create(params.merge(action: UserHistory.actions[:create_category],
-                                    details: details.join("\n"),
-                                    category_id: category.id,
-                                    context: category.url))
+    UserHistory.create!(params.merge(
+      action: UserHistory.actions[:create_category],
+      details: details.join("\n"),
+      category_id: category.id,
+      context: category.url
+    ))
   end
 
   def log_silence_user(user, opts = {})
@@ -313,109 +391,139 @@ class StaffActionLogger
     )
     create_args[:post_id] = opts[:post_id] if opts[:post_id]
 
-    UserHistory.create(create_args)
+    UserHistory.create!(create_args)
   end
 
   def log_unsilence_user(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:unsilence_user],
-                                          target_user_id: user.id))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:unsilence_user],
+      target_user_id: user.id
+    ))
   end
 
   def log_disable_second_factor_auth(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:disabled_second_factor],
-                                          target_user_id: user.id))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:disabled_second_factor],
+      target_user_id: user.id
+    ))
   end
 
   def log_grant_admin(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:grant_admin],
-                                          target_user_id: user.id))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:grant_admin],
+      target_user_id: user.id
+    ))
   end
 
   def log_revoke_admin(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:revoke_admin],
-                                          target_user_id: user.id))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:revoke_admin],
+      target_user_id: user.id
+    ))
   end
 
   def log_grant_moderation(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:grant_moderation],
-                                          target_user_id: user.id))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:grant_moderation],
+      target_user_id: user.id
+    ))
   end
 
   def log_revoke_moderation(user, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:revoke_moderation],
-                                          target_user_id: user.id))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:revoke_moderation],
+      target_user_id: user.id
+    ))
   end
 
   def log_backup_create(opts = {})
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:backup_create],
-                                          ip_address: @admin.ip_address.to_s))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:backup_create],
+      ip_address: @admin.ip_address.to_s
+    ))
   end
 
   def log_backup_download(backup, opts = {})
     raise Discourse::InvalidParameters.new(:backup) unless backup
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:backup_download],
-                                          ip_address: @admin.ip_address.to_s,
-                                          details: backup.filename))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:backup_download],
+      ip_address: @admin.ip_address.to_s,
+      details: backup.filename
+    ))
   end
 
   def log_backup_destroy(backup, opts = {})
     raise Discourse::InvalidParameters.new(:backup) unless backup
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:backup_destroy],
-                                          ip_address: @admin.ip_address.to_s,
-                                          details: backup.filename))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:backup_destroy],
+      ip_address: @admin.ip_address.to_s,
+      details: backup.filename
+    ))
   end
 
   def log_revoke_email(user, reason, opts = {})
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:revoke_email],
-                                          target_user_id: user.id,
-                                          details: reason))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:revoke_email],
+      target_user_id: user.id,
+      details: reason
+    ))
   end
 
   def log_user_deactivate(user, reason, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:deactivate_user],
-                                          target_user_id: user.id,
-                                          details: reason))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:deactivate_user],
+      target_user_id: user.id,
+      details: reason
+    ))
   end
 
   def log_user_activate(user, reason, opts = {})
     raise Discourse::InvalidParameters.new(:user) unless user
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:activate_user],
-                                          target_user_id: user.id,
-                                          details: reason))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:activate_user],
+      target_user_id: user.id,
+      details: reason
+    ))
   end
 
   def log_wizard_step(step, opts = {})
     raise Discourse::InvalidParameters.new(:step) unless step
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:wizard_step],
-                                          context: step.id))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:wizard_step],
+      context: step.id
+    ))
   end
 
   def log_change_readonly_mode(state)
-    UserHistory.create(params.merge(action: UserHistory.actions[:change_readonly_mode],
-                                    previous_value: !state,
-                                    new_value: state))
+    UserHistory.create!(params.merge(
+      action: UserHistory.actions[:change_readonly_mode],
+      previous_value: !state,
+      new_value: state
+    ))
   end
 
   def log_check_personal_message(topic, opts = {})
     raise Discourse::InvalidParameters.new(:topic) unless topic && topic.is_a?(Topic)
-    UserHistory.create(params(opts).merge(action: UserHistory.actions[:check_personal_message],
-                                          topic_id: topic.id,
-                                          context: topic.relative_url))
+    UserHistory.create!(params(opts).merge(
+      action: UserHistory.actions[:check_personal_message],
+      topic_id: topic.id,
+      context: topic.relative_url
+    ))
   end
 
   def log_post_approved(post, opts = {})
     raise Discourse::InvalidParameters.new(:post) unless post && post.is_a?(Post)
     UserHistory.create!(params(opts).merge(
       action: UserHistory.actions[:post_approved],
-      post_id: post.id)
-    )
+      post_id: post.id
+    ))
   end
 
   private

config/locales/client.en.yml

@@ -3401,6 +3401,9 @@ en:
             disabled_second_factor: "disable Two Factor Authentication"
             topic_published: "topic published"
             post_approved: "post approved"
+            create_badge: "create badge"
+            change_badge: "change badge"
+            delete_badge: "delete badge"
         screened_emails:
           title: "Screened Emails"
           description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed."

spec/controllers/admin/badges_controller_spec.rb

@@ -48,6 +48,8 @@ describe Admin::BadgesController do
         expect(response.status).to eq(200)
         expect(json["badge"]["name"]).to eq('test')
         expect(json["badge"]["query"]).to eq('select 1 as user_id, null as granted_at')
+
+        expect(UserHistory.where(acting_user_id: user.id, action: UserHistory.actions[:create_badge]).exists?).to eq(true)
       end
     end
 
@@ -83,14 +85,11 @@ describe Admin::BadgesController do
     end
 
     context '.destroy' do
-      it 'returns success' do
-        delete :destroy, params: { id: badge.id }, format: :json
-        expect(response).to be_success
-      end
-
       it 'deletes the badge' do
         delete :destroy, params: { id: badge.id }, format: :json
-        expect(Badge.where(id: badge.id).count).to eq(0)
+        expect(response).to be_success
+        expect(Badge.where(id: badge.id).exists?).to eq(false)
+        expect(UserHistory.where(acting_user_id: user.id, action: UserHistory.actions[:delete_badge]).exists?).to eq(true)
       end
     end
 
@@ -108,6 +107,8 @@ describe Admin::BadgesController do
         expect(response).to be_success
         editor_badge.reload
         expect(editor_badge.name).to eq(editor_badge_name)
+
+        expect(UserHistory.where(acting_user_id: user.id, action: UserHistory.actions[:change_badge]).exists?).to eq(true)
       end
 
       it 'does not allow query updates if badge_sql is disabled' do