diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index e3e4a21910c..78960fa4064 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -200,6 +200,15 @@ class PostsController < ApplicationController if post.is_first_post? changes[:title] = params[:title] if params[:title] changes[:category_id] = params[:post][:category_id] if params[:post][:category_id] + + if changes[:category_id] && changes[:category_id].to_i != post.topic.category_id.to_i + category = Category.find_by(id: changes[:category_id]) + if category || (changes[:category_id].to_i == 0) + guardian.ensure_can_create_topic_on_category!(category) + else + return render_json_error(I18n.t('category.errors.not_found')) + end + end end # We don't need to validate edits to small action posts by staff diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 8c931afdba0..1f5188eec66 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -226,6 +226,15 @@ class TopicsController < ApplicationController topic = Topic.find_by(id: params[:topic_id]) 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_create_topic_on_category!(category) + else + return render_json_error(I18n.t('category.errors.not_found')) + end + end + changes = {} PostRevisor.tracked_topic_fields.each_key do |f| changes[f] = params[f] if params.has_key?(f) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 04e179adc11..2da213f1b57 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -506,6 +506,7 @@ en: replace_paragraph: "(Replace this first paragraph with a brief description of your new category. This guidance will appear in the category selection area, so try to keep it below 200 characters. **Until you edit this description or create topics, this category won't appear on the categories page.**)" post_template: "%{replace_paragraph}\n\nUse the following paragraphs for a longer description, or to establish category guidelines or rules:\n\n- Why should people use this category? What is it for?\n\n- How exactly is this different than the other categories we already have?\n\n- What should topics in this category generally contain?\n\n- Do we need this category? Can we merge with another category, or subcategory?\n" errors: + not_found: "Category not found!" uncategorized_parent: "Uncategorized can't have a parent category" self_parent: "A subcategory's parent cannot be itself" depth: "You can't nest a subcategory under another" diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index 1774ac7c04f..24b5eaa0de1 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -19,8 +19,12 @@ module TopicGuardian end 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 + can_create_topic?(nil) && - (!category || Category.topic_create_allowed(self).where(id: category.id).count == 1) + (!category || Category.topic_create_allowed(self).where(id: category_id).count == 1) end def can_create_post_on_topic?(topic) diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index 3dd2db5f4f8..79f9ab17cc2 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -69,8 +69,10 @@ class PostRevisor end track_topic_field(:category_id) do |tc, category_id| - tc.record_change('category_id', tc.topic.category_id, category_id) - tc.check_result(tc.topic.change_category_to_id(category_id)) + if 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 end track_topic_field(:tags) do |tc, tags| diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index 84b64825b07..c9a5eab5633 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -38,6 +38,32 @@ describe PostRevisor do end end + context 'editing category' do + + it 'does not revise category when no permission to create a topic in category' do + category = Fabricate(:category) + category.set_permissions(staff: :full) + category.save! + + post = create_post + old_id = post.topic.category_id + + post.revise(post.user, category_id: category.id) + + post.reload + expect(post.topic.category_id).to eq(old_id) + + category.set_permissions(everyone: :full) + category.save! + + post.revise(post.user, category_id: category.id) + + post.reload + expect(post.topic.category_id).to eq(category.id) + end + + end + context 'revise wiki' do before do diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index 0b6722e63eb..85db19b9d3f 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -1136,22 +1136,6 @@ describe TopicsController do expect(@topic.title).to eq('This is a new title for the topic') end - it 'triggers a change of category' do - Topic.any_instance.expects(:change_category_to_id).with(123).returns(true) - put :update, params: { - topic_id: @topic.id, slug: @topic.title, category_id: 123 - }, format: :json - - end - - it 'allows to change category to "uncategorized"' do - Topic.any_instance.expects(:change_category_to_id).with(0).returns(true) - put :update, params: { - topic_id: @topic.id, slug: @topic.title, category_id: "" - }, format: :json - - end - it "returns errors with invalid titles" do put :update, params: { topic_id: @topic.id, slug: @topic.title, title: 'asdf' @@ -1170,7 +1154,6 @@ describe TopicsController do end it "returns errors with invalid categories" do - Topic.any_instance.expects(:change_category_to_id).returns(false) put :update, params: { topic_id: @topic.id, slug: @topic.title, category_id: -1 }, format: :json @@ -1197,8 +1180,9 @@ describe TopicsController do 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: nil + topic_id: @topic.id, slug: @topic.title, title: @topic.title, category_id: @topic.category_id }, format: :json expect(response).to be_success @@ -1212,10 +1196,10 @@ describe TopicsController do end it "can add a category to an uncategorized topic" do - Topic.any_instance.expects(:change_category_to_id).with(456).returns(true) + c = Fabricate(:category) put :update, params: { - topic_id: @topic.id, slug: @topic.title, category_id: 456 + topic_id: @topic.id, slug: @topic.title, category_id: c.id }, format: :json expect(response).to be_success diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 6d290d0e1aa..c7f9d210a05 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -12,11 +12,47 @@ RSpec.describe PostsController do let(:private_post) { Fabricate(:post, user: user, topic: private_topic) } + 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! + + # strange API, why is topic id in here twice + put "/posts/#{post.id}.json", params: { + post: { category_id: category.id, raw: "this is a test edit to post" } + } + + expect(response.status).not_to eq(200) + expect(post.topic.category_id).not_to eq(category.id) + end + + end + describe '#create' do before do sign_in(user) end + it 'can not create a post in a disallowed category' do + + category.set_permissions(staff: :full) + category.save! + + post "/posts.json", params: { + raw: 'this is the test content', + title: 'this is the test title for the topic', + category: category.id, + meta_data: { xyz: 'abc' } + } + + expect(response.status).to eq(403) + end + it 'creates the post' do post "/posts.json", params: { raw: 'this is the test content', diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index a6bfdf56c92..27bbd9a1d25 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -4,6 +4,25 @@ RSpec.describe TopicsController do let(:topic) { Fabricate(:topic) } let(:user) { Fabricate(:user) } + 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! + + # strange API, why is topic id in here twice + put "/t/#{post.topic_id}.json", params: { category_id: category.id, topic_id: post.topic_id } + expect(response.status).not_to eq(200) + + expect(post.topic.category_id).not_to eq(category.id) + end + + end + describe '#show' do let(:private_topic) { Fabricate(:private_message_topic) }