From f31054f75d7c9f69d6bbe100e428547de430fceb Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 26 Nov 2020 14:05:50 +1100 Subject: [PATCH] Isolate and specify template and update validation --- extensions/wizard_field.rb | 7 +- lib/custom_wizard/builder.rb | 126 ++---------------- lib/custom_wizard/step_updater.rb | 10 +- lib/custom_wizard/template.rb | 2 +- .../{validator.rb => validators/template.rb} | 4 +- lib/custom_wizard/validators/update.rb | 116 ++++++++++++++++ lib/custom_wizard/wizard.rb | 4 +- plugin.rb | 3 +- spec/components/custom_wizard/builder_spec.rb | 42 ------ ...tor_spec.rb => template_validator_spec.rb} | 12 +- .../custom_wizard/update_validator_spec.rb | 67 ++++++++++ 11 files changed, 220 insertions(+), 173 deletions(-) rename lib/custom_wizard/{validator.rb => validators/template.rb} (94%) create mode 100644 lib/custom_wizard/validators/update.rb rename spec/components/custom_wizard/{validator_spec.rb => template_validator_spec.rb} (72%) create mode 100644 spec/components/custom_wizard/update_validator_spec.rb diff --git a/extensions/wizard_field.rb b/extensions/wizard_field.rb index f44e44bb..2301c6d3 100644 --- a/extensions/wizard_field.rb +++ b/extensions/wizard_field.rb @@ -1,5 +1,6 @@ module CustomWizardFieldExtension - attr_reader :label, + attr_reader :raw, + :label, :description, :image, :key, @@ -12,7 +13,7 @@ module CustomWizardFieldExtension def initialize(attrs) super - @attrs = attrs || {} + @raw = attrs || {} @description = attrs[:description] @image = attrs[:image] @key = attrs[:key] @@ -25,6 +26,6 @@ module CustomWizardFieldExtension end def label - @label ||= PrettyText.cook(@attrs[:label]) + @label ||= PrettyText.cook(@raw[:label]) end end \ No newline at end of file diff --git a/lib/custom_wizard/builder.rb b/lib/custom_wizard/builder.rb index 9ac4bb73..e530e5e9 100644 --- a/lib/custom_wizard/builder.rb +++ b/lib/custom_wizard/builder.rb @@ -23,19 +23,6 @@ class CustomWizard::Builder sorted_handlers << { priority: priority, wizard_id: wizard_id, block: block } @sorted_handlers.sort_by! { |h| -h[:priority] } end - - def self.sorted_field_validators - @sorted_field_validators ||= [] - end - - def self.field_validators - sorted_field_validators.map { |h| { type: h[:type], block: h[:block] } } - end - - def self.add_field_validator(priority = 0, type, &block) - sorted_field_validators << { priority: priority, type: type, block: block } - @sorted_field_validators.sort_by! { |h| -h[:priority] } - end def mapper CustomWizard::Mapper.new( @@ -91,11 +78,7 @@ class CustomWizard::Builder @updater = updater user = @wizard.user - if step_template['fields'] && step_template['fields'].length - step_template['fields'].each do |field| - validate_field(field, updater, step_template) if field['type'] != 'text_only' - end - end + updater.validate next if updater.errors.any? @@ -107,10 +90,10 @@ class CustomWizard::Builder next if updater.errors.any? - data = updater.fields + submission = updater.submission - if submission = @wizard.current_submission - data = submission.merge(data) + if current_submission = @wizard.current_submission + submission = current_submission.merge(submission) end final_step = updater.step.next.nil? @@ -125,19 +108,19 @@ class CustomWizard::Builder wizard: @wizard, action: action, user: user, - data: data + data: submission ).perform end end end if updater.errors.empty? - if route_to = data['route_to'] - data.delete('route_to') + if route_to = submission['route_to'] + submission.delete('route_to') end if @wizard.save_submissions - save_submissions(data, final_step) + save_submissions(submission, final_step) end if final_step @@ -146,7 +129,7 @@ class CustomWizard::Builder @wizard.user.save_custom_fields(true) end - redirect_url = route_to || data['redirect_on_complete'] || data["redirect_to"] + redirect_url = route_to || submission['redirect_on_complete'] || submission["redirect_to"] updater.result[:redirect_on_complete] = redirect_url elsif route_to updater.result[:redirect_on_next] = route_to @@ -261,101 +244,18 @@ class CustomWizard::Builder end end - def validate_field(field, updater, step_template) - value = updater.fields[field['id']] - min_length = false - - label = field['label'] || I18n.t("#{field['key']}.label") - type = field['type'] - required = field['required'] - id = field['id'].to_s - min_length = field['min_length'] if is_text_type(field) - file_types = field['file_types'] - format = field['format'] - - if required && !value - updater.errors.add(id, I18n.t('wizard.field.required', label: label)) - end - - if min_length && value.is_a?(String) && value.strip.length < min_length.to_i - updater.errors.add(id, I18n.t('wizard.field.too_short', label: label, min: min_length.to_i)) - end - - if is_url_type(field) && !check_if_url(value) - updater.errors.add(id, I18n.t('wizard.field.not_url', label: label)) - end - - if type === 'checkbox' - updater.fields[id] = standardise_boolean(value) - end - - if type === 'upload' && value.present? && !validate_file_type(value, file_types) - updater.errors.add(id, I18n.t('wizard.field.invalid_file', label: label, types: file_types)) - end - - if ['date', 'date_time'].include?(type) && value.present? && !validate_date(value) - updater.errors.add(id, I18n.t('wizard.field.invalid_date')) - end - - if type === 'time' && value.present? && !validate_time(value) - updater.errors.add(id, I18n.t('wizard.field.invalid_time')) - end - - CustomWizard::Builder.field_validators.each do |validator| - if type === validator[:type] - validator[:block].call(field, updater, step_template) - end - end - end - - def validate_file_type(value, file_types) - file_types.split(',') - .map { |t| t.gsub('.', '') } - .include?(File.extname(value['original_filename'])[1..-1]) - end - - def validate_date(value) - begin - Date.parse(value) - true - rescue ArgumentError - false - end - end - - def validate_time(value) - begin - Time.parse(value) - true - rescue ArgumentError - false - end - end - - def is_text_type(field) - ['text', 'textarea', 'composer'].include? field['type'] - end - - def is_url_type(field) - ['url'].include? field['type'] - end - - def check_if_url(value) - value =~ URI::regexp - end - def standardise_boolean(value) ActiveRecord::Type::Boolean.new.cast(value) end - def save_submissions(data, final_step) + def save_submissions(submission, final_step) if final_step - data['submitted_at'] = Time.now.iso8601 + submission['submitted_at'] = Time.now.iso8601 end - if data.present? + if submission.present? @submissions.pop(1) if @wizard.unfinished? - @submissions.push(data) + @submissions.push(submission) @wizard.set_submissions(@submissions) end end diff --git a/lib/custom_wizard/step_updater.rb b/lib/custom_wizard/step_updater.rb index 33509a77..26513a9a 100644 --- a/lib/custom_wizard/step_updater.rb +++ b/lib/custom_wizard/step_updater.rb @@ -1,14 +1,14 @@ class CustomWizard::StepUpdater include ActiveModel::Model - attr_accessor :refresh_required, :fields, :result, :step + attr_accessor :refresh_required, :submission, :result, :step - def initialize(current_user, wizard, step, fields) + def initialize(current_user, wizard, step, submission) @current_user = current_user @wizard = wizard @step = step @refresh_required = false - @fields = fields.to_h.with_indifferent_access + @submission = submission.to_h.with_indifferent_access @result = {} end @@ -38,4 +38,8 @@ class CustomWizard::StepUpdater def refresh_required? @refresh_required end + + def validate + CustomWizard::UpdateValidator.new(self).perform + end end diff --git a/lib/custom_wizard/template.rb b/lib/custom_wizard/template.rb index 584df568..f3e43f17 100644 --- a/lib/custom_wizard/template.rb +++ b/lib/custom_wizard/template.rb @@ -90,7 +90,7 @@ class CustomWizard::Template end def validate_data - validator = CustomWizard::Validator.new(@data, @opts) + validator = CustomWizard::TemplateValidator.new(@data, @opts) validator.perform add_errors_from(validator) end diff --git a/lib/custom_wizard/validator.rb b/lib/custom_wizard/validators/template.rb similarity index 94% rename from lib/custom_wizard/validator.rb rename to lib/custom_wizard/validators/template.rb index 241e3b8e..2ed67deb 100644 --- a/lib/custom_wizard/validator.rb +++ b/lib/custom_wizard/validators/template.rb @@ -1,4 +1,4 @@ -class CustomWizard::Validator +class CustomWizard::TemplateValidator include HasErrors include ActiveModel::Model @@ -49,7 +49,7 @@ class CustomWizard::Validator private def check_required(object, type) - CustomWizard::Validator.required[type].each do |property| + CustomWizard::TemplateValidator.required[type].each do |property| if object[property].blank? errors.add :base, I18n.t("wizard.validation.required", property: property) end diff --git a/lib/custom_wizard/validators/update.rb b/lib/custom_wizard/validators/update.rb new file mode 100644 index 00000000..b3e8ba86 --- /dev/null +++ b/lib/custom_wizard/validators/update.rb @@ -0,0 +1,116 @@ +class ::CustomWizard::UpdateValidator + attr_reader :updater + + def initialize(updater) + @updater = updater + end + + def perform + updater.step.fields.each do |field| + validate_field(field) + end + end + + def validate_field(field) + return if field.type == 'text_only' + + field_id = field.id.to_s + value = @updater.submission[field_id] + min_length = false + label = field.raw[:label] || I18n.t("#{field.key}.label") + type = field.type + required = field.required + min_length = field.min_length if is_text_type(field) + file_types = field.file_types + format = field.format + + if required && !value + @updater.errors.add(field_id, I18n.t('wizard.field.required', label: label)) + end + + if min_length && value.is_a?(String) && value.strip.length < min_length.to_i + @updater.errors.add(field_id, I18n.t('wizard.field.too_short', label: label, min: min_length.to_i)) + end + + if is_url_type(field) && !check_if_url(value) + @updater.errors.add(field_id, I18n.t('wizard.field.not_url', label: label)) + end + + if type === 'checkbox' + @updater.submission[field_id] = standardise_boolean(value) + end + + if type === 'upload' && value.present? && !validate_file_type(value, file_types) + @updater.errors.add(field_id, I18n.t('wizard.field.invalid_file', label: label, types: file_types)) + end + + if ['date', 'date_time'].include?(type) && value.present? && !validate_date(value) + @updater.errors.add(field_id, I18n.t('wizard.field.invalid_date')) + end + + if type === 'time' && value.present? && !validate_time(value) + @updater.errors.add(field_id, I18n.t('wizard.field.invalid_time')) + end + + self.class.field_validators.each do |validator| + if type === validator[:type] + validator[:block].call(field, value, @updater, @step_template) + end + end + end + + def self.sorted_field_validators + @sorted_field_validators ||= [] + end + + def self.field_validators + sorted_field_validators.map { |h| { type: h[:type], block: h[:block] } } + end + + def self.add_field_validator(priority = 0, type, &block) + sorted_field_validators << { priority: priority, type: type, block: block } + @sorted_field_validators.sort_by! { |h| -h[:priority] } + end + + private + + def validate_file_type(value, file_types) + file_types.split(',') + .map { |t| t.gsub('.', '') } + .include?(File.extname(value['original_filename'])[1..-1]) + end + + def validate_date(value) + begin + Date.parse(value) + true + rescue ArgumentError + false + end + end + + def validate_time(value) + begin + Time.parse(value) + true + rescue ArgumentError + false + end + end + + def is_text_type(field) + ['text', 'textarea', 'composer'].include? field.type + end + + def is_url_type(field) + ['url'].include? field.type + end + + def check_if_url(value) + value =~ URI::regexp + end + + def standardise_boolean(value) + ActiveRecord::Type::Boolean.new.cast(value) + end +end \ No newline at end of file diff --git a/lib/custom_wizard/wizard.rb b/lib/custom_wizard/wizard.rb index 96ae4e0e..cf19dc26 100644 --- a/lib/custom_wizard/wizard.rb +++ b/lib/custom_wizard/wizard.rb @@ -107,10 +107,10 @@ class CustomWizard::Wizard end end - def create_updater(step_id, inputs) + def create_updater(step_id, submission) step = @steps.find { |s| s.id == step_id } wizard = self - CustomWizard::StepUpdater.new(user, wizard, step, inputs) + CustomWizard::StepUpdater.new(user, wizard, step, submission) end def unfinished? diff --git a/plugin.rb b/plugin.rb index 4eb89d7b..6f4542df 100644 --- a/plugin.rb +++ b/plugin.rb @@ -50,6 +50,8 @@ after_initialize do ../jobs/clear_after_time_wizard.rb ../jobs/refresh_api_access_token.rb ../jobs/set_after_time_wizard.rb + ../lib/custom_wizard/validators/template.rb + ../lib/custom_wizard/validators/update.rb ../lib/custom_wizard/action_result.rb ../lib/custom_wizard/action.rb ../lib/custom_wizard/builder.rb @@ -59,7 +61,6 @@ after_initialize do ../lib/custom_wizard/log.rb ../lib/custom_wizard/step_updater.rb ../lib/custom_wizard/template.rb - ../lib/custom_wizard/validator.rb ../lib/custom_wizard/wizard.rb ../lib/custom_wizard/api/api.rb ../lib/custom_wizard/api/authorization.rb diff --git a/spec/components/custom_wizard/builder_spec.rb b/spec/components/custom_wizard/builder_spec.rb index dd3b110f..e6fc1105 100644 --- a/spec/components/custom_wizard/builder_spec.rb +++ b/spec/components/custom_wizard/builder_spec.rb @@ -316,48 +316,6 @@ describe CustomWizard::Builder do ).to eq(nil) end end - - context 'validation' do - it 'applies min length to fields that support min length' do - min_length = 3 - - @template[:steps][0][:fields][0][:min_length] = min_length - @template[:steps][0][:fields][1][:min_length] = min_length - @template[:steps][0][:fields][2][:min_length] = min_length - CustomWizard::Template.save(@template.as_json) - - expect( - perform_update('step_1', step_1_field_1: 'Te') - .errors.messages[:step_1_field_1].first - ).to eq(I18n.t('wizard.field.too_short', label: 'Text', min: min_length)) - expect( - perform_update('step_1', step_1_field_2: 'Te') - .errors.messages[:step_1_field_2].first - ).to eq(I18n.t('wizard.field.too_short', label: 'Textarea', min: min_length)) - expect( - perform_update('step_1', step_1_field_3: 'Te') - .errors.messages[:step_1_field_3].first - ).to eq(I18n.t('wizard.field.too_short', label: 'Composer', min: min_length)) - end - - it 'standardises boolean entries' do - perform_update('step_2', step_2_field_5: 'false') - expect( - CustomWizard::Wizard.submissions(@template[:id], user) - .first['step_2_field_5'] - ).to eq(false) - end - - it 'requires required fields' do - @template[:steps][0][:fields][1][:required] = true - CustomWizard::Template.save(@template.as_json) - - expect( - perform_update('step_1', step_1_field_2: nil) - .errors.messages[:step_1_field_2].first - ).to eq(I18n.t('wizard.field.required', label: 'Textarea')) - end - end end end end \ No newline at end of file diff --git a/spec/components/custom_wizard/validator_spec.rb b/spec/components/custom_wizard/template_validator_spec.rb similarity index 72% rename from spec/components/custom_wizard/validator_spec.rb rename to spec/components/custom_wizard/template_validator_spec.rb index 2a27d083..b657af73 100644 --- a/spec/components/custom_wizard/validator_spec.rb +++ b/spec/components/custom_wizard/template_validator_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe CustomWizard::Validator do +describe CustomWizard::TemplateValidator do fab!(:user) { Fabricate(:user) } let(:template) { @@ -11,21 +11,21 @@ describe CustomWizard::Validator do it "validates valid templates" do expect( - CustomWizard::Validator.new(template).perform + CustomWizard::TemplateValidator.new(template).perform ).to eq(true) end it "invalidates templates without required attributes" do template.delete(:id) expect( - CustomWizard::Validator.new(template).perform + CustomWizard::TemplateValidator.new(template).perform ).to eq(false) end it "invalidates templates with duplicate ids if creating a new template" do CustomWizard::Template.save(template) expect( - CustomWizard::Validator.new(template, create: true).perform + CustomWizard::TemplateValidator.new(template, create: true).perform ).to eq(false) end @@ -33,7 +33,7 @@ describe CustomWizard::Validator do template[:after_time] = true template[:after_time_scheduled] = (Time.now + 3.hours).iso8601 expect( - CustomWizard::Validator.new(template).perform + CustomWizard::TemplateValidator.new(template).perform ).to eq(true) end @@ -41,7 +41,7 @@ describe CustomWizard::Validator do template[:after_time] = true template[:after_time_scheduled] = "not a time" expect( - CustomWizard::Validator.new(template).perform + CustomWizard::TemplateValidator.new(template).perform ).to eq(false) end end \ No newline at end of file diff --git a/spec/components/custom_wizard/update_validator_spec.rb b/spec/components/custom_wizard/update_validator_spec.rb new file mode 100644 index 00000000..c8370484 --- /dev/null +++ b/spec/components/custom_wizard/update_validator_spec.rb @@ -0,0 +1,67 @@ +require 'rails_helper' + +describe CustomWizard::UpdateValidator do + fab!(:user) { Fabricate(:user) } + + let(:template) { + JSON.parse(File.open( + "#{Rails.root}/plugins/discourse-custom-wizard/spec/fixtures/wizard.json" + ).read).with_indifferent_access + } + + before do + CustomWizard::Template.save( + JSON.parse(File.open( + "#{Rails.root}/plugins/discourse-custom-wizard/spec/fixtures/wizard.json" + ).read), + skip_jobs: true) + @template = CustomWizard::Template.find('super_mega_fun_wizard') + end + + def perform_validation(step_id, submission) + wizard = CustomWizard::Builder.new(@template[:id], user).build + updater = wizard.create_updater(step_id, submission) + updater.validate + updater + end + + it 'applies min length to text type fields' do + min_length = 3 + + @template[:steps][0][:fields][0][:min_length] = min_length + @template[:steps][0][:fields][1][:min_length] = min_length + @template[:steps][0][:fields][2][:min_length] = min_length + + CustomWizard::Template.save(@template) + + updater = perform_validation('step_1', step_1_field_1: 'Te') + expect( + updater.errors.messages[:step_1_field_1].first + ).to eq(I18n.t('wizard.field.too_short', label: 'Text', min: min_length)) + + updater = perform_validation('step_1', step_1_field_2: 'Te') + expect( + updater.errors.messages[:step_1_field_2].first + ).to eq(I18n.t('wizard.field.too_short', label: 'Textarea', min: min_length)) + + updater = perform_validation('step_1', step_1_field_3: 'Te') + expect( + updater.errors.messages[:step_1_field_3].first + ).to eq(I18n.t('wizard.field.too_short', label: 'Composer', min: min_length)) + end + + it 'standardises boolean entries' do + updater = perform_validation('step_2', step_2_field_5: 'false') + expect(updater.submission['step_2_field_5']).to eq(false) + end + + it 'requires required fields' do + @template[:steps][0][:fields][1][:required] = true + CustomWizard::Template.save(@template) + + updater = perform_validation('step_1', step_1_field_2: nil) + expect( + updater.errors.messages[:step_1_field_2].first + ).to eq(I18n.t('wizard.field.required', label: 'Textarea')) + end +end \ No newline at end of file