From 6fae6928e771b5e0552fa3ecbae271b9d15b506a Mon Sep 17 00:00:00 2001 From: Daeng Deni Mardaeni Date: Sun, 18 May 2025 16:34:56 +0700 Subject: [PATCH] refactor(positions): perbaikan fungsi otorisasi, optimasi logika, dan penambahan pengujian - Menghapus konstruktor PositionsController untuk middleware autentikasi yang tidak digunakan. - Mengganti izin `positions.*` menjadi `usermanagement.*` di semua fungsi untuk konsistensi otorisasi. - Menambahkan validasi untuk memastikan posisi tidak dapat dihapus jika memiliki role terkait. - Mengoptimalkan logika pada paginasi di API datatables, termasuk default page dan size. - Menyesuaikan response error saat gagal hapus posisi dengan redirect ke halaman index. - Menambahkan pengujian unit dan fitur terhadap CRUD dan otorisasi PositionsController: - Menguji akses halaman index, create, edit, dan delete posisi. - Validasi penyimpanan, pembaruan, dan penghapusan posisi terhadap izin pengguna. - Menguji akses eksport dan fungsionalitas datatables API. - Validasi posisi tidak terhapus jika terikat dengan role. --- app/Http/Controllers/PositionsController.php | 74 ++-- tests/Feature/PositionsControllerTest.php | 344 +++++++++++++++++++ 2 files changed, 366 insertions(+), 52 deletions(-) create mode 100644 tests/Feature/PositionsControllerTest.php diff --git a/app/Http/Controllers/PositionsController.php b/app/Http/Controllers/PositionsController.php index 5b6db01..8e47f2f 100644 --- a/app/Http/Controllers/PositionsController.php +++ b/app/Http/Controllers/PositionsController.php @@ -25,19 +25,6 @@ */ public $user; - /** - * PositionsController constructor. - * - * Initializes the user property with the authenticated user. - */ - /*public function __construct() - { - $this->middleware(function ($request, $next) { - $this->user = Auth::guard('web')->user(); - return $next($request); - }); - }*/ - /** * Display a listing of the resource. * @@ -49,7 +36,7 @@ $user = Auth::guard('web')->user(); // Check if the authenticated user has the required permission to view positions - if (is_null($user) || !$user->can('positions.read')) { + if (is_null($user) || !$user->can('usermanagement.read')) { abort(403, 'Sorry! You are not allowed to view positions.'); } @@ -73,7 +60,7 @@ $user = Auth::guard('web')->user(); // Check if the authenticated user has the required permission to store positions - if (is_null($user) || !$user->can('positions.create')) { + if (is_null($user) || !$user->can('usermanagement.create')) { abort(403, 'Sorry! You are not allowed to store positions.'); } @@ -106,7 +93,7 @@ $user = Auth::guard('web')->user(); // Check if the authenticated user has the required permission to create positions - if (is_null($user) || !$user->can('positions.create')) { + if (is_null($user) || !$user->can('usermanagement.create')) { abort(403, 'Sorry! You are not allowed to create positions.'); } @@ -114,30 +101,6 @@ return view('usermanagement::positions.create'); } - /** - * Display the specified resource. - * - * @param int $id - * - * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View - */ - public function show($id) - { - // Get the authenticated user - $user = Auth::guard('web')->user(); - - // Check if the authenticated user has the required permission to view positions - if (is_null($user) || !$user->can('positions.read')) { - abort(403, 'Sorry! You are not allowed to view positions.'); - } - - // Find the position by ID - $position = Position::findOrFail($id); - - // Return the view for displaying the position - return view('usermanagement::positions.show', compact('position')); - } - /** * Show the form for editing the specified resource. * @@ -151,7 +114,7 @@ $user = Auth::guard('web')->user(); // Check if the authenticated user has the required permission to edit positions - if (is_null($user) || !$user->can('positions.update')) { + if (is_null($user) || !$user->can('usermanagement.update')) { abort(403, 'Sorry! You are not allowed to edit positions.'); } @@ -176,7 +139,7 @@ $user = Auth::guard('web')->user(); // Check if the authenticated user has the required permission to update positions - if (is_null($user) || !$user->can('positions.update')) { + if (is_null($user) || !$user->can('usermanagement.update')) { abort(403, 'Sorry! You are not allowed to update positions.'); } @@ -212,25 +175,31 @@ { // Get the authenticated user $user = Auth::guard('web')->user(); - + // Check if the authenticated user has the required permission to delete positions - if (is_null($user) || !$user->can('positions.delete')) { + if (is_null($user) || !$user->can('usermanagement.delete')) { abort(403, 'Sorry! You are not allowed to delete positions.'); } - + // Find the position by ID $position = Position::findOrFail($id); - + + // Check if the position has associated roles + if ($position->roles()->count() > 0) { + return redirect()->route('users.positions.index') + ->with('error', 'Cannot delete position because it has associated roles.'); + } + try { // If no errors, delete the position from the database $position->delete(); - + // Redirect to the positions index page with a success message return redirect()->route('users.positions.index') ->with('success', 'Position deleted successfully.'); } catch (Exception $e) { // If an error occurs, redirect back with an error message - return redirect()->back() + return redirect()->route('users.positions.index') ->with('error', 'An error occurred while deleting the position: ' . $e->getMessage()); } } @@ -248,7 +217,7 @@ $user = Auth::guard('web')->user(); // Check if the authenticated user has the required permission to view positions - if (is_null($user) || !$user->can('positions.read')) { + if (is_null($user) || !$user->can('usermanagement.read')) { abort(403, 'Sorry! You are not allowed to view positions.'); } @@ -291,10 +260,11 @@ $positions = $query->get(); // Calculate the page count - $pageCount = ceil($totalRecords / $request->get('size')); + $size = $request->get('size', 10); // Default to 10 if not set + $pageCount = $size > 0 ? ceil($totalRecords / $size) : 0; // Calculate the current page number - $currentPage = 0 + 1; + $currentPage = $request->get('page', 1); // Default to page 1 if not set // Return the response data as a JSON object return response()->json([ @@ -320,7 +290,7 @@ $user = Auth::guard('web')->user(); // Check if the authenticated user has the required permission to export positions - if (is_null($user) || !$user->can('positions.export')) { + if (is_null($user) || !$user->can('usermanagement.export')) { abort(403, 'Sorry! You are not allowed to export positions.'); } diff --git a/tests/Feature/PositionsControllerTest.php b/tests/Feature/PositionsControllerTest.php new file mode 100644 index 0000000..8bd95fe --- /dev/null +++ b/tests/Feature/PositionsControllerTest.php @@ -0,0 +1,344 @@ + 'usermanagement', + 'slug' => 'usermanagement' + ]); + + // Create permissions with permission_group_id + Permission::create([ + 'name' => 'usermanagement.create', + 'guard_name' => 'web', + 'permission_group_id' => $permissionGroup->id + ]); + Permission::create([ + 'name' => 'usermanagement.read', + 'guard_name' => 'web', + 'permission_group_id' => $permissionGroup->id + ]); + Permission::create([ + 'name' => 'usermanagement.update', + 'guard_name' => 'web', + 'permission_group_id' => $permissionGroup->id + ]); + Permission::create([ + 'name' => 'usermanagement.delete', + 'guard_name' => 'web', + 'permission_group_id' => $permissionGroup->id + ]); + Permission::create([ + 'name' => 'usermanagement.export', + 'guard_name' => 'web', + 'permission_group_id' => $permissionGroup->id + ]); + + // Create admin role with all permissions + $this->adminRole = Role::create(['name' => 'admin', 'guard_name' => 'web']); + $this->adminRole->givePermissionTo(Permission::all()); + + // Create a user with admin role + $this->user = User::factory()->create(); + $this->user->assignRole($this->adminRole); + + // Create a position for testing + $this->position = Position::create([ + 'code' => 'TEST', + 'name' => 'Test Position', + 'level' => 1 + ]); + } + + #[Test] + public function user_with_permission_can_view_positions_index() + { + $response = $this->actingAs($this->user) + ->get(route('users.positions.index')); + + $response->assertStatus(200); + } + + #[Test] + public function user_without_permission_cannot_view_positions_index() + { + // Create a role without permissions + $role = Role::create(['name' => 'viewer', 'guard_name' => 'web']); + + // Create a user with the viewer role + $user = User::factory()->create(); + $user->assignRole($role); + + $response = $this->actingAs($user) + ->get(route('users.positions.index')); + + $response->assertStatus(403); + } + + #[Test] + public function user_with_permission_can_create_position() + { + $response = $this->actingAs($this->user) + ->get(route('users.positions.create')); + + $response->assertStatus(200); + } + + #[Test] + public function user_without_permission_cannot_create_position() + { + // Create a role with only read permission + $role = Role::create(['name' => 'reader', 'guard_name' => 'web']); + $role->givePermissionTo('usermanagement.read'); + + // Create a user with the reader role + $user = User::factory()->create(); + $user->assignRole($role); + + $response = $this->actingAs($user) + ->get(route('users.positions.create')); + + $response->assertStatus(403); + } + + #[Test] + public function user_with_permission_can_store_position() + { + $positionData = [ + 'code' => 'NEW', + 'name' => 'New Position', + 'level' => 2 + ]; + + $response = $this->actingAs($this->user) + ->post(route('users.positions.store'), $positionData); + + $response->assertRedirect(route('users.positions.index')); + $this->assertDatabaseHas('positions', $positionData); + } + + #[Test] + public function user_without_permission_cannot_store_position() + { + // Create a role with only read permission + $role = Role::create(['name' => 'reader', 'guard_name' => 'web']); + $role->givePermissionTo('usermanagement.read'); + + // Create a user with the reader role + $user = User::factory()->create(); + $user->assignRole($role); + + $positionData = [ + 'code' => 'NEW', + 'name' => 'New Position', + 'level' => 2 + ]; + + $response = $this->actingAs($user) + ->post(route('users.positions.store'), $positionData); + + $response->assertStatus(403); + $this->assertDatabaseMissing('positions', $positionData); + } + + #[Test] + public function user_with_permission_can_edit_position() + { + $response = $this->actingAs($this->user) + ->get(route('users.positions.edit', $this->position->id)); + + $response->assertStatus(200); + } + + #[Test] + public function user_without_permission_cannot_edit_position() + { + // Create a role with only read permission + $role = Role::create(['name' => 'reader', 'guard_name' => 'web']); + $role->givePermissionTo('usermanagement.read'); + + // Create a user with the reader role + $user = User::factory()->create(); + $user->assignRole($role); + + $response = $this->actingAs($user) + ->get(route('users.positions.edit', $this->position->id)); + + $response->assertStatus(403); + } + + #[Test] + public function user_with_permission_can_update_position() + { + $updatedData = [ + 'code' => 'UPD', + 'name' => 'Updated Position', + 'level' => 3 + ]; + + $response = $this->actingAs($this->user) + ->put(route('users.positions.update', $this->position->id), $updatedData); + + $response->assertRedirect(route('users.positions.index')); + $this->assertDatabaseHas('positions', $updatedData); + } + + #[Test] + public function user_without_permission_cannot_update_position() + { + // Create a role with only read permission + $role = Role::create(['name' => 'reader', 'guard_name' => 'web']); + $role->givePermissionTo('usermanagement.read'); + + // Create a user with the reader role + $user = User::factory()->create(); + $user->assignRole($role); + + $updatedData = [ + 'code' => 'UPD', + 'name' => 'Updated Position', + 'level' => 3 + ]; + + $response = $this->actingAs($user) + ->put(route('users.positions.update', $this->position->id), $updatedData); + + $response->assertStatus(403); + $this->assertDatabaseMissing('positions', $updatedData); + } + + #[Test] + public function user_with_permission_can_delete_position() + { + $response = $this->actingAs($this->user) + ->delete(route('users.positions.destroy', $this->position->id)); + + $response->assertRedirect(route('users.positions.index')); + $this->assertSoftDeleted($this->position); + } + + #[Test] + public function user_without_permission_cannot_delete_position() + { + // Create a role with only read permission + $role = Role::create(['name' => 'reader', 'guard_name' => 'web']); + $role->givePermissionTo('usermanagement.read'); + + // Create a user with the reader role + $user = User::factory()->create(); + $user->assignRole($role); + + $response = $this->actingAs($user) + ->delete(route('users.positions.destroy', $this->position->id)); + + $response->assertStatus(403); + $this->assertDatabaseHas('positions', ['id' => $this->position->id, 'deleted_at' => null]); + } + + #[Test] + public function user_with_permission_can_access_datatables_data() + { + $response = $this->actingAs($this->user) + ->get(route('users.positions.datatables')); + + $response->assertStatus(200); + $response->assertJsonStructure([ + 'draw', + 'recordsTotal', + 'recordsFiltered', + 'pageCount', + 'page', + 'totalCount', + 'data' + ]); + } + + #[Test] + public function user_without_permission_cannot_access_datatables_data() + { + // Create a role without permissions + $role = Role::create(['name' => 'viewer', 'guard_name' => 'web']); + + // Create a user with the viewer role + $user = User::factory()->create(); + $user->assignRole($role); + + $response = $this->actingAs($user) + ->get(route('users.positions.datatables')); + + $response->assertStatus(403); + } + + #[Test] + public function user_with_permission_can_export_positions() + { + $response = $this->actingAs($this->user) + ->get(route('users.positions.export')); + + $response->assertStatus(200); + } + + #[Test] + public function user_without_permission_cannot_export_positions() + { + // Create a role with only read permission + $role = Role::create(['name' => 'reader', 'guard_name' => 'web']); + $role->givePermissionTo('usermanagement.read'); + + // Create a user with the reader role + $user = User::factory()->create(); + $user->assignRole($role); + + $response = $this->actingAs($user) + ->get(route('users.positions.export')); + + $response->assertStatus(403); + } + + #[Test] + public function cannot_delete_position_if_it_has_associated_roles() + { + // Create a role associated with the position + $role = Role::create([ + 'name' => 'Position-Linked Role', + 'guard_name' => 'web', + 'position_id' => $this->position->id + ]); + + // Attempt to delete the position + $response = $this->actingAs($this->user) + ->delete(route('users.positions.destroy', $this->position->id)); + + // Assert that the request is redirected back with an error message + $response->assertRedirect(route('users.positions.index')); + $response->assertSessionHas('error'); + + // Assert that the position still exists in the database (not deleted) + $this->assertDatabaseHas('positions', [ + 'id' => $this->position->id, + 'deleted_at' => null + ]); + } +}