From 705a852d45101dba836f6d4612721ea3a2f93ae6 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Mon, 25 Nov 2024 20:09:30 -0600 Subject: [PATCH] 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 9305b031a3..b80893d2f3 100644 --- a/app/Actions/Assets/StoreAssetAction.php +++ b/app/Actions/Assets/StoreAssetAction.php @@ -47,7 +47,7 @@ class StoreAssetAction $assigned_asset = null, $assigned_location = null, $last_audit_date = null, - ) + ): Asset|bool { $settings = Setting::getSettings(); @@ -96,7 +96,6 @@ class StoreAssetAction $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 @@ class StoreAssetAction } } - // 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 @@ class StoreAssetAction $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 dc05b4985a..264dc26bf7 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -6,13 +6,14 @@ 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\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 @@ class AssetsController extends Controller { 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 @@ class AssetsController extends Controller 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 @@ class AssetsController extends Controller 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 @@ class AssetsController extends Controller 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 3f889e65d6..2ff9ce66cc 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -15,6 +15,7 @@ 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; @@ -150,7 +151,7 @@ class AssetsController extends Controller ->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 @@ class AssetsController extends Controller ->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 @@ class AssetsController extends Controller 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 @@ class AssetsController extends Controller 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 @@ class AssetsController extends Controller $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 = [];