Upgrade Sonatablock and SonataSeo #1448

Daric971 · 2022-07-13T10:37:32Z

Subject

I am targeting this branch, because I updated to allow version 3 of sonataSeo

Closes #{put_issue_number_here}.

Changelog

### Added
- Added some `Class::newMethod()` to do great stuff.

### Changed
- Config flies (block and admin)
- Admin class
- BlockService classes
- FormType classes
- PageService classes
- Listeners
- mapper
- Tests

### Deprecated

### Removed

### Fixed

### Security

composer.json

src/Block/ContainerBlockService.php

src/Block/BreadcrumbBlockService.php

src/Block/BlockContextManager.php

src/Block/ContainerBlockService.php

src/Block/PageListBlockService.php

src/DependencyInjection/SonataPageExtension.php

src/Form/Type/PageSelectorType.php

src/Form/Type/PageTypeChoiceType.php

src/Form/Type/TemplateChoiceType.php

src/Resources/config/admin.xml

jordisala1991 · 2022-07-13T16:15:30Z

src/Resources/config/block.xml

-            <argument>sonata.page.block.breadcrumb</argument>
-            <argument type="service" id="sonata.templating"/>
-            <argument type="service" id="knp_menu.menu_provider"/>
+            <argument type="service" id="twig"/>


Are all those changes correct?

ContainerBlockService from SonataBlockBundle is a final, I have to decorate, so give it in argurment

Seo has been updated, BaseBreadcrumbMenuBlockService need twig and FactoryInterface(KNP). Plus, sonata.templating disappear from the project

where are you decorating the "SonataBlockBundle"?

BlockContextManager & ContainerBlockService

src/Block/BlockContextManager.php

SonataCI · 2022-07-13T19:20:55Z

Could you please rebase your PR and fix merge conflicts?

composer.json

src/Block/BlockContextManager.php

src/Block/BreadcrumbBlockService.php

eerison · 2022-07-15T09:39:05Z

should be good provide a list of things, pages or commands that was changed! with this we can check when we going to test 4.x branch.

well I'm a bit afraid to say that it's fine to merge, because I can see too much changes and any new test created and if you see, block and admin folder are with low test coverage, then I don't know :/

@Daric971 after you made all those changes every thing works as expected?

VincentLanglet · 2022-07-15T09:43:04Z

should be good provide a list of things, pages or commands that was changed! with this we can check when we going to test 4.x branch.

well I'm a bit afraid to say that it's fine to merge, because I can see too much changes and any new test created and if you see, block and admin folder are with low test coverage, then I don't know :/

@Daric971 after you made all those changes every thing works as expected?

Maybe the first things to do would be to open a PR on 4.x to improve the coverage then

eerison · 2022-07-15T10:10:04Z

should be good provide a list of things, pages or commands that was changed! with this we can check when we going to test 4.x branch.
well I'm a bit afraid to say that it's fine to merge, because I can see too much changes and any new test created and if you see, block and admin folder are with low test coverage, then I don't know :/
@Daric971 after you made all those changes every thing works as expected?

Maybe the first things to do would be to open a PR on 4.x to improve the coverage then

maybe just cover the code that will be changed before change is reasonable. specially when you are not sure how everything works!

and probably we can find code that is not necessary, and leads to create a simple solution.

jordisala1991 · 2022-07-15T13:50:14Z

Increasing phpstan and psalm also gives confidence

Daric971 · 2022-07-18T12:23:58Z

I'm gonna try this change in my project

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

There is also some conflitct on the pr

src/Admin/SharedBlockAdmin.php

src/Resources/config/admin.xml

jordisala1991

Changes on the Form folder should be reverted (they should be handlded on another PR)

SonataCI · 2022-07-19T07:50:54Z

Could you please rebase your PR and fix merge conflicts?

VincentLanglet · 2022-07-19T15:56:29Z

Can you revert the change done in the Form folder @Daric971 ?

We'll add all the typehint in another PR.

src/Admin/BaseBlockAdmin.php

src/Admin/BlockAdmin.php

src/Block/BlockContextManager.php

jordisala1991 · 2022-07-20T06:17:48Z

src/Block/BlockContextManager.php

+        $this->optionsResolver = new OptionsResolver();
+    }
+
+    public function getOptionsResolver(): OptionsResolver


This method is not part of the interface

src/Block/BreadcrumbBlockService.php

SonataCI · 2022-07-20T08:00:55Z

Could you please rebase your PR and fix merge conflicts?

SonataCI · 2022-07-20T12:20:54Z

Could you please rebase your PR and fix merge conflicts?

jordisala1991

We are almost there.

jordisala1991 · 2022-07-20T13:52:35Z

src/Block/PageListBlockService.php

+        return 'sonata.page.block.pagelist';
+    }
+
+    public function buildEditForm(AdminFormMapper $form, BlockInterface $block): void


the code inside this method should go to the configureEditForm instead right?

jordisala1991 · 2022-07-21T19:22:42Z

Do you mind if I start a PR on top of your commits to move this faster? currently we are blocked because we need this PR merged.

jordisala1991 · 2022-07-22T06:51:27Z

This continues here: #1483

VincentLanglet requested changes Jul 13, 2022

View reviewed changes

VincentLanglet mentioned this pull request Jul 13, 2022

Remove deprecated code #1417

Closed

17 tasks

Daric971 force-pushed the upgrade-sonata-block branch from 7525b01 to 8bc6056 Compare July 13, 2022 12:08

VincentLanglet requested changes Jul 13, 2022

View reviewed changes

src/Block/ContainerBlockService.php Outdated Show resolved Hide resolved

src/Block/PageListBlockService.php Show resolved Hide resolved

