when using composition, validation fails on aliased properties

[lastobelus]

Complete Description of Issue

When using composition, if you define an "aliased" property using on:, from:, required validation does not work on that property.

Steps to reproduce

context 'validate id' do
  Song   = Struct.new(:id, :title)
  Album  = Struct.new(:id, :title)

  class ComposedForm < Reform::Form
    include Composition
    property :album_id, on: :album, from: :id
    property :title, on: :album

    property :song_id, on: :song, from: :id

    validation do
      required(:album_id).filled
      required(:title).filled
    end
  end

  context 'composed form' do
    it "validates id" do
      form = ComposedForm.new(album: Album.new(111, "bob"), song: Song.new)
      form.validate({})
      expect(form.to_result.errors).to be_empty
    end
  end
end

Expected behavior

The above spec should pass. Inspecting the form verifies form.album_id is present.

Actual behavior

Spec fails, form has "album_id: is missing" error:

Failures:

  1) validate id composed form validates id
     Failure/Error: expect(form.to_result.errors).to be_empty
       expected `{:album_id=>["is missing"]}.empty?` to return true, got false
     # ./spec/forms/validate_id_on_composed_model_spec.rb:45:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:51:in `block (2 levels) in <top (required)>'

System configuration

Reform version: 2.3.0rc1

[fran-worley]

From memory, we ignor composition when we pass params to dry-validation purposes.

The idea is to validate your input, not the format of the output (composition).

Happy to discuss changing the behavior but it would have to be configurable.

[adamakhtar]

Hi @fran-worley.

To clarify your last comment do you mean validating composed forms is not currently possible in Reform? If that is not the case then what is the correct syntax to validate the form in @lastobelus comment?

I'm having the same problem and looked at docs and google but to no avail.

[karouf]

I hit the same problem trying to use composition. Well it seems that Reform produces a nested hash from the form according to the composition rules and passes it to dry-validation. In that example, the following params are passed to the form:

{}

And the following hash is passed to dry-validation:

{:album=>{"id"=>111, "title"=>"bob"}, :song=>{"id"=>nil}}

Therefore it doesn't find any album_id param.

The following example works though:

context "validate id" do
  Song   = Struct.new(:id, :title)
  Album  = Struct.new(:id, :title)

  class ComposedForm < Reform::Form
    include Composition
    property :album_id, on: :album, from: :id
    property :title, on: :album

    property :song_id, on: :song, from: :id

    validation do
      required(:album).schema do
        required(:id).filled
        required(:title).filled
      end
    end
  end

  context "composed form" do
    it "validates id" do
      form = ComposedForm.new(album: Album.new(111, "bob"), song: Song.new)
      form.validate({})
      expect(form.to_result.errors).to be_empty
    end
  end
end

The trick is to validate a nested hash with dry-validation to match the hash that is passed to it by Reform. Note that the Form#errors hash after validation will reflect the composition and will give you errors in this way: {:album=>[[:id, ["must be an integer"]]]}

[fran-worley]

This is the bit of code that reform uses to transform the input params for dry validation: https://github.com/trailblazer/reform/blob/master/lib/reform/form/dry.rb#L74-L78

I'm unsure what your expectation is (confession - I've never needed composition myself). What format of params should be validated and where should the errors come through?

[karouf]

Since I was declaring an album_id property in the contract block, I was expecting to reference it the same way in the validation block. It's a bit counter-intuitive but probably nothing that couldn't be fixed just by updating the docs.

[fran-worley]

@karouf updating the docs is one thing, but if composition users find it counter-intuitive there might be a better way of doing things no?

Can I assume that the reproduction example from @lastobelus is what your expectation is?

[karouf]

By the way I tested that on 2.2.4 and I see it's a bit different than master. In 2.2.4 #to_nested_hash is called on the form whereas on master #to_hash is called.

Sure I agree that it would benefit from being more intuitive. I was just saying that it's not a show-stopper either, as I was led to believe when I first stumble upon this issue. As a first step, adding some info about that in the docs would probably save some time to some people.

I would need to have a look at the behavior in master but basically the code @lastobelus posted is the way I expected it to work yes.

[fran-worley]

@karouf cool, glad we are on the same page. There are no formal composition/dry validation test cases, probably why the implementation has changed between versions. I'll use @lastobelus code to put a test case in for us to work towards.

Thanks everyone 👍

[simonc]

Hi there. From what I'm experiencing today, it's not limited to composition. When creating a simple form that aliases a property with from: you also need to refer to the original name in validations.

❤️