From e3c2d2fd8273ecb0c7a970142724ebfd3d8d38a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Thu, 7 May 2026 12:55:35 +0100 Subject: [PATCH] Fix duplicate category name validation (#364) ## Summary - validate category names against the existing per-user unique index - return validation errors for create/update duplicate races instead of 500s - cover create, update, soft-delete, cross-user, and DB race cases Fixes PHP-LARAVEL-1E Fixes PHP-LARAVEL-1F ## Tests - php artisan test --compact tests/Feature/Settings/CategoryTest.php - vendor/bin/pint --dirty --format agent - git diff --check --- .../Settings/CategoryController.php | 25 ++- .../Settings/StoreCategoryRequest.php | 8 +- .../Settings/UpdateCategoryRequest.php | 9 +- tests/Feature/Settings/CategoryTest.php | 161 ++++++++++++++++++ 4 files changed, 199 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/Settings/CategoryController.php b/app/Http/Controllers/Settings/CategoryController.php index 364311cf..bf6c58af 100644 --- a/app/Http/Controllers/Settings/CategoryController.php +++ b/app/Http/Controllers/Settings/CategoryController.php @@ -6,8 +6,10 @@ use App\Http\Controllers\Controller; use App\Http\Requests\Settings\StoreCategoryRequest; use App\Http\Requests\Settings\UpdateCategoryRequest; use App\Models\Category; +use Illuminate\Database\UniqueConstraintViolationException; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Http\RedirectResponse; +use Illuminate\Validation\ValidationException; use Inertia\Inertia; use Inertia\Response; @@ -35,7 +37,11 @@ class CategoryController extends Controller */ public function store(StoreCategoryRequest $request): RedirectResponse { - auth()->user()->categories()->create($request->validated()); + try { + auth()->user()->categories()->create($request->validated()); + } catch (UniqueConstraintViolationException $exception) { + $this->throwDuplicateCategoryNameValidationException($exception); + } return to_route('categories.index'); } @@ -47,7 +53,11 @@ class CategoryController extends Controller { $this->authorize('update', $category); - $category->update($request->validated()); + try { + $category->update($request->validated()); + } catch (UniqueConstraintViolationException $exception) { + $this->throwDuplicateCategoryNameValidationException($exception); + } return to_route('categories.index'); } @@ -63,4 +73,15 @@ class CategoryController extends Controller return to_route('categories.index'); } + + private function throwDuplicateCategoryNameValidationException(UniqueConstraintViolationException $exception): never + { + if (! str_contains($exception->getMessage(), 'categories_user_id_name_unique')) { + throw $exception; + } + + throw ValidationException::withMessages([ + 'name' => __('validation.unique', ['attribute' => 'name']), + ]); + } } diff --git a/app/Http/Requests/Settings/StoreCategoryRequest.php b/app/Http/Requests/Settings/StoreCategoryRequest.php index 40718f03..f87bb99e 100644 --- a/app/Http/Requests/Settings/StoreCategoryRequest.php +++ b/app/Http/Requests/Settings/StoreCategoryRequest.php @@ -42,7 +42,13 @@ class StoreCategoryRequest extends FormRequest public function rules(): array { return [ - 'name' => ['required', 'string', 'max:255'], + 'name' => [ + 'required', + 'string', + 'max:255', + Rule::unique('categories', 'name') + ->where('user_id', auth()->id()), + ], 'icon' => ['required', 'string'], 'color' => [ 'required', diff --git a/app/Http/Requests/Settings/UpdateCategoryRequest.php b/app/Http/Requests/Settings/UpdateCategoryRequest.php index c9369a36..0ecc21a5 100644 --- a/app/Http/Requests/Settings/UpdateCategoryRequest.php +++ b/app/Http/Requests/Settings/UpdateCategoryRequest.php @@ -42,7 +42,14 @@ class UpdateCategoryRequest extends FormRequest public function rules(): array { return [ - 'name' => ['required', 'string', 'max:255'], + 'name' => [ + 'required', + 'string', + 'max:255', + Rule::unique('categories', 'name') + ->where('user_id', auth()->id()) + ->ignore($this->route('category')), + ], 'icon' => ['required', 'string'], 'color' => [ 'required', diff --git a/tests/Feature/Settings/CategoryTest.php b/tests/Feature/Settings/CategoryTest.php index 2a84ad98..dc9739cd 100644 --- a/tests/Feature/Settings/CategoryTest.php +++ b/tests/Feature/Settings/CategoryTest.php @@ -3,10 +3,13 @@ use App\Actions\CreateDefaultCategories; use App\Enums\CategoryCashflowDirection; use App\Enums\CategoryType; +use App\Http\Controllers\Settings\CategoryController; +use App\Http\Requests\Settings\StoreCategoryRequest; use App\Models\Category; use App\Models\User; use Illuminate\Database\UniqueConstraintViolationException; use Illuminate\Support\Facades\DB; +use Illuminate\Validation\ValidationException; beforeEach(function () { config(['landing.hide_auth_buttons' => false]); @@ -80,6 +83,72 @@ test('category name is required', function () { $response->assertSessionHasErrors(['name']); }); +test('category names must be unique for each user when creating', function () { + $user = User::factory()->create(); + + Category::factory()->create([ + 'user_id' => $user->id, + 'name' => 'Healthcare', + ]); + + $response = $this->actingAs($user)->post(route('categories.store'), [ + 'name' => 'Healthcare', + 'icon' => 'Heart', + 'color' => 'pink', + 'type' => 'expense', + 'cashflow_direction' => 'hidden', + ]); + + $response->assertSessionHasErrors(['name']); + + expect($user->categories()->where('name', 'Healthcare')->count())->toBe(1); +}); + +test('different users can create categories with the same name', function () { + $user = User::factory()->create(); + $otherUser = User::factory()->create(); + + Category::factory()->create([ + 'user_id' => $otherUser->id, + 'name' => 'Healthcare', + ]); + + $response = $this->actingAs($user)->post(route('categories.store'), [ + 'name' => 'Healthcare', + 'icon' => 'Heart', + 'color' => 'pink', + 'type' => 'expense', + 'cashflow_direction' => 'hidden', + ]); + + $response->assertRedirect(route('categories.index')); + + $this->assertDatabaseHas('categories', [ + 'user_id' => $user->id, + 'name' => 'Healthcare', + ]); +}); + +test('category names from deleted categories remain reserved', function () { + $user = User::factory()->create(); + $category = Category::factory()->create([ + 'user_id' => $user->id, + 'name' => 'Healthcare', + ]); + + $category->delete(); + + $response = $this->actingAs($user)->post(route('categories.store'), [ + 'name' => 'Healthcare', + 'icon' => 'Heart', + 'color' => 'pink', + 'type' => 'expense', + 'cashflow_direction' => 'hidden', + ]); + + $response->assertSessionHasErrors(['name']); +}); + test('category icon is required', function () { $user = User::factory()->create(); @@ -171,6 +240,64 @@ test('authenticated users can update their own category', function () { ]); }); +test('users can keep their category name when updating', function () { + $user = User::factory()->create(); + $category = Category::factory()->create([ + 'user_id' => $user->id, + 'name' => 'Healthcare', + 'type' => 'expense', + ]); + + $response = $this->actingAs($user)->patch( + route('categories.update', $category), + [ + 'name' => 'Healthcare', + 'icon' => 'Heart', + 'color' => 'pink', + 'type' => 'expense', + 'cashflow_direction' => 'hidden', + ] + ); + + $response->assertRedirect(route('categories.index')); + + $this->assertDatabaseHas('categories', [ + 'id' => $category->id, + 'name' => 'Healthcare', + 'color' => 'pink', + ]); +}); + +test('category names must be unique for each user when updating', function () { + $user = User::factory()->create(); + $category = Category::factory()->create([ + 'user_id' => $user->id, + 'name' => 'Old Name', + ]); + Category::factory()->create([ + 'user_id' => $user->id, + 'name' => 'Healthcare', + ]); + + $response = $this->actingAs($user)->patch( + route('categories.update', $category), + [ + 'name' => 'Healthcare', + 'icon' => 'Heart', + 'color' => 'pink', + 'type' => 'expense', + 'cashflow_direction' => 'hidden', + ] + ); + + $response->assertSessionHasErrors(['name']); + + $this->assertDatabaseHas('categories', [ + 'id' => $category->id, + 'name' => 'Old Name', + ]); +}); + test('transfer categories can store a cashflow direction', function () { $user = User::factory()->create(); @@ -324,6 +451,40 @@ test('default categories are created without repeated category lookups', functio ->and($categorySelects)->toBe(1); }); +test('duplicate category name database errors become validation errors', function () { + $user = User::factory()->create(); + $controller = new CategoryController; + + $user->categories()->create([ + 'name' => 'Healthcare', + 'icon' => 'Heart', + 'color' => 'pink', + 'type' => 'expense', + ]); + + $request = Mockery::mock(StoreCategoryRequest::class); + $request->shouldReceive('validated')->once()->andReturn([ + 'name' => 'Healthcare', + 'icon' => 'Heart', + 'color' => 'pink', + 'type' => 'expense', + 'cashflow_direction' => 'hidden', + ]); + + $this->actingAs($user); + + $thrown = null; + + try { + $controller->store($request); + } catch (ValidationException $exception) { + $thrown = $exception; + } + + expect($thrown)->toBeInstanceOf(ValidationException::class); + expect($thrown->errors())->toHaveKey('name'); +}); + test('category names are unique per user', function () { $user = User::factory()->create();