From 4908064f6d15fa4775379efb3d9341ec3edefceb Mon Sep 17 00:00:00 2001 From: Eric Peters Date: Fri, 24 Apr 2026 18:29:32 +0200 Subject: [PATCH] Implement Discord webhook delivery --- .../DeliverNotificationAction.php | 120 +++++++++-- ...ketStatusChangedNotificationDeliveries.php | 2 +- .../RenderDiscordMessageTemplate.php | 21 ++ .../DeliverNotificationActionTest.php | 195 +++++++++++++++++- .../RenderDiscordMessageTemplateTest.php | 95 +++++++++ ...tStatusChangedNotificationDispatchTest.php | 20 +- 6 files changed, 429 insertions(+), 24 deletions(-) create mode 100644 app/Actions/Notifications/RenderDiscordMessageTemplate.php create mode 100644 tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php diff --git a/app/Actions/Notifications/DeliverNotificationAction.php b/app/Actions/Notifications/DeliverNotificationAction.php index 47ebf90..25783f4 100644 --- a/app/Actions/Notifications/DeliverNotificationAction.php +++ b/app/Actions/Notifications/DeliverNotificationAction.php @@ -19,6 +19,8 @@ class DeliverNotificationAction implements ShouldQueue { use Queueable; + private const int DISCORD_CONTENT_LIMIT = 2000; + public int $tries = 3; public int $timeout = 15; @@ -45,22 +47,38 @@ class DeliverNotificationAction implements ShouldQueue return; } - if ($notificationAction->channel !== NotificationChannel::Webhook) { + if (! in_array($notificationAction->channel, [NotificationChannel::Discord, NotificationChannel::Webhook], true)) { return; } $deliveryLog = $this->resolveDeliveryLog($notificationAction); $attemptCount = $this->attempts(); $attemptedAt = CarbonImmutable::now(); + $discordMessage = null; + + if ($notificationAction->channel === NotificationChannel::Discord) { + $discordMessage = app(RenderDiscordMessageTemplate::class)->handle( + $notificationAction->message_template, + $this->payload, + ); + + $validationError = $this->discordMessageValidationError($discordMessage); + + if ($validationError !== null) { + $this->markFailedWithoutResponse($deliveryLog, $attemptCount, $attemptedAt, $validationError); + $this->fail(new \RuntimeException($validationError)); + + return; + } + } try { - $response = Http::acceptJson() - ->asJson() - ->connectTimeout(5) - ->timeout(10) - ->post($notificationAction->delivery_url, $this->payload->toArray()); + $response = match ($notificationAction->channel) { + NotificationChannel::Discord => $this->sendDiscordRequest($notificationAction, $discordMessage ?? ''), + NotificationChannel::Webhook => $this->sendWebhookRequest($notificationAction), + }; } catch (ConnectionException $exception) { - $this->handleConnectionFailure($deliveryLog, $attemptCount, $attemptedAt, $exception); + $this->handleConnectionFailure($deliveryLog, $notificationAction->channel, $attemptCount, $attemptedAt, $exception); return; } @@ -72,14 +90,14 @@ class DeliverNotificationAction implements ShouldQueue } if ($this->shouldRetry($response) && $this->hasAttemptsRemaining($attemptCount)) { - $this->markRetryable($deliveryLog, $response, $attemptCount, $attemptedAt); + $this->markRetryable($deliveryLog, $notificationAction->channel, $response, $attemptCount, $attemptedAt); $this->release($this->backoffDelay($attemptCount)); return; } - $this->markFailed($deliveryLog, $response, $attemptCount, $attemptedAt); - $this->fail(new \RuntimeException($this->responseFailureDescription($response))); + $this->markFailed($deliveryLog, $notificationAction->channel, $response, $attemptCount, $attemptedAt); + $this->fail(new \RuntimeException($this->responseFailureDescription($notificationAction->channel, $response))); } public function failed(?Throwable $exception): void @@ -101,11 +119,12 @@ class DeliverNotificationAction implements ShouldQueue private function handleConnectionFailure( NotificationDeliveryLog $deliveryLog, + NotificationChannel $channel, int $attemptCount, CarbonImmutable $attemptedAt, ConnectionException $exception, ): void { - $errorExcerpt = $this->excerpt($exception->getMessage()) ?? 'Webhook request failed.'; + $errorExcerpt = $this->excerpt($exception->getMessage()) ?? $this->requestFailureDescription($channel); if ($this->hasAttemptsRemaining($attemptCount)) { $deliveryLog->update([ @@ -155,6 +174,7 @@ class DeliverNotificationAction implements ShouldQueue private function markRetryable( NotificationDeliveryLog $deliveryLog, + NotificationChannel $channel, Response $response, int $attemptCount, CarbonImmutable $attemptedAt, @@ -163,7 +183,7 @@ class DeliverNotificationAction implements ShouldQueue 'attempt_count' => $attemptCount, 'delivery_status' => NotificationDeliveryStatus::Retryable, 'delivered_at' => null, - 'error_excerpt' => $this->responseFailureDescription($response), + 'error_excerpt' => $this->responseFailureDescription($channel, $response), 'http_status' => $response->status(), 'last_attempted_at' => $attemptedAt, 'response_excerpt' => $this->responseExcerpt($response), @@ -172,6 +192,7 @@ class DeliverNotificationAction implements ShouldQueue private function markFailed( NotificationDeliveryLog $deliveryLog, + NotificationChannel $channel, Response $response, int $attemptCount, CarbonImmutable $attemptedAt, @@ -180,13 +201,30 @@ class DeliverNotificationAction implements ShouldQueue 'attempt_count' => $attemptCount, 'delivery_status' => NotificationDeliveryStatus::Failed, 'delivered_at' => null, - 'error_excerpt' => $this->responseFailureDescription($response), + 'error_excerpt' => $this->responseFailureDescription($channel, $response), 'http_status' => $response->status(), 'last_attempted_at' => $attemptedAt, 'response_excerpt' => $this->responseExcerpt($response), ]); } + private function markFailedWithoutResponse( + NotificationDeliveryLog $deliveryLog, + int $attemptCount, + CarbonImmutable $attemptedAt, + string $errorExcerpt, + ): void { + $deliveryLog->update([ + 'attempt_count' => $attemptCount, + 'delivery_status' => NotificationDeliveryStatus::Failed, + 'delivered_at' => null, + 'error_excerpt' => $errorExcerpt, + 'http_status' => null, + 'last_attempted_at' => $attemptedAt, + 'response_excerpt' => null, + ]); + } + private function resolveDeliveryLog(NotificationAction $notificationAction): NotificationDeliveryLog { $deliveryLog = $this->loadDeliveryLog(); @@ -226,6 +264,26 @@ class DeliverNotificationAction implements ShouldQueue return $response->serverError() || in_array($response->status(), [408, 429], true); } + private function sendDiscordRequest(NotificationAction $notificationAction, string $message): Response + { + return Http::acceptJson() + ->asJson() + ->connectTimeout(5) + ->timeout(10) + ->post($notificationAction->delivery_url, [ + 'content' => $message, + ]); + } + + private function sendWebhookRequest(NotificationAction $notificationAction): Response + { + return Http::acceptJson() + ->asJson() + ->connectTimeout(5) + ->timeout(10) + ->post($notificationAction->delivery_url, $this->payload->toArray()); + } + private function hasAttemptsRemaining(int $attemptCount): bool { return $attemptCount < $this->tries; @@ -244,15 +302,45 @@ class DeliverNotificationAction implements ShouldQueue return $this->excerpt($response->body()) ?? $this->excerpt($response->reason()); } - private function responseFailureDescription(Response $response): string + private function responseFailureDescription(NotificationChannel $channel, Response $response): string { $reason = $this->excerpt($response->reason()); + $subject = $this->deliverySubject($channel); if ($reason === null) { - return "Webhook returned HTTP {$response->status()}."; + return "{$subject} returned HTTP {$response->status()}."; } - return "Webhook returned HTTP {$response->status()} {$reason}."; + return "{$subject} returned HTTP {$response->status()} {$reason}."; + } + + private function requestFailureDescription(NotificationChannel $channel): string + { + return match ($channel) { + NotificationChannel::Discord => 'Discord webhook request failed.', + NotificationChannel::Webhook => 'Webhook request failed.', + }; + } + + private function deliverySubject(NotificationChannel $channel): string + { + return match ($channel) { + NotificationChannel::Discord => 'Discord webhook', + NotificationChannel::Webhook => 'Webhook', + }; + } + + private function discordMessageValidationError(string $message): ?string + { + if (trim($message) === '') { + return 'Discord message content cannot be empty.'; + } + + if (Str::length($message) > self::DISCORD_CONTENT_LIMIT) { + return 'Discord message content exceeds the 2000 character limit.'; + } + + return null; } /** diff --git a/app/Actions/Notifications/QueueTicketStatusChangedNotificationDeliveries.php b/app/Actions/Notifications/QueueTicketStatusChangedNotificationDeliveries.php index 98a6d35..eb195cd 100644 --- a/app/Actions/Notifications/QueueTicketStatusChangedNotificationDeliveries.php +++ b/app/Actions/Notifications/QueueTicketStatusChangedNotificationDeliveries.php @@ -19,7 +19,7 @@ class QueueTicketStatusChangedNotificationDeliveries ->each(function (NotificationAction $notificationAction) use ($event): void { $notificationDeliveryLogId = null; - if ($notificationAction->channel === NotificationChannel::Webhook) { + if (in_array($notificationAction->channel, [NotificationChannel::Discord, NotificationChannel::Webhook], true)) { $notificationDeliveryLogId = NotificationDeliveryLog::query()->create([ 'attempt_count' => 0, 'channel' => $notificationAction->channel, diff --git a/app/Actions/Notifications/RenderDiscordMessageTemplate.php b/app/Actions/Notifications/RenderDiscordMessageTemplate.php new file mode 100644 index 0000000..d9be56e --- /dev/null +++ b/app/Actions/Notifications/RenderDiscordMessageTemplate.php @@ -0,0 +1,21 @@ + $payload->ticketUrl, + '$ticket_key' => $payload->ticketKey, + '$ticket_title' => $payload->ticketTitle, + '$project_name' => $payload->projectName, + '$status_from' => $payload->fromStatusLabel, + '$status_to' => $payload->toStatusLabel, + '$actor_name' => $payload->actorName ?? '', + ]); + } +} diff --git a/tests/Feature/Notifications/DeliverNotificationActionTest.php b/tests/Feature/Notifications/DeliverNotificationActionTest.php index 6a6503a..84fe9a0 100644 --- a/tests/Feature/Notifications/DeliverNotificationActionTest.php +++ b/tests/Feature/Notifications/DeliverNotificationActionTest.php @@ -59,6 +59,50 @@ class DeliverNotificationActionTest extends TestCase $this->assertNotNull($deliveryLog->delivered_at); } + /** + * Ensure Discord deliveries render the configured template into the expected payload. + */ + public function test_discord_delivery_posts_rendered_content_payload_and_marks_the_log_as_succeeded(): void + { + Http::preventStrayRequests(); + Http::fake([ + 'https://discord.com/api/webhooks/123456/discord-alerts' => Http::response('', 204), + ]); + + $notificationAction = NotificationAction::factory()->discord()->create([ + 'delivery_url' => 'https://discord.com/api/webhooks/123456/discord-alerts', + 'message_template' => '$ticket_key moved from $status_from to $status_to by $actor_name: $url ($unknown)', + ]); + $deliveryLog = $this->createPendingDeliveryLog($notificationAction); + + (new DeliverNotificationAction( + notificationActionId: $notificationAction->id, + payload: $this->payload(), + notificationDeliveryLogId: $deliveryLog->id, + ))->handle(); + + Http::assertSentCount(1); + Http::assertSent(function (Request $request): bool { + return $request->method() === 'POST' + && $request->url() === 'https://discord.com/api/webhooks/123456/discord-alerts' + && $request->hasHeader('Accept', 'application/json') + && $request->hasHeader('Content-Type', 'application/json') + && $request->data() === [ + 'content' => 'CORE-42 moved from Backlog to Doing by Erica Example: https://kanbangit.test/projects/core/tickets/42 ($unknown)', + ]; + }); + + $deliveryLog = $deliveryLog->fresh(); + + $this->assertSame(NotificationDeliveryStatus::Succeeded, $deliveryLog->delivery_status); + $this->assertSame(1, $deliveryLog->attempt_count); + $this->assertSame(204, $deliveryLog->http_status); + $this->assertSame('No Content', $deliveryLog->response_excerpt); + $this->assertNull($deliveryLog->error_excerpt); + $this->assertNotNull($deliveryLog->last_attempted_at); + $this->assertNotNull($deliveryLog->delivered_at); + } + /** * Ensure retryable webhook responses release the job and update the delivery log. */ @@ -96,6 +140,44 @@ class DeliverNotificationActionTest extends TestCase $this->assertNull($deliveryLog->delivered_at); } + /** + * Ensure retryable Discord webhook responses share the same retry and logging behavior. + */ + public function test_retryable_discord_responses_mark_the_log_retryable_and_release_the_job(): void + { + Http::preventStrayRequests(); + Http::fake([ + 'https://discord.com/api/webhooks/123456/discord-alerts' => Http::response([ + 'message' => 'Temporary outage', + ], 503), + ]); + + $notificationAction = NotificationAction::factory()->discord()->create([ + 'delivery_url' => 'https://discord.com/api/webhooks/123456/discord-alerts', + 'message_template' => '$ticket_key moved to $status_to', + ]); + $deliveryLog = $this->createPendingDeliveryLog($notificationAction); + $job = (new DeliverNotificationAction( + notificationActionId: $notificationAction->id, + payload: $this->payload(), + notificationDeliveryLogId: $deliveryLog->id, + ))->withFakeQueueInteractions(); + + $job->handle(); + + $job->assertReleased($job->backoff()[0]); + $job->assertNotFailed(); + + $deliveryLog = $deliveryLog->fresh(); + + $this->assertSame(NotificationDeliveryStatus::Retryable, $deliveryLog->delivery_status); + $this->assertSame(1, $deliveryLog->attempt_count); + $this->assertSame(503, $deliveryLog->http_status); + $this->assertStringContainsString('Temporary outage', (string) $deliveryLog->response_excerpt); + $this->assertSame('Discord webhook returned HTTP 503 Service Unavailable.', $deliveryLog->error_excerpt); + $this->assertNull($deliveryLog->delivered_at); + } + /** * Ensure permanent webhook errors fail immediately without scheduling another retry. */ @@ -132,6 +214,43 @@ class DeliverNotificationActionTest extends TestCase $this->assertStringContainsString('Webhook returned HTTP 422', (string) $deliveryLog->error_excerpt); } + /** + * Ensure permanent Discord webhook errors fail immediately without scheduling another retry. + */ + public function test_permanent_discord_responses_mark_the_log_as_failed(): void + { + Http::preventStrayRequests(); + Http::fake([ + 'https://discord.com/api/webhooks/123456/discord-alerts' => Http::response([ + 'message' => 'The message payload was invalid.', + ], 422), + ]); + + $notificationAction = NotificationAction::factory()->discord()->create([ + 'delivery_url' => 'https://discord.com/api/webhooks/123456/discord-alerts', + 'message_template' => '$ticket_key moved to $status_to', + ]); + $deliveryLog = $this->createPendingDeliveryLog($notificationAction); + $job = (new DeliverNotificationAction( + notificationActionId: $notificationAction->id, + payload: $this->payload(), + notificationDeliveryLogId: $deliveryLog->id, + ))->withFakeQueueInteractions(); + + $job->handle(); + + $job->assertFailed(); + $job->assertNotReleased(); + + $deliveryLog = $deliveryLog->fresh(); + + $this->assertSame(NotificationDeliveryStatus::Failed, $deliveryLog->delivery_status); + $this->assertSame(1, $deliveryLog->attempt_count); + $this->assertSame(422, $deliveryLog->http_status); + $this->assertStringContainsString('invalid', (string) $deliveryLog->response_excerpt); + $this->assertStringContainsString('Discord webhook returned HTTP 422', (string) $deliveryLog->error_excerpt); + } + /** * Ensure connection failures are marked retryable while attempts remain. */ @@ -168,6 +287,70 @@ class DeliverNotificationActionTest extends TestCase $this->assertSame('Connection timed out while contacting the remote endpoint.', $deliveryLog->error_excerpt); } + /** + * Ensure Discord deliveries reject empty rendered messages before sending the request. + */ + public function test_empty_rendered_discord_messages_are_rejected_and_logged_as_failed(): void + { + Http::preventStrayRequests(); + + $notificationAction = NotificationAction::factory()->discord()->create([ + 'message_template' => '$actor_name', + ]); + $deliveryLog = $this->createPendingDeliveryLog($notificationAction); + $job = (new DeliverNotificationAction( + notificationActionId: $notificationAction->id, + payload: $this->payload(actorName: null), + notificationDeliveryLogId: $deliveryLog->id, + ))->withFakeQueueInteractions(); + + $job->handle(); + + Http::assertNothingSent(); + $job->assertFailed(); + $job->assertNotReleased(); + + $deliveryLog = $deliveryLog->fresh(); + + $this->assertSame(NotificationDeliveryStatus::Failed, $deliveryLog->delivery_status); + $this->assertSame(1, $deliveryLog->attempt_count); + $this->assertNull($deliveryLog->http_status); + $this->assertNull($deliveryLog->response_excerpt); + $this->assertSame('Discord message content cannot be empty.', $deliveryLog->error_excerpt); + } + + /** + * Ensure Discord deliveries reject rendered content that exceeds Discord's limit. + */ + public function test_discord_messages_that_exceed_the_content_limit_are_rejected_and_logged_as_failed(): void + { + Http::preventStrayRequests(); + + $notificationAction = NotificationAction::factory()->discord()->create([ + 'message_template' => '$ticket_title', + ]); + $deliveryLog = $this->createPendingDeliveryLog($notificationAction); + $job = (new DeliverNotificationAction( + notificationActionId: $notificationAction->id, + payload: $this->payload(ticketTitle: str_repeat('A', 2001)), + notificationDeliveryLogId: $deliveryLog->id, + ))->withFakeQueueInteractions(); + + $job->handle(); + + Http::assertNothingSent(); + $job->assertFailed(); + $job->assertNotReleased(); + + $deliveryLog = $deliveryLog->fresh(); + + $this->assertSame(NotificationDeliveryStatus::Failed, $deliveryLog->delivery_status); + $this->assertSame(1, $deliveryLog->attempt_count); + $this->assertNull($deliveryLog->http_status); + $this->assertNull($deliveryLog->response_excerpt); + $this->assertSame('Discord message content exceeds the 2000 character limit.', $deliveryLog->error_excerpt); + } + /** * Ensure the last retryable attempt is marked failed once the job exhausts its retries. */ @@ -234,8 +417,10 @@ class DeliverNotificationActionTest extends TestCase ]); } - private function payload(): TicketStatusChangedPayload - { + private function payload( + ?string $actorName = 'Erica Example', + string $ticketTitle = 'Fix webhook delivery', + ): TicketStatusChangedPayload { return new TicketStatusChangedPayload( eventType: NotificationEventType::TicketStatusChanged, projectId: 10, @@ -243,10 +428,10 @@ class DeliverNotificationActionTest extends TestCase projectName: 'Core Platform', ticketId: 42, ticketKey: 'CORE-42', - ticketTitle: 'Fix webhook delivery', + ticketTitle: $ticketTitle, ticketUrl: 'https://kanbangit.test/projects/core/tickets/42', - actorId: 7, - actorName: 'Erica Example', + actorId: $actorName === null ? null : 7, + actorName: $actorName, fromStatusId: 1, fromStatusLabel: 'Backlog', toStatusId: 2, diff --git a/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php b/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php new file mode 100644 index 0000000..283b685 --- /dev/null +++ b/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php @@ -0,0 +1,95 @@ + + */ + public static function supportedVariableProvider(): array + { + return [ + 'url' => [ + 'template' => '$url', + 'expected' => 'https://kanbangit.test/projects/core/tickets/42', + ], + 'ticket_key' => [ + 'template' => '$ticket_key', + 'expected' => 'CORE-42', + ], + 'ticket_title' => [ + 'template' => '$ticket_title', + 'expected' => 'Fix webhook delivery', + ], + 'project_name' => [ + 'template' => '$project_name', + 'expected' => 'Core Platform', + ], + 'status_from' => [ + 'template' => '$status_from', + 'expected' => 'Backlog', + ], + 'status_to' => [ + 'template' => '$status_to', + 'expected' => 'Doing', + ], + 'actor_name' => [ + 'template' => '$actor_name', + 'expected' => 'Erica Example', + ], + ]; + } + + /** + * Ensure each supported Discord template variable renders from the event payload. + */ + #[DataProvider('supportedVariableProvider')] + public function test_it_replaces_supported_template_variables(string $template, string $expected): void + { + $renderedMessage = app(RenderDiscordMessageTemplate::class)->handle($template, $this->payload()); + + $this->assertSame($expected, $renderedMessage); + } + + /** + * Ensure unknown placeholder-like text stays untouched while supported variables still render. + */ + public function test_unknown_template_text_remains_literal(): void + { + $renderedMessage = app(RenderDiscordMessageTemplate::class)->handle( + '[$ticket_key] $unknown moved to $status_to', + $this->payload(), + ); + + $this->assertSame('[CORE-42] $unknown moved to Doing', $renderedMessage); + } + + private function payload(): TicketStatusChangedPayload + { + return new TicketStatusChangedPayload( + eventType: NotificationEventType::TicketStatusChanged, + projectId: 10, + projectKey: 'CORE', + projectName: 'Core Platform', + ticketId: 42, + ticketKey: 'CORE-42', + ticketTitle: 'Fix webhook delivery', + ticketUrl: 'https://kanbangit.test/projects/core/tickets/42', + actorId: 7, + actorName: 'Erica Example', + fromStatusId: 1, + fromStatusLabel: 'Backlog', + toStatusId: 2, + toStatusLabel: 'Doing', + occurredAt: CarbonImmutable::parse('2026-04-24T09:15:30+00:00'), + ); + } +} diff --git a/tests/Feature/Notifications/TicketStatusChangedNotificationDispatchTest.php b/tests/Feature/Notifications/TicketStatusChangedNotificationDispatchTest.php index a3f146a..da8aa82 100644 --- a/tests/Feature/Notifications/TicketStatusChangedNotificationDispatchTest.php +++ b/tests/Feature/Notifications/TicketStatusChangedNotificationDispatchTest.php @@ -117,12 +117,28 @@ class TicketStatusChangedNotificationDispatchTest extends TestCase && $job->payload->fromStatusLabel === 'Backlog' && $job->payload->toStatusId === $doingStatus->id && $job->payload->toStatusLabel === 'Doing' - && $job->payload->eventType === NotificationEventType::TicketStatusChanged; + && $job->payload->eventType === NotificationEventType::TicketStatusChanged + && $job->notificationDeliveryLogId !== null; }); - Queue::assertPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $discordAction->id); + Queue::assertPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $discordAction->id + && $job->notificationDeliveryLogId !== null); Queue::assertNotPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $disabledAction->id); Queue::assertNotPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $foreignProjectAction->id); + $this->assertDatabaseCount('notification_delivery_logs', 2); + $this->assertDatabaseHas('notification_delivery_logs', [ + 'notification_action_id' => $webhookAction->id, + 'delivery_status' => NotificationDeliveryStatus::Pending->value, + 'attempt_count' => 0, + 'http_status' => null, + ]); + $this->assertDatabaseHas('notification_delivery_logs', [ + 'notification_action_id' => $discordAction->id, + 'delivery_status' => NotificationDeliveryStatus::Pending->value, + 'attempt_count' => 0, + 'http_status' => null, + ]); + $this->assertDatabaseHas('ticket_worklog_entries', [ 'ticket_id' => $ticket->id, 'actor_id' => $user->id,