Ok I've updated the fix as it comes from theCheckboxType since theChoiceType got nested ones when expanded and multiple.

I'm unsure this new fix is acceptable because it leads to the opposite problem (if I'm not mistaken):
Let's look at an entity with two boolean attributesb1 andb2. Currently,b1 andb2 aretrue. If now there is a form intended to only manageb1, that is sumbitted with the checkboxunchecked, thenb1 will flip tofalse (this is what the fix corrects, this is the expected behaviour) butb2 will also flip tofalse even though the form is not supposed to affect it.

@Silverspur It wouldn't if there is no field for thatb2 property which should be the case when a form is not supposed to manage it.

You're perfectly right, I got confused. My bad.

Maybe I should add theclear_missing option in theBaseType with a default false value, so it can be overwritten "internally" for nested types needing to force update likeCheckboxType to make this fix global and consistent.

@webmozart, could you confirm ?

@HeahDude Actually theclear_missing option should be also set totrue by default for multiple+expanded 'choice' fields. Otherwise, when submitting the form with zero checkbox checked in the choice field, nothing will happen.

I still find odd the fact that the data related to a fieldpresent in the form type but with no data sent will :

• not be updated if it is, say, aninteger (which is what I expect, and is the whole reason why $clearMissing is set to false for PATCH requests)
• be updated tofalse if this is acheckbox.

I know the root of this issues comes from the way HTTP forms are designed, but I still find this discrepancies quite unconsistent, and I think it somehow questions the pertinence of systematically setting $clearMissing to false for PATCH, since the setting will only apply to certain types of fields.

@Silverspur

a field present in the form type but with no data sent will not be updated

I don't agree with you as it actually depends on the mandatory$clearMissing to know what to do with it.
Either is will update it by unsetting the previous data (new data =null when field is missing or has an empty string value)OR it will ignore it (which is the need of the PATCH method). This is precisely the meaning of this option and you're right when you say this is because of the way the HTTP is designed.

Actually the clear_missing option should be also set to true by default for multiple+expanded 'choice' fields

This was the misinterpretation of my first fix, but it's not true as we found out thanks to your issue ;)
There is no need (tests speak for themselves) to set this option for theChoiceType as this new fix is applied on submitted children forms (seehttps://github.com/symfony/symfony/pull/17771/files#diff-ca5e25b47f3ecc94cd557946aeb486c6R567)

Ok so I may need to clarify the patch I propose here, as I really think this time this is the way to fix it, at least for keeping BC in 2.7.

#16802 and#17799 pointed out that the expandedChoiceType was not updated with a PATCH method because$clearMissing isfalse is that case.

Then thanks to#17899, I understood that for onceChoiceType is not in cause.

A PATCH method would fail forany nestedCheckboxType in a form likeRadioType which extends it.
This is their particularity since in contrast with other types they don't map a "property" field (model data) but more accurately the state of a field as a virtual boolean as they might not been sent via HTTP when (virtually) "false".

So to fix this in a BC way for 2.7, we need IMO this "hack" as theclear_missing option is a kind of a feature which should only be used internally. But maybe we should add a note about it in the reference ofCheckboxType and set it as default false inBaseType to handle very edge cases.

What I suggest from here, is to revert this change inmaster and properly implement this (BC break) by addingFormConfigInterface::getClearMissing() and document it as well.

Another option (still BC) formaster would be to introduce a (possibly abstract)BooleanType which would hold theclear_missing option and makeCheckboxType extending it.

What do you think symfony deciders ?

Actually this is the same case for thecompound option as said in the docs as of 2.3 :

    compound    ~~~~~~~~    **type**: ``boolean`` **default**: ``false``    This option specifies whether the type contains child types or not. This option    is managed internally for built-in types, so there is no need to configure    it explicitly.

@HeahDude I can see a list of task in the PR description. Are they still accurate? Anything we can do to move this PR forward? Thanks.

@fabpot Yes I guess they are, having aclear_missing option would also help dealing with#18998 with more control fromdisabled option IMHO.

I waited for some feedback, I'm not sure if this is acceptable in 2.7. Any thoughts?

Looks like a bug fix to me, so 2.7 seems fine.

Ok this one is finally ready 🎉, had the chance to talk about this with@webmozart in person during the Hack Day in SymfonyCon Berlin, and we agreed on keeping this internal for now.

I've just renamed the optionforce_submit and use it only when there initial data is set but missing on submission, so PATCH method now works for nested checkboxes and radios, this allowsChoiceType to work with PATCH too.

status: needs work

Great work@HeahDude !

@HeahDude What is the state here?

@xabbuh I'm still quite reluctant to merge this fix as is, as it makes impossible to ignore checkboxes or radios with PATCH request (they will always be submitted).

I think I should reduce the scope of this PR to act only on parent forms containing such fields like theChoiceType (which is also buggy with POST), then better document the problem and its solutions, and continue the work on this as a feature on master.

I mean actually problems with PATCH method are quite expected and the component should handle only two cases IMHO:

1. The form contains every possible fields to update:

   • the view is in html and will bug when submitting some checkboxes or radio, adding hidden input should be the way to go (but is not the component responsibility though), this case does not really make sense to me (cf point 2), excepted when it's about compound forms like expandedChoiceType.

   • the data is an array (i.e got from some json data), name should be explicit and empty strings should be used to update any fields tonull (orfalse for boolean types, the conversion is already handled by the component, there are neither report nor bug for this case).

2. The form contains a subset of the possible fields to update which is actually defining the PATCH context:

   • the view in html should submit all the fields either using the hidden input like in case 1, or we could enforce the behavior with this PR fix (this way should be considered a feature IMO).

   • the data comes from an unserialized array, same as case 1 point 2. There are absolutely no problem with this way of submitting data and nothing to change on this side.

So the problem is really about getting the proper array of submitted data when dealing with HTML form's submission.

Wdyt?

I am against this PR in its current state

Like i already stated here and explained in#20210 (comment) the problem is not the theFormInterface, but the wayRequestHandlerInterface prepares the HTTP body to be submitted to the form. TheFormInterface isnt responsible to fix shortcomings of the HTTP requestsContent-Type. TheRequestHandlerInterface was extracted out of theFormInterface in 2.3 exactly for this reason.

TheRequestHandlerInterface can traverse theFormInterface tree and decide, if it needs to sanitize specific values, depending on the request (method, content-type, etc) and of course depending onFormConfigInterface (and thereforeFormType's options).

To be closed? Updated?

Closing for now.

@HeahDude Any change to continue attempt to fix this? I've taken a look at request handler as suggested in#17771 (comment), but it would require recursive iteration over all form children to find ChoiceType there. It would be quite precedens doing that and I don't think people would be happy to merge something like that