Skip to content

Conversation

TomasVotruba
Copy link
Member

No description provided.

@TomasVotruba TomasVotruba enabled auto-merge (squash) February 15, 2025 21:04
@TomasVotruba TomasVotruba merged commit 2eb7139 into main Feb 15, 2025
5 checks passed
@TomasVotruba TomasVotruba deleted the tv-inline-routing branch February 15, 2025 21:04
@trsteel88
Copy link
Contributor

@TomasVotruba this rule isn't carrying the "name" from the prefix. This is resulting in broken route names.

In the example below, there is a route called "app_core.admin.organisation.index" prior to the rule running. After the rule runs, it becomes "index".

For example:

#[Route(path: '/organisation', name: 'app_core.admin.organisation.')]
class OrganisationController extends AbstractController
{
    #[Route(name: 'index')]
    public function indexAction(): Response
    {
        ...
    }

Is being modified to:

class OrganisationController extends AbstractController
{
    #[Route(path: '/organisation', name: 'index')]
    public function indexAction(): Response
    {
        ...
    }

But it should become:

class OrganisationController extends AbstractController
{
    #[Route(path: '/organisation', name: 'app_core.admin.organisation.index')]
    public function indexAction(): Response
    {
        ...
    }

@TomasVotruba
Copy link
Member Author

Good point! Thanks

Could you add failing test fixture?

@trsteel88
Copy link
Contributor

Unfortunately I'm a bit swamped so I won't be able to. I'm not sure if it's been considered either but requirements, method etc and everything else will also need to be merged

@TomasVotruba
Copy link
Member Author

No wories 👍

cc @samsonasik Could you look into fix for this one?

@samsonasik
Copy link
Member

I will try

@samsonasik
Copy link
Member

@trsteel88 hm.., I can't reproduce it, I probably missing something or missing some dependency, could you create simple reproducible repository for it? without it, i am not sure I can try provide a fix for it, thank you.

@samsonasik
Copy link
Member

@trsteel88 ok, I can reproduce on annotation usage, it seems concat string is needed if name is available.

@samsonasik
Copy link
Member

I created PR for it :

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants