Merge remote-tracking branch 'origin/develop'

2025-11-04 16:06:35 +00:00
parent 8c59a8d405 4a39d7c67a
commit 029f3030a7
7 changed files with 155 additions and 59 deletions
@@ -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;
@@ -26,18 +27,19 @@ class DepartmentsController extends Controller
        $allowed_columns = ['id', 'name', 'image', 'users_count', 'notes'];

        $departments = Department::select(
-            'departments.id',
-            'departments.name',
-            'departments.phone',
-            'departments.fax',
-            'departments.location_id',
-            'departments.company_id',
-            'departments.manager_id',
-            'departments.created_at',
-            'departments.updated_at',
-            'departments.image',
-            'departments.notes',
-        )->with('users')->with('location')->with('manager')->with('company')->withCount('users as users_count');
+            [
+                'departments.id',
+                'departments.name',
+                'departments.phone',
+                'departments.fax',
+                'departments.location_id',
+                'departments.company_id',
+                'departments.manager_id',
+                'departments.created_at',
+                'departments.updated_at',
+                'departments.image',
+                'departments.notes'
+            ])->with('users')->with('location')->with('manager')->with('company')->withCount('users as users_count');

        if ($request->filled('search')) {
            $departments = $departments->TextSearch($request->input('search'));
@@ -94,18 +96,17 @@ 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;
-        $department->fill($request->all());
+        $department->fill($request->validated());
        $department = $request->handleImages($department);

        $department->created_by = auth()->id();
        $department->manager_id = ($request->filled('manager_id') ? $request->input('manager_id') : null);

        if ($department->save()) {
-            return response()->json(Helper::formatStandardApiResponse('success', $department, trans('admin/departments/message.create.success')));
+            return response()->json(Helper::formatStandardApiResponse('success', (new DepartmentsTransformer)->transformDepartment($department), trans('admin/departments/message.create.success')));
        }
        return response()->json(Helper::formatStandardApiResponse('error', null, $department->getErrors()));

@@ -121,7 +122,7 @@ class DepartmentsController extends Controller
    public function show($id) : array
    {
        $this->authorize('view', Department::class);
-        $department = Department::findOrFail($id);
+        $department = Department::withCount('users as users_count')->findOrFail($id);
        return (new DepartmentsTransformer)->transformDepartment($department);
    }

@@ -141,7 +142,7 @@ class DepartmentsController extends Controller
        $department = $request->handleImages($department);

        if ($department->save()) {
-            return response()->json(Helper::formatStandardApiResponse('success', $department, trans('admin/departments/message.update.success')));
+            return response()->json(Helper::formatStandardApiResponse('success', (new DepartmentsTransformer)->transformDepartment($department), trans('admin/departments/message.update.success')));
        }

        return response()->json(Helper::formatStandardApiResponse('error', null, $department->getErrors()));
@@ -0,0 +1,32 @@
+<?php
+
+namespace App\Http\Requests;
+
+use App\Models\Department;
+use Illuminate\Foundation\Http\FormRequest;
+use Illuminate\Support\Facades\Gate;
+
+class StoreDepartmentRequest extends ImageUploadRequest
+{
+    /**
+     * Determine if the user is authorized to make this request.
+     */
+    public function authorize(): bool
+    {
+        return Gate::allows('create', new Department);
+    }
+
+    /**
+     * Get the validation rules that apply to the request.
+     *
+     * @return array<string, \Illuminate\Contracts\Validation\ValidationRule|array<mixed>|string>
+     */
+    public function rules(): array
+    {
+        $modelRules = (new Department)->getRules();
+
+        return array_merge(
+            $modelRules,
+        );
+    }
+}
@@ -43,7 +43,7 @@ class DepartmentsTransformer
                    'id' => (int) $department->location->id,
                    'name' => e($department->location->name),
                ] : null,
