thedevdojo / voyager

Voyager - The Missing Laravel Admin
https://voyager.devdojo.com
MIT License
11.77k stars 2.67k forks source link

BRED actions doesn't work for freshly created BREAD item #4598

Closed valkedev closed 4 years ago

valkedev commented 4 years ago

Laravel: 6.6.0 Voyager: 1.3.0 PHP: 7.3.6

Server startet by artisan serve image

Made a clear install of Laravel and then Voyager; Created a table by migration; image Created BREAD for the table; Added items to newly created BREAD object; Other actions on that object delivers 404 image

MrCrayon commented 4 years ago

Can you try to change slug in your BREAD and use something like "tests" to see if it's a problem with characters?

valkedev commented 4 years ago

Can you try to change slug in your BREAD and use something like "tests" to see if it's a problem with characters? Yes. After changing slug to 'test' link to manage object doesn't even work image

MrCrayon commented 4 years ago

I think you'll need to add permissions manually after changing slug. Easier if you create another BREAD probably.

valkedev commented 4 years ago

I think you'll need to add permissions manually after changing slug. Easier if you create another BREAD probably.

But thing is i have a local domain on cyrillic...

MrCrayon commented 4 years ago

I meant easier for testing not as final solution. Problem might not be that but we need to rule it out.

valkedev commented 4 years ago

So i created a new BREAD without any cyrillic. It works. But same thing after changing slug - i can't access objects via menu. What do you mean about the "final solution"?

valkedev commented 4 years ago

also i deleted categories and created new one and i have this image

MrCrayon commented 4 years ago

When you create a BREAD permissions can be automatically created, when you change slug you need to change/create them manually in permissions table, as far as I remember.

When you delete a BREAD you need to remove menu item manually.

MrCrayon commented 4 years ago

The problem is in how Route::resource(), that is used by Voyager to automatically creates routes, generate routes:

|        | GET|HEAD  | admin/категории/{категории}/edit             | voyager.категории.edit

The problematic part is {категории}, I think Laravel has a restriction on which kind of characters you can use there.

If you try to change your route file like this:

Route::group(['prefix' => 'admin'], function () {
    Voyager::routes();

    Route::group(['middleware' => ['admin.user']], function () {
        Route::group(['prefix' => 'категории'], function () {
            Route::get('/{kategorii}/edit', '\TCG\Voyager\Http\Controllers\VoyagerBaseController@edit')->name('voyager.категории.edit');
        });
    });
});

That should work for edit.

Solution Recreate your routes, check how Voyager does it in: https://github.com/the-control-group/voyager/blob/1.3/routes/voyager.php Don't use Route::resource() but create those one like the others and just use {id}.

To be honest since Voyager is an Admin tool I don't see the need for having slugs translated but that's your choice, also I have no idea if you might have problems elsewhere.

valkedev commented 4 years ago

The problem is in how Route::resource(), that is used by Voyager to automatically creates routes, generate routes:

|        | GET|HEAD  | admin/категории/{категории}/edit             | voyager.категории.edit

The problematic part is {категории}, I think Laravel has a restriction on which kind of characters you can use there.

If you try to change your route file like this:

Route::group(['prefix' => 'admin'], function () {
    Voyager::routes();

    Route::group(['middleware' => ['admin.user']], function () {
        Route::group(['prefix' => 'категории'], function () {
            Route::get('/{kategorii}/edit', '\TCG\Voyager\Http\Controllers\VoyagerBaseController@edit')->name('voyager.категории.edit');
        });
    });
});

That should work for edit.

Solution Recreate your routes, check how Voyager does it in: https://github.com/the-control-group/voyager/blob/1.3/routes/voyager.php Don't use Route::resource() but create those one like the others and just use {id}.

To be honest since Voyager is an Admin tool I don't see the need for having slugs translated but that's your choice, also I have no idea if you might have problems elsewhere.

i'll try, thank you. I don't like cyrillic routes but domain is http://оптомед.рф so it would be not so nice to have something like http://оптомед.рф/categories/blablabla The domain is not my choice unfortunately

MrCrayon commented 4 years ago

You are still going to have /edit, at the end but I guess you could change that too. Let us know if it works.

valkedev commented 4 years ago

Temporary solution for major routes. Gonna bring russian community to Laravel to create (sorry, Add) an issue.

