FIX: badge granted titles were not being revoked when badge was revoked

app/controllers/users_controller.rb

@@ -85,7 +85,7 @@ class UsersController < ApplicationController
       email: user.email,
       associated_accounts: user.associated_accounts
     }
-  rescue Discourse::InvalidAccess => e
+  rescue Discourse::InvalidAccess
     render json: failed_json, status: 403
   end
 
@@ -98,7 +98,9 @@ class UsersController < ApplicationController
     user_badge = UserBadge.find_by(id: params[:user_badge_id])
     if user_badge && user_badge.user == user && user_badge.badge.allow_title?
       user.title = user_badge.badge.name
+      user.user_profile.badge_granted_title = true
       user.save!
+      user.user_profile.save!
     else
       user.title = ''
       user.save!

app/jobs/scheduled/badge_grant.rb

@@ -9,9 +9,12 @@ module Jobs
 
     def execute(args)
       return unless SiteSetting.enable_badges
+
       Badge.all.each do |b|
         BadgeGranter.backfill(b)
       end
+
+      BadgeGranter.revoke_ungranted_titles!
     end
 
   end

app/models/user.rb

@@ -676,6 +676,13 @@ class User < ActiveRecord::Base
     @user_fields
   end
 
+  def title=(val)
+    write_attribute(:title, val)
+    if !new_record? && user_profile
+      user_profile.update_column(:badge_granted_title, false)
+    end
+  end
+
   protected
 
   def badge_grant

app/services/badge_granter.rb

@@ -139,24 +139,24 @@ class BadgeGranter
   def self.contract_checks!(sql, opts = {})
     return unless sql.present?
     if Badge::Trigger.uses_post_ids?(opts[:trigger])
-      raise "Contract violation:\nQuery triggers on posts, but does not reference the ':post_ids' array" unless sql.match /:post_ids/
-      raise "Contract violation:\nQuery triggers on posts, but references the ':user_ids' array" if sql.match /:user_ids/
+      raise("Contract violation:\nQuery triggers on posts, but does not reference the ':post_ids' array") unless sql.match(/:post_ids/)
+      raise "Contract violation:\nQuery triggers on posts, but references the ':user_ids' array" if sql.match(/:user_ids/)
     end
     if Badge::Trigger.uses_user_ids?(opts[:trigger])
-      raise "Contract violation:\nQuery triggers on users, but does not reference the ':user_ids' array" unless sql.match /:user_ids/
-      raise "Contract violation:\nQuery triggers on users, but references the ':post_ids' array" if sql.match /:post_ids/
+      raise "Contract violation:\nQuery triggers on users, but does not reference the ':user_ids' array" unless sql.match(/:user_ids/)
+      raise "Contract violation:\nQuery triggers on users, but references the ':post_ids' array" if sql.match(/:post_ids/)
     end
     if opts[:trigger] && !Badge::Trigger.is_none?(opts[:trigger])
-      raise "Contract violation:\nQuery is triggered, but does not reference the ':backfill' parameter.\n(Hint: if :backfill is TRUE, you should ignore the :post_ids/:user_ids)" unless sql.match /:backfill/
+      raise "Contract violation:\nQuery is triggered, but does not reference the ':backfill' parameter.\n(Hint: if :backfill is TRUE, you should ignore the :post_ids/:user_ids)" unless sql.match(/:backfill/)
     end
 
     # TODO these three conditions have a lot of false negatives
     if opts[:target_posts]
-      raise "Contract violation:\nQuery targets posts, but does not return a 'post_id' column" unless sql.match /post_id/
+      raise "Contract violation:\nQuery targets posts, but does not return a 'post_id' column" unless sql.match(/post_id/)
     end
-    raise "Contract violation:\nQuery does not return a 'user_id' column" unless sql.match /user_id/
-    raise "Contract violation:\nQuery does not return a 'granted_at' column" unless sql.match /granted_at/
-    raise "Contract violation:\nQuery ends with a semicolon. Remove the semicolon; your sql will be used in a subquery." if sql.match /;\s*\z/
+    raise "Contract violation:\nQuery does not return a 'user_id' column" unless sql.match(/user_id/)
+    raise "Contract violation:\nQuery does not return a 'granted_at' column" unless sql.match(/granted_at/)
+    raise "Contract violation:\nQuery ends with a semicolon. Remove the semicolon; your sql will be used in a subquery." if sql.match(/;\s*\z/)
   end
 
   # Options:
@@ -302,4 +302,22 @@ class BadgeGranter
 
   end
 
+
+  def self.revoke_ungranted_titles!
+    Badge.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
+                        )
+                   ")
+  end
+
 end

db/migrate/20141007224814_add_badge_granted_title_to_user_profile.rb

@@ -0,0 +1,19 @@
+class AddBadgeGrantedTitleToUserProfile < ActiveRecord::Migration
+  def up
+    add_column :user_profiles, :badge_granted_title, :boolean, default: false
+
+    execute "UPDATE user_profiles SET badge_granted_title = true
+    WHERE EXISTS (
+      SELECT 1 FROM users WHERE users.id = user_id AND title IN ('Leader', 'Regular')
+    )"
+
+    execute "UPDATE user_profiles SET badge_granted_title = true
+    WHERE EXISTS (
+      SELECT 1 FROM users WHERE users.id = user_id AND title IN (SELECT name FROM badges WHERE allow_title)
+    )"
+  end
+
+  def down
+    remove_column :user_profiles, :badge_granted_title
+  end
+end

spec/controllers/users_controller_spec.rb

@@ -968,6 +968,13 @@ describe UsersController do
       badge.update_attributes allow_title: true
       xhr :put, :badge_title, user_badge_id: user_badge.id, username: user.username
       user.reload.title.should == badge.name
+      user.user_profile.badge_granted_title.should == true
+
+      user.title = "testing"
+      user.save
+      user.user_profile.reload
+      user.user_profile.badge_granted_title.should == false
+
     end
   end
 

spec/services/badge_granter_spec.rb

@@ -5,6 +5,35 @@ describe BadgeGranter do
   let(:badge) { Fabricate(:badge) }
   let(:user) { Fabricate(:user) }
 
+  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)
+
+      BadgeGranter.revoke_ungranted_titles!
+
+      user.reload
+      user.title.should == badge.name
+
+      badge.update_column(:allow_title, false)
+      BadgeGranter.revoke_ungranted_titles!
+
+      user.reload
+      user.title.should == ''
+
+      user.title = "CEO"
+      user.save
+
+      BadgeGranter.revoke_ungranted_titles!
+
+      user.reload
+      user.title.should == "CEO"
+    end
+  end
+
   describe 'preview' do
     it 'can correctly preview' do
       Fabricate(:user, email: 'sam@gmail.com')