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.
This commit is contained in:
Dan Ungureanu
2021-02-01 16:16:34 +02:00
committed by GitHub
parent f113648107
commit dd175537f3
9 changed files with 42 additions and 12 deletions

View File

@ -2,7 +2,7 @@
{{#if publishing}} {{#if publishing}}
{{i18n "shared_drafts.publishing"}} {{i18n "shared_drafts.publishing"}}
{{else}} {{else}}
{{html-safe (i18n "shared_drafts.notice" category=topic.category.name)}} {{i18n "shared_drafts.notice"}}
<div class="publish-field"> <div class="publish-field">
<label>{{i18n "shared_drafts.destination_category"}}</label> <label>{{i18n "shared_drafts.destination_category"}}</label>

View File

@ -58,7 +58,7 @@ class ListController < ApplicationController
list = TopicQuery.new(user, list_opts).public_send("list_#{filter}") 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 if @category.id == SiteSetting.shared_drafts_category.to_i
# On shared drafts, show the destination category # On shared drafts, show the destination category
list.topics.each do |t| list.topics.each do |t|

View File

@ -172,7 +172,7 @@ class SiteSerializer < ApplicationSerializer
end end
def include_shared_drafts_category_id? def include_shared_drafts_category_id?
scope.can_create_shared_draft? scope.can_see_shared_draft?
end end
private private

View File

@ -237,7 +237,7 @@ class TopicViewSerializer < ApplicationSerializer
end end
def include_destination_category_id? 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 end
def is_shared_draft def is_shared_draft

View File

@ -1855,7 +1855,7 @@ en:
shared_drafts: shared_drafts:
title: "Shared Drafts" title: "Shared Drafts"
notice: "This topic is only visible to those who can see the <b>%{category}</b> category." notice: "This topic is only visible to those who can publish shared drafts."
destination_category: "Destination Category" destination_category: "Destination Category"
publish: "Publish Shared Draft" publish: "Publish Shared Draft"
confirm_publish: "Are you sure you want to publish this draft?" confirm_publish: "Are you sure you want to publish this draft?"

View File

@ -142,7 +142,7 @@ module PostGuardian
can_create_post?(post.topic) && can_create_post?(post.topic) &&
post.topic.category_id == SiteSetting.shared_drafts_category.to_i && post.topic.category_id == SiteSetting.shared_drafts_category.to_i &&
can_see_category?(post.topic.category) && 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) if post.wiki && (@user.trust_level >= SiteSetting.min_trust_to_edit_wiki_post.to_i)

View File

@ -22,8 +22,10 @@ module TopicGuardian
alias :can_moderate_topic? :can_review_topic? alias :can_moderate_topic? :can_review_topic?
def can_create_shared_draft? 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_admin? if SiteSetting.shared_drafts_min_trust_level.to_s == 'admin'
return is_staff? if SiteSetting.shared_drafts_min_trust_level.to_s == 'staff' return is_staff? if SiteSetting.shared_drafts_min_trust_level.to_s == 'staff'
@ -39,7 +41,7 @@ module TopicGuardian
end end
def can_publish_topic?(topic, category) 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 end
# Creating Methods # Creating Methods
@ -102,7 +104,7 @@ module TopicGuardian
!topic.private_message? && !topic.private_message? &&
topic.category_id == SiteSetting.shared_drafts_category.to_i && topic.category_id == SiteSetting.shared_drafts_category.to_i &&
can_see_category?(topic.category) && can_see_category?(topic.category) &&
can_create_shared_draft? && can_see_shared_draft? &&
can_create_post?(topic) can_create_post?(topic)
) )
@ -176,7 +178,7 @@ module TopicGuardian
return authenticated? && topic.all_allowed_users.where(id: @user.id).exists? return authenticated? && topic.all_allowed_users.where(id: @user.id).exists?
end end
return false if topic.shared_draft && !can_create_shared_draft? return false if topic.shared_draft && !can_see_shared_draft?
category = topic.category category = topic.category
can_see_category?(category) && can_see_category?(category) &&

View File

@ -617,9 +617,8 @@ class TopicQuery
drafts_category_id = SiteSetting.shared_drafts_category.to_i drafts_category_id = SiteSetting.shared_drafts_category.to_i
viewing_shared = category_id && category_id == drafts_category_id 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] if options[:destination_category_id]
destination_category_id = get_category_id(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) topic_ids = SharedDraft.where(category_id: destination_category_id).pluck(:topic_id)

View File

@ -37,6 +37,35 @@ describe TopicGuardian do
end end
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 describe '#can_edit_topic?' do
context 'when the topic is a shared draft' do context 'when the topic is a shared draft' do
let(:tl2_user) { Fabricate(:user, trust_level: TrustLevel[2]) } let(:tl2_user) { Fabricate(:user, trust_level: TrustLevel[2]) }