Sylius route with attributes #334

loic425 · 2021-11-10T06:58:26Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Related tickets
License	MIT

<?php

declare(strict_types=1);

namespace App\Entity;

use App\Annotation\SyliusCrudRoutes;
use App\Annotation\SyliusRoute;
use Sylius\Component\Resource\Model\ResourceInterface;

#[SyliusCrudRoutes(
    alias: 'app.book',
    path: 'library',
    section: 'backend',
    redirect: 'index',
    grid: 'sylius_backend_admin_user',
    except: ['show'],
    vars: [
        'all' => [
            'subheader' => 'sylius.ui.manage_users_able_to_access_administration_panel',
        ],
        'index' => [
            'icon' => 'lock',
        ]
    ],
)]

#[SyliusRoute(
    name: 'app_backend_book_show',
    path: '/library/{id}',
    methods: ['GET'],
    controller: 'app.controller.book::indexAction',
    template: 'backend/book/show.html.twig',
    vars: [
        'subheader' => 'sylius.ui.manage_users_able_to_access_administration_panel',
    ]
)]
class Book implements ResourceInterface
{
    private ?int $id = null;

    public function getId(): ?int
    {
        return $this->id;
    }
}

Crud routes

Single routes

lchrusciel · 2021-12-10T16:04:40Z

src/Bundle/Resources/config/services/routing.xml

@@ -13,14 +13,21 @@

 <container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
    <services>
-        <defaults public="true" />
+        <defaults public="false" />


Keeping it public would make it more coherent with Sylius itself

lchrusciel · 2021-12-10T16:05:02Z

src/Bundle/Resources/config/services/routing.xml

            <argument type="service" id="sylius.resource_registry" />
            <argument type="service">
                <service class="Sylius\Bundle\ResourceBundle\Routing\RouteFactory" />
            </argument>
            <tag name="routing.loader" />
        </service>
+
+        <service id="sylius.routing.loader.attributes" class="Sylius\Bundle\ResourceBundle\Routing\AttributesLoader">
+            <!-- TODO make it configurable -->


What do you mean?

I forgot to remove the TODO :)
That's done.

I just have to remove the TODO.

done for this one.

src/Bundle/Routing/AttributesLoader.php

lchrusciel · 2021-12-10T20:42:43Z

src/Bundle/Routing/AttributesLoader.php

+                $syliusOptions['serialization_version'] = $arguments['serializationVersion'];
+            }
+
+            $route = new Route(


Probably \Sylius\Bundle\ResourceBundle\Routing\RouteFactory should be used. But I'm even thinking about createRoute method extraction from \Sylius\Bundle\ResourceBundle\Routing\ResourceLoader or making it public, so it could be here. Won't it make sense?

src/Bundle/Routing/AttributesLoader.php

lchrusciel · 2021-12-10T20:50:45Z

src/Bundle/test/config/routes/test_with_attributes/sylius_resource.yaml

+sylius_resource:
+    resource: 'sylius.routing.loader.attributes'
+    type: service


Do I understand i correctly, that without this configuration, attributes won't work?

Yes without this configuration, the routes would not be generated.
But It should be added on a Symfony flex recipe.

Shouldn't it be part of sylius_resource configuration tree?

That's not possible.
On ApiPlatform it's done on a recipe.
https://github.com/symfony/recipes/blob/master/api-platform/core/2.5/config/routes/api_platform.yaml

But we have to mention it on documentation.

src/Bundle/spec/Routing/AttributesLoaderSpec.php

src/Bundle/Routing/AttributesLoader.php

lchrusciel · 2021-12-10T20:59:03Z

src/Bundle/Routing/AttributesLoader.php

+
+    }
+
+    private function getResourcesByPath(string $path): array


This class or its parent seems to be a great candidate for extraction as well. This should be probably tested with spec/phpunit to check its finding behaviour. Right now, a lot is happening in this class alone. Is it fetched from some other project? Could it make sense, to reuse this logic somewhere else?

Yes it's extracted from here
https://github.com/symfony/symfony/blob/6.1/src/Symfony/Component/Validator/Command/DebugCommand.php#L172

It's my only one contribution on Symfony 👍

lchrusciel · 2021-12-10T21:01:00Z

really nice work!

lchrusciel · 2021-12-20T19:20:51Z

src/Bundle/Routing/CrudRoutesAttributesLoader.php

+        return $reflectionClass->getAttributes($attributeName);
+    }
+
+    private function getReflectionClasses(): iterable


I would extract this one to separate service/method as well, as it is 100% duplicated. But it is good to go for me as well

src/Component/Reflection/ClassReflection.php

Zales0123 · 2022-01-09T21:30:00Z

src/Bundle/test/config/routes/test_with_attributes/sylius_resource.yaml

@@ -0,0 +1,7 @@
+sylius_crud_routes:


As this configuration is required to make route attribute work, we should probably mention it in some docs or UPGRADE file (or both 💃)

I'd like to add it on a documentation PR. Cause it's not documented yet.

src/Bundle/spec/Routing/RoutesAttributesLoaderSpec.php

src/Bundle/Routing/RoutesAttributesLoader.php

src/Bundle/Tests/Routing/RoutesAttributesLoaderTest.php

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

Zales0123 · 2022-01-11T08:06:28Z

Thanks Loïc! Spectacular work 🎉 🚀

This PR was merged into the 1.9-dev branch

loic425 requested a review from a team as a code owner November 10, 2021 06:58

loic425 force-pushed the draft/sylius-routes-with-attributes branch 3 times, most recently from e2ba131 to c373b7f Compare November 10, 2021 14:24

loic425 changed the title ~~[WIP] Sylius route with attributes~~ Sylius route with attributes Nov 10, 2021

loic425 force-pushed the draft/sylius-routes-with-attributes branch 2 times, most recently from fba67b2 to cc1ae18 Compare November 16, 2021 09:49

lchrusciel reviewed Dec 10, 2021

View reviewed changes

loic425 force-pushed the draft/sylius-routes-with-attributes branch from b8ea7f6 to 43e42a5 Compare December 20, 2021 11:22

lchrusciel approved these changes Dec 20, 2021

View reviewed changes

Zales0123 approved these changes Jan 9, 2022

View reviewed changes

loic425 and others added 16 commits January 11, 2022 00:56

Sylius route with attributes

500410d

Add Single route with template and with repository

35dbac9

Add some missing feature for single routes

fb80bcf

Add some other missing features for single routes

23e4eae

Add Phpspec tests

09075ca

Add missing headers

23f6301

Make mapping paths configurable

66f8512

Skip priority test on Symfony 4

05ee060

Apply suggestions from code review

8246dee

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

Separate boths routing loaders

65c96aa

Fix Psalm and PHPStan

a007770

Remove testing PHP greater than 8.0

4bbaf6c

Fix Phpspec tests

954e199

Fix Phpspec tests for Symfony 4.4

77bff28

Add Unit tests for Class Reflection

9b061fe

Skipped generating routes from resource with priority on Symfony < 5

0696b54

loic425 force-pushed the draft/sylius-routes-with-attributes branch from 04f5653 to 732f512 Compare January 10, 2022 23:57

Add route attributes factory

6c8092b

loic425 force-pushed the draft/sylius-routes-with-attributes branch from 58994e1 to 6c8092b Compare January 11, 2022 00:18

Zales0123 added the Feature New feature proposals. label Jan 11, 2022

Zales0123 approved these changes Jan 11, 2022

View reviewed changes

Zales0123 merged commit 7ddf718 into Sylius:master Jan 11, 2022

loic425 deleted the draft/sylius-routes-with-attributes branch January 11, 2022 08:11

Zales0123 added a commit that referenced this pull request Jan 17, 2022

feature #334 Symfony workflow (loic425)

b87c88d

This PR was merged into the 1.9-dev branch

Zales0123 added a commit to Zales0123/SyliusResourceBundle that referenced this pull request Jan 18, 2022

feature Sylius#334 Symfony workflow (loic425)

ac82b4f

This PR was merged into the 1.9-dev branch

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sylius route with attributes #334

Sylius route with attributes #334

loic425 commented Nov 10, 2021 •

edited

Loading

lchrusciel Dec 10, 2021

loic425 Jan 3, 2022

lchrusciel Dec 10, 2021

loic425 Dec 10, 2021

loic425 Dec 10, 2021

loic425 Jan 3, 2022

lchrusciel Dec 10, 2021

lchrusciel Dec 10, 2021

loic425 Dec 10, 2021

lchrusciel Dec 11, 2021

loic425 Jan 3, 2022 •

edited

Loading

lchrusciel Dec 10, 2021

loic425 Dec 10, 2021

lchrusciel commented Dec 10, 2021

lchrusciel Dec 20, 2021

Zales0123 Jan 9, 2022

loic425 Jan 11, 2022

Zales0123 commented Jan 11, 2022

Sylius route with attributes #334

Sylius route with attributes #334

Conversation

loic425 commented Nov 10, 2021 • edited Loading

Crud routes

Single routes

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 Jan 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lchrusciel commented Dec 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zales0123 commented Jan 11, 2022

loic425 commented Nov 10, 2021 •

edited

Loading

loic425 Jan 3, 2022 •

edited

Loading