Isolate and specify template and update validation

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

lib/custom_wizard/builder.rb

@@ -24,19 +24,6 @@ class CustomWizard::Builder
     @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(
       user: @wizard.user,
@@ -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

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

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

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

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

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?

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

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

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

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