From b4f4cf794bd365b9fd1cfde32a43667856725074 Mon Sep 17 00:00:00 2001 From: cpradio Date: Sun, 3 Apr 2016 11:33:56 -0400 Subject: [PATCH 1/3] Add warning input to flag dialog Added isWarning property pass is_warning along to post_action Added is_warning to possible arguments to receive from post_actions route Only show warning checkbox for staff Only permit the is_warning argument if the user is staff --- .../discourse/controllers/flag-action-type.js.es6 | 1 + .../javascripts/discourse/controllers/flag.js.es6 | 14 +++++++++++++- .../discourse/models/action-summary.js.es6 | 1 + .../javascripts/discourse/templates/modal/flag.hbs | 4 ++++ app/controllers/post_actions_controller.rb | 1 + app/models/post_action.rb | 3 ++- config/locales/client.en.yml | 1 + 7 files changed, 23 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/flag-action-type.js.es6 b/app/assets/javascripts/discourse/controllers/flag-action-type.js.es6 index 4e1ed438c6d..d706a88e1c1 100644 --- a/app/assets/javascripts/discourse/controllers/flag-action-type.js.es6 +++ b/app/assets/javascripts/discourse/controllers/flag-action-type.js.es6 @@ -5,6 +5,7 @@ export default Ember.Controller.extend({ needs: ['flag'], message: Em.computed.alias('controllers.flag.message'), + isWarning: Em.computed.alias('controllers.flag.isWarning'), customPlaceholder: function(){ return I18n.t("flagging.custom_placeholder_" + this.get('model.name_key')); diff --git a/app/assets/javascripts/discourse/controllers/flag.js.es6 b/app/assets/javascripts/discourse/controllers/flag.js.es6 index 5537c7bb6b9..445dd273fa1 100644 --- a/app/assets/javascripts/discourse/controllers/flag.js.es6 +++ b/app/assets/javascripts/discourse/controllers/flag.js.es6 @@ -7,6 +7,7 @@ export default Ember.Controller.extend(ModalFunctionality, { selected: null, flagTopic: null, message: null, + isWarning: false, topicActionByName: null, onShow() { @@ -97,7 +98,7 @@ export default Ember.Controller.extend(ModalFunctionality, { postAction = this.get('topicActionByName.' + this.get('selected.name_key')); } - let params = this.get('selected.is_custom_flag') ? {message: this.get('message')} : {}; + let params = this.get('selected.is_custom_flag') ? {message: this.get('message') } : {}; if (opts) { params = $.extend(params, opts); } this.send('hideModal'); @@ -118,6 +119,11 @@ export default Ember.Controller.extend(ModalFunctionality, { }); }, + createFlagAsWarning() { + this.send('createFlag', {isWarning: true}); + this.set('model.hidden', true); + }, + changePostActionType(action) { this.set('selected', action); }, @@ -133,6 +139,12 @@ export default Ember.Controller.extend(ModalFunctionality, { } }.property('selected.name_key', 'userDetails.can_be_deleted', 'userDetails.can_delete_all_posts'), + canSendWarning: function() { + if (this.get("flagTopic")) return false; + + return (Discourse.User.currentProp('staff') && this.get('selected.name_key') === 'notify_user'); + }.property('selected.name_key'), + usernameChanged: function() { this.set('userDetails', null); this.fetchUserDetails(); diff --git a/app/assets/javascripts/discourse/models/action-summary.js.es6 b/app/assets/javascripts/discourse/models/action-summary.js.es6 index 1a58dc4a0ec..a4f50f52ded 100644 --- a/app/assets/javascripts/discourse/models/action-summary.js.es6 +++ b/app/assets/javascripts/discourse/models/action-summary.js.es6 @@ -59,6 +59,7 @@ export default RestModel.extend({ id: this.get('flagTopic') ? this.get('flagTopic.id') : post.get('id'), post_action_type_id: this.get('id'), message: opts.message, + is_warning: opts.isWarning, take_action: opts.takeAction, flag_topic: this.get('flagTopic') ? true : false }, diff --git a/app/assets/javascripts/discourse/templates/modal/flag.hbs b/app/assets/javascripts/discourse/templates/modal/flag.hbs index 51709ea2dd1..190613c1c84 100644 --- a/app/assets/javascripts/discourse/templates/modal/flag.hbs +++ b/app/assets/javascripts/discourse/templates/modal/flag.hbs @@ -38,6 +38,10 @@
+ {{#if canSendWarning}} + + {{/if}} + {{#if canTakeAction}} {{/if}} diff --git a/app/controllers/post_actions_controller.rb b/app/controllers/post_actions_controller.rb index c23d2ed6f49..324d0b8ad7b 100644 --- a/app/controllers/post_actions_controller.rb +++ b/app/controllers/post_actions_controller.rb @@ -11,6 +11,7 @@ class PostActionsController < ApplicationController args = {} args[:message] = params[:message] if params[:message].present? + args[:is_warning] = params[:is_warning] if params[:is_warning].present? && guardian.is_staff? args[:take_action] = true if guardian.is_staff? && params[:take_action] == 'true' args[:flag_topic] = true if params[:flag_topic] == 'true' diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 08aaae4a0ca..4c7cec96f90 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -215,11 +215,12 @@ SQL title = I18n.t("post_action_types.#{post_action_type}.email_title", title: post.topic.title) body = I18n.t("post_action_types.#{post_action_type}.email_body", message: opts[:message], link: "#{Discourse.base_url}#{post.url}") - + warning = opts[:is_warning] if opts[:is_warning].present? title = title.truncate(255, separator: /\s/) opts = { archetype: Archetype.private_message, + is_warning: warning, title: title, raw: body } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index f3c5ca3f2fd..8749625bbb3 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1753,6 +1753,7 @@ en: action: 'Flag Post' take_action: "Take Action" notify_action: 'Message' + official_warning: 'Official Warning' delete_spammer: "Delete Spammer" delete_confirm: "You are about to delete %{posts} posts and %{topics} topics from this user, remove their account, block signups from their IP address %{ip_address}, and add their email address %{email} to a permanent block list. Are you sure this user is really a spammer?" yes_delete_spammer: "Yes, Delete Spammer" From 95fa34060172529de5cb7eeb6dec4f0e6249325d Mon Sep 17 00:00:00 2001 From: cpradio Date: Sun, 3 Apr 2016 19:44:14 -0400 Subject: [PATCH 2/3] Added spec tests --- app/controllers/post_actions_controller.rb | 1 + lib/guardian/post_guardian.rb | 3 +++ spec/controllers/post_actions_controller_spec.rb | 11 +++++++++++ 3 files changed, 15 insertions(+) diff --git a/app/controllers/post_actions_controller.rb b/app/controllers/post_actions_controller.rb index 324d0b8ad7b..1c478504c36 100644 --- a/app/controllers/post_actions_controller.rb +++ b/app/controllers/post_actions_controller.rb @@ -8,6 +8,7 @@ class PostActionsController < ApplicationController def create taken = PostAction.counts_for([@post], current_user)[@post.id] guardian.ensure_post_can_act!(@post, PostActionType.types[@post_action_type_id], taken_actions: taken) + guardian.ensure_post_can_act!(@post, PostActionType.types[@post_action_type_id], is_warning: params[:is_warning]) args = {} args[:message] = params[:message] if params[:message].present? diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 9c07db3380d..b1ef4951b85 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -32,6 +32,9 @@ module PostGuardian # new users can't notify_user because they are not allowed to send private messages not(action_key == :notify_user && !@user.has_trust_level?(SiteSetting.min_trust_to_send_messages)) && + # non-staff can't send an official warning + not(action_key == :notify_user && !is_staff? && opts[:is_warning].present? && opts[:is_warning] == 'true') && + # can't send private messages if they're disabled globally not(action_key == :notify_user && !SiteSetting.enable_private_messages) && diff --git a/spec/controllers/post_actions_controller_spec.rb b/spec/controllers/post_actions_controller_spec.rb index ca822b832e4..a9780ac2838 100644 --- a/spec/controllers/post_actions_controller_spec.rb +++ b/spec/controllers/post_actions_controller_spec.rb @@ -49,6 +49,17 @@ describe PostActionsController do xhr :post, :create, id: @post.id, post_action_type_id: PostActionType.types[:like], message: 'action message goes here' end + it 'passes the message through as warning' do + PostAction.expects(:act).once.with(@user, @post, PostActionType.types[:like], {message: 'action message goes here', is_warning: true}) + xhr :post, :create, id: @post.id, post_action_type_id: PostActionType.types[:like], message: 'action message goes here', is_warning: true + end + + it "doesn't create message as a warning 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], {message: 'action message goes here'}) + xhr :post, :create, id: @post.id, post_action_type_id: PostActionType.types[:like], message: 'action message goes here', is_warning: true + 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' From 67cd08af9bac6e0376ff66ab60fde77af7c03073 Mon Sep 17 00:00:00 2001 From: cpradio Date: Sun, 3 Apr 2016 20:26:42 -0400 Subject: [PATCH 3/3] Add qunit tests --- test/javascripts/controllers/flag-test.js.es6 | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/javascripts/controllers/flag-test.js.es6 b/test/javascripts/controllers/flag-test.js.es6 index ebff4fd1d7b..4d4775065d3 100644 --- a/test/javascripts/controllers/flag-test.js.es6 +++ b/test/javascripts/controllers/flag-test.js.es6 @@ -64,3 +64,37 @@ test("canDeleteSpammer spam selected", function(){ flagController.set('userDetails', buildAdminUser({can_delete_all_posts: false, can_be_deleted: false})); canDeleteSpammer(flagController, 'spam', false, 'false if current user is staff, selected is spam, user cannot be deleted'); }); + +test("canSendWarning not staff", function(){ + const store = createStore(); + + var flagController = this.subject({ model: buildPost() }); + sandbox.stub(Discourse.User, 'currentProp').withArgs('staff').returns(false); + + const notifyUserFlag = store.createRecord('post-action-type', {name_key: 'notify_user'}); + flagController.set('selected', notifyUserFlag); + equal(flagController.get('canSendWarning'), false, 'false if current user is not staff'); +}); + +var canSendWarning = function(flagController, postActionType, expected, testName) { + const store = createStore(); + const flag = store.createRecord('post-action-type', {name_key: postActionType}); + flagController.set('selected', flag); + + equal(flagController.get('canSendWarning'), expected, testName); +}; + +test("canSendWarning notify_user not selected", function(){ + sandbox.stub(Discourse.User, 'currentProp').withArgs('staff').returns(true); + var flagController = this.subject({ model: buildPost() }); + canSendWarning(flagController, 'off_topic', false, 'false if current user is staff, but selected is off_topic'); + canSendWarning(flagController, 'inappropriate', false, 'false if current user is staff, but selected is inappropriate'); + canSendWarning(flagController, 'spam', false, 'false if current user is staff, but selected is spam'); + canSendWarning(flagController, 'notify_moderators', false, 'false if current user is staff, but selected is notify_moderators'); +}); + +test("canSendWarning notify_user selected", function(){ + sandbox.stub(Discourse.User, 'currentProp').withArgs('staff').returns(true); + var flagController = this.subject({ model: buildPost() }); + canSendWarning(flagController, 'notify_user', true, 'true if current user is staff, selected is notify_user'); +}); \ No newline at end of file