Routes tips - Githubissues

As you maybe already know, you can use Route::view instead of Route::get to only use a function or a controller action to return a view without any logic on it.

So, this route: https://github.com/MorganGonzales/user-crud/blob/5bbe15b0f92175752a95775b0fb8b6d7611a4a8f/routes/web.php#L16-L18

Could change to:

Route::view('/', 'welcome');

Again, I know that maybe by default it was like that, but you can still change it !

You can do this change (also) to:

https://github.com/MorganGonzales/user-crud/blob/5bbe15b0f92175752a95775b0fb8b6d7611a4a8f/routes/web.php#L22

And thanks to that, you would be able to remove a useless controller, hence simpler code!

Route::view('/home', 'home')->name('home');

Avoid defining `middlewares` in your Controller !

Please, always avoid doing this:

https://github.com/MorganGonzales/user-crud/blob/5bbe15b0f92175752a95775b0fb8b6d7611a4a8f/app/Http/Controllers/HomeController.php#L16

The controller may require someone to be logged in, but that is not the controller responsibility to "check" if the user is logged in, think like a request chain, if you allow a user to browse a defined route, then the route definition has the responsibility to check whatever it needs before going to the controller. The Router should check if the user is logged in to access a certain controller, is like having a bell in your door, it is outside your property (mostly) and if you know who it is, you can allow it to go inside, else it will never ever put a foot inside your property, because to access it, he/she must be known to you !

And, based on the next tip, you will see that the other controller has a __contructor but is not defining the middlewares in the constructor, but in other place, so again, super confusing !!!!

https://github.com/MorganGonzales/user-crud/blob/5bbe15b0f92175752a95775b0fb8b6d7611a4a8f/app/Http/Controllers/UserController.php#L13-L16

Use `Route::group` to prevent previous problems

So, to avoid the previous mentioned problems, ALWAYS use Route::group to tell the routes to use a defined middleware.

So your routes should be like:

Route::group(['middleware' => 'auth'], function () {
    Route::get('/home', [App\Http\Controllers\HomeController::class, 'index'])->name('home');
    Route::softDeletes('users', \App\Http\Controllers\UserController::class);
    Route::resource('users', \App\Http\Controllers\UserController::class);
});

Route::middleware('auth')->group(function () {
    Route::get('/home', [App\Http\Controllers\HomeController::class, 'index'])->name('home');
    Route::softDeletes('users', \App\Http\Controllers\UserController::class);
    Route::resource('users', \App\Http\Controllers\UserController::class);
});

Do not define "special" method routes for simple CRUD operations if there are not enough

I saw this code and I was thinking "is that new in Laravel 8 and I don't know it ? Is it a package ?". After digging a little more, I saw it is in RouteServiceProvider.php@boot.

My recommendation is to never do this, even if you have 20 models, and 5 of them do this. You can easily forget what this defines and if you later decide to change this but you desired to do it only in one part (because of the URL, you think it is better), you cannot accomplish this, as it will change for all routes using this...

https://github.com/MorganGonzales/user-crud/blob/5bbe15b0f92175752a95775b0fb8b6d7611a4a8f/routes/web.php#L23

How to fix previous mentioned issue

You should have a Route::resource and then 2 more Route methods that will satisfy your implementation !

Route::resource('users', \App\Http\Controllers\UserController::class);
Route::get('users/trashed', [\App\Http\Controllers\UserController::class, 'trashed'])->name('users.trashed');
Route::patch('users/{user}/restore', [\App\Http\Controllers\UserController::class, 'restore'])->name('users.restore');
Route::delete('users/{user}/delete', [\App\Http\Controllers\UserController::class, 'delete'])->name('users.delete');

Route::resource('users', \App\Http\Controllers\UserController::class);
Route::group(['prefix' => 'users', 'middleware' => 'auth'], function () {
    Route::get('trashed', [\App\Http\Controllers\UserController::class, 'trashed'])->name('users.trashed');
    Route::patch('{user}/restore', [\App\Http\Controllers\UserController::class, 'restore'])->name('users.restore');
    Route::delete('{user}/delete', [\App\Http\Controllers\UserController::class, 'delete'])->name('users.delete');
});

Remember to remove the Route::macro('softDeletes'). https://github.com/MorganGonzales/user-crud/blob/5bbe15b0f92175752a95775b0fb8b6d7611a4a8f/app/Providers/RouteServiceProvider.php#L51-L57

Another CRUD naming tip

I saw that you have defined {user}/delete as of forceDelete the data (not softDelete but really delete), but you also has {user} with DELETE action and this one is softDelete. To not confuse anyone, I recommend you to have the normal delete {user} with DELETE action to softDelete and if you want to forceDelete, instead of having {user}/delete have {user}/destroy with DELETE action, that way is more consistent.

Think it this way, if you delete something (knowing it has softDelete) it is like a "lite" delete, but if you destroy something, is like it will really never come back, that is why is destroy.

MorganGonzales / user-crud

Routes tips #4

Avoid defining `middlewares` in your Controller !

Use `Route::group` to prevent previous problems

Do not define "special" method routes for simple CRUD operations if there are not enough

How to fix previous mentioned issue

Another CRUD naming tip

MorganGonzales / user-crud

Routes tips #4

Avoid defining middlewares in your Controller !

Use Route::group to prevent previous problems

Do not define "special" method routes for simple CRUD operations if there are not enough

How to fix previous mentioned issue

Another CRUD naming tip

Avoid defining `middlewares` in your Controller !

Use `Route::group` to prevent previous problems