From 747b41d0a6254b9148071fbe12ae926b2f95387e Mon Sep 17 00:00:00 2001 From: Julien Nahum Date: Tue, 14 Jan 2025 14:04:34 +0100 Subject: [PATCH] Add error logging for integration failures in AbstractIntegrationHandler - Introduced logging of integration failures to capture detailed error information. - Enhanced error handling by logging the form ID and integration ID along with the exception data. These changes improve the observability of integration processes, aiding in debugging and monitoring. --- .../Handlers/AbstractIntegrationHandler.php | 5 ++++ .../Forms/FormEmailNotification.php | 12 +++++++- .../Feature/Forms/EmailNotificationTest.php | 30 ++++++++++++++++++- 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/api/app/Integrations/Handlers/AbstractIntegrationHandler.php b/api/app/Integrations/Handlers/AbstractIntegrationHandler.php index cf90f267..650e4fd7 100644 --- a/api/app/Integrations/Handlers/AbstractIntegrationHandler.php +++ b/api/app/Integrations/Handlers/AbstractIntegrationHandler.php @@ -10,6 +10,7 @@ use App\Service\Forms\FormSubmissionFormatter; use App\Service\Forms\FormLogicConditionChecker; use Illuminate\Http\Client\RequestException; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Log; use Vinkla\Hashids\Facades\Hashids; abstract class AbstractIntegrationHandler @@ -74,6 +75,10 @@ abstract class AbstractIntegrationHandler 'status' => FormIntegrationsEvent::STATUS_ERROR, 'data' => $this->extractEventDataFromException($e), ]); + Log::error('Integration failed', array_merge([ + 'form_id' => $this->form->id, + 'integration_id' => $this->formIntegration->id, + ], $this->extractEventDataFromException($e))); } } diff --git a/api/app/Notifications/Forms/FormEmailNotification.php b/api/app/Notifications/Forms/FormEmailNotification.php index 5f053c2f..8d37082b 100644 --- a/api/app/Notifications/Forms/FormEmailNotification.php +++ b/api/app/Notifications/Forms/FormEmailNotification.php @@ -106,7 +106,14 @@ class FormEmailNotification extends Notification return $this->integrationData->sender_email; } - return config('mail.from.address'); + $baseEmail = config('mail.from.address'); + if (!config('app.self_hosted')) { + // Insert timestamp before the @ to prevent email grouping + $parts = explode('@', $baseEmail); + return $parts[0] . '+' . time() . '@' . $parts[1]; + } + + return $baseEmail; } private function getSenderName(): string @@ -155,6 +162,9 @@ class FormEmailNotification extends Notification // Add our custom Message-ID as X-Custom-Message-ID $message->getHeaders()->addTextHeader('X-Custom-Message-ID', $messageId); + // Add X-Entity-Ref-ID header for Google+ notifications + $message->getHeaders()->addTextHeader('X-Entity-Ref-ID', $messageId); + // Add References header $message->getHeaders()->addTextHeader('References', $references); diff --git a/api/tests/Feature/Forms/EmailNotificationTest.php b/api/tests/Feature/Forms/EmailNotificationTest.php index 85073609..1ef97c93 100644 --- a/api/tests/Feature/Forms/EmailNotificationTest.php +++ b/api/tests/Feature/Forms/EmailNotificationTest.php @@ -3,6 +3,7 @@ use App\Notifications\Forms\FormEmailNotification; use Tests\Helpers\FormSubmissionDataFactory; use Illuminate\Notifications\AnonymousNotifiable; +use Illuminate\Support\Facades\Notification; it('send email with the submitted data', function () { $user = $this->actingAsUser(); @@ -159,7 +160,7 @@ it('does not use custom sender email in non-self-hosted mode', function () { $notifiable->route('mail', 'test@test.com'); $renderedMail = $mailable->toMail($notifiable); - expect($renderedMail->from[0])->toBe('default@example.com'); + expect($renderedMail->from[0])->toMatch('/^default\+\d+@example\.com$/'); expect($renderedMail->from[1])->toBe('Custom Sender'); expect($renderedMail->subject)->toBe('Custom Subject'); expect(trim($renderedMail->render()))->toContain('Custom content'); @@ -226,3 +227,30 @@ it('send email with empty reply to', function () { $renderedMail = $mailable->toMail($notifiable); expect($renderedMail->replyTo[0][0])->toBe($form->creator->email); }); + +it('uses exact email address without timestamp in self-hosted mode', function () { + config(['app.self_hosted' => true]); + config(['mail.from.address' => 'default@example.com']); + + $user = $this->actingAsUser(); + $workspace = $this->createUserWorkspace($user); + $form = $this->createForm($user, $workspace); + $integrationData = $this->createFormIntegration('email', $form->id, [ + 'send_to' => 'test@test.com', + 'sender_name' => 'Custom Sender', + 'subject' => 'Test Subject', + 'email_content' => 'Test content', + 'include_submission_data' => true, + ]); + + $formData = FormSubmissionDataFactory::generateSubmissionData($form); + + $event = new \App\Events\Forms\FormSubmitted($form, $formData); + $mailable = new FormEmailNotification($event, $integrationData, 'mail'); + $notifiable = new AnonymousNotifiable(); + $notifiable->route('mail', 'test@test.com'); + $renderedMail = $mailable->toMail($notifiable); + + // In self-hosted mode, the email should be exactly as configured without timestamp + expect($renderedMail->from[0])->toBe('default@example.com'); +});