Skip to main content

"The Payoff": Eliminate Trait and One More Controller

Premium
7 min read

This will probably be the "payoff" lesson: we will be able to delete a lot of code that we refactored not to be used in the earlier lessons.

For that, let's look at other Controllers and what we can simplify/eliminate there.


Wait, What's that OwnerOrdersController?

I found a "suspicious" OwnerOrdersController that seems to almost duplicate the purpose of OrderController, just filtering by the order's user_id.

routes/api_v1.php:

Route::apiResource('orders', OrderController::class);
 
// ...
 
Route::apiResource('owners.orders', OwnerOrdersController::class);

My question was: do we really need that separate Controller?

That Route::apiResource('xxx.yyy') syntax is called Nested Resources and works well to build the endpoints like these:

  • GET owners/1/orders - get the list of orders by user_id 1
  • GET owners/2/orders/3 - get the order with ID 3 by user_id 2

But... user_id should come from the Authentication with auth()->id(). What's the purpose of creating a separate Controller?

This is my (a bit longer) sequence of thoughts. Hear me out.

I thought it might look logical if some administrator user wanted to get the orders by a specific user manually, without logging in as that user.

But looking at the contents of this Controller... it felt wrong to me.

app/Http/Controllers/Api/V1/OwnerOrdersController.php

class OwnerOrdersController extends ApiController
{
use AuthorizesRequests;
 
protected $policyClass = OwnerPolicy::class;
 
public function __construct(
protected OrdersService $orderService
) {}
 
/**
* Display a listing of the resource.
*/
public function index($ownerId, OrderFilter $filter)
{
try {
$this->authIsOwner($ownerId);
 
return response()->json(new OrderCollection(
Order::where('user_id', $ownerId)->filter($filter)->paginate()
), Response::HTTP_OK);
} catch (ModelNotFoundException $eModelNotFound) {
return $this->responseNotFound('User not found');
} catch (AuthorizationException $eAuthorizationException) {
return $this->responseNotAuthorized();
}
}
 
// ...

So much duplicated code with the original OrderController, just to additionally call $this->authIsOwner($ownerId) and where('user_id', $ownerId)?

At first, I started refactoring, but then I looked at what was inside that authIsOwner().

app/Http/Controllers/Api/V1/ApiController.php

class ApiController extends Controller
{
use AuthorizesRequests;
 
public function authIsOwner($ownerId)
{
$owner = User::findOrFail($ownerId);
$this->authorize('authIsOwner', $owner); // policy
}
}

Notice: here, $this->authorize() can be called instead of Gate::authorize() because of the use AuthorizesRequests trait added on top. It was added by default in Laravel 10, but since Laravel 11, it has to be added manually if you want to use this "older" syntax.

Next, what's inside that authIsOwner in the Policy?

app/Policies/V1/OwnerPolicy.php:

/**
* Determine whether the user is the Owner of the resources.
*/
public function authIsOwner(User $authUser, User $owner): bool
{
return $authUser->id === $owner->id;
}

So, wait a minute. We are querying the user just to check whether the logged-in user is the same as the owner?

This means we could just use auth()->id() for the user in the query.

So yeah, then I concluded that this Controller probably doesn't even work.

And, again, there were no automated tests to test it.

So, I completely...

The Full Lesson is Only for Premium Members

Want to access all of our courses? (30 h 09 min)

You also get:

55 courses
Premium tutorials
Access to repositories
Private Discord
Get Premium for $129/year or $29/month

Already a member? Login here

Comments & Discussion

No comments yet…

We'd Love Your Feedback

Tell us what you like or what we can improve

Feel free to share anything you like or dislike about this page or the platform in general.