FIX: Unassign user titles when a badge is deleted (#9573)

FIX: Unassign user titles when a badge is deleted (#9573)

diff --git a/app/controllers/admin/badges_controller.rb b/app/controllers/admin/badges_controller.rb
index e3aca68..cfe6460 100644
--- a/app/controllers/admin/badges_controller.rb
+++ b/app/controllers/admin/badges_controller.rb
@@ -129,9 +129,12 @@ class Admin::BadgesController < Admin::AdminController
   end
 
   def destroy
-    badge = find_badge
-    StaffActionLogger.new(current_user).log_badge_deletion(badge)
-    badge.destroy
+    Badge.transaction do
+      badge = find_badge
+      StaffActionLogger.new(current_user).log_badge_deletion(badge)
+      badge.clear_user_titles!
+      badge.destroy!
+    end
     render body: nil
   end
 
diff --git a/app/jobs/regular/bulk_user_title_update.rb b/app/jobs/regular/bulk_user_title_update.rb
index e98c024..6dae521 100644
--- a/app/jobs/regular/bulk_user_title_update.rb
+++ b/app/jobs/regular/bulk_user_title_update.rb
@@ -9,43 +9,17 @@ module Jobs
       new_title = args[:new_title]
       granted_badge_id = args[:granted_badge_id]
       action = args[:action]
+      badge = Badge.find(granted_badge_id) rescue nil
+
+      return unless badge # Deleted badge protection
 
       case action
       when UPDATE_ACTION
-        update_titles_for_granted_badge(new_title, granted_badge_id)
+        badge.update_user_titles!(new_title)
       when RESET_ACTION
-        reset_titles_for_granted_badge(granted_badge_id)
+        badge.reset_user_titles!
       end
     end
 
-    private
-
-    ##
-    # If a badge name or a system badge TranslationOverride changes
-    # then we need to set all titles granted based on that badge to
-    # the new name or custom translation
-    def update_titles_for_granted_badge(new_title, granted_badge_id)
-      DB.exec(<<~SQL, granted_title_badge_id: granted_badge_id, title: new_title, updated_at: Time.now)
-        UPDATE users AS u
-        SET title = :title, updated_at = :updated_at
-        FROM user_profiles AS up
-        WHERE up.user_id = u.id AND up.granted_title_badge_id = :granted_title_badge_id
-      SQL
-    end
-
-    ##
-    # Reset granted titles for a badge back to the original
-    # badge name. When a system badge has its TranslationOverride
-    # revoked we want to have all titles based on that translation
-    # for the badge reset.
-    def reset_titles_for_granted_badge(granted_badge_id)
-      DB.exec(<<~SQL, granted_title_badge_id: granted_badge_id, updated_at: Time.now)
-        UPDATE users AS u
-        SET title = badges.name, updated_at = :updated_at
-        FROM user_profiles AS up
-        INNER JOIN badges ON badges.id = up.granted_title_badge_id
-        WHERE up.user_id = u.id AND up.granted_title_badge_id = :granted_title_badge_id
-      SQL
-    end
   end
 end
diff --git a/app/models/badge.rb b/app/models/badge.rb
index 5608444..69d06de 100644
--- a/app/models/badge.rb
+++ b/app/models/badge.rb
@@ -166,6 +166,44 @@ class Badge < ActiveRecord::Base
     SQL
   end
 
+  def clear_user_titles!
+    DB.exec(<<~SQL, badge_id: self.id, updated_at: Time.zone.now)
+      UPDATE users AS u
+      SET title = '', updated_at = :updated_at
+      FROM user_profiles AS up
+      WHERE up.user_id = u.id AND up.granted_title_badge_id = :badge_id
+    SQL
+    DB.exec(<<~SQL, badge_id: self.id)
+      UPDATE user_profiles AS up
+      SET badge_granted_title = false, granted_title_badge_id = NULL
+      WHERE up.granted_title_badge_id = :badge_id
+    SQL
+  end
+
+  ##
+  # Update all user titles based on a badge to the new name
+  def update_user_titles!(new_title)
+    DB.exec(<<~SQL, granted_title_badge_id: self.id, title: new_title, updated_at: Time.zone.now)
+      UPDATE users AS u
+      SET title = :title, updated_at = :updated_at
+      FROM user_profiles AS up
+      WHERE up.user_id = u.id AND up.granted_title_badge_id = :granted_title_badge_id
+    SQL
+  end
+
+  ##
+  # When a badge has its TranslationOverride cleared, reset
+  # all user titles granted to the standard name.
+  def reset_user_titles!
+    DB.exec(<<~SQL, granted_title_badge_id: self.id, updated_at: Time.zone.now)
+      UPDATE users AS u
+      SET title = badges.name, updated_at = :updated_at
+      FROM user_profiles AS up
+      INNER JOIN badges ON badges.id = up.granted_title_badge_id
+      WHERE up.user_id = u.id AND up.granted_title_badge_id = :granted_title_badge_id
+    SQL
+  end
+
   def self.i18n_name(name)
     name.downcase.tr(' ', '_')
   end
diff --git a/spec/requests/badges_controller_spec.rb b/spec/requests/badges_controller_spec.rb
index 0a4971d..777596a 100644
--- a/spec/requests/badges_controller_spec.rb
+++ b/spec/requests/badges_controller_spec.rb
@@ -42,4 +42,54 @@ describe BadgesController do
       expect(response.media_type).to eq('application/rss+xml')
     end
   end
+
+  context "user profiles" do
+    let(:titled_badge) { Fabricate(:badge, name: 'Protector of the Realm', allow_title: true) }
+    let!(:grant) { UserBadge.create!(user_id: user.id, badge_id: titled_badge.id, granted_at: 1.minute.ago, granted_by_id: -1) }
+
+    it "can be assigned as a title by the user" do
+      sign_in(user)
+      put "/u/#{user.username}/preferences/badge_title.json", params: {
+        user_badge_id: grant.id,
+      }
+      expect(response.status).to eq(200)
+      user.reload
+
+      expect(user.title).to eq(titled_badge.display_name)
+      expect(user.user_profile.granted_title_badge_id).to eq(titled_badge.id)
+    end
+  end
+
+  describe "destroy" do
+    let(:admin) { Fabricate(:admin) }
+
+    context "while assigned as a title" do
+      let(:titled_badge) { Fabricate(:badge, name: 'Protector of the Realm', allow_title: true) }
+      let!(:grant) { UserBadge.create!(user_id: user.id, badge_id: titled_badge.id, granted_at: 1.minute.ago, granted_by_id: -1) }
+
+      before do
+        sign_in(user)
+        put "/u/#{user.username}/preferences/badge_title.json", params: {
+          user_badge_id: grant.id,
+        }
+        user.reload
+        sign_out
+      end
+
+      it "succeeds and unassigns the title from the user" do
+        expect(user.title).to eq(titled_badge.display_name)
+
+        sign_in(admin)
+        badge_id = titled_badge.id
+
+        delete "/admin/badges/#{titled_badge.id}.json"
+        expect(response.status).to be(200)
+        expect(Badge.find_by(id: badge_id)).to be(nil)
+
+        user.reload
+        expect(user.title).to_not eq(titled_badge.display_name)
+        expect(user.user_profile.granted_title_badge_id).to eq(nil)
+      end
+    end
+  end
 end

GitHub sha: 784bf2a1

1 Like

This commit appears in #9573 which was approved by eviltrout. It was merged by riking.