reform icon indicating copy to clipboard operation
reform copied to clipboard

Cannot validate form when using Composition and dry-validation

Open cmrichards opened this issue 7 years ago • 4 comments

Complete Description of Issue

When using dry-validation for validation and using Composition, I cannot get the form to validate.

Steps to reproduce

class TestForm < Reform::Form 
  include Composition
  property :title, on: :first
  property :first_name, on: :second
  validation do 
    required(:title).filled(:str?)
    required(:first_name).filled(:str?)
  end
end

# First try with strings as keys
f = TestForm.new(first: OpenStruct.new, second: OpenStruct.new)
f.validate({"title": "Mr", "first_name": "Bob"})
=> false

# Then try with symbols as keys
f = TestForm.new(first: OpenStruct.new, second: OpenStruct.new)
f.validate({title: "Mr", first_name: "Bob"})
=> false

Expected behavior

It should validate

Actual behavior

It does not validate

System configuration

reform 2.2.4 dry-validation 0.11.1 rails 5.2.0.beta2

cmrichards avatar Jan 11 '18 09:01 cmrichards

We have the same here:

  class RetailerOnboardInput < Reform::Form
    include Composition
    feature Reform::Form::Dry

    property :location_country, on: :investor
    property :first_name, on: :user
    property :last_name, on: :user

    validation :default do
      required(:location_country).filled(:str?)
      required(:first_name).filled(:str?)
      required(:last_name).filled(:str?)
    end
form.valid?
=> false
[2] pry(#<RSpec::ExampleGroups::InvestorRetailerOnboardInput::PositiveCase>)> form.errors
=> #<Reform::Contract::Errors:0x007fa56ebf61c8
 @errors={:location_country=>["is missing"], :first_name=>["is missing"], :last_name=>["is missing"]}>
[3] pry(#<RSpec::ExampleGroups::InvestorRetailerOnboardInput::PositiveCase>)> form.first_name
=> "Tommie"

orbanbotond avatar Mar 21 '18 15:03 orbanbotond

We're having this too.

Essentially, it's very similar to https://github.com/trailblazer/reform/issues/438

If you do

    validation :default do
      required(:investor).schema do
        required(:location_country).filled(:str?)
      end
      required(:user).schema do
        required(:first_name).filled(:str?)
        required(:last_name).filled(:str?)
      end
    end

Then your validation will work, but the errors hash produced will not be usable with rails style form builders.

So we have a problem of what hash is being passed to dry-v

siassaj avatar Apr 12 '18 02:04 siassaj

More info, issue is here:

In file lib/disposable/twin/composition.rb in the Disposable library (0.4.3 for me) we have

     def to_nested_hash(*)
        hash = {}

        @model.each do |name, model| # TODO: provide list of composee attributes in Composition.
          part_properties = schema.find_all { |dfn| dfn[:on] == name }.collect{ |dfn| dfn[:name].to_sym }
          hash[name] = self.class.nested_hash_representer.new(self).to_hash(include: part_properties)
        end

        hash
      end

The method is geared to iterate over the models.

But we need to use a non nested hash in reform::form... reform form needs a normalized_hash or.... actually a the_hash_i_expected

The fix depends on whether Composition in twin is doing the wrong thing with nested_hash (which looks right to me) or if reform's supposed to use a flat hash...

siassaj avatar Apr 12 '18 02:04 siassaj

In 2.1.0, reform/lib/reform/validation.rb

  def valid?
    Groups::Result.new(self.class.validation_groups).(@fields, errors, self)
  end

But in 2.2.4

  def valid?
    Groups::Result.new(self.class.validation_groups).(to_nested_hash, errors, self)
  end

I think it should be @fields, not to_nested_hash

siassaj avatar Apr 12 '18 03:04 siassaj