FIX: faster update of all badges

Introduced badge triggers, introduced concept of badge that happens due to a post but has the post hidden

Delta badge grant happens once a minute, backed by redis

app/jobs/regular/update_badges.rb

@@ -1,42 +0,0 @@
-module Jobs
-  class UpdateBadges < Jobs::Base
-
-    def execute(args)
-      self.send(args[:action], args)
-    end
-
-    def trust_level_change(args)
-      user = User.find(args[:user_id])
-      trust_level = user.trust_level
-      Badge.trust_level_badge_ids.each do |badge_id|
-        user_badge = UserBadge.find_by(user_id: user.id, badge_id: badge_id)
-        if user_badge
-          # Revoke the badge if trust level was lowered.
-          BadgeGranter.revoke(user_badge) if trust_level < badge_id
-        else
-          # Grant the badge if trust level was increased.
-          badge = Badge.find(badge_id)
-          BadgeGranter.grant(badge, user) if trust_level >= badge_id
-        end
-      end
-    end
-
-    def post_like(args)
-      post = Post.find(args[:post_id])
-      user = post.user
-
-      # Grant "Welcome" badge to the user if they do not already have it.
-      BadgeGranter.grant(Badge.find(Badge::Welcome), user)
-
-      Badge.like_badge_counts.each do |badge_id, count|
-        if post.like_count >= count
-          BadgeGranter.grant(Badge.find(badge_id), user, post_id: post.id)
-        else
-          user_badge = UserBadge.find_by(badge_id: badge_id, user_id: user.id, post_id: post.id)
-          user_badge && BadgeGranter.revoke(user_badge)
-        end
-      end
-    end
-
-  end
-end

app/jobs/scheduled/process_badge_backlog.rb

@@ -0,0 +1,8 @@
+module Jobs
+  class ProcessBadgeBacklog < Jobs::Scheduled
+    every 1.minute
+    def execute(args)
+      BadgeGranter.process_queue!
+    end
+  end
+end

app/models/badge.rb

@@ -19,16 +19,15 @@ class Badge < ActiveRecord::Base
 
   module Trigger
     PostAction = 1
-    ReadGuidelines = 2
+    PostRevision = 2
-    PostRevision = 4
+    TrustLevelChange = 4
-    TrustLevelChange = 8
+    UserChange = 8
-    UserChange = 16
   end
 
   module Queries
 
     Reader = <<SQL
