FIX: Couldn't move a topic into the uncategorized category.

lib/guardian/topic_guardian.rb

@@ -20,8 +20,7 @@ module TopicGuardian
 
   def can_create_topic_on_category?(category)
     # allow for category to be a number as well
-    category_id = category
+    category_id = Category === category ? category.id : category
-    category_id = category.id if Category === category
 
     can_create_topic?(nil) &&
     (!category || Category.topic_create_allowed(self).where(id: category_id).count == 1)

lib/post_revisor.rb

@@ -69,7 +69,7 @@ class PostRevisor
   end
 
   track_topic_field(:category_id) do |tc, category_id|
-    if tc.guardian.can_create_topic_on_category?(category_id)
+    if category_id == 0 || tc.guardian.can_create_topic_on_category?(category_id)
       tc.record_change('category_id', tc.topic.category_id, category_id)
       tc.check_result(tc.topic.change_category_to_id(category_id))
     end

spec/controllers/topics_controller_spec.rb

@@ -1089,127 +1089,6 @@ describe TopicsController do
     end
   end
 
-  describe 'update' do
-    it "won't allow us to update a topic when we're not logged in" do
-      put :update, params: { topic_id: 1, slug: 'xyz' }, format: :json
-      expect(response.status).to eq(403)
-    end
-
-    describe 'when logged in' do
-      before do
-        @topic = Fabricate(:topic, user: log_in)
-        Fabricate(:post, topic: @topic)
-      end
-
-      describe 'without permission' do
-        it "raises an exception when the user doesn't have permission to update the topic" do
-          Guardian.any_instance.expects(:can_edit?).with(@topic).returns(false)
-
-          put :update, params: {
-            topic_id: @topic.id, slug: @topic.title
-          }, format: :json
-
-          expect(response).to be_forbidden
-        end
-      end
-
-      describe 'with permission' do
-        before do
-          Guardian.any_instance.expects(:can_edit?).with(@topic).returns(true)
-        end
-
-        it 'succeeds' do
-          put :update, params: {
-            topic_id: @topic.id, slug: @topic.title
-          }, format: :json
-
-          expect(response).to be_success
-          expect(::JSON.parse(response.body)['basic_topic']).to be_present
-        end
-
-        it 'allows a change of title' do
-          put :update, params: {
-            topic_id: @topic.id, slug: @topic.title, title: 'This is a new title for the topic'
-          }, format: :json
-
-          @topic.reload
-          expect(@topic.title).to eq('This is a new title for the topic')
-        end
-
-        it "returns errors with invalid titles" do
-          put :update, params: {
-            topic_id: @topic.id, slug: @topic.title, title: 'asdf'
-          }, format: :json
-
-          expect(response).not_to be_success
-        end
-
-        it "returns errors when the rate limit is exceeded" do
-          EditRateLimiter.any_instance.expects(:performed!).raises(RateLimiter::LimitExceeded.new(60))
-          put :update, params: {
-            topic_id: @topic.id, slug: @topic.title, title: 'This is a new title for the topic'
-          }, format: :json
-
-          expect(response).not_to be_success
-        end
-
-        it "returns errors with invalid categories" do
-          put :update, params: {
-            topic_id: @topic.id, slug: @topic.title, category_id: -1
-          }, format: :json
-
-          expect(response).not_to be_success
-        end
-
-        it "doesn't call the PostRevisor when there is no changes" do
-          PostRevisor.any_instance.expects(:revise!).never
-          put :update, params: {
-            topic_id: @topic.id, slug: @topic.title, title: @topic.title, category_id: @topic.category_id
-          }, format: :json
-
-          expect(response).to be_success
-        end
-
-        context 'when topic is private' do
-          before do
-            @topic.archetype = Archetype.private_message
-            @topic.category = nil
-            @topic.save!
-          end
-
-          context 'when there are no changes' do
-            it 'does not call the PostRevisor' do
-              PostRevisor.any_instance.expects(:revise!).never
-
-              put :update, params: {
-                topic_id: @topic.id, slug: @topic.title, title: @topic.title, category_id: @topic.category_id
-              }, format: :json
-
-              expect(response).to be_success
-            end
-          end
-        end
-
-        context "allow_uncategorized_topics is false" do
-          before do
-            SiteSetting.allow_uncategorized_topics = false
-          end
-
-          it "can add a category to an uncategorized topic" do
-            c = Fabricate(:category)
-
-            put :update, params: {
-              topic_id: @topic.id, slug: @topic.title, category_id: c.id
-            }, format: :json
-
-            expect(response).to be_success
-          end
-        end
-
-      end
-    end
-  end
-
   describe 'invite_group' do
     let :admins do
       Group[:admins]

