From c71411465a03499caf98937857e95ec489e865b4 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Tue, 24 Sep 2024 15:17:35 +0100 Subject: [PATCH 1/2] First pass at better-handling those annoying Rollbars we keep getting --- .../Controllers/Assets/AssetsController.php | 34 ++++++++++++++----- .../lang/en-US/admin/hardware/message.php | 3 ++ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index dceaa9b08a..315c86f725 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -112,8 +112,10 @@ class AssetsController extends Controller $settings = Setting::getSettings(); - $success = false; + $successes = 0; + $failures = 0; $serials = $request->input('serials'); + $last_succesful_asset = null; for ($a = 1; $a <= count($asset_tags); $a++) { $asset = new Asset(); @@ -200,20 +202,36 @@ class AssetsController extends Controller $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; - + $last_succesful_asset = $asset; + $successes++; + + } else { + $failures++; } } session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); - if ($success) { + if ($successes > 0) { + if ($failures > 0) { + //some succeeded, some failed + return redirect()->to(Helper::getRedirectOption($request, $last_succesful_asset->id, 'Assets')) + ->with('success-unescaped', trans_choice('admin/hardware/message.create.multi_success_linked', $successes, ['link' => route('hardware.show', ['hardware' => $last_succesful_asset->id]), 'id', 'tag' => e($last_succesful_asset->asset_tag)])) + ->with('warning', trans_choice('admin/hardware/message.create.partial_failure', $failures)); + } else { + if ($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, $last_succesful_asset->id, 'Assets')) + ->with('success-unescaped', trans('admin/hardware/message.create.success_linked', ['link' => route('hardware.show', ['hardware' => $last_succesful_asset->id]), 'id', 'tag' => e($last_succesful_asset->asset_tag)])); + } else { + //multi-success + return redirect()->to(Helper::getRedirectOption($request, $last_succesful_asset->id, 'Assets')) + ->with('success-unescaped', trans_choice('admin/hardware/message.create.multi_success_linked', $successes, ['link' => route('hardware.show', ['hardware' => $last_succesful_asset->id]), 'id', 'tag' => e($last_succesful_asset->asset_tag)])); + } + } - 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/resources/lang/en-US/admin/hardware/message.php b/resources/lang/en-US/admin/hardware/message.php index 041d32f56c..a33aea1813 100644 --- a/resources/lang/en-US/admin/hardware/message.php +++ b/resources/lang/en-US/admin/hardware/message.php @@ -14,6 +14,9 @@ return [ '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.', + 'multi_success_linked' => 'Asset with tag :tag was created successfully. Click here to view.|:count assets were created succesfully. The last one was :tag. Click here to view.', + 'partial_success_linked' => 'Asset with tag :tag was created successfully. Click here to view.', + 'partial_failure' => 'An asset was unable to be created.|:count assets were unable to be created.' ], 'update' => [ From b6340532d7e34c31d65999e55440241935c6db60 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Tue, 24 Sep 2024 17:15:39 +0100 Subject: [PATCH 2/2] Improve the error and success messages and linking --- .../Controllers/Assets/AssetsController.php | 31 +++++++++---------- .../lang/en-US/admin/hardware/message.php | 5 ++- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 315c86f725..e37d7c59b3 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -112,10 +112,10 @@ class AssetsController extends Controller $settings = Setting::getSettings(); - $successes = 0; - $failures = 0; + $successes = []; + $failures = []; $serials = $request->input('serials'); - $last_succesful_asset = null; + $asset = null; for ($a = 1; $a <= count($asset_tags); $a++) { $asset = new Asset(); @@ -202,33 +202,32 @@ class AssetsController extends Controller $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); } - $last_succesful_asset = $asset; - $successes++; + $successes[] = " $asset->id]) . "' style='color: white;'>" . e($asset->asset_tag) . ""; } else { - $failures++; + $failures[] = join(",", $asset->getErrors()->all()); } } session()->put(['redirect_option' => $request->get('redirect_option'), 'checkout_to_type' => $request->get('checkout_to_type')]); - if ($successes > 0) { - if ($failures > 0) { + if ($successes) { + if ($failures) { //some succeeded, some failed - return redirect()->to(Helper::getRedirectOption($request, $last_succesful_asset->id, 'Assets')) - ->with('success-unescaped', trans_choice('admin/hardware/message.create.multi_success_linked', $successes, ['link' => route('hardware.show', ['hardware' => $last_succesful_asset->id]), 'id', 'tag' => e($last_succesful_asset->asset_tag)])) - ->with('warning', trans_choice('admin/hardware/message.create.partial_failure', $failures)); + 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 ($successes == 1) { + 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, $last_succesful_asset->id, 'Assets')) - ->with('success-unescaped', trans('admin/hardware/message.create.success_linked', ['link' => route('hardware.show', ['hardware' => $last_succesful_asset->id]), 'id', 'tag' => e($last_succesful_asset->asset_tag)])); + 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, $last_succesful_asset->id, 'Assets')) - ->with('success-unescaped', trans_choice('admin/hardware/message.create.multi_success_linked', $successes, ['link' => route('hardware.show', ['hardware' => $last_succesful_asset->id]), 'id', 'tag' => e($last_succesful_asset->asset_tag)])); + 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)])); } } diff --git a/resources/lang/en-US/admin/hardware/message.php b/resources/lang/en-US/admin/hardware/message.php index a33aea1813..874888de9d 100644 --- a/resources/lang/en-US/admin/hardware/message.php +++ b/resources/lang/en-US/admin/hardware/message.php @@ -14,9 +14,8 @@ return [ '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.', - 'multi_success_linked' => 'Asset with tag :tag was created successfully. Click here to view.|:count assets were created succesfully. The last one was :tag. Click here to view.', - 'partial_success_linked' => 'Asset with tag :tag was created successfully. Click here to view.', - 'partial_failure' => 'An asset was unable to be created.|:count assets were unable to be created.' + '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' => [