Migrate weird assigned_type issues, Issue #3972 (#3973)

For a while, prior to 987536930, we did not null assigned_type on
checkin.  This migration manually nulls all assigned_type fields if
assigned_to is unset.  Add a test to AssetTest for this as well...kind
of.  We need to extract an Asset::checkin() method for 4.1 that mirrors
Asset::checkOut() to really test this.

This also fixes a separate (but related) issue.  The Asset importer did
not set assigned_type when importing and creating users.  In this
instance, we assume that if assigned_to is set and assigned_type is not,
then the item was checked out to a user and update the DB accordingly.
Also add a check in ImporterTest for this issue.

app/Importer/ItemImporter.php

@@ -10,6 +10,7 @@ use App\Models\Location;
 use App\Models\Manufacturer;
 use App\Models\Statuslabel;
 use App\Models\Supplier;
+use App\Models\User;
 
 class ItemImporter extends Importer
 {
@@ -68,6 +69,7 @@ class ItemImporter extends Importer
         if(get_class($this) !== UserImporter::class) {
             if ($this->item["user"] = $this->createOrFetchUser($row)) {
                 $this->item['assigned_to'] = $this->item['user']->id;
+                $this->item['assigned_type'] = User::class;
             }
         }
     }

app/Models/Asset.php

@@ -79,6 +79,7 @@ class Asset extends Depreciable
     protected $fillable = [
         'asset_tag',
         'assigned_to',
+        'assigned_type',
         'company_id',
         'image',
         'model_id',

database/migrations/2017_09_18_225619_fix_assigned_type_not_being_nulled.php

@@ -0,0 +1,36 @@
+<?php
+
+use App\Models\Asset;
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class FixAssignedTypeNotBeingNulled extends Migration
+{
+    /**
+     * Run the migrations.
+     * There was a point in the v4 beta process where assigned_type was not nulled on checkin
+     * This manually nulls all assets where there is an assigned_type but not an assigned_to.
+     * @return void
+     */
+    public function up()
+    {
+        // There was a point in the v4 beta process where assigned_type was not nulled on checkin
+        // This manually nulls all assets where there is an assigned_type but not an assigned_to.
+        Asset::whereNotNull('assigned_type')->whereNull('assigned_to')->update(['assigned_type' => null]);
+
+        // Additionally, the importer did not set assigned_type when importing.
+        // In the case where we have an assigned_to but not an assigned_type, set the assigned_type to User.
+        Asset::whereNotNull('assigned_to')->whereNull('assigned_type')->update(['assigned_type' => 'App\Models\User']);
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        //
+    }
+}

tests/unit/AssetTest.php

@@ -4,6 +4,7 @@ use App\Models\Asset;
 use App\Models\AssetModel;
 use App\Models\Company;
 use App\Models\Location;
+use App\Models\User;
 use Illuminate\Foundation\Testing\DatabaseTransactions;
 use Illuminate\Foundation\Testing\WithoutMiddleware;
 use Illuminate\Support\Facades\Hash;
@@ -194,6 +195,7 @@ class AssetTest extends BaseTest
         $asset->expected_checkin = null;
         $asset->last_checkout = null;
         $asset->assigned_to = null;
+        $asset->assigned_type = null;
         $asset->assignedTo()->disassociate($asset);
         $asset->accepted = null;
         $asset->save();
@@ -220,6 +222,13 @@ class AssetTest extends BaseTest
             'target_type'   => get_class($target),
             'target_id'     => $target->id
         ]);
+
+        $this->tester->seeRecord('assets', [
+            'id' => $asset->id,
+            'assigned_to' => $target->id,
+            'assigned_type' => User::class
+        ]);
+
         $this->checkin($asset, $target);
         $this->assertNull($asset->fresh()->assignedTo);
 
@@ -229,6 +238,11 @@ class AssetTest extends BaseTest
             'target_id'     => $target->id
         ]);
 
+        $this->tester->seeRecord('assets', [
+            'id' => $asset->id,
+            'assigned_to' => null,
+            'assigned_type' => null
+        ]);
 
         // An Asset Can be checked out to a asset, and this should be logged.
         $target = factory(App\Models\Asset::class)->create();

tests/unit/ImporterTest.php

@@ -8,6 +8,7 @@ use App\Models\Asset;
 use App\Models\AssetModel;
 use App\Models\Category;
 use App\Models\CustomField;
+use App\Models\User;
 use Illuminate\Foundation\Testing\DatabaseMigrations;
 use Illuminate\Foundation\Testing\DatabaseTransactions;
 use Illuminate\Foundation\Testing\WithoutMiddleware;
@@ -148,6 +149,7 @@ EOT;
                 'fieldset_id' => $customFieldSet->id
             ]);
     }
+
     public function testCustomMappingImport()
     {
         $csv = <<<'EOT'
@@ -174,6 +176,12 @@ EOT;
             'last_name' => 'Nelson',
             'email' => 'bnelson0@cdbaby.com',
         ]);
+        // Grab the user record for use in asserting assigned_to
+        $createdUser = $this->tester->grabRecord('users', [
+            'first_name' => 'Bonnie',
+            'last_name' => 'Nelson',
+            'email' => 'bnelson0@cdbaby.com',
+        ]);
 
         $this->tester->seeRecord('categories', [
             'name' => 'quam'
@@ -203,6 +211,7 @@ EOT;
         $this->tester->seeRecord('suppliers', [
             'name' => 'Blogspan'
         ]);
+
         $this->tester->seeRecord('assets', [
             'name' => 'eget nunc donec quis',
             'serial' => '27aa8378-b0f4-4289-84a4-405da95c6147',
@@ -210,7 +219,9 @@ EOT;
             'notes' => "Curabitur in libero ut massa volutpat convallis. Morbi odio odio, elementum eu, interdum eu, tincidunt in, leo. Maecenas pulvinar lobortis est.",
             'purchase_date' => '2016-04-05 00:00:01',
             'purchase_cost' => 133289.59,
-            'warranty_months' => 14
+            'warranty_months' => 14,
+            'assigned_to' => $createdUser['id'],
+            'assigned_type' => User::class
         ]);
     }
 
@@ -240,7 +251,6 @@ EOT;
         $this->tester->seeRecord('categories', [
             'name' => 'Customers'
         ]);
-
     }
 
     public function testDefaultAccessoryUpdate()
@@ -312,7 +322,6 @@ EOT;
         $this->tester->seeRecord('categories', [
             'name' => 'Customers'
         ]);
-
     }
 
     public function testDefaultConsumableImport()
@@ -343,7 +352,6 @@ EOT;
         $this->tester->seeRecord('categories', [
             'name' => 'Triamterene/Hydrochlorothiazide'
         ]);
-
     }
 
     public function testDefaultConsumableUpdate()
@@ -416,7 +424,6 @@ EOT;
         $this->tester->seeRecord('categories', [
             'name' => 'Triamterene/Hydrochlorothiazide'
         ]);
-
     }
 
     public function testDefaultLicenseImport()
@@ -457,7 +464,6 @@ EOT;
         ]);
 
         $this->tester->seeNumRecords(80, 'license_seats');
-
     }
 
     public function testDefaultLicenseUpdate()
@@ -558,7 +564,6 @@ EOT;
         ]);
 
         $this->tester->seeNumRecords(80, 'license_seats');
-
     }
 
     private function import($importer, $mappings = null)