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

Empty values can't be coerced #273

Closed
SleeplessByte opened this issue Jan 1, 2016 · 10 comments
Closed

Empty values can't be coerced #273

SleeplessByte opened this issue Jan 1, 2016 · 10 comments
Assignees
Labels
Milestone

Comments

@SleeplessByte
Copy link

When a form is submitted and the value is empty (but present as key because it was in the form).

Coercing

'' Can't be coerced into Date
@SleeplessByte SleeplessByte changed the title Empty values can't be coerced and correctly validated Empty values can't be coerced Jan 1, 2016
@jodosha jodosha added the bug label Jan 1, 2016
@jodosha
Copy link
Member

jodosha commented Jan 1, 2016

@SleeplessByte Thanks for reporting this. We'll start reworking Lotus::Model low level mechanisms next week. So we'll take in account this problem.

For the time being, you can avoid to hit this problem, by validating your data.

@jodosha jodosha self-assigned this Jan 1, 2016
@SleeplessByte
Copy link
Author

I actually unset empty values before I pass them to the repository, because the skip? if blank? only skips it for validation. Not for creation.

@jodosha
Copy link
Member

jodosha commented Jan 2, 2016

@SleeplessByte I'm confused. 😢

You should invoke methods on the repository, only if you're sure they are valid. This is a framework design. In pseudo code:

if my_data_is_valid?
  MyRepository.create(my_entity_instance)
end

As opposite of other libraries like ActiveRecord, where the control happens inside the repository role:

MyModel.create(my_data) # check data inside.

Do you have some code to share with us? It would help to understand better your use case 😄 Thanks!

@SleeplessByte
Copy link
Author

@jodosha Sorry about the confusion.

Adding validations to the action#params actually solved this because it filters out empty params. This however is not clear from the documentation. After I updated my gems I think entity.valid? / action#params.valid? both ignored the empty values correctly.

@SleeplessByte
Copy link
Author

Woops. The bug is still there off course for no validations.

@SleeplessByte SleeplessByte reopened this Jan 3, 2016
@jodosha
Copy link
Member

jodosha commented Jan 5, 2016

Woops. The bug is still there off course for no validations.

Which version do you used before, and which one after the upgrade?

@SleeplessByte
Copy link
Author

0.5.0 to 0.6.x + lotusrb edge.

I have not encountered the error since yesterday.

@jodosha
Copy link
Member

jodosha commented Jan 5, 2016

@SleeplessByte Thank you. I suspect this could be related to hanami/validations#82 /cc @hlegius

@hlegius
Copy link
Contributor

hlegius commented Jan 7, 2016

@SleeplessByte if I got you correctly, you're trying to persist params directly thought your Entity, instead of use the validated params (a.k.a Lotus Validation output (with .to_h method) after you've checked if its valid (.valid?, .invalid?). This way, you'll get this coerce issue.

About blank values, since hanami/validations#82, you can have blank param's keys for both size and type validation' attributes.

If you still experiencing some unexpected behaviour on this, please, paste here some of your implementation. I'd be glad to help you!

@SleeplessByte
Copy link
Author

@jodosha It was, that's why switching to edge resolved it.

@hlegius I was and I was not. I realised quickly that I had to do params.valid? to validate parameters. Right now I just have Lotus::Validations in both model and params. In the documentation .to_h is not mentioned, and it should if that is the way to go.

Right now the flow is something along the lines of:

params do
   param :foo do
       param :bar, baz: :validation
       ...
   end
end

def call( params )
  ...
  if params.valid?
     @my_model = MyModel.new( params[ :foo ] )
     @my_model = MyModelRepository.create( @my_model ) or raise SomeException
     ...
  end
  ...
end

And this works.

I had one problem using presence: false (default) and format: but that's resolved as well.

Long story short (\cc @jodosha ): Personally, I don't think it is clear enough that

  • one must validate params and not the model
  • there is no safe handling of faulty values for model params
  • using params after validation will sanitize empty values
  • which validations are available by default, and which are after skip? (easy to check in the source, but still)

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

3 participants