-                'users_count' => e($department->users_count),
+                'users_count' => (int) ($department->users_count),
                'notes' => Helper::parseEscapedMarkedownInline($department->notes),
                'created_at' => Helper::getFormattedDateObject($department->created_at, 'datetime'),
                'updated_at' => Helper::getFormattedDateObject($department->updated_at, 'datetime'),
@@ -31,10 +31,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_across_company_and_location:departments,name',
+        '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',
    ];

    /**
@@ -283,41 +283,36 @@ class ValidationServiceProvider extends ServiceProvider
            }
        });

-        Validator::extend('is_unique_department', function ($attribute, $value, $parameters, $validator) {
+        /**
+         * Check that the 'name' field is unique in the table while within both company_id and location_id
+         * This is only used by Departments right now, but could be used elsewhere in the future.
+         */
+        Validator::extend('is_unique_across_company_and_location', function ($attribute, $value, $parameters, $validator) {
            $data = $validator->getData();
+            $table = array_get($parameters, 0);

-            if (
-                array_key_exists('location_id', $data) && $data['location_id'] !== null &&
-                array_key_exists('company_id', $data) && $data['company_id'] !== null
-            ) {
-                //for updating existing departments
-                if(array_key_exists('id', $data) && $data['id'] !== null){
-                    $count = Department::where('name', $data['name'])
-                        ->where('location_id', $data['location_id'])
-                        ->where('company_id', $data['company_id'])
-                        ->whereNotNull('company_id')
-                        ->whereNotNull('location_id')
-                        ->where('id', '!=', $data['id'])
-                        ->count('name');
+            $count = DB::table($table)->select($attribute)
+                ->where($attribute, $value)
+                ->whereNull('deleted_at');

-                    return $count < 1;
-                }else // for entering in new departments
-                {
-                $count = Department::where('name', $data['name'])
-                    ->where('location_id', $data['location_id'])
-                    ->where('company_id', $data['company_id'])
-                    ->whereNotNull('company_id')
-                    ->whereNotNull('location_id')
-                    ->count('name');
-
-                return $count < 1;
+            if (array_key_exists('id', $data) && $data['id'] !== null) {
+                $count = $count->where('id', '!=', $data['id']);
            }
-        }
-            else {
-                return true;
-        }
+
+            if (array_key_exists('location_id', $data) && $data['location_id'] !== null) {
+                $count = $count->where('location_id', $data['location_id']);
+            }
+
+            if (array_key_exists('company_id', $data) && $data['company_id'] !== null) {
+                $count = $count->where('company_id', $data['company_id']);
+            }
+
+            $count = $count->count('name');
+            return $count < 1;
+
        });

+
        Validator::extend('not_array', function ($attribute, $value, $parameters, $validator) {
            return !is_array($value);
        });
@@ -174,7 +174,7 @@ return [
    'ulid' => 'The :attribute field must be a valid ULID.',
    'uuid' => 'The :attribute field must be a valid UUID.',
    'fmcs_location' => 'Full multiple company support and location scoping is enabled in the Admin Settings, and the selected location and selected company are not compatible.',
-
+    'is_unique_across_company_and_location' => 'The :attribute must be unique within the selected company and location.',

    /*
    |--------------------------------------------------------------------------
@@ -3,8 +3,10 @@
 namespace Tests\Feature\Departments\Api;

 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 Tests\TestCase;
@@ -13,23 +15,31 @@ 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()
    {
-        $response = $this->actingAsForApi(User::factory()->superuser()->create())
+        $company = Company::factory()->create();
+        $location = Location::factory()->create();
+        $manager = User::factory()->create();
+        $user = User::factory()->superuser()->create();
+        $response = $this->actingAsForApi($user)
            ->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());
@@ -37,6 +47,61 @@ 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()
+    {
+        $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'), [
+                'name'       => 'Test Department',
+                'company_id' => [1, 2],
+            ])
+            ->assertOk()
+            ->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');
+    }
+
+
+
 }