From e52bbc1230b5897506d4d6cfc6f215b714a54797 Mon Sep 17 00:00:00 2001 From: chapoi <101828855+chapoi@users.noreply.github.com> Date: Thu, 2 Mar 2023 22:40:53 +0700 Subject: [PATCH] UX/DEV: Review queue redesign fixes (#20239) * UX: add type tag and design update * UX: clarify status copy in reviewQ * DEV: switch to selectKit * UX: color approve/reject buttons in RQ * DEV: regroup actions * UX: add type tag and design update * UX: clarify status copy in reviewQ * Join questions for flagged post with "or" with new I18n function * Move ReviewableScores component out of context * Add CSS classes to reviewable-item based on human type * UX: add table header for scoring * UX: don't display % score * UX: prefix modifier class with dash * UX: reviewQ flag table styling * UX: consistent use of ignore icon * DEV: only show context question on pending status * UX: only show table headers on pending status * DEV: reviewQ regroup actions for hidden posts * UX: reviewQ > approve/reject buttons * UX: reviewQ add fadeout * UX: reviewQ styling * DEV: move scores back into component * UX: reviewQ mobile styling * UX: score table on mobile * UX: reviewQ > move meta info outside table * UX: reviewQ > score layout fixes * DEV: readd `agree_and_keep` and fix the spec tests. * Fix the spec tests * fix the quint test * DEV: readd deleting replies * UX: reviewQ copy tweaks * DEV: readd test for ignore + delete replies * Remove old * FIX: Add perform_ignore back in for backwards compat * DEV: add an action alias `ignore` for `ignore_and_do_nothing`. --------- Co-authored-by: Martin Brennan Co-authored-by: Vinoth Kannan --- .../components/reviewable-bundled-action.hbs | 5 +- .../components/reviewable-flagged-post.hbs | 12 +- .../app/components/reviewable-item.hbs | 13 +- .../app/components/reviewable-score.hbs | 60 ++-------- .../app/components/reviewable-scores.hbs | 49 ++++++-- .../app/helpers/reviewable-status.js | 2 - .../discourse/app/models/reviewable.js | 41 ++++++- .../discourse/tests/acceptance/review-test.js | 4 +- app/assets/javascripts/locales/i18n.js | 13 ++ .../stylesheets/common/base/reviewables.scss | 112 ++++++++++++------ .../stylesheets/mobile/reviewables.scss | 28 ++++- app/jobs/scheduled/auto_queue_handler.rb | 2 +- app/models/reviewable_flagged_post.rb | 60 ++++++---- .../reviewable_score_type_serializer.rb | 6 +- config/locales/client.en.yml | 22 ++-- config/locales/server.en.yml | 45 +++---- lib/post_destroyer.rb | 2 +- .../lib/discourse-markdown/chat-transcript.js | 2 +- .../advanced_user_narrative.rb | 2 +- spec/jobs/truncate_user_flag_stats_spec.rb | 2 +- spec/lib/post_action_creator_spec.rb | 2 +- spec/lib/post_destroyer_spec.rb | 4 +- spec/models/post_action_spec.rb | 3 +- spec/models/post_spec.rb | 4 +- spec/models/reviewable_flagged_post_spec.rb | 14 ++- spec/models/reviewable_spec.rb | 4 +- spec/models/user_spec.rb | 5 +- spec/requests/posts_controller_spec.rb | 2 +- 28 files changed, 334 insertions(+), 186 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/reviewable-bundled-action.hbs b/app/assets/javascripts/discourse/app/components/reviewable-bundled-action.hbs index 9496f71ea06..ffd767d5d28 100644 --- a/app/assets/javascripts/discourse/app/components/reviewable-bundled-action.hbs +++ b/app/assets/javascripts/discourse/app/components/reviewable-bundled-action.hbs @@ -1,11 +1,11 @@ {{#if this.multiple}}
- {{#if this.reviewable.blank_post}} -

{{i18n "review.deleted_post"}}

- {{else}} - {{html-safe this.reviewable.cooked}} - {{/if}} +
+ {{#if this.reviewable.blank_post}} +

{{i18n "review.deleted_post"}}

+ {{else}} + {{html-safe this.reviewable.cooked}} + {{/if}} +
- {{this.reviewable.humanType}} + {{this.reviewable.humanType}} {{#if this.reviewable.reply_count}} {{i18n "review.replies" @@ -65,6 +67,15 @@ {{/component}} {{/if}}
+ + {{#if (eq this.reviewable.type "ReviewableFlaggedPost")}} + {{#if (eq this.reviewable.status 0)}} +

+ {{this.reviewable.flaggedPostContextQuestion}} +

+ {{/if}} + {{/if}} +
{{#if this.reviewable.last_performing_username}}
{{html-safe diff --git a/app/assets/javascripts/discourse/app/components/reviewable-score.hbs b/app/assets/javascripts/discourse/app/components/reviewable-score.hbs index 639aff88b93..cf80c377207 100644 --- a/app/assets/javascripts/discourse/app/components/reviewable-score.hbs +++ b/app/assets/javascripts/discourse/app/components/reviewable-score.hbs @@ -4,25 +4,18 @@ {{avatar this.rs.user imageSize="tiny"}} {{this.rs.user.username}} - - - - {{d-icon this.rs.score_type.icon}} - {{this.title}} + {{format-date this.rs.created_at format="tiny"}} - {{#if this.showStatus}} - - {{d-icon "angle-double-right"}} - + + {{d-icon this.rs.score_type.icon}} + {{this.title}} + + {{#if this.showStatus}} {{#if this.rs.reviewed_by}} @@ -34,46 +27,17 @@ {{/if}} - - {{reviewable-status this.rs.status}} - {{#if this.rs.reviewed_by}} {{format-date this.rs.reviewed_at format="tiny"}} {{/if}} + + + {{reviewable-status this.rs.status}} + + {{else}} {{/if}} - - -{{#if this.rs.reason}} - - -
{{html-safe this.rs.reason}}
- - -{{/if}} - -{{#if this.rs.reviewable_conversation}} - - -
- {{#each - this.rs.reviewable_conversation.conversation_posts - as |p index| - }} - - {{/each}} -
- - {{i18n "review.conversation.view_full"}} - -
-
- - -{{/if}} \ No newline at end of file + \ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/components/reviewable-scores.hbs b/app/assets/javascripts/discourse/app/components/reviewable-scores.hbs index 7758be85088..73d268380da 100644 --- a/app/assets/javascripts/discourse/app/components/reviewable-scores.hbs +++ b/app/assets/javascripts/discourse/app/components/reviewable-scores.hbs @@ -1,9 +1,44 @@ {{#if this.reviewable.reviewable_scores}} - - - {{#each this.reviewable.reviewable_scores as |rs|}} - - {{/each}} - -
+
+ + + + + + + + + + + + + {{#each this.reviewable.reviewable_scores as |rs|}} + + {{/each}} + +
{{i18n "review.scores.submitted_by"}}{{i18n "review.scores.date"}}{{i18n "review.scores.type"}}{{i18n "review.scores.reviewed_by"}}{{i18n "review.scores.reviewed_timestamp"}}{{i18n "review.scores.status"}}
+
+ + {{#each this.reviewable.reviewable_scores as |rs|}} + {{#if rs.reason}} +
{{html-safe rs.reason}}
+ {{/if}} + + {{#if rs.reviewable_conversation}} +
+ {{#each rs.reviewable_conversation.conversation_posts as |p index|}} + + {{/each}} +
+ + {{i18n "review.conversation.view_full"}} + +
+
+ {{/if}} + {{/each}} + {{/if}} \ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/helpers/reviewable-status.js b/app/assets/javascripts/discourse/app/helpers/reviewable-status.js index 4fe4ef2445e..95e36abf7c7 100644 --- a/app/assets/javascripts/discourse/app/helpers/reviewable-status.js +++ b/app/assets/javascripts/discourse/app/helpers/reviewable-status.js @@ -33,12 +33,10 @@ export function htmlStatus(status) { let icon = data.icon ? iconHTML(data.icon) : ""; return ` - ${icon} ${I18n.t("review.statuses." + data.name + ".title")} - `; } diff --git a/app/assets/javascripts/discourse/app/models/reviewable.js b/app/assets/javascripts/discourse/app/models/reviewable.js index a8e0a8ad8d1..db973283cfa 100644 --- a/app/assets/javascripts/discourse/app/models/reviewable.js +++ b/app/assets/javascripts/discourse/app/models/reviewable.js @@ -1,10 +1,10 @@ import categoryFromId from "discourse-common/utils/category-macro"; +import { dasherize, underscore } from "@ember/string"; import I18n from "I18n"; import { Promise } from "rsvp"; import RestModel from "discourse/models/rest"; import { ajax } from "discourse/lib/ajax"; import discourseComputed from "discourse-common/utils/decorators"; -import { underscore } from "@ember/string"; export const PENDING = 0; export const APPROVED = 1; @@ -14,17 +14,50 @@ export const DELETED = 4; const Reviewable = RestModel.extend({ @discourseComputed("type", "topic") - humanType(type, topic) { + resolvedType(type, topic) { // Display "Queued Topic" if the post will create a topic if (type === "ReviewableQueuedPost" && !topic) { - type = "ReviewableQueuedTopic"; + return "ReviewableQueuedTopic"; } - return I18n.t(`review.types.${underscore(type)}.title`, { + return type; + }, + + @discourseComputed("resolvedType") + humanType(resolvedType) { + return I18n.t(`review.types.${underscore(resolvedType)}.title`, { defaultValue: "", }); }, + @discourseComputed("humanType") + humanTypeCssClass(humanType) { + return "-" + dasherize(humanType); + }, + + @discourseComputed + flaggedPostContextQuestion() { + const uniqueReviewableScores = + this.reviewable_scores.uniqBy("score_type.type"); + + if (uniqueReviewableScores.length === 1) { + if (uniqueReviewableScores[0].score_type.type === "notify_moderators") { + return I18n.t("review.context_question.something_else_wrong"); + } + } + + const listOfQuestions = I18n.listJoiner( + uniqueReviewableScores + .map((score) => score.score_type.title.toLowerCase()) + .uniq(), + I18n.t("review.context_question.delimiter") + ); + + return I18n.t("review.context_question.is_this_post", { + reviewable_human_score_types: listOfQuestions, + }); + }, + category: categoryFromId("category_id"), update(updates) { diff --git a/app/assets/javascripts/discourse/tests/acceptance/review-test.js b/app/assets/javascripts/discourse/tests/acceptance/review-test.js index 212ada586fb..60013e2e8eb 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/review-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/review-test.js @@ -114,7 +114,9 @@ acceptance("Review", function (needs) { ); assert.strictEqual( - query(".reviewable-flagged-post .post-body").innerHTML.trim(), + query( + ".reviewable-flagged-post .post-body .post-body__scroll" + ).innerHTML.trim(), "cooked content" ); diff --git a/app/assets/javascripts/locales/i18n.js b/app/assets/javascripts/locales/i18n.js index dab2074fc6d..686363acb5e 100644 --- a/app/assets/javascripts/locales/i18n.js +++ b/app/assets/javascripts/locales/i18n.js @@ -266,6 +266,19 @@ I18n.toHumanSize = function (number, options) { return number; }; +I18n.listJoiner = function (listOfStrings, delimiter) { + if (listOfStrings.length === 1) { + return listOfStrings[0]; + } + + if (listOfStrings.length === 2) { + return listOfStrings[0] + " " + delimiter + " " + listOfStrings[1]; + } + + var lastString = listOfStrings.pop(); + return listOfStrings.concat(delimiter).join(`, `) + " " + lastString; +}; + I18n.pluralizer = function (locale) { var pluralizer = this.pluralizationRules[locale]; if (pluralizer !== undefined) return pluralizer; diff --git a/app/assets/stylesheets/common/base/reviewables.scss b/app/assets/stylesheets/common/base/reviewables.scss index 7b2a48ac706..7adb846fa00 100644 --- a/app/assets/stylesheets/common/base/reviewables.scss +++ b/app/assets/stylesheets/common/base/reviewables.scss @@ -209,8 +209,8 @@ } .reviewable-item { - padding-top: 2em; - border-top: 1px solid var(--primary-low); + background: var(--primary-very-low); + padding: 1.5rem; .topic-statuses { font-size: var(--font-up-2); @@ -225,6 +225,21 @@ align-items: baseline; .reviewable-type { margin-right: 0.25em; + padding: 0.25em 0.5em; + text-transform: uppercase; + font-size: var(--font-down-2); + color: var(--secondary); + border-radius: 8px; + &.-flagged-post, + &.-user, + &.-flagged-chat-message, + &.-aksimet-flagged-post, + &.-aksimet-flagged-user { + background-color: var(--danger-medium); + } + &.-queued-post { + background-color: var(--tertiary); + } } .reply-count { margin-left: 1em; @@ -244,16 +259,28 @@ .reviewable-contents { display: flex; flex-wrap: wrap; - margin-bottom: 2em; + margin: 1.5rem 0 1rem; + background: var(--secondary); + padding: 1rem; } .reviewable-actions { display: flex; flex-wrap: wrap; + gap: 0.5rem; width: 100%; button { white-space: nowrap; + + &.approve-post { + background-color: var(--success); + color: var(--secondary); + } + &.reject-post { + background-color: var(--danger); + color: var(--secondary); + } } .reviewable-action, @@ -285,9 +312,18 @@ } .reviewable-scores { + margin-top: 1.5rem; min-width: 50%; color: var(--primary-high); + &__table-wrapper { + overflow-x: scroll; + } + + th { + white-space: nowrap; + } + .reviewed-by { .date { margin-left: 0.5em; @@ -313,6 +349,16 @@ vertical-align: text-top; } + .approved, + .approved svg { + color: var(--success); + } + + .rejected, + .rejected svg { + color: var(--danger); + } + tbody { border-width: 1px; td { @@ -324,27 +370,19 @@ @include ellipsis; } } - - td:last-of-type { - width: 100%; - white-space: normal; - } > tr > th { text-align: left; } > tr > th, > tr > td { &:not(:empty) { - padding: 0.5em 1em 0.5em 0; + padding: 0.5em; } @include breakpoint("mobile-large") { @include ellipsis; padding-right: 0.5em; } } - .reviewable-score-spacer { - padding-right: 1em; - } } } @@ -373,6 +411,7 @@ } .reviewable-item { + margin-block: 3rem; .show-raw-email { color: var(--primary-medium); font-size: var(--font-down-2); @@ -425,10 +464,26 @@ } .post-body { + position: relative; max-width: var(--topic-body-width); - max-height: 300px; margin-top: 0.5em; - overflow-y: auto; + + &__scroll { + max-height: 300px; + overflow-y: auto; + &:after { + content: ""; + position: absolute; + bottom: 0; + width: 100%; + height: 1.5em; + background: linear-gradient( + to bottom, + rgba(var(--secondary-rgb), 0), + rgba(var(--secondary-rgb), 100%) + ); + } + } p, aside { @@ -453,6 +508,10 @@ margin-right: 0.75em; } } + + &__context-question { + margin-block: 1rem; + } } .editable-fields { @@ -486,28 +545,3 @@ .flag-modal .modal-inner-container .select-kit.reviewable-action-dropdown { width: initial; } - -@media screen and (max-width: 1000px) { - table.reviewable-scores { - width: 100%; - display: block; - tbody { - width: calc(100% - 5px); - display: block; - clear: both; - } - } - tr.reviewable-score { - display: grid; - grid-template-columns: auto auto 1fr; - } - td.reviewable-score-spacer { - display: none; - } -} - -@include breakpoint("mobile-large") { - tr.reviewable-score { - grid-template-columns: auto auto auto; - } -} diff --git a/app/assets/stylesheets/mobile/reviewables.scss b/app/assets/stylesheets/mobile/reviewables.scss index 847037fa6f4..46c9418e3ad 100644 --- a/app/assets/stylesheets/mobile/reviewables.scss +++ b/app/assets/stylesheets/mobile/reviewables.scss @@ -21,7 +21,34 @@ } .reviewable-scores { + display: flex; width: 100%; + overflow-x: scroll; + border-top: 1px solid var(--primary-low); + thead { + tr { + display: flex; + flex-direction: column; + border: 0; + } + } + + tbody { + display: flex; + border: 0; + + .reviewable-score { + display: flex; + flex-direction: column; + } + tr { + border: 0; + } + + td { + white-space: nowrap; + } + } } } @@ -58,7 +85,6 @@ > div, > button { - flex: 0 1 47%; margin-right: 0.25em; margin-bottom: 0.5em; } diff --git a/app/jobs/scheduled/auto_queue_handler.rb b/app/jobs/scheduled/auto_queue_handler.rb index ca67634cfde..7cb7ca54b62 100644 --- a/app/jobs/scheduled/auto_queue_handler.rb +++ b/app/jobs/scheduled/auto_queue_handler.rb @@ -14,7 +14,7 @@ module Jobs .where("created_at < ?", SiteSetting.auto_handle_queued_age.to_i.days.ago) .each do |reviewable| if reviewable.is_a?(ReviewableFlaggedPost) - reviewable.perform(Discourse.system_user, :ignore, expired: true) + reviewable.perform(Discourse.system_user, :ignore_and_do_nothing, expired: true) elsif reviewable.is_a?(ReviewableQueuedPost) reviewable.perform(Discourse.system_user, :reject_post) elsif reviewable.is_a?(ReviewableUser) diff --git a/app/models/reviewable_flagged_post.rb b/app/models/reviewable_flagged_post.rb index 8015cd6e127..a144845f437 100644 --- a/app/models/reviewable_flagged_post.rb +++ b/app/models/reviewable_flagged_post.rb @@ -10,6 +10,7 @@ class ReviewableFlaggedPost < Reviewable agree_and_silence: :agree_and_keep, agree_and_suspend: :agree_and_keep, disagree_and_restore: :disagree, + ignore_and_do_nothing: :ignore, } end @@ -56,6 +57,20 @@ class ReviewableFlaggedPost < Reviewable build_action(actions, :agree_and_keep, icon: "thumbs-up", bundle: agree) end + if guardian.can_delete_post_or_topic?(post) + build_action(actions, :delete_and_agree, icon: "far-trash-alt", bundle: agree) + + if post.reply_count > 0 + build_action( + actions, + :delete_and_agree_replies, + icon: "far-trash-alt", + bundle: agree, + confirm: true, + ) + end + end + if guardian.can_suspend?(target_created_by) build_action( actions, @@ -74,48 +89,43 @@ class ReviewableFlaggedPost < Reviewable end build_action(actions, :agree_and_restore, icon: "far-eye", bundle: agree) if post.user_deleted? - if post.hidden? build_action(actions, :disagree_and_restore, icon: "thumbs-down") else build_action(actions, :disagree, icon: "thumbs-down") end - build_action(actions, :ignore, icon: "external-link-alt") - - delete_user_actions(actions) if potential_spam? && guardian.can_delete_user?(target_created_by) + ignore = + actions.add_bundle( + "#{id}-ignore", + icon: "thumbs-up", + label: "reviewables.actions.ignore.title", + ) + if !post.hidden? + build_action(actions, :ignore_and_do_nothing, icon: "external-link-alt", bundle: ignore) + end if guardian.can_delete_post_or_topic?(post) - delete = - actions.add_bundle( - "#{id}-delete", - icon: "far-trash-alt", - label: "reviewables.actions.delete.title", - ) - build_action(actions, :delete_and_ignore, icon: "external-link-alt", bundle: delete) + build_action(actions, :delete_and_ignore, icon: "far-trash-alt", bundle: ignore) if post.reply_count > 0 build_action( actions, :delete_and_ignore_replies, - icon: "external-link-alt", - confirm: true, - bundle: delete, - ) - end - build_action(actions, :delete_and_agree, icon: "thumbs-up", bundle: delete) - if post.reply_count > 0 - build_action( - actions, - :delete_and_agree_replies, - icon: "external-link-alt", - bundle: delete, + icon: "far-trash-alt", confirm: true, + bundle: ignore, ) end end + + delete_user_actions(actions) if potential_spam? && guardian.can_delete_user?(target_created_by) end def perform_ignore(performed_by, args) + perform_ignore_and_do_nothing(performed_by, args) + end + + def perform_ignore_and_do_nothing(performed_by, args) actions = PostAction .active @@ -221,13 +231,13 @@ class ReviewableFlaggedPost < Reviewable end def perform_delete_and_ignore(performed_by, args) - result = perform_ignore(performed_by, args) + result = perform_ignore_and_do_nothing(performed_by, args) destroyer(performed_by, post).destroy result end def perform_delete_and_ignore_replies(performed_by, args) - result = perform_ignore(performed_by, args) + result = perform_ignore_and_do_nothing(performed_by, args) PostDestroyer.delete_with_replies(performed_by, post, self) result diff --git a/app/serializers/reviewable_score_type_serializer.rb b/app/serializers/reviewable_score_type_serializer.rb index e31e5fafd9a..5db84b89201 100644 --- a/app/serializers/reviewable_score_type_serializer.rb +++ b/app/serializers/reviewable_score_type_serializer.rb @@ -1,7 +1,11 @@ # frozen_string_literal: true class ReviewableScoreTypeSerializer < ApplicationSerializer - attributes :id, :title, :reviewable_priority, :icon + attributes :id, :title, :reviewable_priority, :icon, :type + + def type + ReviewableScore.types[id] + end # Allow us to share post action type translations for backwards compatibility def title diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 3bb954ef481..515abec9467 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -581,28 +581,34 @@ en: scores: about: "This score is calculated based on the trust level of the reporter, the accuracy of their previous flags, and the priority of the item being reported." score: "Score" - date: "Date" - type: "Type" + date: "Report date" + type: "Reason" status: "Status" - submitted_by: "Submitted By" - reviewed_by: "Reviewed By" + submitted_by: "Reported by" + reviewed_by: "Reviewed by" + reviewed_timestamp: "Review date" statuses: pending: title: "Pending" approved: - title: "Approved" + title: "Flag approved" rejected: - title: "Rejected" + title: "Flag rejected" ignored: - title: "Ignored" + title: "Flag ignored" deleted: - title: "Deleted" + title: "Topic or post deleted" reviewed: title: "(all reviewed)" all: title: "(everything)" + context_question: + is_this_post: "Is this post %{reviewable_human_score_types}?" + delimiter: "or" + something_else_wrong: "Is there something else wrong with this post?" + types: reviewable_flagged_post: title: "Flagged Post" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 63c48a5152e..63551b6cdf4 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2456,7 +2456,7 @@ en: search_tokenize_japanese_enabled: "You must disable 'search_tokenize_japanese' before enabling this setting." discourse_connect_cannot_be_enabled_if_second_factor_enforced: "You cannot enable DiscourseConnect if 2FA is enforced." delete_rejected_email_after_days: "This setting cannot be set lower than the delete_email_logs_after_days setting or greater than %{max}" - invalid_uncategorized_category_setting: "The \"Uncategorized\" category cannot be selected if 'allow uncategorized topics' is not enabled." + invalid_uncategorized_category_setting: 'The "Uncategorized" category cannot be selected if ''allow uncategorized topics'' is not enabled.' enable_new_notifications_menu_not_legacy_navigation_menu: "You must set `navigation_menu` to `legacy` before enabling this setting." invalid_search_ranking_weights: "Value is invalid for search_ranking_weights site setting. Example: '{0.1,0.2,0.3,1.0}'. Note that maximum value for each weight is 1.0." @@ -5133,50 +5133,53 @@ en: actions: agree: - title: "Agree..." + title: "Yes" agree_and_keep: - title: "Keep Post" - description: "Agree with flag and keep the post unchanged." + title: "Keep post" + description: "Agree with flag but keep this post unchanged." agree_and_keep_hidden: - title: "Keep Post Hidden" - description: "Agree with flag and leave the post hidden." + title: "Keep post hidden" + description: "Agree with flag and keep the post hidden." agree_and_suspend: - title: "Suspend User" + title: "Suspend user" description: "Agree with flag and suspend the user." agree_and_silence: - title: "Silence User" + title: "Silence user" description: "Agree with flag and silence the user." agree_and_restore: - title: "Restore Post" + title: "Restore post" description: "Restore the post so that all users can see it." agree_and_hide: - title: "Hide Post" - description: "Hide this post and automatically send the user a message urging them to edit it." + title: "Hide post" + description: "Agree with flag and hide this post + automatically send the user a message urging them to edit it." delete_single: title: "Delete" delete: title: "Delete..." delete_and_ignore: - title: "Delete Post and Ignore" - description: "Delete post; if the first post, delete the topic as well" + title: "Ignore flag and delete post" + description: "Ignore the flag by removing it from the queue and delete the post; if the first post, delete the topic as well. " delete_and_ignore_replies: - title: "Delete Post + Replies and Ignore" - description: "Delete post and all of its replies; if the first post, delete the topic as well" + title: "Ignore flag, delete post and replies" + description: "Ignore the flag by removing it from the queue, delete the post and all of its replies; if the first post, delete the topic as well" confirm: "Are you sure you want to delete the replies to the post as well?" delete_and_agree: - title: "Delete Post and Agree" - description: "Delete post; if the first post, delete the topic as well" + title: "Delete post" + description: "Agree with flag and delete this post; if the first post, delete the topic as well." delete_and_agree_replies: - title: "Delete Post + Replies and Agree" - description: "Delete post and all of its replies; if the first post, delete the topic as well" + title: "Delete post and replies" + description: "Agree with flag and delete this post and all of its replies; if the first post, delete the topic as well." confirm: "Are you sure you want to delete the replies to the post as well?" disagree_and_restore: - title: "Disagree and Restore Post" + title: "No, restore post" description: "Restore the post so that all users can see it." disagree: - title: "Disagree" + title: "No" ignore: title: "Ignore" + ignore_and_do_nothing: + title: "Do nothing" + description: "Ignore the flag by removing it from the queue without taking any action. Hidden posts will stay hidden and be handled by the auto-tools." approve: title: "Approve" approve_post: diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 96d87b1b21d..b989fe83fab 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -357,7 +357,7 @@ class PostDestroyer end def ignore(reviewable) - reviewable.perform_ignore(@user, post_was_deleted: true) + reviewable.perform_ignore_and_do_nothing(@user, post_was_deleted: true) reviewable.transition_to(:ignored, @user) end diff --git a/plugins/chat/assets/javascripts/lib/discourse-markdown/chat-transcript.js b/plugins/chat/assets/javascripts/lib/discourse-markdown/chat-transcript.js index 1263a4b919a..515865247e4 100644 --- a/plugins/chat/assets/javascripts/lib/discourse-markdown/chat-transcript.js +++ b/plugins/chat/assets/javascripts/lib/discourse-markdown/chat-transcript.js @@ -232,7 +232,7 @@ export function setup(helper) { }); helper.buildCookFunction((opts, generateCookFunction) => { - if (!opts.discourse.additionalOptions) { + if (!opts.discourse.additionalOptions?.chat) { return; } diff --git a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/advanced_user_narrative.rb b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/advanced_user_narrative.rb index 8b93776603b..6fa8e6b6766 100644 --- a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/advanced_user_narrative.rb +++ b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/advanced_user_narrative.rb @@ -176,7 +176,7 @@ module DiscourseNarrativeBot opts[:delete_removed_posts_after] = 1 result = PostActionCreator.notify_moderators(self.discobot_user, post) - result.reviewable.perform(self.discobot_user, :ignore) + result.reviewable.perform(self.discobot_user, :ignore_and_do_nothing) end PostDestroyer.new(@user, post, opts).destroy diff --git a/spec/jobs/truncate_user_flag_stats_spec.rb b/spec/jobs/truncate_user_flag_stats_spec.rb index ffd1c0981e9..74cdc0e9726 100644 --- a/spec/jobs/truncate_user_flag_stats_spec.rb +++ b/spec/jobs/truncate_user_flag_stats_spec.rb @@ -53,7 +53,7 @@ RSpec.describe Jobs::TruncateUserFlagStats do r0.perform(Discourse.system_user, :agree_and_keep) r1.perform(Discourse.system_user, :disagree) - r2.perform(Discourse.system_user, :ignore) + r2.perform(Discourse.system_user, :ignore_and_do_nothing) r3.perform(Discourse.system_user, :agree_and_keep) user.user_stat.reload diff --git a/spec/lib/post_action_creator_spec.rb b/spec/lib/post_action_creator_spec.rb index 4304febc949..a168f0ea3af 100644 --- a/spec/lib/post_action_creator_spec.rb +++ b/spec/lib/post_action_creator_spec.rb @@ -191,7 +191,7 @@ RSpec.describe PostActionCreator do end describe "When the post was already reviewed by staff" do - before { reviewable.perform(admin, :ignore) } + before { reviewable.perform(admin, :ignore_and_do_nothing) } it "fails because the post was recently reviewed" do freeze_time 10.seconds.from_now diff --git a/spec/lib/post_destroyer_spec.rb b/spec/lib/post_destroyer_spec.rb index eef58dee7c9..e1bc124fdf5 100644 --- a/spec/lib/post_destroyer_spec.rb +++ b/spec/lib/post_destroyer_spec.rb @@ -94,7 +94,7 @@ RSpec.describe PostDestroyer do expect(reply1.deleted_at).to eq(nil) # ignore the flag, we should be able to delete the stub - reviewable.perform(Discourse.system_user, :ignore) + reviewable.perform(Discourse.system_user, :ignore_and_do_nothing) PostDestroyer.destroy_stubs reply1.reload @@ -938,7 +938,7 @@ RSpec.describe PostDestroyer do it "should not send the flags_agreed_and_post_deleted message if flags were ignored" do expect(ReviewableFlaggedPost.pending.count).to eq(1) - flag_result.reviewable.perform(moderator, :ignore) + flag_result.reviewable.perform(moderator, :ignore_and_do_nothing) second_post.reload expect(ReviewableFlaggedPost.pending.count).to eq(0) diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index 25d160afb26..5be13af4f28 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -975,7 +975,8 @@ RSpec.describe PostAction do end it "creates events for ignored" do - events = DiscourseEvent.track_events { reviewable.perform(moderator, :ignore) } + events = + DiscourseEvent.track_events { reviewable.perform(moderator, :ignore_and_do_nothing) } reviewed_event = events.find { |e| e[:event_name] == :flag_reviewed } expect(reviewed_event).to be_present diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 7fdb40fbaa1..71fdb20965a 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -200,7 +200,7 @@ RSpec.describe Post do it "is_flagged? is true if flag was deferred" do result = PostActionCreator.off_topic(user, post) - result.reviewable.perform(admin, :ignore) + result.reviewable.perform(admin, :ignore_and_do_nothing) expect(post.reload.is_flagged?).to eq(true) end @@ -214,7 +214,7 @@ RSpec.describe Post do result = PostActionCreator.spam(user, post) expect(post.reviewable_flag).to eq(result.reviewable) - result.reviewable.perform(admin, :ignore) + result.reviewable.perform(admin, :ignore_and_do_nothing) expect(post.reviewable_flag).to be_nil end diff --git a/spec/models/reviewable_flagged_post_spec.rb b/spec/models/reviewable_flagged_post_spec.rb index f86d6736be0..c16b93ec4f1 100644 --- a/spec/models/reviewable_flagged_post_spec.rb +++ b/spec/models/reviewable_flagged_post_spec.rb @@ -33,7 +33,7 @@ RSpec.describe ReviewableFlaggedPost, type: :model do expect(actions.has?(:delete_user)).to eq(true) expect(actions.has?(:delete_user_block)).to eq(true) expect(actions.has?(:disagree)).to eq(true) - expect(actions.has?(:ignore)).to eq(true) + expect(actions.has?(:ignore_and_do_nothing)).to eq(true) expect(actions.has?(:delete_and_ignore)).to eq(true) expect(actions.has?(:delete_and_ignore_replies)).to eq(false) expect(actions.has?(:delete_and_agree)).to eq(true) @@ -67,7 +67,6 @@ RSpec.describe ReviewableFlaggedPost, type: :model do it "returns delete replies options if there are replies" do post.update(reply_count: 3) expect(reviewable.actions_for(guardian).has?(:delete_and_agree_replies)).to eq(true) - expect(reviewable.actions_for(guardian).has?(:delete_and_ignore_replies)).to eq(true) end it "returns appropriate actions for a hidden post" do @@ -168,7 +167,7 @@ RSpec.describe ReviewableFlaggedPost, type: :model do end it "ignores the flags" do - reviewable.perform(moderator, :ignore) + reviewable.perform(moderator, :ignore_and_do_nothing) expect(reviewable).to be_ignored expect(score.reload).to be_ignored end @@ -273,7 +272,7 @@ RSpec.describe ReviewableFlaggedPost, type: :model do expect(post.hidden).to eq(false) expect(post.hidden_at).to be_blank - reviewable.perform(moderator, :ignore) + reviewable.perform(moderator, :ignore_and_do_nothing) expect(pending_count).to eq(0) post.reload @@ -340,6 +339,11 @@ RSpec.describe ReviewableFlaggedPost, type: :model do it "ignores flagged responses" do SiteSetting.notify_users_after_responses_deleted_on_flagged_post = true flagged_reply = Fabricate(:reviewable_flagged_post, target: reply) + Fabricate( + :post, + reply_to_post_number: flagged_reply.target.post_number, + topic: flagged_reply.target.topic, + ) flagged_post.perform(moderator, :delete_and_agree_replies) expect(flagged_reply.reload).to be_ignored @@ -366,7 +370,7 @@ RSpec.describe ReviewableFlaggedPost, type: :model do let(:reviewable) { Fabricate(:reviewable_flagged_post, score: expected_score) } it "doesn't recalculate the score after ignore" do - reviewable.perform(moderator, :ignore) + reviewable.perform(moderator, :ignore_and_do_nothing) expect(reviewable.score).to eq(expected_score) end diff --git a/spec/models/reviewable_spec.rb b/spec/models/reviewable_spec.rb index a9b0f887097..11893546c29 100644 --- a/spec/models/reviewable_spec.rb +++ b/spec/models/reviewable_spec.rb @@ -173,7 +173,7 @@ RSpec.describe Reviewable, type: :model do it "can filter by who reviewed the flag" do reviewable = Fabricate(:reviewable_flagged_post) admin = Fabricate(:admin) - reviewable.perform(admin, :ignore) + reviewable.perform(admin, :ignore_and_do_nothing) reviewables = Reviewable.list_for(user, status: :all, reviewed_by: admin.username) @@ -438,7 +438,7 @@ RSpec.describe Reviewable, type: :model do it "increases flags_ignored when ignored" do expect(user.user_stat.flags_ignored).to eq(0) - reviewable.perform(Discourse.system_user, :ignore) + reviewable.perform(Discourse.system_user, :ignore_and_do_nothing) expect(user.user_stat.reload.flags_ignored).to eq(1) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a1c4b203437..d18ff573446 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1955,7 +1955,10 @@ RSpec.describe User do .perform(moderator, :agree_and_keep) post_deferred = Fabricate(:post) - PostActionCreator.inappropriate(user, post_deferred).reviewable.perform(moderator, :ignore) + PostActionCreator + .inappropriate(user, post_deferred) + .reviewable + .perform(moderator, :ignore_and_do_nothing) post_disagreed = Fabricate(:post) PostActionCreator diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 68da8396fa1..e31a496960d 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -2268,7 +2268,7 @@ RSpec.describe PostsController do r2 = PostActionCreator.inappropriate(moderator, post_disagreed).reviewable r0.perform(admin, :agree_and_keep) - r1.perform(admin, :ignore) + r1.perform(admin, :ignore_and_do_nothing) r2.perform(admin, :disagree) sign_in(Fabricate(:moderator))