Remove deprecated code #1417

eerison · 2022-04-22T15:57:51Z

⚠️ Deprecations that need to be solved for Symfony 5 upgrade.

Note
those deprecations are waiting for this PR sonata-project/SonataAdminBundle#7867

Remaining direct deprecation notices (6)

Deprecations related with bundles.

1x: The "Sonata\AdminBundle\Admin\AbstractAdmin::getBatchActions()" method is considered final since sonata-project/admin-bundle 3.102. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Admin\PageAdmin".
1x: The "Sonata\AdminBundle\Admin\AbstractAdmin::getBatchActions()" method is considered final since sonata-project/admin-bundle 3.102. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Admin\SnapshotAdmin".
1x in CreateSnapshotsCommandTest::setUp from Sonata\PageBundle\Tests\Command
1x in SnapshotManagerTest::testEnableSnapshots from Sonata\PageBundle\Tests\Entity
PageExtension without deprecated InitRuntimeInterface #1436 1x: The "Sonata\PageBundle\Twig\Extension\PageExtension" class implements "Twig\Extension\InitRuntimeInterface" that is deprecated since Twig 2.7, to be removed in 3.0.
1x in Issue1134Test::testLabelInShowAction from Sonata\PageBundle\Tests\Functional\Ticket

Warning
if you going to work in one of those deprecations, say which one you will get before start, to avoid more the one guy work in the same thing!

Hanmac · 2022-07-07T11:19:31Z

"Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent::getException()" method is deprecated since Symfony 4.4, use "getThrowable()"

is getting fixed by this: #1429

Hanmac · 2022-07-08T09:07:34Z

The "Sonata\PageBundle\Twig\Extension\PageExtension" class implements "Twig\Extension\InitRuntimeInterface" that is deprecated since Twig 2.7, to be removed in 3.0.

isn't fixed by #1423 but will be by #1436

eerison · 2022-07-08T09:14:45Z

The "Sonata\PageBundle\Twig\Extension\PageExtension" class implements "Twig\Extension\InitRuntimeInterface" that is deprecated since Twig 2.7, to be removed in 3.0.

isn't fixed by #1423 but will be by #1436

Ok I changed ;) Thanks :)

Hanmac · 2022-07-09T08:29:29Z

2x: Passing a null message when instantiating a "Symfony\Component\Validator\ConstraintViolation" is deprecated since Symfony 4.4.
1x in UniqueUrlValidatorTest::testValidateWithPageFound from Sonata\PageBundle\Tests\Validator
1x in UniqueUrlValidatorTest::testValidateWithRootUrlAndNoParent from Sonata\PageBundle\Tests\Validator

will be fixed by this: #1437

The Translator was the culprit

Hanmac · 2022-07-09T10:17:26Z

@VincentLanglet how do i fix these two?

1x: The "Sonata\AdminBundle\Admin\AbstractAdmin::getBaseRoutePattern()" method is considered final since sonata-project/admin-bundle 3.102. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Admin\SharedBlockAdmin".
1x in CreateSnapshotsCommandTest::setUp from Sonata\PageBundle\Tests\Command
1x: The "Sonata\AdminBundle\Admin\AbstractAdmin::getBaseRouteName()" method is considered final since sonata-project/admin-bundle 3.102. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Admin\SharedBlockAdmin".

caused by this:

SonataPageBundle/src/Admin/SharedBlockAdmin.php

Lines 39 to 47 in f669175

    
           public function getBaseRoutePattern() 
        
           { 
        
               return sprintf('%s/%s', parent::getBaseRoutePattern(), 'shared'); 
        
           } 
        
           public function getBaseRouteName() 
        
           { 
        
               return sprintf('%s/%s', parent::getBaseRouteName(), 'shared'); 
        
           }

because these are "would be" final, i can't overwrite them like that, and if i would try to set them later, i can't overwrite the cached values?

VincentLanglet · 2022-07-09T19:34:32Z

You can override directly the protected variable
https://github.com/sonata-project/SonataAdminBundle/blob/4.x/src/Admin/AbstractAdmin.php#L112-L119

Can't it do the job ?

Hanmac · 2022-07-10T06:56:41Z

You can override directly the protected variable https://github.com/sonata-project/SonataAdminBundle/blob/4.x/src/Admin/AbstractAdmin.php#L112-L119

Can't it do the job ?

Yes I could set these but that doesn't work like the current function

Because BaseBlockAdmin doesn't set these values they need to be calculated from class. And when they do, the cache values are set. Now setting the variable doesn't change anything because of the cache ones.

for example i tried this:

    public function configure()
    {
        $this->baseRoutePattern = sprintf('%s/%s', $this->getBaseRoutePattern(), 'shared');
        $this->baseRouteName = sprintf('%s/%s', $this->getBaseRouteName(), 'shared');
    }

which doesn't work because of cached values

VincentLanglet · 2022-07-10T08:17:24Z

Ok, I have some questions

What are the current values of routePattern and routeName with the override ? I see it's %s/shared but I need to know the whole value to understand why we can't reproduce it with

protected $baseRoutePattern = 'foo/shared`
protected $baseRouteName = 'foo/shared`

Why does it matter to suffix this with shared ? Can't we just remove the override and let the default behavior ? This is weird for instance to have a / in the baseRouteName, everywhere else we use _.

It will help me to find the best solutions.

Hanmac · 2022-07-10T10:07:27Z

Ok, I have some questions

* What are the current values of routePattern and routeName with the override ? I see it's `%s/shared` but I need to know the whole value to understand why we can't reproduce it with

protected $baseRoutePattern = 'foo/shared`
protected $baseRouteName = 'foo/shared`

* Why does it matter to suffix this with `shared` ? Can't we just remove the override and let the default behavior ? This is weird for instance to have a `/` in the baseRouteName, everywhere else we use `_`.

It will help me to find the best solutions.

the thing is done using %sonata.page.admin.block.entity% parameter for class and getting the dynamic part, then it uses the function above and adds /shared to it

for why it does it to not collide with the other page admin that uses the same entity as class

VincentLanglet · 2022-07-10T10:51:54Z

I currently doesn't see a better way than
sonata-project/SonataAdminBundle#7867
which would allow the override in PageBundle.

WDYT @jordisala1991 ?

Hanmac · 2022-07-11T07:28:39Z

i need to run the tests for this again, but the only thing i see are the big Twig change
and the Command + Services change.

and both of them already got PRs

VincentLanglet · 2022-07-11T07:48:56Z

IMHO the command PR need to be restarted since we plan to

Remove the BaseCommand
Inject dependencies in the constructor
But it needs Feature/1415/Decouple notification-bundle from cleanup snapshot #1434 I think

For twig there is some changes needed #1423

And if you want to help to find a solution, there is this issue #1442

Hanmac · 2022-07-11T08:18:26Z

1x: The "Sonata\Doctrine\Entity\BaseEntityManager::getConnection()" method is deprecated since sonata-project/sonata-doctrine-extensions 1.15 and will be removed in version 2.0. Use "getEntityManager()->getConnection()" instead.
1x in SnapshotManagerTest::testEnableSnapshots from Sonata\PageBundle\Tests\Entity

hm the snapshotmanager uses sql instead of dql or query builder
i will see if i can make that better

VincentLanglet · 2022-07-11T08:22:26Z

On Sql/dql there is this issue #1426 dunno if it's the same subject.

Hanmac · 2022-07-11T08:23:44Z

yeah partly, but also the enable snapshots query

eerison · 2022-07-11T09:16:21Z

IMHO the command PR need to be restarted since we plan to

Remove the BaseCommand

Inject dependencies in the constructor
But it needs Feature/1415/Decouple notification-bundle from cleanup snapshot #1434 I think

For twig there is some changes needed #1423

And if you want to help to find a solution, there is this issue #1442

I'm deprecating the baseCommand in my Pull request ;)

