diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 0240cf0791a..9078f72cbc9 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -846,8 +846,22 @@ class PostsController < ApplicationController .permit(*permitted) .tap do |allowed| allowed[:image_sizes] = params[:image_sizes] - # TODO this does not feel right, we should name what meta_data is allowed - allowed[:meta_data] = params[:meta_data] + + if params.has_key?(:meta_data) + Discourse.deprecate( + "the :meta_data param is deprecated, use the :topic_custom_fields param instead", + since: "3.2", + drop_from: "3.3", + ) + end + + topic_custom_fields = {} + topic_custom_fields.merge!(editable_topic_custom_fields(:meta_data)) + topic_custom_fields.merge!(editable_topic_custom_fields(:topic_custom_fields)) + + if topic_custom_fields.present? + allowed[:topic_opts] = { custom_fields: topic_custom_fields } + end end # Staff are allowed to pass `is_warning` @@ -909,6 +923,25 @@ class PostsController < ApplicationController result.to_h end + def editable_topic_custom_fields(params_key) + if (topic_custom_fields = params[params_key]).present? + editable_topic_custom_fields = Topic.editable_custom_fields(guardian) + + if ( + unpermitted_topic_custom_fields = + topic_custom_fields.except(*editable_topic_custom_fields) + ).present? + raise Discourse::InvalidParameters.new( + "The following keys in :#{params_key} are not permitted: #{unpermitted_topic_custom_fields.keys.join(", ")}", + ) + end + + topic_custom_fields.permit(*editable_topic_custom_fields).to_h + else + {} + end + end + def signature_for(args) +"post##" << Digest::SHA1.hexdigest( args diff --git a/app/models/topic.rb b/app/models/topic.rb index d8f458f9728..934d39941d2 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -295,7 +295,6 @@ class Topic < ActiveRecord::Base attr_accessor :participants attr_accessor :participant_groups attr_accessor :topic_list - attr_accessor :meta_data attr_accessor :include_last_poster attr_accessor :import_mode # set to true to optimize creation and save for imports @@ -612,19 +611,6 @@ class Topic < ActiveRecord::Base topics end - def meta_data=(data) - custom_fields.replace(data) - end - - def meta_data - custom_fields - end - - def update_meta_data(data) - custom_fields.update(data) - save - end - def reload(options = nil) @post_numbers = nil @public_topic_timer = nil @@ -2049,6 +2035,13 @@ class Topic < ActiveRecord::Base tags.reject { |tag| guardian.hidden_tag_names.include?(tag[:name]) } end + def self.editable_custom_fields(guardian) + fields = [] + fields.push(*DiscoursePluginRegistry.public_editable_topic_custom_fields) + fields.push(*DiscoursePluginRegistry.staff_editable_topic_custom_fields) if guardian.is_staff? + fields + end + private def invite_to_private_message(invited_by, target_user, guardian) diff --git a/lib/discourse_plugin_registry.rb b/lib/discourse_plugin_registry.rb index 697d93474f4..77ec2c5db87 100644 --- a/lib/discourse_plugin_registry.rb +++ b/lib/discourse_plugin_registry.rb @@ -77,6 +77,9 @@ class DiscoursePluginRegistry define_filtered_register :staff_user_custom_fields define_filtered_register :public_user_custom_fields + define_filtered_register :staff_editable_topic_custom_fields + define_filtered_register :public_editable_topic_custom_fields + define_filtered_register :self_editable_user_custom_fields define_filtered_register :staff_editable_user_custom_fields diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index a3f3887b3ab..ab31017755b 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -197,6 +197,14 @@ class Plugin::Instance DiscoursePluginRegistry.register_public_user_custom_field(field, self) end + def register_editable_topic_custom_field(field, staff_only: false) + if staff_only + DiscoursePluginRegistry.register_staff_editable_topic_custom_field(field, self) + else + DiscoursePluginRegistry.register_public_editable_topic_custom_field(field, self) + end + end + def register_editable_user_custom_field(field, staff_only: false) if staff_only DiscoursePluginRegistry.register_staff_editable_user_custom_field(field, self) diff --git a/lib/post_creator.rb b/lib/post_creator.rb index f1afa7aac72..732e1f2c514 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -50,7 +50,6 @@ class PostCreator # category - Category to assign to topic # target_usernames - comma delimited list of usernames for membership (private message) # target_group_names - comma delimited list of groups for membership (private message) - # meta_data - Topic meta data hash # created_at - Topic creation time (optional) # pinned_at - Topic pinned time (optional) # pinned_globally - Is the topic pinned globally (optional) diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index 24245291c54..3a685b1d452 100644 --- a/lib/topic_creator.rb +++ b/lib/topic_creator.rb @@ -48,7 +48,7 @@ class TopicCreator setup_tags(topic) if fields = @opts[:custom_fields] - topic.custom_fields.merge!(fields) + topic.custom_fields = fields end DiscourseEvent.trigger(:before_create_topic, topic, self) @@ -122,7 +122,7 @@ class TopicCreator visible: @opts[:visible], } - %i[subtype archetype meta_data import_mode advance_draft].each do |key| + %i[subtype archetype import_mode advance_draft].each do |key| topic_params[key] = @opts[key] if @opts[key].present? end diff --git a/spec/lib/post_creator_spec.rb b/spec/lib/post_creator_spec.rb index 4dc8bdde0d1..bd3419d4e40 100644 --- a/spec/lib/post_creator_spec.rb +++ b/spec/lib/post_creator_spec.rb @@ -23,9 +23,6 @@ RSpec.describe PostCreator do let(:creator_with_category) do PostCreator.new(user, basic_topic_params.merge(category: category.id)) end - let(:creator_with_meta_data) do - PostCreator.new(user, basic_topic_params.merge(meta_data: { hello: "world" })) - end let(:creator_with_image_sizes) do PostCreator.new(user, basic_topic_params.merge(image_sizes: image_sizes)) end @@ -96,6 +93,7 @@ RSpec.describe PostCreator do user, basic_topic_params.merge(topic_opts: { custom_fields: { hello: "world" } }), ) + expect(post.topic.custom_fields).to eq("hello" => "world") end @@ -331,10 +329,6 @@ RSpec.describe PostCreator do expect(creator_with_category.create.topic.category).to eq(category) end - it "adds meta data from the post" do - expect(creator_with_meta_data.create.topic.meta_data["hello"]).to eq("world") - end - it "passes the image sizes through" do Post.any_instance.expects(:image_sizes=).with(image_sizes) creator_with_image_sizes.create diff --git a/spec/lib/topic_creator_spec.rb b/spec/lib/topic_creator_spec.rb index 6b128267452..3add4cd33ac 100644 --- a/spec/lib/topic_creator_spec.rb +++ b/spec/lib/topic_creator_spec.rb @@ -40,20 +40,11 @@ RSpec.describe TopicCreator do expect(TopicCreator.create(moderator, Guardian.new(moderator), valid_attrs)).to be_valid end - it "supports both meta_data and custom_fields" do - opts = - valid_attrs.merge( - meta_data: { - import_topic_id: "foo", - }, - custom_fields: { - import_id: "bar", - }, - ) + it "supports custom_fields that has been registered to the DiscoursePluginRegistry" do + opts = valid_attrs.merge(custom_fields: { import_id: "bar" }) topic = TopicCreator.create(admin, Guardian.new(admin), opts) - expect(topic.custom_fields["import_topic_id"]).to eq("foo") expect(topic.custom_fields["import_id"]).to eq("bar") end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index c47624002eb..8f0f308a8ca 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1705,48 +1705,6 @@ RSpec.describe Topic do end end - describe "meta data" do - fab!(:topic) { Fabricate(:topic, meta_data: { "hello" => "world" }) } - - it "allows us to create a topic with meta data" do - expect(topic.meta_data["hello"]).to eq("world") - end - - context "when updating" do - context "with existing key" do - before { topic.update_meta_data("hello" => "bane") } - - it "updates the key" do - expect(topic.meta_data["hello"]).to eq("bane") - end - end - - context "with a new key" do - before { topic.update_meta_data("city" => "gotham") } - - it "adds the new key" do - expect(topic.meta_data["city"]).to eq("gotham") - expect(topic.meta_data["hello"]).to eq("world") - end - end - - context "with a new key" do - before_all do - topic.update_meta_data("other" => "key") - topic.save! - end - - it "can be loaded" do - expect(Topic.find(topic.id).meta_data["other"]).to eq("key") - end - - it "is in sync with custom_fields" do - expect(Topic.find(topic.id).custom_fields["other"]).to eq("key") - end - end - end - end - describe "after create" do fab!(:topic) { Fabricate(:topic) } diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index c641f1b2dd4..b872340990a 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -1333,9 +1333,6 @@ RSpec.describe PostsController do 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) @@ -1407,15 +1404,12 @@ RSpec.describe PostsController do expect(Post.last.topic.tags.count).to eq(1) end - it "creates the post" do + it "creates the topic and post with the right attributes" do 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(200) @@ -1427,10 +1421,112 @@ RSpec.describe PostsController do expect(new_post.raw).to eq("this is the test content") expect(topic.title).to eq("This is the test title for the topic") expect(topic.category).to eq(category) - expect(topic.meta_data).to eq("xyz" => "abc") expect(topic.visible).to eq(true) end + context "when adding custom fields to topic via the `topic_custom_fields` param" do + it "should return a 400 response code when no custom fields has been permitted" do + sign_in(user) + + post "/posts.json", + params: { + raw: "this is the test content", + title: "this is the test title for the topic", + category: category.id, + topic_custom_fields: { + xyz: "abc", + abc: "xyz", + }, + } + + expect(response.status).to eq(400) + expect(Topic.last.custom_fields).to eq({}) + end + + context "when custom fields has been permitted" do + fab!(:plugin) do + plugin = Plugin::Instance.new + plugin.register_editable_topic_custom_field(:xyz) + plugin.register_editable_topic_custom_field(:abc, staff_only: true) + plugin + end + + it "should return a 400 response when trying to add a staff ony custom field for a non-staff user" do + sign_in(user) + + post "/posts.json", + params: { + raw: "this is the test content", + title: "this is the test title for the topic", + category: category.id, + topic_custom_fields: { + abc: "xyz", + }, + } + + expect(response.status).to eq(400) + expect(Topic.last.custom_fields).to eq({}) + end + + it "should add custom fields to topic that is permitted for a non-staff user" do + sign_in(user) + + post "/posts.json", + params: { + raw: "this is the test content", + title: "this is the test title for the topic", + category: category.id, + topic_custom_fields: { + xyz: "abc", + }, + } + + expect(response.status).to eq(200) + expect(Topic.last.custom_fields).to eq({ "xyz" => "abc" }) + end + + it "should add custom fields to topic that is permitted for a non-staff user via the deprecated `meta_data` param" do + sign_in(user) + + Discourse.expects(:deprecate).with( + "the :meta_data param is deprecated, use the :topic_custom_fields param instead", + anything, + ) + + 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(200) + expect(Topic.last.custom_fields).to eq({ "xyz" => "abc" }) + end + + it "should add custom fields to topic that is permitted for a staff user and public user" do + sign_in(Fabricate(:admin)) + + post "/posts.json", + params: { + raw: "this is the test content", + title: "this is the test title for the topic", + category: category.id, + topic_custom_fields: { + xyz: "abc", + abc: "xyz", + }, + } + + expect(response.status).to eq(200) + expect(Topic.last.custom_fields).to eq({ "xyz" => "abc", "abc" => "xyz" }) + end + end + end + it "can create an uncategorized topic" do title = "this is the test title for the topic" @@ -1562,9 +1658,6 @@ RSpec.describe PostsController do raw: "this is the test content http://fakespamwebsite.com http://fakespamwebsite.com/spam http://fakespamwebsite.com/spammy", title: "this is the test title for the topic", - meta_data: { - xyz: "abc", - }, } expect(response.parsed_body["errors"]).to include(I18n.t(:spamming_host))