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

[3.0.2]problem with buildForm (formtype) #17791

Closed
alex1442 opened this issue Feb 13, 2016 · 6 comments
Closed

[3.0.2]problem with buildForm (formtype) #17791

alex1442 opened this issue Feb 13, 2016 · 6 comments

Comments

@alex1442
Copy link

in ChoiceType::class
if expanded set to true then
parameter 'empty_data' ignored ,

validator return error not null for this value

a similar problem #17789
sf 3.0.2

@alex1442 alex1442 changed the title problem with buildForm (formtype) [3.0.2]problem with buildForm (formtype) Feb 13, 2016
@xabbuh
Copy link
Member

xabbuh commented Feb 14, 2016

Can you explain your issues a bit more and how it is even different from the referenced issue? The best would be to form the Symfony Standard Edition and make the changes needed to reproduce it.

@alex1442
Copy link
Author

in ChoiceType::class, if expanded ===true ,validator return error :
This value should not be blank.
This value should not be null.,
but empty_value is defined.

class SearchMediaType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $data = [ 'size' => 'thumbnail'];/ /defalt values
        $builder->setMethod('GET')
            ->add('tagName',  SearchType::class)
            ->add('size', ChoiceType::class, [
                'choices' => array('thumbnail' => 'thumbnail',
                'low_resolution' => 'low_resolution',
                'standard_resolution' => 'standard_resolution',
                ),
                'choices_as_values' => true,
                'multiple' => false,
                'expanded'=> TRUE,
                'empty_data' => $data['size']
                ]
            );

Size valus asserts:

//searchmediaentity.php
  /**
     * @Assert\NotBlank()
     * @Assert\NotNull()
     * @Assert\Choice({"thumbnail", "low_resolution", "standard_resolution"})
     */
    protected $size;

if expanded ===false then all right

soft info :
PHP 7.0.3-3+deb.sury.org~wily+1

composer.json:
{
    "name": "alex1442/3.0",
    "license": "proprietary",
    "type": "project",
    "autoload": {
        "psr-4": {
            "": "src/"
        },
        "files": [
            "app/AppKernel.php"
        ]
    },
    "autoload-dev": {
        "psr-4": {
            "Tests\\": "tests/"
        }
    },
    "require": {
        "php": ">=5.5.9",
        "symfony/symfony": "3.0.*",
        "doctrine/orm": "^2.5",
        "doctrine/doctrine-bundle": "^1.6",
        "doctrine/doctrine-cache-bundle": "^1.2",
        "symfony/swiftmailer-bundle": "^2.3",
        "symfony/monolog-bundle": "^2.8",
        "sensio/distribution-bundle": "^5.0",
        "sensio/framework-extra-bundle": "^3.0.2",
        "incenteev/composer-parameter-handler": "^2.0",
        "cosenary/instagram": "^2.3",
        "hirak/prestissimo": "^0.1.7"
    },
    "require-dev": {
        "sensio/generator-bundle": "^3.0",
        "symfony/phpunit-bridge": "^3.0"
    },
    "scripts": {
        "post-install-cmd": [
            "Incenteev\\ParameterHandler\\ScriptHandler::buildParameters",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::buildBootstrap",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::clearCache",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::installAssets",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::installRequirementsFile",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::prepareDeploymentTarget"
        ],
        "post-update-cmd": [
            "Incenteev\\ParameterHandler\\ScriptHandler::buildParameters",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::buildBootstrap",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::clearCache",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::installAssets",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::installRequirementsFile",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::prepareDeploymentTarget"
        ]
    },
    "extra": {
        "symfony-app-dir": "app",
        "symfony-bin-dir": "bin",
        "symfony-var-dir": "var",
        "symfony-web-dir": "web",
        "symfony-tests-dir": "tests",
        "symfony-assets-install": "relative",
        "incenteev-parameters": {
            "file": "app/config/parameters.yml"
        }
    }
}

@HeahDude
Copy link
Contributor

Bug confirmed in 2.7, anytime expanded is true, either multiple is true or not.

I've found a fix in ChoiceType add this line 95 :

if (FormUtil::isEmpty($data)) {
    $data = $form->getConfig()->getEmptyData();
}

Status: reviewed

@HeahDude
Copy link
Contributor

This issue comes from RadioListMapper and CheckboxListMapper IMHO, as they reset submitted data after Form::submit() has used empty_data.

I don't know if we can fix them without the "hack" above in the FormEvents::PRE_SUBMIT event when expanded is true.

@HeahDude
Copy link
Contributor

Beware if you set empty_data as a callable you should use this code instead from line 93 of ChoiceType :

$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) {
    $form = $event->getForm();
    $data = $event->getData();

    if (FormUtil::isEmpty($data)) {
        $emptyData = $form->getConfig()->getEmptyData();
        if (is_callable($emptyData)) {
            $emptyData = call_user_func($emptyData, $form, $data);
        }
        $data = (string) $emptyData;
    }
    // ...

@HeahDude
Copy link
Contributor

See #17822 for a cleaner fix.

fabpot added a commit that referenced this issue Jun 28, 2016
…oiceType` (HeahDude)

This PR was merged into the 2.7 branch.

Discussion
----------

[WIP] [2.7] [Form] fix `empty_data` option in expanded `ChoiceType`

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #17791
| License       | MIT
| Doc PR        | -

It might happen because in `Form::submit()` the handling of `empty_data` [line 597](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Form.php#L597) comes after each child of a compound field has been submitted [line 549](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Form.php#L549).

So when `ChoiceType` is `expanded`, `compound` option is defaulted to `true` and it passes its empty submitted data to its children before handling its own `empty_data` option.

This PR uses the listener already added in `ChoiceType` only when `expanded` is true to handle `empty_data` at `PRE_SUBMIT` [line 539](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Form.php#L539).

- [ ] Fix FQCN in tests for 2.8
- [ ] Remove `choices_as_values` in tests for 3.0

Commits
-------

d479adf [Form] fix `empty_data` option in expanded `ChoiceType`
@fabpot fabpot closed this as completed Jun 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants