From 15e54c715f798382eabf47472c514a9423576e10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sigur=C3=B0ur=20Gu=C3=B0brandsson?= Date: Wed, 25 Feb 2015 17:23:57 +0000 Subject: [PATCH 1/4] FIX: Added two user badge triggers Created two triggers that trigger events when a badge is granted or removed. Trigger 1: user_badge_granted Variable - badge_id Variable - user_id Trigger 2: user_badge_removed Variable - badge_id Variable - user_id --- app/models/user_badge.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/user_badge.rb b/app/models/user_badge.rb index 5c6894d4d9a..b176f27b9d0 100644 --- a/app/models/user_badge.rb +++ b/app/models/user_badge.rb @@ -12,10 +12,12 @@ class UserBadge < ActiveRecord::Base after_create do Badge.increment_counter 'grant_count', self.badge_id + DiscourseEvent.trigger(:user_badge_granted, badge_id, user_id) end after_destroy do Badge.decrement_counter 'grant_count', self.badge_id + DiscourseEvent.trigger(:user_badge_removed, badge_id, user_id) end end From bee3bbdc05ed87b2226dc9a5b293bcf5789e8f02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sigur=C3=B0ur=20Gu=C3=B0brandsson?= Date: Thu, 26 Feb 2015 00:50:58 +0000 Subject: [PATCH 2/4] FIX: the badge triggers broke Needed to add self. for the badge trigger variables, otherwise it breaks everything ;) --- app/models/user_badge.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/user_badge.rb b/app/models/user_badge.rb index b176f27b9d0..f78ed824bb0 100644 --- a/app/models/user_badge.rb +++ b/app/models/user_badge.rb @@ -12,12 +12,12 @@ class UserBadge < ActiveRecord::Base after_create do Badge.increment_counter 'grant_count', self.badge_id - DiscourseEvent.trigger(:user_badge_granted, badge_id, user_id) + DiscourseEvent.trigger(:user_badge_granted, self.badge_id, self.user_id) end after_destroy do Badge.decrement_counter 'grant_count', self.badge_id - DiscourseEvent.trigger(:user_badge_removed, badge_id, user_id) + DiscourseEvent.trigger(:user_badge_removed, self.badge_id, self.user_id) end end From 73068d5fa3ba39736d0591c8e92c25dc49545a1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sigur=C3=B0ur=20Gu=C3=B0brandsson?= Date: Thu, 26 Feb 2015 00:55:17 +0000 Subject: [PATCH 3/4] ADD: Spec tests for User Badge triggers NOTE: The DiscourseEvent trigger mechanism is VERY weird. If there are ANY triggers triggered in the chain, you can't only list the one you're looking for, you have to list all triggers in the order they will come. Example: line 98-100 :user_created and :user_verified are triggers that are introduced in PR #3237 so if this PR is accepted but not PR #3237 then lines 98-99 need to be removed. --- .../user_badges_controller_spec.rb | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/spec/controllers/user_badges_controller_spec.rb b/spec/controllers/user_badges_controller_spec.rb index 1bfb3ad31e6..84225b39c64 100644 --- a/spec/controllers/user_badges_controller_spec.rb +++ b/spec/controllers/user_badges_controller_spec.rb @@ -5,7 +5,7 @@ describe UserBadgesController do let(:badge) { Fabricate(:badge) } context 'index' do - it 'doest not leak private info' do + it 'does 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) @@ -63,23 +63,13 @@ describe UserBadgesController do it 'grants badges from staff' do admin = Fabricate(:admin) - post = create_post - log_in_user admin - StaffActionLogger.any_instance.expects(:log_badge_grant).once - - xhr :post, :create, badge_id: badge.id, - username: user.username, - reason: Discourse.base_url + post.url - + xhr :post, :create, badge_id: badge.id, username: user.username expect(response.status).to eq(200) - user_badge = UserBadge.find_by(user: user, badge: badge) - expect(user_badge).to be_present expect(user_badge.granted_by).to eq(admin) - expect(user_badge.post_id).to eq(post.id) end it 'does not grant badges from regular api calls' do @@ -97,6 +87,19 @@ describe UserBadgesController do expect(user_badge).to be_present expect(user_badge.granted_by).to eq(Discourse.system_user) end + + it 'will trigger :user_badge_granted' do + log_in :admin + + # Make sure our extensibility points are triggered + # Stupid DiscourseEvent.clear doesn't work properly .. requires you to list ALL triggers in the chain! + # If there are future triggers added in the user creation chain, they need to be added anywhere you create a user and monitor ANY trigger. + # Perhaps DiscourseEvent needs a little fix so it doesn't break everything once you add a trigger in the chain. + DiscourseEvent.expects(:trigger).with(:user_created, anything).once + DiscourseEvent.expects(:trigger).with(:user_verified, anything).once + DiscourseEvent.expects(:trigger).with(:user_badge_granted, anything, anything).once + xhr :post, :create, badge_id: badge.id, username: user.username + end end context 'destroy' do @@ -114,5 +117,11 @@ describe UserBadgesController do expect(response.status).to eq(200) expect(UserBadge.find_by(id: user_badge.id)).to eq(nil) end + + it 'will trigger :user_badge_removed' do + log_in :admin + DiscourseEvent.expects(:trigger).with(:user_badge_removed, anything, anything).once + xhr :delete, :destroy, id: user_badge.id + end end end From 83f719fb809b37500ac968a9782b116abc56e715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sigur=C3=B0ur=20Gu=C3=B0brandsson?= Date: Thu, 26 Feb 2015 01:24:21 +0000 Subject: [PATCH 4/4] FIX: Cleaned the commit Only changing the code I changed, not other tests. --- spec/controllers/user_badges_controller_spec.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/spec/controllers/user_badges_controller_spec.rb b/spec/controllers/user_badges_controller_spec.rb index 84225b39c64..ac650b20fb6 100644 --- a/spec/controllers/user_badges_controller_spec.rb +++ b/spec/controllers/user_badges_controller_spec.rb @@ -63,13 +63,23 @@ describe UserBadgesController do it 'grants badges from staff' do admin = Fabricate(:admin) + post = create_post + log_in_user admin + StaffActionLogger.any_instance.expects(:log_badge_grant).once - xhr :post, :create, badge_id: badge.id, username: user.username + + xhr :post, :create, badge_id: badge.id, + username: user.username, + reason: Discourse.base_url + post.url + expect(response.status).to eq(200) + user_badge = UserBadge.find_by(user: user, badge: badge) + expect(user_badge).to be_present expect(user_badge.granted_by).to eq(admin) + expect(user_badge.post_id).to eq(post.id) end it 'does not grant badges from regular api calls' do