Fixed #15005 - Improvements on user merge

app/Console/Commands/MergeUsersByUsername.php

@@ -2,6 +2,7 @@
 
 namespace App\Console\Commands;
 
+use App\Events\UserMerged;
 use App\Models\User;
 use Carbon\Carbon;
 use Illuminate\Console\Command;
@@ -51,7 +52,7 @@ public function handle()
 
             $bad_users = User::where('username', '=', trim($parts[0]))
                 ->whereNull('deleted_at')
-                ->with('assets', 'manager', 'userlog', 'licenses', 'consumables', 'accessories', 'managedLocations')
+                ->with('assets', 'manager', 'userlog', 'licenses', 'consumables', 'accessories', 'managedLocations','uploads', 'acceptances')
                 ->get();
 
 
@@ -105,10 +106,26 @@ public function handle()
                     $managedLocation->save();
                 }
 
+                foreach ($bad_user->uploads as $upload) {
+                    $this->info('Updating upload log record '.$upload->id.' to user '.$user->id);
+                    $upload->item_id = $user->id;
+                    $upload->save();
+                }
+
+                foreach ($bad_user->acceptances as $acceptance) {
+                    $this->info('Updating acceptance log record '.$acceptance->id.' to user '.$user->id);
+                    $acceptance->item_id = $user->id;
+                    $acceptance->save();
+                }
+
                 // Mark the user as deleted
                 $this->info('Marking the user as deleted');
                 $bad_user->deleted_at = Carbon::now()->timestamp;
                 $bad_user->save();
+
+                event(new UserMerged($bad_user, $user, null));
+
+
             }
         }
     }

app/Events/UserMerged.php

@@ -15,7 +15,7 @@ class UserMerged
      *
      * @return void
      */
-    public function __construct(User $from_user, User $to_user, User $admin)
+    public function __construct(User $from_user, User $to_user, ?User $admin)
     {
         $this->merged_from        = $from_user;
         $this->merged_to      = $to_user;

app/Http/Controllers/Users/BulkUsersController.php

@@ -317,7 +317,7 @@ public function merge(Request $request)
 
         // Get the users
         $merge_into_user = User::find($request->input('merge_into_id'));
-        $users_to_merge = User::whereIn('id', $user_ids_to_merge)->with('assets', 'licenses', 'consumables','accessories')->get();
+        $users_to_merge = User::whereIn('id', $user_ids_to_merge)->with('assets', 'manager', 'userlog', 'licenses', 'consumables', 'accessories', 'managedLocations','uploads', 'acceptances')->get();
         $admin = User::find(Auth::user()->id);
 
         // Walk users
@@ -344,10 +344,20 @@ public function merge(Request $request)
             }
 
             foreach ($user_to_merge->userlog as $log) {
-                $log->target_id = $user_to_merge->id;
+                $log->target_id = $merge_into_user->id;
                 $log->save();
             }
 
+            foreach ($user_to_merge->uploads as $upload) {
+                $upload->item_id = $merge_into_user->id;
+                $upload->save();
+            }
+
+            foreach ($user_to_merge->acceptances as $acceptance) {
+                $acceptance->item_id = $merge_into_user->id;
+                $acceptance->save();
+            }
+
             User::where('manager_id', '=', $user_to_merge->id)->update(['manager_id' => $merge_into_user->id]);
 
             foreach ($user_to_merge->managedLocations as $managedLocation) {
@@ -356,7 +366,6 @@ public function merge(Request $request)
             }
 
             $user_to_merge->delete();
-            //$user_to_merge->save();
 
             event(new UserMerged($user_to_merge, $merge_into_user, $admin));
 

app/Listeners/LogListener.php

@@ -111,7 +111,7 @@ public function onUserMerged(UserMerged $event)
         $logaction->target_type = User::class;
         $logaction->action_type = 'merged';
         $logaction->note = trans('general.merged_log_this_user_from', $to_from_array);
-        $logaction->user_id = $event->admin->id;
+        $logaction->user_id = $event->admin->id ?? null;
         $logaction->save();
 
         // Add a record to the users being merged TO
@@ -122,7 +122,7 @@ public function onUserMerged(UserMerged $event)
         $logaction->item_type = User::class;
         $logaction->action_type = 'merged';
         $logaction->note = trans('general.merged_log_this_user_into', $to_from_array);
-        $logaction->user_id = $event->admin->id;
+        $logaction->user_id = $event->admin->id ?? null;
         $logaction->save();
 
 

app/Models/User.php

