area17 / twill

Twill is an open source CMS toolkit for Laravel that helps developers rapidly create a custom admin console that is intuitive, powerful and flexible. Chat with us on Discord at https://discord.gg/cnWk7EFv8R.
https://twillcms.com
Apache License 2.0
3.75k stars 572 forks source link

[4.x] Prefix internal Twill users module routes to prevent conflicts with another User model #2583

Open Tofandel opened 4 months ago

Tofandel commented 4 months ago

Given the following twill route

\Illuminate\Support\Facades\Route::prefix('app')->group(function () {
    TwillRoutes::module('users');
});

And the following navigation

        TwillNavigation::addLink(
            NavigationLink::make()
                ->title('Users')
                ->forModule('users')
                ->setChildren([
                    NavigationLink::make()->title('Professions')->forModule('userProfessions'),
                ])
        )

Until I run php artisan route:cache the link in the menu is correct After the cache, the link goes to /users without the group prefix

image image

Setting ->forModule('app.users') (the route being listed as twill.app.users.index results in image

Which is not a descriptive error message

Setting ->forRoute('twill.app.users.index') gives the desired result but it's weird to have the behavior differ between route cached or not (routes are only cached when staging so it's not caught straight away)

Tofandel commented 4 months ago

Very likely due to $this->addToRouteRegistry($slug, $customRoutePrefix); which is a side effect in the route registration

My biggest problem is the fact that the twill users management is registered as the users module which is very easy to conflict with user being a very common model name, maybe we should have the default user route under a prefix to avoid those issues (but would have to be v4 likely or maybe in v3 it could be enabled with an option)