Merge pull request #13546 from snipe/fixes/make_boolean_user_fields_more_consistant

Don’t cast as boolean, do validate as boolean for User validation

app/Models/Asset.php

@@ -82,7 +82,6 @@ class Asset extends Depreciable
         'location_id'    => 'integer',
         'rtd_company_id' => 'integer',
         'supplier_id'    => 'integer',
-        'byod'           => 'boolean',
         'created_at'     => 'datetime',
         'updated_at'   => 'datetime',
         'deleted_at'  => 'datetime',
@@ -105,6 +104,7 @@ class Asset extends Depreciable
         'purchase_cost'   => 'numeric|nullable|gte:0',
         'supplier_id'     => 'exists:suppliers,id|nullable',
         'asset_eol_date'  => 'date|max:10|min:10|nullable',
+        'byod'            => 'boolean',
     ];
 
   /**

app/Models/User.php

@@ -69,15 +69,12 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
     ];
 
     protected $casts = [
-        'activated'    => 'boolean',
         'manager_id'   => 'integer',
         'location_id'  => 'integer',
         'company_id'   => 'integer',
-        'vip'      => 'boolean',
         'created_at'   => 'datetime',
         'updated_at'   => 'datetime',
         'deleted_at'   => 'datetime',
-        'autoassign_licenses'    => 'boolean',
     ];
 
     /**
@@ -103,6 +100,9 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
         'state'                   => 'min:2|max:191|nullable',
         'country'                 => 'min:2|max:191|nullable',
         'zip'                     => 'max:10|nullable',
+        'vip'                     => 'boolean',
+        'remote'                  => 'boolean',
+        'activated'               => 'boolean',
     ];
 
     /**

tests/Feature/Api/Users/UpdateUserApiTest.php

@@ -0,0 +1,68 @@
+<?php
+
+namespace Tests\Feature\Api\Users;
+
+use App\Models\User;
+use Tests\Support\InteractsWithSettings;
+use Tests\TestCase;
+use Tests\Support\InteractsWithAuthentication;
+
+
+class UpdateUserApiTest extends TestCase
+{
+    use InteractsWithSettings;
+    use InteractsWithAuthentication;
+
+    public function testApiUsersCanBeActivatedWithNumber()
+    {
+        $admin = User::factory()->superuser()->create();
+        $user = User::factory()->create(['activated' => 0]);
+
+        $this->actingAsForApi($admin)
+            ->patch(route('api.users.update', $user), [
+                'activated' => 1,
+            ]);
+
+        $this->assertEquals(1, $user->refresh()->activated);
+    }
+
+    public function testApiUsersCanBeActivatedWithBooleanTrue()
+    {
+        $admin = User::factory()->superuser()->create();
+        $user = User::factory()->create(['activated' => false]);
+
+        $this->actingAsForApi($admin)
+            ->patch(route('api.users.update', $user), [
+                'activated' => true,
+            ]);
+
+        $this->assertEquals(1, $user->refresh()->activated);
+    }
+
+    public function testApiUsersCanBeDeactivatedWithNumber()
+    {
+        $admin = User::factory()->superuser()->create();
+        $user = User::factory()->create(['activated' => true]);
+
+        $this->actingAsForApi($admin)
+            ->patch(route('api.users.update', $user), [
+                'activated' => 0,
+            ]);
+
+        $this->assertEquals(0, $user->refresh()->activated);
+    }
+
+    public function testApiUsersCanBeDeactivatedWithBooleanFalse()
+    {
+        $admin = User::factory()->superuser()->create();
+        $user = User::factory()->create(['activated' => true]);
+
+        $this->actingAsForApi($admin)
+            ->patch(route('api.users.update', $user), [
+                'activated' => false,
+            ]);
+
+        $this->assertEquals(0, $user->refresh()->activated);
+    }
+
+}

tests/Feature/Users/UpdateUserTest.php

@@ -10,10 +10,10 @@ class UpdateUserTest extends TestCase
 {
     use InteractsWithSettings;
 
-    public function testUsersCanBeActivated()
+    public function testUsersCanBeActivatedWithNumber()
     {
         $admin = User::factory()->superuser()->create();
-        $user = User::factory()->create(['activated' => false]);
+        $user = User::factory()->create(['activated' => 0]);
 
         $this->actingAs($admin)
             ->put(route('users.update', $user), [
@@ -22,10 +22,25 @@ class UpdateUserTest extends TestCase
                 'activated' => 1,
             ]);
 
-        $this->assertTrue($user->refresh()->activated);
+        $this->assertEquals(1, $user->refresh()->activated);
     }
 
-    public function testUsersCanBeDeactivated()
+    public function testUsersCanBeActivatedWithBooleanTrue()
+    {
+        $admin = User::factory()->superuser()->create();
+        $user = User::factory()->create(['activated' => false]);
+
+        $this->actingAs($admin)
+            ->put(route('users.update', $user), [
+                'first_name' => $user->first_name,
+                'username' => $user->username,
+                'activated' => true,
+            ]);
+
+        $this->assertEquals(1, $user->refresh()->activated);
+    }
+
+    public function testUsersCanBeDeactivatedWithNumber()
     {
         $admin = User::factory()->superuser()->create();
         $user = User::factory()->create(['activated' => true]);
@@ -34,12 +49,25 @@ class UpdateUserTest extends TestCase
             ->put(route('users.update', $user), [
                 'first_name' => $user->first_name,
                 'username' => $user->username,
-                // checkboxes that are not checked are
+                'activated' => 0,
-                // not included in the request payload
-                // 'activated' => 0,
             ]);
 
-        $this->assertFalse($user->refresh()->activated);
+        $this->assertEquals(0, $user->refresh()->activated);
+    }
+
+    public function testUsersCanBeDeactivatedWithBooleanFalse()
+    {
+        $admin = User::factory()->superuser()->create();
+        $user = User::factory()->create(['activated' => true]);
+
+        $this->actingAs($admin)
+            ->put(route('users.update', $user), [
+                'first_name' => $user->first_name,
+                'username' => $user->username,
+                'activated' => false,
+            ]);
+
+        $this->assertEquals(0, $user->refresh()->activated);
     }
 
     public function testUsersUpdatingThemselvesDoNotDeactivateTheirAccount()
@@ -50,12 +78,8 @@ class UpdateUserTest extends TestCase
             ->put(route('users.update', $admin), [
                 'first_name' => $admin->first_name,
                 'username' => $admin->username,
-                // checkboxes that are disabled are not
-                // included in the request payload
-                // even if they are checked
-                // 'activated' => 0,
             ]);
 
-        $this->assertTrue($admin->refresh()->activated);
+        $this->assertEquals(1, $admin->refresh()->activated);
     }
 }