Only 3 tests left to go!

app/Enums/ActionType.php

@@ -10,4 +10,7 @@ enum ActionType: string {
     case Requested = 'requested';
     case DeleteSeats = 'delete seats';
     case AddSeats = 'add seats';
+    case Update = 'update';
+    case Create = 'create';
+    case Delete = 'delete';
 }

app/Http/Controllers/Api/UsersController.php

@@ -746,7 +746,7 @@ class UsersController extends Controller
                 return response()->json(Helper::formatStandardApiResponse('error', trans('general.not_deleted', ['item_type' => trans('general.user')])), 200);
             }
 
-            $user->setLogMessage('restore');
+            $user->setLogMessage(ActionType::Restore);
             if ($user->restore()) {
 
                 return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/users/message.success.restored')), 200);

app/Http/Controllers/Users/BulkUsersController.php

@@ -335,19 +335,31 @@ class BulkUsersController extends Controller
     protected function logItemCheckinAndDelete($items, $itemType)
     {
         foreach ($items as $item) {
+            \Log::error("ITEM DUMP: ".print_r($item, true));
+            if (gettype($item) == 'object' && get_class($item) != 'stdClass') {
+                $real_item = $item;
+            } else {
                 $item_id = $item->id;
-            $logAction = new Actionlog();
 
                 if ($itemType == License::class) {
                     $item_id = $item->license_id; //FIXME - funkery happening here
+                    $real_item = License::find($item->license_id);
+                } else {
+                    $real_item = (new $itemType())::find($item_id);
+                }
+            }
+            if (property_exists($item, 'assigned_type')) {
+                $assigned_to = (new ($item->assigned_type))::find($item->assigned_to);
+            } else {
+                $assigned_to = User::find($item->assigned_to);
             }
 
-            $item->setTarget($item->assignedTo); // will this work?!!?!?!?
+            $real_item->setTarget($assigned_to); // will this work?!!?!?!?
             //$logAction->target_id = $item->assigned_to;
             //$logAction->target_type = User::class;
-            $item->setNote('Bulk checkin items');
+            $real_item->setNote('Bulk checkin items');
-            $item->setLogMessage();
+            $real_item->setLogMessage(ActionType::CheckinFrom);
-            $item->logWithoutSave(ActionType::CheckinFrom);
+            $real_item->logWithoutSave(ActionType::CheckinFrom);
         }
     }
 
@@ -355,7 +367,7 @@ class BulkUsersController extends Controller
     {
         foreach ($accessoryUserRows as $accessoryUserRow) {
             $accessory = Accessory::find($accessoryUserRow->accessory_id);
-            $accessory->setTarget(User::find($accessoryUserRow->assignedTo));
+            $accessory->setTarget(User::find($accessoryUserRow->assigned_to)); //FIXME - what if accessory was checked out to location?
             $accessory->setNote('Bulk checkin items');
             $accessory->logWithoutSave(ActionType::CheckinFrom);
         }

app/Models/Asset.php

@@ -42,7 +42,7 @@ class Asset extends Depreciable
     public static function boot()
     {
         // handle incrementing of asset tags
-        self::created(function (Asset $asset) {
+        self::created(function ($asset) {
             if ($settings = Setting::getSettings()) {
                 $tag = $asset->asset_tag;
                 $prefix = $settings->auto_increment_prefix;
@@ -72,9 +72,10 @@ class Asset extends Depreciable
         });
 
         //calculate and update EOL as necessary
-        self::saving(function (Asset $asset) {
+        self::saving(function ($asset) {
             // determine if calculated eol and then calculate it - this should only happen on a new asset
-            if (is_null($asset->asset_eol_date) && !is_null($asset->purchase_date) && ($asset->model->eol > 0)) {
+            //\Log::error("Asset RAW array: ".print_r($asset->toArray(), true));
+            if (is_null($asset->asset_eol_date) && !is_null($asset->purchase_date) && ($asset->model?->eol > 0)) { //FIXME - I shouldn't have to do this.
                 $asset->asset_eol_date = $asset->purchase_date->addMonths($asset->model->eol)->format('Y-m-d');
                 $asset->eol_explicit = false;
             }

app/Models/Traits/Loggable.php

@@ -16,63 +16,99 @@ trait Loggable
 {
     // an attribute for setting whether or not the item was imported
     public ?bool $imported = false;
-    private ?string $log_message = null;
+    private ?string $log_message = null; //FIXME - meant to be 'private'!!!!!
     private ?Model $item = null;
     private array $log_meta = [];
     private ?Model $target = null;
     private ?string $note = null;
 
-    private ?Location $location = null;
+    private ?Location $location_override = null;
 
-    static array $hide_changes = [];
+    //public static array $hide_changes = [];
 
     public static function bootLoggable()
     {
-        \Log::error("LOGGABLE IS BOOTING!!!!!!!!!!!");
-
         //these tiny methods just set up what the log message is going to be
+        // it looks like 'restoring' fires *BEFORE* 'updating' - so we need to handle that
+        static::restoring(function ($model) {
+            \Log::error("Restor*ing* callback firing...");
+            $model->setLogMessage(ActionType::Restore);
+        });
+
         static::updating(function ($model) {
-            $model->logMessage('update');
+            \Log::error("Updating is fired, current log message is: ".$model->log_message);
+            // if we're doing a restore, this 'updating' hook fires *after* the restoring hook
+            // so we make sure not to overwrite the log_message
+            if (!$model->log_message) {
+                $model->setLogMessage(ActionType::Update);
+            }
         });
 
         static::creating(function ($model) {
-            $model->logMessage('create');
+            $model->setLogMessage(ActionType::Create);
         });
 
-        static::deleting(function ($model) {
+        static::deleting(function ($model) { //TODO - is this only for 'hard' delete? Or soft?
-            $model->logMessage("delete");
+            \Log::error("DELETING TRIGGER HAS FIRED!!!!!!!!!!!!!!!");
+            $model->setLogMessage(ActionType::Delete);
         });
 
-        static::restoring(function ($model) {
+        //static::trashing(function ($model) { //TODO - is *this* the right one?
-            $model->logMessage("restore");
+        //    $model->setLogMessage(ActionType::Delete); // No, no it is very much not. there is 'trashed' but not 'trashING'
-        });
+        //});
 
         // THIS sets up the transaction, and gets the 'diff' between the original for the model,
         // and the way it's about to get saved to.
+        // note that this may run *BEFORE* the more specific events, above? I don't know why that is though.
+        // OPEN QUESTION - does this run on soft-delete? I don't know.
         static::saving(function ($model) {
             //possibly consider a "$this->saveWithoutTransaction" thing you can invoke?
             // use "BEGIN" here?! TODO FIXME
+            $changed = [];
 
             foreach ($model->getRawOriginal() as $key => $value) {
                 if ($model->getRawOriginal()[$key] != $model->getAttributes()[$key]) {
                     $changed[$key]['old'] = $model->getRawOriginal()[$key];
                     $changed[$key]['new'] = $model->getAttributes()[$key];
-                }
+
-                if (in_array($key, self::$hide_changes)) {
+                    if (property_exists(self::class, 'hide_changes') && in_array($key, self::$hide_changes)) {
                         $changed[$key]['old'] = '*************';
                         $changed[$key]['new'] = '*************';
                     }
                 }
+            }
 
-            $this->setLogMeta($changed);
+            $model->setLogMeta($changed);
         });
 
         // THIS is the whole enchilada, the MAIN thing that you've got to do to make things work.
         //if we've set everything up correctly, this should pretty much do everything we want, all in one place
         static::saved(function ($model) {
+            if (!$model->log_message && !$model->log_meta) {
+                //nothing was changed, nothing was saved, nothing happened. So there should be no log message.
+                //FIXME if we do the transaction thing!!!!
+                \Log::error("LOG MESSAGE IS BLANK, ****AND**** log_meta is blank! Not sure what that means though...");
+                return;
+            }
+            if (!$model->log_message) {
+                throw new \Exception("Log Message was unset, but log_meta *does* exist - it's: ".print_r($model->log_meta, true));
+            }
             $model->logWithoutSave();
             // DO COMMIT HERE? TODO FIXME
         });
+        static::deleted(function ($model) {
+            \Log::error("Deleted callback has fired!!!!!!!!!!! I guess that means do stuff here?");
+            $model->logWithoutSave(); //TODO - if we do commits up there, we should do them here too?
+        });
+        static::restored(function ($model) {
+            \Log::error("RestorED callback firing.");
+            $model->logWithoutSave(); //TODO - this is getting duplicative.
+        });
+
+        // CRAP.
+        //static::trashed(function ($model) {
+        //    $model->logWithoutSave(ActionType::Delete);
+        //});
 
     }
 
@@ -99,7 +135,7 @@ trait Loggable
         if ($this->note) {
             $logAction->note = $this->note;
         }
-        if ($this->location) {
+        if ($this->location_override) {
             $logAction->location_id = $this->location->id;
         }
         $logAction->logaction($this->log_message);

app/Models/User.php

@@ -3,6 +3,7 @@
 namespace App\Models;
 
 use App\Http\Traits\UniqueUndeletedTrait;
+use App\Models\Traits\Loggable;
 use App\Models\Traits\Searchable;
 use App\Presenters\Presentable;
 use Illuminate\Auth\Authenticatable;
@@ -33,12 +34,15 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
     use Notifiable;
     use Presentable;
     use Searchable;
+    use Loggable;
+
+    // that 'use Loggable' thing is NEW!
 
     protected $hidden = ['password', 'remember_token', 'permissions', 'reset_password_code', 'persist_code'];
     protected $table = 'users';
     protected $injectUniqueIdentifier = true;
 
-    public static array $hide_changes = ['password', 'remember_token', 'two_factor_secret', 'reset_password_code'];
+    public static array $hide_changes = ['password', 'remember_token', 'two_factor_secret', 'reset_password_code', 'persist_code'];
 
     protected $fillable = [
         'activated',

tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php

@@ -151,6 +151,7 @@ class BulkDeleteAssetsTest extends TestCase
                 'bulk_actions' => 'restore',
             ]);
 
+        \Log::error(print_r(Actionlog::all()->toArray(), true));
         $this->assertDatabaseHas('action_logs',
             [
                 'action_type' => 'restore',

tests/Feature/Importing/Api/ImportAssetsTest.php

@@ -82,6 +82,7 @@ class ImportAssetsTest extends ImportDataTestCase implements TestsPermissionsReq
             ->where('item_id', $newAsset->id)
             ->get();
 
+        \Log::error("Activty logs DUMP: ".print_r($activityLogs->toArray(), true));
         $this->assertCount(2, $activityLogs);
 
         $this->assertEquals('checkout', $activityLogs[0]->action_type);