FIX: Make category change work with shared drafts (#11705)

It used to change the category of the topic, instead of the destination
category (topic.category_id instead of topic.shared_draft.category_id).

The shared drafts controls were displayed only if the current category
matched the 'shared drafts category', which was not true for shared
drafts that had their categories changed (affected by the previous bug).

app/assets/javascripts/discourse/app/components/shared-draft-controls.js

@@ -21,11 +21,15 @@ export default Component.extend({
       bootbox.confirm(I18n.t("shared_drafts.confirm_publish"), (result) => {
         if (result) {
           this.set("publishing", true);
-          let destId = this.get("topic.destination_category_id");
+          const destinationCategoryId = this.topic.destination_category_id;
           this.topic
             .publish()
             .then(() => {
-              this.set("topic.category_id", destId);
+              this.topic.setProperties({
+                category_id: destinationCategoryId,
+                destination_category_id: undefined,
+                is_shared_draft: false,
+              });
             })
             .finally(() => {
               this.set("publishing", false);

app/assets/javascripts/discourse/app/controllers/topic.js

@@ -111,21 +111,9 @@ export default Controller.extend(bufferedProperty("model"), {
     }
   },
 
-  @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 &&
-      isSharedDraft
-    );
+  @discourseComputed("model.postStream.loaded", "model.is_shared_draft")
+  showSharedDraftControls(loaded, isSharedDraft) {
+    return loaded && isSharedDraft;
   },
 
   @discourseComputed("site.mobileView", "model.posts_count")

app/assets/javascripts/discourse/app/models/topic.js

@@ -719,6 +719,10 @@ Topic.reopenClass({
       // The title can be cleaned up server side
       props.title = result.basic_topic.title;
       props.fancy_title = result.basic_topic.fancy_title;
+      if (topic.is_shared_draft) {
+        props.destination_category_id = props.category_id;
+        delete props.category_id;
+      }
       topic.setProperties(props);
     });
   },

app/assets/javascripts/discourse/tests/acceptance/shared-drafts-test.js

@@ -4,7 +4,7 @@ import selectKit from "discourse/tests/helpers/select-kit-helper";
 import { test } from "qunit";
 
 acceptance("Shared Drafts", function () {
-  test("Viewing", async function (assert) {
+  test("Viewing and publishing", async function (assert) {
     await visit("/t/some-topic/9");
     assert.ok(queryAll(".shared-draft-controls").length === 1);
     let categoryChooser = selectKit(".shared-draft-controls .category-chooser");
@@ -15,4 +15,20 @@ acceptance("Shared Drafts", function () {
 
     assert.ok(queryAll(".shared-draft-controls").length === 0);
   });
+
+  test("Updating category", async function (assert) {
+    await visit("/t/some-topic/9");
+    assert.ok(queryAll(".shared-draft-controls").length === 1);
+
+    await click(".edit-topic");
+
+    let categoryChooser = selectKit(".edit-topic-title .category-chooser");
+    await categoryChooser.expand();
+    await categoryChooser.selectRowByValue(7);
+
+    await click(".edit-controls .btn-primary");
+
+    categoryChooser = selectKit(".shared-draft-controls .category-chooser");
+    assert.equal(categoryChooser.header().value(), "7");
+  });
 });

app/controllers/topics_controller.rb

@@ -319,39 +319,44 @@ class TopicsController < ApplicationController
     guardian.ensure_can_edit!(topic)
 
     if params[:category_id] && (params[:category_id].to_i != topic.category_id.to_i)
-      category = Category.find_by(id: params[:category_id])
-
-      if category || (params[:category_id].to_i == 0)
-        guardian.ensure_can_move_topic_to_category!(category)
+      if topic.shared_draft
+        topic.shared_draft.update(category_id: params[:category_id])
+        params.delete(:category_id)
       else
-        return render_json_error(I18n.t('category.errors.not_found'))
-      end
+        category = Category.find_by(id: params[:category_id])
 
-      if category && topic_tags = (params[:tags] || topic.tags.pluck(:name)).reject { |c| c.empty? }
-        if topic_tags.present?
-          allowed_tags = DiscourseTagging.filter_allowed_tags(
-            guardian,
-            category: category
-          ).map(&:name)
+        if category || (params[:category_id].to_i == 0)
+          guardian.ensure_can_move_topic_to_category!(category)
+        else
+          return render_json_error(I18n.t('category.errors.not_found'))
+        end
 
-          invalid_tags = topic_tags - allowed_tags
+        if category && topic_tags = (params[:tags] || topic.tags.pluck(:name)).reject { |c| c.empty? }
+          if topic_tags.present?
+            allowed_tags = DiscourseTagging.filter_allowed_tags(
+              guardian,
+              category: category
+            ).map(&:name)
 
-          # Do not raise an error on a topic's hidden tags when not modifying tags
-          if params[:tags].blank?
-            invalid_tags.each do |tag_name|
-              if DiscourseTagging.hidden_tag_names.include?(tag_name)
-                invalid_tags.delete(tag_name)
+            invalid_tags = topic_tags - allowed_tags
+
+            # Do not raise an error on a topic's hidden tags when not modifying tags
+            if params[:tags].blank?
+              invalid_tags.each do |tag_name|
+                if DiscourseTagging.hidden_tag_names.include?(tag_name)
+                  invalid_tags.delete(tag_name)
+                end
               end
             end
-          end
 
-          invalid_tags = Tag.where_name(invalid_tags).pluck(:name)
+            invalid_tags = Tag.where_name(invalid_tags).pluck(:name)
 
-          if !invalid_tags.empty?
-            if (invalid_tags & DiscourseTagging.hidden_tag_names).present?
-              return render_json_error(I18n.t('category.errors.disallowed_tags_generic'))
-            else
-              return render_json_error(I18n.t('category.errors.disallowed_topic_tags', tags: invalid_tags.join(", ")))
+            if !invalid_tags.empty?
+              if (invalid_tags & DiscourseTagging.hidden_tag_names).present?
+                return render_json_error(I18n.t('category.errors.disallowed_tags_generic'))
+              else
+                return render_json_error(I18n.t('category.errors.disallowed_topic_tags', tags: invalid_tags.join(", ")))
+              end
             end
           end
         end

app/serializers/topic_view_serializer.rb

@@ -237,14 +237,13 @@ class TopicViewSerializer < ApplicationSerializer
   end
 
   def include_destination_category_id?
-    scope.can_create_shared_draft? &&
-      object.topic.category_id == SiteSetting.shared_drafts_category.to_i &&
-      object.topic.shared_draft.present?
+    scope.can_create_shared_draft? && 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?

spec/requests/topics_controller_spec.rb

@@ -1205,6 +1205,21 @@ RSpec.describe TopicsController do
         expect(topic.reload.category_id).not_to eq(category.id)
       end
 
+      context 'updating shared drafts' do
+        fab!(:shared_drafts_category) { Fabricate(:category) }
+        fab!(:topic) { Fabricate(:topic, category: shared_drafts_category) }
+        fab!(:shared_draft) { Fabricate(:shared_draft, topic: topic, category: Fabricate(:category)) }
+
+        it 'changes destination category' do
+          category = Fabricate(:category)
+
+          put "/t/#{topic.id}.json", params: { category_id: category.id }
+
+          expect(response.status).to eq(403)
+          expect(topic.shared_draft.category_id).not_to eq(category.id)
+        end
+      end
+
       describe 'without permission' do
         it "raises an exception when the user doesn't have permission to update the topic" do
           topic.update!(archived: true)