-    SELECT id user_id, current_timestamp granted_at
+    SELECT id user_id, current_timestamp granted_at, NULL post_id
     FROM users
     WHERE id IN
     (
@@ -45,7 +44,7 @@ class Badge < ActiveRecord::Base
 SQL
 
     ReadGuidelines = <<SQL
-    SELECT user_id, read_faq granted_at
+    SELECT user_id, read_faq granted_at, NULL post_id
     FROM user_stats
     WHERE read_faq IS NOT NULL
 SQL
@@ -93,21 +92,30 @@ SQL
 SQL
 
     FirstFlag = <<SQL
-    SELECT pa.user_id, min(pa.created_at) granted_at
+    SELECT pa1.user_id, pa1.created_at granted_at, pa1.post_id
+    FROM (
+      SELECT pa.user_id, min(pa.id) id
       FROM post_actions pa
       JOIN badge_posts p on p.id = pa.post_id
       WHERE post_action_type_id IN (#{PostActionType.flag_types.values.join(",")})
       GROUP BY pa.user_id
+    ) x
+    JOIN post_actions pa1 on pa1.id = x.id
 SQL
 
     FirstLike = <<SQL
-    SELECT pa.user_id, min(post_id) post_id, min(pa.created_at) granted_at
+    SELECT pa1.user_id, pa1.created_at granted_at, pa1.post_id
+    FROM (
+      SELECT pa.user_id, min(pa.id) id
       FROM post_actions pa
       JOIN badge_posts p on p.id = pa.post_id
       WHERE post_action_type_id = 2
       GROUP BY pa.user_id
+    ) x
+    JOIN post_actions pa1 on pa1.id = x.id
 SQL
 
+    # Incorrect, but good enough - (earlies post edited vs first edit)
     Editor = <<SQL
     SELECT p.user_id, min(p.id) post_id, min(p.created_at) granted_at
     FROM badge_posts p
@@ -124,7 +132,7 @@ SQL
 SQL
 
     Autobiographer = <<SQL
-    SELECT u.id user_id, current_timestamp granted_at
+    SELECT u.id user_id, current_timestamp granted_at, NULL post_id
     FROM users u
     JOIN user_profiles up on u.id = up.user_id
     WHERE bio_raw IS NOT NULL AND LENGTH(TRIM(bio_raw)) > #{Badge::AutobiographerMinBioLength} AND
@@ -143,7 +151,7 @@ SQL
     def self.trust_level(level)
       # we can do better with dates, but its hard work figuring this out historically
 "
-    SELECT u.id user_id, current_timestamp granted_at FROM users u
+    SELECT u.id user_id, current_timestamp granted_at, NULL post_id FROM users u
     WHERE trust_level >= #{level.to_i}
 "
     end

app/models/post_action.rb

@@ -152,8 +152,8 @@ class PostAction < ActiveRecord::Base
 
     if row_count == 0
       post_action = create(where_attrs.merge(action_attributes))
-      if post_action && post_action.is_like?
+      if post_action && post_action.errors.count == 0
-        BadgeGranter.update_badges(action: :post_like, post_id: post.id)
+        BadgeGranter.queue_badge_grant(Badge::Trigger::PostAction, post_action: post_action)
       end
     else
       post_action = PostAction.where(where_attrs).first

app/models/user.rb

@@ -81,6 +81,7 @@ class User < ActiveRecord::Base
   after_create :create_user_profile
   after_create :ensure_in_trust_level_group
   after_save :refresh_avatar
+  after_save :badge_grant
 
   before_destroy do
     # These tables don't have primary keys, so destroying them with activerecord is tricky:
@@ -603,25 +604,15 @@ class User < ActiveRecord::Base
 
     if !self.uploaded_avatar_id && gravatar_downloaded
       self.update_column(:uploaded_avatar_id, avatar.gravatar_upload_id)
-      grant_autobiographer
-    else
-      if uploaded_avatar_id_changed?
-        grant_autobiographer
-      end
-    end
-
-  end
-
-  def grant_autobiographer
-    if self.user_profile.bio_raw &&
-          self.user_profile.bio_raw.strip.length > Badge::AutobiographerMinBioLength &&
-          uploaded_avatar_id
-       BadgeGranter.grant(Badge.find(Badge::Autobiographer), self)
     end
   end
 
   protected
 
+  def badge_grant
+    BadgeGranter.queue_badge_grant(Badge::Trigger::UserChange, user: self)
+  end
+
   def update_tracked_topics
     return unless auto_track_topics_after_msecs_changed?
     TrackedTopicsUpdater.new(id, auto_track_topics_after_msecs).call

app/models/user_profile.rb

@@ -3,7 +3,7 @@ class UserProfile < ActiveRecord::Base
 
   validates :user, presence: true
   before_save :cook
-  after_save :assign_autobiographer
+  after_save :trigger_badges
 
   def bio_excerpt
     excerpt = PrettyText.excerpt(bio_cooked, 350)
@@ -38,10 +38,8 @@ class UserProfile < ActiveRecord::Base
 
   protected
 
-  def assign_autobiographer
+  def trigger_badges
-    if bio_raw_changed?
+    BadgeGranter.queue_badge_grant(Badge::Trigger::UserChange, user: self)
-      user.grant_autobiographer
-    end
   end
 
   private

app/models/user_stat.rb

@@ -1,6 +1,7 @@
 class UserStat < ActiveRecord::Base
 
   belongs_to :user
+  after_save :trigger_badges
 
   # Updates the denormalized view counts for all users
   def self.update_view_counts
@@ -64,6 +65,12 @@ class UserStat < ActiveRecord::Base
     cache_last_seen(Time.now.to_f)
   end
 
+  protected
+
+  def trigger_badges
+    BadgeGranter.queue_badge_grant(Badge::Trigger::UserChange, user: self.user)
+  end
+
   private
 
   def last_seen_key

app/serializers/user_badge_serializer.rb

@@ -11,10 +11,11 @@ class UserBadgeSerializer < ApplicationSerializer
   end
 
   def include_post_id?
-    object.post_id && object.post
+    object.badge.show_posts && object.post_id && object.post
   end
 
   alias :include_topic? :include_post_id?
+  alias :include_post_number? :include_post_id?
 
   def post_number
     object.post && object.post.post_number

app/services/badge_granter.rb

@@ -57,13 +57,77 @@ class BadgeGranter
     end
   end
 
-  def self.update_badges(args)
+  def self.queue_badge_grant(type,opt)
-    Jobs.enqueue(:update_badges, args)
+    payload = nil
+
+    case type
+    when Badge::Trigger::PostRevision
+      post = opt[:post]
+      payload = {
+        type: "PostRevision",
+        post_ids: [post.id]
+      }
+    when Badge::Trigger::UserChange
+      user = opt[:user]
+      payload = {
+        type: "UserChange",
+        user_ids: [user.id]
+      }
+    when Badge::Trigger::TrustLevelChange
+      user = opt[:user]
+      payload = {
+        type: "TrustLevelChange",
+        user_ids: [user.id]
+      }
+    when Badge::Trigger::PostAction
+      action = opt[:post_action]
+      payload = {
+        type: "PostAction",
+        post_ids: [action.post_id, action.related_post_id].compact!
+      }
     end
 
-  def self.backfill(badge)
+    $redis.lpush queue_key, payload.to_json if payload
+  end
+
+  def self.clear_queue!
+    $redis.del queue_key
+  end
+
+  def self.process_queue!
+    limit = 1000
+    items = []
+    while limit > 0 && item = $redis.lpop(queue_key)
+      items << JSON.parse(item)
+      limit -= 1
+    end
+
+    items = items.group_by{|i| i["type"]}
+
+    items.each do |type, list|
+      post_ids = list.map{|i| i["post_ids"]}.flatten.compact.uniq
+      user_ids = list.map{|i| i["user_ids"]}.flatten.compact.uniq
+
+      next unless post_ids.present? || user_ids.present?
+      find_by_type(type).each{|badge| backfill(badge, post_ids: post_ids, user_ids: user_ids)}
+    end
+  end
+
+  def self.find_by_type(type)
+    id = "Badge::Trigger::#{type}".constantize
+    Badge.where(trigger: id)
+  end
+
+  def self.queue_key
+    "badge_queue".freeze
+  end
+
+  def self.backfill(badge, opts=nil)
     return unless badge.query.present? && badge.enabled
 
+    post_ids = opts[:post_ids] if opts
+    user_ids = opts[:user_ids] if opts
+
     post_clause = badge.target_posts ? "AND q.post_id = ub.post_id" : ""
     post_id_field = badge.target_posts ? "q.post_id" : "NULL"
 
@@ -77,7 +141,7 @@ class BadgeGranter
              WHERE ub.badge_id = :id AND q.user_id IS NULL
            )"
 
-    Badge.exec_sql(sql, id: badge.id) if badge.auto_revoke
+    Badge.exec_sql(sql, id: badge.id) if badge.auto_revoke && !post_ids && !user_ids
 
     sql = "INSERT INTO user_badges(badge_id, user_id, granted_at, granted_by_id, post_id)
             SELECT :id, q.user_id, q.granted_at, -1, #{post_id_field}
@@ -85,11 +149,15 @@ class BadgeGranter
             LEFT JOIN user_badges ub ON
               ub.badge_id = :id AND ub.user_id = q.user_id
               #{post_clause}
-            WHERE ub.badge_id IS NULL AND q.user_id <> -1
+            /*where*/
             RETURNING id, user_id, granted_at
             "
 
     builder = SqlBuilder.new(sql)
+    builder.where("ub.badge_id IS NULL AND q.user_id <> -1")
+    builder.where("q.post_id in (:post_ids)", post_ids: post_ids) if post_ids.present?
+    builder.where("q.user_id in (:user_ids)", user_ids: user_ids) if user_ids.present?
+
     builder.map_exec(OpenStruct, id: badge.id).each do |row|
 
       # old bronze badges do not matter

db/fixtures/006_badges.rb

@@ -65,6 +65,7 @@ Badge.seed do |b|
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = false
+  b.show_posts = false
   b.query = Badge::Queries::Reader
   b.default_badge_grouping_id = BadgeGrouping::GettingStarted
   b.auto_revoke = false
@@ -76,9 +77,10 @@ Badge.seed do |b|
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = false
+  b.show_posts = false
   b.query = Badge::Queries::ReadGuidelines
   b.default_badge_grouping_id = BadgeGrouping::GettingStarted
-  b.trigger = Badge::Trigger::ReadGuidelines
+  b.trigger = Badge::Trigger::UserChange
 end
 
 Badge.seed do |b|
@@ -87,6 +89,7 @@ Badge.seed do |b|
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
+  b.show_posts = true
   b.query = Badge::Queries::FirstLink
   b.default_badge_grouping_id = BadgeGrouping::GettingStarted
   b.trigger = Badge::Trigger::PostRevision
@@ -98,6 +101,7 @@ Badge.seed do |b|
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
+  b.show_posts = true
   b.query = Badge::Queries::FirstQuote
   b.default_badge_grouping_id = BadgeGrouping::GettingStarted
   b.trigger = Badge::Trigger::PostRevision
@@ -109,6 +113,7 @@ Badge.seed do |b|
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
+  b.show_posts = true
   b.query = Badge::Queries::FirstLike
   b.default_badge_grouping_id = BadgeGrouping::GettingStarted
   b.trigger = Badge::Trigger::PostAction
@@ -119,7 +124,8 @@ Badge.seed do |b|
   b.default_name = "First Flag"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
-  b.target_posts = false
+  b.target_posts = true
+  b.show_posts = false
   b.query = Badge::Queries::FirstFlag
   b.default_badge_grouping_id = BadgeGrouping::Community
   b.trigger = Badge::Trigger::PostAction
@@ -131,6 +137,7 @@ Badge.seed do |b|
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
+  b.show_posts = true
   b.query = Badge::Queries::FirstShare
   b.default_badge_grouping_id = BadgeGrouping::GettingStarted
   b.trigger = Badge::Trigger::PostRevision
@@ -142,6 +149,7 @@ Badge.seed do |b|
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
+  b.show_posts = true
   b.query = Badge::Queries::Welcome
   b.default_badge_grouping_id = BadgeGrouping::Community
   b.trigger = Badge::Trigger::PostAction
@@ -182,6 +190,7 @@ like_badges.each do |spec|
     b.badge_type_id = spec[:type]
     b.multiple_grant = spec[:multiple]
     b.target_posts = true
+    b.show_posts = true
     b.query = Badge::Queries.like_badge(Badge.like_badge_counts[spec[:id]])
     b.default_badge_grouping_id = BadgeGrouping::Posting
     b.trigger = Badge::Trigger::PostAction

db/migrate/20140723011456_add_show_posts_to_badges.rb

@@ -0,0 +1,6 @@
+class AddShowPostsToBadges < ActiveRecord::Migration
+  def change
+    # show posts to users on badge show page
+    add_column :badges, :show_posts, :boolean, null: false, default: false
+  end
+end

lib/post_creator.rb

@@ -79,6 +79,7 @@ class PostCreator
       handle_spam unless @opts[:import_mode]
       track_latest_on_category
       enqueue_jobs
+      BadgeGranter.queue_badge_grant(Badge::Trigger::PostRevision, post: @post)
     end
 
     @post

lib/post_revisor.rb

@@ -28,6 +28,7 @@ class PostRevisor
     @post.advance_draft_sequence
     PostAlerter.new.after_save_post(@post)
     publish_revision
+    BadgeGranter.queue_badge_grant(Badge::Trigger::PostRevision, post: @post)
 
     true
   end

lib/promotion.rb

@@ -72,7 +72,7 @@ class Promotion
       @user.user_profile.recook_bio
       @user.user_profile.save!
       Group.user_trust_level_change!(@user.id, @user.trust_level)
-      BadgeGranter.update_badges(action: :trust_level_change, user_id: @user.id)
+      BadgeGranter.queue_badge_grant(Badge::Trigger::TrustLevelChange, user: @user)
     end
 
     true

spec/controllers/user_badges_controller_spec.rb

@@ -4,6 +4,20 @@ describe UserBadgesController do
   let(:user) { Fabricate(:user) }
   let(:badge) { Fabricate(:badge) }
 
+  context 'index' do
+    it 'doest not leak private info' do
+      badge = Fabricate(:badge, target_posts: true, show_posts: false)
+      p = create_post
+      UserBadge.create(badge: badge, user: user, post_id: p.id, granted_by_id: -1, granted_at: Time.now)
+
+      xhr :get, :index, badge_id: badge.id
+      response.status.should == 200
+      parsed = JSON.parse(response.body)
+      parsed["topics"].should be_nil
+      parsed["user_badges"][0]["post_id"].should == nil
+    end
+  end
+
   context 'index' do
     let!(:user_badge) { UserBadge.create(badge: badge, user: user, granted_by: Discourse.system_user, granted_at: Time.now) }
 

spec/models/post_alert_observer_spec.rb

@@ -15,8 +15,8 @@ describe PostAlertObserver do
       it 'creates a notification' do
         lambda {
           PostAction.act(evil_trout, post, PostActionType.types[:like])
-          # one like and one welcome badge
+          # one like (welcome badge deferred)
-        }.should change(Notification, :count).by(2)
+        }.should change(Notification, :count).by(1)
       end
     end
 

