From b383538a6bb0cee552162a00eb931cb022478a53 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 10 Nov 2020 11:56:11 +1100 Subject: [PATCH] Move to individual custom field saving --- .../components/custom-field-input.js.es6 | 80 +++++++++++++++++-- .../admin-wizards-custom-fields.js.es6 | 36 ++++++--- .../models/custom-wizard-custom-field.js.es6 | 12 ++- .../templates/admin-wizards-custom-fields.hbs | 17 ++-- .../components/custom-field-input.hbs | 45 +++++++---- assets/stylesheets/common/wizard-admin.scss | 71 ++++++++++++---- config/locales/client.en.yml | 10 ++- .../custom_wizard/admin/custom_fields.rb | 61 +++++++------- coverage/.last_run.json | 2 +- lib/custom_wizard/custom_field.rb | 23 ++++-- plugin.rb | 1 + .../custom_wizard/custom_field_spec.rb | 2 +- .../admin/custom_fields_controller_spec.rb | 22 ++++- .../custom_field_serializer_spec.rb | 2 +- 14 files changed, 268 insertions(+), 116 deletions(-) diff --git a/assets/javascripts/discourse/components/custom-field-input.js.es6 b/assets/javascripts/discourse/components/custom-field-input.js.es6 index 81c626f8..31b8dc7c 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, { discourseObserve } from "discourse-common/utils/decorators"; -import { or } from "@ember/object/computed"; +import discourseComputed, { observes } from "discourse-common/utils/decorators"; +import { or, alias } from "@ember/object/computed"; const generateContent = function(array, type) { return array.map(key => ({ @@ -18,11 +18,19 @@ export default Component.extend({ klassContent: generateContent(['topic', 'post', 'group', 'category'], 'klass'), typeContent: generateContent(['string', 'boolean', 'integer', 'json'], 'type'), showInputs: or('field.new', 'field.edit'), + classNames: ['custom-field-input'], + loading: or('saving', 'destroying'), + destroyDisabled: alias('loading'), + closeDisabled: alias('loading'), + + didInsertElement() { + this.set('originalField', JSON.parse(JSON.stringify(this.field))); + }, @discourseComputed('field.klass') - serializerContent(klass) { + serializerContent(klass, p2) { const serializers = this.get(`${klass}Serializers`); - + if (serializers) { return generateContent(serializers, 'serializers'); } else { @@ -30,6 +38,44 @@ export default Component.extend({ } }, + @observes('field.klass') + clearSerializersWhenClassChanges() { + this.set('field.serializers', null); + }, + + compareArrays(array1, array2) { + return array1.length === array2.length && array1.every((value, index) => { + return value === array2[index]; + }); + }, + + @discourseComputed( + 'saving', + 'field.name', + 'field.klass', + 'field.type', + 'field.serializers' + ) + saveDisabled(saving) { + if (saving) return true; + + const originalField = this.originalField; + if (!originalField) return false; + + return ['name', 'klass', 'type', 'serializers'].every(attr => { + let current = this.get(attr); + let original = originalField[attr]; + + if (!current) return false; + + if (attr == 'serializers') { + return this.compareArrays(current, original); + } else { + return current == original; + } + }); + }, + actions: { edit() { this.set('field.edit', true); @@ -42,8 +88,32 @@ export default Component.extend({ }, destroy() { - this.set('removing', true); + this.set('destroying', true); this.removeField(this.field); + }, + + save() { + this.set('saving', true); + + const field = this.field; + + let data = { + id: field.id, + klass: field.klass, + type: field.type, + serializers: field.serializers, + name: field.name + } + + this.saveField(data).then((result) => { + if (result.success) { + this.set('saveIcon', 'check'); + } else { + this.set('saveIcon', 'times'); + } + setTimeout(() => this.set('saveIcon', null), 10000); + this.set('saving', false); + }); } } }); \ No newline at end of file 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 600a5f9e..55b7a602 100644 --- a/assets/javascripts/discourse/controllers/admin-wizards-custom-fields.js.es6 +++ b/assets/javascripts/discourse/controllers/admin-wizards-custom-fields.js.es6 @@ -3,8 +3,10 @@ import EmberObject from '@ember/object'; import { ajax } from 'discourse/lib/ajax'; import { popupAjaxError } from 'discourse/lib/ajax-error'; import CustomWizardCustomField from "../models/custom-wizard-custom-field"; +import { default as discourseComputed } from 'discourse-common/utils/decorators'; export default Controller.extend({ + messageKey: 'create', fieldKeys: ['klass', 'type', 'serializers', 'name'], documentationUrl: "https://thepavilion.io/t/3572", @@ -15,24 +17,36 @@ export default Controller.extend({ ); }, - saveFields() { - this.set('saving', true); - CustomWizardCustomField.saveFields(this.customFields) + saveField(field) { + return CustomWizardCustomField.saveField(field) .then(result => { if (result.success) { - this.set('saveIcon', 'check'); - } else { - this.set('saveIcon', 'times'); + this.setProperties({ + messageKey: 'saved', + messageType: 'success' + }); + } else { + if (result.messages) { + this.setProperties({ + messageKey: 'error', + messageType: 'error', + messageOpts: { messages: result.messages } + }) + } } - setTimeout(() => this.set('saveIcon', ''), 5000); - this.get('customFields').setEach('edit', false); - }).finally(() => { - this.set('saving', false); + + setTimeout(() => this.setProperties({ + messageKey: 'create', + messageType: null, + messageOpts: null + }), 10000); + + return result; }); }, removeField(field) { - CustomWizardCustomField.removeField(field) + return CustomWizardCustomField.destroyField(field) .then(result => { this.get('customFields').removeObject(field); }); diff --git a/assets/javascripts/discourse/models/custom-wizard-custom-field.js.es6 b/assets/javascripts/discourse/models/custom-wizard-custom-field.js.es6 index 068e943b..95276bf6 100644 --- a/assets/javascripts/discourse/models/custom-wizard-custom-field.js.es6 +++ b/assets/javascripts/discourse/models/custom-wizard-custom-field.js.es6 @@ -14,18 +14,16 @@ CustomWizardCustomField.reopenClass({ return ajax(basePath).catch(popupAjaxError); }, - saveFields(customFields) { + saveField(customField) { return ajax(basePath, { type: 'PUT', - dataType: 'json', - contentType: 'application/json', - data: JSON.stringify({ - custom_fields: customFields - }) + data: { + custom_field: customField + } }).catch(popupAjaxError); }, - removeField(field) { + destroyField(field) { return ajax(`${basePath}/${field.name}`, { type: 'DELETE' }).catch(popupAjaxError); diff --git a/assets/javascripts/discourse/templates/admin-wizards-custom-fields.hbs b/assets/javascripts/discourse/templates/admin-wizards-custom-fields.hbs index 3e451294..4633344b 100644 --- a/assets/javascripts/discourse/templates/admin-wizards-custom-fields.hbs +++ b/assets/javascripts/discourse/templates/admin-wizards-custom-fields.hbs @@ -2,16 +2,6 @@

{{i18n 'admin.wizard.custom_field.nav_label'}}

- {{#if saving}} - {{loading-spinner size="small"}} - {{else}} - {{#if saveIcon}} - {{d-icon saveIcon}} - {{/if}} - {{/if}} - {{d-button - label="admin.wizard.custom_field.save" - action="saveFields"}} {{d-button label="admin.wizard.custom_field.add" icon="plus" @@ -20,7 +10,9 @@
{{wizard-message - key='create' + key=messageKey + opts=messageOpts + type=messageType url=documentationUrl component='custom_fields'}} @@ -36,7 +28,8 @@ {{#each customFields as |field|}} {{custom-field-input field=field - removeField=(action 'removeField')}} + removeField=(action 'removeField') + saveField=(action 'saveField')}} {{/each}} {{/if}} diff --git a/assets/javascripts/discourse/templates/components/custom-field-input.hbs b/assets/javascripts/discourse/templates/components/custom-field-input.hbs index 87b5b0ff..7e6598a0 100644 --- a/assets/javascripts/discourse/templates/components/custom-field-input.hbs +++ b/assets/javascripts/discourse/templates/components/custom-field-input.hbs @@ -13,40 +13,51 @@ none="admin.wizard.custom_field.type.select" onChange=(action (mut field.type))}} - + {{multi-select value=field.serializers content=serializerContent none="admin.wizard.custom_field.serializers.select" onChange=(action (mut field.serializers))}} - + {{input value=field.name - placeholder=(i18n "admin.wizard.custom_field.klass.select")}} + placeholder=(i18n "admin.wizard.custom_field.name.select")}} - - {{d-button action="close" icon="times"}} + + {{#if loading}} + {{loading-spinner size="small"}} + {{else}} + {{#if saveIcon}} + {{d-icon saveIcon}} + {{/if}} + {{/if}} + {{d-button + action="destroy" + icon="trash-alt" + class="destroy" + disabled=destroyDisabled}} + {{d-button + icon="save" + action="save" + disabled=saveDisabled + class="save"}} + {{d-button + action="close" + icon="times" + disabled=closeDisabled}} {{else}} - + {{#each field.serializers as |serializer|}} {{/each}} - - + + {{d-button action="edit" icon="pencil-alt"}} - {{#if field.edit}} - {{d-button action="close" icon="times"}} - {{else}} - {{#if destroying}} - {{loading-spinner size="small"}} - {{else}} - {{d-button action="destroy" icon="trash-alt"}} - {{/if}} - {{/if}} {{/if}} \ No newline at end of file diff --git a/assets/stylesheets/common/wizard-admin.scss b/assets/stylesheets/common/wizard-admin.scss index 03009b0c..8816039b 100644 --- a/assets/stylesheets/common/wizard-admin.scss +++ b/assets/stylesheets/common/wizard-admin.scss @@ -1,6 +1,7 @@ @import 'wizard-mapper'; @import 'wizard-manager'; @import 'wizard-api'; +@import 'common/components/buttons'; .admin-wizard-controls { display: flex; @@ -565,16 +566,35 @@ } .admin-wizards-custom-fields { + .admin-wizard-controls { + .buttons { + display: flex; + align-items: center; + + button.btn { + margin-left: 10px; + } + } + } + + .btn.save:enabled { + @extend .btn-primary; + } + + .btn.destroy { + @extend .btn-danger; + } + h3 { margin-bottom: 0; } .select-kit { - width: 200px; + width: 150px; } .select-kit.multi-select { - width: 200px; + width: 300px; .choices .choice, .select-kit-filter .filter-input { @@ -587,20 +607,39 @@ margin: 0; } - td { - vertical-align: top; + table { + td { + vertical-align: top; + + label { + margin: 0; + line-height: 30px; + display: inline-block; + margin-right: 10px; + } + } - label { - margin: 0; - line-height: 30px; + td { + min-width: 170px; + width: 170px; + } + + td.multi-select { + min-width: 300px; + } + + td.input { + min-width: 210px; + width: 210px; + } + + td.actions { + min-width: 100px; + text-align: right; + + button.btn { + margin-left: 5px !important; + } } } - - td:not(:last-of-type) { - min-width: 230px; - } - - td:last-of-type { - text-align: right; - } -} +} \ No newline at end of file diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 09e7962e..5bd88f24 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -73,7 +73,9 @@ en: edit: "You're editing an action" documentation: "Check out the action documentation" custom_fields: - create: "Create or edit a custom field record" + create: "Create, edit or destroy a custom field record" + saved: "Saved custom field" + error: "Failed to save: {{messages}}" documentation: Check out the custom field documentation manager: info: "Export, import or destroy wizards" @@ -296,11 +298,10 @@ en: custom_field: nav_label: "Custom Fields" - add: "Add Custom Field" - save: "Save Custom Fields" + add: "Add" name: label: "Name" - select: "Enter a name" + select: "underscored_name" type: label: "Type" select: "Select a type" @@ -322,6 +323,7 @@ en: topic_view: "Topic View" topic_list_item: "Topic List Item" basic_category: "Category" + basic_group: "Group" post: "Post" submissions: diff --git a/controllers/custom_wizard/admin/custom_fields.rb b/controllers/custom_wizard/admin/custom_fields.rb index 62abaae5..3cace833 100644 --- a/controllers/custom_wizard/admin/custom_fields.rb +++ b/controllers/custom_wizard/admin/custom_fields.rb @@ -4,39 +4,38 @@ class CustomWizard::AdminCustomFieldsController < CustomWizard::AdminController end def update - fields_to_save = [] - - custom_field_params[:custom_fields].each do |field_param| - field_id = nil - field_data = {} - - if saved_field = CustomWizard::CustomField.find(field_param[:name]) - CustomWizard::CustomField::ATTRS.each do |attr| - field_data[attr] = saved_field.send(attr) - end - field_id = saved_field.id - end - + errors = [] + field_id = nil + field_data = {} + + if saved_field = CustomWizard::CustomField.find(field_params[:id].to_i) CustomWizard::CustomField::ATTRS.each do |attr| - field_data[attr] = field_param[attr] + field_data[attr] = saved_field.send(attr) end - - field = CustomWizard::CustomField.new(field_id, field_data) - fields_to_save.push(field) + field_id = saved_field.id end + CustomWizard::CustomField::ATTRS.each do |attr| + field_data[attr] = field_params[attr] + end + + field = CustomWizard::CustomField.new(field_id, field_data) + PluginStoreRow.transaction do - fields_to_save.each do |field| - unless field.save - raise ActiveRecord::Rollback.new, - field.errors.any? ? - field.errors.full_messages.join("\n\n") : - I18n.t("wizard.custom_field.error.save_default", name: field.name) - end + unless field.save + field_errors = field.errors.any? ? + field.errors.full_messages.join("\n\n") : + I18n.t("wizard.custom_field.error.save_default", name: field.name) + errors << field_errors + raise ActiveRecord::Rollback.new end end - - render json: success_json + + if errors.any? + render json: failed_json.merge(messages: errors) + else + render json: success_json + end end def destroy @@ -51,14 +50,14 @@ class CustomWizard::AdminCustomFieldsController < CustomWizard::AdminController private - def custom_field_params - params.permit( - custom_fields: [ + def field_params + params.required(:custom_field) + .permit( + :id, :name, :klass, :type, serializers: [] - ] - ) + ) end end \ No newline at end of file diff --git a/coverage/.last_run.json b/coverage/.last_run.json index 7082b6e5..9671b9f1 100644 --- a/coverage/.last_run.json +++ b/coverage/.last_run.json @@ -1,5 +1,5 @@ { "result": { - "covered_percent": 89.04 + "covered_percent": 89.03 } } diff --git a/lib/custom_wizard/custom_field.rb b/lib/custom_wizard/custom_field.rb index adaf8115..dd8edc5d 100644 --- a/lib/custom_wizard/custom_field.rb +++ b/lib/custom_wizard/custom_field.rb @@ -7,7 +7,7 @@ class ::CustomWizard::CustomField attr_reader :id ATTRS ||= ["name", "klass", "type", "serializers"] - REQUIRED ||= ["name", "klass"] + REQUIRED ||= ["name", "klass", "type"] NAMESPACE ||= "custom_wizard_custom_fields" NAME_MIN_LENGTH ||= 3 @@ -143,11 +143,21 @@ class ::CustomWizard::CustomField PluginStoreRow.where(plugin_name: NAMESPACE, key: name).exists? end - def self.find(name) - records = PluginStoreRow.where(plugin_name: NAMESPACE, key: name) + def self.find(field_id) + record = PluginStoreRow.find_by(id: field_id, plugin_name: NAMESPACE) - if records.exists? - create_from_store(records.first) + if record + create_from_store(record) + else + false + end + end + + def self.find_by_name(name) + record = PluginStoreRow.find_by(key: name, plugin_name: NAMESPACE) + + if record + create_from_store(record) else false end @@ -161,8 +171,9 @@ class ::CustomWizard::CustomField def self.save_to_store(id = nil, key, data) if id - record = PluginStoreRow.find_by(id: id, plugin_name: NAMESPACE, key: key) + record = PluginStoreRow.find_by(id: id, plugin_name: NAMESPACE) return false if !record + record.key = key record.value = data.to_json record.save else diff --git a/plugin.rb b/plugin.rb index 772bd62c..5454c4d4 100644 --- a/plugin.rb +++ b/plugin.rb @@ -31,6 +31,7 @@ if respond_to?(:register_svg_icon) register_svg_icon "far-calendar" register_svg_icon "chevron-right" register_svg_icon "chevron-left" + register_svg_icon "save" end after_initialize do diff --git a/spec/components/custom_wizard/custom_field_spec.rb b/spec/components/custom_wizard/custom_field_spec.rb index 6d7f8708..71f2bd14 100644 --- a/spec/components/custom_wizard/custom_field_spec.rb +++ b/spec/components/custom_wizard/custom_field_spec.rb @@ -31,7 +31,7 @@ describe CustomWizard::CustomField do updated_field_json = custom_field_json['custom_fields'][0] updated_field_json['serializers'] = ["topic_view"] - existing_field = CustomWizard::CustomField.find(updated_field_json["name"]) + existing_field = CustomWizard::CustomField.find_by_name(updated_field_json["name"]) updated_field = CustomWizard::CustomField.new(existing_field.id, updated_field_json) expect(updated_field.save).to eq(true) 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 90e7e618..a4bba7be 100644 --- a/spec/requests/custom_wizard/admin/custom_fields_controller_spec.rb +++ b/spec/requests/custom_wizard/admin/custom_fields_controller_spec.rb @@ -21,12 +21,26 @@ describe CustomWizard::AdminCustomFieldsController do expect(response.parsed_body.length).to eq(4) end - it "updates the list of custom fields" do - custom_field_json['custom_fields'][0]['type'] = 'string' - put "/admin/wizards/custom-fields.json", params: custom_field_json + it "saves custom fields" do + topic_field = CustomWizard::CustomField.find_by_name('topic_field_1') + topic_field_json = topic_field.as_json + topic_field_json['type'] = 'string' + + put "/admin/wizards/custom-fields.json", params: { + custom_field: topic_field_json + } expect(response.status).to eq(200) expect( - CustomWizard::CustomField.find('topic_field_1').type + CustomWizard::CustomField.find_by_name('topic_field_1').type ).to eq('string') end + + it "destroys custom fields" do + topic_field = custom_field_json['custom_fields'][0] + delete "/admin/wizards/custom-fields/#{topic_field["name"]}.json" + expect(response.status).to eq(200) + expect( + CustomWizard::CustomField.exists?('topic_field_1') + ).to eq(false) + end end \ No newline at end of file diff --git a/spec/serializers/custom_wizard/custom_field_serializer_spec.rb b/spec/serializers/custom_wizard/custom_field_serializer_spec.rb index 525b6fd8..63725937 100644 --- a/spec/serializers/custom_wizard/custom_field_serializer_spec.rb +++ b/spec/serializers/custom_wizard/custom_field_serializer_spec.rb @@ -17,7 +17,7 @@ describe CustomWizard::CustomFieldSerializer do end json = CustomWizard::CustomFieldSerializer.new( - CustomWizard::CustomField.find("topic_field_1"), + CustomWizard::CustomField.find_by_name("topic_field_1"), scope: Guardian.new(user), root: false ).as_json