Using Gates Properly: Remove isAble() Method

The next thing I don't like is a specific ->isAble() method instead of using Laravel default Gates/Policies:

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

public function show(Order $order)
{
    $this->isAble('view', $order); // policy
 
    // ...
}
 
public function update(UpdateOrderRequest $request, Order $order)
{
    try {
        $this->isAble('update', $order);
 
        // ...

This method comes from the base ApiController and looks like this:

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

class ApiController extends Controller
{
    protected $policyClass;
 
    public function isAble($ability, $model)
    {
        return Gate::authorize($ability, [$model, $this->policyClass]);
    }

I've been honestly trying to understand the purpose of this extra layer. Maybe I'm wrong here, but I don't see its benefit over just calling Gate::authorize() directly from Controllers.

So, this is my refactored "Laravel way" version.

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

use Illuminate\Support\Facades\Gate;
 
class OrderController extends ApiController
{
    protected $policyClass = OrderPolicy::class; 
 
    // ...
 
    public function index(OrderFilter $filter)
    {
        Gate::authorize('view-any', Order::class); 
 
        // ...
    }
 
    public function show(Order $order)
    {
        $this->isAble('view', $order); 
        Gate::authorize('view', $order); 
 
        // ...
    }
 
    public function store(StoreOrderRequest $request)
    {
        Gate::authorize('create', Order::class); 
 
        // ...
    }
 
    public function update(UpdateOrderRequest $request, Order $order)
    {
        Gate::authorize('update', $order); 
 
        try {
            $this->isAble('update', $order); 
 
            // ...
    }
 
    public function destroy(Order $order)
    {
        $this->isAble('delete', $order); 
        Gate::authorize('delete', $order); 
 
        // ...
    }
}

I also made two extra changes along the way:

• Added Gate::authorize() to the index() and store() methods
• In the update() method, Gate::authorize() should come before the main try-catch block, outside of it

Good. Now, we don't use the ->isAble() method in the Controller. We could delete it, but it's used in another OwnersOrdersController, so we'll get to that later while refactoring that Controller.

Problem with "view-any" in the index()

I've used a typical Policy method called view-any for the list of Orders.

And the Policy does exist. It's registered in the AppServiceProvider:

app/Providers/AppServiceProvider.php:

use App\Models\Order;
use App\Policies\V1\OrderPolicy;
 
// ...
 
public function boot(): void
{
    Gate::policy(Order::class, OrderPolicy::class);
}

However, the problem is that the Policy method is hard-coded to accept only the user with ID 1:

app/Policies/V1/OrderPolicy.php:

class OrderPolicy
{
    /**
     * Determine whether the user can view any models.
     */
    public function viewAny(User $authUser): bool
    {
        return $authUser->id === 1;
    }
 
    // ...

So, this was probably a "temporary solution" for the project's beginning, and I don't know what the correct condition should be. Maybe something like this?

return $authUser->tokenCan(Abilities::View_All_Orders);

I don't have the exact solution because I don't know the project's business logic. But since I'm doing a code review, I just wanted to flag this part as "forgotten".

The Worst Part: NO TESTS FOR PERMISSIONS??!!

While refactoring this code, I realized that this too-many-layers approach was left with quite a few bugs/inconsistencies, mostly because there were no tests to flag the issues.

In the previous lessons, we added a few tests for the 403 forbidden status code so we could test that our changes didn't break anything. The tests are still green.

Also, in general, I think it's an example of over-engineering: such a mix of Controler + BaseController (+ Trait!) + Gate + Policy + Abilities is very hard to debug. It takes time to dissect which layer is responsible for what. So, I'm a big believer in simplicity and sticking to framework standards, which I tried to introduce with my change.

Here's the GitHub commit and minor cleanup for this lesson.