Daric971 force-pushed the upgrade-sonata-block branch from 8bc6056 to 541fa4b Compare July 13, 2022 15:10

VincentLanglet previously approved these changes Jul 13, 2022

View reviewed changes

VincentLanglet requested a review from jordisala1991 July 13, 2022 16:06

jordisala1991 reviewed Jul 13, 2022

View reviewed changes

SonataCI added the pending author label Jul 13, 2022

eerison reviewed Jul 15, 2022

View reviewed changes

composer.json Show resolved Hide resolved

composer.json Show resolved Hide resolved

src/Block/BlockContextManager.php Outdated Show resolved Hide resolved

src/Block/BreadcrumbBlockService.php Outdated Show resolved Hide resolved

SonataCI removed the pending author label Jul 18, 2022

Daric971 dismissed VincentLanglet’s stale review via 7a24620 July 18, 2022 15:22

Daric971 force-pushed the upgrade-sonata-block branch from 541fa4b to 7a24620 Compare July 18, 2022 15:22

jordisala1991 reviewed Jul 19, 2022

View reviewed changes

src/Admin/SharedBlockAdmin.php Outdated Show resolved Hide resolved

jordisala1991 reviewed Jul 19, 2022

View reviewed changes

src/Resources/config/admin.xml Outdated Show resolved Hide resolved

jordisala1991 requested changes Jul 19, 2022

View reviewed changes

SonataCI added pending author and removed pending author labels Jul 19, 2022

Daric971 force-pushed the upgrade-sonata-block branch 2 times, most recently from 2904e7e to a0f473d Compare July 19, 2022 15:34

jordisala1991 reviewed Jul 20, 2022

View reviewed changes

src/Admin/BaseBlockAdmin.php Outdated Show resolved Hide resolved

jordisala1991 reviewed Jul 20, 2022

View reviewed changes

src/Admin/BlockAdmin.php Outdated Show resolved Hide resolved

jordisala1991 reviewed Jul 20, 2022

View reviewed changes

src/Block/BlockContextManager.php Outdated Show resolved Hide resolved

jordisala1991 reviewed Jul 20, 2022

View reviewed changes

src/Block/BreadcrumbBlockService.php Outdated Show resolved Hide resolved

SonataCI added pending author and removed pending author labels Jul 20, 2022

Daric971 force-pushed the upgrade-sonata-block branch from a0f473d to ec45672 Compare July 20, 2022 12:17

SonataCI added the pending author label Jul 20, 2022

Daric971 force-pushed the upgrade-sonata-block branch from ec45672 to 280d63f Compare July 20, 2022 12:42

SonataCI removed the pending author label Jul 20, 2022

Upgrade Sonatablock and SonataSeo

dee879a

Daric971 force-pushed the upgrade-sonata-block branch from 280d63f to dee879a Compare July 20, 2022 13:24

jordisala1991 reviewed Jul 20, 2022

View reviewed changes

jordisala1991 mentioned this pull request Jul 21, 2022

Increase PHPStan to level 5 and Psalm to level 7 #1471

Merged

jordisala1991 closed this Jul 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Sonatablock and SonataSeo #1448

Upgrade Sonatablock and SonataSeo #1448

Daric971 commented Jul 13, 2022

jordisala1991 Jul 13, 2022

Daric971 Jul 18, 2022

Daric971 Jul 18, 2022

eerison Jul 19, 2022

Daric971 Jul 19, 2022

SonataCI commented Jul 13, 2022

eerison commented Jul 15, 2022

VincentLanglet commented Jul 15, 2022

eerison commented Jul 15, 2022 •

edited

Loading

jordisala1991 commented Jul 15, 2022

Daric971 commented Jul 18, 2022

VincentLanglet commented Jul 18, 2022

jordisala1991 left a comment

SonataCI commented Jul 19, 2022

VincentLanglet commented Jul 19, 2022

jordisala1991 Jul 20, 2022

SonataCI commented Jul 20, 2022

SonataCI commented Jul 20, 2022

jordisala1991 left a comment

jordisala1991 Jul 20, 2022

jordisala1991 commented Jul 21, 2022

jordisala1991 commented Jul 22, 2022

Upgrade Sonatablock and SonataSeo #1448

Upgrade Sonatablock and SonataSeo #1448

Conversation

Daric971 commented Jul 13, 2022

Subject

Changelog

jordisala1991 Jul 13, 2022

Choose a reason for hiding this comment

Daric971 Jul 18, 2022

Choose a reason for hiding this comment

Daric971 Jul 18, 2022

Choose a reason for hiding this comment

eerison Jul 19, 2022

Choose a reason for hiding this comment

Daric971 Jul 19, 2022

Choose a reason for hiding this comment

SonataCI commented Jul 13, 2022

eerison commented Jul 15, 2022

VincentLanglet commented Jul 15, 2022

eerison commented Jul 15, 2022 • edited Loading

jordisala1991 commented Jul 15, 2022

Daric971 commented Jul 18, 2022

VincentLanglet commented Jul 18, 2022

jordisala1991 left a comment

Choose a reason for hiding this comment

SonataCI commented Jul 19, 2022

VincentLanglet commented Jul 19, 2022

jordisala1991 Jul 20, 2022

Choose a reason for hiding this comment

SonataCI commented Jul 20, 2022

SonataCI commented Jul 20, 2022

jordisala1991 left a comment

Choose a reason for hiding this comment

jordisala1991 Jul 20, 2022

Choose a reason for hiding this comment

jordisala1991 commented Jul 21, 2022

jordisala1991 commented Jul 22, 2022

eerison commented Jul 15, 2022 •

edited

Loading