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.
No comments yet…