eerison · 2022-07-13T08:03:18Z

Ok I checked again the 4.x branch and I can see only 6 direct deprecation that is a impediment to support Symfony 5 and sonata 4

VincentLanglet · 2022-07-13T08:09:43Z

The first two will need something like this on SonataAdmin 4 sonata-project/SonataAdminBundle#7867

Hanmac · 2022-07-13T08:19:46Z

The "Sonata\Doctrine\Entity\BaseEntityManager::getConnection()"

the Connection thing is part of the Query rework I'm trying to do, i just need to write better testcases

#1446

Hanmac · 2022-07-13T08:38:28Z

1x: The "Sonata\BlockBundle\Block\BlockContextManager" class is considered final since sonata-project/block-bundle 3.0. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Block\BlockContextManager".
1x in BlockContextManagerTest::testGetWithValidData from Sonata\PageBundle\Tests\Block

1x: The "Sonata\BlockBundle\Block\Service\ContainerBlockService" class is considered final since sonata-project/block-bundle 3.0. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Block\ContainerBlockService".
1x in ContainerBlockServiceTest::testExecute from Sonata\PageBundle\Tests\Block

what to do with these two? the blockbundle ones can't be extended anymore. Are the page bundle ones really needed?

otherwise i will think about something

VincentLanglet · 2022-07-13T08:53:39Z

For the second class,

the first method we're doing the same thing than in BlockBundle
the second method we're just overriding the 'SonataBlockBundle' value.
But since the method is/are public we can use decoration/injection instead of extension

For the first class, the simpler solution I see is to provide an AbstractBlockContextManager in BlockBundle with everything final except protected configureSettings. Then it will be extended by BlockBundle and PageBundle differently.

Hanmac · 2022-07-13T08:57:37Z

yeah i was thinking about decoration too

also PageBundle still points to 3.x Block Bundle and not 4.x, but that might be work for another PR

VincentLanglet · 2022-07-13T08:59:48Z

yeah i was thinking about decoration too

also PageBundle still points to 3.x Block Bundle and not 4.x, but that might be work for another PR

Two solutions:

Doing the PR on BlockBundle 3.x, doing a release, and solving the deprecation
Doing the PR on BlockBundle 4.x, doing a release and solving the issue in the same time we bump to blockBundle 4.

If it's not too much work, I prefer the second solution, this avoid a release on an old branch.

VincentLanglet · 2022-07-13T11:37:44Z

Seems like there is #1448 for the block deprecations.
Help on the review is appreciated @Hanmac :)

jordisala1991 · 2022-07-14T21:07:24Z

    public function configure()
    {
        $this->baseRoutePattern = sprintf('%s/%s', $this->getBaseRoutePattern(), 'shared');
        $this->baseRouteName = sprintf('%s/%s', $this->getBaseRouteName(), 'shared');
    }

What about:

    public function configure()
    {
        $this->baseRoutePattern = sprintf('%s/%s', parent::getBaseRoutePattern(), 'shared');
        $this->baseRouteName = sprintf('%s/%s', parent::getBaseRouteName(), 'shared');
    }

Can you try with that? @Hanmac

Hanmac · 2022-07-15T05:37:19Z

    public function configure()
    {
        $this->baseRoutePattern = sprintf('%s/%s', $this->getBaseRoutePattern(), 'shared');
        $this->baseRouteName = sprintf('%s/%s', $this->getBaseRouteName(), 'shared');
    }

What about:

    public function configure()
    {
        $this->baseRoutePattern = sprintf('%s/%s', parent::getBaseRoutePattern(), 'shared');
        $this->baseRouteName = sprintf('%s/%s', parent::getBaseRouteName(), 'shared');
    }

Can you try with that? @Hanmac

i didn't tried yet, but it doesn't work because calling getBaseRoutePattern would cache the result and then setting baseRoutePattern wouldn't have any effect

jordisala1991 · 2022-07-15T21:32:38Z

I just tried, it cant be done without the Pr opened by @VincentLanglet . So this deprecation will be only fixed on 4.x

