From dcaa069bb508ae1ad20e2e00a5d1b3e835604727 Mon Sep 17 00:00:00 2001 From: Vikhyat Korrapati Date: Thu, 20 Mar 2014 01:00:12 +0530 Subject: [PATCH] Log badge grant/revoke to the staff actions log. --- app/controllers/user_badges_controller.rb | 2 +- app/models/user_history.rb | 8 +++-- app/services/badge_granter.rb | 8 ++++- app/services/staff_action_logger.rb | 18 ++++++++++ config/locales/client.en.yml | 2 ++ .../user_badges_controller_spec.rb | 3 ++ spec/services/badge_granter_spec.rb | 6 +++- spec/services/staff_action_logger_spec.rb | 34 +++++++++++++++++++ 8 files changed, 76 insertions(+), 5 deletions(-) diff --git a/app/controllers/user_badges_controller.rb b/app/controllers/user_badges_controller.rb index 2dc333536..ef5bbf334 100644 --- a/app/controllers/user_badges_controller.rb +++ b/app/controllers/user_badges_controller.rb @@ -29,7 +29,7 @@ class UserBadgesController < ApplicationController return end - BadgeGranter.revoke(user_badge) + BadgeGranter.revoke(user_badge, revoked_by: current_user) render json: success_json end diff --git a/app/models/user_history.rb b/app/models/user_history.rb index fccf90cc2..bc4033c2c 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -23,7 +23,9 @@ class UserHistory < ActiveRecord::Base :notified_about_dominating_topic, :suspend_user, :unsuspend_user, - :facebook_no_email) + :facebook_no_email, + :grant_badge, + :revoke_badge) end # Staff actions is a subset of all actions, used to audit actions taken by staff users. @@ -34,7 +36,9 @@ class UserHistory < ActiveRecord::Base :change_site_customization, :delete_site_customization, :suspend_user, - :unsuspend_user] + :unsuspend_user, + :grant_badge, + :revoke_badge] end def self.staff_action_ids diff --git a/app/services/badge_granter.rb b/app/services/badge_granter.rb index 2fb74972d..d164f7d3c 100644 --- a/app/services/badge_granter.rb +++ b/app/services/badge_granter.rb @@ -19,15 +19,21 @@ class BadgeGranter granted_by: @granted_by, granted_at: Time.now) Badge.increment_counter 'grant_count', @badge.id + if @granted_by != Discourse.system_user + StaffActionLogger.new(@granted_by).log_badge_grant(user_badge) + end end user_badge end - def self.revoke(user_badge) + def self.revoke(user_badge, options={}) UserBadge.transaction do user_badge.destroy! Badge.decrement_counter 'grant_count', user_badge.badge.id + if options[:revoked_by] + StaffActionLogger.new(options[:revoked_by]).log_badge_revoke(user_badge) + end end end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index bb9fa1f74..b92deba69 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -74,6 +74,24 @@ class StaffActionLogger })) end + def log_badge_grant(user_badge, opts={}) + raise Discourse::InvalidParameters.new('user_badge is nil') unless user_badge + UserHistory.create( params(opts).merge({ + action: UserHistory.actions[:grant_badge], + target_user_id: user_badge.user_id, + details: user_badge.badge.name + })) + end + + def log_badge_revoke(user_badge, opts={}) + raise Discourse::InvalidParameters.new('user_badge is nil') unless user_badge + UserHistory.create( params(opts).merge({ + action: UserHistory.actions[:revoke_badge], + target_user_id: user_badge.user_id, + details: user_badge.badge.name + })) + end + private def params(opts) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 50b82926c..3cbb8b92e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1484,6 +1484,8 @@ en: delete_site_customization: "delete site customization" suspend_user: "suspend user" unsuspend_user: "unsuspend user" + grant_badge: "grant badge" + revoke_badge: "revoke badge" screened_emails: title: "Screened Emails" description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed." diff --git a/spec/controllers/user_badges_controller_spec.rb b/spec/controllers/user_badges_controller_spec.rb index 4836df3ff..9d3fdbf1f 100644 --- a/spec/controllers/user_badges_controller_spec.rb +++ b/spec/controllers/user_badges_controller_spec.rb @@ -36,6 +36,7 @@ describe UserBadgesController do it 'grants badges from staff' do admin = Fabricate(:admin) log_in_user admin + StaffActionLogger.any_instance.expects(:log_badge_grant).once xhr :post, :create, badge_id: badge.id, username: user.username response.status.should == 200 user_badge = UserBadge.where(user: user, badge: badge).first @@ -51,6 +52,7 @@ describe UserBadgesController do it 'grants badges from master api calls' do api_key = Fabricate(:api_key) + StaffActionLogger.any_instance.expects(:log_badge_grant).never xhr :post, :create, badge_id: badge.id, username: user.username, api_key: api_key.key response.status.should == 200 user_badge = UserBadge.where(user: user, badge: badge).first @@ -71,6 +73,7 @@ describe UserBadgesController do it 'revokes the badge' do log_in :admin + StaffActionLogger.any_instance.expects(:log_badge_revoke).once xhr :delete, :destroy, id: @user_badge.id response.status.should == 200 UserBadge.where(id: @user_badge.id).first.should be_nil diff --git a/spec/services/badge_granter_spec.rb b/spec/services/badge_granter_spec.rb index 1ffcde77f..6801bc969 100644 --- a/spec/services/badge_granter_spec.rb +++ b/spec/services/badge_granter_spec.rb @@ -24,11 +24,13 @@ describe BadgeGranter do it 'sets granted_by if the option is present' do admin = Fabricate(:admin) + StaffActionLogger.any_instance.expects(:log_badge_grant).once user_badge = BadgeGranter.grant(badge, user, granted_by: admin) user_badge.granted_by.should eq(admin) end it 'defaults granted_by to the system user' do + StaffActionLogger.any_instance.expects(:log_badge_grant).never user_badge = BadgeGranter.grant(badge, user) user_badge.granted_by_id.should eq(Discourse.system_user.id) end @@ -47,11 +49,13 @@ describe BadgeGranter do describe 'revoke' do + let(:admin) { Fabricate(:admin) } let!(:user_badge) { BadgeGranter.grant(badge, user) } it 'revokes the badge and decrements grant_count' do badge.reload.grant_count.should eq(1) - BadgeGranter.revoke(user_badge) + StaffActionLogger.any_instance.expects(:log_badge_revoke).with(user_badge) + BadgeGranter.revoke(user_badge, revoked_by: admin) UserBadge.where(user: user, badge: badge).first.should_not be_present badge.reload.grant_count.should eq(0) end diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb index 4ce04aabd..b0466c022 100644 --- a/spec/services/staff_action_logger_spec.rb +++ b/spec/services/staff_action_logger_spec.rb @@ -150,4 +150,38 @@ describe StaffActionLogger do log_record.target_user.should == user end end + + describe "log_badge_grant" do + let(:user) { Fabricate(:user) } + let(:badge) { Fabricate(:badge) } + let(:user_badge) { BadgeGranter.grant(badge, user) } + + it "raises an error when argument is missing" do + expect { logger.log_badge_grant(nil) }.to raise_error(Discourse::InvalidParameters) + end + + it "creates a new UserHistory record" do + log_record = logger.log_badge_grant(user_badge) + log_record.should be_valid + log_record.target_user.should == user + log_record.details.should == badge.name + end + end + + describe "log_badge_revoke" do + let(:user) { Fabricate(:user) } + let(:badge) { Fabricate(:badge) } + let(:user_badge) { BadgeGranter.grant(badge, user) } + + it "raises an error when argument is missing" do + expect { logger.log_badge_revoke(nil) }.to raise_error(Discourse::InvalidParameters) + end + + it "creates a new UserHistory record" do + log_record = logger.log_badge_revoke(user_badge) + log_record.should be_valid + log_record.target_user.should == user + log_record.details.should == badge.name + end + end end