Poc rebased #718

loic425 · 2023-06-21T09:18:08Z

Q	A
Bug fix?	no/yes
New feature?	no/yes
BC breaks?	no/yes
Deprecations?	no/yes
Related tickets	fixes #X, partially #Y, mentioned in #Z
License	MIT

Co-authored-by: Łukasz Chruściel <lchrusciel@gmail.com>

This PR was merged into the poc-rebased branch. Discussion ---------- | Q | A | --------------- | ----- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | | License | MIT Based on #714 Commits ------- 673b2d2 Fix Debug resource commnd test 37ddf17 Skip running Phpspec on PHP 8.2

This PR was merged into the poc-rebased branch. Discussion ---------- | Q | A | --------------- | ----- | Bug fix? | no/yes | New feature? | no/yes | BC breaks? | no/yes | Deprecations? | no/yes  | Related tickets | fixes #X, partially #Y, mentioned in #Z | License | MIT Commits ------- d29d514 [Maintenance] Update the copyright block in all PHP classes bcf4e4e [Maintenance] Update the copyright block in all XML files 8b2b512 [Maintenance] Update the copyright block in all YML files 39b8e1e [Maintenance] Update the copyright and dates in the LICENSE files

Revert "Update licence"

This PR was merged into the poc-rebased branch. Discussion ---------- | Q | A | --------------- | ----- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Related tickets | fixes #686 | License | MIT Commits ------- a8dec75 [New docs] Configure the resource name 8100200 Add item on table of contents 0c54190 Reapply missing commit

Co-authored-by: Mateusz Zalewski <zaleslaw@gmail.com>

This PR was merged into the poc-rebased branch. Discussion ---------- | Q | A | --------------- | ----- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Related tickets | fixes #688 | License | MIT Commits ------- 0dbec8d [New docs] Configure the resource plural name 5080334 Apply suggestions from code review

