sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Validating object using multiple unique constraints doesn't work #6232

Closed mikemix closed 3 years ago

mikemix commented 4 years ago

Environment

Docker php:7.3-apache

Sonata packages

$ composer show --latest 'sonata-project/*'

sonata-project/admin-bundle              3.68.0 3.72.0 The missing Symfony Admin Generator
sonata-project/block-bundle              3.20.0 4.2.0  Symfony SonataBlockBundle
sonata-project/cache                     2.0.1  2.0.1  Cache library
sonata-project/core-bundle               3.20.0 3.20.0 Symfony SonataCoreBundle (abandoned)
Package sonata-project/core-bundle is abandoned, you should avoid using it. No replacement was suggested.
sonata-project/doctrine-extensions       1.8.0  1.8.0  Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.19.0 3.21.0 Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/exporter                  2.3.0  2.3.0  Lightweight Exporter library
sonata-project/form-extensions           0.1.2  1.5.0  Symfony form extensions
sonata-project/twig-extensions           0.1.1  1.3.1  Sonata twig extensions

Symfony packages

$ composer show --latest 'symfony/*'

symfony/asset                      v4.4.11 v4.4.11 Symfony Asset Component
symfony/browser-kit                v4.4.11 v4.4.11 Symfony BrowserKit Component
symfony/cache                      v4.4.11 v4.4.11 Symfony Cache component with PSR-6, PSR-16, and tags
symfony/cache-contracts            v2.1.3  v2.1.3  Generic abstractions related to caching
symfony/config                     v4.4.11 v4.4.11 Symfony Config Component
symfony/console                    v4.4.11 v4.4.11 Symfony Console Component
symfony/css-selector               v4.4.11 v4.4.11 Symfony CssSelector Component
symfony/debug                      v4.4.11 v4.4.11 Symfony Debug Component
symfony/dependency-injection       v4.4.11 v4.4.11 Symfony DependencyInjection Component
symfony/deprecation-contracts      v2.1.3  v2.1.3  A generic function and convention to trigger deprecation notices
symfony/doctrine-bridge            v4.4.11 v4.4.11 Symfony Doctrine Bridge
symfony/dom-crawler                v4.4.11 v4.4.11 Symfony DomCrawler Component
symfony/dotenv                     v4.4.11 v4.4.11 Registers environment variables from a .env file
symfony/error-handler              v4.4.11 v4.4.11 Symfony ErrorHandler Component
symfony/event-dispatcher           v4.4.11 v4.4.11 Symfony EventDispatcher Component
symfony/event-dispatcher-contracts v1.1.9  v2.1.3  Generic abstractions related to dispatching event
symfony/expression-language        v4.4.11 v4.4.11 Symfony ExpressionLanguage Component
symfony/filesystem                 v4.4.11 v4.4.11 Symfony Filesystem Component
symfony/finder                     v4.4.11 v4.4.11 Symfony Finder Component
symfony/flex                       v1.9.1  v1.9.1  Composer plugin for Symfony
symfony/form                       v4.4.11 v4.4.11 Symfony Form Component
symfony/framework-bundle           v4.4.11 v4.4.11 Symfony FrameworkBundle
symfony/http-foundation            v4.4.11 v4.4.11 Symfony HttpFoundation Component
symfony/http-kernel                v4.4.11 v4.4.11 Symfony HttpKernel Component
symfony/inflector                  v4.4.11 v4.4.11 Symfony Inflector Component
symfony/intl                       v4.4.11 v4.4.11 A PHP replacement layer for the C intl extension that includes additional data from the ICU library.
symfony/mailer                     v4.4.11 v4.4.11 Symfony Mailer Component
symfony/messenger                  v4.4.11 v4.4.11 Symfony Messenger Component
symfony/mime                       v4.4.11 v4.4.11 A library to manipulate MIME messages
symfony/monolog-bridge             v4.4.11 v4.4.11 Symfony Monolog Bridge
symfony/monolog-bundle             v3.5.0  v3.5.0  Symfony MonologBundle
symfony/options-resolver           v4.4.11 v4.4.11 Symfony OptionsResolver Component
symfony/orm-pack                   v1.1.0  v2.0.0  A pack for the Doctrine ORM
symfony/phpunit-bridge             v5.1.3  v5.1.3  Symfony PHPUnit Bridge
symfony/polyfill-intl-grapheme     v1.18.0 v1.18.0 Symfony polyfill for intl's grapheme_* functions
symfony/polyfill-intl-icu          v1.18.0 v1.18.0 Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-intl-idn          v1.18.0 v1.18.0 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions
symfony/polyfill-intl-normalizer   v1.18.0 v1.18.0 Symfony polyfill for intl's Normalizer class and related functions
symfony/polyfill-mbstring          v1.18.0 v1.18.0 Symfony polyfill for the Mbstring extension
symfony/polyfill-php80             v1.18.0 v1.18.0 Symfony polyfill backporting some PHP 8.0+ features to lower PHP versions
symfony/process                    v4.4.11 v4.4.11 Symfony Process Component
symfony/profiler-pack              v1.0.4  v1.0.4  A pack for the Symfony web profiler
symfony/property-access            v4.4.11 v4.4.11 Symfony PropertyAccess Component
symfony/property-info              v4.4.11 v4.4.11 Symfony Property Info Component
symfony/proxy-manager-bridge       v4.4.11 v4.4.11 Symfony ProxyManager Bridge
symfony/routing                    v4.4.11 v4.4.11 Symfony Routing Component
symfony/security-acl               v3.0.4  v3.0.4  Symfony Security Component - ACL (Access Control List)
symfony/security-bundle            v4.4.11 v4.4.11 Symfony SecurityBundle
symfony/security-core              v4.4.11 v4.4.11 Symfony Security Component - Core Library
symfony/security-csrf              v4.4.11 v4.4.11 Symfony Security Component - CSRF Library
symfony/security-guard             v4.4.11 v4.4.11 Symfony Security Component - Guard
symfony/security-http              v4.4.11 v4.4.11 Symfony Security Component - HTTP Integration
symfony/serializer                 v4.4.11 v4.4.11 Symfony Serializer Component
symfony/serializer-pack            v1.0.3  v1.0.3  A pack for the Symfony serializer
symfony/service-contracts          v2.1.3  v2.1.3  Generic abstractions related to writing services
symfony/stopwatch                  v4.4.11 v4.4.11 Symfony Stopwatch Component
symfony/string                     v5.1.3  v5.1.3  Symfony String component
symfony/templating                 v4.4.11 v4.4.11 Symfony Templating Component
symfony/test-pack                  v1.0.6  v1.0.6  A pack for functional and end-to-end testing within a Symfony app
symfony/translation                v4.4.11 v4.4.11 Symfony Translation Component
symfony/translation-contracts      v2.1.3  v2.1.3  Generic abstractions related to translation
symfony/twig-bridge                v4.4.11 v4.4.11 Symfony Twig Bridge
symfony/twig-bundle                v4.4.11 v4.4.11 Symfony TwigBundle
symfony/validator                  v4.4.11 v4.4.11 Symfony Validator Component
symfony/var-dumper                 v4.4.11 v4.4.11 Symfony mechanism for exploring and dumping PHP variables
symfony/var-exporter               v4.4.11 v4.4.11 A blend of var_export() + serialize() to turn any serializable data structure to plain PHP code
symfony/web-link                   v4.4.11 v4.4.11 Symfony WebLink Component
symfony/web-profiler-bundle        v4.4.11 v4.4.11 Symfony WebProfilerBundle
symfony/webpack-encore-bundle      v1.7.3  v1.7.3  Integration with your Symfony app & Webpack Encore!
symfony/yaml                       v4.4.11 v4.4.11 Symfony Yaml Component

