From 10780d2448c31533d43fe21843bad375f6bd658b Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Wed, 10 Mar 2021 20:15:04 -0500 Subject: [PATCH] DEV: support json_schema in theme settings (#12294) --- .../discourse/app/components/json-editor.js | 7 ++ .../app/templates/components/json-editor.hbs | 3 +- app/assets/stylesheets/common/base/modal.scss | 22 +++++++ app/models/remote_theme.rb | 2 + app/models/theme.rb | 40 +++++++++++ app/serializers/theme_settings_serializer.rb | 9 ++- config/locales/client.en.yml | 2 +- lib/theme_settings_manager.rb | 4 ++ lib/theme_settings_parser.rb | 1 + .../components/theme_settings_manager_spec.rb | 7 +- .../theme_settings/json_schema_settings.yaml | 4 ++ .../theme_settings/valid_settings.yaml | 8 +++ spec/models/theme_spec.rb | 66 +++++++++++++++++++ 13 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 spec/fixtures/theme_settings/json_schema_settings.yaml diff --git a/app/assets/javascripts/discourse/app/components/json-editor.js b/app/assets/javascripts/discourse/app/components/json-editor.js index 31d87c33afb..9708afe9eb2 100644 --- a/app/assets/javascripts/discourse/app/components/json-editor.js +++ b/app/assets/javascripts/discourse/app/components/json-editor.js @@ -1,6 +1,7 @@ import { action } from "@ember/object"; import Component from "@ember/component"; import { create } from "virtual-dom"; +import discourseComputed from "discourse-common/utils/decorators"; import { iconNode } from "discourse-common/lib/icon-library"; import loadScript from "discourse/lib/load-script"; import { schedule } from "@ember/runloop"; @@ -32,12 +33,18 @@ export default Component.extend({ disable_edit_json: true, disable_properties: true, disable_collapse: true, + show_errors: "always", startval: this.model.value ? JSON.parse(this.model.value) : null, }); }); }); }, + @discourseComputed("model.settingName") + settingName(name) { + return name.replace(/\_/g, " "); + }, + @action saveChanges() { const fieldValue = JSON.stringify(this.editor.getValue()); diff --git a/app/assets/javascripts/discourse/app/templates/components/json-editor.hbs b/app/assets/javascripts/discourse/app/templates/components/json-editor.hbs index e0e53151e73..3c21d434dd5 100644 --- a/app/assets/javascripts/discourse/app/templates/components/json-editor.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/json-editor.hbs @@ -1,4 +1,5 @@ -{{#d-modal-body rawTitle=(i18n "admin.site_settings.json_schema.modal_title" name=model.settingName)}} +{{#d-modal-body rawTitle=(i18n "admin.site_settings.json_schema.modal_title" name=settingName)}} +
{{/d-modal-body}} diff --git a/app/assets/stylesheets/common/base/modal.scss b/app/assets/stylesheets/common/base/modal.scss index 7142d1de90d..39a1ad8ce07 100644 --- a/app/assets/stylesheets/common/base/modal.scss +++ b/app/assets/stylesheets/common/base/modal.scss @@ -821,6 +821,28 @@ display: block !important; text-align: right; } + + .table { + td { + vertical-align: top; + padding: 1em 0; + } + td.compact { + .invalid-feedback { + margin: 0; + font-size: $font-down-1; + color: var(--danger); + } + } + input[type="text"] { + margin-bottom: 0; + width: 95%; + &.is-invalid { + border-color: var(--danger); + outline: 1px solid var(--danger); + } + } + } } .create-invite-modal { diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb index 93abeca91d5..55ac13fac3c 100644 --- a/app/models/remote_theme.rb +++ b/app/models/remote_theme.rb @@ -177,6 +177,8 @@ class RemoteTheme < ActiveRecord::Base updated_fields << theme.set_field(**opts.merge(value: value)) end + theme.convert_settings + # Destroy fields that no longer exist in the remote theme field_ids_to_destroy = theme.theme_fields.pluck(:id) - updated_fields.map { |tf| tf&.id } ThemeField.where(id: field_ids_to_destroy).destroy_all diff --git a/app/models/theme.rb b/app/models/theme.rb index 66888135f75..94972fb2b15 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require_dependency 'global_path' +require 'csv' +require 'json_schemer' class Theme < ActiveRecord::Base include GlobalPath @@ -622,6 +624,44 @@ class Theme < ActiveRecord::Base list_baked_fields(target, name).count > 0 end + def convert_settings + settings.each do |setting| + setting_row = ThemeSetting.where(theme_id: self.id, name: setting.name.to_s).first + + if setting_row && setting_row.data_type != setting.type + if (setting_row.data_type == ThemeSetting.types[:list] && + setting.type == ThemeSetting.types[:string] && + setting.json_schema.present?) + convert_list_to_json_schema(setting_row, setting) + else + Rails.logger.warn("Theme setting type has changed but cannot be converted. \n\n #{setting.inspect}") + end + end + end + end + + def convert_list_to_json_schema(setting_row, setting) + schema = setting.json_schema + return if !schema + keys = schema["items"]["properties"].keys + return if !keys + + current_values = CSV.parse(setting_row.value, { col_sep: '|' }).flatten + new_values = [] + current_values.each do |item| + parts = CSV.parse(item, { col_sep: ',' }).flatten + props = parts.map.with_index { |p, idx| [keys[idx], p] }.to_h + new_values << props + end + + schemer = JSONSchemer.schema(schema) + raise "Schema validation failed" if !schemer.valid?(new_values) + + setting_row.value = new_values.to_json + setting_row.data_type = setting.type + setting_row.save! + end + private def to_scss_variable(name, value) diff --git a/app/serializers/theme_settings_serializer.rb b/app/serializers/theme_settings_serializer.rb index 92f41cf6812..987003d06a1 100644 --- a/app/serializers/theme_settings_serializer.rb +++ b/app/serializers/theme_settings_serializer.rb @@ -2,7 +2,7 @@ class ThemeSettingsSerializer < ApplicationSerializer attributes :setting, :type, :default, :value, :description, :valid_values, - :list_type, :textarea + :list_type, :textarea, :json_schema def setting object.name @@ -53,4 +53,11 @@ class ThemeSettingsSerializer < ApplicationSerializer object.type == ThemeSetting.types[:string] end + def json_schema + object.json_schema + end + + def include_json_schema? + object.type == ThemeSetting.types[:string] && object.json_schema.present? + end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 51972f390f4..3b19fe7f84e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -5052,7 +5052,7 @@ en: simple_list: add_item: "Add item..." json_schema: - edit: Launch JSON Editor + edit: Launch Editor modal_title: "Edit %{name}" badges: diff --git a/lib/theme_settings_manager.rb b/lib/theme_settings_manager.rb index 49ad6a6d612..968f6e9e23a 100644 --- a/lib/theme_settings_manager.rb +++ b/lib/theme_settings_manager.rb @@ -111,6 +111,10 @@ class ThemeSettingsManager def textarea @opts[:textarea] end + + def json_schema + JSON.parse(@opts[:json_schema]) rescue false + end end class Bool < self diff --git a/lib/theme_settings_parser.rb b/lib/theme_settings_parser.rb index 56929ae5370..e12f5d03393 100644 --- a/lib/theme_settings_parser.rb +++ b/lib/theme_settings_parser.rb @@ -41,6 +41,7 @@ class ThemeSettingsParser end opts[:textarea] = !!raw_opts[:textarea] + opts[:json_schema] = raw_opts[:json_schema] opts end diff --git a/spec/components/theme_settings_manager_spec.rb b/spec/components/theme_settings_manager_spec.rb index d66addeb3d5..2659883fd40 100644 --- a/spec/components/theme_settings_manager_spec.rb +++ b/spec/components/theme_settings_manager_spec.rb @@ -132,10 +132,15 @@ describe ThemeSettingsManager do end it "can be a textarea" do - string_setting = find_by_name(:string_setting_02) expect(find_by_name(:string_setting_02).textarea).to eq(false) expect(find_by_name(:string_setting_03).textarea).to eq(true) end + + it "supports json schema" do + expect(find_by_name(:string_setting_03).json_schema).to eq(false) + expect(find_by_name(:invalid_json_schema_setting).json_schema).to eq(false) + expect(find_by_name(:valid_json_schema_setting).json_schema).to be_truthy + end end context "List" do diff --git a/spec/fixtures/theme_settings/json_schema_settings.yaml b/spec/fixtures/theme_settings/json_schema_settings.yaml new file mode 100644 index 00000000000..0862ec4cfb0 --- /dev/null +++ b/spec/fixtures/theme_settings/json_schema_settings.yaml @@ -0,0 +1,4 @@ +test_setting: + default: "" + description: "A JSON schema field type" + json_schema: '{ "type": "array", "uniqueItems": true, "items": { "type": "object", "properties": { "color": { "type": "string" }, "icon": { "type": "string" } }, "additionalProperties": false } }' diff --git a/spec/fixtures/theme_settings/valid_settings.yaml b/spec/fixtures/theme_settings/valid_settings.yaml index 35a6973e663..95be277cf32 100644 --- a/spec/fixtures/theme_settings/valid_settings.yaml +++ b/spec/fixtures/theme_settings/valid_settings.yaml @@ -72,3 +72,11 @@ enum_setting_03: upload_setting: type: upload default: "" + +invalid_json_schema_setting: + default: "" + json_schema: '{ "type": "array", "invalid json"' + +valid_json_schema_setting: + default: "" + json_schema: '{ "type": "array", "uniqueItems": true, "items": { "type": "object", "properties": { "color": { "type": "string" }, "icon": { "type": "string" } }, "additionalProperties": false } }' diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index f19cdb38191..a37f2f2bba1 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -608,6 +608,72 @@ HTML expect(json).to match(/\"boolean_setting\":false/) end + describe "convert_settings" do + + it 'can migrate a list field to a string field with json schema' do + theme.set_field(target: :settings, name: :yaml, value: "valid_json_schema_setting:\n default: \"green,globe\"\n type: \"list\"") + theme.save! + + setting = theme.settings.find { |s| s.name == :valid_json_schema_setting } + setting.value = "red,globe|green,cog|brown,users" + theme.save! + + expect(setting.type).to eq(ThemeSetting.types[:list]) + + yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/valid_settings.yaml") + theme.set_field(target: :settings, name: "yaml", value: yaml) + theme.save! + + theme.convert_settings + setting = theme.settings.find { |s| s.name == :valid_json_schema_setting } + + expect(JSON.parse(setting.value)).to eq(JSON.parse('[{"color":"red","icon":"globe"},{"color":"green","icon":"cog"},{"color":"brown","icon":"users"}]')) + expect(setting.type).to eq(ThemeSetting.types[:string]) + end + + it 'does not update setting if data does not validate against json schema' do + theme.set_field(target: :settings, name: :yaml, value: "valid_json_schema_setting:\n default: \"green,globe\"\n type: \"list\"") + theme.save! + + setting = theme.settings.find { |s| s.name == :valid_json_schema_setting } + + # json_schema_settings.yaml defines only two properties per object and disallows additionalProperties + setting.value = "red,globe,hey|green,cog,hey|brown,users,nay" + theme.save! + + yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/valid_settings.yaml") + theme.set_field(target: :settings, name: "yaml", value: yaml) + theme.save! + + expect { theme.convert_settings }.to raise_error("Schema validation failed") + + setting.value = "red,globe|green,cog|brown" + theme.save! + + expect { theme.convert_settings }.not_to raise_error + + setting = theme.settings.find { |s| s.name == :valid_json_schema_setting } + expect(setting.type).to eq(ThemeSetting.types[:string]) + end + + it 'warns when the theme has modified the setting type but data cannot be converted' do + Rails.logger = FakeLogger.new + theme.set_field(target: :settings, name: :yaml, value: "valid_json_schema_setting:\n default: \"\"\n type: \"list\"") + theme.save! + + setting = theme.settings.find { |s| s.name == :valid_json_schema_setting } + setting.value = "red,globe" + theme.save! + + theme.set_field(target: :settings, name: :yaml, value: "valid_json_schema_setting:\n default: \"\"\n type: \"string\"") + theme.save! + + theme.convert_settings + expect(setting.value).to eq("red,globe") + expect(Rails.logger.warnings[0]).to include("Theme setting type has changed but cannot be converted.") + end + end + describe "theme translations" do it "can list working theme_translation_manager objects" do en_translation = ThemeField.create!(theme_id: theme.id, name: "en", type_id: ThemeField.types[:yaml], target_id: Theme.targets[:translations], value: <<~YAML)