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)

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;
     }
 

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) {

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;

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 })
       );
     },

assets/javascripts/discourse/lib/wizard.js.es6

@@ -120,4 +120,5 @@ export {
   listProperties,
   notificationLevels,
   wizardFieldList,
+  sentenceCase,
 };

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,

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))}}
   </td>
+  <td class="input">
+    {{input
+      value=field.name
+      placeholder=(i18n "admin.wizard.custom_field.name.select")}}
+  </td>
   <td class="multi-select">
     {{multi-select
       value=field.serializers
@@ -20,11 +25,6 @@
       none="admin.wizard.custom_field.serializers.select"
       onChange=(action (mut field.serializers))}}
   </td>
-  <td class="input">
-    {{input
-      value=field.name
-      placeholder=(i18n "admin.wizard.custom_field.name.select")}}
-  </td>
   <td class="actions">
     {{#if loading}}
       {{loading-spinner size="small"}}
@@ -51,13 +51,25 @@
 {{else}}
   <td><label>{{field.klass}}</label></td>
   <td><label>{{field.type}}</label></td>
-  <td class="multi-select">
-    {{#each field.serializers as |serializer|}}
-      <label>{{serializer}}</label>
-    {{/each}}
-  </td>
   <td class="input"><label>{{field.name}}</label></td>
-  <td class="actions">
-    {{d-button action="edit" icon="pencil-alt"}}
+  <td class="multi-select">
+    {{#if isExternal}}
+      &mdash;
+    {{else}}
+      {{#each field.serializers as |serializer|}}
+        <label>{{serializer}}</label>
+      {{/each}}
+    {{/if}}
   </td>
+  {{#if isExternal}}
+    <td class="external">
+      <label title={{i18n "admin.wizard.custom_field.external.title"}}>
+        {{i18n "admin.wizard.custom_field.external.label"}}
+      </label>
+    </td>
+  {{else}}
+    <td class="actions">
+      {{d-button action="edit" icon="pencil-alt"}}
+    </td>
+  {{/if}}
 {{/if}}

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
               )}}
           </div>
         </div>

assets/stylesheets/common/wizard-admin.scss

@@ -667,6 +667,10 @@
         margin-left: 5px !important;
       }
     }
+
+    td.external {
+      font-style: italic;
+    }
   }
 }
 

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"

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)

extensions/custom_field/extension.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+module CustomWizardCustomFieldExtension
+  def custom_field_types
+    @custom_field_types
+  end
+end

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

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

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|

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

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

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"
             }
           ]

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