From 545dbfc07ecd955706afff39d01c11d307b07eec Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Fri, 31 May 2013 17:38:28 -0400 Subject: [PATCH] New Feature: Staff can choose to "Take Action" when flagging to immediately reach hiding thresholds. --- .../flag_action_type_controller.js | 48 +++++++++++ .../discourse/controllers/flag_controller.js | 84 ++++++++----------- .../discourse/controllers/modal_controller.js | 10 +++ .../discourse/mixins/modal_functionality.js | 10 +++ .../discourse/models/action_summary.js | 5 +- .../discourse/models/post_action_type.js | 32 ++----- .../discourse/routes/topic_route.js | 4 +- .../modal/archetype_options.js.handlebars | 6 +- .../templates/modal/flag.js.handlebars | 56 ++++++------- .../templates/modal/invite.js.handlebars | 2 +- .../modal/invite_private.js.handlebars | 2 +- .../templates/modal/modal.js.handlebars | 2 +- .../modal/not_activated.js.handlebars | 2 +- app/controllers/post_actions_controller.rb | 6 +- app/models/post_action.rb | 23 ++--- config/locales/client.en.yml | 1 + ...6_add_staff_took_action_to_post_actions.rb | 5 ++ .../post_actions_controller_spec.rb | 21 ++++- spec/models/post_action_spec.rb | 10 +-- 19 files changed, 194 insertions(+), 135 deletions(-) create mode 100644 app/assets/javascripts/discourse/controllers/flag_action_type_controller.js create mode 100644 db/migrate/20130531210816_add_staff_took_action_to_post_actions.rb diff --git a/app/assets/javascripts/discourse/controllers/flag_action_type_controller.js b/app/assets/javascripts/discourse/controllers/flag_action_type_controller.js new file mode 100644 index 000000000..c099bd022 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/flag_action_type_controller.js @@ -0,0 +1,48 @@ +/** + Supports logic for flags in the modal + + @class FlagActionTypeController + @extends Discourse.ObjectController + @namespace Discourse + @module Discourse +**/ +Discourse.FlagActionTypeController = Discourse.ObjectController.extend({ + needs: ['flag'], + + message: Em.computed.alias('controllers.flag.message'), + + customPlaceholder: function(){ + return Em.String.i18n("flagging.custom_placeholder_" + this.get('name_key')); + }.property('name_key'), + + formattedName: function(){ + return this.get('name').replace("{{username}}", this.get('controllers.flag.username')); + }.property('name'), + + selected: function() { + return this.get('model') === this.get('controllers.flag.selected'); + }.property('controllers.flag.selected'), + + showMessageInput: Em.computed.and('is_custom_flag', 'selected'), + showDescription: Em.computed.not('showMessageInput'), + + customMessageLengthClasses: function() { + return (this.get('message.length') < Discourse.PostActionType.MIN_MESSAGE_LENGTH) ? "too-short" : "ok" + }.property('message.length'), + + customMessageLength: function() { + var len = this.get('message.length') || 0; + var minLen = Discourse.PostActionType.MIN_MESSAGE_LENGTH; + if (len === 0) { + return Em.String.i18n("flagging.custom_message.at_least", { n: minLen }); + } else if (len < minLen) { + return Em.String.i18n("flagging.custom_message.more", { n: minLen - len }); + } else { + return Em.String.i18n("flagging.custom_message.left", { + n: Discourse.PostActionType.MAX_MESSAGE_LENGTH - len + }); + } + }.property('message.length') + +}); + diff --git a/app/assets/javascripts/discourse/controllers/flag_controller.js b/app/assets/javascripts/discourse/controllers/flag_controller.js index fe014fb3d..0d1d6c85f 100644 --- a/app/assets/javascripts/discourse/controllers/flag_controller.js +++ b/app/assets/javascripts/discourse/controllers/flag_controller.js @@ -9,70 +9,58 @@ **/ Discourse.FlagController = Discourse.ObjectController.extend(Discourse.ModalFunctionality, { - // trick to bind user / post to flag - boundFlags: function() { - var _this = this; - var original = this.get('flagsAvailable'); - if(original){ - return $.map(original, function(v){ - var b = Discourse.BoundPostActionType.create(v); - b.set('post', _this.get('model')); - return b; - }); - } - }.property('flagsAvailable.@each'), - changePostActionType: function(action) { - if (this.get('postActionTypeId') === action.id) return false; - - this.get('boundFlags').setEach('selected', false); - action.set('selected', true); - - this.set('postActionTypeId', action.id); - this.set('isCustomFlag', action.is_custom_flag); this.set('selected', action); - return false; }, - showSubmit: function() { - if (this.get('postActionTypeId')) { - if (this.get('isCustomFlag')) { - var m = this.get('selected.message'); - return m && m.length >= 10 && m.length <= 500; - } else { - return true; - } + submitEnabled: function() { + var selected = this.get('selected'); + if (!selected) return false; + + if (selected.get('is_custom_flag')) { + var len = this.get('message.length') || 0; + return len >= Discourse.PostActionType.MIN_MESSAGE_LENGTH && + len <= Discourse.PostActionType.MAX_MESSAGE_LENGTH; } - return false; - }.property('isCustomFlag', 'selected.customMessageLength', 'postActionTypeId'), + return true; + }.property('selected.is_custom_flag', 'message.length'), + + submitDisabled: Em.computed.not('submitEnabled'), + + // Staff accounts can "take action" + canTakeAction: function() { + // We can only take actions on non-custom flags + if (this.get('selected.is_custom_flag')) return false; + return Discourse.User.current('staff'); + }.property('selected.is_custom_flag'), submitText: function(){ - var action = this.get('selected'); if (this.get('selected.is_custom_flag')) { return Em.String.i18n("flagging.notify_action"); } else { return Em.String.i18n("flagging.action"); } - }.property('selected'), + }.property('selected.is_custom_flag'), - createFlag: function() { - var _this = this; + takeAction: function() { + this.createFlag({takeAction: true}) + this.set('hidden', true); + }, - var action = this.get('selected'); - var postAction = this.get('actionByName.' + (action.get('name_key'))); + createFlag: function(opts) { + var flagController = this; + var postAction = this.get('actionByName.' + this.get('selected.name_key')); + var params = this.get('selected.is_custom_flag') ? {message: this.get('message')} : {} - var actionType = Discourse.Site.instance().postActionTypeById(this.get('postActionTypeId')); - if (postAction) { - postAction.act({ - message: action.get('message') - }).then(function() { - return $('#discourse-modal').modal('hide'); - }, function(errors) { - return _this.displayErrors(errors); - }); - } - return false; + if (opts) params = $.extend(params, opts); + + postAction.act(params).then(function() { + flagController.closeModal(); + }, function(errors) { + flagController.displayErrors(errors); + }); } + }); diff --git a/app/assets/javascripts/discourse/controllers/modal_controller.js b/app/assets/javascripts/discourse/controllers/modal_controller.js index 0659c1453..9ad491122 100644 --- a/app/assets/javascripts/discourse/controllers/modal_controller.js +++ b/app/assets/javascripts/discourse/controllers/modal_controller.js @@ -8,6 +8,16 @@ **/ Discourse.ModalController = Discourse.Controller.extend({ + /** + Close the modal. + + @method closeModal + **/ + closeModal: function() { + // Currently uses jQuery to hide it. + $('#discourse-modal').modal('hide'); + } + }); diff --git a/app/assets/javascripts/discourse/mixins/modal_functionality.js b/app/assets/javascripts/discourse/mixins/modal_functionality.js index acb228204..7ddac0f78 100644 --- a/app/assets/javascripts/discourse/mixins/modal_functionality.js +++ b/app/assets/javascripts/discourse/mixins/modal_functionality.js @@ -21,6 +21,16 @@ Discourse.ModalFunctionality = Em.Mixin.create({ message: message, messageClass: messageClass })); + }, + + /** + Close the modal. + + @method closeModal + **/ + closeModal: function() { + // Currently uses jQuery to hide it. + this.get('controllers.modal').closeModal(); } }); diff --git a/app/assets/javascripts/discourse/models/action_summary.js b/app/assets/javascripts/discourse/models/action_summary.js index e21152cea..7b6cc0359 100644 --- a/app/assets/javascripts/discourse/models/action_summary.js +++ b/app/assets/javascripts/discourse/models/action_summary.js @@ -37,6 +37,8 @@ Discourse.ActionSummary = Discourse.Model.extend({ // Perform this action act: function(opts) { + if (!opts) opts = {}; + var action = this.get('actionType.name_key'); // Mark it as acted @@ -63,7 +65,8 @@ Discourse.ActionSummary = Discourse.Model.extend({ data: { id: this.get('post.id'), post_action_type_id: this.get('id'), - message: (opts ? opts.message : void 0) || "" + message: opts.message, + take_action: opts.takeAction } }).then(null, function (error) { actionSummary.removeAction(); diff --git a/app/assets/javascripts/discourse/models/post_action_type.js b/app/assets/javascripts/discourse/models/post_action_type.js index 25bd59371..4ff10880a 100644 --- a/app/assets/javascripts/discourse/models/post_action_type.js +++ b/app/assets/javascripts/discourse/models/post_action_type.js @@ -6,31 +6,9 @@ @namespace Discourse @module Discourse **/ -Discourse.PostActionType = Discourse.Model.extend({ -}); +Discourse.PostActionType = Discourse.Model.extend({}); -Discourse.BoundPostActionType = Discourse.PostActionType.extend({ - customPlaceholder: function(){ - return Em.String.i18n("flagging.custom_placeholder_" + this.get('name_key')); - }.property('name_key'), - - formattedName: function(){ - return this.get('name').replace("{{username}}", this.get('post.username')); - }.property('name'), - - messageChanged: function() { - var len, message, minLen, _ref; - minLen = 10; - len = ((_ref = this.get('message')) ? _ref.length : void 0) || 0; - this.set("customMessageLengthClasses", "too-short custom-message-length"); - if (len === 0) { - message = Em.String.i18n("flagging.custom_message.at_least", { n: minLen }); - } else if (len < minLen) { - message = Em.String.i18n("flagging.custom_message.more", { n: minLen - len }); - } else { - message = Em.String.i18n("flagging.custom_message.left", { n: 500 - len }); - this.set("customMessageLengthClasses", "ok custom-message-length"); - } - this.set("customMessageLength", message); - }.observes("message") -}); +Discourse.PostActionType.reopenClass({ + MIN_MESSAGE_LENGTH: 10, + MAX_MESSAGE_LENGTH: 500 +}) diff --git a/app/assets/javascripts/discourse/routes/topic_route.js b/app/assets/javascripts/discourse/routes/topic_route.js index 5129bdabb..61f929007 100644 --- a/app/assets/javascripts/discourse/routes/topic_route.js +++ b/app/assets/javascripts/discourse/routes/topic_route.js @@ -13,9 +13,7 @@ Discourse.TopicRoute = Discourse.Route.extend({ showFlags: function(post) { Discourse.Route.showModal(this, 'flag', post); - this.controllerFor('flag').setProperties({ - postActionTypeId: null - }); + this.controllerFor('flag').setProperties({ selected: null }); }, showAutoClose: function() { diff --git a/app/assets/javascripts/discourse/templates/modal/archetype_options.js.handlebars b/app/assets/javascripts/discourse/templates/modal/archetype_options.js.handlebars index 1c638975c..80fed824d 100644 --- a/app/assets/javascripts/discourse/templates/modal/archetype_options.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/archetype_options.js.handlebars @@ -1,8 +1,8 @@ -
+
{{view Discourse.ArchetypeOptionsView archetypeBinding="view.archetype"}}
- -
\ No newline at end of file + +
\ No newline at end of file diff --git a/app/assets/javascripts/discourse/templates/modal/flag.js.handlebars b/app/assets/javascripts/discourse/templates/modal/flag.js.handlebars index 4e0b50543..190a7c64b 100644 --- a/app/assets/javascripts/discourse/templates/modal/flag.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/flag.js.handlebars @@ -1,36 +1,30 @@
- {{#if flagsAvailable}} -
- {{#each boundFlags}} -
- - {{#if is_custom_flag}} - {{#if selected}} - {{textarea name="message" class="flag-message" placeholder=customPlaceholder value=message}} -
{{customMessageLength}}
- {{/if}} + + + {{#each flagsAvailable itemController="flagActionType"}} +
+
- {{/each}} - - {{else}} - {{i18n flagging.cant}} + + {{#if showMessageInput}} + {{textarea name="message" class="flag-message" placeholder=customPlaceholder value=message}} +
{{customMessageLength}}
+ {{/if}} +
+ {{else}} + {{i18n flagging.cant}} + {{/each}} + + +
+
+ + + {{#if canTakeAction}} + {{/if}}
-{{#if showSubmit}} -
- -
-{{/if}} diff --git a/app/assets/javascripts/discourse/templates/modal/invite.js.handlebars b/app/assets/javascripts/discourse/templates/modal/invite.js.handlebars index 5d8e3cfb1..e64b1d59e 100644 --- a/app/assets/javascripts/discourse/templates/modal/invite.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/invite.js.handlebars @@ -17,7 +17,7 @@
{{#if finished}} - + {{else}} {{/if}} diff --git a/app/assets/javascripts/discourse/templates/modal/invite_private.js.handlebars b/app/assets/javascripts/discourse/templates/modal/invite_private.js.handlebars index 0a40047b6..1f073c693 100644 --- a/app/assets/javascripts/discourse/templates/modal/invite_private.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/invite_private.js.handlebars @@ -17,7 +17,7 @@
{{#if finished}} - + {{else}} {{/if}} diff --git a/app/assets/javascripts/discourse/templates/modal/modal.js.handlebars b/app/assets/javascripts/discourse/templates/modal/modal.js.handlebars index ded7d6c0d..b55d4e8c6 100644 --- a/app/assets/javascripts/discourse/templates/modal/modal.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/modal.js.handlebars @@ -1,5 +1,5 @@
- +

{{title}}

diff --git a/app/assets/javascripts/discourse/templates/modal/not_activated.js.handlebars b/app/assets/javascripts/discourse/templates/modal/not_activated.js.handlebars index d4ed00665..c536081ae 100644 --- a/app/assets/javascripts/discourse/templates/modal/not_activated.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/not_activated.js.handlebars @@ -7,5 +7,5 @@ {{/if}}
- +
\ No newline at end of file diff --git a/app/controllers/post_actions_controller.rb b/app/controllers/post_actions_controller.rb index ecda9ffa3..bc7819344 100644 --- a/app/controllers/post_actions_controller.rb +++ b/app/controllers/post_actions_controller.rb @@ -9,7 +9,11 @@ class PostActionsController < ApplicationController def create guardian.ensure_post_can_act!(@post, PostActionType.types[@post_action_type_id]) - post_action = PostAction.act(current_user, @post, @post_action_type_id, params[:message]) + args = {} + args[:message] = params[:message] if params[:message].present? + args[:take_action] = true if guardian.is_staff? and params[:take_action] == 'true' + + post_action = PostAction.act(current_user, @post, @post_action_type_id, args) if post_action.blank? || post_action.errors.present? render_json_error(post_action) diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 02b9696de..ca65b67ad 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -8,7 +8,7 @@ class PostAction < ActiveRecord::Base include RateLimiter::OnCreateRecord include Trashable - attr_accessible :post_action_type_id, :post_id, :user_id, :post, :user, :post_action_type, :message, :related_post_id + attr_accessible :post_action_type_id, :post_id, :user_id, :post, :user, :post_action_type, :message, :related_post_id, :staff_took_action belongs_to :post belongs_to :user @@ -70,11 +70,11 @@ class PostAction < ActiveRecord::Base update_flagged_posts_count end - def self.act(user, post, post_action_type_id, message = nil) + def self.act(user, post, post_action_type_id, opts={}) begin title, target_usernames, target_group_names, subtype, body = nil - if message + if opts[:message] [:notify_moderators, :notify_user].each do |k| if post_action_type_id == PostActionType.types[k] if k == :notify_moderators @@ -85,7 +85,7 @@ class PostAction < ActiveRecord::Base title = I18n.t("post_action_types.#{k}.email_title", title: post.topic.title) body = I18n.t("post_action_types.#{k}.email_body", - message: message, + message: opts[:message], link: "#{Discourse.base_url}#{post.url}") subtype = k == :notify_moderators ? TopicSubtype.notify_moderators : TopicSubtype.notify_user end @@ -103,10 +103,12 @@ class PostAction < ActiveRecord::Base raw: body ).create.id end + create( post_id: post.id, user_id: user.id, post_action_type_id: post_action_type_id, - message: message, + message: opts[:message], + staff_took_action: opts[:take_action] || false, related_post_id: related_post_id ) rescue ActiveRecord::RecordNotUnique # can happen despite being .create @@ -177,13 +179,13 @@ class PostAction < ActiveRecord::Base # can weigh flags differently. def self.flag_counts_for(post_id) flag_counts = exec_sql("SELECT SUM(CASE - WHEN pa.deleted_at IS NULL AND u.admin THEN :flags_required_to_hide_post - WHEN pa.deleted_at IS NULL AND (NOT u.admin) THEN 1 + WHEN pa.deleted_at IS NULL AND pa.staff_took_action THEN :flags_required_to_hide_post + WHEN pa.deleted_at IS NULL AND (NOT pa.staff_took_action) THEN 1 ELSE 0 END) AS new_flags, SUM(CASE - WHEN pa.deleted_at IS NOT NULL AND u.admin THEN :flags_required_to_hide_post - WHEN pa.deleted_at IS NOT NULL AND (NOT u.admin) THEN 1 + WHEN pa.deleted_at IS NOT NULL AND pa.staff_took_action THEN :flags_required_to_hide_post + WHEN pa.deleted_at IS NOT NULL AND (NOT pa.staff_took_action) THEN 1 ELSE 0 END) AS old_flags FROM post_actions AS pa @@ -234,7 +236,8 @@ class PostAction < ActiveRecord::Base reason = old_flags > 0 ? Post.hidden_reasons[:flag_threshold_reached_again] : Post.hidden_reasons[:flag_threshold_reached] Post.update_all(["hidden = true, hidden_reason_id = COALESCE(hidden_reason_id, ?)", reason], id: post_id) Topic.update_all({ visible: false }, - ["id = :topic_id AND NOT EXISTS(SELECT 1 FROM POSTS WHERE topic_id = :topic_id AND NOT hidden)", topic_id: post.topic_id]) + ["id = :topic_id AND NOT EXISTS(SELECT 1 FROM POSTS WHERE topic_id = :topic_id AND NOT hidden)", + topic_id: post.topic_id]) # inform user if post.user diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index de7761ba4..9c608f702 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -831,6 +831,7 @@ en: flagging: title: 'Why are you flagging this post?' action: 'Flag Post' + take_action: "Take Action" notify_action: 'Notify' cant: "Sorry, you can't flag this post at this time." custom_placeholder_notify_user: "Why does this post require you to speak to this user directly and privately? Be specific, be constructive, and always be kind." diff --git a/db/migrate/20130531210816_add_staff_took_action_to_post_actions.rb b/db/migrate/20130531210816_add_staff_took_action_to_post_actions.rb new file mode 100644 index 000000000..3b6c2c368 --- /dev/null +++ b/db/migrate/20130531210816_add_staff_took_action_to_post_actions.rb @@ -0,0 +1,5 @@ +class AddStaffTookActionToPostActions < ActiveRecord::Migration + def change + add_column :post_actions, :staff_took_action, :boolean, default: false, null: false + end +end diff --git a/spec/controllers/post_actions_controller_spec.rb b/spec/controllers/post_actions_controller_spec.rb index 29a782a7f..3c0fed3d2 100644 --- a/spec/controllers/post_actions_controller_spec.rb +++ b/spec/controllers/post_actions_controller_spec.rb @@ -9,7 +9,7 @@ describe PostActionsController do describe 'logged in' do before do - @user = log_in + @user = log_in(:moderator) @post = Fabricate(:post, user: Fabricate(:coding_horror)) end @@ -34,9 +34,26 @@ describe PostActionsController do end it 'allows us to create an post action on a post' do - PostAction.expects(:act).once.with(@user, @post, PostActionType.types[:like], nil) + PostAction.expects(:act).once.with(@user, @post, PostActionType.types[:like], {}) xhr :post, :create, id: @post.id, post_action_type_id: PostActionType.types[:like] end + + it 'passes the message through' do + PostAction.expects(:act).once.with(@user, @post, PostActionType.types[:like], {message: 'action message goes here'}) + xhr :post, :create, id: @post.id, post_action_type_id: PostActionType.types[:like], message: 'action message goes here' + end + + it 'passes take_action through' do + PostAction.expects(:act).once.with(@user, @post, PostActionType.types[:like], {take_action: true}) + xhr :post, :create, id: @post.id, post_action_type_id: PostActionType.types[:like], take_action: 'true' + end + + it "doesn't pass take_action through if the user isn't staff" do + Guardian.any_instance.stubs(:is_staff?).returns(false) + PostAction.expects(:act).once.with(@user, @post, PostActionType.types[:like], {}) + xhr :post, :create, id: @post.id, post_action_type_id: PostActionType.types[:like], take_action: 'true' + end + end end diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index e79236dcd..66da90ba8 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -21,7 +21,7 @@ describe PostAction do it "notify moderators integration test" do mod = moderator - action = PostAction.act(codinghorror, post, PostActionType.types[:notify_moderators], "this is my special long message"); + action = PostAction.act(codinghorror, post, PostActionType.types[:notify_moderators], message: "this is my special long message"); posts = Post.joins(:topic) .select('posts.id, topics.subtype, posts.topic_id') @@ -50,7 +50,7 @@ describe PostAction do it "sends an email to all moderators if selected" do post = build(:post, id: 1000) PostCreator.any_instance.expects(:create).returns(post) - PostAction.act(build(:user), build(:post), PostActionType.types[:notify_moderators], "this is my special message"); + PostAction.act(build(:user), build(:post), PostActionType.types[:notify_moderators], message: "this is my special message"); end end @@ -63,7 +63,7 @@ describe PostAction do it "sends an email to user if selected" do PostCreator.any_instance.expects(:create).returns(build(:post)) - PostAction.act(build(:user), post, PostActionType.types[:notify_user], "this is my special message"); + PostAction.act(build(:user), post, PostActionType.types[:notify_user], message: "this is my special message"); end end end @@ -179,9 +179,9 @@ describe PostAction do flag = PostAction.act(Fabricate(:evil_trout), post, PostActionType.types[:spam]) PostAction.flag_counts_for(post.id).should == [0, 1] - # If an admin flags the post, it is counted higher + # If staff takes action, it is ranked higher admin = Fabricate(:admin) - PostAction.act(admin, post, PostActionType.types[:spam]) + pa = PostAction.act(admin, post, PostActionType.types[:spam], take_action: true) PostAction.flag_counts_for(post.id).should == [0, 8] # If a flag is dismissed