From 10d4c4b92e338a7f93de5e23c6393d891d76ab46 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 23 Oct 2024 12:55:47 -0500 Subject: [PATCH 01/42] just a little scaffolding --- app/Actions/Assets/StoreAssetAction.php | 12 ++++++++++++ app/Http/Controllers/Assets/AssetsController.php | 15 ++++++++++++--- app/Http/Requests/StoreAssetRequest.php | 8 ++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 app/Actions/Assets/StoreAssetAction.php diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php new file mode 100644 index 000000000000..6c9c95c269e3 --- /dev/null +++ b/app/Actions/Assets/StoreAssetAction.php @@ -0,0 +1,12 @@ +] * @since [v1.0] */ - public function store(ImageUploadRequest $request) : RedirectResponse + public function store(StoreAssetRequest $request): RedirectResponse { - $this->authorize(Asset::class); - + try { + StoreAssetAction::run($request->validated()); + } catch (\Exception $e) { + return back()->with('error', $e->getMessage()); + } // There are a lot more rules to add here but prevents // errors around `asset_tags` not being present below. + + // so do we want to foreach over the action, or convert the api's asset tags to an array as well + // so we can just easily add it to the action? + // (obviously then this would move up to the request) $this->validate($request, ['asset_tags' => ['required', 'array']]); // Handle asset tags - there could be one, or potentially many. diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php index fb7469ac88f9..c34be2dbfa4f 100644 --- a/app/Http/Requests/StoreAssetRequest.php +++ b/app/Http/Requests/StoreAssetRequest.php @@ -36,6 +36,14 @@ public function prepareForValidation(): void $this->parseLastAuditDate(); + // maybe do something like this? + if (!is_array($this->asset_tag)) { + $this->asset_tag = [$this->asset_tag]; + } + if (!is_array($this->serial)) { + $this->serial = [$this->serial]; + } + $this->merge([ 'asset_tag' => $this->asset_tag ?? Asset::autoincrement_asset(), 'company_id' => $idForCurrentUser, From 6e8ab3723cc006c36ced1c848f7bf3cb380a0b7f Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 23 Oct 2024 13:56:01 -0500 Subject: [PATCH 02/42] bs --- app/Actions/Assets/StoreAssetAction.php | 9 ++++++++- app/Actions/BaseAction.php | 8 ++++++++ app/Http/Controllers/Assets/AssetsController.php | 6 +++++- 3 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 app/Actions/BaseAction.php diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index 6c9c95c269e3..b2e8187ca5ef 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -2,10 +2,17 @@ namespace App\Actions\Assets; -class StoreAssetAction +use App\Actions\BaseAction; +use App\Models\Setting; + +class StoreAssetAction extends BaseAction { public static function run($validatedData) { + $settings = Setting::getSettings(); + foreach ($validatedData['asset_tag'] as $key => $tag) { + + } } diff --git a/app/Actions/BaseAction.php b/app/Actions/BaseAction.php new file mode 100644 index 000000000000..10794ecbbb2d --- /dev/null +++ b/app/Actions/BaseAction.php @@ -0,0 +1,8 @@ +validated()); + DB::beginTransaction(); + foreach ($request->input('asset_tags') as $key => $tag) { + StoreAssetAction::run($request->validated(), $key); + } + DB::commit(); } catch (\Exception $e) { return back()->with('error', $e->getMessage()); } From eaa10249c2930348daf7f256be9c9d0735b620f6 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 29 Oct 2024 16:16:01 -0500 Subject: [PATCH 03/42] alright, this mostly works. --- app/Actions/Assets/StoreAssetAction.php | 117 +++++++++++++++++- app/Http/Controllers/Api/AssetsController.php | 36 ++++++ .../Controllers/Assets/AssetsController.php | 53 ++++++-- app/Http/Requests/StoreAssetRequest.php | 12 +- 4 files changed, 200 insertions(+), 18 deletions(-) diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index b2e8187ca5ef..af771924f601 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -3,17 +3,130 @@ namespace App\Actions\Assets; use App\Actions\BaseAction; +use App\Exceptions\CheckoutNotAllowed; +use App\Models\Asset; +use App\Models\AssetModel; +use App\Models\Company; +use App\Models\Location; use App\Models\Setting; +use App\Models\User; +use Carbon\Carbon; +use Illuminate\Support\Facades\Crypt; +use Illuminate\Support\Facades\Gate; +use Illuminate\Support\MessageBag; class StoreAssetAction extends BaseAction { - public static function run($validatedData) + public static function run( + $model_id,//gonna make these two optional for now... we can either make them required here or use the spread operator when calling... + $status_id,// + $name = null, + $serial = null, + $company_id = null, + $asset_tag = null, + $order_number = null, + $notes = null, + $user_id = null, + $warranty_months = null, + $purchase_cost = null, + $asset_eol_date = null, + $purchase_date = null, + $assigned_to = null, + $supplier_id = null, + $requestable = null, + $rtd_location_id = null, + $location_id = null, //do something with this + $files = null, + $byod = 0, + $assigned_user = null, + $assigned_asset = null, + $assigned_location = null, + $custom_fields = null, + $request = null, //temp for handleImages + $next_audit_date = null, + ) { $settings = Setting::getSettings(); - foreach ($validatedData['asset_tag'] as $key => $tag) { + // initial setting up of asset + $asset = new Asset(); + $asset->model()->associate(AssetModel::find($model_id)); + $asset->name = $name; + $asset->serial = $serial; + $asset->asset_tag = $asset_tag; + $asset->company_id = Company::getIdForCurrentUser($company_id); + $asset->model_id = $model_id; + $asset->order_number = $order_number; + $asset->notes = $notes; + $asset->created_by = auth()->id(); + $asset->status_id = $status_id; + $asset->warranty_months = $warranty_months; + $asset->purchase_cost = $purchase_cost; + $asset->purchase_date = $purchase_date; + $asset->asset_eol_date = $asset_eol_date; + $asset->assigned_to = $assigned_to; + $asset->supplier_id = $supplier_id; + $asset->requestable = $requestable; + $asset->rtd_location_id = $rtd_location_id; + $asset->byod = $byod; + + // set up next audit date + if (!empty($settings->audit_interval)) { + $asset->next_audit_date = Carbon::now()->addMonths($settings->audit_interval)->toDateString(); + } + + // Set location_id to rtd_location_id ONLY if the asset isn't being checked out + if (!$assigned_user && !$assigned_asset && !$assigned_location) { + $asset->location_id = $rtd_location_id; } + if ($request->has('image')) { + $asset = $request->handleImages($asset); + } + + $model = AssetModel::find($model_id); + if (($model) && ($model->fieldset)) { + foreach ($model->fieldset->fields as $field) { + if ($field->field_encrypted == '1') { + if (Gate::allows('assets.view.encrypted_custom_fields')) { + if (is_array($request->input($field->db_column))) { + $asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column))); + } else { + $asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column)); + } + } + } else { + if (is_array($request->input($field->db_column))) { + $asset->{$field->db_column} = implode(', ', $request->input($field->db_column)); + } else { + $asset->{$field->db_column} = $request->input($field->db_column); + } + } + } + } + + if ($asset->isValid() && $asset->save()) { + if (request('assigned_user')) { + $target = User::find(request('assigned_user')); + $location = $target->location_id; + } elseif (request('assigned_asset')) { + $target = Asset::find(request('assigned_asset')); + $location = $target->location_id; + } elseif (request('assigned_location')) { + $target = Location::find(request('assigned_location')); + $location = $target->id; + } + + if (isset($target)) { + $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), $request->input('expected_checkin', null), 'Checked out on asset creation', $request->get('name'), $location); + } + } + + if ($asset->save()) { + return $asset; + } else { + return $asset->getErrors(); + } } } \ No newline at end of file diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index d4a103be3732..d8488f3229ae 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Api; +use App\Actions\Assets\StoreAssetAction; use App\Events\CheckoutableCheckedIn; use App\Http\Requests\StoreAssetRequest; use App\Http\Requests\UpdateAssetRequest; @@ -594,6 +595,41 @@ public function selectlist(Request $request) : array */ public function store(StoreAssetRequest $request): JsonResponse { + $asset_tags = $request->input('asset_tags'); + $serials = $request->input('serials'); + $custom_fields = $request->collect()->filter(function ($value, $key) { + return starts_with($key, '_snipeit_'); + }); + + StoreAssetAction::run( + $request->validated('model_id'), + $request->validated('status_id'), + $request->validated('name'), + $serials[$key], + $request->validated('company_id'), + $asset_tag, + $request->validated('order_number'), + $request->validated('notes'), + $request->validated('user_id'), + $request->validated('warranty_months'), + $request->validated('purchase_cost'), + $request->validated('asset_eol_date'), + $request->validated('purchase_date'), + $request->validated('assigned_to'), + $request->validated('supplier_id'), + $request->validated('requestable'), + $request->validated('rtd_location_id'), + $request->validated('location_id'), + $request->validated('files'), + $request->validated('byod'), + $request->validated('assigned_user'), + $request->validated('assigned_asset'), + $request->validated('assigned_location'), + $custom_fields, + $request, + ); + + $asset = new Asset(); $asset->model()->associate(AssetModel::find((int) $request->get('model_id'))); diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 4f6374265db2..1479d11fd059 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -4,6 +4,7 @@ use App\Actions\Assets\StoreAssetAction; use App\Events\CheckoutableCheckedIn; +use App\Exceptions\CheckoutNotAllowed; use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Http\Requests\ImageUploadRequest; @@ -101,16 +102,50 @@ public function create(Request $request) : View public function store(StoreAssetRequest $request): RedirectResponse { try { - DB::beginTransaction(); - foreach ($request->input('asset_tags') as $key => $tag) { - StoreAssetAction::run($request->validated(), $key); + + $asset_tags = $request->input('asset_tags'); + $serials = $request->input('serials'); + $custom_fields = $request->collect()->filter(function ($value, $key) { + return starts_with($key, '_snipeit_'); + }); + //DB::transaction(function () use ($request, $asset_tags, $serials, $custom_fields) { + foreach ($asset_tags as $key => $asset_tag) { + StoreAssetAction::run( + $request->validated('model_id'), + $request->validated('status_id'), + $request->validated('name'), + $serials[$key], + $request->validated('company_id'), + $asset_tag, + $request->validated('order_number'), + $request->validated('notes'), + $request->validated('user_id'), + $request->validated('warranty_months'), + $request->validated('purchase_cost'), + $request->validated('asset_eol_date'), + $request->validated('purchase_date'), + $request->validated('assigned_to'), + $request->validated('supplier_id'), + $request->validated('requestable'), + $request->validated('rtd_location_id'), + $request->validated('location_id'), + $request->validated('files'), + $request->validated('byod'), + $request->validated('assigned_user'), + $request->validated('assigned_asset'), + $request->validated('assigned_location'), + $custom_fields, + $request, //this is just for the handleImages method... + ); } - DB::commit(); + //}); + return redirect()->route('hardware.index')->with('success', trans('admin/hardware/message.create.success')); + } catch (CheckoutNotAllowed $e) { + return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.create.error')); } catch (\Exception $e) { - return back()->with('error', $e->getMessage()); + return redirect()->back()->withInput()->withErrors($asset); } - // There are a lot more rules to add here but prevents - // errors around `asset_tags` not being present below. + // so do we want to foreach over the action, or convert the api's asset tags to an array as well // so we can just easily add it to the action? @@ -167,6 +202,7 @@ public function store(StoreAssetRequest $request): RedirectResponse } // Create the image (if one was chosen.) + //this one's interesting... if ($request->has('image')) { $asset = $request->handleImages($asset); } @@ -221,11 +257,8 @@ public function store(StoreAssetRequest $request): RedirectResponse if ($success) { - return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) ->with('success-unescaped', trans('admin/hardware/message.create.success_linked', ['link' => route('hardware.show', ['hardware' => $asset->id]), 'id', 'tag' => e($asset->asset_tag)])); - - } return redirect()->back()->withInput()->withErrors($asset->getErrors()); diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php index c34be2dbfa4f..8f013cb218a9 100644 --- a/app/Http/Requests/StoreAssetRequest.php +++ b/app/Http/Requests/StoreAssetRequest.php @@ -37,12 +37,12 @@ public function prepareForValidation(): void $this->parseLastAuditDate(); // maybe do something like this? - if (!is_array($this->asset_tag)) { - $this->asset_tag = [$this->asset_tag]; - } - if (!is_array($this->serial)) { - $this->serial = [$this->serial]; - } + //if (!is_array($this->asset_tag)) { + // $this->asset_tag = [$this->asset_tag]; + //} + //if (!is_array($this->serial)) { + // $this->serial = [$this->serial]; + //} $this->merge([ 'asset_tag' => $this->asset_tag ?? Asset::autoincrement_asset(), From 34886ee69ae03cdd673f0dc6769c9394290412f4 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 29 Oct 2024 19:04:57 -0500 Subject: [PATCH 04/42] almost wrapped up StoreAssetAction.php --- app/Actions/Assets/StoreAssetAction.php | 55 ++++++- app/Http/Controllers/Api/AssetsController.php | 148 +++++------------- .../Controllers/Assets/AssetsController.php | 124 +-------------- app/Http/Requests/StoreAssetRequest.php | 8 - 4 files changed, 96 insertions(+), 239 deletions(-) diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index af771924f601..b6ca0a900cd3 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -13,10 +13,14 @@ use Carbon\Carbon; use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Gate; +use Illuminate\Support\Facades\Log; use Illuminate\Support\MessageBag; class StoreAssetAction extends BaseAction { + /** + * @throws CheckoutNotAllowed + */ public static function run( $model_id,//gonna make these two optional for now... we can either make them required here or use the spread operator when calling... $status_id,// @@ -80,13 +84,19 @@ public static function run( $asset->location_id = $rtd_location_id; } + //api only + if ($request->has('image_source')) { + $request->offsetSet('image', $request->offsetGet('image_source')); + } + if ($request->has('image')) { $asset = $request->handleImages($asset); } $model = AssetModel::find($model_id); - if (($model) && ($model->fieldset)) { + // added instanceof, was only in api before + if (($model) && ($model instanceof AssetModel) && ($model->fieldset)) { foreach ($model->fieldset->fields as $field) { if ($field->field_encrypted == '1') { if (Gate::allows('assets.view.encrypted_custom_fields')) { @@ -106,9 +116,46 @@ public static function run( } } + // this is the api's custom fieldset logic, is there a real difference??????? + //if (($model) && ($model instanceof AssetModel) && ($model->fieldset)) { + // foreach ($model->fieldset->fields as $field) { + // + // // Set the field value based on what was sent in the request + // $field_val = $request->input($field->db_column, null); + // + // // If input value is null, use custom field's default value + // if ($field_val == null) { + // Log::debug('Field value for '.$field->db_column.' is null'); + // $field_val = $field->defaultValue($request->get('model_id')); + // Log::debug('Use the default fieldset value of '.$field->defaultValue($request->get('model_id'))); + // } + // + // // if the field is set to encrypted, make sure we encrypt the value + // if ($field->field_encrypted == '1') { + // Log::debug('This model field is encrypted in this fieldset.'); + // + // if (Gate::allows('assets.view.encrypted_custom_fields')) { + // + // // If input value is null, use custom field's default value + // if (($field_val == null) && ($request->has('model_id') != '')) { + // $field_val = Crypt::encrypt($field->defaultValue($request->get('model_id'))); + // } else { + // $field_val = Crypt::encrypt($request->input($field->db_column)); + // } + // } + // } + // if ($field->element == 'checkbox') { + // if (is_array($field_val)) { + // $field_val = implode(',', $field_val); + // } + // } + // } + + if ($asset->isValid() && $asset->save()) { if (request('assigned_user')) { $target = User::find(request('assigned_user')); + // the api doesn't have these location-y bits - good reason? $location = $target->location_id; } elseif (request('assigned_asset')) { $target = Asset::find(request('assigned_asset')); @@ -121,6 +168,12 @@ public static function run( if (isset($target)) { $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), $request->input('expected_checkin', null), 'Checked out on asset creation', $request->get('name'), $location); } + + //this was in api and not gui + if ($asset->image) { + $asset->image = $asset->getImageUrl(); + } + } if ($asset->save()) { diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index d8488f3229ae..c248624d98c7 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -4,6 +4,7 @@ use App\Actions\Assets\StoreAssetAction; use App\Events\CheckoutableCheckedIn; +use App\Exceptions\CheckoutNotAllowed; use App\Http\Requests\StoreAssetRequest; use App\Http\Requests\UpdateAssetRequest; use App\Http\Traits\MigratesLegacyAssetLocations; @@ -595,122 +596,47 @@ public function selectlist(Request $request) : array */ public function store(StoreAssetRequest $request): JsonResponse { - $asset_tags = $request->input('asset_tags'); - $serials = $request->input('serials'); - $custom_fields = $request->collect()->filter(function ($value, $key) { - return starts_with($key, '_snipeit_'); - }); - - StoreAssetAction::run( - $request->validated('model_id'), - $request->validated('status_id'), - $request->validated('name'), - $serials[$key], - $request->validated('company_id'), - $asset_tag, - $request->validated('order_number'), - $request->validated('notes'), - $request->validated('user_id'), - $request->validated('warranty_months'), - $request->validated('purchase_cost'), - $request->validated('asset_eol_date'), - $request->validated('purchase_date'), - $request->validated('assigned_to'), - $request->validated('supplier_id'), - $request->validated('requestable'), - $request->validated('rtd_location_id'), - $request->validated('location_id'), - $request->validated('files'), - $request->validated('byod'), - $request->validated('assigned_user'), - $request->validated('assigned_asset'), - $request->validated('assigned_location'), - $custom_fields, - $request, - ); - - - $asset = new Asset(); - $asset->model()->associate(AssetModel::find((int) $request->get('model_id'))); - - $asset->fill($request->validated()); - $asset->created_by = auth()->id(); - - /** - * this is here just legacy reasons. Api\AssetController - * used image_source once to allow encoded image uploads. - */ - if ($request->has('image_source')) { - $request->offsetSet('image', $request->offsetGet('image_source')); - } - - $asset = $request->handleImages($asset); - - // Update custom fields in the database. - $model = AssetModel::find($request->input('model_id')); - - // Check that it's an object and not a collection - // (Sometimes people send arrays here and they shouldn't - if (($model) && ($model instanceof AssetModel) && ($model->fieldset)) { - foreach ($model->fieldset->fields as $field) { - - // Set the field value based on what was sent in the request - $field_val = $request->input($field->db_column, null); - - // If input value is null, use custom field's default value - if ($field_val == null) { - Log::debug('Field value for '.$field->db_column.' is null'); - $field_val = $field->defaultValue($request->get('model_id')); - Log::debug('Use the default fieldset value of '.$field->defaultValue($request->get('model_id'))); - } - - // if the field is set to encrypted, make sure we encrypt the value - if ($field->field_encrypted == '1') { - Log::debug('This model field is encrypted in this fieldset.'); - - if (Gate::allows('assets.view.encrypted_custom_fields')) { - - // If input value is null, use custom field's default value - if (($field_val == null) && ($request->has('model_id') != '')) { - $field_val = Crypt::encrypt($field->defaultValue($request->get('model_id'))); - } else { - $field_val = Crypt::encrypt($request->input($field->db_column)); - } - } - } - if ($field->element == 'checkbox') { - if(is_array($field_val)) { - $field_val = implode(',', $field_val); - } - } - - - $asset->{$field->db_column} = $field_val; - } - } - - if ($asset->save()) { - if ($request->get('assigned_user')) { - $target = User::find(request('assigned_user')); - } elseif ($request->get('assigned_asset')) { - $target = Asset::find(request('assigned_asset')); - } elseif ($request->get('assigned_location')) { - $target = Location::find(request('assigned_location')); - } - if (isset($target)) { - $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset creation', e($request->get('name'))); - } - - if ($asset->image) { - $asset->image = $asset->getImageUrl(); - } + try { + $custom_fields = $request->collect()->filter(function ($value, $key) { + return starts_with($key, '_snipeit_'); + }); + $asset = StoreAssetAction::run( + $request->validated('model_id'), + $request->validated('status_id'), + $request->validated('name'), + $request->validated('serial'), + $request->validated('company_id'), + $request->validated('asset_tag'), + $request->validated('order_number'), + $request->validated('notes'), + $request->validated('user_id'), + $request->validated('warranty_months'), + $request->validated('purchase_cost'), + $request->validated('asset_eol_date'), + $request->validated('purchase_date'), + $request->validated('assigned_to'), + $request->validated('supplier_id'), + $request->validated('requestable'), + $request->validated('rtd_location_id'), + $request->validated('location_id'), + $request->validated('files'), + $request->validated('byod'), + $request->validated('assigned_user'), + $request->validated('assigned_asset'), + $request->validated('assigned_location'), + $custom_fields, + $request, + ); return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.create.success'))); + // not sure why we're not using this yet, but i know there's a reason and a reason we want to return response()->json(Helper::formatStandardApiResponse('success', (new AssetsTransformer)->transformAsset($asset), trans('admin/hardware/message.create.success'))); + } catch (CheckoutNotAllowed $e) { + return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage()), 200); + } catch (\Exception $e) { + return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage())); } - - return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 200); } diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 1479d11fd059..3ee352dc8928 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -102,7 +102,6 @@ public function create(Request $request) : View public function store(StoreAssetRequest $request): RedirectResponse { try { - $asset_tags = $request->input('asset_tags'); $serials = $request->input('serials'); $custom_fields = $request->collect()->filter(function ($value, $key) { @@ -110,7 +109,7 @@ public function store(StoreAssetRequest $request): RedirectResponse }); //DB::transaction(function () use ($request, $asset_tags, $serials, $custom_fields) { foreach ($asset_tags as $key => $asset_tag) { - StoreAssetAction::run( + $asset = StoreAssetAction::run( $request->validated('model_id'), $request->validated('status_id'), $request->validated('name'), @@ -139,129 +138,16 @@ public function store(StoreAssetRequest $request): RedirectResponse ); } //}); - return redirect()->route('hardware.index')->with('success', trans('admin/hardware/message.create.success')); + session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); + return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) + ->with('success-unescaped', trans('admin/hardware/message.create.success_linked', ['link' => route('hardware.show', ['hardware' => $asset->id]), 'id', 'tag' => e($asset->asset_tag)])); } catch (CheckoutNotAllowed $e) { return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.create.error')); } catch (\Exception $e) { - return redirect()->back()->withInput()->withErrors($asset); + return redirect()->back()->with('error', trans('admin/hardware/message.create.error')); } - - - // so do we want to foreach over the action, or convert the api's asset tags to an array as well - // so we can just easily add it to the action? // (obviously then this would move up to the request) $this->validate($request, ['asset_tags' => ['required', 'array']]); - - // Handle asset tags - there could be one, or potentially many. - // This is only necessary on create, not update, since bulk editing is handled - // differently - $asset_tags = $request->input('asset_tags'); - - $settings = Setting::getSettings(); - - $success = false; - $serials = $request->input('serials'); - - for ($a = 1; $a <= count($asset_tags); $a++) { - $asset = new Asset(); - $asset->model()->associate(AssetModel::find($request->input('model_id'))); - $asset->name = $request->input('name'); - - // Check for a corresponding serial - if (($serials) && (array_key_exists($a, $serials))) { - $asset->serial = $serials[$a]; - } - - if (($asset_tags) && (array_key_exists($a, $asset_tags))) { - $asset->asset_tag = $asset_tags[$a]; - } - - $asset->company_id = Company::getIdForCurrentUser($request->input('company_id')); - $asset->model_id = $request->input('model_id'); - $asset->order_number = $request->input('order_number'); - $asset->notes = $request->input('notes'); - $asset->created_by = auth()->id(); - $asset->status_id = request('status_id'); - $asset->warranty_months = request('warranty_months', null); - $asset->purchase_cost = request('purchase_cost'); - $asset->purchase_date = request('purchase_date', null); - $asset->asset_eol_date = request('asset_eol_date', null); - $asset->assigned_to = request('assigned_to', null); - $asset->supplier_id = request('supplier_id', null); - $asset->requestable = request('requestable', 0); - $asset->rtd_location_id = request('rtd_location_id', null); - $asset->byod = request('byod', 0); - - if (! empty($settings->audit_interval)) { - $asset->next_audit_date = Carbon::now()->addMonths($settings->audit_interval)->toDateString(); - } - - // Set location_id to rtd_location_id ONLY if the asset isn't being checked out - if (!request('assigned_user') && !request('assigned_asset') && !request('assigned_location')) { - $asset->location_id = $request->input('rtd_location_id', null); - } - - // Create the image (if one was chosen.) - //this one's interesting... - if ($request->has('image')) { - $asset = $request->handleImages($asset); - } - - // Update custom fields in the database. - // Validation for these fields is handled through the AssetRequest form request - $model = AssetModel::find($request->get('model_id')); - - if (($model) && ($model->fieldset)) { - foreach ($model->fieldset->fields as $field) { - if ($field->field_encrypted == '1') { - if (Gate::allows('assets.view.encrypted_custom_fields')) { - if (is_array($request->input($field->db_column))) { - $asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column))); - } else { - $asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column)); - } - } - } else { - if (is_array($request->input($field->db_column))) { - $asset->{$field->db_column} = implode(', ', $request->input($field->db_column)); - } else { - $asset->{$field->db_column} = $request->input($field->db_column); - } - } - } - } - - // Validate the asset before saving - if ($asset->isValid() && $asset->save()) { - if (request('assigned_user')) { - $target = User::find(request('assigned_user')); - $location = $target->location_id; - } elseif (request('assigned_asset')) { - $target = Asset::find(request('assigned_asset')); - $location = $target->location_id; - } elseif (request('assigned_location')) { - $target = Location::find(request('assigned_location')); - $location = $target->id; - } - - if (isset($target)) { - $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), $request->input('expected_checkin', null), 'Checked out on asset creation', $request->get('name'), $location); - } - - $success = true; - - } - } - - session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); - - - if ($success) { - return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) - ->with('success-unescaped', trans('admin/hardware/message.create.success_linked', ['link' => route('hardware.show', ['hardware' => $asset->id]), 'id', 'tag' => e($asset->asset_tag)])); - } - - return redirect()->back()->withInput()->withErrors($asset->getErrors()); } diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php index 8f013cb218a9..fb7469ac88f9 100644 --- a/app/Http/Requests/StoreAssetRequest.php +++ b/app/Http/Requests/StoreAssetRequest.php @@ -36,14 +36,6 @@ public function prepareForValidation(): void $this->parseLastAuditDate(); - // maybe do something like this? - //if (!is_array($this->asset_tag)) { - // $this->asset_tag = [$this->asset_tag]; - //} - //if (!is_array($this->serial)) { - // $this->serial = [$this->serial]; - //} - $this->merge([ 'asset_tag' => $this->asset_tag ?? Asset::autoincrement_asset(), 'company_id' => $idForCurrentUser, From 698434dcf861c32825fb7cbdf9de212381f68cf9 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 30 Oct 2024 13:48:14 -0500 Subject: [PATCH 05/42] no more base action, scaffolded update and delete --- app/Actions/Assets/DestroyAssetAction.php | 12 ++++++++++++ app/Actions/Assets/StoreAssetAction.php | 5 ++--- app/Actions/Assets/UpdateAssetAction.php | 12 ++++++++++++ app/Actions/BaseAction.php | 8 -------- app/Http/Controllers/Api/AssetsController.php | 4 ++-- app/Http/Controllers/Assets/AssetsController.php | 3 ++- app/Http/Requests/{ => Assets}/StoreAssetRequest.php | 3 ++- .../Requests/{ => Assets}/UpdateAssetRequest.php | 3 ++- 8 files changed, 34 insertions(+), 16 deletions(-) create mode 100644 app/Actions/Assets/DestroyAssetAction.php create mode 100644 app/Actions/Assets/UpdateAssetAction.php delete mode 100644 app/Actions/BaseAction.php rename app/Http/Requests/{ => Assets}/StoreAssetRequest.php (97%) rename app/Http/Requests/{ => Assets}/UpdateAssetRequest.php (95%) diff --git a/app/Actions/Assets/DestroyAssetAction.php b/app/Actions/Assets/DestroyAssetAction.php new file mode 100644 index 000000000000..46b5f20c0bfc --- /dev/null +++ b/app/Actions/Assets/DestroyAssetAction.php @@ -0,0 +1,12 @@ +collect()->filter(function ($value, $key) { return starts_with($key, '_snipeit_'); }); + //DB::transaction(function () use ($request, $asset_tags, $serials, $custom_fields) { foreach ($asset_tags as $key => $asset_tag) { $asset = StoreAssetAction::run( diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/Assets/StoreAssetRequest.php similarity index 97% rename from app/Http/Requests/StoreAssetRequest.php rename to app/Http/Requests/Assets/StoreAssetRequest.php index fb7469ac88f9..574216afdf50 100644 --- a/app/Http/Requests/StoreAssetRequest.php +++ b/app/Http/Requests/Assets/StoreAssetRequest.php @@ -1,7 +1,8 @@ Date: Wed, 30 Oct 2024 15:19:34 -0500 Subject: [PATCH 06/42] delete done --- app/Actions/Assets/DestroyAssetAction.php | 28 ++++++++++++- app/Exceptions/Handler.php | 9 ++++ app/Http/Controllers/Api/AssetsController.php | 31 +++++--------- .../Controllers/Assets/AssetsController.php | 42 +++++-------------- .../Requests/Assets/DestroyAssetRequest.php | 29 +++++++++++++ routes/api.php | 5 ++- routes/web/hardware.php | 3 ++ 7 files changed, 92 insertions(+), 55 deletions(-) create mode 100644 app/Http/Requests/Assets/DestroyAssetRequest.php diff --git a/app/Actions/Assets/DestroyAssetAction.php b/app/Actions/Assets/DestroyAssetAction.php index 46b5f20c0bfc..095f703e62c3 100644 --- a/app/Actions/Assets/DestroyAssetAction.php +++ b/app/Actions/Assets/DestroyAssetAction.php @@ -2,11 +2,37 @@ namespace App\Actions\Assets; +use App\Events\CheckoutableCheckedIn; +use App\Models\Asset; +use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Storage; + class DestroyAssetAction { - public static function run() + public static function run(Asset $asset) { + if ($asset->assignedTo) { + + $target = $asset->assignedTo; + $checkin_at = date('Y-m-d H:i:s'); + $originalValues = $asset->getRawOriginal(); + event(new CheckoutableCheckedIn($asset, $target, auth()->user(), 'Checkin on delete', $checkin_at, $originalValues)); + DB::table('assets') + ->where('id', $asset->id) + ->update(['assigned_to' => null]); + } + + + if ($asset->image) { + try { + Storage::disk('public')->delete('assets'.'/'.$asset->image); + } catch (\Exception $e) { + Log::debug($e); + } + } + $asset->delete(); } } \ No newline at end of file diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 2b8eaa362744..4b98391b3125 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -2,6 +2,7 @@ namespace App\Exceptions; +use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler; use App\Helpers\Helper; use Illuminate\Validation\ValidationException; @@ -73,6 +74,14 @@ public function render($request, Throwable $e) return response()->json(Helper::formatStandardApiResponse('error', null, 'Invalid JSON'), 422); } + //if ($e instanceof ModelNotFoundException) { + // $class = get_class($e); + // match($class) { + // + // }; + // return redirect()->route()->with('error', trans('general.model_not_found', ['model' => $e])); + //} + // Handle SCIM exceptions if ($e instanceof SCIMException) { try { diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index aad21f01aa00..09de4fbccdea 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Api; +use App\Actions\Assets\DestroyAssetAction; use App\Actions\Assets\StoreAssetAction; use App\Events\CheckoutableCheckedIn; use App\Exceptions\CheckoutNotAllowed; @@ -735,30 +736,18 @@ public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse * @param int $assetId * @since [v4.0] */ - public function destroy($id) : JsonResponse + public function destroy(Asset $asset): JsonResponse { - $this->authorize('delete', Asset::class); - - if ($asset = Asset::find($id)) { - $this->authorize('delete', $asset); - - if ($asset->assignedTo) { - - $target = $asset->assignedTo; - $checkin_at = date('Y-m-d H:i:s'); - $originalValues = $asset->getRawOriginal(); - event(new CheckoutableCheckedIn($asset, $target, auth()->user(), 'Checkin on delete', $checkin_at, $originalValues)); - DB::table('assets') - ->where('id', $asset->id) - ->update(['assigned_to' => null]); - } - - $asset->delete(); - + //this is probably wrong + //$this->authorize('delete', Asset::class); + $this->authorize('delete', $asset); + try { + DestroyAssetAction::run($asset); return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/hardware/message.delete.success'))); + } catch (\Exception $e) { + report($e); + return response()->json(Helper::formatStandardApiResponse('error', null, 'something went wrong: ')); } - - return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 200); } diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index a127f2d0e336..71ffde0aede1 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -2,15 +2,18 @@ namespace App\Http\Controllers\Assets; +use App\Actions\Assets\DestroyAssetAction; use App\Actions\Assets\StoreAssetAction; use App\Events\CheckoutableCheckedIn; use App\Exceptions\CheckoutNotAllowed; use App\Helpers\Helper; use App\Http\Controllers\Controller; +use App\Http\Requests\Assets\DestroyAssetRequest; use App\Http\Requests\ImageUploadRequest; use App\Http\Requests\Assets\StoreAssetRequest; use App\Models\Actionlog; use App\Http\Requests\UploadFileRequest; +use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Support\Facades\Log; use App\Models\Asset; use App\Models\AssetModel; @@ -107,7 +110,7 @@ public function store(StoreAssetRequest $request): RedirectResponse $custom_fields = $request->collect()->filter(function ($value, $key) { return starts_with($key, '_snipeit_'); }); - + //DB::transaction(function () use ($request, $asset_tags, $serials, $custom_fields) { foreach ($asset_tags as $key => $asset_tag) { $asset = StoreAssetAction::run( @@ -360,39 +363,16 @@ public function update(ImageUploadRequest $request, $assetId = null) : RedirectR * @param int $assetId * @since [v1.0] */ - public function destroy(Request $request, $assetId) : RedirectResponse + public function destroy(Asset $asset): RedirectResponse { - // Check if the asset exists - if (is_null($asset = Asset::find($assetId))) { - // Redirect to the asset management page with error - return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.does_not_exist')); - } - $this->authorize('delete', $asset); - - if ($asset->assignedTo) { - - $target = $asset->assignedTo; - $checkin_at = date('Y-m-d H:i:s'); - $originalValues = $asset->getRawOriginal(); - event(new CheckoutableCheckedIn($asset, $target, auth()->user(), 'Checkin on delete', $checkin_at, $originalValues)); - DB::table('assets') - ->where('id', $asset->id) - ->update(['assigned_to' => null]); - } - - - if ($asset->image) { - try { - Storage::disk('public')->delete('assets'.'/'.$asset->image); - } catch (\Exception $e) { - Log::debug($e); - } + try { + DestroyAssetAction::run($asset); + return redirect()->route('hardware.index')->with('success', trans('admin/hardware/message.delete.success')); + } catch (\Exception $e) { + report($e); + return redirect()->back()->withInput()->withErrors($e->getMessage()); } - - $asset->delete(); - - return redirect()->route('hardware.index')->with('success', trans('admin/hardware/message.delete.success')); } /** diff --git a/app/Http/Requests/Assets/DestroyAssetRequest.php b/app/Http/Requests/Assets/DestroyAssetRequest.php new file mode 100644 index 000000000000..564233ae95be --- /dev/null +++ b/app/Http/Requests/Assets/DestroyAssetRequest.php @@ -0,0 +1,29 @@ +asset); + } + + /** + * Get the validation rules that apply to the request. + * + * @return array|string> + */ + public function rules(): array + { + return [ + // + ]; + } +} diff --git a/routes/api.php b/routes/api.php index 0581a046827d..a33132ef9295 100644 --- a/routes/api.php +++ b/routes/api.php @@ -573,15 +573,16 @@ Route::put('/hardware/{asset}', [Api\AssetsController::class, 'update'])->name('api.assets.put-update'); + Route::delete('/hardware/{asset}', [Api\AssetsController::class, 'destroy'])->name('api.assets.destroy'); + Route::resource('hardware', Api\AssetsController::class, ['names' => [ 'index' => 'api.assets.index', 'show' => 'api.assets.show', 'store' => 'api.assets.store', - 'destroy' => 'api.assets.destroy', ], - 'except' => ['create', 'edit', 'update'], + 'except' => ['create', 'edit', 'update', 'destroy'], 'parameters' => ['asset' => 'asset_id'], ] ); // end assets API routes diff --git a/routes/web/hardware.php b/routes/web/hardware.php index ee888aa1db71..aa8b0c087b5b 100644 --- a/routes/web/hardware.php +++ b/routes/web/hardware.php @@ -163,6 +163,8 @@ function () { }); +Route::delete('/hardware/{asset}', [AssetsController::class, 'destroy'])->name('hardware.destroy'); + Route::resource('hardware', AssetsController::class, [ @@ -172,6 +174,7 @@ function () { 'show' => 'view', ], ], + 'except' => ['destroy'], ]); Route::get('ht/{any?}', From b00f8b5c40d96a39b30c96dd64e138078fd7bd93 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 30 Oct 2024 19:37:51 -0500 Subject: [PATCH 07/42] a couple more tests passing --- app/Actions/Assets/StoreAssetAction.php | 8 ++- app/Http/Controllers/Api/AssetsController.php | 51 +++++++++--------- .../Controllers/Assets/AssetsController.php | 53 +++++++++---------- 3 files changed, 55 insertions(+), 57 deletions(-) diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index e162af60f399..b1dcb573d32b 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -46,7 +46,7 @@ public static function run( $assigned_location = null, $custom_fields = null, $request = null, //temp for handleImages - i'd like to see that moved to a helper or something - or maybe just invoked at the extended request level so that it doesn't need to be done in the action? - $next_audit_date = null, + $last_audit_date = null, ) { $settings = Setting::getSettings(); @@ -72,6 +72,8 @@ public static function run( $asset->requestable = $requestable; $asset->rtd_location_id = $rtd_location_id; $asset->byod = $byod; + $asset->last_audit_date = $last_audit_date; + $asset->location_id = $location_id; // set up next audit date if (!empty($settings->audit_interval)) { @@ -172,10 +174,6 @@ public static function run( if ($asset->image) { $asset->image = $asset->getImageUrl(); } - - } - - if ($asset->save()) { return $asset; } else { return $asset->getErrors(); diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 09de4fbccdea..e28d66afbdb8 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -603,31 +603,32 @@ public function store(StoreAssetRequest $request): JsonResponse return starts_with($key, '_snipeit_'); }); $asset = StoreAssetAction::run( - $request->validated('model_id'), - $request->validated('status_id'), - $request->validated('name'), - $request->validated('serial'), - $request->validated('company_id'), - $request->validated('asset_tag'), - $request->validated('order_number'), - $request->validated('notes'), - $request->validated('user_id'), - $request->validated('warranty_months'), - $request->validated('purchase_cost'), - $request->validated('asset_eol_date'), - $request->validated('purchase_date'), - $request->validated('assigned_to'), - $request->validated('supplier_id'), - $request->validated('requestable'), - $request->validated('rtd_location_id'), - $request->validated('location_id'), - $request->validated('files'), - $request->validated('byod'), - $request->validated('assigned_user'), - $request->validated('assigned_asset'), - $request->validated('assigned_location'), - $custom_fields, - $request, + model_id: $request->validated('model_id'), + status_id: $request->validated('status_id'), + name: $request->validated('name'), + serial: $request->validated('serial'), + company_id: $request->validated('company_id'), + asset_tag: $request->validated('asset_tag'), + order_number: $request->validated('order_number'), + notes: $request->validated('notes'), + user_id: $request->validated('user_id'), + warranty_months: $request->validated('warranty_months'), + purchase_cost: $request->validated('purchase_cost'), + asset_eol_date: $request->validated('asset_eol_date'), + purchase_date: $request->validated('purchase_date'), + assigned_to: $request->validated('assigned_to'), + supplier_id: $request->validated('supplier_id'), + requestable: $request->validated('requestable'), + rtd_location_id: $request->validated('rtd_location_id'), + location_id: $request->validated('location_id'), + files: $request->validated('files'), + byod: $request->validated('byod'), + assigned_user: $request->validated('assigned_user'), + assigned_asset: $request->validated('assigned_asset'), + assigned_location: $request->validated('assigned_location'), + custom_fields: $custom_fields, + request: $request, //this is just for the handleImages method... + last_audit_date: $request->validated('last_audit_date'), ); return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.create.success'))); diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 71ffde0aede1..1dc5034e9bab 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -114,31 +114,32 @@ public function store(StoreAssetRequest $request): RedirectResponse //DB::transaction(function () use ($request, $asset_tags, $serials, $custom_fields) { foreach ($asset_tags as $key => $asset_tag) { $asset = StoreAssetAction::run( - $request->validated('model_id'), - $request->validated('status_id'), - $request->validated('name'), - $serials[$key], - $request->validated('company_id'), - $asset_tag, - $request->validated('order_number'), - $request->validated('notes'), - $request->validated('user_id'), - $request->validated('warranty_months'), - $request->validated('purchase_cost'), - $request->validated('asset_eol_date'), - $request->validated('purchase_date'), - $request->validated('assigned_to'), - $request->validated('supplier_id'), - $request->validated('requestable'), - $request->validated('rtd_location_id'), - $request->validated('location_id'), - $request->validated('files'), - $request->validated('byod'), - $request->validated('assigned_user'), - $request->validated('assigned_asset'), - $request->validated('assigned_location'), - $custom_fields, - $request, //this is just for the handleImages method... + model_id: $request->validated('model_id'), + status_id: $request->validated('status_id'), + name: $request->validated('name'), + serial: $serials[$key], + company_id: $request->validated('company_id'), + asset_tag: $asset_tag, + order_number: $request->validated('order_number'), + notes: $request->validated('notes'), + user_id: $request->validated('user_id'), + warranty_months: $request->validated('warranty_months'), + purchase_cost: $request->validated('purchase_cost'), + asset_eol_date: $request->validated('asset_eol_date'), + purchase_date: $request->validated('purchase_date'), + assigned_to: $request->validated('assigned_to'), + supplier_id: $request->validated('supplier_id'), + requestable: $request->validated('requestable'), + rtd_location_id: $request->validated('rtd_location_id'), + location_id: $request->validated('location_id'), + files: $request->validated('files'), + byod: $request->validated('byod'), + assigned_user: $request->validated('assigned_user'), + assigned_asset: $request->validated('assigned_asset'), + assigned_location: $request->validated('assigned_location'), + custom_fields: $custom_fields, + request: $request, //this is just for the handleImages method... + last_audit_date: $request->validated('last_audit_date'), ); } //}); @@ -150,8 +151,6 @@ public function store(StoreAssetRequest $request): RedirectResponse } catch (\Exception $e) { return redirect()->back()->with('error', trans('admin/hardware/message.create.error')); } - // (obviously then this would move up to the request) - $this->validate($request, ['asset_tags' => ['required', 'array']]); } From a57fffe6969f286b8da0e7bed821283c7b9d652c Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Mon, 4 Nov 2024 13:39:42 -0600 Subject: [PATCH 08/42] a few fixes, tests pass --- app/Actions/Assets/StoreAssetAction.php | 2 +- app/Actions/Assets/UpdateAssetAction.php | 1 - app/Http/Controllers/Assets/AssetsController.php | 8 +++++--- app/Http/Requests/Assets/StoreAssetRequest.php | 15 +++++++++++++-- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index b1dcb573d32b..fa1b56eabd0f 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -176,7 +176,7 @@ public static function run( } return $asset; } else { - return $asset->getErrors(); + dd($asset->getErrors()); //need to figure out how to return errors from watson validating... } } } \ No newline at end of file diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index a6651ba856aa..117c8f0ae6cf 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -8,5 +8,4 @@ public static function run() { // stuff } - } \ No newline at end of file diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 1dc5034e9bab..c650d9f6990b 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -117,7 +117,7 @@ public function store(StoreAssetRequest $request): RedirectResponse model_id: $request->validated('model_id'), status_id: $request->validated('status_id'), name: $request->validated('name'), - serial: $serials[$key], + serial: $request->has('serials') ? $serials[$key] : null, company_id: $request->validated('company_id'), asset_tag: $asset_tag, order_number: $request->validated('order_number'), @@ -138,7 +138,7 @@ public function store(StoreAssetRequest $request): RedirectResponse assigned_asset: $request->validated('assigned_asset'), assigned_location: $request->validated('assigned_location'), custom_fields: $custom_fields, - request: $request, //this is just for the handleImages method... + request: $request, //this is just for the handleImages method... would love to figure out a different way of doing this last_audit_date: $request->validated('last_audit_date'), ); } @@ -149,7 +149,9 @@ public function store(StoreAssetRequest $request): RedirectResponse } catch (CheckoutNotAllowed $e) { return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.create.error')); } catch (\Exception $e) { - return redirect()->back()->with('error', trans('admin/hardware/message.create.error')); + report($e); + dd($e); + return redirect()->back()->with('error', 'something bad'); } } diff --git a/app/Http/Requests/Assets/StoreAssetRequest.php b/app/Http/Requests/Assets/StoreAssetRequest.php index 574216afdf50..c3db8108a5e2 100644 --- a/app/Http/Requests/Assets/StoreAssetRequest.php +++ b/app/Http/Requests/Assets/StoreAssetRequest.php @@ -37,8 +37,10 @@ public function prepareForValidation(): void $this->parseLastAuditDate(); + $asset_tag = $this->parseAssetTag(); + $this->merge([ - 'asset_tag' => $this->asset_tag ?? Asset::autoincrement_asset(), + 'asset_tag' => $asset_tag, 'company_id' => $idForCurrentUser, 'assigned_to' => $assigned_to ?? null, ]); @@ -61,7 +63,6 @@ public function rules(): array // converted to a float via setPurchaseCostAttribute). $modelRules = $this->removeNumericRulesFromPurchaseCost($modelRules); } - return array_merge( $modelRules, ['status_id' => [new AssetCannotBeCheckedOutToNondeployableStatus()]], @@ -101,4 +102,14 @@ private function removeNumericRulesFromPurchaseCost(array $rules): array return $rules; } + + private function parseAssetTag(): mixed + { + // this is for a gui request to make the request pass validation + // this just checks the first asset tag from the gui, watson should pick up if any of the rest of them fail + if ($this->has('asset_tags') && !$this->expectsJson()) { + return $this->input('asset_tags')[1]; + } + return $this->asset_tag ?? Asset::autoincrement_asset(); + } } From 916c0c0394ebbe1c518ee4b774d79d4b4e6e836e Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 12 Nov 2024 14:46:56 -0600 Subject: [PATCH 09/42] good progress on update action --- app/Actions/Assets/UpdateAssetAction.php | 151 +++++++++++++++++- .../Controllers/Assets/AssetsController.php | 94 ++++++----- routes/web/hardware.php | 4 +- 3 files changed, 210 insertions(+), 39 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 117c8f0ae6cf..21ba576153ad 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -2,10 +2,157 @@ namespace App\Actions\Assets; +use App\Events\CheckoutableCheckedIn; +use App\Helpers\Helper; +use App\Models\Asset; +use App\Models\AssetModel; +use App\Models\Company; +use App\Models\Statuslabel; +use Carbon\Carbon; +use Illuminate\Support\Facades\Crypt; +use Illuminate\Support\Facades\Gate; +use Illuminate\Support\Facades\Log; + class UpdateAssetAction { - public static function run() + public static function run( + Asset $asset, + $status_id = null, + $warranty_months = null, + $purchase_cost = null, + $purchase_date = null, + $next_audit_date = null, + $asset_eol_date = null, + $supplier_id = null, + $expected_checkin = null, + $requestable = null, + $rtd_location_id = null, + $byod = null + ) { - // stuff + // Check if the asset exists + //if (!$asset = Asset::find($assetId)) { + // // Redirect to the asset management page with error + // return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.does_not_exist')); + //} + //$this->authorize($asset); + + $asset->status_id = $status_id; + $asset->warranty_months = $warranty_months; + $asset->purchase_cost = $purchase_cost; + $asset->purchase_date = $purchase_date; + $asset->next_audit_date = $next_audit_date; + if ($request->filled('purchase_date') && !$request->filled('asset_eol_date') && ($asset->model->eol > 0)) { + $asset->purchase_date = $request->input('purchase_date', null); + $asset->asset_eol_date = Carbon::parse($request->input('purchase_date'))->addMonths($asset->model->eol)->format('Y-m-d'); + $asset->eol_explicit = false; + } elseif ($request->filled('asset_eol_date')) { + $asset->asset_eol_date = $request->input('asset_eol_date', null); + $months = Carbon::parse($asset->asset_eol_date)->diffInMonths($asset->purchase_date); + if ($asset->model->eol) { + if ($months != $asset->model->eol > 0) { + $asset->eol_explicit = true; + } else { + $asset->eol_explicit = false; + } + } else { + $asset->eol_explicit = true; + } + } elseif (!$request->filled('asset_eol_date') && (($asset->model->eol) == 0)) { + $asset->asset_eol_date = null; + $asset->eol_explicit = false; + } + $asset->supplier_id = $request->input('supplier_id', null); + $asset->expected_checkin = $request->input('expected_checkin', null); + $asset->requestable = $request->input('requestable', 0); + $asset->rtd_location_id = $request->input('rtd_location_id', null); + $asset->byod = $request->input('byod', 0); + + $status = Statuslabel::find($status_id); + + // This is a non-deployable status label - we should check the asset back in. + if (($status && $status->getStatuslabelType() != 'deployable') && ($target = $asset->assignedTo)) { + + $originalValues = $asset->getRawOriginal(); + $asset->assigned_to = null; + $asset->assigned_type = null; + $asset->accepted = null; + + event(new CheckoutableCheckedIn($asset, $target, auth()->user(), 'Checkin on asset update', date('Y-m-d H:i:s'), $originalValues)); + } + + if ($asset->assigned_to == '') { + $asset->location_id = $request->input('rtd_location_id', null); + } + + + if ($request->filled('image_delete')) { + try { + unlink(public_path().'/uploads/assets/'.$asset->image); + $asset->image = ''; + } catch (\Exception $e) { + Log::info($e); + } + } + + // Update the asset data + + $serial = $request->input('serials'); + $asset->serial = $request->input('serials'); + + if (is_array($request->input('serials'))) { + $asset->serial = $serial[1]; + } + + $asset->name = $request->input('name'); + $asset->company_id = Company::getIdForCurrentUser($request->input('company_id')); + $asset->model_id = $request->input('model_id'); + $asset->order_number = $request->input('order_number'); + + $asset_tags = $request->input('asset_tags'); + $asset->asset_tag = $request->input('asset_tags'); + + if (is_array($request->input('asset_tags'))) { + $asset->asset_tag = $asset_tags[1]; + } + + $asset->notes = $request->input('notes'); + + $asset = $request->handleImages($asset); + + // Update custom fields in the database. + // Validation for these fields is handlded through the AssetRequest form request + // FIXME: No idea why this is returning a Builder error on db_column_name. + // Need to investigate and fix. Using static method for now. + $model = AssetModel::find($request->get('model_id')); + if (($model) && ($model->fieldset)) { + foreach ($model->fieldset->fields as $field) { + + if ($field->field_encrypted == '1') { + if (Gate::allows('assets.view.encrypted_custom_fields')) { + if (is_array($request->input($field->db_column))) { + $asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column))); + } else { + $asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column)); + } + } + } else { + if (is_array($request->input($field->db_column))) { + $asset->{$field->db_column} = implode(', ', $request->input($field->db_column)); + } else { + $asset->{$field->db_column} = $request->input($field->db_column); + } + } + } + } + + session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); + + if ($asset->save()) { + return redirect()->to(Helper::getRedirectOption($request, $assetId, 'Assets')) + ->with('success', trans('admin/hardware/message.update.success')); + } + + return redirect()->back()->withInput()->withErrors($asset->getErrors()); } } \ No newline at end of file diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index c650d9f6990b..b5c1bf864393 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -4,11 +4,13 @@ use App\Actions\Assets\DestroyAssetAction; use App\Actions\Assets\StoreAssetAction; +use App\Actions\Assets\UpdateAssetAction; use App\Events\CheckoutableCheckedIn; use App\Exceptions\CheckoutNotAllowed; use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Http\Requests\Assets\DestroyAssetRequest; +use App\Http\Requests\Assets\UpdateAssetRequest; use App\Http\Requests\ImageUploadRequest; use App\Http\Requests\Assets\StoreAssetRequest; use App\Models\Actionlog; @@ -228,46 +230,66 @@ public function show($assetId = null) : View | RedirectResponse * @since [v1.0] * @author [A. Gianotto] [] */ - public function update(ImageUploadRequest $request, $assetId = null) : RedirectResponse + public function update(UpdateAssetRequest $request, Asset $asset): RedirectResponse { // Check if the asset exists - if (! $asset = Asset::find($assetId)) { - // Redirect to the asset management page with error - return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.does_not_exist')); - } - $this->authorize($asset); - - $asset->status_id = $request->input('status_id', null); - $asset->warranty_months = $request->input('warranty_months', null); - $asset->purchase_cost = $request->input('purchase_cost', null); - $asset->purchase_date = $request->input('purchase_date', null); - $asset->next_audit_date = $request->input('next_audit_date', null); - if ($request->filled('purchase_date') && !$request->filled('asset_eol_date') && ($asset->model->eol > 0)) { - $asset->purchase_date = $request->input('purchase_date', null); - $asset->asset_eol_date = Carbon::parse($request->input('purchase_date'))->addMonths($asset->model->eol)->format('Y-m-d'); - $asset->eol_explicit = false; - } elseif ($request->filled('asset_eol_date')) { - $asset->asset_eol_date = $request->input('asset_eol_date', null); - $months = Carbon::parse($asset->asset_eol_date)->diffInMonths($asset->purchase_date); - if($asset->model->eol) { - if($months != $asset->model->eol > 0) { - $asset->eol_explicit = true; - } else { - $asset->eol_explicit = false; - } - } else { - $asset->eol_explicit = true; - } - } elseif (!$request->filled('asset_eol_date') && (($asset->model->eol) == 0)) { - $asset->asset_eol_date = null; - $asset->eol_explicit = false; + //if (! $asset = Asset::find($assetId)) { + // // Redirect to the asset management page with error + // return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.does_not_exist')); + //} + //$this->authorize($asset); + try { + $asset = UpdateAssetAction::run( + $asset, + status_id: $request->validated('status_id'), + warranty_months: $request->validated('warranty_months'), + purchase_cost: $request->validated('purchase_cost'), + purchase_date: $request->validated('purchase_date'), + next_audit_date: $request->validated('next_audit_date'), + asset_eol_date: $request->validated('asset_eol_date'), + supplier_id: $request->validated('supplier_id'), + expected_checkin: $request->validated('expected_checkin'), + requestable: $request->validated('requestable'), + rtd_location_id: $request->validated('rtd_location_id'), + byod: $request->validated('byod') + ); + return redirect()->back()->with('success', trans('admin/hardware/message.update.success')); + } catch (\Exception $e) { + report($e); + return redirect()->back()->with('error', trans('admin/hardware/message.update.error')); } - $asset->supplier_id = $request->input('supplier_id', null); - $asset->expected_checkin = $request->input('expected_checkin', null); - $asset->requestable = $request->input('requestable', 0); - $asset->rtd_location_id = $request->input('rtd_location_id', null); - $asset->byod = $request->input('byod', 0); + + //$asset->status_id = $request->input('status_id', null); + //$asset->warranty_months = $request->input('warranty_months', null); + //$asset->purchase_cost = $request->input('purchase_cost', null); + //$asset->purchase_date = $request->input('purchase_date', null); + //$asset->next_audit_date = $request->input('next_audit_date', null); + //if ($request->filled('purchase_date') && !$request->filled('asset_eol_date') && ($asset->model->eol > 0)) { + // $asset->purchase_date = $request->input('purchase_date', null); + // $asset->asset_eol_date = Carbon::parse($request->input('purchase_date'))->addMonths($asset->model->eol)->format('Y-m-d'); + // $asset->eol_explicit = false; + //} elseif ($request->filled('asset_eol_date')) { + // $asset->asset_eol_date = $request->input('asset_eol_date', null); + // $months = Carbon::parse($asset->asset_eol_date)->diffInMonths($asset->purchase_date); + // if($asset->model->eol) { + // if($months != $asset->model->eol > 0) { + // $asset->eol_explicit = true; + // } else { + // $asset->eol_explicit = false; + // } + // } else { + // $asset->eol_explicit = true; + // } + //} elseif (!$request->filled('asset_eol_date') && (($asset->model->eol) == 0)) { + // $asset->asset_eol_date = null; + // $asset->eol_explicit = false; + //} + //$asset->supplier_id = $request->input('supplier_id', null); + //$asset->expected_checkin = $request->input('expected_checkin', null); + //$asset->requestable = $request->input('requestable', 0); + //$asset->rtd_location_id = $request->input('rtd_location_id', null); + //$asset->byod = $request->input('byod', 0); $status = Statuslabel::find($request->input('status_id')); diff --git a/routes/web/hardware.php b/routes/web/hardware.php index aa8b0c087b5b..f12bc60169c2 100644 --- a/routes/web/hardware.php +++ b/routes/web/hardware.php @@ -165,6 +165,8 @@ function () { Route::delete('/hardware/{asset}', [AssetsController::class, 'destroy'])->name('hardware.destroy'); +Route::match(['put', 'patch'], '/hardware/{asset}', [AssetsController::class, 'update'])->name('hardware.update'); + Route::resource('hardware', AssetsController::class, [ @@ -174,7 +176,7 @@ function () { 'show' => 'view', ], ], - 'except' => ['destroy'], + 'except' => ['destroy', 'update'], ]); Route::get('ht/{any?}', From 47d132703ee472d1b3694326247f73b581bcd28c Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 12 Nov 2024 18:08:29 -0600 Subject: [PATCH 10/42] some more good progress on UpdateAssetAction.php --- app/Actions/Assets/StoreAssetAction.php | 4 +- app/Actions/Assets/UpdateAssetAction.php | 76 +++++++++-------- .../Controllers/Assets/AssetsController.php | 84 +++---------------- .../Requests/Assets/UpdateAssetRequest.php | 1 + 4 files changed, 58 insertions(+), 107 deletions(-) diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index fa1b56eabd0f..be8fe42d0586 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -3,6 +3,7 @@ namespace App\Actions\Assets; use App\Exceptions\CheckoutNotAllowed; +use App\Http\Requests\ImageUploadRequest; use App\Models\Asset; use App\Models\AssetModel; use App\Models\Company; @@ -23,6 +24,7 @@ class StoreAssetAction public static function run( $model_id,//gonna make these two optional for now... we can either make them required here or use the spread operator when calling... $status_id,// + ImageUploadRequest $request, //temp for handleImages - i'd like to see that moved to a helper or something - or maybe just invoked at the extended request level so that it doesn't need to be done in the action? $name = null, $serial = null, $company_id = null, @@ -45,7 +47,7 @@ public static function run( $assigned_asset = null, $assigned_location = null, $custom_fields = null, - $request = null, //temp for handleImages - i'd like to see that moved to a helper or something - or maybe just invoked at the extended request level so that it doesn't need to be done in the action? + $last_audit_date = null, ) { diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 21ba576153ad..fc21731bae88 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -4,6 +4,8 @@ use App\Events\CheckoutableCheckedIn; use App\Helpers\Helper; +use App\Http\Requests\ImageUploadRequest; +use App\Http\Requests\Request; use App\Models\Asset; use App\Models\AssetModel; use App\Models\Company; @@ -17,6 +19,7 @@ class UpdateAssetAction { public static function run( Asset $asset, + ImageUploadRequest $request, $status_id = null, $warranty_months = null, $purchase_cost = null, @@ -25,29 +28,31 @@ public static function run( $asset_eol_date = null, $supplier_id = null, $expected_checkin = null, - $requestable = null, + $requestable = false, $rtd_location_id = null, - $byod = null + $byod = false, + $image_delete = false, + $serials = null, + $name = null, + $company_id = null, + $model_id = null, + $order_number = null, + $asset_tags = null, + $notes = null, ) { - // Check if the asset exists - //if (!$asset = Asset::find($assetId)) { - // // Redirect to the asset management page with error - // return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.does_not_exist')); - //} - //$this->authorize($asset); $asset->status_id = $status_id; $asset->warranty_months = $warranty_months; $asset->purchase_cost = $purchase_cost; $asset->purchase_date = $purchase_date; $asset->next_audit_date = $next_audit_date; - if ($request->filled('purchase_date') && !$request->filled('asset_eol_date') && ($asset->model->eol > 0)) { - $asset->purchase_date = $request->input('purchase_date', null); - $asset->asset_eol_date = Carbon::parse($request->input('purchase_date'))->addMonths($asset->model->eol)->format('Y-m-d'); + if ($purchase_date && !$asset_eol_date && ($asset->model->eol > 0)) { + $asset->purchase_date = $purchase_date; + $asset->asset_eol_date = Carbon::parse($purchase_date)->addMonths($asset->model->eol)->format('Y-m-d'); $asset->eol_explicit = false; - } elseif ($request->filled('asset_eol_date')) { - $asset->asset_eol_date = $request->input('asset_eol_date', null); + } elseif ($asset_eol_date) { + $asset->asset_eol_date = $asset_eol_date ?? null; $months = Carbon::parse($asset->asset_eol_date)->diffInMonths($asset->purchase_date); if ($asset->model->eol) { if ($months != $asset->model->eol > 0) { @@ -58,15 +63,15 @@ public static function run( } else { $asset->eol_explicit = true; } - } elseif (!$request->filled('asset_eol_date') && (($asset->model->eol) == 0)) { + } elseif (!$asset_eol_date && (($asset->model->eol) == 0)) { $asset->asset_eol_date = null; $asset->eol_explicit = false; } - $asset->supplier_id = $request->input('supplier_id', null); - $asset->expected_checkin = $request->input('expected_checkin', null); - $asset->requestable = $request->input('requestable', 0); - $asset->rtd_location_id = $request->input('rtd_location_id', null); - $asset->byod = $request->input('byod', 0); + $asset->supplier_id = $supplier_id; + $asset->expected_checkin = $expected_checkin; + $asset->requestable = $requestable; + $asset->rtd_location_id = $rtd_location_id; + $asset->byod = $byod; $status = Statuslabel::find($status_id); @@ -82,11 +87,11 @@ public static function run( } if ($asset->assigned_to == '') { - $asset->location_id = $request->input('rtd_location_id', null); + $asset->location_id = $rtd_location_id; } - if ($request->filled('image_delete')) { + if ($image_delete) { try { unlink(public_path().'/uploads/assets/'.$asset->image); $asset->image = ''; @@ -95,29 +100,32 @@ public static function run( } } - // Update the asset data + // this is gonna be a whole issue with validation - i'm assuming it's because we're using the same blade + // to do both create (which allows multiple creates) and update. should be fixable. + // and why is the offset 1 and not 0? very confusing. + $serial = $serials; + $asset->serial = $serials; - $serial = $request->input('serials'); - $asset->serial = $request->input('serials'); - - if (is_array($request->input('serials'))) { + if (is_array($serials)) { $asset->serial = $serial[1]; } - $asset->name = $request->input('name'); - $asset->company_id = Company::getIdForCurrentUser($request->input('company_id')); - $asset->model_id = $request->input('model_id'); - $asset->order_number = $request->input('order_number'); + $asset->name = $name; + $asset->company_id = Company::getIdForCurrentUser($company_id); + $asset->model_id = $model_id; + $asset->order_number = $order_number; - $asset_tags = $request->input('asset_tags'); - $asset->asset_tag = $request->input('asset_tags'); + // same thing as serials above + $asset_tags = $asset_tags; + $asset->asset_tag = $asset_tags; - if (is_array($request->input('asset_tags'))) { + if (is_array($asset_tags)) { $asset->asset_tag = $asset_tags[1]; } - $asset->notes = $request->input('notes'); + $asset->notes = $notes; + // andddddddd the handleImages issue reappears - i guess we'll be passing the request through here as well $asset = $request->handleImages($asset); // Update custom fields in the database. diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index b5c1bf864393..f0599bab9324 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -232,16 +232,10 @@ public function show($assetId = null) : View | RedirectResponse */ public function update(UpdateAssetRequest $request, Asset $asset): RedirectResponse { - - // Check if the asset exists - //if (! $asset = Asset::find($assetId)) { - // // Redirect to the asset management page with error - // return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.does_not_exist')); - //} - //$this->authorize($asset); try { $asset = UpdateAssetAction::run( - $asset, + asset: $asset, + request: $request, status_id: $request->validated('status_id'), warranty_months: $request->validated('warranty_months'), purchase_cost: $request->validated('purchase_cost'), @@ -252,7 +246,15 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo expected_checkin: $request->validated('expected_checkin'), requestable: $request->validated('requestable'), rtd_location_id: $request->validated('rtd_location_id'), - byod: $request->validated('byod') + byod: $request->validated('byod'), + image_delete: $request->validated('image_delete'), + serials: null, // this needs to be set up in request somehow + name: $request->validated('name'), + company_id: $request->validated('company_id'), + model_id: $request->validated('model_id'), + order_number: $request->validated('order_number'), + asset_tags: null, // same as serials + notes: $request->validated('notes'), ); return redirect()->back()->with('success', trans('admin/hardware/message.update.success')); } catch (\Exception $e) { @@ -260,66 +262,7 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo return redirect()->back()->with('error', trans('admin/hardware/message.update.error')); } - //$asset->status_id = $request->input('status_id', null); - //$asset->warranty_months = $request->input('warranty_months', null); - //$asset->purchase_cost = $request->input('purchase_cost', null); - //$asset->purchase_date = $request->input('purchase_date', null); - //$asset->next_audit_date = $request->input('next_audit_date', null); - //if ($request->filled('purchase_date') && !$request->filled('asset_eol_date') && ($asset->model->eol > 0)) { - // $asset->purchase_date = $request->input('purchase_date', null); - // $asset->asset_eol_date = Carbon::parse($request->input('purchase_date'))->addMonths($asset->model->eol)->format('Y-m-d'); - // $asset->eol_explicit = false; - //} elseif ($request->filled('asset_eol_date')) { - // $asset->asset_eol_date = $request->input('asset_eol_date', null); - // $months = Carbon::parse($asset->asset_eol_date)->diffInMonths($asset->purchase_date); - // if($asset->model->eol) { - // if($months != $asset->model->eol > 0) { - // $asset->eol_explicit = true; - // } else { - // $asset->eol_explicit = false; - // } - // } else { - // $asset->eol_explicit = true; - // } - //} elseif (!$request->filled('asset_eol_date') && (($asset->model->eol) == 0)) { - // $asset->asset_eol_date = null; - // $asset->eol_explicit = false; - //} - //$asset->supplier_id = $request->input('supplier_id', null); - //$asset->expected_checkin = $request->input('expected_checkin', null); - //$asset->requestable = $request->input('requestable', 0); - //$asset->rtd_location_id = $request->input('rtd_location_id', null); - //$asset->byod = $request->input('byod', 0); - - $status = Statuslabel::find($request->input('status_id')); - - // This is a non-deployable status label - we should check the asset back in. - if (($status && $status->getStatuslabelType() != 'deployable') && ($target = $asset->assignedTo)) { - - $originalValues = $asset->getRawOriginal(); - $asset->assigned_to = null; - $asset->assigned_type = null; - $asset->accepted = null; - - event(new CheckoutableCheckedIn($asset, $target, auth()->user(), 'Checkin on asset update', date('Y-m-d H:i:s'), $originalValues)); - } - - if ($asset->assigned_to == '') { - $asset->location_id = $request->input('rtd_location_id', null); - } - - - if ($request->filled('image_delete')) { - try { - unlink(public_path().'/uploads/assets/'.$asset->image); - $asset->image = ''; - } catch (\Exception $e) { - Log::info($e); - } - } - - // Update the asset data - + // serials???? $serial = $request->input('serials'); $asset->serial = $request->input('serials'); @@ -344,9 +287,6 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo $asset = $request->handleImages($asset); // Update custom fields in the database. - // Validation for these fields is handlded through the AssetRequest form request - // FIXME: No idea why this is returning a Builder error on db_column_name. - // Need to investigate and fix. Using static method for now. $model = AssetModel::find($request->get('model_id')); if (($model) && ($model->fieldset)) { foreach ($model->fieldset->fields as $field) { diff --git a/app/Http/Requests/Assets/UpdateAssetRequest.php b/app/Http/Requests/Assets/UpdateAssetRequest.php index 9a8684d213a4..2046414b7474 100644 --- a/app/Http/Requests/Assets/UpdateAssetRequest.php +++ b/app/Http/Requests/Assets/UpdateAssetRequest.php @@ -34,6 +34,7 @@ public function rules() (new Asset)->getRules(), // this is to overwrite rulesets that include required, and rewrite unique_undeleted [ + 'image_delete' => ['bool'], 'model_id' => ['integer', 'exists:models,id,deleted_at,NULL', 'not_array'], 'status_id' => ['integer', 'exists:status_labels,id'], 'asset_tag' => [ From c91dfaf9df213a994c0a98d6006a49484ce23d00 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Thu, 14 Nov 2024 16:54:23 -0600 Subject: [PATCH 11/42] gui seemingly working, other good progress - api next --- app/Actions/Assets/UpdateAssetAction.php | 35 ++---- app/Http/Controllers/Api/AssetsController.php | 11 +- .../Controllers/Assets/AssetsController.php | 117 +++++++++--------- .../Requests/Assets/UpdateAssetRequest.php | 27 +++- app/Models/Asset.php | 2 + resources/views/hardware/edit.blade.php | 2 +- 6 files changed, 105 insertions(+), 89 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index fc21731bae88..6cd3fa71a960 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -3,9 +3,7 @@ namespace App\Actions\Assets; use App\Events\CheckoutableCheckedIn; -use App\Helpers\Helper; use App\Http\Requests\ImageUploadRequest; -use App\Http\Requests\Request; use App\Models\Asset; use App\Models\AssetModel; use App\Models\Company; @@ -32,16 +30,16 @@ public static function run( $rtd_location_id = null, $byod = false, $image_delete = false, - $serials = null, + $serial = null, $name = null, $company_id = null, $model_id = null, $order_number = null, - $asset_tags = null, + $asset_tag = null, $notes = null, - ) - { + ): \App\Models\SnipeModel { + dump($purchase_date); $asset->status_id = $status_id; $asset->warranty_months = $warranty_months; $asset->purchase_cost = $purchase_cost; @@ -100,28 +98,14 @@ public static function run( } } - // this is gonna be a whole issue with validation - i'm assuming it's because we're using the same blade - // to do both create (which allows multiple creates) and update. should be fixable. - // and why is the offset 1 and not 0? very confusing. - $serial = $serials; - $asset->serial = $serials; - - if (is_array($serials)) { - $asset->serial = $serial[1]; - } + $asset->serial = $serial; $asset->name = $name; $asset->company_id = Company::getIdForCurrentUser($company_id); $asset->model_id = $model_id; $asset->order_number = $order_number; - // same thing as serials above - $asset_tags = $asset_tags; - $asset->asset_tag = $asset_tags; - - if (is_array($asset_tags)) { - $asset->asset_tag = $asset_tags[1]; - } + $asset->asset_tag = $asset_tag; $asset->notes = $notes; @@ -156,11 +140,8 @@ public static function run( session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); - if ($asset->save()) { - return redirect()->to(Helper::getRedirectOption($request, $assetId, 'Assets')) - ->with('success', trans('admin/hardware/message.update.success')); - } + $asset->save(); - return redirect()->back()->withInput()->withErrors($asset->getErrors()); + return $asset; } } \ No newline at end of file diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index e28d66afbdb8..8b15edff7905 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -4,6 +4,7 @@ use App\Actions\Assets\DestroyAssetAction; use App\Actions\Assets\StoreAssetAction; +use App\Actions\Assets\UpdateAssetAction; use App\Events\CheckoutableCheckedIn; use App\Exceptions\CheckoutNotAllowed; use App\Http\Requests\Assets\StoreAssetRequest; @@ -650,6 +651,12 @@ public function store(StoreAssetRequest $request): JsonResponse */ public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse { + try { + UpdateAssetAction::run($asset, $request, ...$request->validated()); + } catch (CheckoutNotAllowed $e) { + return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage()), 200); + } + $asset->fill($request->validated()); if ($request->has('model_id')) { @@ -675,8 +682,8 @@ public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse $asset = $request->handleImages($asset); $model = $asset->model; - - // Update custom fields + + // Update custom fields $problems_updating_encrypted_custom_fields = false; if (($model) && (isset($model->fieldset))) { foreach ($model->fieldset->fields as $field) { diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index f0599bab9324..60b6d459bbd9 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -233,6 +233,15 @@ public function show($assetId = null) : View | RedirectResponse public function update(UpdateAssetRequest $request, Asset $asset): RedirectResponse { try { + $serials = $request->input('serials'); + $asset_tags = $request->input('asset_tags'); + if (is_array($request->input('serials'))) { + $serial = $serials[1]; + } + if (is_array($request->input('asset_tags'))) { + $asset_tag = $asset_tags[1]; + } + $asset = UpdateAssetAction::run( asset: $asset, request: $request, @@ -248,75 +257,67 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo rtd_location_id: $request->validated('rtd_location_id'), byod: $request->validated('byod'), image_delete: $request->validated('image_delete'), - serials: null, // this needs to be set up in request somehow + serial: $serial, // this needs to be set up in request somehow name: $request->validated('name'), company_id: $request->validated('company_id'), model_id: $request->validated('model_id'), order_number: $request->validated('order_number'), - asset_tags: null, // same as serials + asset_tag: $asset_tag, // same as serials notes: $request->validated('notes'), ); - return redirect()->back()->with('success', trans('admin/hardware/message.update.success')); + return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) + ->with('success', trans('admin/hardware/message.update.success')); + } catch (\Watson\Validating\ValidationException $e) { + $errors = $e->getErrors(); + return redirect()->back()->withInput()->withErrors($errors); } catch (\Exception $e) { report($e); - return redirect()->back()->with('error', trans('admin/hardware/message.update.error')); + return redirect()->back()->with('error', trans('admin/hardware/message.update.error'), $asset); } // serials???? - $serial = $request->input('serials'); - $asset->serial = $request->input('serials'); - - if (is_array($request->input('serials'))) { - $asset->serial = $serial[1]; - } - - $asset->name = $request->input('name'); - $asset->company_id = Company::getIdForCurrentUser($request->input('company_id')); - $asset->model_id = $request->input('model_id'); - $asset->order_number = $request->input('order_number'); - - $asset_tags = $request->input('asset_tags'); - $asset->asset_tag = $request->input('asset_tags'); - - if (is_array($request->input('asset_tags'))) { - $asset->asset_tag = $asset_tags[1]; - } - - $asset->notes = $request->input('notes'); - - $asset = $request->handleImages($asset); - - // Update custom fields in the database. - $model = AssetModel::find($request->get('model_id')); - if (($model) && ($model->fieldset)) { - foreach ($model->fieldset->fields as $field) { - - if ($field->field_encrypted == '1') { - if (Gate::allows('assets.view.encrypted_custom_fields')) { - if (is_array($request->input($field->db_column))) { - $asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column))); - } else { - $asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column)); - } - } - } else { - if (is_array($request->input($field->db_column))) { - $asset->{$field->db_column} = implode(', ', $request->input($field->db_column)); - } else { - $asset->{$field->db_column} = $request->input($field->db_column); - } - } - } - } - - session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); - - if ($asset->save()) { - return redirect()->to(Helper::getRedirectOption($request, $assetId, 'Assets')) - ->with('success', trans('admin/hardware/message.update.success')); - } - - return redirect()->back()->withInput()->withErrors($asset->getErrors()); + //$asset->name = $request->input('name'); + //$asset->company_id = Company::getIdForCurrentUser($request->input('company_id')); + //$asset->model_id = $request->input('model_id'); + //$asset->order_number = $request->input('order_number'); + // + // + // + //$asset->notes = $request->input('notes'); + // + //$asset = $request->handleImages($asset); + // + //// Update custom fields in the database. + //$model = AssetModel::find($request->get('model_id')); + //if (($model) && ($model->fieldset)) { + // foreach ($model->fieldset->fields as $field) { + // + // if ($field->field_encrypted == '1') { + // if (Gate::allows('assets.view.encrypted_custom_fields')) { + // if (is_array($request->input($field->db_column))) { + // $asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column))); + // } else { + // $asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column)); + // } + // } + // } else { + // if (is_array($request->input($field->db_column))) { + // $asset->{$field->db_column} = implode(', ', $request->input($field->db_column)); + // } else { + // $asset->{$field->db_column} = $request->input($field->db_column); + // } + // } + // } + //} + // + //session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); + // + //if ($asset->save()) { + // return redirect()->to(Helper::getRedirectOption($request, $assetId, 'Assets')) + // ->with('success', trans('admin/hardware/message.update.success')); + //} + // + //return redirect()->back()->withInput()->withErrors($asset->getErrors()); } /** diff --git a/app/Http/Requests/Assets/UpdateAssetRequest.php b/app/Http/Requests/Assets/UpdateAssetRequest.php index 2046414b7474..1732b750283f 100644 --- a/app/Http/Requests/Assets/UpdateAssetRequest.php +++ b/app/Http/Requests/Assets/UpdateAssetRequest.php @@ -29,9 +29,19 @@ public function authorize() */ public function rules() { + $modelRules = (new Asset)->getRules(); + // TODO: make sure this actually works + if ((Setting::getSettings()->digit_separator === '1.234,56' || '1,234.56') && is_string($this->input('purchase_cost'))) { + // If purchase_cost was submitted as a string with a comma separator + // then we need to ignore the normal numeric rules. + // Since the original rules still live on the model they will be run + // right before saving (and after purchase_cost has been + // converted to a float via setPurchaseCostAttribute). + $modelRules = $this->removeNumericRulesFromPurchaseCost($modelRules); + } $rules = array_merge( parent::rules(), - (new Asset)->getRules(), + $modelRules, // this is to overwrite rulesets that include required, and rewrite unique_undeleted [ 'image_delete' => ['bool'], @@ -49,6 +59,21 @@ public function rules() if (Setting::getSettings()->digit_separator === '1.234,56' && is_string($this->input('purchase_cost'))) { $rules['purchase_cost'] = ['nullable', 'string']; } + return $rules; + } + + private function removeNumericRulesFromPurchaseCost(array $rules): array + { + $purchaseCost = $rules['purchase_cost']; + + // If rule is in "|" format then turn it into an array + if (is_string($purchaseCost)) { + $purchaseCost = explode('|', $purchaseCost); + } + + $rules['purchase_cost'] = array_filter($purchaseCost, function ($rule) { + return $rule !== 'numeric' && $rule !== 'gte:0'; + }); return $rules; } diff --git a/app/Models/Asset.php b/app/Models/Asset.php index ce8b870eb2e0..3f53dda623b4 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -35,6 +35,8 @@ class Asset extends Depreciable use CompanyableTrait; use HasFactory, Loggable, Requestable, Presentable, SoftDeletes, ValidatingTrait, UniqueUndeletedTrait; + protected $throwValidationExceptions = true; + public const LOCATION = 'location'; public const ASSET = 'asset'; public const USER = 'user'; diff --git a/resources/views/hardware/edit.blade.php b/resources/views/hardware/edit.blade.php index efd5d24b9cd0..6e3dd3c54eae 100755 --- a/resources/views/hardware/edit.blade.php +++ b/resources/views/hardware/edit.blade.php @@ -5,7 +5,7 @@ 'topSubmit' => true, 'helpText' => trans('help.assets'), 'helpPosition' => 'right', - 'formAction' => ($item->id) ? route('hardware.update', ['hardware' => $item->id]) : route('hardware.store'), + 'formAction' => ($item->id) ? route('hardware.update', ['asset' => $item->id]) : route('hardware.store'), 'index_route' => 'hardware.index', 'options' => [ 'index' => trans('admin/hardware/form.redirect_to_all', ['type' => 'assets']), From 7ab0b98ff2bda2d2c890ce7991f3ac72d671f83a Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Mon, 18 Nov 2024 12:43:28 -0600 Subject: [PATCH 12/42] var changes --- app/Http/Controllers/Assets/AssetsController.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 60b6d459bbd9..01f17d5718df 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -233,13 +233,13 @@ public function show($assetId = null) : View | RedirectResponse public function update(UpdateAssetRequest $request, Asset $asset): RedirectResponse { try { - $serials = $request->input('serials'); - $asset_tags = $request->input('asset_tags'); + $serial = $request->input('serials'); + $asset_tag = $request->input('asset_tags'); if (is_array($request->input('serials'))) { - $serial = $serials[1]; + $serial = $request->input('serials')[1]; } if (is_array($request->input('asset_tags'))) { - $asset_tag = $asset_tags[1]; + $asset_tag = $request->input('asset_tags')[1]; } $asset = UpdateAssetAction::run( From dd8fb6ef4aa6d1ad444c71618832a1647ea55ca6 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 19 Nov 2024 00:13:28 -0600 Subject: [PATCH 13/42] fixed some things, made bulk progress, tests pass --- app/Actions/Assets/StoreAssetAction.php | 2 +- app/Actions/Assets/UpdateAssetAction.php | 79 ++++++++++--- app/Http/Controllers/Api/AssetsController.php | 36 +++--- .../Controllers/Assets/AssetsController.php | 50 +------- .../Assets/BulkAssetsController.php | 108 ++++++++++-------- 5 files changed, 150 insertions(+), 125 deletions(-) diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index be8fe42d0586..de04dbd6c9f0 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -22,7 +22,7 @@ class StoreAssetAction * @throws CheckoutNotAllowed */ public static function run( - $model_id,//gonna make these two optional for now... we can either make them required here or use the spread operator when calling... + $model_id, $status_id,// ImageUploadRequest $request, //temp for handleImages - i'd like to see that moved to a helper or something - or maybe just invoked at the extended request level so that it doesn't need to be done in the action? $name = null, diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 6cd3fa71a960..354e1dd4fee1 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -7,7 +7,9 @@ use App\Models\Asset; use App\Models\AssetModel; use App\Models\Company; +use App\Models\Location; use App\Models\Statuslabel; +use App\Models\User; use Carbon\Carbon; use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Gate; @@ -17,17 +19,22 @@ class UpdateAssetAction { public static function run( Asset $asset, - ImageUploadRequest $request, + ImageUploadRequest $request, //very much would like this to go away $status_id = null, $warranty_months = null, $purchase_cost = null, $purchase_date = null, + $last_audit_date = null, $next_audit_date = null, $asset_eol_date = null, $supplier_id = null, $expected_checkin = null, $requestable = false, + $location_id = null, $rtd_location_id = null, + $assigned_location = null, // derp, make these work + $assigned_asset = null, + $assigned_user = null, $byod = false, $image_delete = false, $serial = null, @@ -37,16 +44,17 @@ public static function run( $order_number = null, $asset_tag = null, $notes = null, + $isBulk = false, ): \App\Models\SnipeModel { - dump($purchase_date); - $asset->status_id = $status_id; - $asset->warranty_months = $warranty_months; - $asset->purchase_cost = $purchase_cost; - $asset->purchase_date = $purchase_date; - $asset->next_audit_date = $next_audit_date; + $asset->status_id = $status_id ?? $asset->status_id; + $asset->warranty_months = $warranty_months ?? $asset->warranty_months; + $asset->purchase_cost = $purchase_cost ?? $asset->purchase_cost; + $asset->purchase_date = $purchase_date ?? $asset->purchase_date?->format('Y-m-d'); + $asset->next_audit_date = $next_audit_date ?? $asset->next_audit_date; + $asset->last_audit_date = $last_audit_date ?? $asset->last_audit_date; if ($purchase_date && !$asset_eol_date && ($asset->model->eol > 0)) { - $asset->purchase_date = $purchase_date; + $asset->purchase_date = $purchase_date ?? $asset->purchase_date?->format('Y-m-d'); $asset->asset_eol_date = Carbon::parse($purchase_date)->addMonths($asset->model->eol)->format('Y-m-d'); $asset->eol_explicit = false; } elseif ($asset_eol_date) { @@ -68,7 +76,10 @@ public static function run( $asset->supplier_id = $supplier_id; $asset->expected_checkin = $expected_checkin; $asset->requestable = $requestable; - $asset->rtd_location_id = $rtd_location_id; + + $asset->location_id = $location_id; + + !$isBulk ?? $asset->rtd_location_id = $rtd_location_id; $asset->byod = $byod; $status = Statuslabel::find($status_id); @@ -102,20 +113,20 @@ public static function run( $asset->name = $name; $asset->company_id = Company::getIdForCurrentUser($company_id); - $asset->model_id = $model_id; - $asset->order_number = $order_number; + $asset->model_id = $model_id ?? $asset->model_id; + $asset->order_number = $order_number ?? $asset->order_number; - $asset->asset_tag = $asset_tag; + $asset->asset_tag = $asset_tag ?? $asset->asset_tag; $asset->notes = $notes; - // andddddddd the handleImages issue reappears - i guess we'll be passing the request through here as well $asset = $request->handleImages($asset); // Update custom fields in the database. // Validation for these fields is handlded through the AssetRequest form request // FIXME: No idea why this is returning a Builder error on db_column_name. // Need to investigate and fix. Using static method for now. + $model = AssetModel::find($request->get('model_id')); if (($model) && ($model->fieldset)) { foreach ($model->fieldset->fields as $field) { @@ -140,8 +151,48 @@ public static function run( session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); - $asset->save(); + if ($isBulk) { + self::bulkUpdate($asset, $request); + } + + if ($asset->save()) { + // check out stuff + if (!is_null($assigned_user) && ($target = User::find($assigned_user))) { + $location = $target->location_id; + } elseif (!is_null($assigned_asset) && ($target = Asset::find($assigned_asset))) { + $location = $target->location_id; + + Asset::where('assigned_type', \App\Models\Asset::class)->where('assigned_to', $asset->id) + ->update(['location_id' => $target->location_id]); + } elseif (!is_null($assigned_location) && ($target = Location::find($assigned_location))) { + $location = $target->id; + } + if (isset($target)) { + $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); + } + } return $asset; } + + private static function bulkUpdate($asset, $request): void + { + if ($request->filled('rtd_location_id')) { + + if (($request->filled('update_real_loc')) && (($request->input('update_real_loc')) == '0')) { + $asset->rtd_location_id = $request->input('rtd_location_id'); + } + + if (($request->filled('update_real_loc')) && (($request->input('update_real_loc')) == '1')) { + $asset->location_id = $request->input('rtd_location_id'); + $asset->rtd_location_id = $request->input('rtd_location_id'); + } + + if (($request->filled('update_real_loc')) && (($request->input('update_real_loc')) == '2')) { + $asset->location_id = $request->input('rtd_location_id'); + } + + } + + } } \ No newline at end of file diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 8b15edff7905..b11c6a0104c9 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -30,13 +30,11 @@ use App\Models\Location; use App\Models\Setting; use App\Models\User; -use Illuminate\Support\Facades\Auth; use Carbon\Carbon; use Illuminate\Support\Facades\DB; use Illuminate\Http\Request; -use App\Http\Requests\ImageUploadRequest; -use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Route; +use Watson\Validating\ValidationException; /** @@ -655,6 +653,10 @@ public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse UpdateAssetAction::run($asset, $request, ...$request->validated()); } catch (CheckoutNotAllowed $e) { return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage()), 200); + } catch (ValidationException $e) { + return response()->json(Helper::formatStandardApiResponse('error', null, $e->getErrors()), 200); + } catch (\Exception $e) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.something_went_wrong'))); } $asset->fill($request->validated()); @@ -708,20 +710,20 @@ public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse } } if ($asset->save()) { - if (($request->filled('assigned_user')) && ($target = User::find($request->get('assigned_user')))) { - $location = $target->location_id; - } elseif (($request->filled('assigned_asset')) && ($target = Asset::find($request->get('assigned_asset')))) { - $location = $target->location_id; - - Asset::where('assigned_type', \App\Models\Asset::class)->where('assigned_to', $asset->id) - ->update(['location_id' => $target->location_id]); - } elseif (($request->filled('assigned_location')) && ($target = Location::find($request->get('assigned_location')))) { - $location = $target->id; - } - - if (isset($target)) { - $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); - } + //if (($request->filled('assigned_user')) && ($target = User::find($request->get('assigned_user')))) { + // $location = $target->location_id; + //} elseif (($request->filled('assigned_asset')) && ($target = Asset::find($request->get('assigned_asset')))) { + // $location = $target->location_id; + // + // Asset::where('assigned_type', \App\Models\Asset::class)->where('assigned_to', $asset->id) + // ->update(['location_id' => $target->location_id]); + //} elseif (($request->filled('assigned_location')) && ($target = Location::find($request->get('assigned_location')))) { + // $location = $target->id; + //} + // + //if (isset($target)) { + // $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); + //} if ($asset->image) { $asset->image = $asset->getImageUrl(); diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 01f17d5718df..e6394bbda577 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -38,6 +38,7 @@ use Illuminate\Contracts\View\View; use Illuminate\Http\RedirectResponse; use Symfony\Component\HttpFoundation\BinaryFileResponse; +use Watson\Validating\ValidationException; /** * This class controls all actions related to assets for @@ -267,57 +268,12 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo ); return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) ->with('success', trans('admin/hardware/message.update.success')); - } catch (\Watson\Validating\ValidationException $e) { - $errors = $e->getErrors(); - return redirect()->back()->withInput()->withErrors($errors); + } catch (ValidationException $e) { + return redirect()->back()->withInput()->withErrors($e->getErrors()); } catch (\Exception $e) { report($e); return redirect()->back()->with('error', trans('admin/hardware/message.update.error'), $asset); } - - // serials???? - //$asset->name = $request->input('name'); - //$asset->company_id = Company::getIdForCurrentUser($request->input('company_id')); - //$asset->model_id = $request->input('model_id'); - //$asset->order_number = $request->input('order_number'); - // - // - // - //$asset->notes = $request->input('notes'); - // - //$asset = $request->handleImages($asset); - // - //// Update custom fields in the database. - //$model = AssetModel::find($request->get('model_id')); - //if (($model) && ($model->fieldset)) { - // foreach ($model->fieldset->fields as $field) { - // - // if ($field->field_encrypted == '1') { - // if (Gate::allows('assets.view.encrypted_custom_fields')) { - // if (is_array($request->input($field->db_column))) { - // $asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column))); - // } else { - // $asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column)); - // } - // } - // } else { - // if (is_array($request->input($field->db_column))) { - // $asset->{$field->db_column} = implode(', ', $request->input($field->db_column)); - // } else { - // $asset->{$field->db_column} = $request->input($field->db_column); - // } - // } - // } - //} - // - //session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); - // - //if ($asset->save()) { - // return redirect()->to(Helper::getRedirectOption($request, $assetId, 'Assets')) - // ->with('success', trans('admin/hardware/message.update.success')); - //} - // - //return redirect()->back()->withInput()->withErrors($asset->getErrors()); } /** diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index c27cfe3e0c69..4c3c763b30af 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -2,9 +2,11 @@ namespace App\Http\Controllers\Assets; +use App\Actions\Assets\UpdateAssetAction; use App\Helpers\Helper; use App\Http\Controllers\CheckInOutRequest; use App\Http\Controllers\Controller; +use App\Http\Requests\ImageUploadRequest; use App\Models\Asset; use App\Models\AssetModel; use App\Models\Statuslabel; @@ -21,6 +23,7 @@ use Illuminate\Contracts\View\View; use Illuminate\Http\RedirectResponse; use Illuminate\Database\Eloquent\ModelNotFoundException; +use Watson\Validating\ValidationException; class BulkAssetsController extends Controller { @@ -199,25 +202,65 @@ public function edit(Request $request) : View | RedirectResponse * @internal param array $assets * @since [v2.0] */ - public function update(Request $request) : RedirectResponse + public function update(ImageUploadRequest $request): RedirectResponse { + // this should be in request, but request weird, need to think it through a little $this->authorize('update', Asset::class); - $has_errors = 0; - $error_array = array(); - // Get the back url from the session and then destroy the session $bulk_back_url = route('hardware.index'); - + // is this necessary? + if (!$request->filled('ids') || count($request->input('ids')) == 0) { + return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.update.no_assets_selected')); + } if ($request->session()->has('bulk_back_url')) { $bulk_back_url = $request->session()->pull('bulk_back_url'); } + // find and update assets + $assets = Asset::whereIn('id', $request->input('ids'))->get(); + $errors = []; + foreach ($assets as $key => $asset) { + try { + $updatedAsset = UpdateAssetAction::run( + asset: $asset, + request: $request, + status_id: $request->input('status_id'), + warranty_months: $request->input('warranty_months'), + purchase_cost: $request->input('purchase_cost'), + purchase_date: $request->filled('null_purchase_date') ? null : $request->input('purchase_date'), + next_audit_date: $request->filled('null_next_audit_date') ? null : $request->input('next_audit_date'), + supplier_id: $request->input('supplier_id'), + expected_checkin: $request->filled('null_expected_checkin_date') ? null : $request->input('expected_checkin'), + requestable: $request->input('requestable'), + rtd_location_id: $request->input('rtd_location_id'), + name: $request->filled('null_name') ? null : $request->input('name'), + company_id: $request->input('company_id'), + model_id: $request->input('model_id'), + order_number: $request->input('order_number'), + isBulk: true, + ); + // catch exceptions + } catch (ValidationException $e) { + $errors[$key] = $e->getMessage(); + } catch (\Exception $e) { + report($e); + $errors[$key] = trans('general.something_went_wrong'); + } + } + if (!empty($errors)) { + return redirect($bulk_back_url)->with('bulk_asset_errors', $errors); + } + return redirect()->to('index')->with('success', trans('admin/hardware/message.update.assets_do_not_exist')); + + + $has_errors = 0; + $error_array = array(); + + // Get the back url from the session and then destroy the session $custom_field_columns = CustomField::all()->pluck('db_column')->toArray(); - if (! $request->filled('ids') || count($request->input('ids')) == 0) { - return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.update.no_assets_selected')); - } + $assets = Asset::whereIn('id', $request->input('ids'))->get(); @@ -232,24 +275,8 @@ public function update(Request $request) : RedirectResponse * its checkout status. */ - if (($request->filled('name')) - || ($request->filled('purchase_date')) - || ($request->filled('expected_checkin')) - || ($request->filled('purchase_cost')) - || ($request->filled('supplier_id')) - || ($request->filled('order_number')) - || ($request->filled('warranty_months')) - || ($request->filled('rtd_location_id')) - || ($request->filled('requestable')) - || ($request->filled('company_id')) - || ($request->filled('status_id')) - || ($request->filled('model_id')) - || ($request->filled('next_audit_date')) - || ($request->filled('null_name')) - || ($request->filled('null_purchase_date')) - || ($request->filled('null_expected_checkin_date')) - || ($request->filled('null_next_audit_date')) - || ($request->anyFilled($custom_field_columns)) + if ( + ($request->anyFilled($custom_field_columns)) ) { // Let's loop through those assets and build an update array @@ -264,14 +291,14 @@ public function update(Request $request) : RedirectResponse * It's tempting to make these match the request check above, but some of these values require * extra work to make sure the data makes sense. */ - $this->conditionallyAddItem('name') - ->conditionallyAddItem('purchase_date') - ->conditionallyAddItem('expected_checkin') - ->conditionallyAddItem('order_number') - ->conditionallyAddItem('requestable') - ->conditionallyAddItem('supplier_id') - ->conditionallyAddItem('warranty_months') - ->conditionallyAddItem('next_audit_date'); + //$this->conditionallyAddItem('name') + // ->conditionallyAddItem('purchase_date') + // ->conditionallyAddItem('expected_checkin') + // ->conditionallyAddItem('order_number') + // ->conditionallyAddItem('requestable') + // ->conditionallyAddItem('supplier_id') + // ->conditionallyAddItem('warranty_months') + // ->conditionallyAddItem('next_audit_date'); foreach ($custom_field_columns as $key => $custom_field_column) { $this->conditionallyAddItem($custom_field_column); } @@ -296,10 +323,6 @@ public function update(Request $request) : RedirectResponse /** * Blank out fields that were requested to be blanked out via checkbox */ - if ($request->input('null_name')=='1') { - - $this->update_array['name'] = null; - } if ($request->input('null_purchase_date')=='1') { $this->update_array['purchase_date'] = null; @@ -308,18 +331,10 @@ public function update(Request $request) : RedirectResponse } } - if ($request->input('null_expected_checkin_date')=='1') { - $this->update_array['expected_checkin'] = null; - } - if ($request->input('null_next_audit_date')=='1') { $this->update_array['next_audit_date'] = null; } - if ($request->filled('purchase_cost')) { - $this->update_array['purchase_cost'] = $request->input('purchase_cost'); - } - if ($request->filled('company_id')) { $this->update_array['company_id'] = $request->input('company_id'); if ($request->input('company_id') == 'clear') { @@ -393,6 +408,7 @@ public function update(Request $request) : RedirectResponse * WILL NOT BE logged in the edit log_meta data * ------------------------------------------------------------------------------ */ + // it looks like this doesn't do anything anymore 🤔 $changed = []; foreach ($this->update_array as $key => $value) { From 6c1239eee501be70d2a6ff67182dc37ca2b83008 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 19 Nov 2024 10:02:04 -0600 Subject: [PATCH 14/42] cleaned - more work needs to happen --- app/Actions/Assets/UpdateAssetAction.php | 7 +- app/Http/Controllers/Api/AssetsController.php | 78 ------ .../Assets/BulkAssetsController.php | 245 +----------------- 3 files changed, 7 insertions(+), 323 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 354e1dd4fee1..9c314abc5840 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -157,19 +157,24 @@ public static function run( if ($asset->save()) { // check out stuff + //$location = Location::find($asset->location_id); if (!is_null($assigned_user) && ($target = User::find($assigned_user))) { $location = $target->location_id; } elseif (!is_null($assigned_asset) && ($target = Asset::find($assigned_asset))) { $location = $target->location_id; - Asset::where('assigned_type', \App\Models\Asset::class)->where('assigned_to', $asset->id) ->update(['location_id' => $target->location_id]); } elseif (!is_null($assigned_location) && ($target = Location::find($assigned_location))) { $location = $target->id; } + if (isset($target)) { $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); } + + if ($asset->image) { + $asset->image = $asset->getImageUrl(); + } } return $asset; diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index b11c6a0104c9..f6188396d732 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -658,84 +658,6 @@ public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse } catch (\Exception $e) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.something_went_wrong'))); } - - $asset->fill($request->validated()); - - if ($request->has('model_id')) { - $asset->model()->associate(AssetModel::find($request->validated()['model_id'])); - } - if ($request->has('company_id')) { - $asset->company_id = Company::getIdForCurrentUser($request->validated()['company_id']); - } - if ($request->has('rtd_location_id') && !$request->has('location_id')) { - $asset->location_id = $request->validated()['rtd_location_id']; - } - if ($request->input('last_audit_date')) { - $asset->last_audit_date = Carbon::parse($request->input('last_audit_date'))->startOfDay()->format('Y-m-d H:i:s'); - } - - /** - * this is here just legacy reasons. Api\AssetController - * used image_source once to allow encoded image uploads. - */ - if ($request->has('image_source')) { - $request->offsetSet('image', $request->offsetGet('image_source')); - } - - $asset = $request->handleImages($asset); - $model = $asset->model; - - // Update custom fields - $problems_updating_encrypted_custom_fields = false; - if (($model) && (isset($model->fieldset))) { - foreach ($model->fieldset->fields as $field) { - $field_val = $request->input($field->db_column, null); - - if ($request->has($field->db_column)) { - if ($field->element == 'checkbox') { - if(is_array($field_val)) { - $field_val = implode(',', $field_val); - } - } - if ($field->field_encrypted == '1') { - if (Gate::allows('assets.view.encrypted_custom_fields')) { - $field_val = Crypt::encrypt($field_val); - } else { - $problems_updating_encrypted_custom_fields = true; - continue; - } - } - $asset->{$field->db_column} = $field_val; - } - } - } - if ($asset->save()) { - //if (($request->filled('assigned_user')) && ($target = User::find($request->get('assigned_user')))) { - // $location = $target->location_id; - //} elseif (($request->filled('assigned_asset')) && ($target = Asset::find($request->get('assigned_asset')))) { - // $location = $target->location_id; - // - // Asset::where('assigned_type', \App\Models\Asset::class)->where('assigned_to', $asset->id) - // ->update(['location_id' => $target->location_id]); - //} elseif (($request->filled('assigned_location')) && ($target = Location::find($request->get('assigned_location')))) { - // $location = $target->id; - //} - // - //if (isset($target)) { - // $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); - //} - - if ($asset->image) { - $asset->image = $asset->getImageUrl(); - } - - if ($problems_updating_encrypted_custom_fields) { - return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.encrypted_warning'))); - } else { - return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.success'))); - } - } - return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 200); } diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 4c3c763b30af..e7a8184ddc7f 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -249,250 +249,7 @@ public function update(ImageUploadRequest $request): RedirectResponse if (!empty($errors)) { return redirect($bulk_back_url)->with('bulk_asset_errors', $errors); } - return redirect()->to('index')->with('success', trans('admin/hardware/message.update.assets_do_not_exist')); - - - $has_errors = 0; - $error_array = array(); - - // Get the back url from the session and then destroy the session - - $custom_field_columns = CustomField::all()->pluck('db_column')->toArray(); - - - - - - $assets = Asset::whereIn('id', $request->input('ids'))->get(); - - - - /** - * If ANY of these are filled, prepare to update the values on the assets. - * - * Additional checks will be needed for some of them to make sure the values - * make sense (for example, changing the status ID to something incompatible with - * its checkout status. - */ - - if ( - ($request->anyFilled($custom_field_columns)) - - ) { - // Let's loop through those assets and build an update array - foreach ($assets as $asset) { - - $this->update_array = []; - - /** - * Leave out model_id and status here because we do math on that later. We have to do some - * extra validation and checks on those two. - * - * It's tempting to make these match the request check above, but some of these values require - * extra work to make sure the data makes sense. - */ - //$this->conditionallyAddItem('name') - // ->conditionallyAddItem('purchase_date') - // ->conditionallyAddItem('expected_checkin') - // ->conditionallyAddItem('order_number') - // ->conditionallyAddItem('requestable') - // ->conditionallyAddItem('supplier_id') - // ->conditionallyAddItem('warranty_months') - // ->conditionallyAddItem('next_audit_date'); - foreach ($custom_field_columns as $key => $custom_field_column) { - $this->conditionallyAddItem($custom_field_column); - } - - if (!($asset->eol_explicit)) { - if ($request->filled('model_id')) { - $model = AssetModel::find($request->input('model_id')); - if ($model->eol > 0) { - if ($request->filled('purchase_date')) { - $this->update_array['asset_eol_date'] = Carbon::parse($request->input('purchase_date'))->addMonths($model->eol)->format('Y-m-d'); - } else { - $this->update_array['asset_eol_date'] = Carbon::parse($asset->purchase_date)->addMonths($model->eol)->format('Y-m-d'); - } - } else { - $this->update_array['asset_eol_date'] = null; - } - } elseif (($request->filled('purchase_date')) && ($asset->model->eol > 0)) { - $this->update_array['asset_eol_date'] = Carbon::parse($request->input('purchase_date'))->addMonths($asset->model->eol)->format('Y-m-d'); - } - } - - /** - * Blank out fields that were requested to be blanked out via checkbox - */ - - if ($request->input('null_purchase_date')=='1') { - $this->update_array['purchase_date'] = null; - if (!($asset->eol_explicit)) { - $this->update_array['asset_eol_date'] = null; - } - } - - if ($request->input('null_next_audit_date')=='1') { - $this->update_array['next_audit_date'] = null; - } - - if ($request->filled('company_id')) { - $this->update_array['company_id'] = $request->input('company_id'); - if ($request->input('company_id') == 'clear') { - $this->update_array['company_id'] = null; - } - } - - /** - * We're trying to change the model ID - we need to do some extra checks here to make sure - * the custom field values work for the custom fieldset rules around this asset. Uniqueness - * and requiredness across the fieldset is particularly important, since those are - * fieldset-specific attributes. - */ - if ($request->filled('model_id')) { - $this->update_array['model_id'] = AssetModel::find($request->input('model_id'))->id; - } - - /** - * We're trying to change the status ID - we need to do some extra checks here to - * make sure the status label type is one that makes sense for the state of the asset, - * for example, we shouldn't be able to make an asset archived if it's currently assigned - * to someone/something. - */ - if ($request->filled('status_id')) { - $updated_status = Statuslabel::find($request->input('status_id')); - - // We cannot assign a non-deployable status type if the asset is already assigned. - // This could probably be added to a form request. - // If the asset isn't assigned, we don't care what the status is. - // Otherwise we need to make sure the status type is still a deployable one. - if ( - ($asset->assigned_to == '') - || ($updated_status->deployable == '1') && ($asset->assetstatus->deployable == '1') - ) { - $this->update_array['status_id'] = $updated_status->id; - } - - } - - /** - * We're changing the location ID - figure out which location we should apply - * this change to: - * - * 0 - RTD location only - * 1 - location ID and RTD location ID - * 2 - location ID only - * - * Note: this is kinda dumb and we should just use human-readable values IMHO. - snipe - */ - if ($request->filled('rtd_location_id')) { - - if (($request->filled('update_real_loc')) && (($request->input('update_real_loc')) == '0')) { - $this->update_array['rtd_location_id'] = $request->input('rtd_location_id'); - } - - if (($request->filled('update_real_loc')) && (($request->input('update_real_loc')) == '1')) { - $this->update_array['location_id'] = $request->input('rtd_location_id'); - $this->update_array['rtd_location_id'] = $request->input('rtd_location_id'); - } - - if (($request->filled('update_real_loc')) && (($request->input('update_real_loc')) == '2')) { - $this->update_array['location_id'] = $request->input('rtd_location_id'); - } - - } - - - /** - * ------------------------------------------------------------------------------ - * ANYTHING that happens past this foreach - * WILL NOT BE logged in the edit log_meta data - * ------------------------------------------------------------------------------ - */ - // it looks like this doesn't do anything anymore 🤔 - $changed = []; - - foreach ($this->update_array as $key => $value) { - - if ($this->update_array[$key] != $asset->{$key}) { - $changed[$key]['old'] = $asset->{$key}; - $changed[$key]['new'] = $this->update_array[$key]; - } - - } - - /** - * Start all the custom fields shenanigans - */ - - // Does the model have a fieldset? - if ($asset->model->fieldset) { - foreach ($asset->model->fieldset->fields as $field) { - - if ((array_key_exists($field->db_column, $this->update_array)) && ($field->field_encrypted == '1')) { - if (Gate::allows('admin')) { - $decrypted_old = Helper::gracefulDecrypt($field, $asset->{$field->db_column}); - - /* - * Check if the decrypted existing value is different from one we just submitted - * and if not, pull it out of the object since it shouldn't really be updating at all. - * If we don't do this, it will try to re-encrypt it, and the same value encrypted two - * different times will have different values, so it will *look* like it was updated - * but it wasn't. - */ - if ($decrypted_old != $this->update_array[$field->db_column]) { - $asset->{$field->db_column} = Crypt::encrypt($this->update_array[$field->db_column]); - } else { - /* - * Remove the encrypted custom field from the update_array, since nothing changed - */ - unset($this->update_array[$field->db_column]); - unset($asset->{$field->db_column}); - } - - /* - * These custom fields aren't encrypted, just carry on as usual - */ - } - } else { - - if ((array_key_exists($field->db_column, $this->update_array)) && ($asset->{$field->db_column} != $this->update_array[$field->db_column])) { - - // Check if this is an array, and if so, flatten it - if (is_array($this->update_array[$field->db_column])) { - $asset->{$field->db_column} = implode(', ', $this->update_array[$field->db_column]); - } else { - $asset->{$field->db_column} = $this->update_array[$field->db_column]; - } - } - } - - } // endforeach - } - - - // Check if it passes validation, and then try to save - if (!$asset->update($this->update_array)) { - - // Build the error array - foreach ($asset->getErrors()->toArray() as $key => $message) { - for ($x = 0; $x < count($message); $x++) { - $error_array[$key][] = trans('general.asset') . ' ' . $asset->id . ': ' . $message[$x]; - $has_errors++; - } - } - - } // end if saved - - } // end asset foreach - - if ($has_errors > 0) { - return redirect($bulk_back_url)->with('bulk_asset_errors', $error_array); - } - - return redirect($bulk_back_url)->with('success', trans('admin/hardware/message.update.success')); - } - // no values given, nothing to update - return redirect($bulk_back_url)->with('warning', trans('admin/hardware/message.update.nothing_updated')); + return redirect($bulk_back_url)->with('success', trans('bulk.update.success')); } /** From 3e97f0274cf24a3d8b5bd06352e0cabba14b37fb Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 19 Nov 2024 10:04:10 -0600 Subject: [PATCH 15/42] rm extra code --- app/Http/Controllers/Api/AssetsController.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index f6188396d732..c82a6e46df81 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -34,6 +34,7 @@ use Illuminate\Support\Facades\DB; use Illuminate\Http\Request; use Illuminate\Support\Facades\Route; +use PHPUnit\TextUI\Help; use Watson\Validating\ValidationException; @@ -650,7 +651,8 @@ public function store(StoreAssetRequest $request): JsonResponse public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse { try { - UpdateAssetAction::run($asset, $request, ...$request->validated()); + $updatedAsset = UpdateAssetAction::run($asset, $request, ...$request->validated()); + return response()->json(Helper::formatStandardApiResponse('success', trans('admin/hardware/message.update.success'))); } catch (CheckoutNotAllowed $e) { return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage()), 200); } catch (ValidationException $e) { From 9f815996c741326300b8825dcfc560a3d71dd7c0 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 19 Nov 2024 14:45:54 -0600 Subject: [PATCH 16/42] tests 'n stuff --- app/Actions/Assets/StoreAssetAction.php | 2 -- app/Actions/Assets/UpdateAssetAction.php | 14 ++++++++++++-- app/Http/Controllers/Api/AssetsController.php | 4 ++-- app/Http/Controllers/Assets/AssetsController.php | 6 ++---- .../Controllers/Assets/BulkAssetsController.php | 1 + tests/Feature/Assets/Api/UpdateAssetTest.php | 1 + tests/Unit/AssetTest.php | 6 ++++-- tests/Unit/DepreciationTest.php | 4 ++-- 8 files changed, 24 insertions(+), 14 deletions(-) diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index de04dbd6c9f0..9305b031a329 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -46,8 +46,6 @@ public static function run( $assigned_user = null, $assigned_asset = null, $assigned_location = null, - $custom_fields = null, - $last_audit_date = null, ) { diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 9c314abc5840..b083b78422e6 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -14,9 +14,13 @@ use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Log; +use Watson\Validating\ValidationException; class UpdateAssetAction { + /** + * @throws ValidationException + */ public static function run( Asset $asset, ImageUploadRequest $request, //very much would like this to go away @@ -111,7 +115,11 @@ public static function run( $asset->serial = $serial; - $asset->name = $name; + if ($request->filled('null_name')) { + $asset->name = null; + } else { + $asset->name = $name ?? $asset->name; + } $asset->company_id = Company::getIdForCurrentUser($company_id); $asset->model_id = $model_id ?? $asset->model_id; $asset->order_number = $order_number ?? $asset->order_number; @@ -127,10 +135,12 @@ public static function run( // FIXME: No idea why this is returning a Builder error on db_column_name. // Need to investigate and fix. Using static method for now. - $model = AssetModel::find($request->get('model_id')); + $model = $asset->model; if (($model) && ($model->fieldset)) { + dump($model->fieldset->fields); foreach ($model->fieldset->fields as $field) { + if ($field->field_encrypted == '1') { if (Gate::allows('assets.view.encrypted_custom_fields')) { if (is_array($request->input($field->db_column))) { diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index c82a6e46df81..4b586260bbf2 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -652,7 +652,7 @@ public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse { try { $updatedAsset = UpdateAssetAction::run($asset, $request, ...$request->validated()); - return response()->json(Helper::formatStandardApiResponse('success', trans('admin/hardware/message.update.success'))); + return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.success'))); } catch (CheckoutNotAllowed $e) { return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage()), 200); } catch (ValidationException $e) { @@ -680,7 +680,7 @@ public function destroy(Asset $asset): JsonResponse return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/hardware/message.delete.success'))); } catch (\Exception $e) { report($e); - return response()->json(Helper::formatStandardApiResponse('error', null, 'something went wrong: ')); + return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.something_went_wrong'))); } } diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index e6394bbda577..555174d9ebdd 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -119,6 +119,7 @@ public function store(StoreAssetRequest $request): RedirectResponse $asset = StoreAssetAction::run( model_id: $request->validated('model_id'), status_id: $request->validated('status_id'), + request: $request, name: $request->validated('name'), serial: $request->has('serials') ? $serials[$key] : null, company_id: $request->validated('company_id'), @@ -140,8 +141,6 @@ public function store(StoreAssetRequest $request): RedirectResponse assigned_user: $request->validated('assigned_user'), assigned_asset: $request->validated('assigned_asset'), assigned_location: $request->validated('assigned_location'), - custom_fields: $custom_fields, - request: $request, //this is just for the handleImages method... would love to figure out a different way of doing this last_audit_date: $request->validated('last_audit_date'), ); } @@ -153,7 +152,6 @@ public function store(StoreAssetRequest $request): RedirectResponse return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.create.error')); } catch (\Exception $e) { report($e); - dd($e); return redirect()->back()->with('error', 'something bad'); } } @@ -283,7 +281,7 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo * @param int $assetId * @since [v1.0] */ - public function destroy(Asset $asset): RedirectResponse + public function destroy(Asset $asset, $request): RedirectResponse { $this->authorize('delete', $asset); try { diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index e7a8184ddc7f..d9849cbf7af6 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Assets; +use App\Actions\Assets\StoreAssetAction; use App\Actions\Assets\UpdateAssetAction; use App\Helpers\Helper; use App\Http\Controllers\CheckInOutRequest; diff --git a/tests/Feature/Assets/Api/UpdateAssetTest.php b/tests/Feature/Assets/Api/UpdateAssetTest.php index df4448a2db5c..55983d6c15ac 100644 --- a/tests/Feature/Assets/Api/UpdateAssetTest.php +++ b/tests/Feature/Assets/Api/UpdateAssetTest.php @@ -80,6 +80,7 @@ public function testAllAssetAttributesAreStored() ->assertStatusMessageIs('success') ->json(); + dd($response); $updatedAsset = Asset::find($response['payload']['id']); $this->assertEquals('2024-06-02', $updatedAsset->asset_eol_date); diff --git a/tests/Unit/AssetTest.php b/tests/Unit/AssetTest.php index d0f3af623373..bf9d4f6fdc0a 100644 --- a/tests/Unit/AssetTest.php +++ b/tests/Unit/AssetTest.php @@ -7,6 +7,7 @@ use Carbon\Carbon; use Tests\TestCase; use App\Models\Setting; +use Watson\Validating\ValidationException; class AssetTest extends TestCase { @@ -31,7 +32,8 @@ public function testAutoIncrementCollision() $b = Asset::factory()->make(['asset_tag' => Asset::autoincrement_asset() ]); $this->assertTrue($a->save()); - $this->assertFalse($b->save()); + $this->expectException(ValidationException::class); + $b->save(); } public function testAutoIncrementDouble() @@ -181,7 +183,7 @@ public function testWarrantyExpiresAttribute() ] )->id, 'warranty_months' => 24, - 'purchase_date' => Carbon::createFromDate(2017, 1, 1)->hour(0)->minute(0)->second(0) + 'purchase_date' => Carbon::createFromDate(2017, 1, 1)->hour(0)->minute(0)->second(0)->format("Y-m-d") ]); diff --git a/tests/Unit/DepreciationTest.php b/tests/Unit/DepreciationTest.php index c170ac0e1dfc..f3d1097e8d14 100644 --- a/tests/Unit/DepreciationTest.php +++ b/tests/Unit/DepreciationTest.php @@ -39,7 +39,7 @@ public function testDepreciationAmount() ->create( [ 'category_id' => Category::factory()->assetLaptopCategory()->create(), - 'purchase_date' => now()->subDecade(), + 'purchase_date' => now()->subDecade()->format("Y-m-d"), 'purchase_cost' => 4000, ]); $asset->model->update([ @@ -63,7 +63,7 @@ public function testDepreciationPercentage() ->create( [ 'category_id' => Category::factory()->assetLaptopCategory()->create(), - 'purchase_date' => now()->subDecade(), + 'purchase_date' => now()->subDecade()->format("Y-m-d"), 'purchase_cost' => 4000, ]); $asset->model->update([ From 3cf583ab0305bc286dbf7f09910758030d34b1c7 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 19 Nov 2024 17:45:35 -0600 Subject: [PATCH 17/42] Add CustomFieldPermissionException handling --- app/Actions/Assets/UpdateAssetAction.php | 54 ++++++++++++++----- .../CustomFieldPermissionException.php | 10 ++++ app/Http/Controllers/Api/AssetsController.php | 4 +- .../Assets/BulkAssetsController.php | 8 +++ tests/Feature/Assets/Api/UpdateAssetTest.php | 3 +- 5 files changed, 62 insertions(+), 17 deletions(-) create mode 100644 app/Exceptions/CustomFieldPermissionException.php diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index b083b78422e6..1d02d19ae36a 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -3,6 +3,7 @@ namespace App\Actions\Assets; use App\Events\CheckoutableCheckedIn; +use App\Exceptions\CustomFieldPermissionException; use App\Http\Requests\ImageUploadRequest; use App\Models\Asset; use App\Models\AssetModel; @@ -20,6 +21,7 @@ class UpdateAssetAction { /** * @throws ValidationException + * @throws CustomFieldPermissionException */ public static function run( Asset $asset, @@ -135,26 +137,50 @@ public static function run( // FIXME: No idea why this is returning a Builder error on db_column_name. // Need to investigate and fix. Using static method for now. + //if (($model) && ($model->fieldset)) { + // dump($model->fieldset->fields); + // foreach ($model->fieldset->fields as $field) { + // + // + // if ($field->field_encrypted == '1') { + // if (Gate::allows('assets.view.encrypted_custom_fields')) { + // if (is_array($request->input($field->db_column))) { + // $asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column))); + // } else { + // $asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column)); + // } + // throw new CustomFieldPermissionException(); + // continue; + // } + // } else { + // if (is_array($request->input($field->db_column))) { + // $asset->{$field->db_column} = implode(', ', $request->input($field->db_column)); + // } else { + // $asset->{$field->db_column} = $request->input($field->db_column); + // } + // } + // } + //} $model = $asset->model; - if (($model) && ($model->fieldset)) { - dump($model->fieldset->fields); + if (($model) && (isset($model->fieldset))) { foreach ($model->fieldset->fields as $field) { + $field_val = $request->input($field->db_column, null); - - if ($field->field_encrypted == '1') { - if (Gate::allows('assets.view.encrypted_custom_fields')) { - if (is_array($request->input($field->db_column))) { - $asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column))); - } else { - $asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column)); + if ($request->has($field->db_column)) { + if ($field->element == 'checkbox') { + if (is_array($field_val)) { + $field_val = implode(',', $field_val); } } - } else { - if (is_array($request->input($field->db_column))) { - $asset->{$field->db_column} = implode(', ', $request->input($field->db_column)); - } else { - $asset->{$field->db_column} = $request->input($field->db_column); + if ($field->field_encrypted == '1') { + if (Gate::allows('assets.view.encrypted_custom_fields')) { + $field_val = Crypt::encrypt($field_val); + } else { + throw new CustomFieldPermissionException(); + continue; + } } + $asset->{$field->db_column} = $field_val; } } } diff --git a/app/Exceptions/CustomFieldPermissionException.php b/app/Exceptions/CustomFieldPermissionException.php new file mode 100644 index 000000000000..4425d00cb8dd --- /dev/null +++ b/app/Exceptions/CustomFieldPermissionException.php @@ -0,0 +1,10 @@ +validated('assigned_user'), assigned_asset: $request->validated('assigned_asset'), assigned_location: $request->validated('assigned_location'), - custom_fields: $custom_fields, request: $request, //this is just for the handleImages method... last_audit_date: $request->validated('last_audit_date'), ); @@ -657,6 +657,8 @@ public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage()), 200); } catch (ValidationException $e) { return response()->json(Helper::formatStandardApiResponse('error', null, $e->getErrors()), 200); + } catch (CustomFieldPermissionException $e) { + return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.encrypted_warning'))); } catch (\Exception $e) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.something_went_wrong'))); } diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index d9849cbf7af6..b082905a92f7 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -4,6 +4,7 @@ use App\Actions\Assets\StoreAssetAction; use App\Actions\Assets\UpdateAssetAction; +use App\Exceptions\CustomFieldPermissionException; use App\Helpers\Helper; use App\Http\Controllers\CheckInOutRequest; use App\Http\Controllers\Controller; @@ -209,6 +210,7 @@ public function update(ImageUploadRequest $request): RedirectResponse $this->authorize('update', Asset::class); // Get the back url from the session and then destroy the session $bulk_back_url = route('hardware.index'); + $custom_field_problem = false; // is this necessary? if (!$request->filled('ids') || count($request->input('ids')) == 0) { return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.update.no_assets_selected')); @@ -242,6 +244,9 @@ public function update(ImageUploadRequest $request): RedirectResponse // catch exceptions } catch (ValidationException $e) { $errors[$key] = $e->getMessage(); + + } catch (CustomFieldPermissionException $e) { + $custom_field_problem = true; } catch (\Exception $e) { report($e); $errors[$key] = trans('general.something_went_wrong'); @@ -250,6 +255,9 @@ public function update(ImageUploadRequest $request): RedirectResponse if (!empty($errors)) { return redirect($bulk_back_url)->with('bulk_asset_errors', $errors); } + if ($custom_field_problem) { + return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.update.encrypted_warning')); + } return redirect($bulk_back_url)->with('success', trans('bulk.update.success')); } diff --git a/tests/Feature/Assets/Api/UpdateAssetTest.php b/tests/Feature/Assets/Api/UpdateAssetTest.php index 55983d6c15ac..1cb6879d1ed4 100644 --- a/tests/Feature/Assets/Api/UpdateAssetTest.php +++ b/tests/Feature/Assets/Api/UpdateAssetTest.php @@ -80,14 +80,13 @@ public function testAllAssetAttributesAreStored() ->assertStatusMessageIs('success') ->json(); - dd($response); $updatedAsset = Asset::find($response['payload']['id']); $this->assertEquals('2024-06-02', $updatedAsset->asset_eol_date); $this->assertEquals('random_string', $updatedAsset->asset_tag); $this->assertEquals($userAssigned->id, $updatedAsset->assigned_to); $this->assertTrue($updatedAsset->company->is($company)); - $this->assertTrue($updatedAsset->location->is($location)); + $this->assertTrue($updatedAsset->location->is($location)); //fix all location setting $this->assertTrue($updatedAsset->model->is($model)); $this->assertEquals('A New Asset', $updatedAsset->name); $this->assertEquals('Some notes', $updatedAsset->notes); From 0353c7a03b7bc51d4c555f1a117201db651bc396 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 19 Nov 2024 17:56:56 -0600 Subject: [PATCH 18/42] Fix error handling in BulkAssetsController Updated error handling to ensure all exceptions are properly stored in the errors array and removed unused custom_field_problem variable. Also, uncommented location assignment code in UpdateAssetAction for clearer logic. --- app/Actions/Assets/UpdateAssetAction.php | 2 +- app/Http/Controllers/Assets/BulkAssetsController.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 1d02d19ae36a..ab93b2dd9743 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -193,7 +193,7 @@ public static function run( if ($asset->save()) { // check out stuff - //$location = Location::find($asset->location_id); + $location = Location::find($asset->location_id); if (!is_null($assigned_user) && ($target = User::find($assigned_user))) { $location = $target->location_id; } elseif (!is_null($assigned_asset) && ($target = Asset::find($assigned_asset))) { diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index b082905a92f7..837c44d6bf73 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -244,9 +244,9 @@ public function update(ImageUploadRequest $request): RedirectResponse // catch exceptions } catch (ValidationException $e) { $errors[$key] = $e->getMessage(); - } catch (CustomFieldPermissionException $e) { - $custom_field_problem = true; + $errors[$key] = $e->getMessage(); + //$custom_field_problem = true; } catch (\Exception $e) { report($e); $errors[$key] = trans('general.something_went_wrong'); @@ -255,9 +255,9 @@ public function update(ImageUploadRequest $request): RedirectResponse if (!empty($errors)) { return redirect($bulk_back_url)->with('bulk_asset_errors', $errors); } - if ($custom_field_problem) { - return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.update.encrypted_warning')); - } + //if ($custom_field_problem) { + // return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.update.encrypted_warning')); + //} return redirect($bulk_back_url)->with('success', trans('bulk.update.success')); } From 94e7310cd93a64c49088decee1e8aa42980695dc Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 19 Nov 2024 19:57:26 -0600 Subject: [PATCH 19/42] Fix bulk asset update error handling and improve access checks Improved the error handling in the BulkAssetsController by re-enabling custom field permission checks. Updated the UpdateAssetAction to better handle encrypted custom fields and streamlined user roles in the BulkEditAssetsTest. --- app/Actions/Assets/UpdateAssetAction.php | 6 ++++-- app/Http/Controllers/Assets/BulkAssetsController.php | 10 +++++----- tests/Feature/Assets/Ui/BulkEditAssetsTest.php | 5 +++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index ab93b2dd9743..11c0085af0ca 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -6,7 +6,6 @@ use App\Exceptions\CustomFieldPermissionException; use App\Http\Requests\ImageUploadRequest; use App\Models\Asset; -use App\Models\AssetModel; use App\Models\Company; use App\Models\Location; use App\Models\Statuslabel; @@ -137,6 +136,7 @@ public static function run( // FIXME: No idea why this is returning a Builder error on db_column_name. // Need to investigate and fix. Using static method for now. + // the gui method //if (($model) && ($model->fieldset)) { // dump($model->fieldset->fields); // foreach ($model->fieldset->fields as $field) { @@ -161,6 +161,7 @@ public static function run( // } // } //} + // the api method $model = $asset->model; if (($model) && (isset($model->fieldset))) { foreach ($model->fieldset->fields as $field) { @@ -173,11 +174,12 @@ public static function run( } } if ($field->field_encrypted == '1') { + dump(Gate::allows('assets.view.encrypted_custom_fields')); + dump(auth()->user()->can('assets.view.encrypted_custom_fields')); if (Gate::allows('assets.view.encrypted_custom_fields')) { $field_val = Crypt::encrypt($field_val); } else { throw new CustomFieldPermissionException(); - continue; } } $asset->{$field->db_column} = $field_val; diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 837c44d6bf73..3281d0f49a13 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -245,8 +245,8 @@ public function update(ImageUploadRequest $request): RedirectResponse } catch (ValidationException $e) { $errors[$key] = $e->getMessage(); } catch (CustomFieldPermissionException $e) { - $errors[$key] = $e->getMessage(); - //$custom_field_problem = true; + //$errors[$key] = $e->getMessage(); + $custom_field_problem = true; } catch (\Exception $e) { report($e); $errors[$key] = trans('general.something_went_wrong'); @@ -255,9 +255,9 @@ public function update(ImageUploadRequest $request): RedirectResponse if (!empty($errors)) { return redirect($bulk_back_url)->with('bulk_asset_errors', $errors); } - //if ($custom_field_problem) { - // return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.update.encrypted_warning')); - //} + if ($custom_field_problem) { + return redirect($bulk_back_url)->with('error', trans('admin/hardware/message.update.encrypted_warning')); + } return redirect($bulk_back_url)->with('success', trans('bulk.update.success')); } diff --git a/tests/Feature/Assets/Ui/BulkEditAssetsTest.php b/tests/Feature/Assets/Ui/BulkEditAssetsTest.php index 44e905248208..7962663cb4ea 100644 --- a/tests/Feature/Assets/Ui/BulkEditAssetsTest.php +++ b/tests/Feature/Assets/Ui/BulkEditAssetsTest.php @@ -211,7 +211,7 @@ public function testBulkEditAssetsAcceptsAndUpdatesEncryptedCustomFields() $id_array = $assets->pluck('id')->toArray(); - $this->actingAs(User::factory()->admin()->create())->post(route('hardware/bulksave'), [ + $this->actingAs(User::factory()->superuser()->create())->post(route('hardware/bulksave'), [ 'ids' => $id_array, $encrypted->db_column => 'New Encrypted Text', ])->assertStatus(302); @@ -225,7 +225,8 @@ public function testBulkEditAssetsRequiresadminToUpdateEncryptedCustomFields() { $this->markIncompleteIfMySQL('Custom Fields tests do not work on mysql'); $edit_user = User::factory()->editAssets()->create(); - $admin_user = User::factory()->admin()->create(); + // admin used to work, but now only superuser does???? + $admin_user = User::factory()->superuser()->create(); CustomField::factory()->testEncrypted()->create(); From 1704a07663a880dcd1c46333b60e9e1f94ea2ad0 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 20 Nov 2024 09:58:13 -0600 Subject: [PATCH 20/42] Allow null values for asset dates in update action This commit improves the asset update functionality by allowing null values for purchase date, next audit date, and expected check-in date when specified in the request. Additionally, minor corrections were made in the test code to enhance readability and correctness. --- app/Actions/Assets/UpdateAssetAction.php | 23 ++++++++++++++++--- .../Feature/Assets/Ui/BulkEditAssetsTest.php | 2 +- tests/Unit/DepreciationTest.php | 6 +++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 11c0085af0ca..887b4a35caa0 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -55,8 +55,21 @@ public static function run( $asset->status_id = $status_id ?? $asset->status_id; $asset->warranty_months = $warranty_months ?? $asset->warranty_months; $asset->purchase_cost = $purchase_cost ?? $asset->purchase_cost; - $asset->purchase_date = $purchase_date ?? $asset->purchase_date?->format('Y-m-d'); - $asset->next_audit_date = $next_audit_date ?? $asset->next_audit_date; + if ($request->input('null_purchase_date') === '1') { + dump('filled'); + $asset->purchase_date = null; + if (!($asset->eol_explicit)) { + $asset->asset_eol_date = null; + } + } else { + $asset->purchase_date = $purchase_date ?? $asset->purchase_date?->format('Y-m-d'); + } + if ($request->input('null_next_audit_date') == '1') { + $asset->next_audit_date = null; + } else { + $asset->next_audit_date = $next_audit_date ?? $asset->next_audit_date; + } + $asset->last_audit_date = $last_audit_date ?? $asset->last_audit_date; if ($purchase_date && !$asset_eol_date && ($asset->model->eol > 0)) { $asset->purchase_date = $purchase_date ?? $asset->purchase_date?->format('Y-m-d'); @@ -79,7 +92,11 @@ public static function run( $asset->eol_explicit = false; } $asset->supplier_id = $supplier_id; - $asset->expected_checkin = $expected_checkin; + if ($request->input('null_expected_checkin_date') == '1') { + $asset->expected_checkin = null; + } else { + $asset->expected_checkin = $expected_checkin ?? $asset->expected_checkin; + } $asset->requestable = $requestable; $asset->location_id = $location_id; diff --git a/tests/Feature/Assets/Ui/BulkEditAssetsTest.php b/tests/Feature/Assets/Ui/BulkEditAssetsTest.php index 7962663cb4ea..5b6779032bdf 100644 --- a/tests/Feature/Assets/Ui/BulkEditAssetsTest.php +++ b/tests/Feature/Assets/Ui/BulkEditAssetsTest.php @@ -221,7 +221,7 @@ public function testBulkEditAssetsAcceptsAndUpdatesEncryptedCustomFields() }); } - public function testBulkEditAssetsRequiresadminToUpdateEncryptedCustomFields() + public function testBulkEditAssetsRequiresAdminToUpdateEncryptedCustomFields() { $this->markIncompleteIfMySQL('Custom Fields tests do not work on mysql'); $edit_user = User::factory()->editAssets()->create(); diff --git a/tests/Unit/DepreciationTest.php b/tests/Unit/DepreciationTest.php index f3d1097e8d14..7fa18c3c126c 100644 --- a/tests/Unit/DepreciationTest.php +++ b/tests/Unit/DepreciationTest.php @@ -38,7 +38,8 @@ public function testDepreciationAmount() ->laptopMbp() ->create( [ - 'category_id' => Category::factory()->assetLaptopCategory()->create(), + //not sure how this ever worked... do these need a category?? + //'category_id' => Category::factory()->assetLaptopCategory()->create(), 'purchase_date' => now()->subDecade()->format("Y-m-d"), 'purchase_cost' => 4000, ]); @@ -62,7 +63,8 @@ public function testDepreciationPercentage() ->laptopMbp() ->create( [ - 'category_id' => Category::factory()->assetLaptopCategory()->create(), + //not sure how this ever worked... + //'category_id' => Category::factory()->assetLaptopCategory()->create(), 'purchase_date' => now()->subDecade()->format("Y-m-d"), 'purchase_cost' => 4000, ]); From 03d9f936abd29c6613cf8425071c3833134b27b9 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 20 Nov 2024 10:17:35 -0600 Subject: [PATCH 21/42] Update asset model association and location logic handling Refactor the asset update process to properly associate models and handle location ID changes based on request input. Introduce checks for model_id, company_id, and last_audit_date to ensure correct updates. Add commentary to clarify the handling of location ID logic and temporarily resolve issues with location assignment when requests do not expect JSON responses. --- app/Actions/Assets/UpdateAssetAction.php | 31 +++++++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 887b4a35caa0..10b3801ab878 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -6,6 +6,7 @@ use App\Exceptions\CustomFieldPermissionException; use App\Http\Requests\ImageUploadRequest; use App\Models\Asset; +use App\Models\AssetModel; use App\Models\Company; use App\Models\Location; use App\Models\Statuslabel; @@ -101,7 +102,19 @@ public static function run( $asset->location_id = $location_id; - !$isBulk ?? $asset->rtd_location_id = $rtd_location_id; + $asset->rtd_location_id = $rtd_location_id ?? $asset->rtd_location_id; + if ($request->has('model_id')) { + $asset->model()->associate(AssetModel::find($request->validated()['model_id'])); + } + if ($request->has('company_id')) { + $asset->company_id = Company::getIdForCurrentUser($request->validated()['company_id']); + } + if ($request->has('rtd_location_id') && !$request->has('location_id')) { + $asset->location_id = $request->validated()['rtd_location_id']; + } + if ($request->input('last_audit_date')) { + $asset->last_audit_date = Carbon::parse($request->input('last_audit_date'))->startOfDay()->format('Y-m-d H:i:s'); + } $asset->byod = $byod; $status = Statuslabel::find($status_id); @@ -117,7 +130,9 @@ public static function run( event(new CheckoutableCheckedIn($asset, $target, auth()->user(), 'Checkin on asset update', date('Y-m-d H:i:s'), $originalValues)); } - if ($asset->assigned_to == '') { + //this is causing an issue while setting location_id - this came from the gui but doesn't seem to work as expected in the api - + //throwing on !expectsJson for now until we can work out how to handle this better + if ($asset->assigned_to == '' && !$request->expectsJson()) { $asset->location_id = $rtd_location_id; } @@ -237,8 +252,17 @@ public static function run( private static function bulkUpdate($asset, $request): void { + /** + * We're changing the location ID - figure out which location we should apply + * this change to: + * + * 0 - RTD location only + * 1 - location ID and RTD location ID + * 2 - location ID only + * + * Note: this is kinda dumb and we should just use human-readable values IMHO. - snipe + */ if ($request->filled('rtd_location_id')) { - if (($request->filled('update_real_loc')) && (($request->input('update_real_loc')) == '0')) { $asset->rtd_location_id = $request->input('rtd_location_id'); } @@ -251,7 +275,6 @@ private static function bulkUpdate($asset, $request): void if (($request->filled('update_real_loc')) && (($request->input('update_real_loc')) == '2')) { $asset->location_id = $request->input('rtd_location_id'); } - } } From 617fe8569984dbcba20019ab95b95fbd439bdc9e Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 20 Nov 2024 13:11:03 -0600 Subject: [PATCH 22/42] Fix typos and use correct array notation in asset updates Corrected spelling mistakes in test files and updated code to use the correct array notation in request validation for asset updates. This ensures better readability and consistency in the codebase while preventing potential errors related to incorrect data handling. --- app/Actions/Assets/UpdateAssetAction.php | 11 ++++++++--- app/Http/Controllers/Assets/AssetsController.php | 7 ++++--- tests/Feature/Assets/Ui/EditAssetTest.php | 6 +++--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 10b3801ab878..e7b09fc7eef5 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -104,13 +104,13 @@ public static function run( $asset->rtd_location_id = $rtd_location_id ?? $asset->rtd_location_id; if ($request->has('model_id')) { - $asset->model()->associate(AssetModel::find($request->validated()['model_id'])); + $asset->model()->associate(AssetModel::find($request->validated('model_id'))); } if ($request->has('company_id')) { - $asset->company_id = Company::getIdForCurrentUser($request->validated()['company_id']); + $asset->company_id = Company::getIdForCurrentUser($request->validated('company_id')); } if ($request->has('rtd_location_id') && !$request->has('location_id')) { - $asset->location_id = $request->validated()['rtd_location_id']; + $asset->location_id = $request->validated('rtd_location_id'); } if ($request->input('last_audit_date')) { $asset->last_audit_date = Carbon::parse($request->input('last_audit_date'))->startOfDay()->format('Y-m-d H:i:s'); @@ -121,13 +121,17 @@ public static function run( // This is a non-deployable status label - we should check the asset back in. if (($status && $status->getStatuslabelType() != 'deployable') && ($target = $asset->assignedTo)) { + dump('status logic'); $originalValues = $asset->getRawOriginal(); $asset->assigned_to = null; $asset->assigned_type = null; $asset->accepted = null; + dump($asset->assigned_to); event(new CheckoutableCheckedIn($asset, $target, auth()->user(), 'Checkin on asset update', date('Y-m-d H:i:s'), $originalValues)); + // reset this to null so checkout logic doesn't happen below + $target = null; } //this is causing an issue while setting location_id - this came from the gui but doesn't seem to work as expected in the api - @@ -239,6 +243,7 @@ public static function run( } if (isset($target)) { + dump($target); $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); } diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 555174d9ebdd..cf460ab401cb 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -241,7 +241,7 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo $asset_tag = $request->input('asset_tags')[1]; } - $asset = UpdateAssetAction::run( + $updatedAsset = UpdateAssetAction::run( asset: $asset, request: $request, status_id: $request->validated('status_id'), @@ -264,13 +264,14 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo asset_tag: $asset_tag, // same as serials notes: $request->validated('notes'), ); - return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) + dump('returned'.$asset->assigned_to); + return redirect()->to(Helper::getRedirectOption($request, $updatedAsset->id, 'Assets')) ->with('success', trans('admin/hardware/message.update.success')); } catch (ValidationException $e) { return redirect()->back()->withInput()->withErrors($e->getErrors()); } catch (\Exception $e) { report($e); - return redirect()->back()->with('error', trans('admin/hardware/message.update.error'), $asset); + return redirect()->back()->with('error', trans('admin/hardware/message.update.error')); } } diff --git a/tests/Feature/Assets/Ui/EditAssetTest.php b/tests/Feature/Assets/Ui/EditAssetTest.php index 27f00b531321..ebe7bf9a9710 100644 --- a/tests/Feature/Assets/Ui/EditAssetTest.php +++ b/tests/Feature/Assets/Ui/EditAssetTest.php @@ -74,7 +74,7 @@ public function testNewCheckinIsLoggedIfStatusChangedToUndeployable() $user = User::factory()->create(); $deployable_status = Statuslabel::factory()->rtd()->create(); - $achived_status = Statuslabel::factory()->archived()->create(); + $archived_status = Statuslabel::factory()->archived()->create(); $asset = Asset::factory()->assignedToUser($user)->create(['status_id' => $deployable_status->id]); $this->assertTrue($asset->assignedTo->is($user)); @@ -83,7 +83,7 @@ public function testNewCheckinIsLoggedIfStatusChangedToUndeployable() $this->actingAs(User::factory()->viewAssets()->editAssets()->create()) ->from(route('hardware.edit', $asset->id)) ->put(route('hardware.update', $asset->id), [ - 'status_id' => $achived_status->id, + 'status_id' => $archived_status->id, 'model_id' => $asset->model_id, 'asset_tags' => $asset->asset_tag, ], @@ -95,7 +95,7 @@ public function testNewCheckinIsLoggedIfStatusChangedToUndeployable() $asset = Asset::find($asset->id); $this->assertNull($asset->assigned_to); $this->assertNull($asset->assigned_type); - $this->assertEquals($achived_status->id, $asset->status_id); + $this->assertEquals($archived_status->id, $asset->status_id); Event::assertDispatched(function (CheckoutableCheckedIn $event) use ($currentTimestamp) { return Carbon::parse($event->action_date)->diffInSeconds($currentTimestamp) < 2; From 6ddaa0bb3e3cf0991c85d5a66ab0d0eb4e71b4e5 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 20 Nov 2024 13:25:06 -0600 Subject: [PATCH 23/42] Handle exceptions when saving assets in AssetImporter Added try-catch blocks to manage ValidationException and generic exceptions during asset saving. This ensures proper logging of errors and reporting of unexpected issues, improving robustness and error handling in the asset import process. --- app/Importer/AssetImporter.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/app/Importer/AssetImporter.php b/app/Importer/AssetImporter.php index 1112a04e3508..ab38c3672212 100644 --- a/app/Importer/AssetImporter.php +++ b/app/Importer/AssetImporter.php @@ -7,6 +7,7 @@ use App\Models\User; use App\Events\CheckoutableCheckedIn; use Illuminate\Support\Facades\Crypt; +use Watson\Validating\ValidationException; class AssetImporter extends ItemImporter { @@ -172,26 +173,30 @@ public function createAssetIfNotExists(array $row) // This sets an attribute on the Loggable trait for the action log $asset->setImported(true); - if ($asset->save()) { - + try { + $asset->save(); $this->log('Asset '.$this->item['name'].' with serial number '.$this->item['serial'].' was created'); // If we have a target to checkout to, lets do so. //-- created_by is a property of the abstract class Importer, which this class inherits from and it's set by //-- the class that needs to use it (command importer or GUI importer inside the project). if (isset($target) && ($target !== false)) { - if (!is_null($asset->assigned_to)){ + if (!is_null($asset->assigned_to)) { if ($asset->assigned_to != $target->id) { event(new CheckoutableCheckedIn($asset, User::find($asset->assigned_to), auth()->user(), 'Checkin from CSV Importer', $checkin_date)); } } - $asset->fresh()->checkOut($target, $this->created_by, $checkout_date, null, 'Checkout from CSV Importer', $asset->name); + $asset->fresh()->checkOut($target, $this->created_by, $checkout_date, null, 'Checkout from CSV Importer', $asset->name); } return; + } catch (ValidationException $e) { + $this->logError($asset, 'Asset "'.$this->item['name'].'"'); + } catch (\Exception $e) { + report($e); + $this->logError($asset, trans('general.something_went_wrong')); } - $this->logError($asset, 'Asset "'.$this->item['name'].'"'); } From 10126b83628784d8568fa01725e6c543588b1f2d Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 20 Nov 2024 16:42:56 -0600 Subject: [PATCH 24/42] Fix error handling in BulkAssetsController Updated exception handling to capture detailed validation errors in BulkAssetsController. Corrected formatting in notifications view for displaying error messages. --- app/Http/Controllers/Assets/BulkAssetsController.php | 3 ++- resources/views/notifications.blade.php | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 3281d0f49a13..b3446b1b7bbc 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -243,7 +243,7 @@ public function update(ImageUploadRequest $request): RedirectResponse ); // catch exceptions } catch (ValidationException $e) { - $errors[$key] = $e->getMessage(); + $errors = $e->validator->errors()->toArray(); } catch (CustomFieldPermissionException $e) { //$errors[$key] = $e->getMessage(); $custom_field_problem = true; @@ -253,6 +253,7 @@ public function update(ImageUploadRequest $request): RedirectResponse } } if (!empty($errors)) { + //dump($errors); return redirect($bulk_back_url)->with('bulk_asset_errors', $errors); } if ($custom_field_problem) { diff --git a/resources/views/notifications.blade.php b/resources/views/notifications.blade.php index 7d74e5d04345..67871192d413 100755 --- a/resources/views/notifications.blade.php +++ b/resources/views/notifications.blade.php @@ -136,7 +136,7 @@ {{ trans('general.notification_error') }}: {{ trans('general.notification_bulk_error_hint') }} - @foreach($messages as $key => $message) + @foreach($messages as $key => $message) @for ($x = 0; $x < count($message); $x++)
  • {{ $message[$x] }}
    • From a3cdb3b62ff78cb71976b6c795c40d9a39ddd3b9 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Thu, 21 Nov 2024 11:11:11 -0600 Subject: [PATCH 25/42] Fix error message format in BulkAssetsController Previously, the error message was set as a string, which led to an inconsistency in how errors were handled. This change updates the error message to be an array, ensuring it aligns with the expected format. --- app/Http/Controllers/Assets/BulkAssetsController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index b3446b1b7bbc..75deaa8c9264 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -249,7 +249,7 @@ public function update(ImageUploadRequest $request): RedirectResponse $custom_field_problem = true; } catch (\Exception $e) { report($e); - $errors[$key] = trans('general.something_went_wrong'); + $errors[$key] = [trans('general.something_went_wrong')]; } } if (!empty($errors)) { From a4fefc2b5aaf7c8d3e89d86fffbeca0e4d5f2339 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Thu, 21 Nov 2024 11:30:27 -0600 Subject: [PATCH 26/42] get rid of comment --- app/Exceptions/Handler.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 4b98391b3125..0a6f02087944 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -74,14 +74,6 @@ public function render($request, Throwable $e) return response()->json(Helper::formatStandardApiResponse('error', null, 'Invalid JSON'), 422); } - //if ($e instanceof ModelNotFoundException) { - // $class = get_class($e); - // match($class) { - // - // }; - // return redirect()->route()->with('error', trans('general.model_not_found', ['model' => $e])); - //} - // Handle SCIM exceptions if ($e instanceof SCIMException) { try { From 5aa2f307cd7efa4d0a78ef841508ac09e4b16358 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Fri, 22 Nov 2024 20:49:52 -0600 Subject: [PATCH 27/42] Remove debug dump statements This commit removes various debug dump statements from BulkAssetsController and UpdateAssetAction. These changes clean up the code and improve readability by eliminating unnecessary debugging output. Maintaining clean code ensures better maintainability and performance. --- app/Actions/Assets/UpdateAssetAction.php | 8 -------- app/Http/Controllers/Assets/AssetsController.php | 1 - app/Http/Controllers/Assets/BulkAssetsController.php | 1 - 3 files changed, 10 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index e7b09fc7eef5..d53385f0320f 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -57,7 +57,6 @@ public static function run( $asset->warranty_months = $warranty_months ?? $asset->warranty_months; $asset->purchase_cost = $purchase_cost ?? $asset->purchase_cost; if ($request->input('null_purchase_date') === '1') { - dump('filled'); $asset->purchase_date = null; if (!($asset->eol_explicit)) { $asset->asset_eol_date = null; @@ -121,13 +120,10 @@ public static function run( // This is a non-deployable status label - we should check the asset back in. if (($status && $status->getStatuslabelType() != 'deployable') && ($target = $asset->assignedTo)) { - dump('status logic'); - $originalValues = $asset->getRawOriginal(); $asset->assigned_to = null; $asset->assigned_type = null; $asset->accepted = null; - dump($asset->assigned_to); event(new CheckoutableCheckedIn($asset, $target, auth()->user(), 'Checkin on asset update', date('Y-m-d H:i:s'), $originalValues)); // reset this to null so checkout logic doesn't happen below @@ -174,7 +170,6 @@ public static function run( // the gui method //if (($model) && ($model->fieldset)) { - // dump($model->fieldset->fields); // foreach ($model->fieldset->fields as $field) { // // @@ -210,8 +205,6 @@ public static function run( } } if ($field->field_encrypted == '1') { - dump(Gate::allows('assets.view.encrypted_custom_fields')); - dump(auth()->user()->can('assets.view.encrypted_custom_fields')); if (Gate::allows('assets.view.encrypted_custom_fields')) { $field_val = Crypt::encrypt($field_val); } else { @@ -243,7 +236,6 @@ public static function run( } if (isset($target)) { - dump($target); $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); } diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index cf460ab401cb..3f889e65d609 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -264,7 +264,6 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo asset_tag: $asset_tag, // same as serials notes: $request->validated('notes'), ); - dump('returned'.$asset->assigned_to); return redirect()->to(Helper::getRedirectOption($request, $updatedAsset->id, 'Assets')) ->with('success', trans('admin/hardware/message.update.success')); } catch (ValidationException $e) { diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 75deaa8c9264..7bd9b7fd78bc 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -253,7 +253,6 @@ public function update(ImageUploadRequest $request): RedirectResponse } } if (!empty($errors)) { - //dump($errors); return redirect($bulk_back_url)->with('bulk_asset_errors', $errors); } if ($custom_field_problem) { From 93e42ce939d032f4a88d2c92d4a93e3cce282a52 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Mon, 25 Nov 2024 15:50:47 -0600 Subject: [PATCH 28/42] Remove unused code from UpdateAssetAction.php Removed commented-out code related to handling encrypted custom fields in UpdateAssetAction.php. This cleanup helps to maintain a more readable and maintainable codebase. --- app/Actions/Assets/UpdateAssetAction.php | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index d53385f0320f..91fccb24a890 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -169,30 +169,6 @@ public static function run( // Need to investigate and fix. Using static method for now. // the gui method - //if (($model) && ($model->fieldset)) { - // foreach ($model->fieldset->fields as $field) { - // - // - // if ($field->field_encrypted == '1') { - // if (Gate::allows('assets.view.encrypted_custom_fields')) { - // if (is_array($request->input($field->db_column))) { - // $asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column))); - // } else { - // $asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column)); - // } - // throw new CustomFieldPermissionException(); - // continue; - // } - // } else { - // if (is_array($request->input($field->db_column))) { - // $asset->{$field->db_column} = implode(', ', $request->input($field->db_column)); - // } else { - // $asset->{$field->db_column} = $request->input($field->db_column); - // } - // } - // } - //} - // the api method $model = $asset->model; if (($model) && (isset($model->fieldset))) { foreach ($model->fieldset->fields as $field) { From 705a852d45101dba836f6d4612721ea3a2f93ae6 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Mon, 25 Nov 2024 20:09:30 -0600 Subject: [PATCH 29/42] Refactor error handling and remove redundant code Refactored error handling by replacing `\Exception` with `Exception` for consistency. Removed redundant comments and unused code to improve readability and maintainability of the `StoreAssetAction` class. --- app/Actions/Assets/StoreAssetAction.php | 42 +------------------ app/Http/Controllers/Api/AssetsController.php | 17 ++++---- .../Controllers/Assets/AssetsController.php | 11 ++--- 3 files changed, 16 insertions(+), 54 deletions(-) diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index 9305b031a329..b80893d2f385 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -47,7 +47,7 @@ public static function run( $assigned_asset = null, $assigned_location = null, $last_audit_date = null, - ) + ): Asset|bool { $settings = Setting::getSettings(); @@ -96,7 +96,6 @@ public static function run( $model = AssetModel::find($model_id); - // added instanceof, was only in api before if (($model) && ($model instanceof AssetModel) && ($model->fieldset)) { foreach ($model->fieldset->fields as $field) { if ($field->field_encrypted == '1') { @@ -117,42 +116,6 @@ public static function run( } } - // this is the api's custom fieldset logic, is there a real difference??????? - //if (($model) && ($model instanceof AssetModel) && ($model->fieldset)) { - // foreach ($model->fieldset->fields as $field) { - // - // // Set the field value based on what was sent in the request - // $field_val = $request->input($field->db_column, null); - // - // // If input value is null, use custom field's default value - // if ($field_val == null) { - // Log::debug('Field value for '.$field->db_column.' is null'); - // $field_val = $field->defaultValue($request->get('model_id')); - // Log::debug('Use the default fieldset value of '.$field->defaultValue($request->get('model_id'))); - // } - // - // // if the field is set to encrypted, make sure we encrypt the value - // if ($field->field_encrypted == '1') { - // Log::debug('This model field is encrypted in this fieldset.'); - // - // if (Gate::allows('assets.view.encrypted_custom_fields')) { - // - // // If input value is null, use custom field's default value - // if (($field_val == null) && ($request->has('model_id') != '')) { - // $field_val = Crypt::encrypt($field->defaultValue($request->get('model_id'))); - // } else { - // $field_val = Crypt::encrypt($request->input($field->db_column)); - // } - // } - // } - // if ($field->element == 'checkbox') { - // if (is_array($field_val)) { - // $field_val = implode(',', $field_val); - // } - // } - // } - - if ($asset->isValid() && $asset->save()) { if (request('assigned_user')) { $target = User::find(request('assigned_user')); @@ -175,8 +138,7 @@ public static function run( $asset->image = $asset->getImageUrl(); } return $asset; - } else { - dd($asset->getErrors()); //need to figure out how to return errors from watson validating... } + return false; } } \ No newline at end of file diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index dc05b4985a0b..264dc26bf752 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -6,13 +6,14 @@ use App\Actions\Assets\StoreAssetAction; use App\Actions\Assets\UpdateAssetAction; use App\Events\CheckoutableCheckedIn; -use App\Exceptions\CheckoutNotAllowed; use App\Exceptions\CustomFieldPermissionException; +use App\Exceptions\CheckoutNotAllowed; use App\Http\Requests\Assets\StoreAssetRequest; use App\Http\Requests\Assets\UpdateAssetRequest; use App\Http\Traits\MigratesLegacyAssetLocations; use App\Models\CheckoutAcceptance; use App\Models\LicenseSeat; +use Exception; use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\JsonResponse; use Illuminate\Support\Facades\Crypt; @@ -600,12 +601,10 @@ public function store(StoreAssetRequest $request): JsonResponse { try { - $custom_fields = $request->collect()->filter(function ($value, $key) { - return starts_with($key, '_snipeit_'); - }); $asset = StoreAssetAction::run( model_id: $request->validated('model_id'), status_id: $request->validated('status_id'), + request: $request, // this is for handleImages and custom fields name: $request->validated('name'), serial: $request->validated('serial'), company_id: $request->validated('company_id'), @@ -627,16 +626,15 @@ public function store(StoreAssetRequest $request): JsonResponse assigned_user: $request->validated('assigned_user'), assigned_asset: $request->validated('assigned_asset'), assigned_location: $request->validated('assigned_location'), - request: $request, //this is just for the handleImages method... last_audit_date: $request->validated('last_audit_date'), ); return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.create.success'))); - // not sure why we're not using this yet, but i know there's a reason and a reason we want to + // not sure why we're not using this yet, but i know there's a reason return response()->json(Helper::formatStandardApiResponse('success', (new AssetsTransformer)->transformAsset($asset), trans('admin/hardware/message.create.success'))); } catch (CheckoutNotAllowed $e) { return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage()), 200); - } catch (\Exception $e) { + } catch (Exception $e) { return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage())); } } @@ -659,7 +657,8 @@ public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse return response()->json(Helper::formatStandardApiResponse('error', null, $e->getErrors()), 200); } catch (CustomFieldPermissionException $e) { return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.encrypted_warning'))); - } catch (\Exception $e) { + } catch (Exception $e) { + report($e); return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.something_went_wrong'))); } } @@ -680,7 +679,7 @@ public function destroy(Asset $asset): JsonResponse try { DestroyAssetAction::run($asset); return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/hardware/message.delete.success'))); - } catch (\Exception $e) { + } catch (Exception $e) { report($e); return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.something_went_wrong'))); } diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 3f889e65d609..2ff9ce66cc5b 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -15,6 +15,7 @@ use App\Http\Requests\Assets\StoreAssetRequest; use App\Models\Actionlog; use App\Http\Requests\UploadFileRequest; +use Exception; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Support\Facades\Log; use App\Models\Asset; @@ -150,7 +151,7 @@ public function store(StoreAssetRequest $request): RedirectResponse ->with('success-unescaped', trans('admin/hardware/message.create.success_linked', ['link' => route('hardware.show', ['hardware' => $asset->id]), 'id', 'tag' => e($asset->asset_tag)])); } catch (CheckoutNotAllowed $e) { return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.create.error')); - } catch (\Exception $e) { + } catch (Exception $e) { report($e); return redirect()->back()->with('error', 'something bad'); } @@ -268,7 +269,7 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo ->with('success', trans('admin/hardware/message.update.success')); } catch (ValidationException $e) { return redirect()->back()->withInput()->withErrors($e->getErrors()); - } catch (\Exception $e) { + } catch (Exception $e) { report($e); return redirect()->back()->with('error', trans('admin/hardware/message.update.error')); } @@ -287,7 +288,7 @@ public function destroy(Asset $asset, $request): RedirectResponse try { DestroyAssetAction::run($asset); return redirect()->route('hardware.index')->with('success', trans('admin/hardware/message.delete.success')); - } catch (\Exception $e) { + } catch (Exception $e) { report($e); return redirect()->back()->withInput()->withErrors($e->getMessage()); } @@ -404,7 +405,7 @@ public function getBarCode($assetId = null) file_put_contents($barcode_file, $barcode_obj->getPngData()); return response($barcode_obj->getPngData())->header('Content-type', 'image/png'); - } catch (\Exception $e) { + } catch (Exception $e) { Log::debug('The barcode format is invalid.'); return response(file_get_contents(public_path('uploads/barcodes/invalid_barcode.gif')))->header('Content-type', 'image/gif'); @@ -506,7 +507,7 @@ public function postImportHistory(Request $request) $isCheckinHeaderExplicit = in_array('checkin date', (array_map('strtolower', $header))); try { $results = $csv->getRecords(); - } catch (\Exception $e) { + } catch (Exception $e) { return back()->with('error', trans('general.error_in_import_file', ['error' => $e->getMessage()])); } $item = []; From aeb5e90037395f3a03e6fbc736370aec5415f445 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 26 Nov 2024 12:40:53 -0600 Subject: [PATCH 30/42] Add CheckoutNotAllowed exception in UpdateAssetAction This change introduces the CheckoutNotAllowed exception to the UpdateAssetAction class. It ensures that the action can handle scenarios where checking out an asset is not permitted, improving the robustness of the application. --- app/Actions/Assets/UpdateAssetAction.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 91fccb24a890..f63999e0eca5 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -3,6 +3,7 @@ namespace App\Actions\Assets; use App\Events\CheckoutableCheckedIn; +use App\Exceptions\CheckoutNotAllowed; use App\Exceptions\CustomFieldPermissionException; use App\Http\Requests\ImageUploadRequest; use App\Models\Asset; @@ -22,6 +23,7 @@ class UpdateAssetAction /** * @throws ValidationException * @throws CustomFieldPermissionException + * @throws CheckoutNotAllowed */ public static function run( Asset $asset, From 233355ab489318282e210b47d9a4506a872fe2bd Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 26 Nov 2024 12:52:09 -0600 Subject: [PATCH 31/42] Refactor UpdateAssetAction for clarity and remove comments Removed outdated and redundant comments for improved code clarity and readability. Renamed `bulkUpdate` method to `bulkLocationUpdate` for better context and understanding. --- app/Actions/Assets/UpdateAssetAction.php | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index f63999e0eca5..5bfb57e17f93 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -40,7 +40,7 @@ public static function run( $requestable = false, $location_id = null, $rtd_location_id = null, - $assigned_location = null, // derp, make these work + $assigned_location = null, $assigned_asset = null, $assigned_user = null, $byod = false, @@ -165,12 +165,6 @@ public static function run( $asset = $request->handleImages($asset); - // Update custom fields in the database. - // Validation for these fields is handlded through the AssetRequest form request - // FIXME: No idea why this is returning a Builder error on db_column_name. - // Need to investigate and fix. Using static method for now. - - // the gui method $model = $asset->model; if (($model) && (isset($model->fieldset))) { foreach ($model->fieldset->fields as $field) { @@ -197,7 +191,7 @@ public static function run( session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); if ($isBulk) { - self::bulkUpdate($asset, $request); + self::bulkLocationUpdate($asset, $request); } if ($asset->save()) { @@ -225,7 +219,7 @@ public static function run( return $asset; } - private static function bulkUpdate($asset, $request): void + private static function bulkLocationUpdate($asset, $request): void { /** * We're changing the location ID - figure out which location we should apply From 233656df01c7487c61ccc22985d78eb1a3910a93 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 26 Nov 2024 14:28:22 -0600 Subject: [PATCH 32/42] Add tests for storing assets with all fields and multiple entries Introduced StoreAssetTest to verify the handling of full asset records and multiple asset entries in the database. Updated StoreAssetAction and AssetsController to support next audit date validation and improved error handling. --- app/Actions/Assets/StoreAssetAction.php | 4 +- .../Controllers/Assets/AssetsController.php | 7 +- tests/Feature/Assets/Ui/StoreAssetTest.php | 134 ++++++++++++++++++ 3 files changed, 139 insertions(+), 6 deletions(-) create mode 100644 tests/Feature/Assets/Ui/StoreAssetTest.php diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index b80893d2f385..4fbf73cbaec0 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -47,6 +47,7 @@ public static function run( $assigned_asset = null, $assigned_location = null, $last_audit_date = null, + $next_audit_date = null, ): Asset|bool { $settings = Setting::getSettings(); @@ -73,10 +74,11 @@ public static function run( $asset->rtd_location_id = $rtd_location_id; $asset->byod = $byod; $asset->last_audit_date = $last_audit_date; + $asset->next_audit_date = $next_audit_date; $asset->location_id = $location_id; // set up next audit date - if (!empty($settings->audit_interval)) { + if (!empty($settings->audit_interval) && is_null($next_audit_date)) { $asset->next_audit_date = Carbon::now()->addMonths($settings->audit_interval)->toDateString(); } diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 2ff9ce66cc5b..47ae6892aa2e 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -111,10 +111,6 @@ public function store(StoreAssetRequest $request): RedirectResponse try { $asset_tags = $request->input('asset_tags'); $serials = $request->input('serials'); - $custom_fields = $request->collect()->filter(function ($value, $key) { - return starts_with($key, '_snipeit_'); - }); - //DB::transaction(function () use ($request, $asset_tags, $serials, $custom_fields) { foreach ($asset_tags as $key => $asset_tag) { $asset = StoreAssetAction::run( @@ -143,6 +139,7 @@ public function store(StoreAssetRequest $request): RedirectResponse assigned_asset: $request->validated('assigned_asset'), assigned_location: $request->validated('assigned_location'), last_audit_date: $request->validated('last_audit_date'), + next_audit_date: $request->validated('next_audit_date'), ); } //}); @@ -153,7 +150,7 @@ public function store(StoreAssetRequest $request): RedirectResponse return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.create.error')); } catch (Exception $e) { report($e); - return redirect()->back()->with('error', 'something bad'); + return redirect()->back()->with('error', trans('general.something_went_wrong')); } } diff --git a/tests/Feature/Assets/Ui/StoreAssetTest.php b/tests/Feature/Assets/Ui/StoreAssetTest.php new file mode 100644 index 000000000000..51dfd3806adc --- /dev/null +++ b/tests/Feature/Assets/Ui/StoreAssetTest.php @@ -0,0 +1,134 @@ +createAssets()->create(); + $model = AssetModel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); + $defaultLocation = Location::factory()->create(); + $supplier = Supplier::factory()->create(); + $file = UploadedFile::fake()->image("test.jpg", 2000); + + + $response = $this->actingAs($user) + ->post(route('hardware.store'), [ + 'redirect_option' => 'item', + 'name' => 'Test Asset', + 'model_id' => $model->id, + 'status_id' => $status->id, + // ugh, this is because for some reason asset tags and serials are expected to start at an index of [1], so throwing an empty in for [0] + 'asset_tags' => ['', 'TEST-ASSET'], + 'serials' => ['', 'TEST-SERIAL'], + 'notes' => 'Test Notes', + 'rtd_location_id' => $defaultLocation->id, + 'requestable' => true, + 'image' => $file, + 'warranty_months' => 12, + 'next_audit_date' => Carbon::now()->addMonths(12)->format('Y-m-d'), + 'byod' => true, + 'order_number' => 'TEST-ORDER', + 'purchase_date' => Carbon::now()->format('Y-m-d'), + 'asset_eol_date' => Carbon::now()->addMonths(36)->format('Y-m-d'), + 'supplier_id' => $supplier->id, + 'purchase_cost' => 1234.56, + ])->assertSessionHasNoErrors(); + + $storedAsset = Asset::where('asset_tag', 'TEST-ASSET')->sole(); + + $response->assertRedirect(route('hardware.show', ['hardware' => $storedAsset->id])); + + $this->assertDatabaseHas('assets', [ + 'id' => $storedAsset->id, + 'name' => 'Test Asset', + 'model_id' => $model->id, + 'status_id' => $status->id, + 'asset_tag' => 'TEST-ASSET', + 'serial' => 'TEST-SERIAL', + 'notes' => 'Test Notes', + 'rtd_location_id' => $defaultLocation->id, + 'requestable' => 1, + 'image' => $storedAsset->image, + 'warranty_months' => 12, + 'next_audit_date' => Carbon::now()->addMonths(12)->format('Y-m-d'), + 'byod' => 1, + 'order_number' => 'TEST-ORDER', + 'purchase_date' => Carbon::now()->format('Y-m-d'), + 'asset_eol_date' => Carbon::now()->addMonths(36)->format('Y-m-d'), + 'supplier_id' => $supplier->id, + 'purchase_cost' => 1234.56, + ]); + } + + public function test_multiple_assets_are_stored() + { + $user = User::factory()->createAssets()->create(); + $model = AssetModel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); + $defaultLocation = Location::factory()->create(); + $supplier = Supplier::factory()->create(); + $file = UploadedFile::fake()->image("test.jpg", 2000); + + $this->actingAs($user)->post(route('hardware.store'), [ + 'redirect_option' => 'index', + 'name' => 'Test Assets', + 'model_id' => $model->id, + 'status_id' => $status->id, + 'asset_tags' => ['', 'TEST-ASSET-1', 'TEST-ASSET-2'], + 'serials' => ['', 'TEST-SERIAL-1', 'TEST-SERIAL-2'], + 'notes' => 'Test Notes', + 'rtd_location_id' => $defaultLocation->id, + 'requestable' => true, + 'image' => $file, + 'warranty_months' => 12, + 'next_audit_date' => Carbon::now()->addMonths(12)->format('Y-m-d'), + 'byod' => true, + 'order_number' => 'TEST-ORDER', + 'purchase_date' => Carbon::now()->format('Y-m-d'), + 'asset_eol_date' => Carbon::now()->addMonths(36)->format('Y-m-d'), + 'supplier_id' => $supplier->id, + 'purchase_cost' => 1234.56, + ])->assertRedirect(route('hardware.index'))->assertSessionHasNoErrors(); + + $storedAsset = Asset::where('asset_tag', 'TEST-ASSET-1')->sole(); + $storedAsset2 = Asset::where('asset_tag', 'TEST-ASSET-2')->sole(); + + $commonData = [ + 'name' => 'Test Assets', + 'model_id' => $model->id, + 'status_id' => $status->id, + 'notes' => 'Test Notes', + 'rtd_location_id' => $defaultLocation->id, + 'requestable' => 1, + 'warranty_months' => 12, + 'next_audit_date' => Carbon::now()->addMonths(12)->format('Y-m-d'), + 'byod' => 1, + 'order_number' => 'TEST-ORDER', + 'purchase_date' => Carbon::now()->format('Y-m-d'), + 'asset_eol_date' => Carbon::now()->addMonths(36)->format('Y-m-d'), + 'supplier_id' => $supplier->id, + 'purchase_cost' => 1234.56, + ]; + + $this->assertDatabaseHas('assets', array_merge($commonData, ['asset_tag' => 'TEST-ASSET-1', 'serial' => 'TEST-SERIAL-1', 'image' => $storedAsset->image])); + $this->assertDatabaseHas('assets', array_merge($commonData, ['asset_tag' => 'TEST-ASSET-2', 'serial' => 'TEST-SERIAL-2', 'image' => $storedAsset2->image])); + } + +} \ No newline at end of file From 09d4e0cb053f7f8573099c30df92d0cf504b9b09 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 26 Nov 2024 14:37:45 -0600 Subject: [PATCH 33/42] Remove unnecessary parameter and add delete asset tests Removed the redundant $request parameter from the destroy method in AssetsController. Added feature tests to verify asset deletion functionality with proper permissions and ensure deletion is forbidden without permissions. --- .../Controllers/Assets/AssetsController.php | 2 +- tests/Feature/Assets/Ui/DeleteAssetTest.php | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 tests/Feature/Assets/Ui/DeleteAssetTest.php diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 47ae6892aa2e..a20a4a80d277 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -279,7 +279,7 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo * @param int $assetId * @since [v1.0] */ - public function destroy(Asset $asset, $request): RedirectResponse + public function destroy(Asset $asset): RedirectResponse { $this->authorize('delete', $asset); try { diff --git a/tests/Feature/Assets/Ui/DeleteAssetTest.php b/tests/Feature/Assets/Ui/DeleteAssetTest.php new file mode 100644 index 000000000000..915247109eef --- /dev/null +++ b/tests/Feature/Assets/Ui/DeleteAssetTest.php @@ -0,0 +1,31 @@ +deleteAssets()->create(); + + $asset = Asset::factory()->create(); + $this->actingAs($user) + ->delete(route('hardware.destroy', $asset)) + ->assertRedirect(route('hardware.index')); + } + + public function test_asset_cannot_be_deleted_without_permissions() + { + $user = User::factory()->create(); + + $asset = Asset::factory()->create(); + $this->actingAs($user) + ->delete(route('hardware.destroy', $asset)) + ->assertForbidden(); + } + +} \ No newline at end of file From e056da6b2ae7443ad9a1483cebb4d8e0858b6bdf Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 26 Nov 2024 14:47:16 -0600 Subject: [PATCH 34/42] Add permissions checks to asset tests Implement tests to ensure users without appropriate permissions are denied access to asset endpoints. Update tests to verify proper soft deletions and asset existence checks in delete scenarios. --- tests/Feature/Assets/Ui/DeleteAssetTest.php | 4 ++++ tests/Feature/Assets/Ui/EditAssetTest.php | 13 +++++++++++++ tests/Feature/Assets/Ui/StoreAssetTest.php | 15 +++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/tests/Feature/Assets/Ui/DeleteAssetTest.php b/tests/Feature/Assets/Ui/DeleteAssetTest.php index 915247109eef..948f0d683313 100644 --- a/tests/Feature/Assets/Ui/DeleteAssetTest.php +++ b/tests/Feature/Assets/Ui/DeleteAssetTest.php @@ -16,6 +16,8 @@ public function test_asset_can_be_deleted_with_permissions() $this->actingAs($user) ->delete(route('hardware.destroy', $asset)) ->assertRedirect(route('hardware.index')); + + $this->assertSoftDeleted($asset); } public function test_asset_cannot_be_deleted_without_permissions() @@ -26,6 +28,8 @@ public function test_asset_cannot_be_deleted_without_permissions() $this->actingAs($user) ->delete(route('hardware.destroy', $asset)) ->assertForbidden(); + + $this->assertModelExists($asset); } } \ No newline at end of file diff --git a/tests/Feature/Assets/Ui/EditAssetTest.php b/tests/Feature/Assets/Ui/EditAssetTest.php index ebe7bf9a9710..d085036f2bd0 100644 --- a/tests/Feature/Assets/Ui/EditAssetTest.php +++ b/tests/Feature/Assets/Ui/EditAssetTest.php @@ -68,6 +68,19 @@ public function testAssetEditPostIsRedirectedIfRedirectSelectionIsItem() $this->assertDatabaseHas('assets', ['asset_tag' => 'New Asset Tag']); } + public function test_user_without_permission_is_denied() + { + $user = User::factory()->create(); + $asset = Asset::factory()->create(); + + $this->actingAs($user)->put(route('hardware.update', $asset), [ + 'name' => 'New name', + 'asset_tags' => 'New Asset Tag', + 'status_id' => StatusLabel::factory()->create()->id, + 'model_id' => AssetModel::factory()->create()->id, + ])->assertForbidden(); + } + public function testNewCheckinIsLoggedIfStatusChangedToUndeployable() { Event::fake([CheckoutableCheckedIn::class]); diff --git a/tests/Feature/Assets/Ui/StoreAssetTest.php b/tests/Feature/Assets/Ui/StoreAssetTest.php index 51dfd3806adc..560dec41251a 100644 --- a/tests/Feature/Assets/Ui/StoreAssetTest.php +++ b/tests/Feature/Assets/Ui/StoreAssetTest.php @@ -131,4 +131,19 @@ public function test_multiple_assets_are_stored() $this->assertDatabaseHas('assets', array_merge($commonData, ['asset_tag' => 'TEST-ASSET-2', 'serial' => 'TEST-SERIAL-2', 'image' => $storedAsset2->image])); } + public function test_user_without_permission_denied() + { + $user = User::factory()->create(); + $model = AssetModel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); + + $this->actingAs($user)->post(route('hardware.store'), [ + 'redirect_option' => 'index', + 'name' => 'Test Assets', + 'model_id' => $model->id, + 'status_id' => $status->id, + 'asset_tags' => ['', 'TEST-ASSET-1'], + 'serials' => ['', 'TEST-SERIAL-1'], + ])->assertForbidden(); + } } \ No newline at end of file From ac1d09add0bb0f3842f70e1b7df6a6b6b18028c1 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 26 Nov 2024 14:49:50 -0600 Subject: [PATCH 35/42] Fix authorization check in AssetsController Removed incorrect and commented-out authorization check in the destroy method. Ensured proper authorization by explicitly authorizing the asset instance before attempting deletion. --- app/Http/Controllers/Api/AssetsController.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 264dc26bf752..b73ddff3df6c 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -673,8 +673,6 @@ public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse */ public function destroy(Asset $asset): JsonResponse { - //this is probably wrong - //$this->authorize('delete', Asset::class); $this->authorize('delete', $asset); try { DestroyAssetAction::run($asset); From 0a1aba54065fcc700acc059d4ee7117bd5893ddc Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Fri, 29 Nov 2024 14:32:19 -0600 Subject: [PATCH 36/42] a little work for conflicts --- app/Actions/Assets/StoreAssetAction.php | 1 - app/Http/Controllers/Api/AssetsController.php | 1 - app/Http/Controllers/Assets/AssetsController.php | 6 ++++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index 4fbf73cbaec0..04f9b3fa01d2 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -134,7 +134,6 @@ public static function run( if (isset($target)) { $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), $request->input('expected_checkin', null), 'Checked out on asset creation', $request->get('name'), $location); } - //this was in api and not gui if ($asset->image) { $asset->image = $asset->getImageUrl(); diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index b73ddff3df6c..69f993f2d989 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -599,7 +599,6 @@ public function selectlist(Request $request) : array */ public function store(StoreAssetRequest $request): JsonResponse { - try { $asset = StoreAssetAction::run( model_id: $request->validated('model_id'), diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index a20a4a80d277..e0f4a5116a7d 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -108,6 +108,8 @@ public function create(Request $request) : View */ public function store(StoreAssetRequest $request): RedirectResponse { + $successes = []; + $errors = []; try { $asset_tags = $request->input('asset_tags'); $serials = $request->input('serials'); @@ -141,6 +143,10 @@ public function store(StoreAssetRequest $request): RedirectResponse last_audit_date: $request->validated('last_audit_date'), next_audit_date: $request->validated('next_audit_date'), ); + $successes[] = " $asset->id])."' style='color: white;'>".e($asset->asset_tag).""; + if (!$asset) { + $failures[] = join(",", $asset->getErrors()->all()); + } } //}); session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); From 16740b41e426af45413da863a11eca9a83618fce Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 3 Dec 2024 13:50:30 -0600 Subject: [PATCH 37/42] Simplify asset handling logic for creation and update Refactor asset creation and update processes by removing redundant save checks and organizing error handling logic. Enhance success and failure feedback in the asset creation flow to improve clarity and user experience. Add support for detailed multi-success and partial failure messages to alert users of outcomes. --- app/Actions/Assets/StoreAssetAction.php | 40 +++++++-------- app/Actions/Assets/UpdateAssetAction.php | 33 ++++++------ .../Controllers/Assets/AssetsController.php | 50 ++++++++++++------- .../lang/en-US/admin/hardware/message.php | 4 +- 4 files changed, 71 insertions(+), 56 deletions(-) diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index 04f9b3fa01d2..157d537f8e25 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -118,28 +118,26 @@ public static function run( } } - if ($asset->isValid() && $asset->save()) { - if (request('assigned_user')) { - $target = User::find(request('assigned_user')); - // the api doesn't have these location-y bits - good reason? - $location = $target->location_id; - } elseif (request('assigned_asset')) { - $target = Asset::find(request('assigned_asset')); - $location = $target->location_id; - } elseif (request('assigned_location')) { - $target = Location::find(request('assigned_location')); - $location = $target->id; - } + $asset->save(); + if (request('assigned_user')) { + $target = User::find(request('assigned_user')); + // the api doesn't have these location-y bits - good reason? + $location = $target->location_id; + } elseif (request('assigned_asset')) { + $target = Asset::find(request('assigned_asset')); + $location = $target->location_id; + } elseif (request('assigned_location')) { + $target = Location::find(request('assigned_location')); + $location = $target->id; + } - if (isset($target)) { - $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), $request->input('expected_checkin', null), 'Checked out on asset creation', $request->get('name'), $location); - } - //this was in api and not gui - if ($asset->image) { - $asset->image = $asset->getImageUrl(); - } - return $asset; + if (isset($target)) { + $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), $request->input('expected_checkin', null), 'Checked out on asset creation', $request->get('name'), $location); + } + //this was in api and not gui + if ($asset->image) { + $asset->image = $asset->getImageUrl(); } - return false; + return $asset; } } \ No newline at end of file diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index 5bfb57e17f93..b82276469fdc 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -194,26 +194,25 @@ public static function run( self::bulkLocationUpdate($asset, $request); } - if ($asset->save()) { + $asset->save(); // check out stuff - $location = Location::find($asset->location_id); - if (!is_null($assigned_user) && ($target = User::find($assigned_user))) { - $location = $target->location_id; - } elseif (!is_null($assigned_asset) && ($target = Asset::find($assigned_asset))) { - $location = $target->location_id; - Asset::where('assigned_type', \App\Models\Asset::class)->where('assigned_to', $asset->id) - ->update(['location_id' => $target->location_id]); - } elseif (!is_null($assigned_location) && ($target = Location::find($assigned_location))) { - $location = $target->id; - } + $location = Location::find($asset->location_id); + if (!is_null($assigned_user) && ($target = User::find($assigned_user))) { + $location = $target->location_id; + } elseif (!is_null($assigned_asset) && ($target = Asset::find($assigned_asset))) { + $location = $target->location_id; + Asset::where('assigned_type', \App\Models\Asset::class)->where('assigned_to', $asset->id) + ->update(['location_id' => $target->location_id]); + } elseif (!is_null($assigned_location) && ($target = Location::find($assigned_location))) { + $location = $target->id; + } - if (isset($target)) { - $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); - } + if (isset($target)) { + $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); + } - if ($asset->image) { - $asset->image = $asset->getImageUrl(); - } + if ($asset->image) { + $asset->image = $asset->getImageUrl(); } return $asset; diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index e0f4a5116a7d..6cb725656d4f 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -109,12 +109,12 @@ public function create(Request $request) : View public function store(StoreAssetRequest $request): RedirectResponse { $successes = []; + $failures = []; $errors = []; - try { - $asset_tags = $request->input('asset_tags'); - $serials = $request->input('serials'); - //DB::transaction(function () use ($request, $asset_tags, $serials, $custom_fields) { - foreach ($asset_tags as $key => $asset_tag) { + $asset_tags = $request->input('asset_tags'); + $serials = $request->input('serials'); + foreach ($asset_tags as $key => $asset_tag) { + try { $asset = StoreAssetAction::run( model_id: $request->validated('model_id'), status_id: $request->validated('status_id'), @@ -144,23 +144,39 @@ public function store(StoreAssetRequest $request): RedirectResponse next_audit_date: $request->validated('next_audit_date'), ); $successes[] = " $asset->id])."' style='color: white;'>".e($asset->asset_tag).""; - if (!$asset) { - $failures[] = join(",", $asset->getErrors()->all()); + } catch (ValidationException|CheckoutNotAllowed $e) { + $errors[] = $e->getMessage(); + } catch (Exception $e) { + report($e); + } + } + $failures[] = join(",", $errors); + session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); + + if ($successes) { + if ($failures) { + //some succeeded, some failed + return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) //FIXME - not tested + ->with('success-unescaped', trans_choice('admin/hardware/message.create.multi_success_linked', $successes, ['links' => join(", ", $successes)])) + ->with('warning', trans_choice('admin/hardware/message.create.partial_failure', $failures, ['failures' => join("; ", $failures)])); + } else { + if (count($successes) == 1) { + //the most common case, keeping it so we don't have to make every use of that translation string be trans_choice'ed + //and re-translated + return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) + ->with('success-unescaped', trans('admin/hardware/message.create.success_linked', ['link' => route('hardware.show', ['hardware' => $asset->id]), 'id', 'tag' => e($asset->asset_tag)])); + } else { + //multi-success + return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) + ->with('success-unescaped', trans_choice('admin/hardware/message.create.multi_success_linked', $successes, ['links' => join(", ", $successes)])); } } - //}); - session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); - return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) - ->with('success-unescaped', trans('admin/hardware/message.create.success_linked', ['link' => route('hardware.show', ['hardware' => $asset->id]), 'id', 'tag' => e($asset->asset_tag)])); - } catch (CheckoutNotAllowed $e) { - return redirect()->route('hardware.index')->with('error', trans('admin/hardware/message.create.error')); - } catch (Exception $e) { - report($e); - return redirect()->back()->with('error', trans('general.something_went_wrong')); } + // this shouldn't happen, but php complains if there's no final return + return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) + ->with('success-unescaped', trans('admin/hardware/message.create.success_linked', ['link' => route('hardware.show', ['hardware' => $asset->id]), 'id', 'tag' => e($asset->asset_tag)])); } - /** * Returns a view that presents a form to edit an existing asset. * diff --git a/resources/lang/en-US/admin/hardware/message.php b/resources/lang/en-US/admin/hardware/message.php index 27877e02f9fc..10dfbff6fd15 100644 --- a/resources/lang/en-US/admin/hardware/message.php +++ b/resources/lang/en-US/admin/hardware/message.php @@ -13,7 +13,9 @@ 'create' => [ 'error' => 'Asset was not created, please try again. :(', 'success' => 'Asset created successfully. :)', - 'success_linked' => 'Asset with tag :tag was created successfully. Click here to view.', + 'success_linked' => 'Asset with tag :tag was created successfully. Click here to view.', + 'multi_success_linked' => 'Asset with tag :links was created successfully.|:count assets were created succesfully. :links.', + 'partial_failure' => 'An asset was unable to be created. Reason: :failures|:count assets were unable to be created. Reasons: :failures', ], 'update' => [ From 5e475c3558a408fa0ce24b8fd87384906f1ed9d8 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 3 Dec 2024 14:41:30 -0600 Subject: [PATCH 38/42] Remove unused imports from AssetsController files. Simplify the code by cleaning up unused import statements in both `AssetsController.php` and `Api/AssetsController.php`. This reduces potential confusion and helps maintain cleaner code for future development and maintenance. --- app/Http/Controllers/Api/AssetsController.php | 5 ----- app/Http/Controllers/Assets/AssetsController.php | 10 ---------- 2 files changed, 15 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index c1ee2ffb9756..c665e069bddc 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -16,8 +16,6 @@ use Exception; use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\JsonResponse; -use Illuminate\Support\Facades\Crypt; -use Illuminate\Support\Facades\Gate; use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Http\Requests\AssetCheckoutRequest; @@ -26,7 +24,6 @@ use App\Http\Transformers\SelectlistTransformer; use App\Models\Asset; use App\Models\AssetModel; -use App\Models\Company; use App\Models\CustomField; use App\Models\License; use App\Models\Location; @@ -35,8 +32,6 @@ use Carbon\Carbon; use Illuminate\Support\Facades\DB; use Illuminate\Http\Request; -use App\Http\Requests\ImageUploadRequest; -use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Route; use Watson\Validating\ValidationException; use App\View\Label; diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 312e1bbc2bcb..a0504f9bee01 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -5,34 +5,24 @@ use App\Actions\Assets\DestroyAssetAction; use App\Actions\Assets\StoreAssetAction; use App\Actions\Assets\UpdateAssetAction; -use App\Events\CheckoutableCheckedIn; use App\Exceptions\CheckoutNotAllowed; use App\Helpers\Helper; use App\Http\Controllers\Controller; -use App\Http\Requests\Assets\DestroyAssetRequest; use App\Http\Requests\Assets\UpdateAssetRequest; -use App\Http\Requests\ImageUploadRequest; use App\Http\Requests\Assets\StoreAssetRequest; use App\Models\Actionlog; use App\Http\Requests\UploadFileRequest; use Exception; -use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Support\Facades\Log; use App\Models\Asset; use App\Models\AssetModel; use App\Models\CheckoutRequest; use App\Models\Company; -use App\Models\Location; use App\Models\Setting; -use App\Models\Statuslabel; use App\Models\User; use App\View\Label; use Carbon\Carbon; -use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Gate; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Crypt; -use Illuminate\Support\Facades\Storage; use Illuminate\Support\Facades\Validator; use League\Csv\Reader; use Illuminate\Http\Response; From 19771d26fbb257427689645ff5a298ea9122cb8b Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 3 Dec 2024 14:46:32 -0600 Subject: [PATCH 39/42] formatting --- app/Http/Controllers/Assets/AssetsController.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index a0504f9bee01..bd23a2aa8e13 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -161,11 +161,10 @@ public function store(StoreAssetRequest $request): RedirectResponse ->with('success-unescaped', trans_choice('admin/hardware/message.create.multi_success_linked', $successes, ['links' => join(", ", $successes)])); } } - } + } // this shouldn't happen, but php complains if there's no final return return redirect()->to(Helper::getRedirectOption($request, $asset->id, 'Assets')) ->with('success-unescaped', trans('admin/hardware/message.create.success_linked', ['link' => route('hardware.show', ['hardware' => $asset->id]), 'id', 'tag' => e($asset->asset_tag)])); - } /** From 38d8d3076df4d044a9e6fdd8d612d4f18ee6b6cf Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 4 Dec 2024 21:43:47 -0600 Subject: [PATCH 40/42] Refactor asset handling and streamline code This refactoring introduces helper methods for handling image uploads, custom fields, and checkouts, improving code organization and readability. Unnecessary imports, comments, and unused variables were removed for cleaner code. Error handling was also improved with additional exception reporting in the asset update flow. --- app/Actions/Assets/DestroyAssetAction.php | 4 +- app/Actions/Assets/StoreAssetAction.php | 77 ++++++++++++------- app/Actions/Assets/UpdateAssetAction.php | 62 ++++++++------- app/Exceptions/Handler.php | 1 - app/Http/Controllers/Api/AssetsController.php | 3 +- .../Controllers/Assets/AssetsController.php | 3 +- .../Assets/BulkAssetsController.php | 3 - .../Requests/Assets/UpdateAssetRequest.php | 1 - tests/Feature/Assets/Api/UpdateAssetTest.php | 2 +- tests/Feature/Assets/Ui/StoreAssetTest.php | 1 - 10 files changed, 90 insertions(+), 67 deletions(-) diff --git a/app/Actions/Assets/DestroyAssetAction.php b/app/Actions/Assets/DestroyAssetAction.php index 095f703e62c3..6ea96abede8d 100644 --- a/app/Actions/Assets/DestroyAssetAction.php +++ b/app/Actions/Assets/DestroyAssetAction.php @@ -7,6 +7,8 @@ use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; +use Exception; + class DestroyAssetAction { @@ -27,7 +29,7 @@ public static function run(Asset $asset) if ($asset->image) { try { Storage::disk('public')->delete('assets'.'/'.$asset->image); - } catch (\Exception $e) { + } catch (Exception $e) { Log::debug($e); } } diff --git a/app/Actions/Assets/StoreAssetAction.php b/app/Actions/Assets/StoreAssetAction.php index 157d537f8e25..b9a40546a03a 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -9,12 +9,11 @@ use App\Models\Company; use App\Models\Location; use App\Models\Setting; +use App\Models\SnipeModel; use App\Models\User; use Carbon\Carbon; use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Gate; -use Illuminate\Support\Facades\Log; -use Illuminate\Support\MessageBag; class StoreAssetAction { @@ -31,7 +30,6 @@ public static function run( $asset_tag = null, $order_number = null, $notes = null, - $user_id = null, $warranty_months = null, $purchase_cost = null, $asset_eol_date = null, @@ -40,8 +38,7 @@ public static function run( $supplier_id = null, $requestable = null, $rtd_location_id = null, - $location_id = null, //do something with this - $files = null, + $location_id = null, $byod = 0, $assigned_user = null, $assigned_asset = null, @@ -87,17 +84,43 @@ public static function run( $asset->location_id = $rtd_location_id; } - //api only - if ($request->has('image_source')) { - $request->offsetSet('image', $request->offsetGet('image_source')); - } + $asset = self::handleImages($request, $asset); - if ($request->has('image')) { - $asset = $request->handleImages($asset); + $model = AssetModel::find($model_id); + + self::handleCustomFields($model, $request, $asset); + + $asset->save(); + if (request('assigned_user')) { + $target = User::find(request('assigned_user')); + // the api doesn't have these location-y bits - good reason? + $location = $target->location_id; + } elseif (request('assigned_asset')) { + $target = Asset::find(request('assigned_asset')); + $location = $target->location_id; + } elseif (request('assigned_location')) { + $target = Location::find(request('assigned_location')); + $location = $target->id; } - $model = AssetModel::find($model_id); + if (isset($target)) { + self::handleCheckout($target, $asset, $request, $location); + } + //this was in api and not gui + if ($asset->image) { + $asset->image = $asset->getImageUrl(); + } + return $asset; + } + /** + * @param $model + * @param ImageUploadRequest $request + * @param Asset|\App\Models\SnipeModel $asset + * @return void + */ + private static function handleCustomFields($model, ImageUploadRequest $request, $asset): void + { if (($model) && ($model instanceof AssetModel) && ($model->fieldset)) { foreach ($model->fieldset->fields as $field) { if ($field->field_encrypted == '1') { @@ -117,27 +140,23 @@ public static function run( } } } + } - $asset->save(); - if (request('assigned_user')) { - $target = User::find(request('assigned_user')); - // the api doesn't have these location-y bits - good reason? - $location = $target->location_id; - } elseif (request('assigned_asset')) { - $target = Asset::find(request('assigned_asset')); - $location = $target->location_id; - } elseif (request('assigned_location')) { - $target = Location::find(request('assigned_location')); - $location = $target->id; + private static function handleImages($request, $asset) + { + //api + if ($request->has('image_source')) { + $request->offsetSet('image', $request->offsetGet('image_source')); } - if (isset($target)) { - $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), $request->input('expected_checkin', null), 'Checked out on asset creation', $request->get('name'), $location); - } - //this was in api and not gui - if ($asset->image) { - $asset->image = $asset->getImageUrl(); + if ($request->has('image')) { + $asset = $request->handleImages($asset); } return $asset; } + + private static function handleCheckout($target, $asset, $request, $location): void + { + $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), $request->input('expected_checkin', null), 'Checked out on asset creation', $request->get('name'), $location); + } } \ No newline at end of file diff --git a/app/Actions/Assets/UpdateAssetAction.php b/app/Actions/Assets/UpdateAssetAction.php index b82276469fdc..7cde179c5430 100644 --- a/app/Actions/Assets/UpdateAssetAction.php +++ b/app/Actions/Assets/UpdateAssetAction.php @@ -10,6 +10,7 @@ use App\Models\AssetModel; use App\Models\Company; use App\Models\Location; +use App\Models\SnipeModel; use App\Models\Statuslabel; use App\Models\User; use Carbon\Carbon; @@ -53,7 +54,8 @@ public static function run( $asset_tag = null, $notes = null, $isBulk = false, - ): \App\Models\SnipeModel { + ): SnipeModel + { $asset->status_id = $status_id ?? $asset->status_id; $asset->warranty_months = $warranty_months ?? $asset->warranty_months; @@ -165,30 +167,7 @@ public static function run( $asset = $request->handleImages($asset); - $model = $asset->model; - if (($model) && (isset($model->fieldset))) { - foreach ($model->fieldset->fields as $field) { - $field_val = $request->input($field->db_column, null); - - if ($request->has($field->db_column)) { - if ($field->element == 'checkbox') { - if (is_array($field_val)) { - $field_val = implode(',', $field_val); - } - } - if ($field->field_encrypted == '1') { - if (Gate::allows('assets.view.encrypted_custom_fields')) { - $field_val = Crypt::encrypt($field_val); - } else { - throw new CustomFieldPermissionException(); - } - } - $asset->{$field->db_column} = $field_val; - } - } - } - - session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); + self::handleCustomFields($request, $asset); if ($isBulk) { self::bulkLocationUpdate($asset, $request); @@ -208,7 +187,7 @@ public static function run( } if (isset($target)) { - $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); + self::handleCheckout($asset, $target, $request, $location); } if ($asset->image) { @@ -246,4 +225,35 @@ private static function bulkLocationUpdate($asset, $request): void } } + + private static function handleCustomFields($request, $asset): void + { + $model = $asset->model; + if (($model) && (isset($model->fieldset))) { + foreach ($model->fieldset->fields as $field) { + $field_val = $request->input($field->db_column, null); + + if ($request->has($field->db_column)) { + if ($field->element == 'checkbox') { + if (is_array($field_val)) { + $field_val = implode(',', $field_val); + } + } + if ($field->field_encrypted == '1') { + if (Gate::allows('assets.view.encrypted_custom_fields')) { + $field_val = Crypt::encrypt($field_val); + } else { + throw new CustomFieldPermissionException(); + } + } + $asset->{$field->db_column} = $field_val; + } + } + } + } + + private static function handleCheckout($asset, $target, $request, $location): void + { + $asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), '', 'Checked out on asset update', e($request->get('name')), $location); + } } \ No newline at end of file diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 0a6f02087944..2b8eaa362744 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -2,7 +2,6 @@ namespace App\Exceptions; -use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler; use App\Helpers\Helper; use Illuminate\Validation\ValidationException; diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index c665e069bddc..5b52da246f33 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -614,7 +614,6 @@ public function store(StoreAssetRequest $request): JsonResponse asset_tag: $request->validated('asset_tag'), order_number: $request->validated('order_number'), notes: $request->validated('notes'), - user_id: $request->validated('user_id'), warranty_months: $request->validated('warranty_months'), purchase_cost: $request->validated('purchase_cost'), asset_eol_date: $request->validated('asset_eol_date'), @@ -624,7 +623,6 @@ public function store(StoreAssetRequest $request): JsonResponse requestable: $request->validated('requestable'), rtd_location_id: $request->validated('rtd_location_id'), location_id: $request->validated('location_id'), - files: $request->validated('files'), byod: $request->validated('byod'), assigned_user: $request->validated('assigned_user'), assigned_asset: $request->validated('assigned_asset'), @@ -638,6 +636,7 @@ public function store(StoreAssetRequest $request): JsonResponse } catch (CheckoutNotAllowed $e) { return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage()), 200); } catch (Exception $e) { + report($e); return response()->json(Helper::formatStandardApiResponse('error', null, $e->getMessage())); } } diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index bd23a2aa8e13..26d62787b881 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -115,7 +115,6 @@ public function store(StoreAssetRequest $request): RedirectResponse asset_tag: $asset_tag, order_number: $request->validated('order_number'), notes: $request->validated('notes'), - user_id: $request->validated('user_id'), warranty_months: $request->validated('warranty_months'), purchase_cost: $request->validated('purchase_cost'), asset_eol_date: $request->validated('asset_eol_date'), @@ -125,7 +124,6 @@ public function store(StoreAssetRequest $request): RedirectResponse requestable: $request->validated('requestable'), rtd_location_id: $request->validated('rtd_location_id'), location_id: $request->validated('location_id'), - files: $request->validated('files'), byod: $request->validated('byod'), assigned_user: $request->validated('assigned_user'), assigned_asset: $request->validated('assigned_asset'), @@ -274,6 +272,7 @@ public function update(UpdateAssetRequest $request, Asset $asset): RedirectRespo asset_tag: $asset_tag, // same as serials notes: $request->validated('notes'), ); + session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); return redirect()->to(Helper::getRedirectOption($request, $updatedAsset->id, 'Assets')) ->with('success', trans('admin/hardware/message.update.success')); } catch (ValidationException $e) { diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 7bd9b7fd78bc..c4170a9a3455 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -2,7 +2,6 @@ namespace App\Http\Controllers\Assets; -use App\Actions\Assets\StoreAssetAction; use App\Actions\Assets\UpdateAssetAction; use App\Exceptions\CustomFieldPermissionException; use App\Helpers\Helper; @@ -241,11 +240,9 @@ public function update(ImageUploadRequest $request): RedirectResponse order_number: $request->input('order_number'), isBulk: true, ); - // catch exceptions } catch (ValidationException $e) { $errors = $e->validator->errors()->toArray(); } catch (CustomFieldPermissionException $e) { - //$errors[$key] = $e->getMessage(); $custom_field_problem = true; } catch (\Exception $e) { report($e); diff --git a/app/Http/Requests/Assets/UpdateAssetRequest.php b/app/Http/Requests/Assets/UpdateAssetRequest.php index 1732b750283f..5c8d524bdee9 100644 --- a/app/Http/Requests/Assets/UpdateAssetRequest.php +++ b/app/Http/Requests/Assets/UpdateAssetRequest.php @@ -30,7 +30,6 @@ public function authorize() public function rules() { $modelRules = (new Asset)->getRules(); - // TODO: make sure this actually works if ((Setting::getSettings()->digit_separator === '1.234,56' || '1,234.56') && is_string($this->input('purchase_cost'))) { // If purchase_cost was submitted as a string with a comma separator // then we need to ignore the normal numeric rules. diff --git a/tests/Feature/Assets/Api/UpdateAssetTest.php b/tests/Feature/Assets/Api/UpdateAssetTest.php index 1cb6879d1ed4..df4448a2db5c 100644 --- a/tests/Feature/Assets/Api/UpdateAssetTest.php +++ b/tests/Feature/Assets/Api/UpdateAssetTest.php @@ -86,7 +86,7 @@ public function testAllAssetAttributesAreStored() $this->assertEquals('random_string', $updatedAsset->asset_tag); $this->assertEquals($userAssigned->id, $updatedAsset->assigned_to); $this->assertTrue($updatedAsset->company->is($company)); - $this->assertTrue($updatedAsset->location->is($location)); //fix all location setting + $this->assertTrue($updatedAsset->location->is($location)); $this->assertTrue($updatedAsset->model->is($model)); $this->assertEquals('A New Asset', $updatedAsset->name); $this->assertEquals('Some notes', $updatedAsset->notes); diff --git a/tests/Feature/Assets/Ui/StoreAssetTest.php b/tests/Feature/Assets/Ui/StoreAssetTest.php index 560dec41251a..db1cd1eb09c7 100644 --- a/tests/Feature/Assets/Ui/StoreAssetTest.php +++ b/tests/Feature/Assets/Ui/StoreAssetTest.php @@ -12,7 +12,6 @@ use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\Storage; use Tests\TestCase; -use function Amp\Promise\wait; class StoreAssetTest extends TestCase { From 7edff4b2bae85e91bf89ca363f8c92be2c12fb31 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Thu, 23 Jan 2025 12:54:10 -0600 Subject: [PATCH 41/42] rm a couple of comments --- tests/Unit/DepreciationTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/Unit/DepreciationTest.php b/tests/Unit/DepreciationTest.php index 7fa18c3c126c..507523d581f1 100644 --- a/tests/Unit/DepreciationTest.php +++ b/tests/Unit/DepreciationTest.php @@ -38,8 +38,6 @@ public function testDepreciationAmount() ->laptopMbp() ->create( [ - //not sure how this ever worked... do these need a category?? - //'category_id' => Category::factory()->assetLaptopCategory()->create(), 'purchase_date' => now()->subDecade()->format("Y-m-d"), 'purchase_cost' => 4000, ]); @@ -63,8 +61,6 @@ public function testDepreciationPercentage() ->laptopMbp() ->create( [ - //not sure how this ever worked... - //'category_id' => Category::factory()->assetLaptopCategory()->create(), 'purchase_date' => now()->subDecade()->format("Y-m-d"), 'purchase_cost' => 4000, ]); From 2e958e6799c4f40e79e3fdb6244bcf2c3370c430 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Thu, 23 Jan 2025 13:19:33 -0600 Subject: [PATCH 42/42] added a permission to admin factory --- database/factories/UserFactory.php | 2 +- tests/Feature/Assets/Ui/BulkEditAssetsTest.php | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/database/factories/UserFactory.php b/database/factories/UserFactory.php index 4b752b736f96..f9c39fa87101 100644 --- a/database/factories/UserFactory.php +++ b/database/factories/UserFactory.php @@ -107,7 +107,7 @@ public function admin() return User::where('permissions->superuser', '1')->first() ?? User::factory()->firstAdmin(); }, ]; - }); + })->appendPermission(['assets.view.encrypted_custom_fields' => '1']); } public function viewAssets() diff --git a/tests/Feature/Assets/Ui/BulkEditAssetsTest.php b/tests/Feature/Assets/Ui/BulkEditAssetsTest.php index 5b6779032bdf..6708ebb3154c 100644 --- a/tests/Feature/Assets/Ui/BulkEditAssetsTest.php +++ b/tests/Feature/Assets/Ui/BulkEditAssetsTest.php @@ -225,8 +225,7 @@ public function testBulkEditAssetsRequiresAdminToUpdateEncryptedCustomFields() { $this->markIncompleteIfMySQL('Custom Fields tests do not work on mysql'); $edit_user = User::factory()->editAssets()->create(); - // admin used to work, but now only superuser does???? - $admin_user = User::factory()->superuser()->create(); + $admin_user = User::factory()->admin()->create(); CustomField::factory()->testEncrypted()->create();