From cff68ef0dd31a286ac80c38ba29f9bd4e6b0f8a7 Mon Sep 17 00:00:00 2001
From: Gerhard Schlager <gerhard.schlager@discourse.org>
Date: Wed, 19 Aug 2020 23:23:31 +0200
Subject: [PATCH] FIX: User titles from translated badge names were
 automatically revoked

It also cleans up the denormalized data about badge titles in the user_profiles table.
---
 app/services/badge_granter.rb       | 38 +++++++++-----
 spec/services/badge_granter_spec.rb | 81 +++++++++++++++++++++++++----
 2 files changed, 96 insertions(+), 23 deletions(-)

diff --git a/app/services/badge_granter.rb b/app/services/badge_granter.rb
index dc21f5bb4a4..1e1059b791e 100644
--- a/app/services/badge_granter.rb
+++ b/app/services/badge_granter.rb
@@ -397,21 +397,33 @@ class BadgeGranter
 
   def self.revoke_ungranted_titles!
     DB.exec <<~SQL
-      UPDATE users SET title = ''
-      WHERE NOT title IS NULL AND
-         title <> '' AND
-         EXISTS (
-            SELECT 1
-            FROM user_profiles
-            WHERE user_id = users.id AND badge_granted_title
-         ) AND
-         title NOT IN (
-            SELECT name
-            FROM badges
-            WHERE allow_title AND enabled AND
-              badges.id IN (SELECT badge_id FROM user_badges ub where ub.user_id = users.id)
+      UPDATE users u
+      SET title = ''
+      FROM user_profiles up
+      WHERE u.title IS NOT NULL
+        AND u.title <> ''
+        AND up.user_id = u.id
+        AND up.badge_granted_title
+        AND up.granted_title_badge_id IS NOT NULL
+        AND NOT EXISTS(
+          SELECT 1
+          FROM badges b
+                 JOIN user_badges ub ON ub.user_id = u.id AND ub.badge_id = b.id
+          WHERE b.id = up.granted_title_badge_id
+            AND b.allow_title
+            AND b.enabled
         )
     SQL
+
+    DB.exec <<~SQL
+      UPDATE user_profiles up
+      SET badge_granted_title    = FALSE,
+          granted_title_badge_id = NULL
+      FROM users u
+      WHERE up.user_id = u.id
+        AND (u.title IS NULL OR u.title = '')
+        AND (up.badge_granted_title OR up.granted_title_badge_id IS NOT NULL)
+    SQL
   end
 
   def self.notification_locale(locale)
diff --git a/spec/services/badge_granter_spec.rb b/spec/services/badge_granter_spec.rb
index f36f93b02bc..e683c81b10b 100644
--- a/spec/services/badge_granter_spec.rb
+++ b/spec/services/badge_granter_spec.rb
@@ -17,33 +17,94 @@ describe BadgeGranter do
   end
 
   describe 'revoke_titles' do
-    it 'can correctly revoke titles' do
-      badge = Fabricate(:badge, allow_title: true)
-      user = Fabricate(:user, title: badge.name)
-      user.reload
-
-      user.user_profile.update_column(:badge_granted_title, true)
+    let(:user) { Fabricate(:user) }
+    let(:badge) { Fabricate(:badge, allow_title: true) }
 
+    it 'revokes title when badge is not allowed as title' do
       BadgeGranter.grant(badge, user)
-      BadgeGranter.revoke_ungranted_titles!
+      user.update!(title: badge.name)
 
+      BadgeGranter.revoke_ungranted_titles!
       user.reload
       expect(user.title).to eq(badge.name)
+      expect(user.user_profile.badge_granted_title).to eq(true)
+      expect(user.user_profile.granted_title_badge_id).to eq(badge.id)
 
       badge.update_column(:allow_title, false)
       BadgeGranter.revoke_ungranted_titles!
-
       user.reload
-      expect(user.title).to eq('')
+      expect(user.title).to be_blank
+      expect(user.user_profile.badge_granted_title).to eq(false)
+      expect(user.user_profile.granted_title_badge_id).to be_nil
+    end
 
+    it 'revokes title when badge is disabled' do
+      BadgeGranter.grant(badge, user)
+      user.update!(title: badge.name)
+
+      BadgeGranter.revoke_ungranted_titles!
+      user.reload
+      expect(user.title).to eq(badge.name)
+      expect(user.user_profile.badge_granted_title).to eq(true)
+      expect(user.user_profile.granted_title_badge_id).to eq(badge.id)
+
+      badge.update_column(:enabled, false)
+      BadgeGranter.revoke_ungranted_titles!
+      user.reload
+      expect(user.title).to be_blank
+      expect(user.user_profile.badge_granted_title).to eq(false)
+      expect(user.user_profile.granted_title_badge_id).to be_nil
+    end
+
+    it 'revokes title when user badge is revoked' do
+      BadgeGranter.grant(badge, user)
+      user.update!(title: badge.name)
+
+      BadgeGranter.revoke_ungranted_titles!
+      user.reload
+      expect(user.title).to eq(badge.name)
+      expect(user.user_profile.badge_granted_title).to eq(true)
+      expect(user.user_profile.granted_title_badge_id).to eq(badge.id)
+
+      BadgeGranter.revoke(user.user_badges.first)
+      BadgeGranter.revoke_ungranted_titles!
+      user.reload
+      expect(user.title).to be_blank
+      expect(user.user_profile.badge_granted_title).to eq(false)
+      expect(user.user_profile.granted_title_badge_id).to be_nil
+    end
+
+    it 'does not revoke custom title' do
       user.title = "CEO"
-      user.save
+      user.save!
 
       BadgeGranter.revoke_ungranted_titles!
 
       user.reload
       expect(user.title).to eq("CEO")
     end
+
+    it 'does not revoke localized title' do
+      badge = Badge.find(Badge::Regular)
+      badge_name = nil
+      BadgeGranter.grant(badge, user)
+
+      I18n.with_locale(:de) do
+        badge_name = badge.display_name
+        user.update!(title: badge_name)
+      end
+
+      user.reload
+      expect(user.title).to eq(badge_name)
+      expect(user.user_profile.badge_granted_title).to eq(true)
+      expect(user.user_profile.granted_title_badge_id).to eq(badge.id)
+
+      BadgeGranter.revoke_ungranted_titles!
+      user.reload
+      expect(user.title).to eq(badge_name)
+      expect(user.user_profile.badge_granted_title).to eq(true)
+      expect(user.user_profile.granted_title_badge_id).to eq(badge.id)
+    end
   end
 
   describe 'preview' do