- Finish target module

- Fix branch and user policy
ianriizky · Feb 12, 2022 · eac1672 · eac1672
1 parent 1b9f038
commit eac1672
Show file tree

Hide file tree

Showing 9 changed files with 185 additions and 260 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,9 @@
+<?php
+
+use function Pest\Laravel\actingAs;
+
+it('has monitoring index page', function () {
+    actingAs(pest_create_random_user())
+        ->get(route('monitoring.index'))
+        ->assertOk();
+});
diff --git a/tests/Feature/Monitoring/TargetTest.php b/tests/Feature/Monitoring/TargetTest.php
@@ -0,0 +1,144 @@
+<?php
+
+use App\Models\Branch;
+use App\Models\Target;
+use Illuminate\Support\Arr;
+
+use function Pest\Laravel\actingAs;
+use function Pest\Laravel\assertDatabaseHas;
+use function Pest\Laravel\assertModelExists;
+use function Pest\Laravel\assertModelMissing;
+
+beforeEach(function () {
+    $this->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();
+    }
+});