Merge pull request #14809 from snipe/features/more_tests

Added a few more tests

app/Http/Controllers/Api/UsersController.php

@@ -79,6 +79,10 @@ class UsersController extends Controller
             ->withCount('assets as assets_count', 'licenses as licenses_count', 'accessories as accessories_count', 'consumables as consumables_count', 'managesUsers as manages_users_count', 'managedLocations as manages_locations_count');
 
 
+        if ($request->filled('search') != '') {
+            $users = $users->TextSearch($request->input('search'));
+        }
+
         if ($request->filled('activated')) {
             $users = $users->where('users.activated', '=', $request->input('activated'));
         }
@@ -201,8 +205,12 @@ class UsersController extends Controller
 
         if ($request->filled('location_id') != '') {
             $users = $users->UserLocation($request->input('location_id'), $request->input('search'));
-         } else {
-            $users = $users->TextSearch($request->input('search'));
+         }
+
+        if (($request->filled('deleted')) && ($request->input('deleted') == 'true')) {
+            $users = $users->onlyTrashed();
+        } elseif (($request->filled('all')) && ($request->input('all') == 'true')) {
+            $users = $users->withTrashed();
         }
 
         $order = $request->input('order') === 'asc' ? 'asc' : 'desc';
@@ -254,7 +262,7 @@ class UsersController extends Controller
                         'licenses_count',
                         'consumables_count',
                         'accessories_count',
-                        'manages_user_count',
+                        'manages_users_count',
                         'manages_locations_count',
                         'phone',
                         'address',
@@ -274,16 +282,12 @@ class UsersController extends Controller
                         'website',
                     ];
 
-                $sort = in_array($request->get('sort'), $allowed_columns) ? $request->get('sort') : 'first_name';
+                $sort = in_array($request->input('sort'), $allowed_columns) ? $request->input('sort') : 'first_name';
                 $users = $users->orderBy($sort, $order);
                 break;
         }
 
-        if (($request->filled('deleted')) && ($request->input('deleted') == 'true')) {
-            $users = $users->onlyTrashed();
-        } elseif (($request->filled('all')) && ($request->input('all') == 'true')) {
-            $users = $users->withTrashed();
-        }
+
 
         // Apply companyable scope
         $users = Company::scopeCompanyables($users);
