diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index 996e5def746..a09f99261e8 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -110,10 +110,21 @@ export default Controller.extend(bufferedProperty("model"), { } }, - @discourseComputed("model.postStream.loaded", "model.category_id") - showSharedDraftControls(loaded, categoryId) { + @discourseComputed( + "model.postStream.loaded", + "model.category_id", + "model.is_shared_draft" + ) + showSharedDraftControls(loaded, categoryId, isSharedDraft) { let draftCat = this.site.shared_drafts_category_id; - return loaded && draftCat && categoryId && draftCat === categoryId; + + return ( + loaded && + draftCat && + categoryId && + draftCat === categoryId && + isSharedDraft + ); }, @discourseComputed("site.mobileView", "model.posts_count") diff --git a/app/assets/javascripts/discourse/tests/fixtures/topic.js b/app/assets/javascripts/discourse/tests/fixtures/topic.js index 34915dd7e02..fbb7f696c9f 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/topic.js +++ b/app/assets/javascripts/discourse/tests/fixtures/topic.js @@ -4032,6 +4032,7 @@ export default { archetype: "regular", slug: "this-is-a-test-topic", category_id: 24, + is_shared_draft: true, word_count: 15, deleted_at: null, user_id: 1, diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index 910e2ed5d84..6fc25d672e3 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -62,7 +62,7 @@ class ListController < ApplicationController if @category.id == SiteSetting.shared_drafts_category.to_i # On shared drafts, show the destination category list.topics.each do |t| - t.includes_destination_category = true + t.includes_destination_category = t.shared_draft.present? end else # When viewing a non-shared draft category, find topics whose diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index e5a2f9387a9..fa778fda331 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -168,6 +168,8 @@ class TopicsController < ApplicationController topic = Topic.find(params[:id]) category = Category.find(params[:destination_category_id]) + raise Discourse::InvalidParameters if category.id == SiteSetting.shared_drafts_category.to_i + guardian.ensure_can_publish_topic!(topic, category) topic = TopicPublisher.new(topic, current_user, category.id).publish! diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 5be4ccc4c05..19e288bf71f 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -74,7 +74,8 @@ class TopicViewSerializer < ApplicationSerializer :show_read_indicator, :requested_group_name, :thumbnails, - :user_last_posted_at + :user_last_posted_at, + :is_shared_draft ) has_one :details, serializer: TopicViewDetailsSerializer, root: false, embed: :objects @@ -241,6 +242,11 @@ class TopicViewSerializer < ApplicationSerializer object.topic.shared_draft.present? end + def is_shared_draft + include_destination_category_id? + end + alias_method :include_is_shared_draft?, :include_destination_category_id? + def include_pending_posts? scope.authenticated? && object.queued_posts_enabled end diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index ed64497d7da..01cb6c815f0 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -176,6 +176,8 @@ module TopicGuardian return authenticated? && topic.all_allowed_users.where(id: @user.id).exists? end + return false if topic.shared_draft && !can_create_shared_draft? + category = topic.category can_see_category?(category) && (!category.read_restricted || !is_staged? || secure_category_ids.include?(category.id) || topic.user == user) diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index 993abba96de..30c8de2dd4e 100644 --- a/lib/topic_creator.rb +++ b/lib/topic_creator.rb @@ -56,8 +56,10 @@ class TopicCreator private def create_shared_draft(topic) - return unless @opts[:shared_draft] && @opts[:category].present? - SharedDraft.create(topic_id: topic.id, category_id: @opts[:category]) + return if @opts[:shared_draft].blank? || @opts[:shared_draft] == 'false' + + category_id = @opts[:category].blank? ? SiteSetting.shared_drafts_category.to_i : @opts[:category] + SharedDraft.create(topic_id: topic.id, category_id: category_id) end def create_warning(topic) diff --git a/lib/topic_query.rb b/lib/topic_query.rb index f5253eadad7..812d03de21b 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -619,15 +619,23 @@ class TopicQuery viewing_shared = category_id && category_id == drafts_category_id can_create_shared = guardian.can_create_shared_draft? - if can_create_shared && 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) - result.where(id: topic_ids) - elsif can_create_shared && viewing_shared - result.includes(:shared_draft).references(:shared_draft) - else - result.where('topics.category_id != ?', drafts_category_id) + if can_create_shared + 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) + + return result.where(id: topic_ids) + end + + if viewing_shared + return result.includes(:shared_draft).references(:shared_draft) + end + + elsif viewing_shared + return result.joins('LEFT OUTER JOIN shared_drafts sd ON sd.topic_id = topics.id').where('sd.id IS NULL') end + + result.where('topics.category_id != ?', drafts_category_id) end def apply_ordering(result, options) diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index f4f13b6a888..67ba06024f0 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -1257,6 +1257,7 @@ describe TopicQuery do shared_drafts_category.set_permissions(group => :full) shared_drafts_category.save SiteSetting.shared_drafts_category = shared_drafts_category.id + SiteSetting.shared_drafts_min_trust_level = TrustLevel[3] end context "destination_category_id" do @@ -1269,6 +1270,24 @@ describe TopicQuery do list = TopicQuery.new(admin, destination_category_id: category.id).list_latest expect(list.topics).to include(topic) end + + it 'allow group members with enough trust level to query destination_category_id' do + member = Fabricate(:user, trust_level: TrustLevel[3]) + group.add(member) + + list = TopicQuery.new(member, destination_category_id: category.id).list_latest + + expect(list.topics).to include(topic) + end + + it "doesn't allow group members without enough trust level to query destination_category_id" do + member = Fabricate(:user, trust_level: TrustLevel[2]) + group.add(member) + + list = TopicQuery.new(member, destination_category_id: category.id).list_latest + + expect(list.topics).not_to include(topic) + end end context "latest" do @@ -1287,6 +1306,14 @@ describe TopicQuery do list = TopicQuery.new(user).list_latest expect(list.topics).not_to include(topic) end + + it "doesn't include shared draft topics for group members with access to shared drafts" do + member = Fabricate(:user, trust_level: TrustLevel[3]) + group.add(member) + + list = TopicQuery.new(member).list_latest + expect(list.topics).not_to include(topic) + end end context "unread" do diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 36ab0ee0021..4dc874c3543 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -3605,6 +3605,11 @@ RSpec.describe TopicsController do expect(result.category_id).to eq(category.id) expect(result.visible).to eq(true) end + + it 'fails if the destination category is the shared drafts category' do + put "/t/#{topic.id}/publish.json", params: { destination_category_id: shared_drafts_category.id } + expect(response.status).to eq(400) + end end end end