spec/models/post_timing_spec.rb

@@ -2,9 +2,6 @@ require 'spec_helper'
 
 describe PostTiming do
 
-  it { should belong_to :topic }
-  it { should belong_to :user }
-
   it { should validate_presence_of :post_number }
   it { should validate_presence_of :msecs }
 
@@ -76,14 +73,14 @@ describe PostTiming do
 
       PostAction.act(user2, post, PostActionType.types[:like])
 
-      post.user.unread_notifications.should == 2
+      post.user.unread_notifications.should == 1
-      post.user.unread_notifications_by_type.should == {Notification.types[:granted_badge] => 1, Notification.types[:liked] => 1 }
+      post.user.unread_notifications_by_type.should == {Notification.types[:liked] => 1 }
 
       PostTiming.process_timings(post.user, post.topic_id, 1, [[post.post_number, 100]])
 
       post.user.reload
-      post.user.unread_notifications_by_type.should == {Notification.types[:granted_badge] => 1}
+      post.user.unread_notifications_by_type.should == {}
-      post.user.unread_notifications.should == 1
+      post.user.unread_notifications.should == 0
 
     end
   end

spec/services/badge_granter_spec.rb

@@ -48,21 +48,6 @@ describe BadgeGranter do
     end
   end
 
-  describe 'autobiographer' do
-    it 'grants autobiographer correctly' do
-      user = Fabricate(:user)
-      user.user_profile.bio_raw = "I filled my bio"
-      user.user_profile.save!
-
-      Badge.find(Badge::Autobiographer).grant_count.should == 0
-
-      user.uploaded_avatar_id = 100
-      user.save
-
-      Badge.find(Badge::Autobiographer).grant_count.should == 1
-    end
-  end
-
   describe 'grant' do
 
     it 'grants a badge' do
