From 233ad92112bc04fdde5227ce78d130c86bb2c4a5 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 7 Nov 2024 12:41:31 +0000 Subject: [PATCH 1/8] Move Loggable to Traits directory; use trait boot method for observers --- app/Models/Accessory.php | 1 + app/Models/Asset.php | 8 +-- app/Models/AssetModel.php | 5 +- app/Models/Component.php | 1 + app/Models/Consumable.php | 12 +--- app/Models/License.php | 4 +- app/Models/LicenseSeat.php | 1 + app/Models/{ => Traits}/Loggable.php | 104 ++++++++++++++++++++++++++- app/Observers/AssetObserver.php | 87 ---------------------- 9 files changed, 115 insertions(+), 108 deletions(-) rename app/Models/{ => Traits}/Loggable.php (68%) diff --git a/app/Models/Accessory.php b/app/Models/Accessory.php index fc1bb36ab4..cd1bfd62f8 100755 --- a/app/Models/Accessory.php +++ b/app/Models/Accessory.php @@ -4,6 +4,7 @@ namespace App\Models; use App\Helpers\Helper; use App\Models\Traits\Acceptable; +use App\Models\Traits\Loggable; use App\Models\Traits\Searchable; use App\Presenters\Presentable; use Illuminate\Database\Eloquent\Factories\HasFactory; diff --git a/app/Models/Asset.php b/app/Models/Asset.php index c571578172..4e19137d54 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -7,19 +7,17 @@ use App\Exceptions\CheckoutNotAllowed; use App\Helpers\Helper; use App\Http\Traits\UniqueUndeletedTrait; use App\Models\Traits\Acceptable; +use App\Models\Traits\Loggable; use App\Models\Traits\Searchable; -use App\Presenters\Presentable; use App\Presenters\AssetPresenter; -use Illuminate\Support\Facades\Auth; +use App\Presenters\Presentable; use Carbon\Carbon; -use Illuminate\Support\Facades\DB; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Database\Eloquent\Casts\Attribute; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Support\Facades\Storage; use Watson\Validating\ValidatingTrait; -use Illuminate\Database\Eloquent\Casts\Attribute; -use Illuminate\Database\Eloquent\Model; /** * Model for Assets. diff --git a/app/Models/AssetModel.php b/app/Models/AssetModel.php index 02b5df40d1..f5388fdb3a 100755 --- a/app/Models/AssetModel.php +++ b/app/Models/AssetModel.php @@ -2,15 +2,16 @@ namespace App\Models; +use App\Http\Traits\TwoColumnUniqueUndeletedTrait; +use App\Models\Traits\Loggable; use App\Models\Traits\Searchable; +use App\Presenters\AssetModelPresenter; use App\Presenters\Presentable; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Storage; use Watson\Validating\ValidatingTrait; -use \App\Presenters\AssetModelPresenter; -use App\Http\Traits\TwoColumnUniqueUndeletedTrait; /** * Model for Asset Models. Asset Models contain higher level diff --git a/app/Models/Component.php b/app/Models/Component.php index fb77bf0824..2d673495ed 100644 --- a/app/Models/Component.php +++ b/app/Models/Component.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\Models\Traits\Loggable; use App\Models\Traits\Searchable; use App\Presenters\Presentable; use Illuminate\Database\Eloquent\Factories\HasFactory; diff --git a/app/Models/Consumable.php b/app/Models/Consumable.php index 30161e8429..fc7142c02d 100644 --- a/app/Models/Consumable.php +++ b/app/Models/Consumable.php @@ -4,21 +4,15 @@ namespace App\Models; use App\Helpers\Helper; use App\Models\Traits\Acceptable; +use App\Models\Traits\Loggable; use App\Models\Traits\Searchable; +use App\Presenters\ConsumablePresenter; use App\Presenters\Presentable; use Illuminate\Database\Eloquent\Factories\HasFactory; +use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Support\Facades\Storage; use Watson\Validating\ValidatingTrait; -use Illuminate\Database\Eloquent\Relations\Relation; -use App\Presenters\ConsumablePresenter; -use App\Models\Actionlog; -use App\Models\ConsumableAssignment; -use App\Models\User; -use App\Models\Location; -use App\Models\Manufacturer; -use App\Models\Supplier; -use App\Models\Category; class Consumable extends SnipeModel { diff --git a/app/Models/License.php b/app/Models/License.php index 0997c1e57b..fc453d267a 100755 --- a/app/Models/License.php +++ b/app/Models/License.php @@ -3,13 +3,13 @@ namespace App\Models; use App\Helpers\Helper; +use App\Models\Traits\Loggable; use App\Models\Traits\Searchable; use App\Presenters\Presentable; use Carbon\Carbon; -use Illuminate\Support\Facades\DB; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\SoftDeletes; -use Illuminate\Support\Facades\Auth; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Session; use Watson\Validating\ValidatingTrait; diff --git a/app/Models/LicenseSeat.php b/app/Models/LicenseSeat.php index 397a141468..09a68f6c39 100755 --- a/app/Models/LicenseSeat.php +++ b/app/Models/LicenseSeat.php @@ -3,6 +3,7 @@ namespace App\Models; use App\Models\Traits\Acceptable; +use App\Models\Traits\Loggable; use App\Notifications\CheckinLicenseNotification; use App\Notifications\CheckoutLicenseNotification; use App\Presenters\Presentable; diff --git a/app/Models/Loggable.php b/app/Models/Traits/Loggable.php similarity index 68% rename from app/Models/Loggable.php rename to app/Models/Traits/Loggable.php index bd12050dc7..de6eb96ba0 100644 --- a/app/Models/Loggable.php +++ b/app/Models/Traits/Loggable.php @@ -1,17 +1,115 @@ getAttributes(); + $attributesOriginal = $model->getRawOriginal(); + $same_checkout_counter = false; + $same_checkin_counter = false; + $restoring_or_deleting = false; + + + // This is a gross hack to prevent the double logging when restoring an asset + if (array_key_exists('deleted_at', $attributes) && array_key_exists('deleted_at', $attributesOriginal)) { + $restoring_or_deleting = (($attributes['deleted_at'] != $attributesOriginal['deleted_at'])); + } + + if (array_key_exists('checkout_counter', $attributes) && array_key_exists('checkout_counter', $attributesOriginal)) { + $same_checkout_counter = (($attributes['checkout_counter'] == $attributesOriginal['checkout_counter'])); + } + + if (array_key_exists('checkin_counter', $attributes) && array_key_exists('checkin_counter', $attributesOriginal)) { + $same_checkin_counter = (($attributes['checkin_counter'] == $attributesOriginal['checkin_counter'])); + } + + // If the asset isn't being checked out or audited, log the update. + // (Those other actions already create log entries.) + if (($attributes['assigned_to'] == $attributesOriginal['assigned_to']) + && ($same_checkout_counter) && ($same_checkin_counter) + && ((isset($attributes['next_audit_date']) ? $attributes['next_audit_date'] : null) == (isset($attributesOriginal['next_audit_date']) ? $attributesOriginal['next_audit_date'] : null)) + && ($attributes['last_checkout'] == $attributesOriginal['last_checkout']) && (!$restoring_or_deleting)) { + $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 (empty($changed)) { + return; + } + + $logAction = new Actionlog(); + $logAction->item_type = self::class; + $logAction->item_id = $model->id; + $logAction->created_at = date('Y-m-d H:i:s'); + $logAction->created_by = auth()->id(); + $logAction->log_meta = json_encode($changed); + $logAction->logaction('update'); + } + }); + static::updating(function ($model) { + + }); + + /** + * Listen to the Asset deleting event. + * + * @param Asset $asset + * @return void + */ + static::deleting(function ($model) { + $logAction = new Actionlog(); + $logAction->item_type = self::class; + $logAction->item_id = $model->id; + $logAction->created_at = date('Y-m-d H:i:s'); + $logAction->created_by = auth()->id(); + $logAction->logaction('delete'); + }); + + /** + * Listen to the Asset deleting event. + * + * @param Asset $asset + * @return void + */ + static::restoring(function ($model) { + $logAction = new Actionlog(); + $logAction->item_type = self::class; + $logAction->item_id = $model->id; + $logAction->created_at = date('Y-m-d H:i:s'); + $logAction->created_by = auth()->id(); + $logAction->logaction('restore'); + + }); + } + /** * @author Daniel Meltzer * @since [v3.4] diff --git a/app/Observers/AssetObserver.php b/app/Observers/AssetObserver.php index b44d098123..9a51f65e27 100644 --- a/app/Observers/AssetObserver.php +++ b/app/Observers/AssetObserver.php @@ -10,63 +10,6 @@ use Carbon\Carbon; class AssetObserver { - /** - * Listen to the Asset updating event. This fires automatically every time an existing asset is saved. - * - * @param Asset $asset - * @return void - */ - public function updating(Asset $asset) - { - $attributes = $asset->getAttributes(); - $attributesOriginal = $asset->getRawOriginal(); - $same_checkout_counter = false; - $same_checkin_counter = false; - $restoring_or_deleting = false; - - - // This is a gross hack to prevent the double logging when restoring an asset - if (array_key_exists('deleted_at', $attributes) && array_key_exists('deleted_at', $attributesOriginal)){ - $restoring_or_deleting = (($attributes['deleted_at'] != $attributesOriginal['deleted_at'])); - } - - if (array_key_exists('checkout_counter', $attributes) && array_key_exists('checkout_counter', $attributesOriginal)){ - $same_checkout_counter = (($attributes['checkout_counter'] == $attributesOriginal['checkout_counter'])); - } - - if (array_key_exists('checkin_counter', $attributes) && array_key_exists('checkin_counter', $attributesOriginal)){ - $same_checkin_counter = (($attributes['checkin_counter'] == $attributesOriginal['checkin_counter'])); - } - - // If the asset isn't being checked out or audited, log the update. - // (Those other actions already create log entries.) - if (($attributes['assigned_to'] == $attributesOriginal['assigned_to']) - && ($same_checkout_counter) && ($same_checkin_counter) - && ((isset( $attributes['next_audit_date']) ? $attributes['next_audit_date'] : null) == (isset($attributesOriginal['next_audit_date']) ? $attributesOriginal['next_audit_date']: null)) - && ($attributes['last_checkout'] == $attributesOriginal['last_checkout']) && (!$restoring_or_deleting)) - { - $changed = []; - - foreach ($asset->getRawOriginal() as $key => $value) { - if ((array_key_exists($key, $asset->getAttributes())) && ($asset->getRawOriginal()[$key] != $asset->getAttributes()[$key])) { - $changed[$key]['old'] = $asset->getRawOriginal()[$key]; - $changed[$key]['new'] = $asset->getAttributes()[$key]; - } - } - - if (empty($changed)){ - return; - } - - $logAction = new Actionlog(); - $logAction->item_type = Asset::class; - $logAction->item_id = $asset->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->log_meta = json_encode($changed); - $logAction->logaction('update'); - } - } /** * Listen to the Asset created event, and increment @@ -115,37 +58,7 @@ class AssetObserver $logAction->logaction('create'); } - /** - * Listen to the Asset deleting event. - * - * @param Asset $asset - * @return void - */ - public function deleting(Asset $asset) - { - $logAction = new Actionlog(); - $logAction->item_type = Asset::class; - $logAction->item_id = $asset->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('delete'); - } - /** - * Listen to the Asset deleting event. - * - * @param Asset $asset - * @return void - */ - public function restoring(Asset $asset) - { - $logAction = new Actionlog(); - $logAction->item_type = Asset::class; - $logAction->item_id = $asset->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('restore'); - } /** * Executes every time an asset is saved. From 5ce37eaa3407d5d20c610987a160397891e33937 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 7 Nov 2024 16:54:51 +0000 Subject: [PATCH 2/8] First pass at huge Actionlog refactor --- app/Enums/ActionType.php | 13 ++ .../Api/ManufacturersController.php | 10 +- app/Http/Controllers/Api/UsersController.php | 22 +-- .../Controllers/AssetModelsController.php | 9 +- app/Http/Controllers/LocationsController.php | 8 +- .../Controllers/ManufacturersController.php | 8 +- .../Controllers/Users/BulkUsersController.php | 41 ++-- .../Controllers/Users/UsersController.php | 9 +- app/Http/Controllers/ViewAssetsController.php | 56 +++--- app/Models/Asset.php | 59 ++++++ app/Models/Consumable.php | 29 +++ app/Models/License.php | 19 +- app/Models/Requestable.php | 1 + app/Models/Setting.php | 8 + app/Models/Traits/Loggable.php | 177 ++++++++++-------- app/Models/User.php | 2 + app/Observers/AccessoryObserver.php | 62 ------ app/Observers/AssetObserver.php | 100 ---------- app/Observers/ComponentObserver.php | 62 ------ app/Observers/ConsumableObserver.php | 104 ---------- app/Observers/LicenseObserver.php | 62 ------ app/Observers/SettingObserver.php | 21 --- app/Observers/UserObserver.php | 149 --------------- 23 files changed, 271 insertions(+), 760 deletions(-) create mode 100644 app/Enums/ActionType.php delete mode 100644 app/Observers/AccessoryObserver.php delete mode 100644 app/Observers/AssetObserver.php delete mode 100644 app/Observers/ComponentObserver.php delete mode 100644 app/Observers/ConsumableObserver.php delete mode 100644 app/Observers/LicenseObserver.php delete mode 100644 app/Observers/SettingObserver.php delete mode 100644 app/Observers/UserObserver.php diff --git a/app/Enums/ActionType.php b/app/Enums/ActionType.php new file mode 100644 index 0000000000..a77efa30ea --- /dev/null +++ b/app/Enums/ActionType.php @@ -0,0 +1,13 @@ +json(Helper::formatStandardApiResponse('error', trans('general.not_deleted', ['item_type' => trans('general.manufacturer')])), 200); } + $manufacturer->setLogMessage('restore'); if ($manufacturer->restore()) { - - $logaction = new Actionlog(); - $logaction->item_type = Manufacturer::class; - $logaction->item_id = $manufacturer->id; - $logaction->created_at = date('Y-m-d H:i:s'); - $logaction->created_by = auth()->id(); - $logaction->logaction('restore'); - + return response()->json(Helper::formatStandardApiResponse('success', trans('admin/manufacturers/message.restore.success')), 200); } diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index 1d34f90a39..e17f073c49 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Api; +use App\Enums\ActionType; use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Http\Requests\SaveUserRequest; @@ -701,17 +702,8 @@ class UsersController extends Controller $this->authorize('update', $user); $user->two_factor_secret = null; $user->two_factor_enrolled = 0; - $user->saveQuietly(); - - // Log the reset - $logaction = new Actionlog(); - $logaction->target_type = User::class; - $logaction->target_id = $user->id; - $logaction->item_type = User::class; - $logaction->item_id = $user->id; - $logaction->created_at = date('Y-m-d H:i:s'); - $logaction->created_by = auth()->id(); - $logaction->logaction('2FA reset'); + $user->setLogMessage(ActionType::TwoFactorReset); + $user->save(); return response()->json(['message' => trans('admin/settings/general.two_factor_reset_success')], 200); } catch (\Exception $e) { @@ -754,15 +746,9 @@ class UsersController extends Controller return response()->json(Helper::formatStandardApiResponse('error', trans('general.not_deleted', ['item_type' => trans('general.user')])), 200); } + $user->setLogMessage('restore'); if ($user->restore()) { - $logaction = new Actionlog(); - $logaction->item_type = User::class; - $logaction->item_id = $user->id; - $logaction->created_at = date('Y-m-d H:i:s'); - $logaction->created_by = auth()->id(); - $logaction->logaction('restore'); - return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/users/message.success.restored')), 200); } diff --git a/app/Http/Controllers/AssetModelsController.php b/app/Http/Controllers/AssetModelsController.php index f77f718f88..0790bd8b8e 100755 --- a/app/Http/Controllers/AssetModelsController.php +++ b/app/Http/Controllers/AssetModelsController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers; +use App\Enums\ActionType; use App\Helpers\Helper; use App\Http\Requests\ImageUploadRequest; use App\Http\Requests\StoreAssetModelRequest; @@ -224,14 +225,8 @@ class AssetModelsController extends Controller return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.asset_model')])); } + $model->setLogMessage(ActionType::Restore); if ($model->restore()) { - $logaction = new Actionlog(); - $logaction->item_type = AssetModel::class; - $logaction->item_id = $model->id; - $logaction->created_at = date('Y-m-d H:i:s'); - $logaction->created_by = auth()->id(); - $logaction->logaction('restore'); - // Redirect them to the deleted page if there are more, otherwise the section index $deleted_models = AssetModel::onlyTrashed()->count(); diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index 7cf44d604b..fbce721f39 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/app/Http/Controllers/LocationsController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers; +use App\Enums\ActionType; use App\Http\Requests\ImageUploadRequest; use App\Models\Actionlog; use App\Models\Asset; @@ -265,13 +266,8 @@ class LocationsController extends Controller return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.location')])); } + $location->setLogMessage(ActionType::Restore); if ($location->restore()) { - $logaction = new Actionlog(); - $logaction->item_type = Location::class; - $logaction->item_id = $location->id; - $logaction->created_at = date('Y-m-d H:i:s'); - $logaction->created_by = auth()->id(); - $logaction->logaction('restore'); return redirect()->route('locations.index')->with('success', trans('admin/locations/message.restore.success')); } diff --git a/app/Http/Controllers/ManufacturersController.php b/app/Http/Controllers/ManufacturersController.php index 985ec769fc..24266c9633 100755 --- a/app/Http/Controllers/ManufacturersController.php +++ b/app/Http/Controllers/ManufacturersController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers; +use App\Enums\ActionType; use App\Http\Requests\ImageUploadRequest; use App\Models\Actionlog; use App\Models\Manufacturer; @@ -194,13 +195,8 @@ class ManufacturersController extends Controller return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.manufacturer')])); } + $manufacturer->setLogMessage(ActionType::Restore); if ($manufacturer->restore()) { - $logaction = new Actionlog(); - $logaction->item_type = Manufacturer::class; - $logaction->item_id = $manufacturer->id; - $logaction->created_at = date('Y-m-d H:i:s'); - $logaction->created_by = auth()->id(); - $logaction->logaction('restore'); // Redirect them to the deleted page if there are more, otherwise the section index $deleted_manufacturers = Manufacturer::onlyTrashed()->count(); diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index 119da3f7c4..996931dddf 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Users; +use App\Enums\ActionType; use App\Events\UserMerged; use App\Helpers\Helper; use App\Http\Controllers\Controller; @@ -338,45 +339,35 @@ class BulkUsersController extends Controller $logAction = new Actionlog(); if ($itemType == License::class){ - $item_id = $item->license_id; + $item_id = $item->license_id; //FIXME - funkery happening here } - $logAction->item_id = $item_id; - // We can't rely on get_class here because the licenses/accessories fetched above are not eloquent models, but simply arrays. - $logAction->item_type = $itemType; - $logAction->target_id = $item->assigned_to; - $logAction->target_type = User::class; - $logAction->created_by = auth()->id(); - $logAction->note = 'Bulk checkin items'; - $logAction->logaction('checkin from'); + $item->setTarget($item->assignedTo); // 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); } } private function logAccessoriesCheckin(Collection $accessoryUserRows): void { foreach ($accessoryUserRows as $accessoryUserRow) { - $logAction = new Actionlog(); - $logAction->item_id = $accessoryUserRow->accessory_id; - $logAction->item_type = Accessory::class; - $logAction->target_id = $accessoryUserRow->assigned_to; - $logAction->target_type = User::class; - $logAction->created_at = auth()->id(); - $logAction->note = 'Bulk checkin items'; - $logAction->logaction('checkin from'); + $accessory = Accessory::find($accessoryUserRow->accessory_id); + $accessory->setTarget(User::find($accessoryUserRow->assignedTo)); + $accessory->setNote('Bulk checkin items'); + $accessory->logWithoutSave(ActionType::CheckinFrom); } } private function logConsumablesCheckin(Collection $consumableUserRows): void { foreach ($consumableUserRows as $consumableUserRow) { - $logAction = new Actionlog(); - $logAction->item_id = $consumableUserRow->consumable_id; - $logAction->item_type = Consumable::class; - $logAction->target_id = $consumableUserRow->assigned_to; - $logAction->target_type = User::class; - $logAction->created_at = auth()->id(); - $logAction->note = 'Bulk checkin items'; - $logAction->logaction('checkin from'); + $consumable = Consumable::find($consumableUserRow->consumable_id); + $consumable->setTarget(auth()->user()); + $consumable->setNote('Bulk checkin items'); + $consumable->logWithoutSave(ActionType::CheckinFrom); } } diff --git a/app/Http/Controllers/Users/UsersController.php b/app/Http/Controllers/Users/UsersController.php index b88ea3160a..2eecc7bc04 100755 --- a/app/Http/Controllers/Users/UsersController.php +++ b/app/Http/Controllers/Users/UsersController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Users; +use App\Enums\ActionType; use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Http\Requests\DeleteUserRequest; @@ -358,14 +359,8 @@ class UsersController extends Controller return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.user')])); } + $user->setLogMessage(ActionType::Restore); if ($user->restore()) { - $logaction = new Actionlog(); - $logaction->item_type = User::class; - $logaction->item_id = $user->id; - $logaction->created_at = date('Y-m-d H:i:s'); - $logaction->created_by = auth()->id(); - $logaction->logaction('restore'); - // Redirect them to the deleted page if there are more, otherwise the section index $deleted_users = User::onlyTrashed()->count(); if ($deleted_users > 0) { diff --git a/app/Http/Controllers/ViewAssetsController.php b/app/Http/Controllers/ViewAssetsController.php index 12c300e5bd..87c84285b5 100755 --- a/app/Http/Controllers/ViewAssetsController.php +++ b/app/Http/Controllers/ViewAssetsController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers; +use App\Enums\ActionType; use App\Models\Actionlog; use App\Models\Asset; use App\Models\AssetModel; @@ -93,17 +94,18 @@ class ViewAssetsController extends Controller $user = auth()->user(); - $logaction = new Actionlog(); - $logaction->item_id = $data['asset_id'] = $item->id; - $logaction->item_type = $fullItemType; - $logaction->created_at = $data['requested_date'] = date('Y-m-d H:i:s'); - + //$logaction = new Actionlog(); + //$logaction->item_id = $data['asset_id'] = $item->id; + //$logaction->item_type = $fullItemType; + //$logaction->created_at = $data['requested_date'] = date('Y-m-d H:i:s'); + $data['requested_date'] = date('Y-m-d H:i:s'); if ($user->location_id) { - $logaction->location_id = $user->location_id; + $item->setLocation($user->location); } - $logaction->target_id = $data['user_id'] = auth()->id(); - $logaction->target_type = User::class; + $item->setTarget($user); + //$logaction->target_id = $data['user_id'] = auth()->id(); + //$logaction->target_type = User::class; $data['item_quantity'] = $request->has('request-quantity') ? e($request->input('request-quantity')) : 1; $data['requested_by'] = $user->present()->fullName(); @@ -122,7 +124,7 @@ class ViewAssetsController extends Controller if (($item_request = $item->isRequestedBy($user)) || $cancel_by_admin) { $item->cancelRequest($requestingUser); $data['item_quantity'] = ($item_request) ? $item_request->qty : 1; - $logaction->logaction('request_canceled'); + $item->logWithoutSave(ActionType::RequestCanceled); if (($settings->alert_email != '') && ($settings->alerts_enabled == '1') && (! config('app.lock_passwords'))) { $settings->notify(new RequestAssetCancelation($data)); @@ -130,9 +132,9 @@ class ViewAssetsController extends Controller return redirect()->back()->with('success')->with('success', trans('admin/hardware/message.requests.canceled')); } else { - $item->request(); + $item->request(); //!!!!!!!!!!!!! if (($settings->alert_email != '') && ($settings->alerts_enabled == '1') && (! config('app.lock_passwords'))) { - $logaction->logaction('requested'); + $item->logWithoutSave(ActionType::Requested); $settings->notify(new RequestAssetNotification($data)); } @@ -163,25 +165,23 @@ class ViewAssetsController extends Controller $data['item_quantity'] = 1; $settings = Setting::getSettings(); - $logaction = new Actionlog(); - $logaction->item_id = $data['asset_id'] = $asset->id; - $logaction->item_type = $data['item_type'] = Asset::class; - $logaction->created_at = $data['requested_date'] = date('Y-m-d H:i:s'); + //$logaction = new Actionlog(); + $data['asset_id'] = $asset->id; + $data['item_type'] = Asset::class; + $data['requested_date'] = date('Y-m-d H:i:s'); - if ($user->location_id) { - $logaction->location_id = $user->location_id; - } - $logaction->target_id = $data['user_id'] = auth()->id(); - $logaction->target_type = User::class; + $asset->setLocation = $user->location; + $asset->setTarget($user); + $data['user_id'] = auth()->id(); // If it's already requested, cancel the request. if ($asset->isRequestedBy(auth()->user())) { - $asset->cancelRequest(); - $asset->decrement('requests_counter', 1); + $asset->cancelRequest(); //wait, what? + $asset->decrement('requests_counter', 1); //this too - $logaction->logaction('request canceled'); + $asset->logWithoutSave(ActionType::RequestCanceled); try { - $settings->notify(new RequestAssetCancelation($data)); + $settings->notify(new RequestAssetCancelation($data)); //and probably this } catch (\Exception $e) { Log::warning($e); } @@ -189,11 +189,11 @@ class ViewAssetsController extends Controller ->with('success')->with('success', trans('admin/hardware/message.requests.canceled')); } - $logaction->logaction('requested'); - $asset->request(); - $asset->increment('requests_counter', 1); + $asset->logWithoutSave(ActionType::Requested); //ARGH + $asset->request(); //HERE <- + $asset->increment('requests_counter', 1); //ARGH try { - $settings->notify(new RequestAssetNotification($data)); + $settings->notify(new RequestAssetNotification($data)); // ANd this. } catch (\Exception $e) { Log::warning($e); } diff --git a/app/Models/Asset.php b/app/Models/Asset.php index 4e19137d54..aa3518b7b5 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -39,6 +39,65 @@ class Asset extends Depreciable use Acceptable; + public static function boot() + { + // handle incrementing of asset tags + self::created(function (Asset $asset) { + if ($settings = Setting::getSettings()) { + $tag = $asset->asset_tag; + $prefix = $settings->auto_increment_prefix; + $number = substr($tag, strlen($prefix)); + // IF - auto_increment_assets is on, AND (there is no prefix OR the prefix matches the start of the tag) + // AND the rest of the string after the prefix is all digits, THEN... + if ($settings->auto_increment_assets && ($prefix == '' || strpos($tag, $prefix) === 0) && 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 && $next_asset_tag < PHP_INT_MAX) { + $settings->next_auto_tag_base = $next_asset_tag; + $settings->save(); + } + + } else { + // legacy method + $settings->increment('next_auto_tag_base'); + $settings->save(); + } + } + + }); + + //calculate and update EOL as necessary + self::saving(function (Asset $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)) { + $asset->asset_eol_date = $asset->purchase_date->addMonths($asset->model->eol)->format('Y-m-d'); + $asset->eol_explicit = false; + } + + // determine if explicit and set eol_explicit to true + if (!is_null($asset->asset_eol_date) && !is_null($asset->purchase_date)) { + if ($asset->model->eol > 0) { + $months = Carbon::parse($asset->asset_eol_date)->diffInMonths($asset->purchase_date); + if ($months != $asset->model->eol) { + $asset->eol_explicit = true; + } + } + } elseif (!is_null($asset->asset_eol_date) && is_null($asset->purchase_date)) { + $asset->eol_explicit = true; + } + if ((!is_null($asset->asset_eol_date)) && (!is_null($asset->purchase_date)) && (is_null($asset->model->eol) || ($asset->model->eol == 0))) { + $asset->eol_explicit = true; + } + + }); + parent::boot(); + } + /** * Run after the checkout acceptance was declined by the user * diff --git a/app/Models/Consumable.php b/app/Models/Consumable.php index fc7142c02d..c2518f7733 100644 --- a/app/Models/Consumable.php +++ b/app/Models/Consumable.php @@ -11,6 +11,7 @@ use App\Presenters\Presentable; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Database\Eloquent\SoftDeletes; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Watson\Validating\ValidatingTrait; @@ -48,6 +49,34 @@ class Consumable extends SnipeModel 'purchase_date' => 'date_format:Y-m-d|nullable', ]; + public static function boot() + { + self::deleting(function ($consumable) { + $consumable->users()->detach(); + $uploads = $consumable->uploads; + + foreach ($uploads as $file) { + try { + Storage::delete('private_uploads/consumables/'.$file->filename); + $file->delete(); + } catch (\Exception $e) { + Log::info($e); + } + } + + + try { + Storage::disk('public')->delete('consumables/'.$consumable->image); + } catch (\Exception $e) { + Log::info($e); + } + + $consumable->image = null; + + }); + parent::boot(); + } + /** * Whether the model should inject it's identifier to the unique * validation rules before attempting validation. If this property diff --git a/app/Models/License.php b/app/Models/License.php index fc453d267a..29d09ce527 100755 --- a/app/Models/License.php +++ b/app/Models/License.php @@ -12,6 +12,7 @@ use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Session; use Watson\Validating\ValidatingTrait; +use App\Enums\ActionType; class License extends Depreciable { @@ -180,13 +181,8 @@ class License extends Depreciable $seatsAvailableForDelete->pop()->delete(); } // Log Deletion of seats. - $logAction = new Actionlog; - $logAction->item_type = self::class; - $logAction->item_id = $license->id; - $logAction->created_by = auth()->id() ?: 1; // We don't have an id while running the importer from CLI. - $logAction->note = "deleted {$change} seats"; - $logAction->target_id = null; - $logAction->logaction('delete seats'); + $license->setNote("deleted {$change} seats"); + $license->logWithoutSave(ActionType::DeleteSeats); return true; } @@ -212,13 +208,8 @@ class License extends Depreciable // On initial create, we shouldn't log the addition of seats. if ($license->id) { //Log the addition of license to the log. - $logAction = new Actionlog(); - $logAction->item_type = self::class; - $logAction->item_id = $license->id; - $logAction->created_by = auth()->id() ?: 1; // Importer. - $logAction->note = "added {$change} seats"; - $logAction->target_id = null; - $logAction->logaction('add seats'); + $license->setNote("added {$change} seats"); + $license->logWithoutSave(ActionType::AddSeats); } return true; diff --git a/app/Models/Requestable.php b/app/Models/Requestable.php index 4dead82bb3..ef3e9a84ff 100644 --- a/app/Models/Requestable.php +++ b/app/Models/Requestable.php @@ -28,6 +28,7 @@ trait Requestable public function request($qty = 1) { + // THIS is where the requested log action thing should go, yeah? FIXME $this->requests()->save( new CheckoutRequest(['user_id' => auth()->id(), 'qty' => $qty]) ); diff --git a/app/Models/Setting.php b/app/Models/Setting.php index 8f8299a3c5..e61eda065d 100755 --- a/app/Models/Setting.php +++ b/app/Models/Setting.php @@ -74,6 +74,14 @@ class Setting extends Model 'require_checkinout_notes' => 'boolean', ]; + public static function boot() + { + self::saved(function ($model) { + Cache::forget(Setting::SETUP_CHECK_KEY); + }); + parent::boot(); + } + /** * Get the app settings. * Cache is expired on Setting model saved in EventServiceProvider. diff --git a/app/Models/Traits/Loggable.php b/app/Models/Traits/Loggable.php index de6eb96ba0..99a94bf966 100644 --- a/app/Models/Traits/Loggable.php +++ b/app/Models/Traits/Loggable.php @@ -2,6 +2,7 @@ namespace App\Models\Traits; +use App\Enums\ActionType; use App\Models\Actionlog; use App\Models\Asset; use App\Models\License; @@ -9,105 +10,119 @@ use App\Models\LicenseSeat; use App\Models\Location; use App\Models\Setting; use App\Notifications\AuditNotification; +use Illuminate\Database\Eloquent\Model; trait Loggable { // an attribute for setting whether or not the item was imported public ?bool $imported = false; + private ?string $log_message = null; + private ?Model $item = null; + private array $log_meta = []; + private ?Model $target = null; + private ?string $note = null; + + private ?Location $location = null; + + static array $hide_changes = []; public static function bootLoggable() { \Log::error("LOGGABLE IS BOOTING!!!!!!!!!!!"); - - /** - * Listen to the Asset updating event. This fires automatically every time an existing asset is saved. - * - * @param Asset $asset - * @return void - */ - static::saving(function ($model) { - $attributes = $model->getAttributes(); - $attributesOriginal = $model->getRawOriginal(); - $same_checkout_counter = false; - $same_checkin_counter = false; - $restoring_or_deleting = false; - - // This is a gross hack to prevent the double logging when restoring an asset - if (array_key_exists('deleted_at', $attributes) && array_key_exists('deleted_at', $attributesOriginal)) { - $restoring_or_deleting = (($attributes['deleted_at'] != $attributesOriginal['deleted_at'])); - } - - if (array_key_exists('checkout_counter', $attributes) && array_key_exists('checkout_counter', $attributesOriginal)) { - $same_checkout_counter = (($attributes['checkout_counter'] == $attributesOriginal['checkout_counter'])); - } - - if (array_key_exists('checkin_counter', $attributes) && array_key_exists('checkin_counter', $attributesOriginal)) { - $same_checkin_counter = (($attributes['checkin_counter'] == $attributesOriginal['checkin_counter'])); - } - - // If the asset isn't being checked out or audited, log the update. - // (Those other actions already create log entries.) - if (($attributes['assigned_to'] == $attributesOriginal['assigned_to']) - && ($same_checkout_counter) && ($same_checkin_counter) - && ((isset($attributes['next_audit_date']) ? $attributes['next_audit_date'] : null) == (isset($attributesOriginal['next_audit_date']) ? $attributesOriginal['next_audit_date'] : null)) - && ($attributes['last_checkout'] == $attributesOriginal['last_checkout']) && (!$restoring_or_deleting)) { - $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 (empty($changed)) { - return; - } - - $logAction = new Actionlog(); - $logAction->item_type = self::class; - $logAction->item_id = $model->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->log_meta = json_encode($changed); - $logAction->logaction('update'); - } - }); + //these tiny methods just set up what the log message is going to be static::updating(function ($model) { - + $model->logMessage('update'); + }); + + static::creating(function ($model) { + $model->logMessage('create'); }); - /** - * Listen to the Asset deleting event. - * - * @param Asset $asset - * @return void - */ static::deleting(function ($model) { - $logAction = new Actionlog(); - $logAction->item_type = self::class; - $logAction->item_id = $model->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('delete'); + $model->logMessage("delete"); }); - /** - * Listen to the Asset deleting event. - * - * @param Asset $asset - * @return void - */ static::restoring(function ($model) { - $logAction = new Actionlog(); - $logAction->item_type = self::class; - $logAction->item_id = $model->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('restore'); - + $model->logMessage("restore"); }); + + // 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. + static::saving(function ($model) { + //possibly consider a "$this->saveWithoutTransaction" thing you can invoke? + // use "BEGIN" here?! TODO FIXME + + 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'] = '*************'; + } + } + + $this->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) { + $model->logWithoutSave(); + // DO COMMIT HERE? TODO FIXME + }); + + } + + // and THIS is the main, primary logging system + // it *can* be called on its own, but in *general* you should let it trigger from the 'save' + public function logWithoutSave(ActionType $logaction = null) + { + if ($logaction) { + $this->setLogMessage($logaction); + } + $logAction = new Actionlog(); + $logAction->item_type = self::class; + $logAction->item_id = $this->id; + $logAction->created_at = date('Y-m-d H:i:s'); + $logAction->created_by = auth()->id(); + if ($this->imported) { + $logAction->setActionSource('importer'); + } + $logAction->log_meta = $this->log_meta ? json_encode($this->log_meta) : null; + if ($this->target) { + $logAction->target_type = $this->target::class; + $logAction->target_id = $this->target->id; + } + if ($this->note) { + $logAction->note = $this->note; + } + if ($this->location) { + $logAction->location_id = $this->location->id; + } + $logAction->logaction($this->log_message); + } + + public function setLogMessage(ActionType $message) + { + $this->log_message = $message->value; + } + + public function setLogMeta(array $changed) + { + $this->log_meta = $changed; + } + + public function setTarget(Model $target) + { + $this->target = $target; + } + + public function setNote(string $note) + { + $this->note = $note; } /** diff --git a/app/Models/User.php b/app/Models/User.php index fac4e69e47..018a10aac2 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -38,6 +38,8 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo protected $table = 'users'; protected $injectUniqueIdentifier = true; + public static array $hide_changes = ['password', 'remember_token', 'two_factor_secret', 'reset_password_code']; + protected $fillable = [ 'activated', 'address', diff --git a/app/Observers/AccessoryObserver.php b/app/Observers/AccessoryObserver.php deleted file mode 100644 index 0f8b2492cd..0000000000 --- a/app/Observers/AccessoryObserver.php +++ /dev/null @@ -1,62 +0,0 @@ -item_type = Accessory::class; - $logAction->item_id = $accessory->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('update'); - } - - /** - * Listen to the Accessory created event when - * a new accessory is created. - * - * @param Accessory $accessory - * @return void - */ - public function created(Accessory $accessory) - { - $logAction = new Actionlog(); - $logAction->item_type = Accessory::class; - $logAction->item_id = $accessory->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - if($accessory->imported) { - $logAction->setActionSource('importer'); - } - $logAction->logaction('create'); - } - - /** - * Listen to the Accessory deleting event. - * - * @param Accessory $accessory - * @return void - */ - public function deleting(Accessory $accessory) - { - $logAction = new Actionlog(); - $logAction->item_type = Accessory::class; - $logAction->item_id = $accessory->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('delete'); - } -} diff --git a/app/Observers/AssetObserver.php b/app/Observers/AssetObserver.php deleted file mode 100644 index 9a51f65e27..0000000000 --- a/app/Observers/AssetObserver.php +++ /dev/null @@ -1,100 +0,0 @@ -asset_tag; - $prefix = (string)($settings->auto_increment_prefix ?? ''); - $number = substr($tag, strlen($prefix)); - // IF - auto_increment_assets is on, AND (there is no prefix OR the prefix matches the start of the tag) - // AND the rest of the string after the prefix is all digits, THEN... - if ($settings->auto_increment_assets && ($prefix=='' || strpos($tag, $prefix) === 0) && 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 && $next_asset_tag < PHP_INT_MAX) { - $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; // can we instead say $logAction->item = $asset ? - $logAction->item_id = $asset->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - if($asset->imported) { - $logAction->setActionSource('importer'); - } - $logAction->logaction('create'); - } - - - - /** - * Executes every time an asset is saved. - * - * This matters specifically because any database fields affected here MUST already exist on - * the assets table (and/or any related models), or related migrations WILL fail. - * - * For example, if there is a database migration that's a bit older and modifies an asset, if the save - * fires before a field gets created in a later migration and that field in the later migration - * is used in this observer, it doesn't actually exist yet and the migration will break unless we - * use saveQuietly() in the migration which skips this observer. - * - * @see https://github.com/snipe/snipe-it/issues/13723#issuecomment-1761315938 - */ - public function saving(Asset $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)){ - $asset->asset_eol_date = $asset->purchase_date->addMonths($asset->model->eol)->format('Y-m-d'); - $asset->eol_explicit = false; - } - - // determine if explicit and set eol_explicit to true - if (!is_null($asset->asset_eol_date) && !is_null($asset->purchase_date)) { - if($asset->model->eol > 0) { - $months = Carbon::parse($asset->asset_eol_date)->diffInMonths($asset->purchase_date); - if($months != $asset->model->eol) { - $asset->eol_explicit = true; - } - } - } elseif (!is_null($asset->asset_eol_date) && is_null($asset->purchase_date)) { - $asset->eol_explicit = true; - } - if ((!is_null($asset->asset_eol_date)) && (!is_null($asset->purchase_date)) && (is_null($asset->model->eol) || ($asset->model->eol == 0))) { - $asset->eol_explicit = true; - } - - } -} diff --git a/app/Observers/ComponentObserver.php b/app/Observers/ComponentObserver.php deleted file mode 100644 index cd2c58c367..0000000000 --- a/app/Observers/ComponentObserver.php +++ /dev/null @@ -1,62 +0,0 @@ -item_type = Component::class; - $logAction->item_id = $component->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('update'); - } - - /** - * Listen to the Component created event when - * a new component is created. - * - * @param Component $component - * @return void - */ - public function created(Component $component) - { - $logAction = new Actionlog(); - $logAction->item_type = Component::class; - $logAction->item_id = $component->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - if($component->imported) { - $logAction->setActionSource('importer'); - } - $logAction->logaction('create'); - } - - /** - * Listen to the Component deleting event. - * - * @param Component $component - * @return void - */ - public function deleting(Component $component) - { - $logAction = new Actionlog(); - $logAction->item_type = Component::class; - $logAction->item_id = $component->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('delete'); - } -} diff --git a/app/Observers/ConsumableObserver.php b/app/Observers/ConsumableObserver.php deleted file mode 100644 index 57471cee9c..0000000000 --- a/app/Observers/ConsumableObserver.php +++ /dev/null @@ -1,104 +0,0 @@ -getRawOriginal() as $key => $value) { - // Check and see if the value changed - if ($consumable->getRawOriginal()[$key] != $consumable->getAttributes()[$key]) { - $changed[$key]['old'] = $consumable->getRawOriginal()[$key]; - $changed[$key]['new'] = $consumable->getAttributes()[$key]; - } - } - - if (count($changed) > 0) { - $logAction = new Actionlog(); - $logAction->item_type = Consumable::class; - $logAction->item_id = $consumable->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->log_meta = json_encode($changed); - $logAction->logaction('update'); - } - } - - /** - * Listen to the Consumable created event when - * a new consumable is created. - * - * @param Consumable $consumable - * @return void - */ - public function created(Consumable $consumable) - { - $logAction = new Actionlog(); - $logAction->item_type = Consumable::class; - $logAction->item_id = $consumable->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - if($consumable->imported) { - $logAction->setActionSource('importer'); - } - $logAction->logaction('create'); - } - - /** - * Listen to the Consumable deleting event. - * - * @param Consumable $consumable - * @return void - */ - public function deleting(Consumable $consumable) - { - - $consumable->users()->detach(); - $uploads = $consumable->uploads; - - foreach ($uploads as $file) { - try { - Storage::delete('private_uploads/consumables/'.$file->filename); - $file->delete(); - } catch (\Exception $e) { - Log::info($e); - } - } - - - - try { - Storage::disk('public')->delete('consumables/'.$consumable->image); - } catch (\Exception $e) { - Log::info($e); - } - - $consumable->image = null; - $consumable->save(); - - - - $logAction = new Actionlog(); - $logAction->item_type = Consumable::class; - $logAction->item_id = $consumable->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('delete'); - } -} diff --git a/app/Observers/LicenseObserver.php b/app/Observers/LicenseObserver.php deleted file mode 100644 index 4e355bf639..0000000000 --- a/app/Observers/LicenseObserver.php +++ /dev/null @@ -1,62 +0,0 @@ -item_type = License::class; - $logAction->item_id = $license->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('update'); - } - - /** - * Listen to the License created event when - * a new license is created. - * - * @param License $license - * @return void - */ - public function created(License $license) - { - $logAction = new Actionlog(); - $logAction->item_type = License::class; - $logAction->item_id = $license->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - if($license->imported) { - $logAction->setActionSource('importer'); - } - $logAction->logaction('create'); - } - - /** - * Listen to the License deleting event. - * - * @param License $license - * @return void - */ - public function deleting(License $license) - { - $logAction = new Actionlog(); - $logAction->item_type = License::class; - $logAction->item_id = $license->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('delete'); - } -} diff --git a/app/Observers/SettingObserver.php b/app/Observers/SettingObserver.php deleted file mode 100644 index ec9dec3f23..0000000000 --- a/app/Observers/SettingObserver.php +++ /dev/null @@ -1,21 +0,0 @@ -getRawOriginal() as $key => $value) { - - // Make sure the info is in the allow fields array - if (in_array($key, $allowed_fields)) { - - // Check and see if the value changed - if ($user->getRawOriginal()[$key] != $user->getAttributes()[$key]) { - - $changed[$key]['old'] = $user->getRawOriginal()[$key]; - $changed[$key]['new'] = $user->getAttributes()[$key]; - - // Do not store the hashed password in changes - if ($key == 'password') { - $changed['password']['old'] = '*************'; - $changed['password']['new'] = '*************'; - } - - } - } - - } - - if (count($changed) > 0) { - $logAction = new Actionlog(); - $logAction->item_type = User::class; - $logAction->item_id = $user->id; - $logAction->target_type = User::class; // can we instead say $logAction->item = $asset ? - $logAction->target_id = $user->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->log_meta = json_encode($changed); - $logAction->logaction('update'); - } - - - } - - /** - * Listen to the User created event, and increment - * the next_auto_tag_base value in the settings table when i - * a new asset is created. - * - * @param User $user - * @return void - */ - public function created(User $user) - { - $logAction = new Actionlog(); - $logAction->item_type = User::class; // can we instead say $logAction->item = $asset ? - $logAction->item_id = $user->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('create'); - } - - /** - * Listen to the User deleting event. - * - * @param User $user - * @return void - */ - public function deleting(User $user) - { - $logAction = new Actionlog(); - $logAction->item_type = User::class; - $logAction->item_id = $user->id; - $logAction->target_type = User::class; // can we instead say $logAction->item = $asset ? - $logAction->target_id = $user->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('delete'); - } - - /** - * Listen to the User deleting event. - * - * @param User $user - * @return void - */ - public function restoring(User $user) - { - $logAction = new Actionlog(); - $logAction->item_type = User::class; - $logAction->item_id = $user->id; - $logAction->target_type = User::class; // can we instead say $logAction->item = $asset ? - $logAction->target_id = $user->id; - $logAction->created_at = date('Y-m-d H:i:s'); - $logAction->created_by = auth()->id(); - $logAction->logaction('restore'); - } - - -} From 908db64a83c12cf3e8ba9c0cc5bd139f6eaf7628 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 7 Nov 2024 16:56:29 +0000 Subject: [PATCH 3/8] Yank observers from AppServiceProvider --- app/Providers/AppServiceProvider.php | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index aa2604bce5..55fa7da078 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -2,21 +2,7 @@ namespace App\Providers; -use App\Models\Accessory; -use App\Models\Asset; -use App\Models\Component; -use App\Models\Consumable; -use App\Models\License; -use App\Models\User; -use App\Models\Setting; use App\Models\SnipeSCIMConfig; -use App\Observers\AccessoryObserver; -use App\Observers\AssetObserver; -use App\Observers\UserObserver; -use App\Observers\ComponentObserver; -use App\Observers\ConsumableObserver; -use App\Observers\LicenseObserver; -use App\Observers\SettingObserver; use Illuminate\Routing\UrlGenerator; use Illuminate\Support\Facades\Schema; use Illuminate\Support\ServiceProvider; @@ -66,13 +52,6 @@ class AppServiceProvider extends ServiceProvider \Illuminate\Pagination\Paginator::useBootstrap(); Schema::defaultStringLength(191); - Asset::observe(AssetObserver::class); - User::observe(UserObserver::class); - Accessory::observe(AccessoryObserver::class); - Component::observe(ComponentObserver::class); - Consumable::observe(ConsumableObserver::class); - License::observe(LicenseObserver::class); - Setting::observe(SettingObserver::class); } /** From 4301029fcead3ea866e42a251d3000bb5a0050a9 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 7 Nov 2024 20:23:42 +0000 Subject: [PATCH 4/8] Only 3 tests left to go! --- app/Enums/ActionType.php | 3 + app/Http/Controllers/Api/UsersController.php | 2 +- .../Controllers/Users/BulkUsersController.php | 30 +++++--- app/Models/Asset.php | 7 +- app/Models/Traits/Loggable.php | 72 ++++++++++++++----- app/Models/User.php | 6 +- .../Assets/Ui/BulkDeleteAssetsTest.php | 1 + .../Importing/Api/ImportAssetsTest.php | 1 + 8 files changed, 90 insertions(+), 32 deletions(-) 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); From 88d214ea8882827ad09320a374efde9c4a19d687 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 7 Nov 2024 22:14:19 +0000 Subject: [PATCH 5/8] All but one test passing --- .../Controllers/Users/BulkUsersController.php | 7 +++++-- app/Listeners/LogListener.php | 4 ++-- app/Models/Asset.php | 6 +++--- app/Models/Traits/Loggable.php | 20 ++++++++++++------- .../Ui/BulkActions/BulkDeleteUsersTest.php | 9 +++++---- tests/Feature/Users/Ui/MergeUsersTest.php | 11 ++++++---- 6 files changed, 35 insertions(+), 22 deletions(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index c56c914f93..0790e64211 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -457,9 +457,12 @@ class BulkUsersController extends Controller $managedLocation->save(); } - $user_to_merge->delete(); + $user_to_merge->setNote('Deleting user cuz he was MERGED'); + $user_to_merge->delete(); //BELETED! + \Log::error("User has been DELETED!!!!!!!!!!!"); + \Log::error("User's action log is: ".print_r($user_to_merge->userlog()->get()->toArray(), true)); - event(new UserMerged($user_to_merge, $merge_into_user, $admin)); + event(new UserMerged($user_to_merge, $merge_into_user, $admin)); //HATE } diff --git a/app/Listeners/LogListener.php b/app/Listeners/LogListener.php index 27a9168484..972e27d496 100644 --- a/app/Listeners/LogListener.php +++ b/app/Listeners/LogListener.php @@ -110,7 +110,7 @@ class LogListener $logaction->target_id = $event->merged_to->id; $logaction->target_type = User::class; $logaction->action_type = 'merged'; - $logaction->note = trans('general.merged_log_this_user_from', $to_from_array); + $logaction->note = trans('general.merged_log_this_user_from', $to_from_array); //didnt...work? Or deleted? $logaction->created_by = $event->admin->id ?? null; $logaction->save(); @@ -121,7 +121,7 @@ class LogListener $logaction->item_id = $event->merged_to->id; $logaction->item_type = User::class; $logaction->action_type = 'merged'; - $logaction->note = trans('general.merged_log_this_user_into', $to_from_array); + $logaction->note = trans('general.merged_log_this_user_into', $to_from_array); //worked? $logaction->created_by = $event->admin->id ?? null; $logaction->save(); diff --git a/app/Models/Asset.php b/app/Models/Asset.php index d03aa5b4a8..bcbdb1e986 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -403,7 +403,7 @@ class Asset extends Depreciable $this->last_checkout = $checkout_at; $this->name = $name; - $this->assignedTo()->associate($target); + $this->assignedTo()->associate($target); //THIS is causing the save? if ($location != null) { $this->location_id = $location; @@ -423,7 +423,7 @@ class Asset extends Depreciable $originalValues['action_date'] = date('Y-m-d H:i:s'); } - if ($this->save()) { + if ($this->saveQuietly()) { //THIS is the save that fires that's making the update FIXME - on checkout, this causes an update. if (is_int($admin)) { $checkedOutBy = User::findOrFail($admin); } elseif ($admin && get_class($admin) === \App\Models\User::class) { @@ -431,7 +431,7 @@ class Asset extends Depreciable } else { $checkedOutBy = auth()->user(); } - event(new CheckoutableCheckedOut($this, $target, $checkedOutBy, $note, $originalValues)); + event(new CheckoutableCheckedOut($this, $target, $checkedOutBy, $note, $originalValues)); //THIS is probably causing the checkout? $this->increment('checkout_counter', 1); diff --git a/app/Models/Traits/Loggable.php b/app/Models/Traits/Loggable.php index 862547bbc0..72c8907363 100644 --- a/app/Models/Traits/Loggable.php +++ b/app/Models/Traits/Loggable.php @@ -49,7 +49,10 @@ trait Loggable }); static::deleting(function ($model) { //TODO - is this only for 'hard' delete? Or soft? - \Log::error("DELETING TRIGGER HAS FIRED!!!!!!!!!!!!!!!"); + \Log::error("DELETING TRIGGER HAS FIRED!!!!!!!!!!!!!!! for id: ".$model->id." old log_message was: ".$model->log_message); + if (self::class == \App\Models\User::class) { //FIXME - Janky AF! + $model->setTarget($model); //FIXME - this makes *NO* sense!!!! + } $model->setLogMessage(ActionType::Delete); }); @@ -97,8 +100,9 @@ trait Loggable // 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? + \Log::error("Deleted callback has fired!!!!!!!!!!! I guess that means do stuff here? For id: ".$model->id); + $results = $model->logWithoutSave(); //TODO - if we do commits up there, we should do them here too? + \Log::error("result of logging without save? ".($results ? 'true' : 'false')); }); static::restored(function ($model) { \Log::error("RestorED callback firing."); @@ -114,10 +118,10 @@ trait Loggable // and THIS is the main, primary logging system // it *can* be called on its own, but in *general* you should let it trigger from the 'save' - public function logWithoutSave(ActionType $logaction = null) + public function logWithoutSave(ActionType $log_action = null): bool { - if ($logaction) { - $this->setLogMessage($logaction); + if ($log_action) { + $this->setLogMessage($log_action); } $logAction = new Actionlog(); $logAction->item_type = self::class; @@ -138,7 +142,9 @@ trait Loggable if ($this->location_override) { $logAction->location_id = $this->location->id; } - $logAction->logaction($this->log_message); + + \Log::error("Here is the logaction BEFORE we save it ($this->log_message)...".print_r($logAction->toArray(), true)); + return $logAction->logaction($this->log_message); } public function setLogMessage(ActionType $message) diff --git a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php index 46ba023c1b..8ca2ce731f 100644 --- a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -148,9 +148,9 @@ class BulkDeleteUsersTest extends TestCase // These assertions check against a bug where the wrong value from // consumables_users was being populated in action_logs.item_id. - $this->assertActionLogCheckInEntryFor($userA, $consumableA); - $this->assertActionLogCheckInEntryFor($userA, $consumableB); - $this->assertActionLogCheckInEntryFor($userC, $consumableA); + $this->assertActionLogCheckInEntryFor($userA, $consumableA, 'consumable A, user A'); + $this->assertActionLogCheckInEntryFor($userA, $consumableB, 'consumable B, userA'); + $this->assertActionLogCheckInEntryFor($userC, $consumableA, 'consumable A, user C'); } public function test_license_seats_can_be_bulk_checked_in() @@ -246,8 +246,9 @@ class BulkDeleteUsersTest extends TestCase } } - private function assertActionLogCheckInEntryFor(User $user, Model $model): void + private function assertActionLogCheckInEntryFor(User $user, Model $model, $message = null): void { + \Log::error("IF this fails the message is $message"); $this->assertDatabaseHas('action_logs', [ 'action_type' => 'checkin from', 'target_id' => $user->id, diff --git a/tests/Feature/Users/Ui/MergeUsersTest.php b/tests/Feature/Users/Ui/MergeUsersTest.php index a9ae11171b..a6679dab77 100644 --- a/tests/Feature/Users/Ui/MergeUsersTest.php +++ b/tests/Feature/Users/Ui/MergeUsersTest.php @@ -192,7 +192,7 @@ class MergeUsersTest extends TestCase $this->assertEquals(3, $user_to_merge_into->refresh()->userlog->count()); $response = $this->actingAs(User::factory()->editUsers()->viewUsers()->create()) - ->post(route('users.merge.save', $user1->id), + ->post(route('users.merge.save'/*, $user1->id*/), [ 'ids_to_merge' => [$user1->id, $user2->id], 'merge_into_id' => $user_to_merge_into->id @@ -203,9 +203,12 @@ class MergeUsersTest extends TestCase $this->followRedirects($response)->assertSee('Success'); // This needs to be 2 more than the otherwise expected because the merge action itself is logged for the two merging users - $this->assertEquals(11, $user_to_merge_into->refresh()->userlog->count()); - $this->assertEquals(2, $user1->refresh()->userlog->count()); - $this->assertEquals(2, $user2->refresh()->userlog->count()); + $this->assertEquals(11, $user_to_merge_into->refresh()->userlog->count()); // this one worked? + \Log::error("The userlog is: ".print_r($user1->refresh()->userlog->toArray(), true)); + $this->assertTrue($user1->refresh()->trashed(), "User 1 should be trashed and isn't!"); + $this->assertTrue($user2->refresh()->trashed(), "User 2 should be trashed and isn't!"); + $this->assertEquals(2, $user1->refresh()->userlog->count()); //this one did not - only one. + $this->assertEquals(2, $user2->refresh()->userlog->count()); //also did not (and not sure why) } From eccaaa7f91170a0598c039d85b0916586f4c1c0c Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Fri, 8 Nov 2024 16:05:23 +0000 Subject: [PATCH 6/8] All tests now pass. --- app/Http/Controllers/Users/BulkUsersController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index 0790e64211..6e6c8e475c 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -375,9 +375,10 @@ class BulkUsersController extends Controller private function logConsumablesCheckin(Collection $consumableUserRows): void { + \Log::error("Logging Consumables Checkin!"); foreach ($consumableUserRows as $consumableUserRow) { $consumable = Consumable::find($consumableUserRow->consumable_id); - $consumable->setTarget(auth()->user()); + $consumable->setTarget(User::find($consumableUserRow->assigned_to)); $consumable->setNote('Bulk checkin items'); $consumable->logWithoutSave(ActionType::CheckinFrom); } From 27ac7f772aa9e09232a2378bc59994c953a96022 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Wed, 13 Nov 2024 00:07:06 +0000 Subject: [PATCH 7/8] Cleanups, renames, clearing out unneeded changes, unused log messages --- .../Controllers/Users/BulkUsersController.php | 21 +++++++------------ app/Http/Controllers/ViewAssetsController.php | 4 ++-- app/Listeners/LogListener.php | 4 ++-- app/Models/License.php | 4 ++-- app/Models/Traits/Loggable.php | 8 +++---- .../Assets/Ui/BulkDeleteAssetsTest.php | 1 - .../Importing/Api/ImportAssetsTest.php | 1 - .../Ui/BulkActions/BulkDeleteUsersTest.php | 9 ++++---- tests/Feature/Users/Ui/MergeUsersTest.php | 9 ++++---- 9 files changed, 26 insertions(+), 35 deletions(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index 6e6c8e475c..9a99895bbb 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -335,7 +335,6 @@ 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 { @@ -354,10 +353,10 @@ class BulkUsersController extends Controller $assigned_to = User::find($item->assigned_to); } - $real_item->setTarget($assigned_to); // will this work?!!?!?!? + $real_item->setLogTarget($assigned_to); // will this work?!!?!?!? //$logAction->target_id = $item->assigned_to; //$logAction->target_type = User::class; - $real_item->setNote('Bulk checkin items'); + $real_item->setLogNote('Bulk checkin items'); $real_item->setLogMessage(ActionType::CheckinFrom); $real_item->logWithoutSave(ActionType::CheckinFrom); } @@ -367,19 +366,18 @@ class BulkUsersController extends Controller { foreach ($accessoryUserRows as $accessoryUserRow) { $accessory = Accessory::find($accessoryUserRow->accessory_id); - $accessory->setTarget(User::find($accessoryUserRow->assigned_to)); //FIXME - what if accessory was checked out to location? - $accessory->setNote('Bulk checkin items'); + $accessory->setLogTarget(User::find($accessoryUserRow->assigned_to)); //FIXME - what if accessory was checked out to location? + $accessory->setLogNote('Bulk checkin items'); $accessory->logWithoutSave(ActionType::CheckinFrom); } } private function logConsumablesCheckin(Collection $consumableUserRows): void { - \Log::error("Logging Consumables Checkin!"); foreach ($consumableUserRows as $consumableUserRow) { $consumable = Consumable::find($consumableUserRow->consumable_id); - $consumable->setTarget(User::find($consumableUserRow->assigned_to)); - $consumable->setNote('Bulk checkin items'); + $consumable->setLogTarget(User::find($consumableUserRow->assigned_to)); + $consumable->setLogNote('Bulk checkin items'); $consumable->logWithoutSave(ActionType::CheckinFrom); } } @@ -458,12 +456,9 @@ class BulkUsersController extends Controller $managedLocation->save(); } - $user_to_merge->setNote('Deleting user cuz he was MERGED'); - $user_to_merge->delete(); //BELETED! - \Log::error("User has been DELETED!!!!!!!!!!!"); - \Log::error("User's action log is: ".print_r($user_to_merge->userlog()->get()->toArray(), true)); + $user_to_merge->delete(); - event(new UserMerged($user_to_merge, $merge_into_user, $admin)); //HATE + event(new UserMerged($user_to_merge, $merge_into_user, $admin)); } diff --git a/app/Http/Controllers/ViewAssetsController.php b/app/Http/Controllers/ViewAssetsController.php index 87c84285b5..e0889aea43 100755 --- a/app/Http/Controllers/ViewAssetsController.php +++ b/app/Http/Controllers/ViewAssetsController.php @@ -103,7 +103,7 @@ class ViewAssetsController extends Controller $item->setLocation($user->location); } - $item->setTarget($user); + $item->setLogTarget($user); //$logaction->target_id = $data['user_id'] = auth()->id(); //$logaction->target_type = User::class; @@ -171,7 +171,7 @@ class ViewAssetsController extends Controller $data['requested_date'] = date('Y-m-d H:i:s'); $asset->setLocation = $user->location; - $asset->setTarget($user); + $asset->setLogTarget($user); $data['user_id'] = auth()->id(); // If it's already requested, cancel the request. diff --git a/app/Listeners/LogListener.php b/app/Listeners/LogListener.php index 972e27d496..27a9168484 100644 --- a/app/Listeners/LogListener.php +++ b/app/Listeners/LogListener.php @@ -110,7 +110,7 @@ class LogListener $logaction->target_id = $event->merged_to->id; $logaction->target_type = User::class; $logaction->action_type = 'merged'; - $logaction->note = trans('general.merged_log_this_user_from', $to_from_array); //didnt...work? Or deleted? + $logaction->note = trans('general.merged_log_this_user_from', $to_from_array); $logaction->created_by = $event->admin->id ?? null; $logaction->save(); @@ -121,7 +121,7 @@ class LogListener $logaction->item_id = $event->merged_to->id; $logaction->item_type = User::class; $logaction->action_type = 'merged'; - $logaction->note = trans('general.merged_log_this_user_into', $to_from_array); //worked? + $logaction->note = trans('general.merged_log_this_user_into', $to_from_array); $logaction->created_by = $event->admin->id ?? null; $logaction->save(); diff --git a/app/Models/License.php b/app/Models/License.php index 29d09ce527..b830a4d2ae 100755 --- a/app/Models/License.php +++ b/app/Models/License.php @@ -181,7 +181,7 @@ class License extends Depreciable $seatsAvailableForDelete->pop()->delete(); } // Log Deletion of seats. - $license->setNote("deleted {$change} seats"); + $license->setLogNote("deleted {$change} seats"); $license->logWithoutSave(ActionType::DeleteSeats); return true; @@ -208,7 +208,7 @@ class License extends Depreciable // On initial create, we shouldn't log the addition of seats. if ($license->id) { //Log the addition of license to the log. - $license->setNote("added {$change} seats"); + $license->setLogNote("added {$change} seats"); $license->logWithoutSave(ActionType::AddSeats); } diff --git a/app/Models/Traits/Loggable.php b/app/Models/Traits/Loggable.php index 72c8907363..94f397411b 100644 --- a/app/Models/Traits/Loggable.php +++ b/app/Models/Traits/Loggable.php @@ -16,7 +16,7 @@ trait Loggable { // an attribute for setting whether or not the item was imported public ?bool $imported = false; - private ?string $log_message = null; //FIXME - meant to be 'private'!!!!! + private ?string $log_message = null; private ?Model $item = null; private array $log_meta = []; private ?Model $target = null; @@ -51,7 +51,7 @@ trait Loggable static::deleting(function ($model) { //TODO - is this only for 'hard' delete? Or soft? \Log::error("DELETING TRIGGER HAS FIRED!!!!!!!!!!!!!!! for id: ".$model->id." old log_message was: ".$model->log_message); if (self::class == \App\Models\User::class) { //FIXME - Janky AF! - $model->setTarget($model); //FIXME - this makes *NO* sense!!!! + $model->setLogTarget($model); //FIXME - this makes *NO* sense!!!! } $model->setLogMessage(ActionType::Delete); }); @@ -157,12 +157,12 @@ trait Loggable $this->log_meta = $changed; } - public function setTarget(Model $target) + public function setLogTarget(Model $target) { $this->target = $target; } - public function setNote(string $note) + public function setLogNote(string $note) { $this->note = $note; } diff --git a/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php b/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php index b206592a2b..38c69f3b99 100644 --- a/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php +++ b/tests/Feature/Assets/Ui/BulkDeleteAssetsTest.php @@ -151,7 +151,6 @@ 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 21d6d358f5..0f54b22e92 100644 --- a/tests/Feature/Importing/Api/ImportAssetsTest.php +++ b/tests/Feature/Importing/Api/ImportAssetsTest.php @@ -82,7 +82,6 @@ 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); diff --git a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php index 8ca2ce731f..46ba023c1b 100644 --- a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -148,9 +148,9 @@ class BulkDeleteUsersTest extends TestCase // These assertions check against a bug where the wrong value from // consumables_users was being populated in action_logs.item_id. - $this->assertActionLogCheckInEntryFor($userA, $consumableA, 'consumable A, user A'); - $this->assertActionLogCheckInEntryFor($userA, $consumableB, 'consumable B, userA'); - $this->assertActionLogCheckInEntryFor($userC, $consumableA, 'consumable A, user C'); + $this->assertActionLogCheckInEntryFor($userA, $consumableA); + $this->assertActionLogCheckInEntryFor($userA, $consumableB); + $this->assertActionLogCheckInEntryFor($userC, $consumableA); } public function test_license_seats_can_be_bulk_checked_in() @@ -246,9 +246,8 @@ class BulkDeleteUsersTest extends TestCase } } - private function assertActionLogCheckInEntryFor(User $user, Model $model, $message = null): void + private function assertActionLogCheckInEntryFor(User $user, Model $model): void { - \Log::error("IF this fails the message is $message"); $this->assertDatabaseHas('action_logs', [ 'action_type' => 'checkin from', 'target_id' => $user->id, diff --git a/tests/Feature/Users/Ui/MergeUsersTest.php b/tests/Feature/Users/Ui/MergeUsersTest.php index a6679dab77..7c8a1b9121 100644 --- a/tests/Feature/Users/Ui/MergeUsersTest.php +++ b/tests/Feature/Users/Ui/MergeUsersTest.php @@ -192,7 +192,7 @@ class MergeUsersTest extends TestCase $this->assertEquals(3, $user_to_merge_into->refresh()->userlog->count()); $response = $this->actingAs(User::factory()->editUsers()->viewUsers()->create()) - ->post(route('users.merge.save'/*, $user1->id*/), + ->post(route('users.merge.save', $user1->id), [ 'ids_to_merge' => [$user1->id, $user2->id], 'merge_into_id' => $user_to_merge_into->id @@ -203,12 +203,11 @@ class MergeUsersTest extends TestCase $this->followRedirects($response)->assertSee('Success'); // This needs to be 2 more than the otherwise expected because the merge action itself is logged for the two merging users - $this->assertEquals(11, $user_to_merge_into->refresh()->userlog->count()); // this one worked? - \Log::error("The userlog is: ".print_r($user1->refresh()->userlog->toArray(), true)); + $this->assertEquals(11, $user_to_merge_into->refresh()->userlog->count()); $this->assertTrue($user1->refresh()->trashed(), "User 1 should be trashed and isn't!"); $this->assertTrue($user2->refresh()->trashed(), "User 2 should be trashed and isn't!"); - $this->assertEquals(2, $user1->refresh()->userlog->count()); //this one did not - only one. - $this->assertEquals(2, $user2->refresh()->userlog->count()); //also did not (and not sure why) + $this->assertEquals(2, $user1->refresh()->userlog->count()); + $this->assertEquals(2, $user2->refresh()->userlog->count()); } From 23c4d82c92ffe7c9e614dc4258857dd36c2e8d13 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Sun, 23 Feb 2025 12:44:12 +0000 Subject: [PATCH 8/8] Cleanup, removing unnecessary log types on restores --- app/Http/Controllers/Api/ManufacturersController.php | 1 - app/Http/Controllers/Api/UsersController.php | 1 - app/Http/Controllers/AssetModelsController.php | 1 - app/Http/Controllers/LocationsController.php | 1 - app/Http/Controllers/ManufacturersController.php | 1 - 5 files changed, 5 deletions(-) diff --git a/app/Http/Controllers/Api/ManufacturersController.php b/app/Http/Controllers/Api/ManufacturersController.php index ef2ba23251..5dd3c19435 100644 --- a/app/Http/Controllers/Api/ManufacturersController.php +++ b/app/Http/Controllers/Api/ManufacturersController.php @@ -217,7 +217,6 @@ class ManufacturersController extends Controller return response()->json(Helper::formatStandardApiResponse('error', trans('general.not_deleted', ['item_type' => trans('general.manufacturer')])), 200); } - $manufacturer->setLogMessage('restore'); if ($manufacturer->restore()) { return response()->json(Helper::formatStandardApiResponse('success', trans('admin/manufacturers/message.restore.success')), 200); diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index 2ad306ec5d..af6494dfa2 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -746,7 +746,6 @@ class UsersController extends Controller return response()->json(Helper::formatStandardApiResponse('error', trans('general.not_deleted', ['item_type' => trans('general.user')])), 200); } - $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/AssetModelsController.php b/app/Http/Controllers/AssetModelsController.php index 0790bd8b8e..65e6b04070 100755 --- a/app/Http/Controllers/AssetModelsController.php +++ b/app/Http/Controllers/AssetModelsController.php @@ -225,7 +225,6 @@ class AssetModelsController extends Controller return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.asset_model')])); } - $model->setLogMessage(ActionType::Restore); if ($model->restore()) { // Redirect them to the deleted page if there are more, otherwise the section index diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index fbce721f39..7618c21536 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/app/Http/Controllers/LocationsController.php @@ -266,7 +266,6 @@ class LocationsController extends Controller return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.location')])); } - $location->setLogMessage(ActionType::Restore); if ($location->restore()) { return redirect()->route('locations.index')->with('success', trans('admin/locations/message.restore.success')); diff --git a/app/Http/Controllers/ManufacturersController.php b/app/Http/Controllers/ManufacturersController.php index 24266c9633..cdd8c77987 100755 --- a/app/Http/Controllers/ManufacturersController.php +++ b/app/Http/Controllers/ManufacturersController.php @@ -195,7 +195,6 @@ class ManufacturersController extends Controller return redirect()->back()->with('error', trans('general.not_deleted', ['item_type' => trans('general.manufacturer')])); } - $manufacturer->setLogMessage(ActionType::Restore); if ($manufacturer->restore()) { // Redirect them to the deleted page if there are more, otherwise the section index