diff --git a/app/Enums/ActionType.php b/app/Enums/ActionType.php index a77efa30ea..70675932b5 100644 --- a/app/Enums/ActionType.php +++ b/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'; } \ No newline at end of file diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index e17f073c49..2ad306ec5d 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/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); diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index 996931dddf..c56c914f93 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -335,19 +335,31 @@ class BulkUsersController extends Controller protected function logItemCheckinAndDelete($items, $itemType) { foreach ($items as $item) { - $item_id = $item->id; - $logAction = new Actionlog(); + \Log::error("ITEM DUMP: ".print_r($item, true)); + if (gettype($item) == 'object' && get_class($item) != 'stdClass') { + $real_item = $item; + } else { + $item_id = $item->id; - if ($itemType == License::class){ - $item_id = $item->license_id; //FIXME - funkery happening here + 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'); - $item->setLogMessage(); - $item->logWithoutSave(ActionType::CheckinFrom); + $real_item->setNote('Bulk checkin items'); + $real_item->setLogMessage(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); } diff --git a/app/Models/Asset.php b/app/Models/Asset.php index aa3518b7b5..d03aa5b4a8 100644 --- a/app/Models/Asset.php +++ b/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; } diff --git a/app/Models/Traits/Loggable.php b/app/Models/Traits/Loggable.php index 99a94bf966..862547bbc0 100644 --- a/app/Models/Traits/Loggable.php +++ b/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) { - $model->logMessage("delete"); + static::deleting(function ($model) { //TODO - is this only for 'hard' delete? Or soft? + \Log::error("DELETING TRIGGER HAS FIRED!!!!!!!!!!!!!!!"); + $model->setLogMessage(ActionType::Delete); }); - static::restoring(function ($model) { - $model->logMessage("restore"); - }); + //static::trashing(function ($model) { //TODO - is *this* the right one? + // $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)) { - $changed[$key]['old'] = '*************'; - $changed[$key]['new'] = '*************'; + + 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); diff --git a/app/Models/User.php b/app/Models/User.php index 018a10aac2..163aa42e00 100644 --- a/app/Models/User.php +++ b/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', diff --git a/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php b/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php index 38c69f3b99..b206592a2b 100644 --- a/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php +++ b/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', diff --git a/tests/Feature/Importing/Api/ImportAssetsTest.php b/tests/Feature/Importing/Api/ImportAssetsTest.php index 0f54b22e92..21d6d358f5 100644 --- a/tests/Feature/Importing/Api/ImportAssetsTest.php +++ b/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);