Route::group(['prefix' => 'admin'], function () {
    Voyager::routes();

    $translations = [
        'каталог' => 'category',
        'продукт' => 'product'
    ];

    $actions = [
        'edit' => 'get',
        'show' => 'get',
        'destroy' => 'delete',
        'update' => 'put',
    ];

    foreach($translations as $cyr => $lat) {
        foreach($actions as $action => $method) {
            Route::group(['middleware' => ['admin.user']], function () use ($cyr, $lat, $action, $method) {
                Route::group(['prefix' => $cyr], function () use ($cyr, $lat, $action, $method) {
                    Route::{$method}('/{'.$lat.'}/'.$action, '\TCG\Voyager\Http\Controllers\VoyagerBaseController@'.$action)->name('voyager.'.$cyr.'.'.$action);
                });
            });
        }
    }
});
MrCrayon commented 4 years ago

@falke2 hope you don't mind some advice: If you want to respect Laravel standard use plural form for slugs. I would change the order like this and also append action only where needed:

    Route::group(['middleware' => ['admin.user']], function () {
        foreach ($translations as $cyr => $lat) {
            Route::group(['prefix' => $cyr], function () use ($cyr, $lat) {
                foreach ($actions as $action => $method) {
                    $append = $action == 'edit' ? '/'.$action : '';

                    Route::{$method}('/{'.$lat.'}'.$append, '\TCG\Voyager\Http\Controllers\VoyagerBaseController@'.$action)->name('voyager.'.$cyr.'.'.$action);
                }
            });
        }
    });

And finally you can try to use:

$append = $action == 'edit' ? '/'. strtolower(__('voyager::generic.'.$action)) : '';
MrCrayon commented 4 years ago

So I was poking around and turns out it's not even Laravel problem it goes back to Symfony: https://github.com/symfony/symfony/blob/324272111c689eb44e216d98556a1ceda1ddda2d/src/Symfony/Component/Routing/RouteCompiler.php#L114

Adding regex u option does it for me.

valkedev commented 4 years ago

So I was poking around and turns out it's not even Laravel problem it goes back to Symfony: https://github.com/symfony/symfony/blob/324272111c689eb44e216d98556a1ceda1ddda2d/src/Symfony/Component/Routing/RouteCompiler.php#L114

Adding regex u option does it for me.

Awesome ty!

valkedev commented 4 years ago

Met another problem image it's caused because of raw url encoding and reaching 32 characters limit: https://github.com/symfony/symfony/issues/8933

Looks like it's not Symphony but Laravel issue. Laravel Route class doesn't contain $options property that Symphony's has. This option used to provide custom class to compile route and forced using of utf8...

MrCrayon commented 4 years ago

Honestly I don't know why Laravel Route:resource('resource_name') creates routes like this: /resource_name/{resource_name}/edit instead of /resource_name/{id}/edit

MrCrayon commented 4 years ago

Actually Route::resource accept options, you can try to foreach this:

    Route::group(['middleware' => ['admin.user']], function () {
        Route::group(['as' => 'voyager.'], function () {
            Route::resource($cyr, '\TCG\Voyager\Http\Controllers\VoyagerBaseController', ['parameters' => [$cyr => 'id']]);
        });
    });

We should probably change it in Voyager...

valkedev commented 4 years ago

Actually Route::resource accept options, you can try to foreach this:

    Route::group(['middleware' => ['admin.user']], function () {
        Route::group(['as' => 'voyager.'], function () {
            Route::resource($cyr, '\TCG\Voyager\Http\Controllers\VoyagerBaseController', ['parameters' => [$cyr => 'id']]);
        });
    });

We should probably change it in Voyager...

I think so because localized domain names are kinda popular. It's common to see english written urls but not when your domain is written on your native language.

An i completely forgot my frontend gonna be written on Nuxt (Vue) lol. Utf8 routing works fine there out of the box

MrCrayon commented 4 years ago

No warrany it will be merged but there #4605

For /create and /edit translations you can put this in your ServiceProvider boot method:

        Route::resourceVerbs([
            'create' => strtolower(__('voyager::generic.add')),
            'edit' => strtolower(__('voyager::generic.edit')),
        ]);
github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. If you have further questions please ask in our Slack group.