trailblazer / reform

Form objects decoupled from models.
https://trailblazer.to/2.1/docs/reform.html
MIT License
2.49k stars 184 forks source link

validation blocks with inherit: true option ignore other options #533

Open YaroslavZahoruiko opened 2 years ago

YaroslavZahoruiko commented 2 years ago

I think that when we use inherit: true, we should merge new options to parent validation bock as it work in property inherit: true and move the validation position to the end of the groups. https://github.com/trailblazer/reform/blob/master/lib/reform/validation/groups.rb#L17

apotonick commented 2 years ago

Could you make a quick example of the validation blocks and "positions" for such an :inherit case, @YaroslavZahoruiko ? :beers:

YaroslavZahoruiko commented 2 years ago

@apotonick   in module or in inherited class

property :name
validation :valid_name do
  required(:name).filled(:str?)
end

in the main class

ValidNamePresent = proc { valid_name.present? }
validation :valid_name, inherit: true, if: ValidNamePresent

or we want add dependency from other validation

validation :default do
end

validation :valid_name, inherit: true, if: :default

this code will not work as a quick fix i suggest fixing in https://github.com/trailblazer/reform/blob/master/lib/reform/validation/groups.rb#L17

  def add(name, options)
    if options[:inherit] && (cfg = find { |cfg| cfg.first == name })
      self.delete(cfg)
      group = cfg[1]
      options.reverse_merge(cfg[2])
    end

    i = index_for(options)

    self.insert(i, [name, group ||= @group_class.new, options]) # Group.new
    group
  end
YaroslavZahoruiko commented 2 years ago

but we need to think about another validation dependency module

module AddressForm
property :address do
    property :first_name
    property :last_name
    property :phone
end

validation :address_validation do
    #some validation
end

validation :phone_validation if: :address_validation do
    #some validation
end

condition for phone_validation will always be true because the position phone_validation will be before address_validation when we will overwrite it