From e5349e43af24398cab548f269a912de009a1f7a4 Mon Sep 17 00:00:00 2001 From: Isaac Janzen <50783505+janzenisaac@users.noreply.github.com> Date: Thu, 29 Dec 2022 10:07:03 -0600 Subject: [PATCH] DEV: Update group moderator behavior to better mimic staff (#19618) # Context When a topic is reviewable by a group we give those group moderators some admin abilities including the ability to delete a topic. # Problem There are two main problems: 1. Currently when a group moderator deletes a topic they are redirected to root (not the same for staff) 2. Viewing the categories deleted topics (`c/foo/1/?status=deleted`) does not display the deleted topic to the group moderator (not the same for staff). # Fix If the `deleted_by` user is part a group that matches the `reviewable_by_group` on a topic then don't redirect. This is the default interaction for staff to give them the ability to do things like restore the topic in case it was accidentally deleted. To render the deleted topics as expected for the group moderator I am utilizing [the guardian scope of `guardian.can_see_deleted_topics?` for said category](https://github.com/discourse/discourse/pull/19618/files#diff-288e61b8bacdb29d9c2e05b42da6837b0036dcf1867332d977ca7c5e74a44297R802-R803) --- app/assets/javascripts/discourse/app/models/topic.js | 7 ++++++- lib/topic_query.rb | 4 ++-- spec/lib/topic_query_spec.rb | 8 +++++++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index 662a050ceab..f4729483375 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -467,7 +467,12 @@ const Topic = RestModel.extend({ "details.can_permanently_delete": this.siteSettings.can_permanently_delete && deleted_by.admin, }); - if (!deleted_by.staff) { + if ( + !deleted_by.staff && + !deleted_by.groups.some( + (group) => group.name === this.category.reviewable_by_group_name + ) + ) { DiscourseURL.redirectTo("/"); } }) diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 635f05c7d7a..48dced2a69d 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -799,8 +799,8 @@ class TopicQuery when 'unlisted' result = result.where('NOT topics.visible') when 'deleted' - guardian = @guardian - if guardian.is_staff? + category = Category.find_by(id: options[:category]) + if @guardian.can_see_deleted_topics?(category) result = result.where('topics.deleted_at IS NOT NULL') require_deleted_clause = false end diff --git a/spec/lib/topic_query_spec.rb b/spec/lib/topic_query_spec.rb index 3f3b64c41ae..03d970820c1 100644 --- a/spec/lib/topic_query_spec.rb +++ b/spec/lib/topic_query_spec.rb @@ -226,10 +226,16 @@ RSpec.describe TopicQuery do describe 'deleted filter' do it "filters deleted topics correctly" do - _topic = Fabricate(:topic, deleted_at: 1.year.ago) + SiteSetting.enable_category_group_moderation = true + group_moderator = Fabricate(:user) + group = Fabricate(:group) + group.add(group_moderator) + category = Fabricate(:category, reviewable_by_group: group) + topic = Fabricate(:topic, category: category, deleted_at: 1.year.ago) expect(TopicQuery.new(admin, status: 'deleted').list_latest.topics.size).to eq(1) expect(TopicQuery.new(moderator, status: 'deleted').list_latest.topics.size).to eq(1) + expect(TopicQuery.new(group_moderator, status: 'deleted', category: category.id).list_latest.topics.size).to eq(1) expect(TopicQuery.new(user, status: 'deleted').list_latest.topics.size).to eq(0) expect(TopicQuery.new(nil, status: 'deleted').list_latest.topics.size).to eq(0) end