From c9050886ac5c87e19bf20b2ae51eebe4a0e11ce5 Mon Sep 17 00:00:00 2001 From: Koda Date: Sat, 25 Apr 2026 00:38:51 +0000 Subject: [PATCH 1/2] Add assignee changed notification event support --- ...uildProjectNotificationSettingsPayload.php | 10 +- .../BuildTicketAssigneeChangedPayload.php | 63 ++++ .../MatchNotificationFilters.php | 10 + .../Notifications/TicketAssigneeChanged.php | 26 ++ .../TicketStatusChangedPayload.php | 43 ++- app/Actions/Tickets/UpdateProjectTicket.php | 37 +- app/Enums/NotificationEventType.php | 2 + app/Enums/NotificationFilterAttribute.php | 8 + app/Providers/AppServiceProvider.php | 6 + .../DeliverNotificationActionTest.php | 8 + .../RenderDiscordMessageTemplateTest.php | 12 + ...ssigneeChangedNotificationDispatchTest.php | 342 ++++++++++++++++++ ...jectNotificationSettingsControllerTest.php | 3 + 13 files changed, 558 insertions(+), 12 deletions(-) create mode 100644 app/Actions/Notifications/BuildTicketAssigneeChangedPayload.php create mode 100644 app/Actions/Notifications/TicketAssigneeChanged.php create mode 100644 tests/Feature/Notifications/TicketAssigneeChangedNotificationDispatchTest.php diff --git a/app/Actions/Notifications/BuildProjectNotificationSettingsPayload.php b/app/Actions/Notifications/BuildProjectNotificationSettingsPayload.php index 19d9cf8..ef2a338 100644 --- a/app/Actions/Notifications/BuildProjectNotificationSettingsPayload.php +++ b/app/Actions/Notifications/BuildProjectNotificationSettingsPayload.php @@ -60,7 +60,7 @@ class BuildProjectNotificationSettingsPayload 'value' => '$project_name', ], [ - 'description' => 'Origin status label for status moves; blank for ticket create.', + 'description' => 'Origin status label for status moves; blank for ticket create and assignee changes.', 'value' => '$status_from', ], [ @@ -71,6 +71,14 @@ class BuildProjectNotificationSettingsPayload 'description' => 'Actor name when the event has a known user.', 'value' => '$actor_name', ], + [ + 'description' => 'Previous assignee name when the event updates assignment; blank otherwise.', + 'value' => '$assignee_from', + ], + [ + 'description' => 'New assignee name when the event updates assignment; blank when unassigned.', + 'value' => '$assignee_to', + ], ]; } } diff --git a/app/Actions/Notifications/BuildTicketAssigneeChangedPayload.php b/app/Actions/Notifications/BuildTicketAssigneeChangedPayload.php new file mode 100644 index 0000000..6302327 --- /dev/null +++ b/app/Actions/Notifications/BuildTicketAssigneeChangedPayload.php @@ -0,0 +1,63 @@ +loadMissing('ticketStatus.boardColumn'); + + return new TicketStatusChangedPayload( + eventType: NotificationEventType::TicketAssigneeChanged, + projectId: $project->id, + projectKey: $project->key, + projectName: $project->name, + ticketId: $ticket->id, + ticketKey: $ticket->ticket_key, + ticketTitle: $ticket->title, + ticketUrl: route('projects.tickets.show', [$project, $ticket]), + actorId: $actor?->id, + actorName: $actor?->name, + fromStatusId: null, + fromStatusLabel: null, + toStatusId: $ticket->ticket_status_id, + toStatusLabel: $this->statusLabel($ticket->ticketStatus), + occurredAt: $occurredAt, + fromAssigneeId: $fromAssigneeId, + fromAssigneeName: $fromAssigneeName, + toAssigneeId: $toAssigneeId, + toAssigneeName: $toAssigneeName, + ); + } + + private function statusLabel(?TicketStatus $ticketStatus): string + { + if ($ticketStatus === null) { + return 'Unknown status'; + } + + $ticketStatus->loadMissing('boardColumn'); + + if ($ticketStatus->boardColumn === null || $ticketStatus->boardColumn->name === $ticketStatus->name) { + return $ticketStatus->name; + } + + return "{$ticketStatus->boardColumn->name} / {$ticketStatus->name}"; + } +} diff --git a/app/Actions/Notifications/MatchNotificationFilters.php b/app/Actions/Notifications/MatchNotificationFilters.php index 68ac489..1c620a5 100644 --- a/app/Actions/Notifications/MatchNotificationFilters.php +++ b/app/Actions/Notifications/MatchNotificationFilters.php @@ -207,6 +207,16 @@ class MatchNotificationFilters 'id' => $actorId, 'name' => $actorName, ], + 'assignee' => [ + 'from' => [ + 'id' => $payload->fromAssigneeId, + 'name' => $payload->fromAssigneeName, + ], + 'to' => [ + 'id' => $payload->toAssigneeId, + 'name' => $payload->toAssigneeName, + ], + ], 'status' => [ 'from' => [ 'id' => $fromStatusId, diff --git a/app/Actions/Notifications/TicketAssigneeChanged.php b/app/Actions/Notifications/TicketAssigneeChanged.php new file mode 100644 index 0000000..5bbfcec --- /dev/null +++ b/app/Actions/Notifications/TicketAssigneeChanged.php @@ -0,0 +1,26 @@ +payload; + } +} diff --git a/app/Actions/Notifications/TicketStatusChangedPayload.php b/app/Actions/Notifications/TicketStatusChangedPayload.php index cc6997b..2d1e9b3 100644 --- a/app/Actions/Notifications/TicketStatusChangedPayload.php +++ b/app/Actions/Notifications/TicketStatusChangedPayload.php @@ -17,11 +17,16 @@ final readonly class TicketStatusChangedPayload implements NotificationEventPayl public string $ticketUrl, public ?int $actorId, public ?string $actorName, - public int $fromStatusId, - public string $fromStatusLabel, + public ?int $fromStatusId, + public ?string $fromStatusLabel, public int $toStatusId, public string $toStatusLabel, public CarbonImmutable $occurredAt, + public ?int $fromAssigneeId = null, + public ?string $fromAssigneeName = null, + public ?int $toAssigneeId = null, + public ?string $toAssigneeName = null, + public NotificationEventType $eventType = NotificationEventType::TicketStatusChanged, ) {} public function projectId(): int @@ -50,9 +55,11 @@ final readonly class TicketStatusChangedPayload implements NotificationEventPayl '$ticket_key' => $this->ticketKey, '$ticket_title' => $this->ticketTitle, '$project_name' => $this->projectName, - '$status_from' => $this->fromStatusLabel, + '$status_from' => $this->fromStatusLabel ?? '', '$status_to' => $this->toStatusLabel, '$actor_name' => $this->actorName ?? '', + '$assignee_from' => $this->fromAssigneeName ?? '', + '$assignee_to' => $this->toAssigneeName ?? '', ]; } @@ -62,7 +69,11 @@ final readonly class TicketStatusChangedPayload implements NotificationEventPayl * project: array{id: int, key: string, name: string}, * ticket: array{id: int, key: string, title: string}, * actor: array{id: int, name: string}|null, - * from: array{id: int, label: string}, + * assignee: array{ + * from: array{id: int, name: string}|null, + * to: array{id: int, name: string}|null + * }, + * from: array{id: int, label: string}|null, * to: array{id: int, label: string}, * ticket_url: string, * occurred_at: string @@ -71,7 +82,7 @@ final readonly class TicketStatusChangedPayload implements NotificationEventPayl public function toArray(): array { return [ - 'event_type' => NotificationEventType::TicketStatusChanged->value, + 'event_type' => $this->eventType->value, 'project' => [ 'id' => $this->projectId, 'key' => $this->projectKey, @@ -88,10 +99,26 @@ final readonly class TicketStatusChangedPayload implements NotificationEventPayl 'id' => $this->actorId, 'name' => $this->actorName, ], - 'from' => [ - 'id' => $this->fromStatusId, - 'label' => $this->fromStatusLabel, + 'assignee' => [ + 'from' => $this->fromAssigneeId === null || $this->fromAssigneeName === null + ? null + : [ + 'id' => $this->fromAssigneeId, + 'name' => $this->fromAssigneeName, + ], + 'to' => $this->toAssigneeId === null || $this->toAssigneeName === null + ? null + : [ + 'id' => $this->toAssigneeId, + 'name' => $this->toAssigneeName, + ], ], + 'from' => $this->fromStatusId === null || $this->fromStatusLabel === null + ? null + : [ + 'id' => $this->fromStatusId, + 'label' => $this->fromStatusLabel, + ], 'to' => [ 'id' => $this->toStatusId, 'label' => $this->toStatusLabel, diff --git a/app/Actions/Tickets/UpdateProjectTicket.php b/app/Actions/Tickets/UpdateProjectTicket.php index 9b21690..f1e598a 100644 --- a/app/Actions/Tickets/UpdateProjectTicket.php +++ b/app/Actions/Tickets/UpdateProjectTicket.php @@ -2,12 +2,15 @@ namespace App\Actions\Tickets; +use App\Actions\Notifications\BuildTicketAssigneeChangedPayload; +use App\Actions\Notifications\TicketAssigneeChanged; use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData; use App\Enums\TicketWorklogEntryType; use App\Models\Ticket; use App\Models\TicketFieldDefinition; use App\Models\TicketFieldValue; use App\Models\User; +use Carbon\CarbonImmutable; use Illuminate\Support\Arr; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; @@ -15,6 +18,7 @@ use Illuminate\Support\Facades\DB; class UpdateProjectTicket { public function __construct( + private readonly BuildTicketAssigneeChangedPayload $buildTicketAssigneeChangedPayload, private readonly CreateTicketWorklogEntry $createTicketWorklogEntry, private readonly SyncProjectTicketFieldValues $syncProjectTicketFieldValues, ) {} @@ -67,6 +71,7 @@ class UpdateProjectTicket 'parentTicket', 'project', 'ticketPriority', + 'ticketStatus.boardColumn', 'ticketType', ]); @@ -96,6 +101,7 @@ class UpdateProjectTicket $changes = []; $summaryLabels = []; + $assigneeChange = null; $this->trackScalarChange( $changes, @@ -173,13 +179,23 @@ class UpdateProjectTicket } if ($ticket->assignee_user_id !== $ticketAttributes['assignee_user_id']) { + $toAssigneeId = $ticketAttributes['assignee_user_id']; + $toAssigneeName = $toAssigneeId === null + ? null + : User::query()->find($toAssigneeId)?->name; + $changes['assignee'] = [ 'from' => $ticket->assignee?->name, - 'to' => $ticketAttributes['assignee_user_id'] === null - ? null - : User::query()->find($ticketAttributes['assignee_user_id'])?->name, + 'to' => $toAssigneeName, ]; $summaryLabels[] = 'assignee'; + + $assigneeChange = [ + 'from_id' => $ticket->assignee_user_id, + 'from_name' => $ticket->assignee?->name, + 'to_id' => $toAssigneeId, + 'to_name' => $toAssigneeName, + ]; } if ($ticket->author_user_id !== $ticketAttributes['author_user_id']) { @@ -225,6 +241,21 @@ class UpdateProjectTicket $changes, ); + if ($assigneeChange !== null) { + TicketAssigneeChanged::dispatch( + $this->buildTicketAssigneeChangedPayload->handle( + $ticket->project, + $ticket, + $actor, + $assigneeChange['from_id'], + $assigneeChange['from_name'], + $assigneeChange['to_id'], + $assigneeChange['to_name'], + CarbonImmutable::now(), + ), + ); + } + return true; }); } diff --git a/app/Enums/NotificationEventType.php b/app/Enums/NotificationEventType.php index 629e2af..2da3dfa 100644 --- a/app/Enums/NotificationEventType.php +++ b/app/Enums/NotificationEventType.php @@ -6,12 +6,14 @@ enum NotificationEventType: string { case TicketStatusChanged = 'ticket_status_changed'; case TicketCreate = 'ticket_create'; + case TicketAssigneeChanged = 'ticket_assignee_changed'; public function label(): string { return match ($this) { self::TicketStatusChanged => 'Ticket status changed', self::TicketCreate => 'Ticket create', + self::TicketAssigneeChanged => 'Assignee changed', }; } diff --git a/app/Enums/NotificationFilterAttribute.php b/app/Enums/NotificationFilterAttribute.php index bcd9305..3eedd91 100644 --- a/app/Enums/NotificationFilterAttribute.php +++ b/app/Enums/NotificationFilterAttribute.php @@ -13,6 +13,10 @@ enum NotificationFilterAttribute: string case TicketUrl = 'ticket.url'; case ActorId = 'actor.id'; case ActorName = 'actor.name'; + case AssigneeFromId = 'assignee.from.id'; + case AssigneeFromName = 'assignee.from.name'; + case AssigneeToId = 'assignee.to.id'; + case AssigneeToName = 'assignee.to.name'; case StatusFromId = 'status.from.id'; case StatusFromLabel = 'status.from.label'; case StatusToId = 'status.to.id'; @@ -30,6 +34,10 @@ enum NotificationFilterAttribute: string self::TicketUrl => 'Ticket URL', self::ActorId => 'Actor ID', self::ActorName => 'Actor name', + self::AssigneeFromId => 'From assignee ID', + self::AssigneeFromName => 'From assignee name', + self::AssigneeToId => 'To assignee ID', + self::AssigneeToName => 'To assignee name', self::StatusFromId => 'From status ID', self::StatusFromLabel => 'From status label', self::StatusToId => 'To status ID', diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index c8685f8..e2489fa 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -3,6 +3,7 @@ namespace App\Providers; use App\Actions\Notifications\QueueNotificationEventDeliveries; +use App\Actions\Notifications\TicketAssigneeChanged; use App\Actions\Notifications\TicketCreated; use App\Actions\Notifications\TicketStatusChanged; use App\Actions\Permissions\PermissionService; @@ -105,5 +106,10 @@ class AppServiceProvider extends ServiceProvider TicketCreated::class, QueueNotificationEventDeliveries::class, ); + + Event::listen( + TicketAssigneeChanged::class, + QueueNotificationEventDeliveries::class, + ); } } diff --git a/tests/Feature/Notifications/DeliverNotificationActionTest.php b/tests/Feature/Notifications/DeliverNotificationActionTest.php index d693f5a..d60c295 100644 --- a/tests/Feature/Notifications/DeliverNotificationActionTest.php +++ b/tests/Feature/Notifications/DeliverNotificationActionTest.php @@ -558,6 +558,10 @@ class DeliverNotificationActionTest extends TestCase * project: array{id: int, key: string, name: string}, * ticket: array{id: int, key: string, title: string}, * actor: array{id: int, name: string}, + * assignee: array{ + * from: array{id: int, name: string}|null, + * to: array{id: int, name: string}|null + * }, * from: array{id: int, label: string}, * to: array{id: int, label: string}, * ticket_url: string, @@ -582,6 +586,10 @@ class DeliverNotificationActionTest extends TestCase 'id' => 7, 'name' => 'Erica Example', ], + 'assignee' => [ + 'from' => null, + 'to' => null, + ], 'from' => [ 'id' => 1, 'label' => 'Backlog', diff --git a/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php b/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php index 6f21c71..ae5616b 100644 --- a/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php +++ b/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php @@ -45,6 +45,14 @@ class RenderDiscordMessageTemplateTest extends TestCase 'template' => '$actor_name', 'expected' => 'Erica Example', ], + 'assignee_from' => [ + 'template' => '$assignee_from', + 'expected' => 'Alex Analyst', + ], + 'assignee_to' => [ + 'template' => '$assignee_to', + 'expected' => 'Priya Planner', + ], ]; } @@ -89,6 +97,10 @@ class RenderDiscordMessageTemplateTest extends TestCase toStatusId: 2, toStatusLabel: 'Doing', occurredAt: CarbonImmutable::parse('2026-04-24T09:15:30+00:00'), + fromAssigneeId: 11, + fromAssigneeName: 'Alex Analyst', + toAssigneeId: 12, + toAssigneeName: 'Priya Planner', )); } } diff --git a/tests/Feature/Notifications/TicketAssigneeChangedNotificationDispatchTest.php b/tests/Feature/Notifications/TicketAssigneeChangedNotificationDispatchTest.php new file mode 100644 index 0000000..b380a09 --- /dev/null +++ b/tests/Feature/Notifications/TicketAssigneeChangedNotificationDispatchTest.php @@ -0,0 +1,342 @@ +create([ + 'name' => 'Erica Example', + ]); + $fromAssignee = User::factory()->create([ + 'name' => 'Alex Analyst', + ]); + $toAssignee = User::factory()->create([ + 'name' => 'Priya Planner', + ]); + $project = Project::factory()->create([ + 'key' => 'CORE', + 'name' => 'Core Platform', + ]); + $otherProject = Project::factory()->create(); + + app(InitializeProjectTicketMetadata::class)->handle($project); + app(InitializeProjectTicketMetadata::class)->handle($otherProject); + + $backlogStatus = $this->backlogStatus($project); + $ticket = $this->createTicket( + $project, + $backlogStatus, + 'Fix webhook dispatch', + $fromAssignee, + ); + + $webhookAction = NotificationAction::factory() + ->webhook() + ->for($project) + ->create([ + 'name' => 'Primary webhook', + 'event_type' => NotificationEventType::TicketAssigneeChanged, + ]); + $discordAction = NotificationAction::factory() + ->discord() + ->for($project) + ->create([ + 'name' => 'Discord alerts', + 'event_type' => NotificationEventType::TicketAssigneeChanged, + ]); + $disabledAction = NotificationAction::factory() + ->disabled() + ->for($project) + ->create([ + 'name' => 'Disabled webhook', + 'event_type' => NotificationEventType::TicketAssigneeChanged, + ]); + $statusChangedAction = NotificationAction::factory() + ->for($project) + ->create([ + 'name' => 'Status changed webhook', + 'event_type' => NotificationEventType::TicketStatusChanged, + ]); + $filterEqualsAction = NotificationAction::factory() + ->webhook() + ->for($project) + ->create([ + 'name' => 'Filtered to assignee webhook', + 'event_type' => NotificationEventType::TicketAssigneeChanged, + 'filters' => [ + [ + 'attribute' => 'assignee.to.id', + 'operator' => 'equals', + 'value' => (string) $toAssignee->id, + ], + ], + ]); + $filterExistsAction = NotificationAction::factory() + ->webhook() + ->for($project) + ->create([ + 'name' => 'Filtered from assignee exists webhook', + 'event_type' => NotificationEventType::TicketAssigneeChanged, + 'filters' => [ + [ + 'attribute' => 'assignee.from.id', + 'operator' => 'exists', + 'value' => null, + ], + ], + ]); + $filterNotExistsAction = NotificationAction::factory() + ->webhook() + ->for($project) + ->create([ + 'name' => 'Filtered from assignee not exists webhook', + 'event_type' => NotificationEventType::TicketAssigneeChanged, + 'filters' => [ + [ + 'attribute' => 'assignee.from.id', + 'operator' => 'not_exists', + 'value' => null, + ], + ], + ]); + $filterInAction = NotificationAction::factory() + ->webhook() + ->for($project) + ->create([ + 'name' => 'Filtered in assignee webhook', + 'event_type' => NotificationEventType::TicketAssigneeChanged, + 'filters' => [ + [ + 'attribute' => 'assignee.to.name', + 'operator' => 'in', + 'value' => ['Priya Planner', 'Someone Else'], + ], + ], + ]); + $foreignProjectAction = NotificationAction::factory() + ->for($otherProject) + ->create([ + 'name' => 'Foreign webhook', + 'event_type' => NotificationEventType::TicketAssigneeChanged, + ]); + + $this->syncProjectPermissions($project, $actor, ['edit', 'view']); + + $this->actingAs($actor) + ->patch(route('projects.tickets.update', [$project, $ticket]), $this->updatePayload($ticket, [ + 'assignee_user_id' => $toAssignee->id, + ])) + ->assertRedirect(route('projects.tickets.show', [$project, $ticket])) + ->assertSessionHas('status', 'CORE-1 was updated.'); + + Queue::assertPushed(DeliverNotificationAction::class, 5); + Queue::assertPushed(DeliverNotificationAction::class, function (DeliverNotificationAction $job) use ($actor, $backlogStatus, $fromAssignee, $project, $ticket, $toAssignee, $webhookAction): bool { + $payload = $job->event->payload(); + + return $job->notificationActionId === $webhookAction->id + && $payload->projectId === $project->id + && $payload->projectKey === 'CORE' + && $payload->projectName === 'Core Platform' + && $payload->ticketId === $ticket->id + && $payload->ticketKey === 'CORE-1' + && $payload->ticketTitle === 'Fix webhook dispatch' + && $payload->ticketUrl === route('projects.tickets.show', [$project, $ticket]) + && $payload->actorId === $actor->id + && $payload->actorName === 'Erica Example' + && $payload->fromStatusId === null + && $payload->fromStatusLabel === null + && $payload->toStatusId === $backlogStatus->id + && $payload->toStatusLabel === 'Backlog' + && $payload->fromAssigneeId === $fromAssignee->id + && $payload->fromAssigneeName === 'Alex Analyst' + && $payload->toAssigneeId === $toAssignee->id + && $payload->toAssigneeName === 'Priya Planner' + && $payload->eventType === NotificationEventType::TicketAssigneeChanged + && $job->notificationDeliveryLogId !== null; + }); + Queue::assertPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $discordAction->id + && $job->event->eventType() === NotificationEventType::TicketAssigneeChanged + && $job->notificationDeliveryLogId !== null); + Queue::assertPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $filterEqualsAction->id + && $job->event->eventType() === NotificationEventType::TicketAssigneeChanged + && $job->notificationDeliveryLogId !== null); + Queue::assertPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $filterExistsAction->id + && $job->event->eventType() === NotificationEventType::TicketAssigneeChanged + && $job->notificationDeliveryLogId !== null); + Queue::assertPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $filterInAction->id + && $job->event->eventType() === NotificationEventType::TicketAssigneeChanged + && $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 === $statusChangedAction->id); + Queue::assertNotPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $filterNotExistsAction->id); + Queue::assertNotPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $foreignProjectAction->id); + + $this->assertDatabaseCount('notification_delivery_logs', 5); + $this->assertDatabaseHas('notification_delivery_logs', [ + 'notification_action_id' => $webhookAction->id, + 'delivery_status' => NotificationDeliveryStatus::Pending->value, + 'event_type' => NotificationEventType::TicketAssigneeChanged->value, + 'attempt_count' => 0, + 'http_status' => null, + ]); + $this->assertDatabaseHas('notification_delivery_logs', [ + 'notification_action_id' => $discordAction->id, + 'delivery_status' => NotificationDeliveryStatus::Pending->value, + 'event_type' => NotificationEventType::TicketAssigneeChanged->value, + 'attempt_count' => 0, + 'http_status' => null, + ]); + $this->assertDatabaseHas('notification_delivery_logs', [ + 'notification_action_id' => $filterEqualsAction->id, + 'delivery_status' => NotificationDeliveryStatus::Pending->value, + 'event_type' => NotificationEventType::TicketAssigneeChanged->value, + 'attempt_count' => 0, + 'http_status' => null, + ]); + $this->assertDatabaseHas('notification_delivery_logs', [ + 'notification_action_id' => $filterExistsAction->id, + 'delivery_status' => NotificationDeliveryStatus::Pending->value, + 'event_type' => NotificationEventType::TicketAssigneeChanged->value, + 'attempt_count' => 0, + 'http_status' => null, + ]); + $this->assertDatabaseHas('notification_delivery_logs', [ + 'notification_action_id' => $filterInAction->id, + 'delivery_status' => NotificationDeliveryStatus::Pending->value, + 'event_type' => NotificationEventType::TicketAssigneeChanged->value, + 'attempt_count' => 0, + 'http_status' => null, + ]); + + $this->assertDatabaseHas('ticket_worklog_entries', [ + 'ticket_id' => $ticket->id, + 'actor_id' => $actor->id, + 'entry_type' => 'updated', + 'summary' => 'Updated assignee.', + ]); + } + + /** + * Ensure no-op assignee updates keep their response behavior and do not queue deliveries. + */ + public function test_no_op_assignee_updates_do_not_queue_notification_deliveries(): void + { + Queue::fake(); + + $actor = User::factory()->create(); + $assignee = User::factory()->create(); + $project = Project::factory()->create([ + 'key' => 'CORE', + ]); + + app(InitializeProjectTicketMetadata::class)->handle($project); + + $backlogStatus = $this->backlogStatus($project); + $ticket = $this->createTicket( + $project, + $backlogStatus, + 'Keep the same assignee', + $assignee, + ); + + NotificationAction::factory() + ->for($project) + ->create([ + 'name' => 'Assignee webhook', + 'event_type' => NotificationEventType::TicketAssigneeChanged, + ]); + + $this->syncProjectPermissions($project, $actor, ['edit', 'view']); + + $this->actingAs($actor) + ->patch(route('projects.tickets.update', [$project, $ticket]), $this->updatePayload($ticket, [ + 'assignee_user_id' => $assignee->id, + ])) + ->assertRedirect(route('projects.tickets.show', [$project, $ticket])) + ->assertSessionHas('status', 'No ticket changes were detected.'); + + Queue::assertNothingPushed(); + $this->assertDatabaseCount('notification_delivery_logs', 0); + $this->assertDatabaseCount('ticket_worklog_entries', 0); + } + + private function backlogStatus(Project $project): TicketStatus + { + return $project->ticketStatuses() + ->whereNull('board_column_id') + ->firstOrFail(); + } + + private function createTicket( + Project $project, + TicketStatus $ticketStatus, + string $title, + ?User $assignee = null, + ): Ticket { + $priority = TicketPriority::factory()->for($project)->create([ + 'name' => 'High', + 'position' => 2, + ]); + + return Ticket::factory()->create([ + 'assignee_user_id' => $assignee?->id, + 'project_id' => $project->id, + 'sequence_number' => 1, + 'ticket_key' => 'CORE-1', + 'ticket_priority_id' => $priority->id, + 'ticket_status_id' => $ticketStatus->id, + 'ticket_type_id' => $project->ticketTypes()->firstOrFail()->id, + 'title' => $title, + ]); + } + + /** + * @param array $overrides + * @return array + */ + private function updatePayload(Ticket $ticket, array $overrides = []): array + { + return [ + 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromStored( + $ticket->acceptance_criteria_data, + $ticket->acceptance_criteria, + ), + 'assignee_user_id' => $ticket->assignee_user_id, + 'author_user_id' => $ticket->author_user_id, + 'blocked_comment' => $ticket->blocked_comment, + 'description' => $ticket->description, + 'field_values' => [], + 'is_blocked' => $ticket->is_blocked, + 'parent_ticket_id' => $ticket->parent_ticket_id, + 'story_points' => $ticket->story_points, + 'ticket_priority_id' => $ticket->ticket_priority_id, + 'ticket_type_id' => $ticket->ticket_type_id, + 'title' => $ticket->title, + ...$overrides, + ]; + } +} diff --git a/tests/Feature/Projects/ProjectNotificationSettingsControllerTest.php b/tests/Feature/Projects/ProjectNotificationSettingsControllerTest.php index d92e5e3..5d2c362 100644 --- a/tests/Feature/Projects/ProjectNotificationSettingsControllerTest.php +++ b/tests/Feature/Projects/ProjectNotificationSettingsControllerTest.php @@ -63,12 +63,15 @@ class ProjectNotificationSettingsControllerTest extends TestCase ->where('channelOptions.0.value', NotificationChannel::Discord->value) ->where('eventTypeOptions.0.value', NotificationEventType::TicketStatusChanged->value) ->where('eventTypeOptions.1.value', NotificationEventType::TicketCreate->value) + ->where('eventTypeOptions.2.value', NotificationEventType::TicketAssigneeChanged->value) ->where('discordAllowedMentionOptions.0.value', 'everyone') ->where('discordAllowedMentionOptions.2.value', 'users') ->where('filterAttributeOptions.0.value', 'project.id') ->where('filterOperatorOptions.0.value', 'equals') ->where('templateVariables.0.value', '$url') ->where('templateVariables.6.value', '$actor_name') + ->where('templateVariables.7.value', '$assignee_from') + ->where('templateVariables.8.value', '$assignee_to') ->missing('notificationActions.0.delivery_url'), ); } From eb58853ee00f2ce4cab48beddfa37707f40a9e4e Mon Sep 17 00:00:00 2001 From: Koda Date: Wed, 20 May 2026 14:12:09 +0000 Subject: [PATCH 2/2] Use dedicated assignee notification payload --- .../BuildTicketAssigneeChangedPayload.php | 6 +- .../Notifications/TicketAssigneeChanged.php | 2 +- .../TicketAssigneeChangedPayload.php | 129 ++++++++++++++++++ .../TicketStatusChangedPayload.php | 43 ++---- .../DeliverNotificationActionTest.php | 8 -- .../RenderDiscordMessageTemplateTest.php | 8 +- ...ssigneeChangedNotificationDispatchTest.php | 4 +- 7 files changed, 147 insertions(+), 53 deletions(-) create mode 100644 app/Actions/Notifications/TicketAssigneeChangedPayload.php diff --git a/app/Actions/Notifications/BuildTicketAssigneeChangedPayload.php b/app/Actions/Notifications/BuildTicketAssigneeChangedPayload.php index 6302327..75e8e3e 100644 --- a/app/Actions/Notifications/BuildTicketAssigneeChangedPayload.php +++ b/app/Actions/Notifications/BuildTicketAssigneeChangedPayload.php @@ -2,7 +2,6 @@ namespace App\Actions\Notifications; -use App\Enums\NotificationEventType; use App\Models\Project; use App\Models\Ticket; use App\Models\TicketStatus; @@ -20,11 +19,10 @@ class BuildTicketAssigneeChangedPayload ?int $toAssigneeId, ?string $toAssigneeName, CarbonImmutable $occurredAt, - ): TicketStatusChangedPayload { + ): TicketAssigneeChangedPayload { $ticket->loadMissing('ticketStatus.boardColumn'); - return new TicketStatusChangedPayload( - eventType: NotificationEventType::TicketAssigneeChanged, + return new TicketAssigneeChangedPayload( projectId: $project->id, projectKey: $project->key, projectName: $project->name, diff --git a/app/Actions/Notifications/TicketAssigneeChanged.php b/app/Actions/Notifications/TicketAssigneeChanged.php index 5bbfcec..d49643e 100644 --- a/app/Actions/Notifications/TicketAssigneeChanged.php +++ b/app/Actions/Notifications/TicketAssigneeChanged.php @@ -11,7 +11,7 @@ final readonly class TicketAssigneeChanged implements NotificationEvent, ShouldD use Dispatchable; public function __construct( - public TicketStatusChangedPayload $payload, + public TicketAssigneeChangedPayload $payload, ) {} public function eventType(): NotificationEventType diff --git a/app/Actions/Notifications/TicketAssigneeChangedPayload.php b/app/Actions/Notifications/TicketAssigneeChangedPayload.php new file mode 100644 index 0000000..0eceb6b --- /dev/null +++ b/app/Actions/Notifications/TicketAssigneeChangedPayload.php @@ -0,0 +1,129 @@ +projectId; + } + + /** + * @return array + */ + public function deliveryMetadata(): array + { + return [ + 'project_key' => $this->projectKey, + 'ticket_key' => $this->ticketKey, + ]; + } + + /** + * @return array + */ + public function discordTemplateVariables(): array + { + return [ + '$url' => $this->ticketUrl, + '$ticket_key' => $this->ticketKey, + '$ticket_title' => $this->ticketTitle, + '$project_name' => $this->projectName, + '$status_from' => $this->fromStatusLabel ?? '', + '$status_to' => $this->toStatusLabel, + '$actor_name' => $this->actorName ?? '', + '$assignee_from' => $this->fromAssigneeName ?? '', + '$assignee_to' => $this->toAssigneeName ?? '', + ]; + } + + /** + * @return array{ + * event_type: string, + * project: array{id: int, key: string, name: string}, + * ticket: array{id: int, key: string, title: string}, + * actor: array{id: int, name: string}|null, + * assignee: array{ + * from: array{id: int, name: string}|null, + * to: array{id: int, name: string}|null + * }, + * from: array{id: int, label: string}|null, + * to: array{id: int, label: string}, + * ticket_url: string, + * occurred_at: string + * } + */ + public function toArray(): array + { + return [ + 'event_type' => NotificationEventType::TicketAssigneeChanged->value, + 'project' => [ + 'id' => $this->projectId, + 'key' => $this->projectKey, + 'name' => $this->projectName, + ], + 'ticket' => [ + 'id' => $this->ticketId, + 'key' => $this->ticketKey, + 'title' => $this->ticketTitle, + ], + 'actor' => $this->actorId === null || $this->actorName === null + ? null + : [ + 'id' => $this->actorId, + 'name' => $this->actorName, + ], + 'assignee' => [ + 'from' => $this->fromAssigneeId === null || $this->fromAssigneeName === null + ? null + : [ + 'id' => $this->fromAssigneeId, + 'name' => $this->fromAssigneeName, + ], + 'to' => $this->toAssigneeId === null || $this->toAssigneeName === null + ? null + : [ + 'id' => $this->toAssigneeId, + 'name' => $this->toAssigneeName, + ], + ], + 'from' => $this->fromStatusId === null || $this->fromStatusLabel === null + ? null + : [ + 'id' => $this->fromStatusId, + 'label' => $this->fromStatusLabel, + ], + 'to' => [ + 'id' => $this->toStatusId, + 'label' => $this->toStatusLabel, + ], + 'ticket_url' => $this->ticketUrl, + 'occurred_at' => $this->occurredAt->toISOString(), + ]; + } +} diff --git a/app/Actions/Notifications/TicketStatusChangedPayload.php b/app/Actions/Notifications/TicketStatusChangedPayload.php index 2d1e9b3..cc6997b 100644 --- a/app/Actions/Notifications/TicketStatusChangedPayload.php +++ b/app/Actions/Notifications/TicketStatusChangedPayload.php @@ -17,16 +17,11 @@ final readonly class TicketStatusChangedPayload implements NotificationEventPayl public string $ticketUrl, public ?int $actorId, public ?string $actorName, - public ?int $fromStatusId, - public ?string $fromStatusLabel, + public int $fromStatusId, + public string $fromStatusLabel, public int $toStatusId, public string $toStatusLabel, public CarbonImmutable $occurredAt, - public ?int $fromAssigneeId = null, - public ?string $fromAssigneeName = null, - public ?int $toAssigneeId = null, - public ?string $toAssigneeName = null, - public NotificationEventType $eventType = NotificationEventType::TicketStatusChanged, ) {} public function projectId(): int @@ -55,11 +50,9 @@ final readonly class TicketStatusChangedPayload implements NotificationEventPayl '$ticket_key' => $this->ticketKey, '$ticket_title' => $this->ticketTitle, '$project_name' => $this->projectName, - '$status_from' => $this->fromStatusLabel ?? '', + '$status_from' => $this->fromStatusLabel, '$status_to' => $this->toStatusLabel, '$actor_name' => $this->actorName ?? '', - '$assignee_from' => $this->fromAssigneeName ?? '', - '$assignee_to' => $this->toAssigneeName ?? '', ]; } @@ -69,11 +62,7 @@ final readonly class TicketStatusChangedPayload implements NotificationEventPayl * project: array{id: int, key: string, name: string}, * ticket: array{id: int, key: string, title: string}, * actor: array{id: int, name: string}|null, - * assignee: array{ - * from: array{id: int, name: string}|null, - * to: array{id: int, name: string}|null - * }, - * from: array{id: int, label: string}|null, + * from: array{id: int, label: string}, * to: array{id: int, label: string}, * ticket_url: string, * occurred_at: string @@ -82,7 +71,7 @@ final readonly class TicketStatusChangedPayload implements NotificationEventPayl public function toArray(): array { return [ - 'event_type' => $this->eventType->value, + 'event_type' => NotificationEventType::TicketStatusChanged->value, 'project' => [ 'id' => $this->projectId, 'key' => $this->projectKey, @@ -99,26 +88,10 @@ final readonly class TicketStatusChangedPayload implements NotificationEventPayl 'id' => $this->actorId, 'name' => $this->actorName, ], - 'assignee' => [ - 'from' => $this->fromAssigneeId === null || $this->fromAssigneeName === null - ? null - : [ - 'id' => $this->fromAssigneeId, - 'name' => $this->fromAssigneeName, - ], - 'to' => $this->toAssigneeId === null || $this->toAssigneeName === null - ? null - : [ - 'id' => $this->toAssigneeId, - 'name' => $this->toAssigneeName, - ], + 'from' => [ + 'id' => $this->fromStatusId, + 'label' => $this->fromStatusLabel, ], - 'from' => $this->fromStatusId === null || $this->fromStatusLabel === null - ? null - : [ - 'id' => $this->fromStatusId, - 'label' => $this->fromStatusLabel, - ], 'to' => [ 'id' => $this->toStatusId, 'label' => $this->toStatusLabel, diff --git a/tests/Feature/Notifications/DeliverNotificationActionTest.php b/tests/Feature/Notifications/DeliverNotificationActionTest.php index d60c295..d693f5a 100644 --- a/tests/Feature/Notifications/DeliverNotificationActionTest.php +++ b/tests/Feature/Notifications/DeliverNotificationActionTest.php @@ -558,10 +558,6 @@ class DeliverNotificationActionTest extends TestCase * project: array{id: int, key: string, name: string}, * ticket: array{id: int, key: string, title: string}, * actor: array{id: int, name: string}, - * assignee: array{ - * from: array{id: int, name: string}|null, - * to: array{id: int, name: string}|null - * }, * from: array{id: int, label: string}, * to: array{id: int, label: string}, * ticket_url: string, @@ -586,10 +582,6 @@ class DeliverNotificationActionTest extends TestCase 'id' => 7, 'name' => 'Erica Example', ], - 'assignee' => [ - 'from' => null, - 'to' => null, - ], 'from' => [ 'id' => 1, 'label' => 'Backlog', diff --git a/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php b/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php index ae5616b..2ff69b9 100644 --- a/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php +++ b/tests/Feature/Notifications/RenderDiscordMessageTemplateTest.php @@ -3,8 +3,8 @@ namespace Tests\Feature\Notifications; use App\Actions\Notifications\RenderDiscordMessageTemplate; -use App\Actions\Notifications\TicketStatusChanged; -use App\Actions\Notifications\TicketStatusChangedPayload; +use App\Actions\Notifications\TicketAssigneeChanged; +use App\Actions\Notifications\TicketAssigneeChangedPayload; use Carbon\CarbonImmutable; use PHPUnit\Framework\Attributes\DataProvider; use Tests\TestCase; @@ -80,9 +80,9 @@ class RenderDiscordMessageTemplateTest extends TestCase $this->assertSame('[CORE-42] $unknown moved to Doing', $renderedMessage); } - private function event(): TicketStatusChanged + private function event(): TicketAssigneeChanged { - return new TicketStatusChanged(new TicketStatusChangedPayload( + return new TicketAssigneeChanged(new TicketAssigneeChangedPayload( projectId: 10, projectKey: 'CORE', projectName: 'Core Platform', diff --git a/tests/Feature/Notifications/TicketAssigneeChangedNotificationDispatchTest.php b/tests/Feature/Notifications/TicketAssigneeChangedNotificationDispatchTest.php index ea6136b..ec8ef39 100644 --- a/tests/Feature/Notifications/TicketAssigneeChangedNotificationDispatchTest.php +++ b/tests/Feature/Notifications/TicketAssigneeChangedNotificationDispatchTest.php @@ -3,6 +3,7 @@ namespace Tests\Feature\Notifications; use App\Actions\Notifications\DeliverNotificationAction; +use App\Actions\Notifications\TicketAssigneeChangedPayload; use App\Actions\ProjectTicketMetadata\InitializeProjectTicketMetadata; use App\Actions\Tickets\Support\AcceptanceCriteriaChecklistMapper; use App\Enums\NotificationDeliveryStatus; @@ -158,6 +159,7 @@ class TicketAssigneeChangedNotificationDispatchTest extends TestCase $payload = $job->event->payload(); return $job->notificationActionId === $webhookAction->id + && $payload instanceof TicketAssigneeChangedPayload && $payload->projectId === $project->id && $payload->projectKey === 'CORE' && $payload->projectName === 'Core Platform' @@ -175,7 +177,7 @@ class TicketAssigneeChangedNotificationDispatchTest extends TestCase && $payload->fromAssigneeName === 'Alex Analyst' && $payload->toAssigneeId === $toAssignee->id && $payload->toAssigneeName === 'Priya Planner' - && $payload->eventType === NotificationEventType::TicketAssigneeChanged + && $payload->toArray()['event_type'] === NotificationEventType::TicketAssigneeChanged->value && $job->notificationDeliveryLogId !== null; }); Queue::assertPushed(DeliverNotificationAction::class, fn (DeliverNotificationAction $job): bool => $job->notificationActionId === $discordAction->id