One more Controller that looked "suspicious" to me was OrderOwnerController
.
And yes, the naming is confusing: there was OwnerOrdersController
(deleted by now) and OrderOwnerController
. Let's see what's inside.
OrderOwnerController: Do We REALLY Need This?
Here's the code of that Controller.
app/Http/Controllers/Api/V1/OrderOwnerController.php
namespace App\Http\Controllers\Api\V1; use App\Http\Filters\V1\OwnerFilter;use App\Http\Resources\V1\UserCollection;use App\Http\Resources\V1\UserResource;use App\Models\User;use Illuminate\Http\Response; class OrderOwnerController extends ApiController{ /** * Display a listing of the resource. */ public function index(OwnerFilter $filter) { return response()->json(new UserCollection( User::with('orders')->filter($filter)->paginate() ), Response::HTTP_OK); } /** * Display the specified resource. */ public function show(User $owner) { if ($this->include('orders')) { return response()->json(new UserResource($owner->load('orders')), Response::HTTP_OK); } return response()->json(new UserResource($owner), Response::HTTP_OK); }}
Ok, so it lists the Users with UserCollection
or UserResource
.
It doesn't do anything with orders specifically, except for including the relationship when needed.
So, my question was: why was it called Order Owner Controller?
I decided to go on a quest: force a breaking change in this API (author, sorry if you're reading this) and rename this to a proper clear UserController
.
Yes, it's a risky change, but I think the project is small enough for now and early enough to introduce this naming change.
Step 1. Rename OwnerFilter
class to UserFilter
This Controller uses a Filter class to process query parameters and filter the data. We don't need to know what's inside. We just need to rename the class and make this change:
app/Http/Controllers/Api/V1/OrderOwnerController.php
use App\Http\Filters\V1\OwnerFilter; use App\Http\Filters\V1\UserFilter; class OrderOwnerController extends ApiController{ /** * Display a listing of the resource. */ public function index(OwnerFilter $filter) public function index(UserFilter $filter) // ...
We can take a brief look at that UserFilter
class.
app/Http/Filters/V1/UserFilter.php
namespace App\Http\Filters\V1; class UserFilter extends QueryFilter{ protected $sortable = [ 'id', 'name', 'email', 'createdAt' => 'created_at', 'updatedAt' => 'updated_at', ]; public function name($value) { $like_str = str_replace('*', '%', $value); return $this->builder->where('name', 'like', $like_str); } public function email($value) { $like_str = str_replace('*', '%', $value); return $this->builder->where('email', 'like', $like_str); }}
Reviewing the structure of those Filters could be a separate, long topic, but in this case, I've chosen the strategy of "don't touch what works."
Potentially, I would probably refactor it to use a package like spatie/laravel-query-builder instead of reinventing the wheel with not-so-custom logic. But I decided NOT to do it in this course.
Step 2. Rename OrderOwnerController
to UserController
We need to change the name of the class and the filename and then change Routes to this:
routes/api_v1.php:
use App\Http\Controllers\Api\V1\OrderOwnerController; use App\Http\Controllers\Api\V1\UserController; // ... Route::apiResource('owners', OrderOwnerController::class); Route::apiResource('users', UserController::class);
And yes, I know changing the endpoint URL is a breaking change in the API. If the old URL is actually used, then, as a fallback, we may leave the old URL, too, pointing to the new Controller name.
Step 3. Change the routes used in the API Resources
I noticed that API Resources also mention the endpoints, so we need to change it there:
app/Http/Resources/V1/UserCollection.php
return [ 'data' => $this->collection, 'links' => [ 'self' => route('owners.index'), 'self' => route('users.index'), ],
And also here:
app/Http/Resources/V1/UserResource.php
'links' => [ 'self' => route('owners.show', ['owner' => $this->id]), 'self' => route('users.show', ['user' => $this->id]), ],
Step 4. Shorten UserController
to not return response()->json()
We had performed a similar refactoring for the OrderController
, so let's repeat it here:
app/Http/Controllers/Api/V1/UserController.php
public function index(UserFilter $filter){ return response()->json(new UserCollection( User::with('orders')->filter($filter)->paginate() ), Response::HTTP_OK); return new UserCollection( User::with('orders')->filter($filter)->paginate() ); } public function show(User $user){ if ($this->include('orders')) { $user->load('orders'); } return response()->json(new UserResource($owner), Response::HTTP_OK); return new UserResource($user); }
Step 5: TESTS!
Guess what: the codebase didn't have any tests for this OrderOwnerController
, now UserController
. So, let's write them, at least for the simple cases.
php artisan make:test UserTest
tests/Feature/UserTest.php
namespace Tests\Feature; use App\Models\User;use Illuminate\Foundation\Testing\RefreshDatabase;use Tests\TestCase; class UserTest extends TestCase{ use RefreshDatabase; public function test_shows_users_successfully() { $user = User::factory()->create(); $response = $this ->actingAs($user) ->getJson('/api/v1/users'); $response->assertStatus(200); } public function test_shows_single_user_successfully() { $user = User::factory()->create(); $response = $this ->actingAs($user) ->getJson('/api/v1/users/'.$user->id); $response->assertStatus(200); }}
Two more tests were added to our test suite, all green!
Here's the GitHub commit for this change.
No comments yet…