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

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).

diff --git a/app/assets/javascripts/discourse/app/components/shared-draft-controls.js b/app/assets/javascripts/discourse/app/components/shared-draft-controls.js
index cc0347a..a68fe3c 100644
--- a/app/assets/javascripts/discourse/app/components/shared-draft-controls.js
+++ b/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);
diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js
index a27429c..e050aca 100644
--- a/app/assets/javascripts/discourse/app/controllers/topic.js
+++ b/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")
diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js
index c350c5e..8796bc4 100644
--- a/app/assets/javascripts/discourse/app/models/topic.js
+++ b/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);
     });
   },
diff --git a/app/assets/javascripts/discourse/tests/acceptance/shared-drafts-test.js b/app/assets/javascripts/discourse/tests/acceptance/shared-drafts-test.js
index 5e6d236..493d361 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/shared-drafts-test.js
+++ b/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");
+  });
 });
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 33885ab..fc460e9 100644
--- a/app/controllers/topics_controller.rb
+++ b/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)
-
-          invalid_tags = topic_tags - allowed_tags
+        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
 
-          # 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)
+        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)
+
+            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
diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb
index 19e288b..6aff785 100644
--- a/app/serializers/topic_view_serializer.rb
+++ b/app/serializers/topic_view_serializer.rb
@@ -237,14 +237,13 @@ class TopicViewSerializer < ApplicationSerializer
   end
 
   def include_destination_category_id?
-    scope.can_create_shared_draft? &&

[... diff too long, it was truncated ...]

GitHub sha: c3bab3ef

This commit appears in #11705 which was approved by davidtaylorhq. It was merged by udan11.