search

laravel-json-api / eloquent

Serialize Eloquent models to JSON API resources
MIT License
12 stars 15 forks source link

Fix wrong Boolean validation #24

Closed sort72 closed 2 years ago

sort72 commented 2 years ago

is_bool() consider 1 and 0 are not booleans. Eloquent may store a boolean like 1 or 0 in database, so if value is 1 or 0 it's considered as a boolean.

This PR fixes a validation error when updating a resource with laravel-json-api. If you have an attribute which is Boolean in resource Schema and don't provide a value when updating the resource, old value will be value retrieved from database (So may be 1 or 0) and it would say provided value is not a bool (Since is_bool() php function consider those values as false).

lindyhopchris commented 2 years ago

You should be using a boolean cast on your model. So I don't think I can accept this PR.

lindyhopchris commented 2 years ago

Actually, scrub that - I'm reading this the wrong way round.

assertValue() is used on the value from JSON. The value in JSON should be a boolean, which is why is_bool is correct. I.e. there's no reason why 1 or 0 should be used in JSON when there is an actual boolean type in JSON.

lindyhopchris commented 2 years ago

In relation to the old value from the database - you absolutely need to use a boolean casts on your model, as that is how a boolean column should be set up on the model.

sort72 commented 2 years ago

In relation to the old value from the database - you absolutely need to use a boolean casts on your model, as that is how a boolean column should be set up on the model.

Totally forgot casting boolean in this case. Thanks!

lindyhopchris commented 2 years ago

no problem. thanks for taking the time for doing the PR, hope I didn't sound too abrupt closing it down!