eerison · 2022-07-18T09:01:47Z

I will work on those two deprecations

#1470 1x: The "Sonata\AdminBundle\Admin\AbstractAdmin::getBaseRoutePattern()" method is considered final since sonata-project/admin-bundle 3.102. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Admin\SharedBlockAdmin".
1x in BaseBlockAdminTest::testSettingAsEditedOnPreBatchDeleteAction from Sonata\PageBundle\Tests\Admin

#1470 1x: The "Sonata\AdminBundle\Admin\AbstractAdmin::getBaseRouteName()" method is considered final since sonata-project/admin-bundle 3.102. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Admin\SharedBlockAdmin".
1x in BaseBlockAdminTest::testSettingAsEditedOnPreBatchDeleteAction from Sonata\PageBundle\Tests\Admin

eerison · 2022-07-18T09:04:31Z

I just tried, it cant be done without the Pr opened by @VincentLanglet . So this deprecation will be only fixed on 4.x

tried what? I don't get it!

VincentLanglet · 2022-07-18T11:32:05Z

I just tried, it cant be done without the Pr opened by @VincentLanglet . So this deprecation will be only fixed on 4.x

tried what? I don't get it!

You currently cannot solve the deprecations, we need to finish and release sonata-project/SonataAdminBundle#7867. And this will only be solve in 4.x.

The issue is that you cannot override

    public function getBaseRoutePattern()
    {
        return sprintf('%s/%s', parent::getBaseRoutePattern(), 'shared');
    }
    public function getBaseRouteName()
    {
        return sprintf('%s/%s', parent::getBaseRouteName(), 'shared');
    }

eerison · 2022-07-18T11:38:55Z

I just tried, it cant be done without the Pr opened by @VincentLanglet . So this deprecation will be only fixed on 4.x

tried what? I don't get it!

You currently cannot solve the deprecations, we need to finish and release sonata-project/SonataAdminBundle#7867. And this will only be solve in 4.x.

The issue is that you cannot override
    public function getBaseRoutePattern()
    {
        return sprintf('%s/%s', parent::getBaseRoutePattern(), 'shared');
    }
    public function getBaseRouteName()
    {
        return sprintf('%s/%s', parent::getBaseRouteName(), 'shared');
    }

Yeah it's the issue, and when this PR pass, should I be able to override getBaseRoutePattern?

Edit 1: and your PR will fix all those deprecations that are missing?

Hanmac · 2022-07-18T12:12:33Z

1x: The "Sonata\BlockBundle\Block\BlockContextManager" class is considered final since sonata-project/block-bundle 3.0. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Block\BlockContextManager".
1x in BlockContextManagerTest::testGetWithValidData from Sonata\PageBundle\Tests\Block
1x: The "Sonata\BlockBundle\Block\Service\ContainerBlockService" class is considered final since sonata-project/block-bundle 3.0. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Block\ContainerBlockService".
1x in ContainerBlockServiceTest::testExecute from Sonata\PageBundle\Tests\Block

this will be fixed by #1448

VincentLanglet · 2022-07-18T12:30:17Z

Yeah it's the issue, and when this PR pass, should I be able to override getBaseRoutePattern?

You'll be able to override generateBaseRoutePattern. But only in the same PR than the one which bump the SonataAdmin dependency. So it's all or nothing.

eerison · 2022-07-22T08:29:35Z

I guess it's just missing to add sonata admin 4 to solve this deprecation

1x: The "Sonata\AdminBundle\Admin\AbstractAdmin::getBaseRoutePattern()" method is considered final since sonata-project/admin-bundle 3.102. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Admin\SharedBlockAdmin".
1x in BaseBlockAdminTest::testSettingAsEditedOnPreBatchDeleteAction from Sonata\PageBundle\Tests\Admin

VincentLanglet · 2022-07-22T08:32:44Z

It requires a SonataAdmin release first.

VincentLanglet · 2022-07-24T08:21:05Z

