diff --git a/app/Http/Controllers/TargetController.php b/app/Http/Controllers/TargetController.php index 9ba1e7c..7d933d1 100644 --- a/app/Http/Controllers/TargetController.php +++ b/app/Http/Controllers/TargetController.php @@ -6,6 +6,7 @@ use App\Http\Requests\Target\UpdateRequest; use App\Http\Resources\DataTables\TargetResource; use App\Models\Target; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; use Yajra\DataTables\Facades\DataTables; @@ -42,14 +43,11 @@ public function datatable(Request $request) { $this->authorize('viewAny', Target::class); - if ($request->user()->isStaff()) { - $query = $request->user()->branch->targets(); - } else { - $query = Target::query(); - } - - $query + $query = Target::query() ->join('branches', 'targets.branch_id', '=', 'branches.id') + ->when($request->user()->isManager() || $request->user()->isStaff(), function (Builder $query) use ($request) { + $query->where('branches.id', $request->user()->branch->getKey()); + }) ->select('targets.*', 'branches.name as branch_name'); return DataTables::eloquent($query) @@ -158,7 +156,7 @@ public function destroyMultiple(Request $request) { DB::transaction(function () use ($request) { foreach ($request->input('checkbox', []) as $id) { - $target = Target::find($id, 'id'); + $target = Target::find($id, ['id', 'branch_id']); $this->authorize('delete', $target); diff --git a/app/Http/Requests/Target/AbstractRequest.php b/app/Http/Requests/Target/AbstractRequest.php index cc5113a..9725d6a 100644 --- a/app/Http/Requests/Target/AbstractRequest.php +++ b/app/Http/Requests/Target/AbstractRequest.php @@ -6,6 +6,7 @@ use App\Infrastructure\Foundation\Http\FormRequest; use App\Models\Branch; use App\Models\Target; +use App\Rules\BranchExists; use App\Rules\DateFormatLocaleISO; use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\Validator; @@ -41,7 +42,7 @@ public function authorize() public function rules() { return [ - 'branch_id' => [Rule::requiredIf($this->user()->isAdmin()), Rule::exists(Branch::class, 'id')], + 'branch_id' => [Rule::requiredIf($this->user()->isAdmin()), new BranchExists($this->user())], 'periodicity' => ['required', new EnumRule(Periodicity::class)], 'start_date_end_date' => ['required', function ($attribute, $value, $fail) { [$start_date, $end_date] = $this->splitStartEndDate($this->input('start_date_end_date')); @@ -140,10 +141,10 @@ protected static function splitStartEndDate(string $value): array */ public function getBranch(string $key = 'branch_id'): Branch { - if ($this->user()->isStaff()) { - return $this->user()->branch; + if ($this->user()->isAdmin()) { + return Branch::find($this->input($key)); } - return Branch::find($this->input($key)); + return $this->user()->branch; } } diff --git a/app/Policies/BranchPolicy.php b/app/Policies/BranchPolicy.php index 7a4f971..9cc1cf4 100644 --- a/app/Policies/BranchPolicy.php +++ b/app/Policies/BranchPolicy.php @@ -44,7 +44,7 @@ public function viewAny(User $user) */ public function view(User $user, Branch $model) { - return $user->branch->is($model); + return $model->is($user->branch); } /** diff --git a/app/Policies/TargetPolicy.php b/app/Policies/TargetPolicy.php index 4eb7a5b..85f1050 100644 --- a/app/Policies/TargetPolicy.php +++ b/app/Policies/TargetPolicy.php @@ -19,7 +19,7 @@ class TargetPolicy */ public function before(User $user, $ability) { - if ($user->isAdmin() && in_array($ability, ['viewAny', 'view'])) { + if ($user->isAdmin()) { return true; } } @@ -32,19 +32,19 @@ public function before(User $user, $ability) */ public function viewAny(User $user) { - return $user->isStaff(); + return $user->isManager() || $user->isStaff(); } /** * Determine whether the user can view the model. * * @param \App\Models\User $user - * @param \App\Models\Target $target + * @param \App\Models\Target $model * @return \Illuminate\Auth\Access\Response|bool */ - public function view(User $user, Target $target) + public function view(User $user, Target $model) { - return $user->isStaff() && $target->branch->users->contains($user); + return $model->branch->users->contains($user); } /** @@ -55,45 +55,41 @@ public function view(User $user, Target $target) */ public function create(User $user) { - return $user->isAdmin() || $user->isStaff(); + return $user->isManager(); } /** * Determine whether the user can update the model. * * @param \App\Models\User $user - * @param \App\Models\Target $target + * @param \App\Models\Target $model * @return \Illuminate\Auth\Access\Response|bool */ - public function update(User $user, Target $target) + public function update(User $user, Target $model) { - return - $user->isAdmin() || - ($user->isStaff() && $target->branch->users->contains($user)); + return $this->create($user) && $this->view($user, $model); } /** * Determine whether the user can delete the model. * * @param \App\Models\User $user - * @param \App\Models\Target $target + * @param \App\Models\Target $model * @return \Illuminate\Auth\Access\Response|bool */ - public function delete(User $user, Target $target) + public function delete(User $user, Target $model) { - return - $user->isAdmin() || - ($user->isStaff() && $target->branch->users->contains($user)); + return $this->update($user, $model); } /** * Determine whether the user can restore the model. * * @param \App\Models\User $user - * @param \App\Models\Target $target + * @param \App\Models\Target $model * @return \Illuminate\Auth\Access\Response|bool */ - public function restore(User $user, Target $target) + public function restore(User $user, Target $model) { return $user->isAdmin(); } @@ -102,10 +98,10 @@ public function restore(User $user, Target $target) * Determine whether the user can permanently delete the model. * * @param \App\Models\User $user - * @param \App\Models\Target $target + * @param \App\Models\Target $model * @return \Illuminate\Auth\Access\Response|bool */ - public function forceDelete(User $user, Target $target) + public function forceDelete(User $user, Target $model) { return $user->isAdmin(); } diff --git a/app/Policies/UserPolicy.php b/app/Policies/UserPolicy.php index 5d09716..fc6bfd7 100644 --- a/app/Policies/UserPolicy.php +++ b/app/Policies/UserPolicy.php @@ -66,7 +66,7 @@ public function create(User $user) */ public function update(User $user, User $model) { - return $user->isManager() && $user->branch->is($model->branch); + return $this->create($user) && $user->branch->is($model->branch); } /** diff --git a/tests/Feature/Master/UserTest.php b/tests/Feature/Master/UserTest.php index 97b9ad5..330c476 100644 --- a/tests/Feature/Master/UserTest.php +++ b/tests/Feature/Master/UserTest.php @@ -46,8 +46,10 @@ assertDatabaseHas(User::class, Arr::only($data, 'username')); Event::assertDispatched(Registered::class); + } elseif ($this->user->isStaff()) { + $response->assertForbidden(); } else { - $response->assertSessionHas('errors'); + $response->assertSessionHasErrors('branch_id'); } }); @@ -92,8 +94,6 @@ : $branch ); - Event::fake(); - $response = actingAs($this->user)->put(route('master.user.update', $user), $data); if ($this->user->isAdmin() || ($this->user->isManager() && $this->user->branch->is($branch))) { diff --git a/tests/Feature/Monitoring/IndexTest.php b/tests/Feature/Monitoring/IndexTest.php new file mode 100644 index 0000000..f5861fa --- /dev/null +++ b/tests/Feature/Monitoring/IndexTest.php @@ -0,0 +1,9 @@ +get(route('monitoring.index')) + ->assertOk(); +}); diff --git a/tests/Feature/Monitoring/TargetTest.php b/tests/Feature/Monitoring/TargetTest.php new file mode 100644 index 0000000..406f47e --- /dev/null +++ b/tests/Feature/Monitoring/TargetTest.php @@ -0,0 +1,144 @@ +user = pest_create_user(\App\Models\Role::ROLE_MANAGER); +}); + +it('has target index page', function () { + actingAs($this->user) + ->get(route('monitoring.target.index')) + ->assertOk(); +}); + +it('has target create page', function () { + $response = actingAs($this->user)->get(route('monitoring.target.create')); + + if ($this->user->isAdmin() || $this->user->isManager()) { + $response->assertOk(); + } else { + $response->assertForbidden(); + } +}); + +it('can store target', function () { + $data = Target::factory()->rawForm( + $branch = Branch::inRandomOrder()->first('id') + ); + + $response = actingAs($this->user)->post(route('monitoring.target.store'), $data); + + if ($this->user->isAdmin() || ($this->user->isManager() && $this->user->branch->is($branch))) { + $response->assertRedirect(route('monitoring.target.index')); + + assertDatabaseHas(Target::class, Arr::except($data, 'start_date_end_date')); + } elseif ($this->user->isStaff()) { + $response->assertForbidden(); + } else { + $response->assertSessionHasErrors('branch_id'); + } +}); + +it('has target show page', function () { + /** @var \App\Models\Target $target */ + $target = Target::factory() + ->for($branch = Branch::inRandomOrder()->first('id')) + ->create(); + + $response = actingAs($this->user)->get(route('monitoring.target.show', $target)); + + if ($this->user->isAdmin() || + ($this->user->isManager() && $this->user->branch->is($branch)) || + $this->user->is($target)) { + $response->assertOk(); + } else { + $response->assertForbidden(); + } +}); + +it('has target edit page', function () { + /** @var \App\Models\Target $target */ + $target = Target::factory() + ->for($branch = Branch::inRandomOrder()->first('id')) + ->create(); + + $response = actingAs($this->user)->get(route('monitoring.target.edit', $target)); + + if ($this->user->isAdmin() || ($this->user->isManager() && $this->user->branch->is($branch))) { + $response->assertOk(); + } else { + $response->assertForbidden(); + } +}); + +it('can update target', function () { + /** @var \App\Models\Target $target */ + $target = Target::factory() + ->for($branch = Branch::inRandomOrder()->first('id')) + ->create(); + + $data = Target::factory()->rawForm( + $this->user->isAdmin() + ? Branch::inRandomOrder()->first('id') + : $branch + ); + + $response = actingAs($this->user)->put(route('monitoring.target.update', $target), $data); + + $updatedTarget = Target::where(Arr::except($data, 'start_date_end_date'))->first(); + + if ($this->user->isAdmin() || ($this->user->isManager() && $this->user->branch->is($branch))) { + $response->assertRedirect(route('monitoring.target.edit', $updatedTarget)); + + assertModelExists($updatedTarget); + } else { + $response->assertForbidden(); + } +}); + +it('can destroy target', function () { + /** @var \App\Models\Target $target */ + $target = Target::factory() + ->for($branch = Branch::inRandomOrder()->first('id')) + ->create(); + + $response = actingAs($this->user)->delete(route('monitoring.target.destroy', $target)); + + if ($this->user->isAdmin() || ($this->user->isManager() && $this->user->branch->is($branch))) { + $response->assertRedirect(route('monitoring.target.index')); + + assertModelMissing($target); + } else { + $response->assertForbidden(); + } +}); + +it('can destroy multiple target', function () { + /** @var \Illuminate\Database\Eloquent\Collection<\App\Models\Target> $targets */ + $targets = Target::factory() + ->for($branch = Branch::inRandomOrder()->first('id')) + ->count(rand(1, 5)) + ->create(); + + $response = actingAs($this->user)->delete(route('monitoring.target.destroy-multiple', [ + 'checkbox' => $targets->pluck('id')->toArray(), + ])); + + if ($this->user->isAdmin() || ($this->user->isManager() && $this->user->branch->is($branch))) { + $response->assertRedirect(route('monitoring.target.index')); + + foreach ($targets as $target) { + assertModelMissing($target); + } + } else { + $response->assertForbidden(); + } +}); diff --git a/tests/Feature/MonitoringTest.php b/tests/Feature/MonitoringTest.php deleted file mode 100644 index 45c9a3d..0000000 --- a/tests/Feature/MonitoringTest.php +++ /dev/null @@ -1,223 +0,0 @@ -user = pest_create_manager_from_existed_branch(); -}); - -it('has monitoring index page', function () { - actingAs($this->admin) - ->get(route('monitoring.index')) - ->assertOk(); -}); - -#region target -it('has target index page', function () { - actingAs($this->admin) - ->get(route('monitoring.target.index')) - ->assertOk(); -}); - -it('has target create page', function () { - actingAs($this->admin) - ->get(route('monitoring.target.create')) - ->assertOk(); -}); - -it('can store target', function () { - /** @var \App\Models\Branch $branch */ - $branch = Branch::all('id')->random(); - - $data = Target::factory()->rawForm($branch); - - actingAs($this->admin) - ->post(route('monitoring.target.store'), $data) - ->assertRedirect(route('monitoring.target.index')); - - assertDatabaseHas(Target::class, Arr::except($data, 'start_date_end_date')); -}); - -it('has target show page', function () { - /** @var \App\Models\Target $target */ - $target = Target::factory() - ->forBranch() - ->create(); - - actingAs($this->admin) - ->get(route('monitoring.target.show', $target)) - ->assertOk(); -}); - -it('has target edit page', function () { - /** @var \App\Models\Target $target */ - $target = Target::factory() - ->forBranch() - ->create(); - - actingAs($this->admin) - ->get(route('monitoring.target.edit', $target)) - ->assertOk(); -}); - -it('can update target', function () { - /** @var \App\Models\Target $target */ - $target = Target::factory() - ->forBranch() - ->create(); - - /** @var \App\Models\Branch $branch */ - $branch = Branch::all('id')->random(); - - $data = Target::factory()->rawForm($branch); - - $response = actingAs($this->admin) - ->put(route('monitoring.target.update', $target), $data); - - $updatedTarget = Target::where(Arr::except($data, 'start_date_end_date'))->first(); - - $response->assertRedirect(route('monitoring.target.edit', $updatedTarget)); - - assertModelExists($updatedTarget); -}); - -it('can destroy target', function () { - /** @var \App\Models\Target $target */ - $target = Target::factory() - ->forBranch() - ->create(); - - actingAs($this->admin) - ->delete(route('monitoring.target.destroy', $target)) - ->assertRedirect(route('monitoring.target.index')); - - assertModelMissing($target); -}); - -it('can destroy multiple target', function () { - /** @var \Illuminate\Database\Eloquent\Collection<\App\Models\Target> $targets */ - $targets = Target::factory() - ->forBranch() - ->count(rand(1, 5)) - ->create(); - - actingAs($this->admin) - ->delete(route('monitoring.target.destroy-multiple', [ - 'checkbox' => $targets->pluck('id')->toArray(), - ])) - ->assertRedirect(route('monitoring.target.index')); - - foreach ($targets as $target) { - assertModelMissing($target); - } -}); -#endregion target - -#region event -it('has event index page', function () { - actingAs($this->admin) - ->get(route('monitoring.event.index')) - ->assertOk(); -}); - -it('has event create page', function () { - actingAs($this->admin) - ->get(route('monitoring.event.create')) - ->assertOk(); -}); - -it('can store event', function () { - /** @var \App\Models\Branch $branch */ - $branch = Branch::all('id')->random(); - - $data = Event::factory()->rawForm($branch); - - actingAs($this->admin) - ->post(route('monitoring.event.store'), $data) - ->assertRedirect(route('monitoring.event.index')); - - assertDatabaseHas(Event::class, Arr::except($data, 'start_date_end_date')); -}); - -it('has event show page', function () { - /** @var \App\Models\Event $event */ - $event = Event::factory() - ->forBranch() - ->create(); - - actingAs($this->admin) - ->get(route('monitoring.event.show', $event)) - ->assertOk(); -}); - -it('has event edit page', function () { - /** @var \App\Models\Event $event */ - $event = Event::factory() - ->forBranch() - ->create(); - - actingAs($this->admin) - ->get(route('monitoring.event.edit', $event)) - ->assertOk(); -}); - -it('can update event', function () { - /** @var \App\Models\Event $event */ - $event = Event::factory() - ->forBranch() - ->create(); - - /** @var \App\Models\Branch $branch */ - $branch = Branch::all('id')->random(); - - $data = Event::factory()->rawForm($branch); - - $response = actingAs($this->admin) - ->put(route('monitoring.event.update', $event), $data); - - $updatedEvent = Event::where(Arr::except($data, 'start_date_end_date'))->first(); - - $response->assertRedirect(route('monitoring.event.edit', $updatedEvent)); - - assertModelExists($updatedEvent); -}); - -it('can destroy event', function () { - /** @var \App\Models\Event $event */ - $event = Event::factory() - ->forBranch() - ->create(); - - actingAs($this->admin) - ->delete(route('monitoring.event.destroy', $event)) - ->assertRedirect(route('monitoring.event.index')); - - assertModelMissing($event); -}); - -it('can destroy multiple event', function () { - /** @var \Illuminate\Database\Eloquent\Collection<\App\Models\Event> $events */ - $events = Event::factory() - ->forBranch() - ->count(rand(1, 5)) - ->create(); - - actingAs($this->admin) - ->delete(route('monitoring.event.destroy-multiple', [ - 'checkbox' => $events->pluck('id')->toArray(), - ])) - ->assertRedirect(route('monitoring.event.index')); - - foreach ($events as $event) { - assertModelMissing($event); - } -}); -#endregion event