From dd175537f3fb5f4a62a5935f65443fc9bedb37e3 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Mon, 1 Feb 2021 16:16:34 +0200 Subject: [PATCH] FIX: Existing shared drafts should be accessible (#11915) Disabling shared drafts used to leave topics in an inconsistent state where they were not displayed as shared drafts and thus there was no way of publishing them. Moreover, they were accessible just to users who have permissions to create shared drafts. This commit adds another permission check that is used for most operations and the old can_create_shared_draft? remains used just when creating a new shared draft. --- .../components/shared-draft-controls.hbs | 2 +- app/controllers/list_controller.rb | 2 +- app/serializers/site_serializer.rb | 2 +- app/serializers/topic_view_serializer.rb | 2 +- config/locales/client.en.yml | 2 +- lib/guardian/post_guardian.rb | 2 +- lib/guardian/topic_guardian.rb | 10 ++++--- lib/topic_query.rb | 3 +- .../guardian/topic_guardian_spec.rb | 29 +++++++++++++++++++ 9 files changed, 42 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/discourse/app/templates/components/shared-draft-controls.hbs b/app/assets/javascripts/discourse/app/templates/components/shared-draft-controls.hbs index 03bc3de807b..4501449c1b1 100644 --- a/app/assets/javascripts/discourse/app/templates/components/shared-draft-controls.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/shared-draft-controls.hbs @@ -2,7 +2,7 @@ {{#if publishing}} {{i18n "shared_drafts.publishing"}} {{else}} - {{html-safe (i18n "shared_drafts.notice" category=topic.category.name)}} + {{i18n "shared_drafts.notice"}}
diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index 6fc25d672e3..0db1872368a 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -58,7 +58,7 @@ class ListController < ApplicationController list = TopicQuery.new(user, list_opts).public_send("list_#{filter}") - if guardian.can_create_shared_draft? && @category.present? + if guardian.can_see_shared_draft? && @category.present? if @category.id == SiteSetting.shared_drafts_category.to_i # On shared drafts, show the destination category list.topics.each do |t| diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index f7915050aa9..7aa736c9b42 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -172,7 +172,7 @@ class SiteSerializer < ApplicationSerializer end def include_shared_drafts_category_id? - scope.can_create_shared_draft? + scope.can_see_shared_draft? end private diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 6aff785ce19..7691d7e10ac 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -237,7 +237,7 @@ class TopicViewSerializer < ApplicationSerializer end def include_destination_category_id? - scope.can_create_shared_draft? && object.topic.shared_draft.present? + scope.can_see_shared_draft? && object.topic.shared_draft.present? end def is_shared_draft diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 65a8c55f8e2..760b5799f08 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1855,7 +1855,7 @@ en: shared_drafts: title: "Shared Drafts" - notice: "This topic is only visible to those who can see the %{category} category." + notice: "This topic is only visible to those who can publish shared drafts." destination_category: "Destination Category" publish: "Publish Shared Draft" confirm_publish: "Are you sure you want to publish this draft?" diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 7af04242671..5ee297eec0d 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -142,7 +142,7 @@ module PostGuardian can_create_post?(post.topic) && post.topic.category_id == SiteSetting.shared_drafts_category.to_i && can_see_category?(post.topic.category) && - can_create_shared_draft? + can_see_shared_draft? ) if post.wiki && (@user.trust_level >= SiteSetting.min_trust_to_edit_wiki_post.to_i) diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index d295fd91dcf..ea6c1bc8b7f 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -22,8 +22,10 @@ module TopicGuardian alias :can_moderate_topic? :can_review_topic? def can_create_shared_draft? - return false unless SiteSetting.shared_drafts_enabled? + SiteSetting.shared_drafts_enabled? && can_see_shared_draft? + end + def can_see_shared_draft? return is_admin? if SiteSetting.shared_drafts_min_trust_level.to_s == 'admin' return is_staff? if SiteSetting.shared_drafts_min_trust_level.to_s == 'staff' @@ -39,7 +41,7 @@ module TopicGuardian end def can_publish_topic?(topic, category) - can_create_shared_draft? && can_see?(topic) && can_create_topic_on_category?(category) + can_see_shared_draft? && can_see?(topic) && can_create_topic_on_category?(category) end # Creating Methods @@ -102,7 +104,7 @@ module TopicGuardian !topic.private_message? && topic.category_id == SiteSetting.shared_drafts_category.to_i && can_see_category?(topic.category) && - can_create_shared_draft? && + can_see_shared_draft? && can_create_post?(topic) ) @@ -176,7 +178,7 @@ module TopicGuardian return authenticated? && topic.all_allowed_users.where(id: @user.id).exists? end - return false if topic.shared_draft && !can_create_shared_draft? + return false if topic.shared_draft && !can_see_shared_draft? category = topic.category can_see_category?(category) && diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 812d03de21b..a12229e0453 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -617,9 +617,8 @@ class TopicQuery drafts_category_id = SiteSetting.shared_drafts_category.to_i viewing_shared = category_id && category_id == drafts_category_id - can_create_shared = guardian.can_create_shared_draft? - if can_create_shared + if guardian.can_see_shared_draft? if options[:destination_category_id] destination_category_id = get_category_id(options[:destination_category_id]) topic_ids = SharedDraft.where(category_id: destination_category_id).pluck(:topic_id) diff --git a/spec/components/guardian/topic_guardian_spec.rb b/spec/components/guardian/topic_guardian_spec.rb index 60bc247291d..c42d44a0796 100644 --- a/spec/components/guardian/topic_guardian_spec.rb +++ b/spec/components/guardian/topic_guardian_spec.rb @@ -37,6 +37,35 @@ describe TopicGuardian do end end + describe '#can_see_shared_draft?' do + it 'when shared_drafts are disabled (existing shared drafts)' do + SiteSetting.shared_drafts_min_trust_level = 'admin' + + expect(Guardian.new(admin).can_see_shared_draft?).to eq(true) + end + + it 'when user is a moderator and access is set to admin' do + SiteSetting.shared_drafts_category = category.id + SiteSetting.shared_drafts_min_trust_level = 'admin' + + expect(Guardian.new(moderator).can_see_shared_draft?).to eq(false) + end + + it 'when user is a moderator and access is set to staff' do + SiteSetting.shared_drafts_category = category.id + SiteSetting.shared_drafts_min_trust_level = 'staff' + + expect(Guardian.new(moderator).can_see_shared_draft?).to eq(true) + end + + it 'when user is TL3 and access is set to TL2' do + SiteSetting.shared_drafts_category = category.id + SiteSetting.shared_drafts_min_trust_level = '2' + + expect(Guardian.new(tl3_user).can_see_shared_draft?).to eq(true) + end + end + describe '#can_edit_topic?' do context 'when the topic is a shared draft' do let(:tl2_user) { Fabricate(:user, trust_level: TrustLevel[2]) }