8000 Update for ChoiceType field via PATCH HTTP method doesn't work · Issue #17799 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Update for ChoiceType field via PATCH HTTP method doesn't work #17799

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.

Sign up for GitHub

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

Jump to bottom
Closed
ostrolucky opened this issue Feb 14, 2016 · 21 comments
Closed

Update for ChoiceType field via PATCH HTTP method doesn't work #17799

ostrolucky opened this issue Feb 14, 2016 · 21 comments
Labels
Bug Form Keep open Status: Reviewed

Comments

@ostrolucky
Copy link
Contributor

Form having only one field (choiceType) works fine if sent as POST, but doesn't if same data are sent as PATCH. Tested on symfony 2.7.9 and 3.0.2.
@HeahDude
Copy link
Contributor

Hi @gadelat, with a PATCH method, $clearMissing is false when submitting, see :
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/NativeRequestHandler.php#L129
So it may be related to #16802, could you please try the fix of this pending PR #17771 ?

ostrolucky added a commit to ostrolucky/sfBug that referenced this issue Feb 15, 2016
@ostrolucky
symfony/symfony#17799 bug example
af2529b
@HeahDude
Copy link
Contributor

I can confirm your bug as of 2.7, but even if the the fix provided by #17771 might work, I've experience the same experience as you with validation.

We cannot close this one as duplicate of #16802 yet since in your case the proper fix would be :

elseif (!$isSubmitted && !$clearMissing && array_key_exists($name, $submittedData) && 'placeholder' != $name) {
    $child->submit($submittedData[$name], $clearMissing);
}

Note that the validation does fail but the data has been updated anyway.

Errors are showing up in wdt if choices before matching the submitted one were unsuccessful.

I can't get a way to fix both issue at the same time, my test here does fail with the above code.

Has anyone got a clue ?

@HeahDude
Copy link
Contributor

Ok I think I found a proper way to fix it, could you please try to add this at line 560 of Form :

$clearMissing = $this->getConfig()->getOption('expanded', false) ?: $clearMissing;

@HeahDude HeahDude mentioned this issue Feb 16, 2016
Closed
3 tasks
@ostrolucky
Copy link
Contributor Author

Works for both Symfony 2.7 and Symfony 3

@HeahDude
Copy link
Contributor

Thank you for your feedback.

@HeahDude
Copy link
Contributor

"unconfirmed" label should be removed.

Status: reviewed

@Silverspur
Copy link

I found this thread looking for a solution of the tackled issue in the case of a expanded+multiple choice field (array of checkboxes). The patch clearly solves the problem in this case, but I'm also wondering how to deal with a single checkbox, which is not solved here.

Since $clearMissing is false, the submission of a form with an unchecked checkbox won't make its related data flip from true to false. I see one solution, i.e. to systematically explicitly send from the front end a reserved value, like __false, for any unchecked checkbox whose value must be updated, and using in Symfony a data transformer to link this specific value to 'null'. I find this a bit of hacking. Do I miss something obvious?

@HeahDude
Copy link
Contributor

Hi @Silverspur, thank you for your feedback.

You're right at this time, this patch I provided is more like a "hack" to get things working, I'm still looking for a "proper" way to fix this globally.

If you encountered this issue with a single CheckboxType I would suggest you open another issue and provide more information about the steps needed to reproduce your issue.

@Silverspur Silverspur mentioned this issue Feb 23, 2016
Open
@leevigraham
Copy link
Contributor

$clearMissing = $this->getConfig()->getOption('expanded', false) ?: $clearMissing;

I fixed my issue by removing 'expanded' => true from my forms.I guess this isn't an option for everyone tho.

@liverbool
Copy link
Contributor
liverbool commented Apr 27, 2017 •
edited
Loading

bug confirm: sf ^3.2 (expanded = true)

// \Symfony\Component\Form\Extension\Core\DataMapper\RadioListMapper
public function mapFormsToData($radios, &$choice)
    {
        if (null !== $choice && !is_string($choice)) {
            throw new UnexpectedTypeException($choice, 'null or string');
        }

        $choice = null;

        foreach ($radios as $radio) {
            // fixed by adding: && $radio->isSubmitted()
            if ($radio->getData() && $radio->isSubmitted()) {
                if ('placeholder' === $radio->getName()) {
                    return;
                }

                $choice = $radio->getConfig()->getOption('value');

                return;
            }
        }
    }

@SlKelevro
Copy link

Same bug here, SF 3.2.7, but my guess is it's been there for a very long time. Luckily, setting 'expanded' => false works for me, but it's not an option in general. Tried fix from @liverbool ($checkbox->isSubmitted() in CheckboxListMapper in my case) and it seems to work.

Example, just in case:

        $existing  = ['items' => [1, 3]];
        $submitted = ['items' => [2, 4]];

        $builder = $this->createFormBuilder($existing);
        $builder->add(
            'items',
            ChoiceType::class,
            [
                'choices'  => [
                    'One'   => 1,
                    'Two'   => 2,
                    'Three' => 3,
                    'Four'  => 4,
                    'Five'  => 5,
                ],
                'expanded' => true,
                'multiple' => true,
            ]
        );

        $form = $builder->getForm();
        $form->submit($submitted, false);

        var_dump($form->getData()['items']);

Actual:

  0 => int 1
  1 => int 3
  2 => int 2
  3 => int 4

Expected:

  0 => int 2
  1 => int 4

@jr-k
Copy link
jr-k commented Oct 17, 2017

Still the issue now (3.3), is it possible to include the hack of HeahDude ? Thanks

@Viktor-Bredihin
Copy link

any update on this one? The bug request created 3 years ago and still no fix

@xabbuh
Copy link
Member
xabbuh commented Feb 8, 2019

You could be the one to write the fix. I will happily review a pull request.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@ostrolucky
Copy link
Contributor Author

yes

@carsonbot carsonbot removed the Stalled label Dec 19, 2020
@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3

@xabbuh xabbuh added Keep open and removed Stalled labels Jul 7, 2021
@magikid-sourceability
Copy link

Since Symfony 3.3 is no longer supported, has anyone tested to see if this is still an issue on Symfony 5 or 6?

@connorhu
Copy link
Contributor
connorhu commented Mar 14, 2024 •
edited
Loading

This issue is no longer relevant (if SlKelevro's code is correct and the test I wrote is correct).

Test: https://github.com/connorhu/symfony-issue-verify/blob/gh17799/tests/Issues/GH17799Test.php

git clone --branch gh17799 https://github.com/connorhu/symfony-issue-verify && cd symfony-issue-verify && ./init.sh 17799

@ostrolucky
Copy link
Contributor Author

Indeed, this looks solved now! Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Form Keep open Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
0