From 318aff1ef0f29b77b4e3cfd1f308d7e752278c27 Mon Sep 17 00:00:00 2001
From: snipe <snipe@snipe.net>
Date: Sat, 17 Aug 2024 00:27:44 +0100
Subject: [PATCH] Fixed behavior for null model numbers

Signed-off-by: snipe <snipe@snipe.net>
---
 .../Traits/TwoColumnUniqueUndeletedTrait.php    |  2 ++
 app/Providers/ValidationServiceProvider.php     | 17 ++++++++++++-----
 .../AssetModels/Api/CreateAssetModelsTest.php   |  5 +++--
 .../AssetModels/Ui/CreateAssetModelsTest.php    |  5 +++--
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/app/Http/Traits/TwoColumnUniqueUndeletedTrait.php b/app/Http/Traits/TwoColumnUniqueUndeletedTrait.php
index bd60254f1a..0cf0edb455 100644
--- a/app/Http/Traits/TwoColumnUniqueUndeletedTrait.php
+++ b/app/Http/Traits/TwoColumnUniqueUndeletedTrait.php
@@ -16,10 +16,12 @@ trait TwoColumnUniqueUndeletedTrait
         $column = $parameters[0];
         $value = $this->{$parameters[0]};
 
+        // This is an existing model we're updating so ignore the current ID ($this->getKey())
         if ($this->exists) {
             return 'two_column_unique_undeleted:'.$this->table.','.$this->getKey().','.$column.','.$value;
         }
 
+        // This is a new record, so we can ignore the current ID
         return 'two_column_unique_undeleted:'.$this->table.',0,'.$column.','.$value;
     }
 }
diff --git a/app/Providers/ValidationServiceProvider.php b/app/Providers/ValidationServiceProvider.php
index d5b1595022..1f3abca8a6 100644
--- a/app/Providers/ValidationServiceProvider.php
+++ b/app/Providers/ValidationServiceProvider.php
@@ -88,18 +88,25 @@ class ValidationServiceProvider extends ServiceProvider
          *
          * $parameters[0] - the name of the first table we're looking at
          * $parameters[1] - the ID (this will be 0 on new creations)
-         * $parameters[2] - the name of the second table we're looking at
+         * $parameters[2] - the name of the second field we're looking at
          * $parameters[3] - the value that the request is passing for the second table we're
          *                  checking for uniqueness across
          *
          */
         Validator::extend('two_column_unique_undeleted', function ($attribute, $value, $parameters, $validator) {
+
             if (count($parameters)) {
+                
                 $count = DB::table($parameters[0])
-                    ->select('id')->where($attribute, '=', $value)
-                    ->where('id', '!=', $parameters[1])
-                    ->where($parameters[2], $parameters[3])
-                    ->whereNull('deleted_at')
+                    ->select('id')
+                    ->where($attribute, '=', $value)
+                    ->where('id', '!=', $parameters[1]);
+
+                if ($parameters[3]!='') {
+                    $count = $count->where($parameters[2], $parameters[3]);
+                }
+
+                $count = $count->whereNull('deleted_at')
                     ->count();
 
                 return $count < 1;
diff --git a/tests/Feature/AssetModels/Api/CreateAssetModelsTest.php b/tests/Feature/AssetModels/Api/CreateAssetModelsTest.php
index 3879436b79..2928d926f6 100644
--- a/tests/Feature/AssetModels/Api/CreateAssetModelsTest.php
+++ b/tests/Feature/AssetModels/Api/CreateAssetModelsTest.php
@@ -24,7 +24,7 @@ class CreateAssetModelsTest extends TestCase
         $response = $this->actingAsForApi(User::factory()->superuser()->create())
             ->postJson(route('api.models.store'), [
                 'name' => 'Test AssetModel',
-                'category_id' => Category::factory()->create()->id
+                'category_id' => Category::factory()->assetLaptopCategory()->create()->id
             ])
             ->assertOk()
             ->assertStatusMessageIs('success')
@@ -65,6 +65,7 @@ class CreateAssetModelsTest extends TestCase
             ->postJson(route('api.models.store'), [
                 'name' => 'Test Model',
                 'model_number' => '1234',
+                'category_id' => Category::factory()->assetLaptopCategory()->create()->id
             ])
             ->assertStatus(200)
             ->assertOk()
@@ -86,6 +87,7 @@ class CreateAssetModelsTest extends TestCase
         $this->actingAsForApi(User::factory()->superuser()->create())
             ->postJson(route('api.models.store'), [
                 'name' => 'Test Model',
+                'category_id' => Category::factory()->assetLaptopCategory()->create()->id
             ])
             ->assertStatus(200)
             ->assertOk()
@@ -93,7 +95,6 @@ class CreateAssetModelsTest extends TestCase
             ->assertJson([
                 'messages' => [
                     'name'    => ['The name must be unique across models and model number. '],
-                    'model_number'    => ['The model number must be unique across models and name. '],
                 ],
             ])
             ->json();
diff --git a/tests/Feature/AssetModels/Ui/CreateAssetModelsTest.php b/tests/Feature/AssetModels/Ui/CreateAssetModelsTest.php
index 2722e8a34d..b0c6f65a59 100644
--- a/tests/Feature/AssetModels/Ui/CreateAssetModelsTest.php
+++ b/tests/Feature/AssetModels/Ui/CreateAssetModelsTest.php
@@ -24,6 +24,7 @@ class CreateAssetModelsTest extends TestCase
         $this->assertFalse(AssetModel::where('name', 'Test Model')->exists());
 
         $this->actingAs(User::factory()->superuser()->create())
+            ->from(route('models.create'))
             ->post(route('models.store'), [
                 'name' => 'Test Model',
                 'category_id' => Category::factory()->create()->id
@@ -85,9 +86,9 @@ class CreateAssetModelsTest extends TestCase
                 'category_id' => Category::factory()->create()->id
             ])
             ->assertStatus(302)
-            ->assertSessionHasErrors(['name','model_number'])
+            ->assertSessionHasErrors(['name'])
             ->assertRedirect(route('models.create'))
-            ->assertInvalid(['name','model_number']);
+            ->assertInvalid(['name']);
 
         $this->followRedirects($response)->assertSee(trans('general.error'));