I guess it's just missing to add sonata admin 4 to solve this deprecation

1x: The "Sonata\AdminBundle\Admin\AbstractAdmin::getBaseRoutePattern()" method is considered final since sonata-project/admin-bundle 3.102. It may change without further notice as of its next major version. You should not extend it from "Sonata\PageBundle\Admin\SharedBlockAdmin".
1x in BaseBlockAdminTest::testSettingAsEditedOnPreBatchDeleteAction from Sonata\PageBundle\Tests\Admin

Will be done with #1485 so we can be close this

eerison added the feature label Apr 22, 2022

eerison mentioned this issue Apr 22, 2022

Symfony 5 and 6 support #1410

Closed

VincentLanglet added this to the 4.0 milestone Apr 22, 2022

eerison mentioned this issue Jul 12, 2022

Change sonata.templating by twig where it's possible #1444

Merged

eerison mentioned this issue Jul 14, 2022

Support symfony 5 and Sonata 4 #1416

Closed

10 tasks

Amin-Hosseini mentioned this issue Jul 15, 2022

Remove deprecated getConnection #1462

Closed

eerison mentioned this issue Jul 18, 2022

Support multi site using symfony runtime #1454

Closed

VincentLanglet mentioned this issue Jul 18, 2022

Remove deprecated method related with src/Admin/SharedBlockAdmin #1470

Closed

This was referenced Jul 23, 2022

Upgrade to Sonata 4 #1485

Merged

Remove everything related to target #1488

Merged

VincentLanglet closed this as completed Jul 24, 2022

Remove deprecated code #1417

Remove deprecated code #1417

Comments

eerison commented Apr 22, 2022 • edited Loading

⚠️ Deprecations that need to be solved for Symfony 5 upgrade.

Deprecations related with bundles.

Hanmac commented Jul 7, 2022 • edited Loading

Hanmac commented Jul 8, 2022 • edited Loading

eerison commented Jul 8, 2022

Hanmac commented Jul 9, 2022

Hanmac commented Jul 9, 2022

VincentLanglet commented Jul 9, 2022

Hanmac commented Jul 10, 2022 • edited Loading

VincentLanglet commented Jul 10, 2022

Hanmac commented Jul 10, 2022

VincentLanglet commented Jul 10, 2022

Hanmac commented Jul 11, 2022

VincentLanglet commented Jul 11, 2022

Hanmac commented Jul 11, 2022

VincentLanglet commented Jul 11, 2022

Hanmac commented Jul 11, 2022

eerison commented Jul 11, 2022

eerison commented Jul 13, 2022 • edited Loading

VincentLanglet commented Jul 13, 2022

Hanmac commented Jul 13, 2022

Hanmac commented Jul 13, 2022 • edited Loading

VincentLanglet commented Jul 13, 2022

Hanmac commented Jul 13, 2022

VincentLanglet commented Jul 13, 2022

VincentLanglet commented Jul 13, 2022

jordisala1991 commented Jul 14, 2022

Hanmac commented Jul 15, 2022

jordisala1991 commented Jul 15, 2022

eerison commented Jul 18, 2022

eerison commented Jul 18, 2022

VincentLanglet commented Jul 18, 2022

eerison commented Jul 18, 2022 • edited Loading

Hanmac commented Jul 18, 2022

VincentLanglet commented Jul 18, 2022

eerison commented Jul 22, 2022 • edited Loading

VincentLanglet commented Jul 22, 2022

VincentLanglet commented Jul 24, 2022

eerison commented Apr 22, 2022 •

edited

Loading

Hanmac commented Jul 7, 2022 •

edited

Loading

Hanmac commented Jul 8, 2022 •

edited

Loading

Hanmac commented Jul 10, 2022 •

edited

Loading

eerison commented Jul 13, 2022 •

edited

Loading

Hanmac commented Jul 13, 2022 •

edited

Loading

eerison commented Jul 18, 2022 •

edited

Loading

eerison commented Jul 22, 2022 •

edited

Loading