@@ -535,20 +539,29 @@ class UsersController extends Controller
 
         if ($user) {
 
+            if ($user->id === Auth::id()) {
+                // Redirect to the user management page
+                return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.error.cannot_delete_yourself')));
+            }
+
             if (($user->assets) && ($user->assets->count() > 0)) {
-                return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.error.delete_has_assets')));
+                return response()->json(Helper::formatStandardApiResponse('error', null, trans_choice('admin/users/message.error.delete_has_assets_var', $user->assets()->count(), ['count'=> $user->assets()->count()])));
             }
 
             if (($user->licenses) && ($user->licenses->count() > 0)) {
-                return response()->json(Helper::formatStandardApiResponse('error', null, 'This user still has ' . $user->licenses->count() . ' license(s) associated with them and cannot be deleted.'));
+                return response()->json(Helper::formatStandardApiResponse('error', null, trans_choice('admin/users/message.error.delete_has_licenses_var', $user->licenses()->count(), ['count'=> $user->licenses()->count()])));
             }
 
             if (($user->accessories) && ($user->accessories->count() > 0)) {
-                return response()->json(Helper::formatStandardApiResponse('error', null, 'This user still has ' . $user->accessories->count() . ' accessories associated with them.'));
+                return response()->json(Helper::formatStandardApiResponse('error', null, trans_choice('admin/users/message.error.delete_has_accessories_var', $user->accessories()->count(), ['count'=> $user->accessories()->count()])));
             }
 
             if (($user->managedLocations()) && ($user->managedLocations()->count() > 0)) {
-                return response()->json(Helper::formatStandardApiResponse('error', null, 'This user still has ' . $user->managedLocations()->count() . ' locations that they manage.'));
+                return response()->json(Helper::formatStandardApiResponse('error', null, trans_choice('admin/users/message.error.delete_has_locations_var', $user->managedLocations()->count(), ['count'=> $user->managedLocations()->count()])));
+            }
+
+            if (($user->managesUsers()) && ($user->managesUsers()->count() > 0)) {
+                return response()->json(Helper::formatStandardApiResponse('error', null, trans_choice('admin/users/message.error.delete_has_users_var', $user->managesUsers()->count(), ['count'=> $user->managesUsers()->count()])));
             }
 
             if ($user->delete()) {

app/Http/Controllers/Users/UsersController.php

@@ -346,31 +346,33 @@ class UsersController extends Controller
             if ($user->id === Auth::id()) {
                 // Redirect to the user management page
                 return redirect()->route('users.index')
-                    ->with('error', 'We would feel really bad if you deleted yourself, please reconsider.');
+                    ->with('error', trans('admin/users/message.error.cannot_delete_yourself'));
             }
 
-            if (($user->assets()) && (($assetsCount = $user->assets()->count()) > 0)) {
+            if (($user->assets()) && ($user->assets()->count() > 0)) {
                 // Redirect to the user management page
                 return redirect()->route('users.index')
-                    ->with('error', 'This user still has '.$assetsCount.' assets associated with them.');
+                    ->with('error', trans_choice('admin/users/message.error.delete_has_assets_var', $user->assets()->count(), ['count'=> $user->assets()->count()]));
             }
 
-            if (($user->licenses()) && (($licensesCount = $user->licenses()->count())) > 0) {
-                // Redirect to the user management page
-                return redirect()->route('users.index')
-                    ->with('error', 'This user still has '.$licensesCount.' licenses associated with them.');
+            if (($user->licenses()) && ($user->licenses()->count() > 0)) {
+                return redirect()->route('users.index')->with('error', trans_choice('admin/users/message.error.delete_has_licenses_var', $user->licenses()->count(), ['count'=> $user->licenses()->count()]));
             }
 
-            if (($user->accessories()) && (($accessoriesCount = $user->accessories()->count()) > 0)) {
+            if (($user->accessories()) && ($user->accessories()->count() > 0)) {
                 // Redirect to the user management page
-                return redirect()->route('users.index')
-                    ->with('error', 'This user still has '.$accessoriesCount.' accessories associated with them.');
+                return redirect()->route('users.index')->with('error', trans_choice('admin/users/message.error.delete_has_accessories_var', $user->accessories()->count(), ['count'=> $user->accessories()->count()]));
             }
 
-            if (($user->managedLocations()) && (($managedLocationsCount = $user->managedLocations()->count())) > 0) {
+            if (($user->managedLocations()) && ($user->managedLocations()->count() > 0)) {
                 // Redirect to the user management page
                 return redirect()->route('users.index')
-                    ->with('error', 'This user still has '.$managedLocationsCount.' locations that they manage.');
+                    ->with('error', trans_choice('admin/users/message.error.delete_has_locations_var', $user->managedLocations()->count(), ['count'=> $user->managedLocations()->count()]));
+            }
+
+            if (($user->managesUsers()) && ($user->managesUsers()->count() > 0)) {
+                return redirect()->route('users.index')
+                    ->with('error', trans_choice('admin/users/message.error.delete_has_users_var', $user->managesUsers()->count(), ['count'=> $user->managesUsers()->count()]));
             }
 
             // Delete the user

resources/lang/en-US/admin/users/message.php

@@ -37,10 +37,16 @@ return array(
         'update' => 'There was an issue updating the user. Please try again.',
         'delete' => 'There was an issue deleting the user. Please try again.',
         'delete_has_assets' => 'This user has items assigned and could not be deleted.',
+        'delete_has_assets_var' => 'This user still has an asset assigned. Please check it in first.|This user still has :count assets assigned. Please check their assets in first.',
+        'delete_has_licenses_var' => 'This user still has a license seats assigned. Please check it in first.|This user still has :count license seats assigned. Please check them in first.',
+        'delete_has_accessories_var' => 'This user still has an accessory assigned. Please check it in first.|This user still has :count accessories assigned. Please check their assets in first.',
+        'delete_has_locations_var' => 'This user still manages a location. Please select another manager first.|This user still manages :count locations. Please select another manager first.',
+        'delete_has_users_var' => 'This user still manages another user. Please select another manager for that user first.|This user still manages :count users. Please select another manager for them first.',
         'unsuspend' => 'There was an issue unsuspending the user. Please try again.',
         'import'    => 'There was an issue importing users. Please try again.',
         'asset_already_accepted' => 'This asset has already been accepted.',
         'accept_or_decline' => 'You must either accept or decline this asset.',
+        'cannot_delete_yourself' => 'We would feel really bad if you deleted yourself, please reconsider.',
         'incorrect_user_accepted' => 'The asset you have attempted to accept was not checked out to you.',
         'ldap_could_not_connect' => 'Could not connect to the LDAP server. Please check your LDAP server configuration in the LDAP config file. <br>Error from LDAP Server:',
         'ldap_could_not_bind' => 'Could not bind to the LDAP server. Please check your LDAP server configuration in the LDAP config file. <br>Error from LDAP Server: ',

tests/Feature/Api/Users/DeleteUsersTest.php

@@ -0,0 +1,115 @@
+<?php
+
+namespace Tests\Feature\Api\Users;
+
+use App\Models\Asset;
+use App\Models\Company;
+use App\Models\Location;
+use App\Models\User;
+use App\Models\LicenseSeat;
+use Tests\TestCase;
+
+class DeleteUsersTest extends TestCase
+{
+
+
+    public function testDisallowUserDeletionViaApiIfStillManagingPeople()
+    {
+        $manager = User::factory()->create();
+        User::factory()->count(5)->create(['manager_id' => $manager->id]);
+        $this->assertFalse($manager->isDeletable());
+
+        $this->actingAsForApi(User::factory()->deleteUsers()->create())
+            ->deleteJson(route('api.users.destroy', $manager->id))
+            ->assertOk()
+            ->assertStatus(200)
+            ->assertStatusMessageIs('error')
+            ->json();
+    }
+
+    public function testDisallowUserDeletionViaApiIfStillManagingLocations()
+    {
+        $manager = User::factory()->create();
+        Location::factory()->count(5)->create(['manager_id' => $manager->id]);
+
+        $this->assertFalse($manager->isDeletable());
+
+        $this->actingAsForApi(User::factory()->deleteUsers()->create())
+            ->deleteJson(route('api.users.destroy', $manager->id))
+            ->assertOk()
+            ->assertStatus(200)
+            ->assertStatusMessageIs('error')
+            ->json();
+    }
+
+    public function testDisallowUserDeletionViaApiIfStillHasLicenses()
+    {
+        $manager = User::factory()->create();
+        LicenseSeat::factory()->count(5)->create(['assigned_to' => $manager->id]);
+
+        $this->assertFalse($manager->isDeletable());
+
+        $this->actingAsForApi(User::factory()->deleteUsers()->create())
+            ->deleteJson(route('api.users.destroy', $manager->id))
+            ->assertOk()
+            ->assertStatus(200)
+            ->assertStatusMessageIs('error')
+            ->json();
+    }
+
+    public function testDisallowUserDeletionIfNoDeletePermissions()
+    {
+
+        $this->actingAsForApi(User::factory()->create())
+            ->deleteJson(route('api.users.destroy', User::factory()->create()))
+            ->assertStatus(403)
+            ->json();
+    }
+
+    public function testDisallowUserDeletionIfNotInSameCompanyAndNotSuperadmin()
+    {
+        $this->settings->enableMultipleFullCompanySupport();
+        [$companyA, $companyB] = Company::factory()->count(2)->create();
+
+        $superUser = $companyA->users()->save(User::factory()->superuser()->make());
+        $userInCompanyA = $companyA->users()->save(User::factory()->deleteUsers()->make());
+        $userInCompanyB = $companyB->users()->save(User::factory()->deleteUsers()->make());
+
+        $this->actingAsForApi($userInCompanyA)
+            ->deleteJson(route('api.users.destroy', $userInCompanyB))
+            ->assertStatus(403)
+            ->json();
+
+        $this->actingAsForApi($userInCompanyB)
+            ->deleteJson(route('api.users.destroy', $userInCompanyA))
+            ->assertStatus(403)
+            ->json();
+
+        $this->actingAsForApi($superUser)
+            ->deleteJson(route('api.users.destroy', $userInCompanyA))
+            ->assertOk()
+            ->assertStatus(200)
+            ->assertStatusMessageIs('success')
+            ->json();
+
+    }
+
+    public function testUsersCannotDeleteThemselves()
+    {
+        $user = User::factory()->deleteUsers()->create();
+        $this->actingAsForApi($user)
+            ->deleteJson(route('api.users.destroy', $user))
+            ->assertOk()
+            ->assertStatus(200)
+            ->assertStatusMessageIs('error')
+            ->json();
+
+    }
+
+
+
+
+
+
+
+}

tests/Feature/Api/Users/UsersDeleteTest.php

@@ -1,42 +0,0 @@
-<?php
-
-namespace Tests\Feature\Api\Users;
-
-use App\Models\Location;
-use App\Models\User;
-use Laravel\Passport\Passport;
-use Tests\TestCase;
-
-class UsersDeleteTest extends TestCase
-{
-
-
-    public function testDisallowUserDeletionIfStillManagingPeople()
-    {
-        $manager = User::factory()->create(['first_name' => 'Manager', 'last_name' => 'McManagerson']);
-        User::factory()->create(['first_name' => 'Lowly', 'last_name' => 'Worker', 'manager_id' => $manager->id]);
-        $this->actingAs(User::factory()->deleteUsers()->create())->assertFalse($manager->isDeletable());
-    }
-
-    public function testDisallowUserDeletionIfStillManagingLocations()
-    {
-        $manager = User::factory()->create(['first_name' => 'Manager', 'last_name' => 'McManagerson']);
-        Location::factory()->create(['manager_id' => $manager->id]);
-        $this->actingAs(User::factory()->deleteUsers()->create())->assertFalse($manager->isDeletable());
-    }
-
-    public function testAllowUserDeletionIfNotManagingLocations()
-    {
-        $manager = User::factory()->create(['first_name' => 'Manager', 'last_name' => 'McManagerson']);
-        $this->actingAs(User::factory()->deleteUsers()->create())->assertTrue($manager->isDeletable());
-    }
-
-    public function testDisallowUserDeletionIfNoDeletePermissions()
-    {
-        $manager = User::factory()->create(['first_name' => 'Manager', 'last_name' => 'McManagerson']);
-        Location::factory()->create(['manager_id' => $manager->id]);
-        $this->actingAs(User::factory()->editUsers()->create())->assertFalse($manager->isDeletable());
-    }
-
-
-}

tests/Feature/Users/DeleteUsersTest.php

@@ -0,0 +1,66 @@
+<?php
+
+namespace Tests\Feature\Users;
+
+use App\Models\Location;
+use App\Models\User;
+use Laravel\Passport\Passport;
+use Tests\TestCase;
+
+class DeleteUsersTest extends TestCase
+{
+
+
+    public function testDisallowUserDeletionIfStillManagingPeople()
+    {
+        $manager = User::factory()->create();
+        User::factory()->count(3)->create(['manager_id' => $manager->id]);
+
+        $this->actingAs(User::factory()->deleteUsers()->create())->assertFalse($manager->isDeletable());
+
+        $response = $this->actingAs(User::factory()->deleteUsers()->viewUsers()->create())
+            ->delete(route('users.destroy', $manager->id))
+            ->assertStatus(302)
+            ->assertRedirect(route('users.index'));
+
+        $this->followRedirects($response)->assertSee('Error');
+    }
+
+    public function testDisallowUserDeletionIfStillManagingLocations()
+    {
+        $manager = User::factory()->create();
+        Location::factory()->count(3)->create(['manager_id' => $manager->id]);
+
+        $this->actingAs(User::factory()->deleteUsers()->create())->assertFalse($manager->isDeletable());
+
+        $response = $this->actingAs(User::factory()->deleteUsers()->viewUsers()->create())
+            ->delete(route('users.destroy', $manager->id))
+            ->assertStatus(302)
+            ->assertRedirect(route('users.index'));
+
+           $this->followRedirects($response)->assertSee('Error');
+    }
+
+    public function testAllowUserDeletionIfNotManagingLocations()
+    {
+        $manager = User::factory()->create();
+        $this->actingAs(User::factory()->deleteUsers()->create())->assertTrue($manager->isDeletable());
+
+        $response = $this->actingAs(User::factory()->deleteUsers()->viewUsers()->create())
+            ->delete(route('users.destroy', $manager->id))
+            ->assertStatus(302)
+            ->assertRedirect(route('users.index'));
+
+        $this->followRedirects($response)->assertSee('Success');
+
+    }
+
+    public function testDisallowUserDeletionIfNoDeletePermissions()
+    {
+        $manager = User::factory()->create();
+        Location::factory()->create(['manager_id' => $manager->id]);
+        $this->actingAs(User::factory()->editUsers()->create())->assertFalse($manager->isDeletable());
+    }
+
+
+}