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

Add a configurePersistentParameters method #6873

Conversation

VincentLanglet
Copy link
Member

Subject

This allow to declare getPersistentParameters as final.

This prevents the developer to override incorrectly the method and loosing the benefit of the extensions.
For instance, if you look at the doc I updated, the override was losing the code.

        foreach ($this->getExtensions() as $extension) {
                $params = $extension->getPersistentParameters($this);

                // NEXT_MAJOR: Remove this check, since return typehint is added
                if (!\is_array($params)) {
                    throw new \RuntimeException(sprintf(
                        'The %s::getPersistentParameters must return an array',
                        \get_class($extension)
                    ));
                }

                $parameters = array_merge($parameters, $params);
        }

Plus it add consistency: You want to override somethings ? Use the protected method configure*.

Changelog

### Added
- `AbstractAdmin::configurePersistentParameters()`
- `AdminExtensionInterface::configurePersistentParameters()`

### Deprecated
- Overriding `AbstractAdmin::getPersistentParameters()`
- `AdminExtensionInterface::getPersistentParameters()`

@VincentLanglet VincentLanglet force-pushed the configurePersistentParameters branch from fb33ec2 to ad749da Compare February 16, 2021 22:18
@VincentLanglet VincentLanglet requested a review from a team February 16, 2021 22:19
docs/reference/routing.rst Outdated Show resolved Hide resolved
src/Admin/AbstractAdmin.php Outdated Show resolved Hide resolved
docs/reference/routing.rst Outdated Show resolved Hide resolved
@VincentLanglet VincentLanglet force-pushed the configurePersistentParameters branch from ad749da to 1035fc6 Compare February 17, 2021 00:12
@VincentLanglet VincentLanglet requested review from phansys and a team February 17, 2021 00:13
@VincentLanglet VincentLanglet requested a review from a team February 17, 2021 09:36
@VincentLanglet VincentLanglet merged commit e4a72b8 into sonata-project:3.x Feb 18, 2021
@VincentLanglet VincentLanglet deleted the configurePersistentParameters branch February 18, 2021 00:54
@@ -32,6 +32,7 @@
* @method void configureDefaultFilterValues(AdminInterface $admin, array &$filterValues)
* @method void configureDefaultSortValues(AdminInterface $admin, array &$sortValues)
* @method void configureFormOptions(AdminInterface $admin, array &$formOptions)
* @method array configurePersistentParameters(AdminInterface $admin, array $parameters)
Copy link
Member

Choose a reason for hiding this comment

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

This causes some deprecation warnings, because the AbstractAdminExtension does not provide this method @VincentLanglet

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

Successfully merging this pull request may close these issues.

4 participants