From 0a4b1b655d44271439b2804605d2423dec938c46 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 30 Oct 2023 10:24:35 +1000 Subject: [PATCH] FIX: Alter "Take Action" default behaviour to hide post (#24088) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes an issue where clicking the default "Take Action" option on a flag for a post doesn't always end up with the post hidden. This is because the "take_action" score bonus doesn’t take into account the final score required to hide the post. Especially with the `hide_post_sensitivity` site setting set to `low` sensitivity, there is a likelihood the score needed to hide the post won’t be reached. Now, the default "Take Action" button has been changed to "Hide Post" to reflect what is actually happening and the description has been improved, and if "Take Action" is clicked we _always_ hide the post regardless of score and sensitivity settings. This way the action reflects expectations of the user. --- .../discourse/app/components/modal/flag.hbs | 9 ++--- .../discourse/app/components/modal/flag.js | 2 +- .../tests/acceptance/flag-post-test.js | 11 ++++-- config/locales/client.en.yml | 6 ++-- lib/post_action_creator.rb | 4 ++- spec/lib/post_action_creator_spec.rb | 36 ++++++++++++++----- spec/system/flagging_post_spec.rb | 30 ++++++++++++++++ spec/system/page_objects/modals/base.rb | 14 ++++++-- spec/system/page_objects/modals/flag.rb | 25 +++++++++++++ spec/system/page_objects/pages/topic.rb | 2 ++ 10 files changed, 118 insertions(+), 21 deletions(-) create mode 100644 spec/system/flagging_post_spec.rb create mode 100644 spec/system/page_objects/modals/flag.rb diff --git a/app/assets/javascripts/discourse/app/components/modal/flag.hbs b/app/assets/javascripts/discourse/app/components/modal/flag.hbs index 8fc67e7dc97..b8ebc6d6750 100644 --- a/app/assets/javascripts/discourse/app/components/modal/flag.hbs +++ b/app/assets/javascripts/discourse/app/components/modal/flag.hbs @@ -32,7 +32,7 @@ <:footer> = Reviewable.score_required_to_hide_post + if score >= Reviewable.score_required_to_hide_post || @take_action + @post.hide!(@post_action_type_id) + end end # Special case: If you have TL3 and the user is TL0, and the flag is spam, diff --git a/spec/lib/post_action_creator_spec.rb b/spec/lib/post_action_creator_spec.rb index fef228d75b3..36e723d56ec 100644 --- a/spec/lib/post_action_creator_spec.rb +++ b/spec/lib/post_action_creator_spec.rb @@ -271,18 +271,38 @@ RSpec.describe PostActionCreator do end describe "take_action" do - before { PostActionCreator.create(Fabricate(:user), post, :inappropriate) } + it "will hide the post" do + PostActionCreator + .new(Fabricate(:moderator), post, PostActionType.types[:spam], take_action: true) + .perform + .reviewable + expect(post.reload).to be_hidden + end - it "will agree with the old reviewable" do - reviewable = + context "when there is another reviewable on the post" do + before { PostActionCreator.create(Fabricate(:user), post, :inappropriate) } + + it "will agree with the old reviewable" do + reviewable = + PostActionCreator + .new(Fabricate(:moderator), post, PostActionType.types[:spam], take_action: true) + .perform + .reviewable + expect(reviewable.reload).to be_approved + expect(reviewable.reviewable_scores).to all(be_agreed) + end + end + + context "when hide_post_sensitivity is low" do + before { SiteSetting.hide_post_sensitivity = Reviewable.sensitivities[:low] } + + it "still hides the post without considering the score" do PostActionCreator .new(Fabricate(:moderator), post, PostActionType.types[:spam], take_action: true) .perform .reviewable - scores = reviewable.reviewable_scores - expect(scores[0]).to be_agreed - expect(scores[1]).to be_agreed - expect(reviewable.reload).to be_approved + expect(post.reload).to be_hidden + end end end @@ -307,7 +327,7 @@ RSpec.describe PostActionCreator do score = result.reviewable.reviewable_scores.last expect(score.reason).to eq("queued_by_staff") - expect(post.reload.hidden?).to eq(true) + expect(post.reload).to be_hidden end it "hides the topic even if it has replies" do diff --git a/spec/system/flagging_post_spec.rb b/spec/system/flagging_post_spec.rb new file mode 100644 index 00000000000..b760335838e --- /dev/null +++ b/spec/system/flagging_post_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# frozen_string_literal: true + +describe "Flagging post", type: :system, js: true do + fab!(:current_user) { Fabricate(:admin) } + fab!(:first_post) { Fabricate(:post) } + fab!(:post_to_flag) { Fabricate(:post, topic: first_post.topic) } + + let(:topic_page) { PageObjects::Pages::Topic.new } + let(:flag_modal) { PageObjects::Modals::Flag.new } + + before { sign_in(current_user) } + + describe "Using Take Action" do + it "can select the default action to hide the post, agree with other flags, and reach the flag threshold" do + other_flag = Fabricate(:flag, post: post_to_flag, user: Fabricate(:moderator)) + expect(other_flag.reload.agreed_at).to be_nil + topic_page.visit_topic(post_to_flag.topic) + topic_page.expand_post_actions(post_to_flag) + topic_page.click_post_action_button(post_to_flag, :flag) + flag_modal.choose_type(:off_topic) + flag_modal.take_action(:agree_and_hide) + expect( + topic_page.post_by_number(post_to_flag).ancestor(".topic-post.post-hidden"), + ).to be_present + expect(other_flag.reload.agreed_at).to be_present + end + end +end diff --git a/spec/system/page_objects/modals/base.rb b/spec/system/page_objects/modals/base.rb index daa81e5eea9..3b8025ee3e8 100644 --- a/spec/system/page_objects/modals/base.rb +++ b/spec/system/page_objects/modals/base.rb @@ -6,6 +6,16 @@ module PageObjects include Capybara::DSL include RSpec::Matchers + BODY_SELECTOR = "" + + def body + find(".modal-body#{BODY_SELECTOR}") + end + + def footer + find(".modal-footer") + end + def close find(".modal-close").click end @@ -19,11 +29,11 @@ module PageObjects end def click_primary_button - find(".modal-footer .btn-primary").click + footer.find(".btn-primary").click end def has_content?(content) - find(".modal-body").has_content?(content) + body.has_content?(content) end def open? diff --git a/spec/system/page_objects/modals/flag.rb b/spec/system/page_objects/modals/flag.rb new file mode 100644 index 00000000000..1e2da7cba0a --- /dev/null +++ b/spec/system/page_objects/modals/flag.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module PageObjects + module Modals + class Flag < PageObjects::Modals::Base + BODY_SELECTOR = ".flag-modal-body" + MODAL_SELECTOR = ".flag-modal" + + def choose_type(type) + body.find("#radio_#{type}").click + end + + def confirm_flag + click_primary_button + end + + def take_action(action) + select_kit = + PageObjects::Components::SelectKit.new(".modal-footer .reviewable-action-dropdown") + select_kit.expand + select_kit.select_row_by_value(action) + end + end + end +end diff --git a/spec/system/page_objects/pages/topic.rb b/spec/system/page_objects/pages/topic.rb index 988151d0508..2061ff5ce53 100644 --- a/spec/system/page_objects/pages/topic.rb +++ b/spec/system/page_objects/pages/topic.rb @@ -84,6 +84,8 @@ module PageObjects post_by_number(post).find(".bookmark.with-reminder").click when :reply post_by_number(post).find(".post-controls .reply").click + when :flag + post_by_number(post).find(".post-controls .create-flag").click end end