From c780ae9d25f416f9134cbeab29d951ddc47d50ac Mon Sep 17 00:00:00 2001 From: Kane York Date: Mon, 14 Jun 2021 14:01:17 -0700 Subject: [PATCH] FEATURE: Add a messages view for all official warnings of a user (#12659) Moderators are allowed to see the warnings list, with an access warning. https://meta.discourse.org/t/why-arent-warnings-easily-accessible-like-suspensions-are/164043 --- .../app/controllers/user-private-messages.js | 6 +++ .../discourse/app/routes/app-route-map.js | 1 + .../routes/user-private-messages-warnings.js | 9 ++++ .../discourse/app/templates/user.hbs | 6 ++- .../discourse/app/templates/user/messages.hbs | 3 ++ .../discourse/tests/acceptance/user-test.js | 15 ++++++ .../tests/helpers/create-pretender.js | 4 ++ app/controllers/list_controller.rb | 8 ++- config/locales/client.en.yml | 3 ++ config/routes.rb | 1 + lib/guardian/user_guardian.rb | 4 ++ lib/topic_query.rb | 8 +++ spec/requests/list_controller_spec.rb | 52 +++++++++++++++++++ 13 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/routes/user-private-messages-warnings.js diff --git a/app/assets/javascripts/discourse/app/controllers/user-private-messages.js b/app/assets/javascripts/discourse/app/controllers/user-private-messages.js index 222b819c772..3cc26e8361a 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-private-messages.js +++ b/app/assets/javascripts/discourse/app/controllers/user-private-messages.js @@ -5,6 +5,7 @@ import I18n from "I18n"; import Topic from "discourse/models/topic"; import bootbox from "bootbox"; import discourseComputed from "discourse-common/utils/decorators"; +import { VIEW_NAME_WARNINGS } from "discourse/routes/user-private-messages-warnings"; export default Controller.extend({ userTopicsList: controller("user-topics-list"), @@ -27,6 +28,11 @@ export default Controller.extend({ return bulkSelectEnabled && selected && selected.length > 0; }, + @discourseComputed("viewingSelf", "pmView", "currentUser.admin") + showWarningsWarning(viewingSelf, pmView, isAdmin) { + return pmView === VIEW_NAME_WARNINGS && !viewingSelf && !isAdmin; + }, + bulkOperation(operation) { const selected = this.selected; let params = { type: operation }; diff --git a/app/assets/javascripts/discourse/app/routes/app-route-map.js b/app/assets/javascripts/discourse/app/routes/app-route-map.js index 2bce5afc37d..6ccf9c94bdd 100644 --- a/app/assets/javascripts/discourse/app/routes/app-route-map.js +++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js @@ -149,6 +149,7 @@ export default function () { function () { this.route("sent"); this.route("archive"); + this.route("warnings"); this.route("group", { path: "group/:name" }); this.route("groupArchive", { path: "group/:name/archive" }); this.route("tags"); diff --git a/app/assets/javascripts/discourse/app/routes/user-private-messages-warnings.js b/app/assets/javascripts/discourse/app/routes/user-private-messages-warnings.js new file mode 100644 index 00000000000..a53e9b1017e --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/user-private-messages-warnings.js @@ -0,0 +1,9 @@ +import createPMRoute from "discourse/routes/build-private-messages-route"; + +export const VIEW_NAME_WARNINGS = "warnings"; + +export default createPMRoute( + VIEW_NAME_WARNINGS, + "private-messages-warnings", + null /* no message bus notifications */ +); diff --git a/app/assets/javascripts/discourse/app/templates/user.hbs b/app/assets/javascripts/discourse/app/templates/user.hbs index 0b6f752bf9f..1635bde2516 100644 --- a/app/assets/javascripts/discourse/app/templates/user.hbs +++ b/app/assets/javascripts/discourse/app/templates/user.hbs @@ -38,7 +38,11 @@ {{/if}} {{#if model.warnings_received_count}} -
{{model.warnings_received_count}}{{i18n "user.staff_counters.warnings_received"}}
+
+ {{#link-to "userPrivateMessages.warnings" model}} + {{model.warnings_received_count}}{{i18n "user.staff_counters.warnings_received"}} + {{/link-to}} +
{{/if}} {{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/user/messages.hbs b/app/assets/javascripts/discourse/app/templates/user/messages.hbs index 004b58b4eba..23eaded6dd0 100644 --- a/app/assets/javascripts/discourse/app/templates/user/messages.hbs +++ b/app/assets/javascripts/discourse/app/templates/user/messages.hbs @@ -87,5 +87,8 @@ }} {{/if}} + {{#if showWarningsWarning}} +
{{html-safe (i18n "admin.user.warnings_list_warning")}}
+ {{/if}} {{outlet}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-test.js index 7c4137a6dfe..371c8be8f36 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-test.js @@ -121,3 +121,18 @@ acceptance( }); } ); + +acceptance("User Routes - Moderator viewing warnings", function (needs) { + needs.user({ + username: "notEviltrout", + moderator: true, + staff: true, + admin: false, + }); + + test("Messages - Warnings", async function (assert) { + await visit("/u/eviltrout/messages/warnings"); + assert.ok($("body.user-messages-page").length, "has the body class"); + assert.ok($("div.alert-info").length, "has the permissions alert"); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js index 4e2b1ad305a..2b843d62672 100644 --- a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js +++ b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js @@ -210,6 +210,10 @@ export function applyDefaultHandlers(pretender) { return response(fixturesByUrl["/topics/private-messages/eviltrout.json"]); }); + pretender.get("/topics/private-messages-warnings/eviltrout.json", () => { + return response(fixturesByUrl["/topics/private-messages/eviltrout.json"]); + }); + pretender.get("/topics/feature_stats.json", () => { return response({ pinned_in_category_count: 0, diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index f9f41f3d068..ecad047db9a 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -168,7 +168,12 @@ class ListController < ApplicationController def message_route(action) target_user = fetch_user_from_params({ include_inactive: current_user.try(:staff?) }, [:user_stat, :user_option]) - guardian.ensure_can_see_private_messages!(target_user.id) + case action + when :private_messages_warnings + guardian.ensure_can_see_warnings!(target_user) + else + guardian.ensure_can_see_private_messages!(target_user.id) + end list_opts = build_topic_list_options list = generate_list_for(action.to_s, target_user, list_opts) url_prefix = "topics" @@ -185,6 +190,7 @@ class ListController < ApplicationController private_messages_group private_messages_group_archive private_messages_tag + private_messages_warnings }.each do |action| generate_message_route(action) end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 57ef0b2c24f..6fc103c8862 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1177,6 +1177,7 @@ en: failed_to_move: "Failed to move selected messages (perhaps your network is down)" select_all: "Select All" tags: "Tags" + warnings: "Official Warnings" preferences_nav: account: "Account" @@ -4912,6 +4913,8 @@ en: flags_given_count: Flags Given flags_received_count: Flags Received warnings_received_count: Warnings Received + warnings_list_warning: | + As a moderator, you may not be able to view all of these topics. If necessary, ask an admin or the issuing moderator to give @moderators access to the message. flags_given_received_count: "Flags Given / Received" approve: "Approve" approved_by: "approved by" diff --git a/config/routes.rb b/config/routes.rb index 6946889d644..ee1b2644dde 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -762,6 +762,7 @@ Discourse::Application.routes.draw do get "private-messages-archive/:username" => "list#private_messages_archive", as: "topics_private_messages_archive", defaults: { format: :json } get "private-messages-unread/:username" => "list#private_messages_unread", as: "topics_private_messages_unread", defaults: { format: :json } get "private-messages-tags/:username/:tag_id.json" => "list#private_messages_tag", as: "topics_private_messages_tag", defaults: { format: :json } + get "private-messages-warnings/:username" => "list#private_messages_warnings", as: "topics_private_messages_warnings", defaults: { format: :json } get "groups/:group_name" => "list#group_topics", as: "group_topics", group_name: RouteFormat.username scope "/private-messages-group/:username", group_name: RouteFormat.username do diff --git a/lib/guardian/user_guardian.rb b/lib/guardian/user_guardian.rb index 250adfe1793..17a908984a5 100644 --- a/lib/guardian/user_guardian.rb +++ b/lib/guardian/user_guardian.rb @@ -84,6 +84,10 @@ module UserGuardian can_merge_user?(source_user) && !target_user.nil? end + def can_see_warnings?(user) + user && (is_me?(user) || is_staff?) + end + def can_reset_bounce_score?(user) user && is_staff? end diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 5a648b9f270..dd402eb23b9 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -365,6 +365,14 @@ class TopicQuery create_list(:private_messages, {}, list) end + def list_private_messages_warnings(user) + list = private_messages_for(user, :user) + list = list.where('topics.subtype = ?', TopicSubtype.moderator_warning) + # Exclude official warnings that the user created, instead of received + list = list.where('topics.user_id <> ?', user.id) + create_list(:private_messages, {}, list) + end + def list_category_topic_ids(category) query = default_results(category: category.id) pinned_ids = query.where('topics.pinned_at IS NOT NULL AND topics.category_id = ?', category.id).limit(nil).order('pinned_at DESC').pluck(:id) diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 5f692988789..283ca35c167 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -703,6 +703,58 @@ RSpec.describe ListController do end end + describe "#private_messages_warnings" do + let(:target_user) { Fabricate(:user) } + let(:admin) { Fabricate(:admin) } + let(:moderator1) { Fabricate(:moderator) } + let(:moderator2) { Fabricate(:moderator) } + + let(:create_args) do + { title: 'you need a warning buddy!', + raw: "you did something bad and I'm telling you about it!", + is_warning: true, + target_usernames: target_user.username, + archetype: Archetype.private_message } + end + + let(:warning_post) do + creator = PostCreator.new(moderator1, create_args) + creator.create + end + let(:warning_topic) { warning_post.topic } + + before do + warning_topic + end + + it "returns 403 error for unrelated users" do + sign_in(Fabricate(:user)) + get "/topics/private-messages-warnings/#{target_user.username}.json" + expect(response.status).to eq(403) + end + + it "shows the warning to moderators and admins" do + [moderator1, moderator2, admin].each do |viewer| + sign_in(viewer) + get "/topics/private-messages-warnings/#{target_user.username}.json" + + expect(response.status).to eq(200) + json = response.parsed_body + expect(json["topic_list"]["topics"].size).to eq(1) + expect(json["topic_list"]["topics"][0]["id"]).to eq(warning_topic.id) + end + end + + it "does not show the warning as applying to the authoring moderator" do + sign_in(admin) + get "/topics/private-messages-warnings/#{moderator1.username}.json" + + expect(response.status).to eq(200) + json = response.parsed_body + expect(json["topic_list"]["topics"].size).to eq(0) + end + end + describe 'read' do it 'raises an error when not logged in' do get "/read"