From b635822a856c281f0dae91d5ad662a92c4ad8d86 Mon Sep 17 00:00:00 2001 From: Godfrey M Date: Thu, 9 Jan 2025 12:51:54 -0800 Subject: [PATCH 1/4] add try/catch around notification failing for inactive slack hooks --- app/Listeners/CheckoutableListener.php | 81 +++++++++++++++----------- 1 file changed, 48 insertions(+), 33 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 22867da32f2..461a3162f25 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -46,7 +46,7 @@ class CheckoutableListener */ public function onCheckedOut($event) { - if ($this->shouldNotSendAnyNotifications($event->checkoutable)){ + if ($this->shouldNotSendAnyNotifications($event->checkoutable)) { return; } @@ -57,7 +57,7 @@ public function onCheckedOut($event) $acceptance = $this->getCheckoutAcceptance($event); $adminCcEmailsArray = []; - if($settings->admin_cc_email !== '') { + if ($settings->admin_cc_email !== '') { $adminCcEmail = $settings->admin_cc_email; $adminCcEmailsArray = array_map('trim', explode(',', $adminCcEmail)); } @@ -65,7 +65,7 @@ public function onCheckedOut($event) $mailable = $this->getCheckoutMailType($event, $acceptance); $notifiable = $this->getNotifiables($event); - if ($event->checkedOutTo->locale){ + if ($event->checkedOutTo->locale) { $mailable->locale($event->checkedOutTo->locale); } // Send email notifications @@ -77,41 +77,50 @@ public function onCheckedOut($event) * 3. The item should send an email at check-in/check-out */ - if ($event->checkoutable->requireAcceptance() || $event->checkoutable->getEula() || - $this->checkoutableShouldSendEmail($event)) { - Log::info('Sending checkout email, Locale: ' . ($event->checkedOutTo->locale ?? 'default')); - if (!empty($notifiable)) { - Mail::to($notifiable)->cc($ccEmails)->send($mailable); - } elseif (!empty($ccEmails)) { - Mail::cc($ccEmails)->send($mailable); - } - Log::info('Checkout Mail sent.'); + if ($event->checkoutable->requireAcceptance() || $event->checkoutable->getEula() || + $this->checkoutableShouldSendEmail($event)) { + Log::info('Sending checkout email, Locale: ' . ($event->checkedOutTo->locale ?? 'default')); + if (!empty($notifiable)) { + Mail::to($notifiable)->cc($ccEmails)->send($mailable); + } elseif (!empty($ccEmails)) { + Mail::cc($ccEmails)->send($mailable); } + Log::info('Checkout Mail sent.'); + } } catch (ClientException $e) { Log::debug("Exception caught during checkout email: " . $e->getMessage()); } catch (Exception $e) { Log::debug("Exception caught during checkout email: " . $e->getMessage()); } // Send Webhook notification - try{ - if ($this->shouldSendWebhookNotification()) { - if ($this->newMicrosoftTeamsWebhookEnabled()) { - $message = $this->getCheckoutNotification($event)->toMicrosoftTeams(); - $notification = new TeamsNotification(Setting::getSettings()->webhook_endpoint); - $notification->success()->sendMessage($message[0], $message[1]); // Send the message to Microsoft Teams - } else { - Notification::route($this->webhookSelected(), Setting::getSettings()->webhook_endpoint) - ->notify($this->getCheckoutNotification($event, $acceptance)); - } + try { + if ($this->shouldSendWebhookNotification()) { + if ($this->newMicrosoftTeamsWebhookEnabled()) { + $message = $this->getCheckoutNotification($event)->toMicrosoftTeams(); + $notification = new TeamsNotification(Setting::getSettings()->webhook_endpoint); + $notification->success()->sendMessage($message[0], $message[1]); // Send the message to Microsoft Teams + } else { + + Notification::route($this->webhookSelected(), Setting::getSettings()->webhook_endpoint) + ->notify($this->getCheckoutNotification($event, $acceptance)); } + } } catch (ClientException $e) { - Log::debug("Exception caught during checkout notification: " . $e->getMessage()); + Log::warning("ClientException caught during checkin notification: " . $e->getMessage()); + return redirect()->back()->with('error', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure your URL is still valid.'); } catch (Exception $e) { - Log::debug("Exception caught during checkout notification: " . $e->getMessage()); + Log::error(ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed:', [ + 'error' => $e->getMessage(), + 'webhook_endpoint' => Setting::getSettings()->webhook_endpoint, + 'event' => $event, + ]); + return redirect()->back()->with('error', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure your URL is still valid.'); } } + + /** * Notify the user and post to webhook about the checked in checkoutable */ @@ -178,18 +187,24 @@ public function onCheckedIn($event) try { if ($this->shouldSendWebhookNotification()) { if ($this->newMicrosoftTeamsWebhookEnabled()) { - $message = $this->getCheckinNotification($event)->toMicrosoftTeams(); - $notification = new TeamsNotification(Setting::getSettings()->webhook_endpoint); - $notification->success()->sendMessage($message[0], $message[1]); // Send the message to Microsoft Teams - } else { - Notification::route($this->webhookSelected(), Setting::getSettings()->webhook_endpoint) - ->notify($this->getCheckinNotification($event)); - } + $message = $this->getCheckinNotification($event)->toMicrosoftTeams(); + $notification = new TeamsNotification(Setting::getSettings()->webhook_endpoint); + $notification->success()->sendMessage($message[0], $message[1]); // Send the message to Microsoft Teams + } else { + Notification::route($this->webhookSelected(), Setting::getSettings()->webhook_endpoint) + ->notify($this->getCheckinNotification($event)); } + } } catch (ClientException $e) { - Log::warning("Exception caught during checkin notification: " . $e->getMessage()); + Log::warning("ClientException caught during checkin notification: " . $e->getMessage()); + return redirect()->back()->with('error', ucfirst(Setting::getSettings()->webhook_selected) .' webhook notification failed: Check to make sure the URL is still valid.'); } catch (Exception $e) { - Log::warning("Exception caught during checkin notification: " . $e->getMessage()); + Log::error(ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed:', [ + 'error' => $e->getMessage(), + 'webhook_endpoint' => Setting::getSettings()->webhook_endpoint, + 'event' => $event, + ]); + return redirect()->back()->with('error', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure the URL is still valid.'); } } From 152c23b8f3572b7c982a0194b6c827294790b1f5 Mon Sep 17 00:00:00 2001 From: Godfrey M Date: Thu, 9 Jan 2025 12:55:25 -0800 Subject: [PATCH 2/4] changed log from warning to error --- app/Listeners/CheckoutableListener.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 461a3162f25..e97894411ac 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -106,7 +106,7 @@ public function onCheckedOut($event) } } } catch (ClientException $e) { - Log::warning("ClientException caught during checkin notification: " . $e->getMessage()); + Log::error("ClientException caught during checkin notification: " . $e->getMessage()); return redirect()->back()->with('error', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure your URL is still valid.'); } catch (Exception $e) { Log::error(ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed:', [ @@ -196,7 +196,7 @@ public function onCheckedIn($event) } } } catch (ClientException $e) { - Log::warning("ClientException caught during checkin notification: " . $e->getMessage()); + Log::error("ClientException caught during checkin notification: " . $e->getMessage()); return redirect()->back()->with('error', ucfirst(Setting::getSettings()->webhook_selected) .' webhook notification failed: Check to make sure the URL is still valid.'); } catch (Exception $e) { Log::error(ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed:', [ From 082a974ee35d064dfed58b1cb28ee5fc130e0959 Mon Sep 17 00:00:00 2001 From: Godfrey M Date: Thu, 9 Jan 2025 14:26:54 -0800 Subject: [PATCH 3/4] changed from redirect with error to with warning --- app/Listeners/CheckoutableListener.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index e97894411ac..dafb7744661 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -107,14 +107,14 @@ public function onCheckedOut($event) } } catch (ClientException $e) { Log::error("ClientException caught during checkin notification: " . $e->getMessage()); - return redirect()->back()->with('error', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure your URL is still valid.'); + return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure your URL is still valid.'); } catch (Exception $e) { Log::error(ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed:', [ 'error' => $e->getMessage(), 'webhook_endpoint' => Setting::getSettings()->webhook_endpoint, 'event' => $event, ]); - return redirect()->back()->with('error', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure your URL is still valid.'); + return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure your URL is still valid.'); } } @@ -197,14 +197,14 @@ public function onCheckedIn($event) } } catch (ClientException $e) { Log::error("ClientException caught during checkin notification: " . $e->getMessage()); - return redirect()->back()->with('error', ucfirst(Setting::getSettings()->webhook_selected) .' webhook notification failed: Check to make sure the URL is still valid.'); + return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) .' webhook notification failed: Check to make sure the URL is still valid.'); } catch (Exception $e) { Log::error(ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed:', [ 'error' => $e->getMessage(), 'webhook_endpoint' => Setting::getSettings()->webhook_endpoint, 'event' => $event, ]); - return redirect()->back()->with('error', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure the URL is still valid.'); + return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure the URL is still valid.'); } } From 001ccf1ce95328a01e81f3d9b3efbd5c2a8d358f Mon Sep 17 00:00:00 2001 From: Godfrey M Date: Thu, 9 Jan 2025 14:47:07 -0800 Subject: [PATCH 4/4] adds translation string instead of hardcoded message --- app/Listeners/CheckoutableListener.php | 8 ++++---- resources/lang/en-US/admin/settings/message.php | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index dafb7744661..b297a4f7207 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -107,14 +107,14 @@ public function onCheckedOut($event) } } catch (ClientException $e) { Log::error("ClientException caught during checkin notification: " . $e->getMessage()); - return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure your URL is still valid.'); + return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) .trans('admin/settings/message.webhook.webhook_fail') ); } catch (Exception $e) { Log::error(ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed:', [ 'error' => $e->getMessage(), 'webhook_endpoint' => Setting::getSettings()->webhook_endpoint, 'event' => $event, ]); - return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure your URL is still valid.'); + return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) . trans('admin/settings/message.webhook.webhook_fail')); } } @@ -197,14 +197,14 @@ public function onCheckedIn($event) } } catch (ClientException $e) { Log::error("ClientException caught during checkin notification: " . $e->getMessage()); - return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) .' webhook notification failed: Check to make sure the URL is still valid.'); + return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) .trans('admin/settings/message.webhook.webhook_fail')); } catch (Exception $e) { Log::error(ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed:', [ 'error' => $e->getMessage(), 'webhook_endpoint' => Setting::getSettings()->webhook_endpoint, 'event' => $event, ]); - return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed: Check to make sure the URL is still valid.'); + return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) .trans('admin/settings/message.webhook.webhook_fail')); } } diff --git a/resources/lang/en-US/admin/settings/message.php b/resources/lang/en-US/admin/settings/message.php index c91575144e5..98a8893937a 100644 --- a/resources/lang/en-US/admin/settings/message.php +++ b/resources/lang/en-US/admin/settings/message.php @@ -45,5 +45,6 @@ 'error' => 'Something went wrong. :app responded with: :error_message', 'error_redirect' => 'ERROR: 301/302 :endpoint returns a redirect. For security reasons, we don’t follow redirects. Please use the actual endpoint.', 'error_misc' => 'Something went wrong. :( ', + 'webhook_fail' => ' webhook notification failed: Check to make sure the URL is still valid.', ] ];