From 2ad2ed2eb2c2152ec4840735f852b2d755a9df69 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 13 Mar 2018 10:20:47 +0800 Subject: [PATCH] FIX: Couldn't move a topic into the uncategorized category. --- lib/guardian/topic_guardian.rb | 3 +- lib/post_revisor.rb | 2 +- spec/controllers/topics_controller_spec.rb | 121 ----------------- spec/requests/topics_controller_spec.rb | 148 +++++++++++++++++++-- 4 files changed, 138 insertions(+), 136 deletions(-) diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index 014e707afd5..6aac2ac216b 100644 --- a/lib/guardian/topic_guardian.rb +++ b/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.id if Category === category + category_id = Category === category ? category.id : category can_create_topic?(nil) && (!category || Category.topic_create_allowed(self).where(id: category_id).count == 1) diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index 48dd64071c8..38b5c56a452 100644 --- a/lib/post_revisor.rb +++ b/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 diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index 85db19b9d3f..5283489aac2 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/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] diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index b4e00c54fea..3df66f4283f 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/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 - post = create_post - sign_in(post.user) - - 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) + it "won't allow us to update a topic when we're not logged in" do + put "/t/1.json", params: { slug: 'xyz' } + expect(response.status).to eq(403) 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