@@ -128,10 +113,57 @@ describe BadgeGranter do
     let(:user) { Fabricate(:user) }
     let(:liker) { Fabricate(:user) }
 
+    before do
+      BadgeGranter.clear_queue!
+    end
+
+    it "grants autobiographer" do
+      user.user_profile.bio_raw  = "THIS IS MY bio it a long bio I like my bio"
+      user.uploaded_avatar_id = 10
+      user.user_profile.save
+      user.save
+
+      BadgeGranter.process_queue!
+      UserBadge.where(user_id: user.id, badge_id: Badge::Autobiographer).count.should eq(1)
+    end
+
+    it "grants read guidlines" do
+      user.user_stat.read_faq = Time.now
+      user.user_stat.save
+
+      BadgeGranter.process_queue!
+      UserBadge.where(user_id: user.id, badge_id: Badge::ReadGuidelines).count.should eq(1)
+    end
+
+    it "grants first link" do
+      post = create_post
+      post2 = create_post(raw: "#{Discourse.base_url}/t/slug/#{post.topic_id}")
+
+      BadgeGranter.process_queue!
+      UserBadge.where(user_id: post2.user.id, badge_id: Badge::FirstLink).count.should eq(1)
+    end
+
+    it "grants first edit" do
+      SiteSetting.ninja_edit_window = 0
+      post = create_post
+      user = post.user
+
+      UserBadge.where(user_id: user.id, badge_id: Badge::Editor).count.should eq(0)
+
+      PostRevisor.new(post).revise!(user, "This is my new test 1235 123")
+      BadgeGranter.process_queue!
+
+      UserBadge.where(user_id: user.id, badge_id: Badge::Editor).count.should eq(1)
+    end
+
     it "grants and revokes trust level badges" do
       user.change_trust_level!(:elder)
