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

Enable or disable removing false data for JsonToFormDecoder #883

Merged
merged 2 commits into from
Oct 14, 2014
Merged

Enable or disable removing false data for JsonToFormDecoder #883

merged 2 commits into from
Oct 14, 2014

Conversation

meyerbaptiste
Copy link
Contributor

The JsonToFormDecoder class remove false data like a classic form.
But, in the case where we have a form with just one checkbox or patch just one checkbox it does not work because form is not submitted due to missing of parameters in the request and so it is invalid.
This issue can be fixed by replacing the value by null instead of remove it.

So in this PR I propose that we add a container parameter for this service which can enable or disable removing false data.

I use a container parameter because it is a tricky case which does not require to implement a configuration system of the decoder but which leaves the possibility to the developer to modify it if necessary.

`fos_rest.decoder.jsontoform` removes false values. In the case you have a form with just one checkbox or need to patch one checkbox it does not work. Instead of remove false value you can transform it to null. For this you need to override a parameter:
```yaml
parameters:
fos_rest.decoder.jsontoform.remove_false_data: false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we do add thigs feature, then this should be handled via the DI extension

@lsmith77
Copy link
Member

lsmith77 commented Oct 8, 2014

I always have concerns about adding options that have a global effect since it means that people combining different Bundles that require different config will run into trouble. at the same time adding more features that are configured by path etc add overhead.

sigh :-/

@dunglas
Copy link
Contributor

dunglas commented Oct 9, 2014

This limitation only occurs when using the PATCH HTTP method with the JsonToFormDecoder:

  • If the HTTP method is set to PATCH, the Symfony Form Component only updates entity properties submitted by the user.
  • The component also assumes checkboxes behave like in application/x-www-form-urlencoded. To set its value to false, the data for this checkbox must be unset (it's what currently do JsonToFormDecoder). Or, as @meyerbaptiste noticed, set to null.
  • Actually, if JsonToFormDecoder is used together with the PATCH method, a checkbox cannot be set to false because the PATCH mode take precedence and the checkbox is ignored (because unseted).

As the role of JsonToFormDecoder is only converting JSON data to a format accepted by the Form Component, this PR can be simplified a lot: just change the current behavior to always set $value to null and remove the unset call. It will be much simpler and will work in all use cases.

@sroze
Copy link
Contributor

sroze commented Oct 9, 2014

👍

@lsmith77
Copy link
Member

lsmith77 commented Oct 9, 2014

I never used the decoder myself but I guess this could be argued to be a bug fix rather than a BC break ..

@sroze
Copy link
Contributor

sroze commented Oct 9, 2014

@lsmith77 I haven't the same idea: if some one was using this decoder before this change, and was testing values for instance with array_key_exists to make a process based on the checkbox value, it will break his process as the key now exists, but its value is null.

That's why I think that the idea of @meyerbaptiste was acceptable: it don't introduce BC break but allow to fix the issue, and also allow to change this default behaviour (that is now unset instead of null value) in the next major version.

lsmith77 added a commit that referenced this pull request Oct 14, 2014
Enable or disable removing false data for JsonToFormDecoder
@lsmith77 lsmith77 merged commit d83fa5f into FriendsOfSymfony:master Oct 14, 2014
@lsmith77 lsmith77 added this to the 1.5.0 milestone Jan 27, 2015
@adapik
Copy link

adapik commented Feb 7, 2019

OK, now how I cannot distinguish empty field from false field? No chance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants