From 9a9ad9bda8d72fdba768cb3dab5c861f603cf813 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 3 Jul 2014 17:29:44 +1000 Subject: [PATCH] FEATURE: Badge progress - Refactor model so it stores backfill query - Implement autobiographer - Remove sample badge - Correct featured badges to only include a badge once --- app/controllers/users_controller.rb | 2 +- app/jobs/regular/update_badges.rb | 8 +- app/jobs/scheduled/badge_grant.rb | 4 +- app/models/badge.rb | 80 +++++++++++++++---- app/models/category.rb | 8 +- app/models/category_search_data.rb | 2 + app/models/post.rb | 2 + app/models/post_search_data.rb | 2 + app/models/topic_link.rb | 1 + app/models/user.rb | 21 ++++- app/models/user_badge.rb | 1 - app/models/user_profile.rb | 22 +++-- app/models/user_search_data.rb | 2 + app/services/badge_granter.rb | 61 ++++++-------- config/locales/client.en.yml | 3 + db/fixtures/601_badges.rb | 59 +++++--------- .../20140703022838_add_fields_to_badges.rb | 7 ++ spec/services/badge_granter_spec.rb | 46 +++++++++-- 18 files changed, 216 insertions(+), 115 deletions(-) create mode 100644 db/migrate/20140703022838_add_fields_to_badges.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 438c2e8a9de..f5725f6280d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -233,7 +233,7 @@ class UsersController < ApplicationController end flash[:success] = I18n.t(message) - end + end def change_email params.require(:email) diff --git a/app/jobs/regular/update_badges.rb b/app/jobs/regular/update_badges.rb index 644118b8bac..702d876ca74 100644 --- a/app/jobs/regular/update_badges.rb +++ b/app/jobs/regular/update_badges.rb @@ -28,11 +28,11 @@ module Jobs # Grant "Welcome" badge to the user if they do not already have it. BadgeGranter.grant(Badge.find(5), user) - Badge.like_badge_info.each do |b| - if post.like_count >= b[:count] - BadgeGranter.grant(Badge.find(b[:id]), user, post_id: post.id) + 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: b[:id], user_id: user.id, post_id: post.id) + 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 diff --git a/app/jobs/scheduled/badge_grant.rb b/app/jobs/scheduled/badge_grant.rb index 9c1b48f8721..cbc591fb2dc 100644 --- a/app/jobs/scheduled/badge_grant.rb +++ b/app/jobs/scheduled/badge_grant.rb @@ -4,7 +4,9 @@ module Jobs every 1.day def execute(args) - BadgeGranter.backfill_like_badges + Badge.all.each do |b| + BadgeGranter.backfill(b) + end end end diff --git a/app/models/badge.rb b/app/models/badge.rb index a631db58f51..da3af7de827 100644 --- a/app/models/badge.rb +++ b/app/models/badge.rb @@ -1,4 +1,58 @@ class Badge < ActiveRecord::Base + + # badge ids + Welcome = 5 + NicePost = 6 + GoodPost = 7 + GreatPost = 8 + Autobiographer = 9 + + # other consts + AutobiographerMinBioLength = 10 + + + module Queries + Welcome = < #{Badge::AutobiographerMinBioLength} AND + uploaded_avatar_id IS NOT NULL +SQL + + def self.like_badge(count) + # we can do better with dates, but its hard work +" + SELECT p.user_id, p.id post_id, p.updated_at granted_at FROM posts p + JOIN topics t on p.topic_id = t.id + WHERE p.deleted_at IS NULL AND + t.deleted_at IS NULL AND + t.visible AND + p.like_count >= #{count.to_i} +" + end + + 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 + WHERE trust_level >= #{level.to_i} +" + end + end + belongs_to :badge_type has_many :user_badges, dependent: :destroy @@ -7,15 +61,19 @@ class Badge < ActiveRecord::Base validates :allow_title, inclusion: [true, false] validates :multiple_grant, inclusion: [true, false] - Welcome = 5 - NicePost = 6 - GoodPost = 7 - GreatPost = 8 def self.trust_level_badge_ids (1..4).to_a end + def self.like_badge_counts + @like_badge_counts ||= { + NicePost => 10, + GoodPost => 25, + GreatPost => 50 + } + end + def reset_grant_count! self.grant_count = UserBadge.where(badge_id: id).count save! @@ -25,14 +83,6 @@ class Badge < ActiveRecord::Base !self.multiple_grant? end - def self.like_badge_info - [ - {id: NicePost, count: 10}, - {id: GoodPost, count: 25}, - {id: GreatPost, count: 100} - ] - end - end # == Schema Information @@ -49,9 +99,11 @@ end # allow_title :boolean default(FALSE), not null # multiple_grant :boolean default(FALSE), not null # icon :string(255) default("fa-certificate") +# listable :boolean default(TRUE) +# target_posts :boolean default(FALSE) +# query :text # # Indexes # -# index_badges_on_badge_type_id (badge_type_id) -# index_badges_on_name (name) UNIQUE +# index_badges_on_name (name) UNIQUE # diff --git a/app/models/category.rb b/app/models/category.rb index d8898f12256..1c1052a7429 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -359,10 +359,12 @@ end # email_in_allow_strangers :boolean default(FALSE) # topics_day :integer default(0) # posts_day :integer default(0) +# logo_url :string(255) +# background_url :string(255) # # Indexes # -# index_categories_on_email_in (email_in) UNIQUE -# index_categories_on_parent_category_id_and_name (parent_category_id,name) UNIQUE -# index_categories_on_topic_count (topic_count) +# index_categories_on_email_in (email_in) UNIQUE +# index_categories_on_topic_count (topic_count) +# unique_index_categories_on_name (name) UNIQUE # diff --git a/app/models/category_search_data.rb b/app/models/category_search_data.rb index 269ab3f137f..bfbd1b316a4 100644 --- a/app/models/category_search_data.rb +++ b/app/models/category_search_data.rb @@ -10,6 +10,8 @@ end # # category_id :integer not null, primary key # search_data :tsvector +# raw_data :text +# locale :text # # Indexes # diff --git a/app/models/post.rb b/app/models/post.rb index 649168173d8..89b76d4a63a 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -587,6 +587,8 @@ end # cook_method :integer default(1), not null # wiki :boolean default(FALSE), not null # baked_at :datetime +# baked_version :integer +# hidden_at :datetime # # Indexes # diff --git a/app/models/post_search_data.rb b/app/models/post_search_data.rb index 8ebab6ed79c..1611df2718b 100644 --- a/app/models/post_search_data.rb +++ b/app/models/post_search_data.rb @@ -10,6 +10,8 @@ end # # post_id :integer not null, primary key # search_data :tsvector +# raw_data :text +# locale :string(255) # # Indexes # diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index ba6ecff670f..ef27432a8cd 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -227,6 +227,7 @@ end # # Indexes # +# index_topic_links_on_post_id (post_id) # index_topic_links_on_topic_id (topic_id) # unique_post_links (topic_id,post_id,url) UNIQUE # diff --git a/app/models/user.rb b/app/models/user.rb index 6c177f71780..6d8d89f3cab 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -495,7 +495,13 @@ class User < ActiveRecord::Base end def featured_user_badges - user_badges.joins(:badge).order('badges.badge_type_id ASC, badges.grant_count ASC').includes(:user, :granted_by, badge: :badge_type).limit(3) + user_badges + .joins(:badge) + .order('badges.badge_type_id ASC, badges.grant_count ASC') + .includes(:user, :granted_by, badge: :badge_type) + .where("user_badges.id in (select min(u2.id) + from user_badges u2 where u2.user_id = ? group by u2.badge_id)", id) + .limit(3) end def self.count_by_signup_date(sinceDaysAgo=30) @@ -605,10 +611,23 @@ 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 update_tracked_topics diff --git a/app/models/user_badge.rb b/app/models/user_badge.rb index 9802f9e6d5b..3d5e3bc1421 100644 --- a/app/models/user_badge.rb +++ b/app/models/user_badge.rb @@ -45,5 +45,4 @@ end # Indexes # # index_user_badges_on_badge_id_and_user_id (badge_id,user_id) -# index_user_badges_on_user_id (user_id) # diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb index 1cc3c48bfdc..1740e3413a2 100644 --- a/app/models/user_profile.rb +++ b/app/models/user_profile.rb @@ -3,6 +3,7 @@ class UserProfile < ActiveRecord::Base validates :user, presence: true before_save :cook + after_save :assign_autobiographer def bio_excerpt excerpt = PrettyText.excerpt(bio_cooked, 350) @@ -35,6 +36,14 @@ class UserProfile < ActiveRecord::Base self.save! end + protected + + def assign_autobiographer + if bio_raw_changed? + user.grant_autobiographer + end + end + private def cook @@ -51,10 +60,11 @@ end # # Table name: user_profiles # -# user_id :integer not null, primary key -# bio_cooked :text -# bio_raw :text -# location :string(255) -# website :string(255) -# profile_background :string(255) +# user_id :integer not null, primary key +# location :string(255) +# website :string(255) +# bio_raw :text +# bio_cooked :text +# dismissed_banner_key :integer +# profile_background :string(255) # diff --git a/app/models/user_search_data.rb b/app/models/user_search_data.rb index 0ca6c1ca568..fab53372915 100644 --- a/app/models/user_search_data.rb +++ b/app/models/user_search_data.rb @@ -9,6 +9,8 @@ end # # user_id :integer not null, primary key # search_data :tsvector +# raw_data :text +# locale :text # # Indexes # diff --git a/app/services/badge_granter.rb b/app/services/badge_granter.rb index 1b543dcc121..dfec5d9c97e 100644 --- a/app/services/badge_granter.rb +++ b/app/services/badge_granter.rb @@ -55,49 +55,36 @@ class BadgeGranter Jobs.enqueue(:update_badges, args) end + def self.backfill(badge) + return unless badge.query.present? - def self.backfill_like_badges - Badge.like_badge_info.each do |info| - sql = " - DELETE FROM user_badges - WHERE badge_id = :id AND - NOT EXISTS (SELECT 1 FROM posts p - JOIN topics t on p.topic_id = t.id - WHERE p.deleted_at IS NULL AND - t.deleted_at IS NULL AND - t.visible AND - post_id = p.id AND - p.like_count >= :count - ) - " + post_clause = badge.target_posts ? "AND q.post_id = ub.post_id" : "" + post_id_field = badge.target_posts ? "q.post_id" : "NULL" - Badge.exec_sql(sql, info) + sql = "DELETE FROM user_badges + WHERE id in ( + SELECT ub.id + FROM user_badges ub + LEFT JOIN ( #{badge.query} ) q + ON q.user_id = ub.user_id + #{post_clause} + WHERE ub.id = :id AND q.user_id IS NULL + )" - sql = " - INSERT INTO user_badges(badge_id, user_id, granted_at, granted_by_id, post_id) - SELECT :id, p.user_id, :now, -1, p.id - FROM posts p - JOIN topics t on p.topic_id = t.id - WHERE p.deleted_at IS NULL AND - t.deleted_at IS NULL AND - t.visible AND - p.like_count >= :count AND - NOT EXISTS (SELECT 1 FROM user_badges ub - WHERE ub.post_id = p.id AND - ub.badge_id = :id AND - ub.user_id = p.user_id) - " + Badge.exec_sql(sql, id: badge.id) - Badge.exec_sql(sql, info.merge(now: Time.now)) + 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} + FROM ( #{badge.query} ) q + LEFT JOIN user_badges ub ON + ub.id = :id AND ub.user_id = q.user_id + #{post_clause} + WHERE ub.id IS NULL" - sql = " - UPDATE badges b - SET grant_count = (SELECT COUNT(*) FROM user_badges WHERE badge_id = :id) - WHERE b.id = :id - " + Badge.exec_sql(sql, id: badge.id) + + badge.reset_grant_count! - Badge.exec_sql(sql, info) - end end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6b847934f8d..ca64d397922 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1947,6 +1947,9 @@ en: welcome: name: Welcome description: Received a like. + autobiographer: + name: Autobiographer + description: Filled user profile information. nice_post: name: Nice Post description: Received 10 likes on a post. This badge can be granted multiple times. diff --git a/db/fixtures/601_badges.rb b/db/fixtures/601_badges.rb index e4479ffc860..f09c4cd8675 100644 --- a/db/fixtures/601_badges.rb +++ b/db/fixtures/601_badges.rb @@ -6,49 +6,35 @@ trust_level_badges = [ {id: 4, name: "Elder", type: 1} ] -backfill_trust_level_badges = false - trust_level_badges.each do |spec| - backfill_trust_level_badges ||= Badge.find_by(id: spec[:id]).nil? - Badge.seed do |b| b.id = spec[:id] b.name = spec[:name] b.badge_type_id = spec[:type] + b.query = Badge::Queries.trust_level(spec[:id]) end end -if backfill_trust_level_badges - puts "Backfilling trust level badges!" - - - Badge.trust_level_badge_ids.each do |badge_id| - sql = <= :trust_level AND - id NOT IN (SELECT user_id FROM user_badges WHERE badge_id = :badge_id) AND - id <> :system_id -SQL - User.exec_sql(sql, badge_id: badge_id, now: Time.now, system_id: Discourse.system_user.id, trust_level: badge_id) - - end - - Badge.where(id: Badge.trust_level_badge_ids).each {|badge| badge.reset_grant_count! } +Badge.seed do |b| + b.id = Badge::Welcome + b.name = "Welcome" + b.badge_type_id = 3 + b.multiple_grant = false + b.target_posts = true + b.query = Badge::Queries::Welcome end + +Badge.seed do |b| + b.id = Badge::Autobiographer + b.name = "Autobiographer" + b.badge_type_id = 3 + b.multiple_grant = false + b.query = Badge::Queries::Autobiographer +end + # # Like system badges. like_badges = [ - {id: 5, name: "Welcome", type: 3, multiple: false}, {id: 6, name: "Nice Post", type: 3, multiple: true}, {id: 7, name: "Good Post", type: 2, multiple: true}, {id: 8, name: "Great Post", type: 1, multiple: true} @@ -60,14 +46,7 @@ like_badges.each do |spec| b.name = spec[:name] b.badge_type_id = spec[:type] b.multiple_grant = spec[:multiple] - end -end - -# Create an example badge if one does not already exist. -if Badge.find_by(id: 101).nil? - Badge.seed do |b| - b.id = 101 - b.name = "Example Badge" - b.badge_type_id = 3 + b.target_posts = true + b.query = Badge::Queries.like_badge(Badge.like_badge_counts[spec[:id]]) end end diff --git a/db/migrate/20140703022838_add_fields_to_badges.rb b/db/migrate/20140703022838_add_fields_to_badges.rb new file mode 100644 index 00000000000..69cfd32ec26 --- /dev/null +++ b/db/migrate/20140703022838_add_fields_to_badges.rb @@ -0,0 +1,7 @@ +class AddFieldsToBadges < ActiveRecord::Migration + def change + add_column :badges, :listable, :boolean, default: true + add_column :badges, :target_posts, :boolean, default: false + add_column :badges, :query, :text + end +end diff --git a/spec/services/badge_granter_spec.rb b/spec/services/badge_granter_spec.rb index 6fc10efb93b..c16f1b3782e 100644 --- a/spec/services/badge_granter_spec.rb +++ b/spec/services/badge_granter_spec.rb @@ -5,14 +5,31 @@ describe BadgeGranter do let(:badge) { Fabricate(:badge) } let(:user) { Fabricate(:user) } - before do - SiteSetting.enable_badges = true - end + describe 'backfill' do + + it 'has no broken badge queries' do + Badge.all.each do |b| + BadgeGranter.backfill(b) + end + end + + it 'can backfill the welcome badge' do + post = Fabricate(:post) + user2 = Fabricate(:user) + PostAction.act(user2, post, PostActionType.types[:like]) + + UserBadge.destroy_all + BadgeGranter.backfill(Badge.find(Badge::Welcome)) + + b = UserBadge.first + b.user_id.should == post.user_id + b.post_id.should == post.id + end - describe 'backfill like badges' do it 'should grant missing badges' do post = Fabricate(:post, like_count: 30) - BadgeGranter.backfill_like_badges + BadgeGranter.backfill(Badge.find(Badge::NicePost)) + BadgeGranter.backfill(Badge.find(Badge::GoodPost)) # TODO add welcome post.user.user_badges.pluck(:badge_id).sort.should == [Badge::NicePost,Badge::GoodPost] @@ -22,6 +39,21 @@ 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 @@ -111,11 +143,11 @@ describe BadgeGranter do BadgeGranter.update_badges(action: :post_like, post_id: post.id) UserBadge.find_by(user_id: user.id, badge_id: 7).should_not be_nil # Great post badge - post.update_attributes like_count: 100 + post.update_attributes like_count: 50 BadgeGranter.update_badges(action: :post_like, post_id: post.id) UserBadge.find_by(user_id: user.id, badge_id: 8).should_not be_nil # Revoke badges on unlike - post.update_attributes like_count: 99 + post.update_attributes like_count: 49 BadgeGranter.update_badges(action: :post_like, post_id: post.id) UserBadge.find_by(user_id: user.id, badge_id: 8).should be_nil end