@@ -481,8 +481,6 @@ public function assetlog()
     /**
      * Establishes the user -> uploads relationship
      *
-     * @todo I don't think we use this?
-     *
      * @author A. Gianotto <[email protected]>
      * @since [v3.0]
      * @return \Illuminate\Database\Eloquent\Relations\Relation
@@ -496,6 +494,21 @@ public function uploads()
             ->orderBy('created_at', 'desc');
     }
 
+    /**
+     * Establishes the user -> acceptances relationship
+     *
+     * @author A. Gianotto <[email protected]>
+     * @since [v7.0.7]
+     * @return \Illuminate\Database\Eloquent\Relations\Relation
+     */
+    public function acceptances()
+    {
+        return $this->hasMany(\App\Models\Actionlog::class, 'target_id')
+            ->where('target_type', self::class)
+            ->where('action_type', '=', 'accepted')
+            ->orderBy('created_at', 'desc');
+    }
+
     /**
      * Establishes the user -> requested assets relationship
      *

database/factories/ActionlogFactory.php

@@ -105,4 +105,64 @@ public function licenseCheckoutToUser()
             ];
         });
     }
+
+    public function filesUploaded()
+    {
+        return $this->state(function () {
+
+            return [
+                'created_at'  => $this->faker->dateTimeBetween('-1 years', 'now', date_default_timezone_get()),
+                'action_type' => 'uploaded',
+                'item_type'  => User::class,
+                'filename'  => $this->faker->unixTime('now'),
+            ];
+        });
+    }
+
+    public function acceptedSignature()
+    {
+        return $this->state(function () {
+
+            $asset = Asset::factory()->create();
+
+            return [
+                'created_at'  => $this->faker->dateTimeBetween('-1 years', 'now', date_default_timezone_get()),
+                'action_type' => 'accepted',
+                'item_id' => $asset->id,
+                'item_type'  => Asset::class,
+                'target_type'  => User::class,
+                'accept_signature'  => $this->faker->unixTime('now'),
+            ];
+        });
+    }
+
+    public function acceptedEula()
+    {
+        return $this->state(function () {
+
+            $asset = Asset::factory()->create();
+
+            return [
+                'created_at'  => $this->faker->dateTimeBetween('-1 years', 'now', date_default_timezone_get()),
+                'action_type' => 'accepted',
+                'item_id' => $asset->id,
+                'item_type'  => Asset::class,
+                'target_type'  => User::class,
+                'filename'  => $this->faker->unixTime('now'),
+            ];
+        });
+    }
+
+    public function logUserUpdate()
+    {
+        return $this->state(function () {
+
+            return [
+                'created_at'  => $this->faker->dateTimeBetween('-1 years', 'now', date_default_timezone_get()),
+                'action_type' => 'update',
+                'target_type'  => User::class,
+                'item_type'  => User::class,
+            ];
+        });
+    }
 }

database/factories/ConsumableFactory.php

@@ -7,6 +7,7 @@
 use App\Models\Consumable;
 use App\Models\Manufacturer;
 use App\Models\User;
+use Carbon\Carbon;
 use Illuminate\Database\Eloquent\Factories\Factory;
 use App\Models\Supplier;
 
@@ -116,4 +117,16 @@ public function requiringAcceptance()
             $consumable->category->update(['require_acceptance' => 1]);
         });
     }
+
+    public function checkedOutToUser(User $user = null)
+    {
+        return $this->afterCreating(function (Consumable $consumable) use ($user) {
+            $consumable->users()->attach($consumable->id, [
+                'consumable_id' => $consumable->id,
+                'created_at' => Carbon::now(),
+                'user_id' => 1,
+                'assigned_to' => $user->id ?? User::factory()->create()->id,
+            ]);
+        });
+    }
 }

tests/Feature/Console/MergeUsersTest.php

@@ -0,0 +1,143 @@
+<?php
+
+namespace Tests\Feature\Users\Console;
+
+use App\Models\Accessory;
+use App\Models\Asset;
+use App\Models\Consumable;
+use App\Models\LicenseSeat;
+use App\Models\User;
+use App\Models\Actionlog;
+use Tests\TestCase;
+
+
+class MergeUsersTest extends TestCase
+{
+    public function testAssetsAreTransferredOnUserMerge()
+    {
+        $user1 = User::factory()->create(['username' => 'user1']);
+        $user_to_merge_into = User::factory()->create(['username' => '[email protected]']);
+
+        Asset::factory()->count(3)->assignedToUser($user1)->create();
+        Asset::factory()->count(3)->assignedToUser($user_to_merge_into)->create();
+
+        $this->artisan('snipeit:merge-users')->assertExitCode(0);
+
+        $this->assertNotEquals(3, $user_to_merge_into->refresh()->assets->count());
+        $this->assertEquals(6, $user_to_merge_into->refresh()->assets->count());
+        $this->assertEquals(0, $user1->refresh()->assets->count());
+
+    }
+
+    public function testLicensesAreTransferredOnUserMerge(): void
+    {
+        $user1 = User::factory()->create(['username' => 'user1']);
+        $user_to_merge_into = User::factory()->create(['username' => '[email protected]']);
+
+        LicenseSeat::factory()->count(3)->create(['assigned_to' => $user1->id]);
+        LicenseSeat::factory()->count(3)->create(['assigned_to' => $user_to_merge_into->id]);
+
+        $this->assertEquals(3, $user_to_merge_into->refresh()->licenses->count());
+
+        $this->artisan('snipeit:merge-users')->assertExitCode(0);
+
+        $this->assertNotEquals(3, $user_to_merge_into->refresh()->licenses->count());
+        $this->assertEquals(6, $user_to_merge_into->refresh()->licenses->count());
+        $this->assertEquals(0, $user1->refresh()->licenses->count());
+
+    }
+
+    public function testAccessoriesTransferredOnUserMerge(): void
+    {
+        $user1 = User::factory()->create(['username' => 'user1']);
+        $user_to_merge_into = User::factory()->create(['username' => '[email protected]']);
+
+        Accessory::factory()->count(3)->checkedOutToUser($user1)->create();
+        Accessory::factory()->count(3)->checkedOutToUser($user_to_merge_into)->create();
+
+        $this->assertEquals(3, $user_to_merge_into->refresh()->accessories->count());
+
+        $this->artisan('snipeit:merge-users')->assertExitCode(0);
+
+        $this->assertNotEquals(3, $user_to_merge_into->refresh()->accessories->count());
+        $this->assertEquals(6, $user_to_merge_into->refresh()->accessories->count());
+        $this->assertEquals(0, $user1->refresh()->accessories->count());
+
+    }
+
+    public function testConsumablesTransferredOnUserMerge(): void
+    {
+        $user1 = User::factory()->create(['username' => 'user1']);
+        $user_to_merge_into = User::factory()->create(['username' => '[email protected]']);
+
+        Consumable::factory()->count(3)->checkedOutToUser($user1)->create();
+        Consumable::factory()->count(3)->checkedOutToUser($user_to_merge_into)->create();
+
+        $this->assertEquals(3, $user_to_merge_into->refresh()->consumables->count());
+
+        $this->artisan('snipeit:merge-users')->assertExitCode(0);
+
+        $this->assertNotEquals(3, $user_to_merge_into->refresh()->consumables->count());
+        $this->assertEquals(6, $user_to_merge_into->refresh()->consumables->count());
+        $this->assertEquals(0, $user1->refresh()->consumables->count());
+
+    }
+
+    public function testFilesAreTransferredOnUserMerge(): void
+    {
+        $user1 = User::factory()->create(['username' => 'user1']);
+        $user_to_merge_into = User::factory()->create(['username' => '[email protected]']);
+
+        Actionlog::factory()->count(3)->filesUploaded()->create(['item_id' => $user1->id]);
+        Actionlog::factory()->count(3)->filesUploaded()->create(['item_id' => $user_to_merge_into->id]);
+
+        $this->assertEquals(3, $user_to_merge_into->refresh()->uploads->count());
+
+        $this->artisan('snipeit:merge-users')->assertExitCode(0);
+
+        $this->assertNotEquals(3, $user_to_merge_into->refresh()->uploads->count());
+        $this->assertEquals(6, $user_to_merge_into->refresh()->uploads->count());
+        $this->assertEquals(0, $user1->refresh()->uploads->count());
+
+    }
+
+    public function testAcceptancesAreTransferredOnUserMerge(): void
+    {
+        $user1 = User::factory()->create(['username' => 'user1']);
+        $user_to_merge_into = User::factory()->create(['username' => '[email protected]']);
+
+        Actionlog::factory()->count(3)->acceptedSignature()->create(['target_id' => $user1->id]);
+        Actionlog::factory()->count(3)->acceptedSignature()->create(['target_id' => $user_to_merge_into->id]);
+
+        $this->assertEquals(3, $user_to_merge_into->refresh()->acceptances->count());
+
+        $this->artisan('snipeit:merge-users')->assertExitCode(0);
+
+        $this->assertNotEquals(3, $user_to_merge_into->refresh()->acceptances->count());
+        $this->assertEquals(6, $user_to_merge_into->refresh()->acceptances->count());
+        $this->assertEquals(0, $user1->refresh()->acceptances->count());
+
+    }
+
+    public function testUserUpdateHistoryIsTransferredOnUserMerge(): void
+    {
+        $user1 = User::factory()->create(['username' => 'user1']);
+        $user_to_merge_into = User::factory()->create(['username' => '[email protected]']);
+
+        Actionlog::factory()->count(3)->logUserUpdate()->create(['target_id' => $user1->id, 'item_id' => $user1->id]);
+        Actionlog::factory()->count(3)->logUserUpdate()->create(['target_id' => $user_to_merge_into->id, 'item_id' => $user_to_merge_into->id]);
+
+        $this->assertEquals(3, $user_to_merge_into->refresh()->userlog->count());
+
+        $this->artisan('snipeit:merge-users')->assertExitCode(0);
+
+        $this->assertNotEquals(3, $user_to_merge_into->refresh()->userlog->count());
+
+        // This needs to be more than the otherwise expected because the merge action itself is logged for the two merging users
+        $this->assertEquals(7, $user_to_merge_into->refresh()->userlog->count());
+        $this->assertEquals(1, $user1->refresh()->userlog->count());
+
+    }
+
+
+}