Improvements to asset_tag auto-incrementing, with auto-fixups for gaps

app/Http/Controllers/Api/AssetsController.php

@@ -545,7 +545,8 @@ class AssetsController extends Controller
         $asset->model_id                = $request->get('model_id');
         $asset->order_number            = $request->get('order_number');
         $asset->notes                   = $request->get('notes');
-        $asset->asset_tag               = $request->get('asset_tag', Asset::autoincrement_asset());
+        $asset->asset_tag               = $request->get('asset_tag', Asset::autoincrement_asset()); //yup, problem :/
+        // NO IT IS NOT!!! This is never firing; we SHOW the asset_tag you're going to get, so it *will* be filled in!
         $asset->user_id                 = Auth::id();
         $asset->archived                = '0';
         $asset->physical                = '1';

app/Models/Asset.php

@@ -785,24 +785,17 @@ class Asset extends Depreciable
      * @since [v4.0]
      * @return string | false
      */
-    public static function autoincrement_asset()
+    public static function autoincrement_asset(int $additional_increment = 0)
     {
         $settings = \App\Models\Setting::getSettings();
 
 
         if ($settings->auto_increment_assets == '1') {
-            $temp_asset_tag = \DB::table('assets')
-                ->where('physical', '=', '1')
-                ->max('asset_tag');
-
-            $asset_tag_digits = preg_replace('/\D/', '', $temp_asset_tag);
-            $asset_tag = preg_replace('/^0*/', '', $asset_tag_digits);
-
             if ($settings->zerofill_count > 0) {
-                return $settings->auto_increment_prefix.self::zerofill($settings->next_auto_tag_base, $settings->zerofill_count);
+                return $settings->auto_increment_prefix.self::zerofill($settings->next_auto_tag_base + $additional_increment, $settings->zerofill_count);
             }
 
-            return $settings->auto_increment_prefix.$settings->next_auto_tag_base;
+            return $settings->auto_increment_prefix.($settings->next_auto_tag_base + $additional_increment);
         } else {
             return false;
         }

app/Observers/AssetObserver.php

@@ -71,12 +71,33 @@ class AssetObserver
     public function created(Asset $asset)
     {
         if ($settings = Setting::getSettings()) {
-            $settings->increment('next_auto_tag_base');
+            $tag = $asset->asset_tag;
-            $settings->save();
+            $prefix = $settings->auto_increment_prefix;
+            $number = substr($tag, strlen($prefix));
+            // IF - auto_increment_assets is on, AND (the prefix matches the start of the tag OR there is no prefix)
+            //      AND the rest of the string after the prefix is all digits, THEN...
+            if ($settings->auto_increment_assets && (strpos($tag, $prefix) === 0 || $prefix=='') && preg_match('/\d+/',$number) === 1) {
+                // new way of auto-trueing-up auto_increment ID's
+                $next_asset_tag = intval($number, 10) + 1;
+                // we had to use 'intval' because the $number could be '01234' and
+                // might get interpreted in Octal instead of decimal
+
+                // only modify the 'next' one if it's *bigger* than the stored base
+                //
+                if($next_asset_tag > $settings->next_auto_tag_base) {
+                    $settings->next_auto_tag_base = $next_asset_tag;
+                    $settings->save();
+                }
+
+            } else {
+                // legacy method
+                $settings->increment('next_auto_tag_base');
+                $settings->save();
+            }
         }
 
         $logAction = new Actionlog();
-        $logAction->item_type = Asset::class;
+        $logAction->item_type = Asset::class; // can we instead say $logAction->item = $asset ?
         $logAction->item_id = $asset->id;
         $logAction->created_at = date('Y-m-d H:i:s');
         $logAction->user_id = Auth::id();

tests/Support/Settings.php

@@ -56,6 +56,17 @@ class Settings
         ]);
     }
 
+    public function enableAutoIncrement(): Settings
+    {
+        return $this->update([
+            'auto_increment_assets' => 1,
+            'auto_increment_prefix' => 'ABCD',
+            'next_auto_tag_base' => '123',
+            'zerofill_count' => 5
+        ]);
+
+    }
+
     /**
      * @param array $attributes Attributes to modify in the application's settings.
      */

tests/Unit/AssetTest.php

