From 7afd7da2b44820dc7da4acd6b110299ce3b9368d Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 21 May 2025 11:44:54 -0500 Subject: [PATCH 1/4] initial work, more testing/tests needed --- .../Controllers/Api/DepartmentsController.php | 3 +- app/Http/Requests/StoreDepartmentRequest.php | 32 +++++++++++++++++++ .../Departments/Api/CreateDepartmentsTest.php | 12 +++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 app/Http/Requests/StoreDepartmentRequest.php diff --git a/app/Http/Controllers/Api/DepartmentsController.php b/app/Http/Controllers/Api/DepartmentsController.php index 167d5cbc2a..ec8a605caf 100644 --- a/app/Http/Controllers/Api/DepartmentsController.php +++ b/app/Http/Controllers/Api/DepartmentsController.php @@ -4,6 +4,7 @@ namespace App\Http\Controllers\Api; use App\Helpers\Helper; use App\Http\Controllers\Controller; +use App\Http\Requests\StoreDepartmentRequest; use App\Http\Transformers\DepartmentsTransformer; use App\Http\Transformers\SelectlistTransformer; use App\Models\Department; @@ -94,7 +95,7 @@ class DepartmentsController extends Controller * @since [v4.0] * @param \App\Http\Requests\ImageUploadRequest $request */ - public function store(ImageUploadRequest $request) : JsonResponse + public function store(StoreDepartmentRequest $request): JsonResponse { $this->authorize('create', Department::class); $department = new Department; diff --git a/app/Http/Requests/StoreDepartmentRequest.php b/app/Http/Requests/StoreDepartmentRequest.php new file mode 100644 index 0000000000..3b94e19326 --- /dev/null +++ b/app/Http/Requests/StoreDepartmentRequest.php @@ -0,0 +1,32 @@ +|string> + */ + public function rules(): array + { + $modelRules = (new Department)->getRules(); + + return array_merge( + $modelRules, + ); + } +} diff --git a/tests/Feature/Departments/Api/CreateDepartmentsTest.php b/tests/Feature/Departments/Api/CreateDepartmentsTest.php index e0f975dd7f..4b887bfc3d 100644 --- a/tests/Feature/Departments/Api/CreateDepartmentsTest.php +++ b/tests/Feature/Departments/Api/CreateDepartmentsTest.php @@ -3,6 +3,7 @@ namespace Tests\Feature\Departments\Api; use App\Models\AssetModel; +use App\Models\Company; use App\Models\Department; use App\Models\Category; use App\Models\User; @@ -39,4 +40,15 @@ class CreateDepartmentsTest extends TestCase $this->assertEquals('Test Note', $department->notes); } + public function testNoArraysAllowed() + { + $response = $this->actingAsForApi(User::factory()->superuser()->create()) + ->postJson(route('api.departments.store'), [ + 'name' => 'Test Department', + 'company_id' => [1, 2], + ]) + ->assertOk() + ->assertStatusMessageIs('error'); + } + } From 7571ff007f6a870543d0b554e8839c922333455c Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 21 May 2025 12:51:21 -0500 Subject: [PATCH 2/4] add fillable properties to rules, some tests, move authorization to request --- .../Controllers/Api/DepartmentsController.php | 3 +- app/Models/Department.php | 11 ++-- .../Departments/Api/CreateDepartmentsTest.php | 53 ++++++++++++++++--- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/app/Http/Controllers/Api/DepartmentsController.php b/app/Http/Controllers/Api/DepartmentsController.php index ec8a605caf..adbe4bc35c 100644 --- a/app/Http/Controllers/Api/DepartmentsController.php +++ b/app/Http/Controllers/Api/DepartmentsController.php @@ -97,9 +97,8 @@ class DepartmentsController extends Controller */ public function store(StoreDepartmentRequest $request): JsonResponse { - $this->authorize('create', Department::class); $department = new Department; - $department->fill($request->all()); + $department->fill($request->validated()); $department = $request->handleImages($department); $department->created_by = auth()->id(); diff --git a/app/Models/Department.php b/app/Models/Department.php index 592fd840b1..4b9eb849f7 100644 --- a/app/Models/Department.php +++ b/app/Models/Department.php @@ -30,10 +30,13 @@ class Department extends SnipeModel ]; protected $rules = [ - 'name' => 'required|max:255|is_unique_department', - 'location_id' => 'numeric|nullable', - 'company_id' => 'numeric|nullable', - 'manager_id' => 'numeric|nullable', + 'name' => 'required|max:255|is_unique_department', + 'location_id' => 'numeric|nullable|exists:locations,id', + 'company_id' => 'numeric|nullable|exists:companies,id', + 'manager_id' => 'numeric|nullable|exists:users,id', + 'phone' => 'string|max:255|nullable', + 'fax' => 'string|max:255|nullable', + 'notes' => 'string|max:255|nullable', ]; /** diff --git a/tests/Feature/Departments/Api/CreateDepartmentsTest.php b/tests/Feature/Departments/Api/CreateDepartmentsTest.php index 4b887bfc3d..0ddb25319e 100644 --- a/tests/Feature/Departments/Api/CreateDepartmentsTest.php +++ b/tests/Feature/Departments/Api/CreateDepartmentsTest.php @@ -8,29 +8,37 @@ use App\Models\Department; use App\Models\Category; use App\Models\User; use Illuminate\Testing\Fluent\AssertableJson; +use phpDocumentor\Reflection\Location; use Tests\TestCase; class CreateDepartmentsTest extends TestCase { - public function testRequiresPermissionToCreateDepartment() + public function test_requires_permission_to_create_department() { $this->actingAsForApi(User::factory()->create()) ->postJson(route('api.departments.store')) ->assertForbidden(); } - public function testCanCreateDepartment() + public function test_can_create_department_with_all_fields() { + $company = Company::factory()->create(); + //$location = Location::factory()->create(); ??? + $manager = User::factory()->create(); $response = $this->actingAsForApi(User::factory()->superuser()->create()) ->postJson(route('api.departments.store'), [ - 'name' => 'Test Department', - 'notes' => 'Test Note', + 'name' => 'Test Department', + 'company_id' => $company->id, + //'location_id' => $location->id, + 'manager_id' => $manager->id, + 'notes' => 'Test Note', + 'phone' => '1234567890', + 'fax' => '1234567890', ]) ->assertOk() ->assertStatusMessageIs('success') - ->assertStatus(200) ->json(); $this->assertTrue(Department::where('name', 'Test Department')->exists()); @@ -40,7 +48,27 @@ class CreateDepartmentsTest extends TestCase $this->assertEquals('Test Note', $department->notes); } - public function testNoArraysAllowed() + public function test_name_required_for_department() + { + $response = $this->actingAsForApi(User::factory()->superuser()->create()) + ->postJson(route('api.departments.store'), [ + 'company_id' => Company::factory()->create()->id, + ]) + ->assertOk() + ->assertStatusMessageIs('error'); + } + + public function test_only_name_required_for_department() + { + $response = $this->actingAsForApi(User::factory()->superuser()->create()) + ->postJson(route('api.departments.store'), [ + 'name' => 'Test Department', + ]) + ->assertOk() + ->assertStatusMessageIs('success'); + } + + public function test_arrays_not_allowed_for_numeric_fields() { $response = $this->actingAsForApi(User::factory()->superuser()->create()) ->postJson(route('api.departments.store'), [ @@ -51,4 +79,17 @@ class CreateDepartmentsTest extends TestCase ->assertStatusMessageIs('error'); } + public function test_arrays_of_strings_are_not_allowed_for_numeric_fields() + { + $response = $this->actingAsForApi(User::factory()->superuser()->create()) + ->postJson(route('api.departments.store'), [ + 'name' => 'Test Department', + 'company_id' => ['1', '2'], + ]) + ->assertOk() + ->assertStatusMessageIs('error'); + } + + + } From 120316bae0a47a075860ef297d2e0d431649b87a Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 21 May 2025 13:36:31 -0500 Subject: [PATCH 3/4] wrong location import --- tests/Feature/Departments/Api/CreateDepartmentsTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Departments/Api/CreateDepartmentsTest.php b/tests/Feature/Departments/Api/CreateDepartmentsTest.php index 0ddb25319e..381665da5c 100644 --- a/tests/Feature/Departments/Api/CreateDepartmentsTest.php +++ b/tests/Feature/Departments/Api/CreateDepartmentsTest.php @@ -6,9 +6,9 @@ use App\Models\AssetModel; use App\Models\Company; use App\Models\Department; use App\Models\Category; +use App\Models\Location; use App\Models\User; use Illuminate\Testing\Fluent\AssertableJson; -use phpDocumentor\Reflection\Location; use Tests\TestCase; class CreateDepartmentsTest extends TestCase @@ -25,13 +25,13 @@ class CreateDepartmentsTest extends TestCase public function test_can_create_department_with_all_fields() { $company = Company::factory()->create(); - //$location = Location::factory()->create(); ??? + $location = Location::factory()->create(); $manager = User::factory()->create(); $response = $this->actingAsForApi(User::factory()->superuser()->create()) ->postJson(route('api.departments.store'), [ 'name' => 'Test Department', 'company_id' => $company->id, - //'location_id' => $location->id, + 'location_id' => $location->id, 'manager_id' => $manager->id, 'notes' => 'Test Note', 'phone' => '1234567890', From 1b397cd780e8cbb3a9703bfc8db1e5a68dcc6c71 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 21 May 2025 18:17:45 -0500 Subject: [PATCH 4/4] assertDbHas --- .../Departments/Api/CreateDepartmentsTest.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/Feature/Departments/Api/CreateDepartmentsTest.php b/tests/Feature/Departments/Api/CreateDepartmentsTest.php index 381665da5c..4d8e968572 100644 --- a/tests/Feature/Departments/Api/CreateDepartmentsTest.php +++ b/tests/Feature/Departments/Api/CreateDepartmentsTest.php @@ -27,7 +27,8 @@ class CreateDepartmentsTest extends TestCase $company = Company::factory()->create(); $location = Location::factory()->create(); $manager = User::factory()->create(); - $response = $this->actingAsForApi(User::factory()->superuser()->create()) + $user = User::factory()->superuser()->create(); + $response = $this->actingAsForApi($user) ->postJson(route('api.departments.store'), [ 'name' => 'Test Department', 'company_id' => $company->id, @@ -46,6 +47,17 @@ class CreateDepartmentsTest extends TestCase $department = Department::find($response['payload']['id']); $this->assertEquals('Test Department', $department->name); $this->assertEquals('Test Note', $department->notes); + + $this->assertDatabaseHas('departments', [ + 'name' => 'Test Department', + 'company_id' => $company->id, + 'location_id' => $location->id, + 'manager_id' => $manager->id, + 'notes' => 'Test Note', + 'phone' => '1234567890', + 'fax' => '1234567890', + 'created_by' => $user->id, + ]); } public function test_name_required_for_department()