[Routing] Allow to set name prefixes from the configuration #25178

sroze · 2017-11-27T17:19:57Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#19612
License	MIT
Doc PR	ø

This allows setting name prefixes to routes while importing them. Typically, we can then import multiple times a similar file. This was originally requested by 🎸 @chrisguitarguy in #19612

app:
    resource: ../controller/routing.yml

api:
    resource: ../controller/routing.yml
    name_prefix: api_
    prefix: /api

<?xml version="1.0" encoding="UTF-8" ?>
<routes xmlns="http://symfony.com/schema/routing"
        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:schemaLocation="http://symfony.com/schema/routing
        http://symfony.com/schema/routing/routing-1.0.xsd">

    <import resource="../controller/routing.xml" />
    <import resource="../controller/routing.xml" prefix="/api" name-prefix="api_" />

</routes>

stof · 2017-11-27T17:58:26Z

src/Symfony/Component/Routing/Tests/RouteCollectionTest.php

+    public function testAddNamePrefix()
+    {
+        $collection = new RouteCollection();
+        $collection->add('foo', $foo = new Route('/foo'));


please also add an existing api_foo route, to be sure that the prefixing does not delete this route due to overriding.

Good catch, added.

stof · 2017-11-27T18:00:01Z

Does the PHP DSL also need this feature, or is it already available in it ?

sroze · 2017-11-28T05:36:19Z

@stof it was't. I've added it to the trait.

sroze · 2017-11-30T15:57:01Z

Status: Needs review

fabpot · 2017-12-01T14:17:52Z

src/Symfony/Component/Routing/Loader/Configurator/Traits/RouteTrait.php

@@ -124,4 +124,18 @@

        return $this;
    }
+
+    /**
+     * Add a prefix to the name of all the routes within the collection.


fabpot · 2017-12-01T14:18:23Z

src/Symfony/Component/Routing/RouteCollection.php

@@ -153,6 +153,22 @@ public function addPrefix($prefix, array $defaults = array(), array $requirement
        }
    }

+    /**
+     * Add a prefix to the name of all the routes within in the collection.


fabpot · 2017-12-01T14:18:31Z

src/Symfony/Component/Routing/RouteCollection.php

+    /**
+     * Add a prefix to the name of all the routes within in the collection.
+     *
+     * @param string $prefix


Should be removed

fabpot · 2017-12-01T14:18:38Z

src/Symfony/Component/Routing/Loader/Configurator/Traits/RouteTrait.php

+    /**
+     * Add a prefix to the name of all the routes within the collection.
+     *
+     * @param string $prefix


Should be removed

chalasr · 2017-12-01T14:22:26Z

src/Symfony/Component/Routing/Loader/Configurator/Traits/RouteTrait.php

+     *
+     * @return $this
+     */
+    final public function addNamePrefix(string $prefix)


: self to fully get rid of the phpdoc?

Well, we still need the description I'd say, as it's meant to be used by developers

Getting rid of @return $this would be nice though.

but... on the trait I'm not sure if this makes sense... as (at least in my mind) self refers to the trait here, not the class useing the trait.

There is no such thing as referring to a trait. PHP merges traits into the class, so self will always point to the correct class.

I was talking conceptually, by reading this. But if you two believe it's better just to put self within the trait, I'm happy with it as I don't mind that much tbh. Changed 👍

sroze

Updated based on @fabpot's comments.

sroze · 2017-12-01T15:22:16Z

src/Symfony/Component/Routing/Loader/Configurator/Traits/RouteTrait.php

+     *
+     * @return $this
+     */
+    final public function addNamePrefix(string $prefix)


Well, we still need the description I'd say, as it's meant to be used by developers

Add the `addNamePrefix` method to the PHP trait

fabpot · 2017-12-02T16:31:57Z

Thank you @sroze.

@chrisguitarguy

…ation (sroze) This PR was squashed before being merged into the 4.1-dev branch (closes #25178). Discussion ---------- [Routing] Allow to set name prefixes from the configuration | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #19612 | License | MIT | Doc PR | ø This allows setting name prefixes to routes while importing them. Typically, we can then import multiple times a similar file. This was originally requested by 🎸 @chrisguitarguy in #19612 ```yaml app: resource: ../controller/routing.yml api: resource: ../controller/routing.yml name_prefix: api_ prefix: /api ``` ```xml <?xml version="1.0" encoding="UTF-8" ?> <routes xmlns="http://symfony.com/schema/routing" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/routing http://symfony.com/schema/routing/routing-1.0.xsd"> <import resource="../controller/routing.xml" /> <import resource="../controller/routing.xml" prefix="/api" name-prefix="api_" /> </routes> ``` Commits ------- 880d7e7 [Routing] Allow to set name prefixes from the configuration

…rekas) This PR was merged into the 4.1-dev branch. Discussion ---------- [Routing] Fix name-prefixing when using PHP DSL | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Fixes bad implem merged in #25178 Commits ------- 0053eee [Routing] Fix name-prefixing when using PHP DSL

carsonbot added Status: Needs Review Routing Feature labels Nov 27, 2017

Allow to set name prefixes from the configuration

e0a8690

sroze force-pushed the feature/19612-add-name-prefix-to-routes branch from 0875723 to e0a8690 Compare November 27, 2017 17:20

sroze mentioned this pull request Nov 27, 2017

[Routing] Route Name Prefixing #19612

Closed

stof added this to the 4.1 milestone Nov 27, 2017

carsonbot added Status: Needs Work and removed Status: Needs Review labels Nov 27, 2017

stof requested changes Nov 27, 2017

View reviewed changes

sroze force-pushed the feature/19612-add-name-prefix-to-routes branch 2 times, most recently from 841d5ee to a4c836c Compare November 28, 2017 05:14

carsonbot added Status: Needs Review and removed Status: Needs Work labels Nov 30, 2017

fabpot reviewed Dec 1, 2017

View reviewed changes

chalasr reviewed Dec 1, 2017

View reviewed changes

sroze force-pushed the feature/19612-add-name-prefix-to-routes branch from a4c836c to 9f7c4e7 Compare December 1, 2017 15:26

sroze commented Dec 1, 2017

View reviewed changes

sroze force-pushed the feature/19612-add-name-prefix-to-routes branch from 9f7c4e7 to dd15433 Compare December 1, 2017 16:01

Ensure routes are not overwritten if have the same name

e895402

Add the `addNamePrefix` method to the PHP trait

sroze force-pushed the feature/19612-add-name-prefix-to-routes branch from dd15433 to e895402 Compare December 1, 2017 16:02

carsonbot removed the Status: Needs Review label Dec 2, 2017

Simperfit approved these changes Dec 2, 2017

View reviewed changes

carsonbot added the Status: Reviewed label Dec 2, 2017

fabpot approved these changes Dec 2, 2017

View reviewed changes

fabpot closed this Dec 2, 2017

sroze deleted the feature/19612-add-name-prefix-to-routes branch December 2, 2017 17:38

javiereguiluz mentioned this pull request Jan 8, 2018

[Routing] Allow to set name prefixes from the configuration symfony/symfony-docs#9020

Closed

nicolas-grekas mentioned this pull request Mar 19, 2018

[Routing] Fix name-prefixing when using PHP DSL #26597

Merged

fabpot mentioned this pull request May 7, 2018

Release v4.1.0-BETA1 #27181

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Routing] Allow to set name prefixes from the configuration #25178

[Routing] Allow to set name prefixes from the configuration #25178

sroze commented Nov 27, 2017 •

edited

Loading

stof Nov 27, 2017

sroze Nov 28, 2017

stof commented Nov 27, 2017

sroze commented Nov 28, 2017

sroze commented Nov 30, 2017

fabpot Dec 1, 2017

fabpot Dec 1, 2017

fabpot Dec 1, 2017

fabpot Dec 1, 2017

chalasr Dec 1, 2017

sroze Dec 1, 2017

fabpot Dec 1, 2017

sroze Dec 1, 2017

fabpot Dec 1, 2017

sroze Dec 1, 2017

sroze left a comment

sroze Dec 1, 2017

fabpot commented Dec 2, 2017

[Routing] Allow to set name prefixes from the configuration #25178

[Routing] Allow to set name prefixes from the configuration #25178

Conversation

sroze commented Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stof commented Nov 27, 2017

sroze commented Nov 28, 2017

sroze commented Nov 30, 2017

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

sroze left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Dec 2, 2017

sroze commented Nov 27, 2017 •

edited

Loading