@@ -12,24 +12,120 @@ class AssetTest extends TestCase
 {
     use InteractsWithSettings;
 
-    // public function testAutoIncrementMixed()
+    public function testAutoIncrementCollision()
-    // {
+    {
-    //     $expected = '123411';
+        $this->settings->enableAutoIncrement();
-    //     $next = Asset::nextAutoIncrement(
-    //         collect([
-    //             ['asset_tag' => '0012345'],
-    //             ['asset_tag' => 'WTF00134'],
-    //             ['asset_tag' => 'WTF-745'],
-    //             ['asset_tag' => '0012346'],
-    //             ['asset_tag' => '00123410'],
-    //             ['asset_tag' => 'U8T7597h77'],
-    //         ])
-    //     );
 
-    //     \Log::debug('Next: '.$next);
+        // we have to do this by hand to 'simulate' two web pages being open at the same time
-    //     $this->assertEquals($expected, $next);
+        $a = Asset::factory()->make(['asset_tag' => Asset::autoincrement_asset() ]);
-    // }
+        $b = Asset::factory()->make(['asset_tag' => Asset::autoincrement_asset() ]);
 
+        $this->assertTrue($a->save());
+        $this->assertFalse($b->save());
+    }
+
+    public function testAutoIncrementDouble()
+    {
+        // make one asset with the autoincrement *ONE* higher than the next auto-increment
+        // make sure you can then still make another
+        $this->settings->enableAutoIncrement();
+
+        $gap_number = Asset::autoincrement_asset(1);
+        $final_number = Asset::autoincrement_asset(2);
+        $a = Asset::factory()->make(['asset_tag' => $gap_number]); //make an asset with an ID that is one *over* the next increment
+        $b = Asset::factory()->make(['asset_tag' => Asset::autoincrement_asset()]); //but also make one with one that is *at* the next increment
+        $this->assertTrue($a->save());
+        $this->assertTrue($b->save());
+
+        //and ensure a final asset ends up at *two* over what would've been the next increment at the start
+        $c = Asset::factory()->make(['asset_tag' => Asset::autoincrement_asset()]);
+        $this->assertTrue($c->save());
+        $this->assertEquals($c->asset_tag, $final_number);
+    }
+
+    public function testAutoIncrementGapAndBackfill()
+    {
+        // make one asset 3 higher than the next auto-increment
+        // manually make one that's 1 lower than that
+        // make sure the next one is one higher than the 3 higher one.
+        $this->settings->enableAutoIncrement();
+
+        $big_gap = Asset::autoincrement_asset(3);
+        $final_result = Asset::autoincrement_asset(4);
+        $backfill_one = Asset::autoincrement_asset(0);
+        $backfill_two = Asset::autoincrement_asset(1);
+        $backfill_three = Asset::autoincrement_asset(2);
+        $a = Asset::factory()->create(['asset_tag' => $big_gap]);
+        $this->assertModelExists($a);
+
+        $b = Asset::factory()->create(['asset_tag' => $backfill_one]);
+        $this->assertModelExists($b);
+
+        $c = Asset::factory()->create(['asset_tag' => $backfill_two]);
+        $this->assertModelExists($c);
+
+        $d = Asset::factory()->create(['asset_tag' => $backfill_three]);
+        $this->assertModelExists($d);
+
+        $final = Asset::factory()->create(['asset_tag' => Asset::autoincrement_asset()]);
+        $this->assertModelExists($final);
+        $this->assertEquals($final->asset_tag, $final_result);
+    }
+
+    public function testPrefixlessAutoincrementBackfill()
+    {
+        // TODO: COPYPASTA FROM above, is there a way to still run this test but not have it be so duplicative?
+        $this->settings->enableAutoIncrement()->set(['auto_increment_prefix' => '']);
+
+        $big_gap = Asset::autoincrement_asset(3);
+        $final_result = Asset::autoincrement_asset(4);
+        $backfill_one = Asset::autoincrement_asset(0);
+        $backfill_two = Asset::autoincrement_asset(1);
+        $backfill_three = Asset::autoincrement_asset(2);
+        $a = Asset::factory()->create(['asset_tag' => $big_gap]);
+        $this->assertModelExists($a);
+
+        $b = Asset::factory()->create(['asset_tag' => $backfill_one]);
+        $this->assertModelExists($b);
+
+        $c = Asset::factory()->create(['asset_tag' => $backfill_two]);
+        $this->assertModelExists($c);
+
+        $d = Asset::factory()->create(['asset_tag' => $backfill_three]);
+        $this->assertModelExists($d);
+
+        $final = Asset::factory()->create(['asset_tag' => Asset::autoincrement_asset()]);
+        $this->assertModelExists($final);
+        $this->assertEquals($final->asset_tag, $final_result);
+    }
+
+    public function testUnzerofilledPrefixlessAutoincrementBackfill()
+    {
+        // TODO: COPYPASTA FROM above (AGAIN), is there a way to still run this test but not have it be so duplicative?
+        $this->settings->enableAutoIncrement()->set(['auto_increment_prefix' => '','zerofill_count' => 0]);
+
+        $big_gap = Asset::autoincrement_asset(3);
+        $final_result = Asset::autoincrement_asset(4);
+        $backfill_one = Asset::autoincrement_asset(0);
+        $backfill_two = Asset::autoincrement_asset(1);
+        $backfill_three = Asset::autoincrement_asset(2);
+        $a = Asset::factory()->create(['asset_tag' => $big_gap]);
+        $this->assertModelExists($a);
+
+        $b = Asset::factory()->create(['asset_tag' => $backfill_one]);
+        $this->assertModelExists($b);
+
+        $c = Asset::factory()->create(['asset_tag' => $backfill_two]);
+        $this->assertModelExists($c);
+
+        $d = Asset::factory()->create(['asset_tag' => $backfill_three]);
+        $this->assertModelExists($d);
+
+        $final = Asset::factory()->create(['asset_tag' => Asset::autoincrement_asset()]);
+        $this->assertModelExists($final);
+        $this->assertEquals($final->asset_tag, $final_result);
+    }
+    
     public function testWarrantyExpiresAttribute()
     {