From af3e61fe75916439d5530c2592de24a5b294d81f Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 8 Jun 2021 21:39:49 +1000 Subject: [PATCH] Add custom field improvements (#115) * Add custom field improvements This PR does a few things to improve our support of custom fields 1. Adds custom fields added by other plugins to the list in admin/wizards/custom-fields and the custom field list in the mapper selector 2. Adds support for json custom fields in the wizard actions * Make eslint happy * Make prettier happy * Make rubocop happy * Make ember template lint happy * Don't assume we have the context in the selector * Ensure custom fields don't require optional attributes (with tests) --- .../components/custom-field-input.js.es6 | 8 +-- .../components/wizard-custom-action.js.es6 | 5 ++ .../components/wizard-mapper-selector.js.es6 | 41 +++++++++++++-- .../admin-wizards-custom-fields.js.es6 | 4 +- .../javascripts/discourse/lib/wizard.js.es6 | 1 + .../routes/admin-wizards-wizard-show.js.es6 | 5 +- .../components/custom-field-input.hbs | 36 ++++++++----- .../components/wizard-custom-action.hbs | 2 +- assets/stylesheets/common/wizard-admin.scss | 4 ++ config/locales/client.en.yml | 5 +- controllers/custom_wizard/admin/admin.rb | 2 +- extensions/custom_field/extension.rb | 6 +++ lib/custom_wizard/action.rb | 41 +++++++++++---- lib/custom_wizard/custom_field.rb | 40 +++++++++++++-- plugin.rb | 11 ++-- spec/components/custom_wizard/action_spec.rb | 36 +++++++++++++ .../custom_wizard/custom_field_spec.rb | 50 +++++++++++++++++++ spec/fixtures/wizard.json | 30 +++++++++-- .../admin/custom_fields_controller_spec.rb | 4 +- 19 files changed, 276 insertions(+), 55 deletions(-) create mode 100644 extensions/custom_field/extension.rb diff --git a/assets/javascripts/discourse/components/custom-field-input.js.es6 b/assets/javascripts/discourse/components/custom-field-input.js.es6 index f2dca4c7..e49c6f1d 100644 --- a/assets/javascripts/discourse/components/custom-field-input.js.es6 +++ b/assets/javascripts/discourse/components/custom-field-input.js.es6 @@ -1,6 +1,6 @@ import Component from "@ember/component"; import discourseComputed, { observes } from "discourse-common/utils/decorators"; -import { alias, or } from "@ember/object/computed"; +import { alias, equal, or } from "@ember/object/computed"; import I18n from "I18n"; const generateContent = function (array, type) { @@ -29,6 +29,7 @@ export default Component.extend({ loading: or("saving", "destroying"), destroyDisabled: alias("loading"), closeDisabled: alias("loading"), + isExternal: equal("field.id", "external"), didInsertElement() { this.set("originalField", JSON.parse(JSON.stringify(this.field))); @@ -61,13 +62,14 @@ export default Component.extend({ @discourseComputed( "saving", + "isExternal", "field.name", "field.klass", "field.type", "field.serializers" ) - saveDisabled(saving) { - if (saving) { + saveDisabled(saving, isExternal) { + if (saving || isExternal) { return true; } diff --git a/assets/javascripts/discourse/components/wizard-custom-action.js.es6 b/assets/javascripts/discourse/components/wizard-custom-action.js.es6 index c8309f10..feb83754 100644 --- a/assets/javascripts/discourse/components/wizard-custom-action.js.es6 +++ b/assets/javascripts/discourse/components/wizard-custom-action.js.es6 @@ -62,6 +62,11 @@ export default Component.extend(UndoChanges, { return key; }, + @discourseComputed("action.type") + customFieldsContext(type) { + return `action.${type}`; + }, + @discourseComputed("wizard.steps") runAfterContent(steps) { let content = steps.map(function (step) { diff --git a/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 b/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 index 6d65d782..7d9b0bbd 100644 --- a/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 +++ b/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 @@ -6,11 +6,24 @@ import { } from "discourse-common/utils/decorators"; import { getOwner } from "discourse-common/lib/get-owner"; import { defaultSelectionType, selectionTypes } from "../lib/wizard-mapper"; -import { generateName, snakeCase, userProperties } from "../lib/wizard"; +import { + generateName, + sentenceCase, + snakeCase, + userProperties, +} from "../lib/wizard"; import Component from "@ember/component"; import { bind, later } from "@ember/runloop"; import I18n from "I18n"; +const customFieldActionMap = { + topic: ["create_topic", "send_message"], + post: ["create_topic", "send_message"], + category: ["create_category"], + group: ["create_group"], + user: ["update_profile"], +}; + export default Component.extend({ classNameBindings: [":mapper-selector", "activeType"], @@ -188,11 +201,19 @@ export default Component.extend({ customFields ) { let content; + let context; + let contextType; + + if (this.options.context) { + let contextAttrs = this.options.context.split("."); + context = contextAttrs[0]; + contextType = contextAttrs[1]; + } if (activeType === "wizardField") { content = wizardFields; - if (this.options.context === "field") { + if (context === "field") { content = content.filter((field) => field.id !== currentFieldId); } } @@ -204,7 +225,7 @@ export default Component.extend({ type: a.type, })); - if (this.options.context === "action") { + if (context === "action") { content = content.filter((a) => a.id !== currentActionId); } } @@ -218,7 +239,7 @@ export default Component.extend({ .concat(userFields || []); if ( - this.options.context === "action" && + context === "action" && this.inputType === "association" && this.selectorType === "key" ) { @@ -234,7 +255,17 @@ export default Component.extend({ } if (activeType === "customField") { - content = customFields; + content = customFields + .filter((f) => { + return ( + f.type !== "json" && + customFieldActionMap[f.klass].includes(contextType) + ); + }) + .map((f) => ({ + id: f.name, + name: `${sentenceCase(f.klass)} ${f.name} (${f.type})`, + })); } return content; diff --git a/assets/javascripts/discourse/controllers/admin-wizards-custom-fields.js.es6 b/assets/javascripts/discourse/controllers/admin-wizards-custom-fields.js.es6 index 2081cfe3..404c6afd 100644 --- a/assets/javascripts/discourse/controllers/admin-wizards-custom-fields.js.es6 +++ b/assets/javascripts/discourse/controllers/admin-wizards-custom-fields.js.es6 @@ -3,12 +3,12 @@ import CustomWizardCustomField from "../models/custom-wizard-custom-field"; export default Controller.extend({ messageKey: "create", - fieldKeys: ["klass", "type", "serializers", "name"], + fieldKeys: ["klass", "type", "name", "serializers"], documentationUrl: "https://thepavilion.io/t/3572", actions: { addField() { - this.get("customFields").pushObject( + this.get("customFields").unshiftObject( CustomWizardCustomField.create({ edit: true }) ); }, diff --git a/assets/javascripts/discourse/lib/wizard.js.es6 b/assets/javascripts/discourse/lib/wizard.js.es6 index 1896b1fe..98bdbfdd 100644 --- a/assets/javascripts/discourse/lib/wizard.js.es6 +++ b/assets/javascripts/discourse/lib/wizard.js.es6 @@ -120,4 +120,5 @@ export { listProperties, notificationLevels, wizardFieldList, + sentenceCase, }; diff --git a/assets/javascripts/discourse/routes/admin-wizards-wizard-show.js.es6 b/assets/javascripts/discourse/routes/admin-wizards-wizard-show.js.es6 index eaa6591c..cb2d54c3 100644 --- a/assets/javascripts/discourse/routes/admin-wizards-wizard-show.js.es6 +++ b/assets/javascripts/discourse/routes/admin-wizards-wizard-show.js.es6 @@ -2,7 +2,6 @@ import CustomWizard from "../models/custom-wizard"; import { ajax } from "discourse/lib/ajax"; import DiscourseRoute from "discourse/routes/discourse"; import I18n from "I18n"; -import { selectKitContent } from "../lib/wizard"; export default DiscourseRoute.extend({ model(params) { @@ -33,9 +32,7 @@ export default DiscourseRoute.extend({ wizardList: parentModel.wizard_list, fieldTypes, userFields: parentModel.userFields, - customFields: selectKitContent( - parentModel.custom_fields.map((f) => f.name) - ), + customFields: parentModel.custom_fields, apis: parentModel.apis, themes: parentModel.themes, wizard, diff --git a/assets/javascripts/discourse/templates/components/custom-field-input.hbs b/assets/javascripts/discourse/templates/components/custom-field-input.hbs index 205b1644..43a97be8 100644 --- a/assets/javascripts/discourse/templates/components/custom-field-input.hbs +++ b/assets/javascripts/discourse/templates/components/custom-field-input.hbs @@ -13,6 +13,11 @@ none="admin.wizard.custom_field.type.select" onChange=(action (mut field.type))}} + + {{input + value=field.name + placeholder=(i18n "admin.wizard.custom_field.name.select")}} + {{multi-select value=field.serializers @@ -20,11 +25,6 @@ none="admin.wizard.custom_field.serializers.select" onChange=(action (mut field.serializers))}} - - {{input - value=field.name - placeholder=(i18n "admin.wizard.custom_field.name.select")}} - {{#if loading}} {{loading-spinner size="small"}} @@ -51,13 +51,25 @@ {{else}} - - {{#each field.serializers as |serializer|}} - - {{/each}} - - - {{d-button action="edit" icon="pencil-alt"}} + + {{#if isExternal}} + — + {{else}} + {{#each field.serializers as |serializer|}} + + {{/each}} + {{/if}} + {{#if isExternal}} + + + + {{else}} + + {{d-button action="edit" icon="pencil-alt"}} + + {{/if}} {{/if}} diff --git a/assets/javascripts/discourse/templates/components/wizard-custom-action.hbs b/assets/javascripts/discourse/templates/components/wizard-custom-action.hbs index f06e0d89..4c645cf7 100644 --- a/assets/javascripts/discourse/templates/components/wizard-custom-action.hbs +++ b/assets/javascripts/discourse/templates/components/wizard-custom-action.hbs @@ -738,7 +738,7 @@ wizardActionSelection="value" userFieldSelection="value" keyPlaceholder="admin.wizard.action.custom_fields.key" - context="action" + context=customFieldsContext )}} diff --git a/assets/stylesheets/common/wizard-admin.scss b/assets/stylesheets/common/wizard-admin.scss index 3c4b78da..9c2838eb 100644 --- a/assets/stylesheets/common/wizard-admin.scss +++ b/assets/stylesheets/common/wizard-admin.scss @@ -667,6 +667,10 @@ margin-left: 5px !important; } } + + td.external { + font-style: italic; + } } } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 0c364853..43b86698 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -75,7 +75,7 @@ en: edit: "You're editing an action" documentation: "Check out the action documentation" custom_fields: - create: "Create, edit or destroy a custom field record" + create: "View, create, edit and destroy custom fields" saved: "Saved custom field" error: "Failed to save: {{messages}}" documentation: Check out the custom field documentation @@ -322,6 +322,9 @@ en: custom_field: nav_label: "Custom Fields" add: "Add" + external: + label: "from another plugin" + title: "This custom field has been added by another plugin. You can use it in your wizards but you can't edit the field here." name: label: "Name" select: "underscored_name" diff --git a/controllers/custom_wizard/admin/admin.rb b/controllers/custom_wizard/admin/admin.rb index 8d5e3cad..c99954d6 100644 --- a/controllers/custom_wizard/admin/admin.rb +++ b/controllers/custom_wizard/admin/admin.rb @@ -14,7 +14,7 @@ class CustomWizard::AdminController < ::Admin::AdminController end def custom_field_list - serialize_data(CustomWizard::CustomField.list, CustomWizard::CustomFieldSerializer) + serialize_data(CustomWizard::CustomField.full_list, CustomWizard::CustomFieldSerializer) end def render_error(message) diff --git a/extensions/custom_field/extension.rb b/extensions/custom_field/extension.rb new file mode 100644 index 00000000..876f56d4 --- /dev/null +++ b/extensions/custom_field/extension.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true +module CustomWizardCustomFieldExtension + def custom_field_types + @custom_field_types + end +end diff --git a/lib/custom_wizard/action.rb b/lib/custom_wizard/action.rb index d68e978b..5388a326 100644 --- a/lib/custom_wizard/action.rb +++ b/lib/custom_wizard/action.rb @@ -454,32 +454,51 @@ class CustomWizard::Action data: data, user: user ).perform - - registered_fields = CustomWizard::CustomField.cached_list + registered_fields = CustomWizard::CustomField.full_list field_map.each do |field| keyArr = field[:key].split('.') value = field[:value] if keyArr.length > 1 - klass = keyArr.first - name = keyArr.last + klass = keyArr.first.to_sym + name = keyArr.second + + if keyArr.length === 3 && name.include?("{}") + name = name.gsub("{}", "") + json_attr = keyArr.last + type = :json + end else name = keyArr.first end - registered = registered_fields.select { |f| f[:name] == name } - if registered.first.present? - klass = registered.first[:klass] + registered = registered_fields.select { |f| f.name == name }.first + if registered.present? + klass = registered.klass + type = registered.type end - if klass === 'topic' + next if type === :json && json_attr.blank? + + if klass === :topic params[:topic_opts] ||= {} params[:topic_opts][:custom_fields] ||= {} - params[:topic_opts][:custom_fields][name] = value + + if type === :json + params[:topic_opts][:custom_fields][name] ||= {} + params[:topic_opts][:custom_fields][name][json_attr] = value + else + params[:topic_opts][:custom_fields][name] = value + end else - params[:custom_fields] ||= {} - params[:custom_fields][name] = value + if type === :json + params[:custom_fields][name] ||= {} + params[:custom_fields][name][json_attr] = value + else + params[:custom_fields] ||= {} + params[:custom_fields][name] = value + end end end end diff --git a/lib/custom_wizard/custom_field.rb b/lib/custom_wizard/custom_field.rb index e3f01a1a..9cc185ba 100644 --- a/lib/custom_wizard/custom_field.rb +++ b/lib/custom_wizard/custom_field.rb @@ -66,10 +66,12 @@ class ::CustomWizard::CustomField value = send(attr) i18n_key = "wizard.custom_field.error" - if value.blank? - if REQUIRED.include?(attr) - add_error(I18n.t("#{i18n_key}.required_attribute", attr: attr)) - end + if value.blank? && REQUIRED.include?(attr) + add_error(I18n.t("#{i18n_key}.required_attribute", attr: attr)) + break + end + + if attr == 'serializers' && !value.is_a?(Array) next end @@ -140,7 +142,7 @@ class ::CustomWizard::CustomField fields.select do |cf| if attr == :serializers - cf[attr].include?(value) + cf[attr] && cf[attr].include?(value) else cf[attr] == value end @@ -215,4 +217,32 @@ class ::CustomWizard::CustomField def self.enabled? any? end + + def self.external_list + external = [] + + CLASSES.keys.each do |klass| + field_types = klass.to_s.classify.constantize.custom_field_types + + if field_types.present? + field_types.each do |name, type| + unless list.any? { |field| field.name === name } + field = new( + 'external', + name: name, + klass: klass, + type: type + ) + external.push(field) + end + end + end + end + + external + end + + def self.full_list + (list + external_list).uniq + end end diff --git a/plugin.rb b/plugin.rb index 0beab1cc..a4278f65 100644 --- a/plugin.rb +++ b/plugin.rb @@ -91,6 +91,7 @@ after_initialize do ../extensions/users_controller.rb ../extensions/custom_field/preloader.rb ../extensions/custom_field/serializer.rb + ../extensions/custom_field/extension.rb ].each do |path| load File.expand_path(path, __FILE__) end @@ -183,18 +184,18 @@ after_initialize do end CustomWizard::CustomField::CLASSES.keys.each do |klass| + class_constant = klass.to_s.classify.constantize + add_model_callback(klass, :after_initialize) do if CustomWizard::CustomField.enabled? CustomWizard::CustomField.list_by(:klass, klass.to_s).each do |field| - klass.to_s - .classify - .constantize - .register_custom_field_type(field[:name], field[:type].to_sym) + class_constant.register_custom_field_type(field[:name], field[:type].to_sym) end end end - klass.to_s.classify.constantize.singleton_class.prepend CustomWizardCustomFieldPreloader + class_constant.singleton_class.prepend CustomWizardCustomFieldPreloader + class_constant.singleton_class.prepend CustomWizardCustomFieldExtension end CustomWizard::CustomField.serializers.each do |serializer_klass| diff --git a/spec/components/custom_wizard/action_spec.rb b/spec/components/custom_wizard/action_spec.rb index 28f2cab8..9910c0bd 100644 --- a/spec/components/custom_wizard/action_spec.rb +++ b/spec/components/custom_wizard/action_spec.rb @@ -72,6 +72,42 @@ describe CustomWizard::Action do raw: "topic body" ).exists?).to eq(false) end + + it "adds custom fields" do + wizard = CustomWizard::Builder.new(@template[:id], user).build + wizard.create_updater(wizard.steps.first.id, + step_1_field_1: "Topic Title", + step_1_field_2: "topic body" + ).update + wizard.create_updater(wizard.steps.second.id, {}).update + wizard.create_updater(wizard.steps.last.id, + step_3_field_3: category.id + ).update + + topic = Topic.where( + title: "Topic Title", + category_id: category.id + ).first + topic_custom_field = TopicCustomField.where( + name: "topic_field", + value: "Topic custom field value", + topic_id: topic.id + ) + topic_json_custom_field = TopicCustomField.where(" + name = 'topic_json_field' AND + (value::json->>'key_1') = 'Key 1 value' AND + (value::json->>'key_2') = 'Key 2 value' AND + topic_id = #{topic.id}" + ) + post_custom_field = PostCustomField.where( + name: "post_field", + value: "Post custom field value", + post_id: topic.first_post.id + ) + expect(topic_custom_field.exists?).to eq(true) + expect(topic_json_custom_field.exists?).to eq(true) + expect(post_custom_field.exists?).to eq(true) + end end context 'sending a message' do diff --git a/spec/components/custom_wizard/custom_field_spec.rb b/spec/components/custom_wizard/custom_field_spec.rb index 3c9f1706..b17e26c6 100644 --- a/spec/components/custom_wizard/custom_field_spec.rb +++ b/spec/components/custom_wizard/custom_field_spec.rb @@ -49,6 +49,40 @@ describe CustomWizard::CustomField do end context "validation" do + it "does not save without required attributes" do + invalid_field_json = custom_field_json['custom_fields'].first + invalid_field_json['klass'] = nil + + custom_field = CustomWizard::CustomField.new(nil, invalid_field_json) + expect(custom_field.save).to eq(false) + expect(custom_field.valid?).to eq(false) + expect(custom_field.errors.full_messages.first).to eq( + I18n.t("wizard.custom_field.error.required_attribute", attr: "klass") + ) + expect( + PluginStoreRow.where( + plugin_name: CustomWizard::CustomField::NAMESPACE, + key: custom_field.name + ).exists? + ).to eq(false) + end + + it "does save without optional attributes" do + field_json = custom_field_json['custom_fields'].first + field_json['serializers'] = nil + + custom_field = CustomWizard::CustomField.new(nil, field_json) + expect(custom_field.save).to eq(true) + expect(custom_field.valid?).to eq(true) + expect( + PluginStoreRow.where(" + plugin_name = '#{CustomWizard::CustomField::NAMESPACE}' AND + key = '#{custom_field.name}' AND + value::jsonb = '#{field_json.except('name').to_json}'::jsonb + ",).exists? + ).to eq(true) + end + it "does not save with an unsupported class" do invalid_field_json = custom_field_json['custom_fields'].first invalid_field_json['klass'] = 'user' @@ -178,6 +212,22 @@ describe CustomWizard::CustomField do it "lists saved custom field records by attribute value" do expect(CustomWizard::CustomField.list_by(:klass, 'topic').length).to eq(1) end + + it "lists saved custom field records by optional values" do + field_json = custom_field_json['custom_fields'].first + field_json['serializers'] = nil + + custom_field = CustomWizard::CustomField.new(nil, field_json) + expect(CustomWizard::CustomField.list_by(:serializers, ['post']).length).to eq(0) + end + + it "lists custom field records added by other plugins " do + expect(CustomWizard::CustomField.external_list.length).to eq(11) + end + + it "lists all custom field records" do + expect(CustomWizard::CustomField.full_list.length).to eq(15) + end end it "is enabled if there are custom fields" do diff --git a/spec/fixtures/wizard.json b/spec/fixtures/wizard.json index c21d445c..9a1fab40 100644 --- a/spec/fixtures/wizard.json +++ b/spec/fixtures/wizard.json @@ -391,10 +391,34 @@ "pairs": [ { "index": 0, - "key": "custom_field_1", + "key": "post_field", "key_type": "text", - "value": "title", - "value_type": "user_field", + "value": "Post custom field value", + "value_type": "text", + "connector": "association" + }, + { + "index": 1, + "key": "topic.topic_field", + "key_type": "text", + "value": "Topic custom field value", + "value_type": "text", + "connector": "association" + }, + { + "index": 2, + "key": "topic.topic_json_field{}.key_1", + "key_type": "text", + "value": "Key 1 value", + "value_type": "text", + "connector": "association" + }, + { + "index": 3, + "key": "topic.topic_json_field{}.key_2", + "key_type": "text", + "value": "Key 2 value", + "value_type": "text", "connector": "association" } ] diff --git a/spec/requests/custom_wizard/admin/custom_fields_controller_spec.rb b/spec/requests/custom_wizard/admin/custom_fields_controller_spec.rb index e006e65a..8c1a8550 100644 --- a/spec/requests/custom_wizard/admin/custom_fields_controller_spec.rb +++ b/spec/requests/custom_wizard/admin/custom_fields_controller_spec.rb @@ -17,9 +17,9 @@ describe CustomWizard::AdminCustomFieldsController do sign_in(admin_user) end - it "returns the list of custom fields" do + it "returns the full list of custom fields" do get "/admin/wizards/custom-fields.json" - expect(response.parsed_body.length).to eq(4) + expect(response.parsed_body.length).to eq(15) end it "saves custom fields" do