Converting many separate route groups to one

I am working on some legacy code and I feel I can convert all the different route groups to one. This is the situation now: There is page that shows a link of links to some data tables. each link goes to the table's URL:

example.com/tables/table1
example.com/tables/table2
example.com/tables/table3

Each page of the tables has its own group of routes for the show and other actions, but is also surrounded by role Middleware, because each table can be accessed by different groups of users:

Route::prefix('tables')->group(function () {
    // table_1
    Route::middleware(['role:role_x'])->group(function () {
        Route::get('/table1', [TablesController::class, 'table1_show'])->name('name1_1')
        Route::patch('/table1', [TablesController::class, 'table1_patch'])->name('name1_2')
        Route::post('/table1', [TablesController::class, 'table1_post'])->name('name1_3')
        Route::get('/table1/sub_route', [TablesController::class, 'table1_sub_show'])->name('name1_4') 
    });

    // table_2
    Route::middleware(['role:role_y'])->group(function () {
        Route::get('/table2', [TablesController::class, 'table2_show'])->name('name2_1')
        Route::patch('/table2', [TablesController::class, 'table2_patch'])->name('name2_2')
        Route::post('/table2', [TablesController::class, 'table2_post'])->name('name2_3')
        Route::get('/table2/sub_route', [TablesController::class, 'table2_sub_show'])->name('name2_4') 
    });

    // and many more table route groups
});

I was thinking about converting all these similar groups to one, something like that:

Route::prefix('tables')->group(function () {        
        Route::get('/{table_name}', 
        Route::patch('/{table_name}', 
        Route::post('/{table_name}', 
        Route::get('/{table_name}/sub_route', 
});

And move the role Middleware to either the Controller or the Form Request authorization part.

But I am not sure that it's the correct way/syntax. Also currently each route explicitly points to specific Controller action and has its own name, because there are a few different things in the code currently, for example, each controller method uses the corresponding Model that belongs to the table name in the Controller. For that I was thinking to use the endpoint to fetch the corresponding Model / table that we take the data from instead of explicitly doing so in separate Controller action for each table.

Is it a good idea? Or there are some things that might not work this way?

vincent15000

3 years ago

Level 63

Yes it's a good idea.

You should better manage the roles middleware inside the controller (not in the form request).

About the model name you need to use (according to the route), perhaps you could write this in a configuration file and retrieve the model name from the table name ?

<?php

return [
	'models' => [
		'table1' => 'model_name_1',
		'table2' => 'model_name_2,
		'table3' => 'model_name_3',		
	]
]

You don't really give detailed information about the names, so it's just an idea according to what I have understood.

1 like

Ligonsker

3 years ago

Level 8

@vincent15000 You are pretty much spot on, the Model names don't exactly match the table names so I'd need some conversion like you wrote. But each table has its own Model.

Now I am using this Middleware for role checking: ( https://laravel.com/docs/9.x/middleware#middleware-parameters )

<?php 
namespace App\Http\Middleware; 
use Closure; 
class EnsureUserHasRole
{
    public function handle($request, Closure $next, $role)
    {
        if (! $request->user()->hasRole($role)) {
            // Redirect...
        } 
        return $next($request);
    } 
}

Should I then just extract the piece of code to the Controller, for example in the table1_show action:

class TablesControllerextends Controller
{
    public function table1_show(Request $request)
    {
        if (! $request->user()->hasRole($role)) {
            // Redirect...
        } 
    // continue Controller logic
    }

Or maybe put it in a more general place? so that it checks all other actions?

Also, why you said not to do it in the authorize() method of a Form Request:

// form request
public function authorize()
{
return $this->user()->hasRole($role)
}

1 like

vincent15000

3 years ago

Level 63

@Ligonsker The purpose of a form request is to check what a user has filled in a form before sending the data to be stored in the database. The purpose of a form request is not to check a role. You have to block the access to the action before doing the action, that means to check the authorization in the controller.

1 like

Ligonsker

3 years ago

Level 8

@vincent15000 But, the Form Request happens before entering the Controller. And the authorize method triggers even before the rules, so it's basically the very first thing that is being checked when using a form request

Please or to participate in this conversation.