PHP version

$ php -v

PHP 7.3.5 (cli) (built: May  8 2019 02:40:53) ( NTS )

Subject

Steps to reproduce

    public function validate(ErrorElement $errorElement, $object): void
    {
        $errorElement
            ->addConstraint(new UniqueEntity(['fields' => ['slug'], 'message' => 'Slug already in use']))
            ->addConstraint(new UniqueEntity(['fields' => ['email'], 'message' => 'E-mail already in use']));
    }

Expected results

Slug is validated to be unique, then email is validated to be unique.

Actual results

Slug is validated correctly, email validation is skipped which results in Failed to update object exception, because of the SQLSTATE[23000]: Integrity constraint violation.

If I replace the order of constraints, email is validated correctly, but slug's validation is skipped.

VincentLanglet commented 4 years ago

Why not using Annotation and a validation group ?

Do you have a repository to reproduce the bug ? Did you tried to debug the validation ? Do you know where the problem is coming from ?

mikemix commented 4 years ago

Why not using Annotation and a validation group ?

Because I'm not using annotations at all.

Do you have a repository to reproduce the bug ?

Not public so no.

Did you tried to debug the validation ? Do you know where the problem is coming from ?

Tried, but failed, there's so many things going under the hood :)

VincentLanglet commented 4 years ago

Why not using Annotation and a validation group ?

Because I'm not using annotations at all.

I think you still can add constraints like you would have done in a FormType:

->add('email', TextType::class, [
    'constraints' => new UniqueEntity(),
])

The validate method will/should be deprecated in the next major.

Did you tried to debug the validation ? Do you know where the problem is coming from ?

Tried, but failed, there's so many things going under the hood :)

Yeah, that's not gonna be easy.

mikemix commented 4 years ago

I think you still can add constraints like you would have done in a FormType:

No. UniqueEntity is a class constraint.

mikemix commented 4 years ago

This should work fine, this is a bug.

VincentLanglet commented 4 years ago

I think you still can add constraints like you would have done in a FormType:

No. UniqueEntity is a class constraint.

Then I recommend you to add the constraint directly on the class, instead of relying on the admin. The validate method will be removed in the next major of Sonata.

This should work fine, this is a bug.

I never say it wasn't. But if you look at the interface:

    /**
     * NEXT_MAJOR: remove this method.
     *
     * @param object $object
     *
     * @deprecated this feature cannot be stable, use a custom validator,
     *             the feature will be removed with Symfony 2.2
     */
    public function validate(ErrorElement $errorElement, $object);
mikemix commented 4 years ago

Then I recommend you to add the constraint directly on the class, instead of relying on the admin.

This is why I'm relying on the admin in the first place – separation of concerns.

VincentLanglet commented 4 years ago

This is why I'm relying on the admin in the first place – separation of concerns.

I'm not sure I would call this a separation of concern. Your validation is database-related. And you're making this database-related validation in the configuration of the admin, which is just a configuration to display a FormType.

If you have another form on some page, since the validation is database-related, you'll want to have this UniqueEntity validation too.

Where do you describe your database schema ? In your entity ? Then a database-related constraint should be there. There is multiple way to add this validation: https://symfony.com/doc/current/validation.html#constraint-configuration

Anyway that's not the question. If you interested by fixing this bug, you're welcome. I was just warning you that since we're trying to release the next major release relatively soon, I'm not sure we'll spend time to fix a bug on a deprecated feature which does have others solutions.

Devristo commented 4 years ago

Couldn't \Sonata\AdminBundle\Admin\AbstractAdmin::$formOptions be used in this case as well?

VincentLanglet commented 4 years ago

Couldn't \Sonata\AdminBundle\Admin\AbstractAdmin::$formOptions be used in this case as well?

Since the form is created this way

$formBuilder = $this->getFormContractor()->getFormBuilder(
        $this->getUniqid(),
        $this->formOptions
);

You should be able to use $formOptions = [ 'constraints' => ... ], indeed.

mikemix commented 4 years ago

Will try this, thanks.

mikemix commented 3 years ago

Thanks, this works:

    public function configure(): void
    {
        $this->formOptions['constraints'] = [
            new UniqueEntity([
                'fields' => ['country', 'taxNumber'],
            ]),
            new UniqueEntity([
                'fields' => ['slug'],
            ]),
        ];
    }
romeugodoi commented 3 years ago

This not work for me

sonata admin version: 3.55.0

Thanks, this works:

    public function configure(): void
    {
        $this->formOptions['constraints'] = [
            new UniqueEntity([
                'fields' => ['country', 'taxNumber'],
            ]),
            new UniqueEntity([
                'fields' => ['slug'],
            ]),
        ];
    }
7ochem commented 3 years ago

So in the meantime, $this->formOptions is deprecated and you should now use:

    protected function configureFormOptions(array &$formOptions)
    {
        parent::configureFormOptions($formOptions);
        $formOptions['constraints'] = [
            new UniqueEntity([
                'fields' => [ ..... ],
            ]),
        ];
    }
romeugodoi commented 3 years ago

So in the meantime, $this->formOptions is deprecated and you should now use:

    protected function configureFormOptions(array &$formOptions)
    {
        parent::configureFormOptions($formOptions);
        $formOptions['constraints'] = [
            new UniqueEntity([
                'fields' => [ ..... ],
            ]),
        ];
    }

Very grateful for your reply! Thank you so much