+      BadgeGranter.process_queue!
       UserBadge.where(user_id: user.id, badge_id: Badge.trust_level_badge_ids).count.should eq(4)
+
       user.change_trust_level!(:basic)
+      BadgeGranter.backfill(Badge.find(1))
+      BadgeGranter.backfill(Badge.find(2))
       UserBadge.where(user_id: user.id, badge_id: 1).first.should_not be_nil
       UserBadge.where(user_id: user.id, badge_id: 2).first.should be_nil
     end
@@ -139,26 +171,35 @@ describe BadgeGranter do
     it "grants system like badges" do
       post = create_post(user: user)
       # Welcome badge
-      PostAction.act(liker, post, PostActionType.types[:like])
+      action = PostAction.act(liker, post, PostActionType.types[:like])
+      BadgeGranter.process_queue!
       UserBadge.find_by(user_id: user.id, badge_id: 5).should_not be_nil
+
       # Nice post badge
       post.update_attributes like_count: 10
-      BadgeGranter.update_badges(action: :post_like, post_id: post.id)
+
-      BadgeGranter.update_badges(action: :post_like, post_id: post.id)
+      BadgeGranter.queue_badge_grant(Badge::Trigger::PostAction, post_action: action)
+      BadgeGranter.process_queue!
+
       UserBadge.find_by(user_id: user.id, badge_id: 6).should_not be_nil
       UserBadge.where(user_id: user.id, badge_id: 6).count.should == 1
+
       # Good post badge
       post.update_attributes like_count: 25
-      BadgeGranter.update_badges(action: :post_like, post_id: post.id)
+      BadgeGranter.queue_badge_grant(Badge::Trigger::PostAction, post_action: action)
+      BadgeGranter.process_queue!
       UserBadge.find_by(user_id: user.id, badge_id: 7).should_not be_nil
+
       # Great post badge
       post.update_attributes like_count: 50
-      BadgeGranter.update_badges(action: :post_like, post_id: post.id)
+      BadgeGranter.queue_badge_grant(Badge::Trigger::PostAction, post_action: action)
+      BadgeGranter.process_queue!
       UserBadge.find_by(user_id: user.id, badge_id: 8).should_not be_nil
+
       # Revoke badges on unlike
       post.update_attributes like_count: 49
-      BadgeGranter.update_badges(action: :post_like, post_id: post.id)
+      BadgeGranter.backfill(Badge.find(Badge::GreatPost))
-      UserBadge.find_by(user_id: user.id, badge_id: 8).should be_nil
+      UserBadge.find_by(user_id: user.id, badge_id: Badge::GreatPost).should be_nil
     end
   end