Courses

Laravel API Code Review and Refactor

Static String Letters to PHP Enum

The next thing I suggest fixing is hard-coded letters as Order statuses.

I found these 'P', 'F', and 'C' in the Factory:

database/factories/OrderFactory.php

class OrderFactory extends Factory
{
public function definition(): array
{
return [
'user_id' => User::factory(),
'name' => $this->faker->words(3, true),
'description' => $this->faker->paragraph(),
'date' => $this->faker->date,
'status' => $this->faker->randomElement(['P', 'F', 'C']),
// F = fulfilled, P = pending, C = canceled
];
}
}

So, you need to write a comment to others, to explain what those letters mean?

But what if that other developer finds those letters NOT in the Factory but somewhere else?

Here are the fragments from two FormRequest classes:

app/Http/Requests/Api/V1/StoreOrderRequest.php

'data.attributes.status' => 'required|string|in:P,F,C',

app/Http/Requests/Api/V1/UpdateOrderRequest.php

'data.attributes.status' => 'required|string|in:P,F,C',

No comments here on what those letters mean?

Also, in the Tests:

tests/Feature/OrderCreateTest.php

public function test_create_order_successfully()
{
// Define payload for the new order
$orderData = [
'data' => [
'attributes' => [
'user_id' => $user->id,
'name' => 'Test Order',
'description' => 'Test Order Description',
'status' => 'P',
'date' => now()->toDateString(),
],
],
];

What's that 'P', again?

A better way is to introduce a PHP Enum class.


Generate Enum

So, we run the Terminal command:

php artisan make:enum Enums/OrderStatus --string

Inside, we can leave the same three letters as values but choose the meaningful names as the constants.

app/Enums/OrderStatus.php:

namespace App\Enums;
 
enum OrderStatus: string
{
case FULFILLED = 'F';
case PENDING = 'P';
case CANCELED = 'C';
}

Then, let's introduce a Cast to the Model:

app/Models/Order.php:

use App\Enums\OrderStatus;
 
// ...
 
class Order extends Model
{
// ...
 
protected function casts(): array
{
return [
'status' => OrderStatus::class,
];
}

Then, the Factory would look like this:

database/factories/OrderFactory.php

public function definition(): array
{
return [
// ...
 
'status' => $this->faker->randomElement(['P', 'F', 'C']),
'status' => $this->faker->randomElement(OrderStatus::cases()),
];
}

Finally, the change in the validation rules of Form Requests:

app/Http/Requests/Api/V1/StoreOrderRequest.php

use App\Enums\OrderStatus;
use Illuminate\Validation\Rule;
 
class StoreOrderRequest extends BaseOrderRequest
{
public function rules(): array
{
return [
// ...
 
'data.attributes.status' => 'required|string|in:P,F,C',
'data.attributes.status' => ['required', 'string', Rule::enum(OrderStatus::class)],
 
// ...
];
}
}

app/Http/Requests/Api/V1/UpdateOrderRequest.php

use App\Enums\OrderStatus;
use Illuminate\Validation\Rule;
 
class UpdateOrderRequest extends BaseOrderRequest
{
public function rules(): array
{
return [
// ...
 
'data.attributes.status' => 'required|string|in:P,F,C',
'data.attributes.status' => ['required', 'string', Rule::enum(OrderStatus::class)],
 
// ...
];
}
}

Notice: In the Form Requests, we have to switch from the | separator-based rules list to the array because there is no appropriate alternative syntax for the Rule::enum() function.

Finally, in the Feature Test classes, we make this replacement in multiple methods:

tests/Feature/OrderCreateTest.php

use App\Enums\OrderStatus;
 
// ...
 
$orderData = [
'data' => [
'attributes' => [
'user_id' => $user->id,
'name' => 'Test Order',
'description' => 'Test Order Description',
'status' => 'P',
'status' => OrderStatus::PENDING->value,
'date' => now()->toDateString(),
],
],
];

Automated Test to Check The Refactoring

We introduced a pretty significant change, and the codebase didn't have the automated test to check this. Luckily, this one is pretty simple to write.

Let's check what happens if we pass the status ABC to the create order endpoint.

tests/Feature/OrderCreateTest.php

public function test_create_order_fails_with_incorrect_status()
{
$user = User::factory()->create();
 
// Create a product with sufficient stock
$product = Product::factory()->create(['stock' => 10]);
 
// Define payload for the new order
$orderData = [
'data' => [
'attributes' => [
'user_id' => $user->id,
'name' => 'Test Order',
'description' => 'Test Order Description',
'status' => 'ABC', // status not from the OrderStatus Enum
'date' => now()->toDateString(),
],
'relationships' => [
'products' => [
['id' => $product->id, 'quantity' => 5, 'price' => 100],
],
],
],
];
 
// Send a POST request to create the order
$response = $this
->actingAs($user)
->postJson('/api/v1/orders', $orderData);
 
$response->assertStatus(422);
}

Do we have this new test passing? Yes, we do!


Alternative Approaches

We could also make the database values clearer by saving those fulfilled values instead of just one letter, but this may break earlier orders with the front-end. That would be a breaking change in API. In this refactoring, I'm trying not to introduce breaking changes.

Another alternative, of course, is to introduce a DB table order_statuses with a foreign key column order_status_id, but that would also be a breaking change.

Here's the GitHub commit for what we've done in this lesson.

Previous: Route Model Binding

No comments yet…

avatar
You can use Markdown