This PR was merged into the poc-rebased branch. Discussion ---------- | Q | A | --------------- | ----- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Related tickets | | License | MIT [Preview](https://github.com/Sylius/SyliusResourceBundle/blob/8bdb41f96799c19a6832bdc1b2f50c24e70f82ba/docs/index.md) Commits ------- 7355dfd [New docs] docs' pagination 22599aa Fix next page on configure your operations to redirect page

Zales0123

Obviously, it was impossible to review it fully, but it was already reviewed in partial PRs so it seems as be ready to merge 🖖

Zales0123 · 2023-06-21T09:25:45Z

UPGRADE.md

@@ -1,3 +1,19 @@
+## UPGRADE FOR `1.11.x`
+
+### FROM `1.10.x` to `1.11.x`


It would be probably wise to test this upgrade before (or just after) merge on some real project (Monfony? 😅), as it's very suspicious it needs only one thing to be changed 😄

truth been told, there are chances that this is the only needed change as we wanted to be backward compatible 🎉

Zales0123 · 2023-06-21T09:53:26Z

src/Bundle/DependencyInjection/Compiler/RegisterStateMachinePass.php

+        $this->registerWinzouStateMachine($container);
+        $this->registerSymfonyWorkflowStateMachine($container);


It the same as two lines below 🤔

Zales0123 · 2023-06-21T09:54:41Z

src/Bundle/DependencyInjection/Driver/AbstractDriver.php

+        $container->setDefinition($metadata->getServiceId('factory'), $definition)
+            ->addTag('sylius.resource_factory')
+        ;


Suggested change

$container->setDefinition($metadata->getServiceId('factory'), $definition)

->addTag('sylius.resource_factory')

;

$container

->setDefinition($metadata->getServiceId('factory'), $definition)

->addTag('sylius.resource_factory')

;

?

Zales0123 · 2023-06-21T09:58:00Z

src/Bundle/DependencyInjection/SyliusResourceExtension.php

+        $container->registerForAutoconfiguration(ProviderInterface::class)
+            ->addTag('sylius.state_provider')
+        ;


Suggested change

$container->registerForAutoconfiguration(ProviderInterface::class)

->addTag('sylius.state_provider')

;

$container

->registerForAutoconfiguration(ProviderInterface::class)

->addTag('sylius.state_provider')

;

or

Suggested change

$container->registerForAutoconfiguration(ProviderInterface::class)

->addTag('sylius.state_provider')

;

$container->registerForAutoconfiguration(ProviderInterface::class)->addTag('sylius.state_provider');

if it fits the line characters limit 💃

I know I'm meticulous, but it's what makes Sylius Sylius xD

Zales0123 · 2023-06-21T10:08:18Z

src/Bundle/SyliusResourceBundle.php

@@ -39,6 +39,8 @@ final class SyliusResourceBundle extends Bundle

    public const DRIVER_DOCTRINE_PHPCR_ODM = 'doctrine/phpcr-odm';

+    public const NO_DRIVER = false;


It seems to not be used anywhere

Zales0123 · 2023-06-21T20:37:08Z

src/Component/Metadata/Operation/HttpOperationInitiator.php

+        if (str_contains($resource, '.')) {
+            $metadata = $this->resourceRegistry->get($resource);
+        } else {
+            $metadata = $this->resourceRegistry->getByClass($resource);
+        }


There are a few places where this if-else statement is repeated - probably a space for some ResourceMetadataProvider 💃

Zales0123 · 2023-06-21T20:58:48Z

src/Component/Metadata/Resource/Factory/AttributesResourceMetadataCollectionFactory.php

+use Sylius\Component\Resource\Symfony\Request\State\Responder;
+use Sylius\Component\Resource\Symfony\Routing\Factory\OperationRouteNameFactory;
+
+final class AttributesResourceMetadataCollectionFactory implements ResourceMetadataCollectionFactoryInterface


Let's obviously leave it as it is right now, but we should think about some refactoring of this class - its spec has almost 1000 lines of code 😅

Zales0123 · 2023-06-21T21:55:50Z

src/Component/Symfony/EventDispatcher/OperationEventHandler.php

+        $request = $context->get(RequestOption::class)?->request();
+
+        if (
+            'html' === $request?->getRequestFormat() &&


Let's avoid hardcoding strings in the code and use a private constant here

Zales0123 · 2023-06-21T21:57:23Z

src/Component/Symfony/EventListener/AddFormatListener.php

+        if (
+            null === $operation
+        ) {


Suggested change

if (

null === $operation

) {

if (null === $operation)

{

💃

Zales0123 · 2023-06-21T21:59:46Z

src/Component/Symfony/EventListener/FormListener.php

+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Event\ViewEvent;
+
+final class FormListener


There is not spec for this file, as far as I see

loic425 · 2023-06-22T05:53:12Z

Thx a lot for this huge review. I'll male the change this morning.

lchrusciel · 2023-06-22T06:09:06Z

UPGRADE.md

@@ -1,3 +1,19 @@
+## UPGRADE FOR `1.11.x`
+
+### FROM `1.10.x` to `1.11.x`


truth been told, there are chances that this is the only needed change as we wanted to be backward compatible 🎉

lchrusciel · 2023-06-22T06:11:16Z

Thank you, @loic425!

loic425 and others added 30 commits June 21, 2023 09:59

Move path and route prefix into http operation

3d6f705

Fix Phpspec tests

60c3cc3

Fix Phpspec tests

f830fe5

Add context

4240e87

Remove custom controller

4a6929c

Fix coding standard

79f58c8

Add update operation with minimal data

b86dd7a

Improve arguments' order

3167920

Remove controller

a8014f8

Add delete operation with minimal data

2e168ae

Improve arguments' order

3da5722

Remove controller

0ad8ff3

Add show operation with minimal data

3e40989

Remove controller

b996d88

Add index

5ec64ed

Symplify it

74e722f

Add last specifications

9909164

Add operations

3c8d3fd

Add resource

51637a7

Fix coding standard

139aed5

Add operations

f7e4858

Add providers and processors

0822d1f

Apply suggestions from code review

f6313c9

Co-authored-by: Łukasz Chruściel <lchrusciel@gmail.com>

Fix coding standard

7f987b0

Add some assertions

23cf316

Add experimental tag on callable provider & processor

7dd159b

Improve provider and processor with suggested changes

8c03601

Fix coding standard

65c03e4

Add request option

d21ca8d

Add missing http foundation on component

3e160ec

loic425 and others added 5 commits June 21, 2023 10:07

Configure the path and short name

3fdbabc

Fix register state machine pass

edbdd05

Fix Debug resource commnd test

673b2d2

Skip running Phpspec on PHP 8.2

37ddf17

loic425 requested a review from a team as a code owner June 21, 2023 09:18

Rafikooo added 4 commits June 21, 2023 16:58

[Maintenance] Update the copyright block in all PHP classes

d29d514

[Maintenance] Update the copyright block in all XML files

bcf4e4e

[Maintenance] Update the copyright block in all YML files

8b2b512

[Maintenance] Update the copyright and dates in the LICENSE files

39b8e1e

loic425 force-pushed the poc-rebased branch from 39b8e1e to 61a1ca1 Compare June 21, 2023 15:24

Zales0123 and others added 13 commits June 21, 2023 22:02

[New docs] Configure the resource name

a8dec75

Add item on table of contents

8100200

Reapply missing commit

0c54190

Revert "Update licence"

ca39894

Merge pull request #720 from Sylius/revert-719-update-licence

7a3d8c9

Revert "Update licence"

[New docs] Configure the resource plural name

0dbec8d

Apply suggestions from code review

5080334

Co-authored-by: Mateusz Zalewski <zaleslaw@gmail.com>

[New docs] docs' pagination

7355dfd

Fix next page on configure your operations to redirect page

22599aa

Zales0123 approved these changes Jun 21, 2023

View reviewed changes

lchrusciel approved these changes Jun 22, 2023

View reviewed changes

lchrusciel merged commit 8ff079f into 1.11 Jun 22, 2023
69 checks passed

lchrusciel deleted the poc-rebased branch June 22, 2023 06:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Poc rebased #718

Poc rebased #718

loic425 commented Jun 21, 2023

Zales0123 left a comment

Zales0123 Jun 21, 2023

lchrusciel Jun 22, 2023

Zales0123 Jun 21, 2023

Zales0123 Jun 21, 2023

Zales0123 Jun 21, 2023

Zales0123 Jun 21, 2023

Zales0123 Jun 21, 2023

Zales0123 Jun 21, 2023

Zales0123 Jun 21, 2023

Zales0123 Jun 21, 2023

Zales0123 Jun 21, 2023

loic425 commented Jun 22, 2023

lchrusciel Jun 22, 2023

lchrusciel commented Jun 22, 2023

		@@ -1,3 +1,19 @@
		## UPGRADE FOR `1.11.x`

		### FROM `1.10.x` to `1.11.x`

		$this->registerWinzouStateMachine($container);
		$this->registerSymfonyWorkflowStateMachine($container);

		@@ -39,6 +39,8 @@ final class SyliusResourceBundle extends Bundle

		public const DRIVER_DOCTRINE_PHPCR_ODM = 'doctrine/phpcr-odm';

		public const NO_DRIVER = false;

Poc rebased #718

Poc rebased #718

Conversation

loic425 commented Jun 21, 2023

Zales0123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loic425 commented Jun 22, 2023

Choose a reason for hiding this comment

lchrusciel commented Jun 22, 2023