spec/requests/topics_controller_spec.rb

@@ -6,20 +6,144 @@ RSpec.describe TopicsController do
 
   describe '#update' do
 
-    it 'can not change category to a disallowed category' do
+    it "won't allow us to update a topic when we're not logged in" do
-      post = create_post
+      put "/t/1.json", params: { slug: 'xyz' }
-      sign_in(post.user)
+      expect(response.status).to eq(403)
-
-      category = Fabricate(:category)
-      category.set_permissions(staff: :full)
-      category.save!
-
-      put "/t/#{post.topic_id}.json", params: { category_id: category.id }
-      expect(response.status).not_to eq(200)
-
-      expect(post.topic.category_id).not_to eq(category.id)
     end
 
+    describe 'when logged in' do
+      let(:topic) { Fabricate(:topic, user: user) }
+
+      before do
+        Fabricate(:post, topic: topic)
+        sign_in(user)
+      end
+
+      it 'can not change category to a disallowed category' do
+        category = Fabricate(:category)
+        category.set_permissions(staff: :full)
+        category.save!
+
+        put "/t/#{topic.id}.json", params: { category_id: category.id }
+
+        expect(response.status).not_to eq(200)
+        expect(topic.category_id).not_to eq(category.id)
+      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)
+          put "/t/#{topic.slug}/#{topic.id}.json"
+
+          expect(response.status).to eq(403)
+        end
+      end
+
+      describe 'with permission' do
+        it 'succeeds' do
+          put "/t/#{topic.slug}/#{topic.id}.json"
+
+          expect(response.status).to eq(200)
+          expect(::JSON.parse(response.body)['basic_topic']).to be_present
+        end
+
+        it "can update a topic to an uncategorized topic" do
+          topic.update!(category: Fabricate(:category))
+
+          put "/t/#{topic.slug}/#{topic.id}.json", params: {
+            category_id: ""
+          }
+
+          expect(response.status).to eq(200)
+          expect(topic.reload.category_id).to eq(SiteSetting.uncategorized_category_id)
+        end
+
+        it 'allows a change of title' do
+          put "/t/#{topic.slug}/#{topic.id}.json", params: {
+            title: 'This is a new title for the topic'
+          }
+
+          topic.reload
+          expect(topic.title).to eq('This is a new title for the topic')
+        end
+
+        it "returns errors with invalid titles" do
+          put "/t/#{topic.slug}/#{topic.id}.json", params: {
+            title: 'asdf'
+          }
+
+          expect(response.status).to eq(422)
+          expect(JSON.parse(response.body)['errors']).to be_present
+        end
+
+        it "returns errors when the rate limit is exceeded" do
+          EditRateLimiter.any_instance.expects(:performed!).raises(RateLimiter::LimitExceeded.new(60))
+
+          put "/t/#{topic.slug}/#{topic.id}.json", params: {
+            title: 'This is a new title for the topic'
+          }
+
+          expect(response.status).to eq(429)
+        end
+
+        it "returns errors with invalid categories" do
+          put "/t/#{topic.slug}/#{topic.id}.json", params: {
+            category_id: -1
+          }
+
+          expect(response.status).to eq(422)
+        end
+
+        it "doesn't call the PostRevisor when there is no changes" do
+          PostRevisor.any_instance.expects(:revise!).never
+
+          put "/t/#{topic.slug}/#{topic.id}.json", params: {
+            category_id: topic.category_id
+          }
+
+          expect(response.status).to eq(200)
+        end
+
+        context 'when topic is private' do
+          before do
+            topic.update!(
+              archetype: Archetype.private_message,
+              category: nil,
+              allowed_users: [topic.user]
+            )
+          end
+
+          context 'when there are no changes' do
+            it 'does not call the PostRevisor' do
+              PostRevisor.any_instance.expects(:revise!).never
+
+              put "/t/#{topic.slug}/#{topic.id}.json", params: {
+                category_id: topic.category_id
+              }
+
+              expect(response.status).to eq(200)
+            end
+          end
+        end
+
+        context "allow_uncategorized_topics is false" do
+          before do
+            SiteSetting.allow_uncategorized_topics = false
+          end
+
+          it "can add a category to an uncategorized topic" do
+            category = Fabricate(:category)
+
+            put "/t/#{topic.slug}/#{topic.id}.json", params: {
+              category_id: category.id
+            }
+
+            expect(response.status).to eq(200)
+            expect(topic.reload.category).to eq(category)
+          end
+        end
+      end
+    end
   end
 
   describe '#show' do