Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Populator is not called when the form fragment is nil #400

Open
shepmaster opened this issue Sep 15, 2016 · 7 comments
Open

Populator is not called when the form fragment is nil #400

shepmaster opened this issue Sep 15, 2016 · 7 comments
Labels
Milestone

Comments

@shepmaster
Copy link
Contributor

shepmaster commented Sep 15, 2016

I have an existing API that passes nil for certain values that should actually be missing. To deal with this in a constrained manner, I'd like my form object to handle that.

After flailing around with skip_if, I was pointed towards populate. However, it appears that a populator is not called when the fragment is nil:

require 'reform'
require "reform/form/active_model/validations"

Reform::Form.class_eval do
  include Reform::Form::ActiveModel::Validations
end

Refinements = Struct.new(:name)

class RefinementsForm < Reform::Form
  property :name, default: "Alice", populator: ->(_) { skip! }
end

form = RefinementsForm.new(Refinements.new)

if form.validate("name" => nil)
  form.sync
else
  raise "Not validated"
end

puts "Name: #{form.name.inspect} (should be the default: Alice)"

This code will print

Name: nil (should be the default: Alice)

If I change the input to any other value: form.validate("name" => ""), then the populator is called, the skip! executed, and the output is correct:

Name: "Alice" (should be the default: Alice)

It feels incorrect that an explicit nil does not trigger the populator but it does set the value to nil.


Reform 2.2.1 with Ruby 2.3.1p112 on OS X

@shepmaster
Copy link
Contributor Author

I think it's likely to be related to OverwriteOnNil. This returns Representable::Pipeline::Stop, which is then returned from the pipeline iterator.

@apotonick
Copy link
Member

The populator is not called for a nil fragment... that is indeed unexpected behavior.....

@shepmaster
Copy link
Contributor Author

Any suggested areas of Reform or it's dependencies you suggest I poke around in? Any kinds of test cases that would be useful to provide?

@ollieh-m
Copy link

ollieh-m commented Oct 19, 2017

I've encountered this issue trying to validate that a valid nested property (user) is set on a parent form. If I pass in 'user' => {email: '', name: ''} the parent form is correctly invalid because the user form requires email and name to be present. But if I pass in 'user' => nil, the user property does not get set to an instance of the user form, I assume because no populator is called for user. parent_form.user is nil. No validations run on user.

It would sort of make sense for the populator to be skipped if the input fragment was completely ignored and not written to the form. But skipping the user populator and still have user=() called, means I end up with an invalid user saved on the parent model.

(Note that I only get an invalid user saved if the form is initialized with a new user - that new user gets saved.)

@fran-worley
Copy link
Collaborator

@ollieh-m you can solve your validation issue (if you're using dry-validation) but writing your validations on the parent:
http://dry-rb.org/gems/dry-validation/nested-data/

Otherwise, you'd have to write a custom validator on the parent to check that :user is a hash and has two properties email, and name.

@ollieh-m
Copy link

Thanks @fran-worley. I'm not using dry-validation but I guess that's another reason to make the leap. I have got round the validation issue by just validating that user is present - so if user is set to nil, or is completely missing with no user form provided on initialization, the validation will fail...

@contentfree
Copy link

The populator is not called for a nil fragment... that is indeed unexpected behavior..... – @apotonick

Has there been any insight on this in the last few years? I came across this issue today because my populator wouldn't run (and the property was given a nil).

@emaglio emaglio added this to the 3.0 milestone Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants