Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RouteCollection::getRouteName #6776

Merged
merged 1 commit into from
Jan 25, 2021

Conversation

VincentLanglet
Copy link
Member

@VincentLanglet VincentLanglet commented Jan 15, 2021

Subject

I am targeting this branch, because BC.

Currently, I don't see any way to get the routeName of the postAdmin list.
It's computed as sprintf('%s_%s', $this->baseRouteName, $name); in RouteCollection::add and it's done the same way in the AbstractAdmin.

  • I would like to avoid writing the same thing in my code, I think the developer should rely in a method instead
  • In the Sonata code, it should be implemented only once

I'm not sure about the name of the method and where to put it.

Now you can do $this->admin->getRoutes()->getRouteName('show').
I would have prefer to do $this->admin->getRouteName('show') but then it would be harder to call it in RouteCollection.

Changelog

### Added
- Added `RouteCollection::getRouteName`
- Added `RouteCollectionInterface::getRouteName`

@VincentLanglet VincentLanglet changed the title Add AdminInterface::getRoute Add RouteCollection::getRouteName Jan 19, 2021
@VincentLanglet VincentLanglet requested a review from a team January 19, 2021 01:19
@VincentLanglet VincentLanglet marked this pull request as ready for review January 22, 2021 12:19
@VincentLanglet
Copy link
Member Author

Friendly ping @sonata-project/contributors :)

OskarStark
OskarStark previously approved these changes Jan 22, 2021
@VincentLanglet VincentLanglet requested a review from a team January 22, 2021 13:18
@franmomu
Copy link
Member

Should be added to RouteCollectionInterface?

@VincentLanglet
Copy link
Member Author

Should be added to RouteCollectionInterface?

Yes indeed, added.

@@ -123,6 +125,9 @@ public function getBaseControllerName();
*/
public function getBaseRouteName();

// NEXT_MAJOR: Uncomment the following line.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// NEXT_MAJOR: Uncomment the following line.
// NEXT_MAJOR: Uncomment the following line and remove corresponding @method annotation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@VincentLanglet
Copy link
Member Author

Please take a look @OskarStark :) (Or you can dismiss the review @greg0ire, since the change asked was made)

@greg0ire greg0ire dismissed OskarStark’s stale review January 25, 2021 10:40

Requested change was applied

@greg0ire greg0ire merged commit c5590a1 into sonata-project:3.x Jan 25, 2021
@greg0ire
Copy link
Contributor

Thanks @VincentLanglet!

@VincentLanglet VincentLanglet deleted the getRoute branch March 27, 2021 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants