Merge pull request 'Implement Discord webhook delivery' (#59) from codex/v0.5-webhook-notification/05-discord-webhook-delivery into main

Reviewed-on: OpenSource/KanbanGit#59
2026-04-24 16:50:18 +00:00
parent f0800b1112 bd2355b7cd
commit 32edc255a3
6 changed files with 435 additions and 24 deletions
@@ -59,6 +59,53 @@ 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)',
+                    'allowed_mentions' => [
+                        'parse' => [],
+                    ],
+                ];
+        });
+
+        $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 +143,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 +217,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 +290,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 +420,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 +431,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,
@@ -0,0 +1,95 @@
+<?php
+
+namespace Tests\Feature\Notifications;
+
+use App\Actions\Notifications\RenderDiscordMessageTemplate;
+use App\Actions\Notifications\TicketStatusChangedPayload;
+use App\Enums\NotificationEventType;
+use Carbon\CarbonImmutable;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Tests\TestCase;
+
+class RenderDiscordMessageTemplateTest extends TestCase
+{
+    /**
+     * @return array<string, array{template: string, expected: string}>
+     */
+    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'),
+        );
+    }
+}
@@ -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,