From 7805bd057eadfb50875962602d797ff927029c46 Mon Sep 17 00:00:00 2001 From: Koda Date: Tue, 19 May 2026 22:20:15 +0000 Subject: [PATCH 1/6] Refactor acceptance criteria into model --- app/Actions/Tickets/CreateProjectTicket.php | 3 +- .../Support/TicketAcceptanceCriteriaData.php | 97 +++++++++++++++---- app/Actions/Tickets/UpdateProjectTicket.php | 27 +++--- app/Http/Resources/TicketResource.php | 6 +- app/Mcp/Support/KanbanGitMcp.php | 6 +- app/Models/AcceptanceCriterion.php | 45 +++++++++ app/Models/Ticket.php | 9 +- ...eptance_criteria_data_to_tickets_table.php | 28 ------ ...03500_create_acceptance_criteria_table.php | 39 ++++++++ .../Projects/ProjectTicketControllerTest.php | 29 +++++- 10 files changed, 217 insertions(+), 72 deletions(-) create mode 100644 app/Models/AcceptanceCriterion.php delete mode 100644 database/migrations/2026_04_24_203500_add_acceptance_criteria_data_to_tickets_table.php create mode 100644 database/migrations/2026_04_24_203500_create_acceptance_criteria_table.php diff --git a/app/Actions/Tickets/CreateProjectTicket.php b/app/Actions/Tickets/CreateProjectTicket.php index 2f8c551..2598012 100644 --- a/app/Actions/Tickets/CreateProjectTicket.php +++ b/app/Actions/Tickets/CreateProjectTicket.php @@ -55,7 +55,6 @@ class CreateProjectTicket $ticket = Ticket::query()->create([ 'acceptance_criteria' => TicketAcceptanceCriteriaData::toLegacyText($acceptanceCriteria), - 'acceptance_criteria_data' => TicketAcceptanceCriteriaData::forStorage($acceptanceCriteria), 'assignee_user_id' => Arr::get($attributes, 'assignee_user_id'), 'author_user_id' => $actor?->id, 'description' => Arr::get($attributes, 'description'), @@ -69,6 +68,8 @@ class CreateProjectTicket 'story_points' => Arr::get($attributes, 'story_points'), ]); + TicketAcceptanceCriteriaData::syncTicket($ticket, $acceptanceCriteria); + $occurredAt = CarbonImmutable::now(); if ($actor !== null) { diff --git a/app/Actions/Tickets/Support/TicketAcceptanceCriteriaData.php b/app/Actions/Tickets/Support/TicketAcceptanceCriteriaData.php index 9a066f3..3378505 100644 --- a/app/Actions/Tickets/Support/TicketAcceptanceCriteriaData.php +++ b/app/Actions/Tickets/Support/TicketAcceptanceCriteriaData.php @@ -2,6 +2,9 @@ namespace App\Actions\Tickets\Support; +use App\Models\AcceptanceCriterion; +use App\Models\Ticket; +use Illuminate\Database\Eloquent\Collection as EloquentCollection; use Illuminate\Support\Arr; class TicketAcceptanceCriteriaData @@ -9,6 +12,7 @@ class TicketAcceptanceCriteriaData public const MAX_INDENT_LEVEL = 8; /** + * @param iterable $criteria * @return array{ * summary: string|null, * items: array * } */ - public static function fromStored(mixed $storedData, ?string $legacyText): array + public static function fromModels(iterable $criteria, ?string $legacyText): array { - $normalizedStoredData = is_array($storedData) - ? self::normalize($storedData) - : self::empty(); + $summary = self::normalize($legacyText)['summary']; + $models = ($criteria instanceof EloquentCollection ? $criteria : collect($criteria)) + ->sort(function (AcceptanceCriterion $left, AcceptanceCriterion $right): int { + return [ + $left->parent_acceptance_criterion_id ?? 0, + $left->position, + $left->id, + ] <=> [ + $right->parent_acceptance_criterion_id ?? 0, + $right->position, + $right->id, + ]; + }) + ->values(); - if (self::hasContent($normalizedStoredData)) { - return $normalizedStoredData; + if ($models->isEmpty()) { + return self::normalize($legacyText); } - return self::normalize($legacyText); + /** @var array> $grouped */ + $grouped = $models->groupBy('parent_acceptance_criterion_id')->all(); + $items = []; + + self::flattenItems($items, $grouped, null, 0); + + return [ + 'summary' => $summary, + 'items' => $items, + ]; } /** @@ -71,6 +95,7 @@ class TicketAcceptanceCriteriaData } /** + * @param Ticket $ticket * @param array{ * summary: string|null, * items: array * } $acceptanceCriteria - * @return array{ - * summary: string|null, - * items: array - * }|null */ - public static function forStorage(array $acceptanceCriteria): ?array + public static function syncTicket(Ticket $ticket, array $acceptanceCriteria): void { $normalized = self::normalize($acceptanceCriteria); - return self::hasContent($normalized) - ? $normalized - : null; + $ticket->acceptanceCriteria()->delete(); + + if ($normalized['items'] === []) { + return; + } + + $parentStack = []; + $positionsByParent = []; + + foreach ($normalized['items'] as $item) { + $indent = min($item['indent'], count($parentStack)); + $parentId = $indent === 0 ? null : $parentStack[$indent - 1]; + $positionKey = $parentId ?? 'root'; + $position = $positionsByParent[$positionKey] ?? 0; + + $criterion = $ticket->acceptanceCriteria()->create([ + 'parent_acceptance_criterion_id' => $parentId, + 'position' => $position, + 'text' => $item['text'], + 'is_checked' => $item['checked'], + ]); + + $positionsByParent[$positionKey] = $position + 1; + $parentStack[$indent] = $criterion->id; + $parentStack = array_slice($parentStack, 0, $indent + 1); + } + + $ticket->unsetRelation('acceptanceCriteria'); } /** @@ -154,6 +196,23 @@ class TicketAcceptanceCriteriaData ]; } + /** + * @param array $items + * @param array> $grouped + */ + private static function flattenItems(array &$items, array $grouped, ?int $parentId, int $indent): void + { + foreach ($grouped[$parentId] ?? [] as $criterion) { + $items[] = [ + 'checked' => $criterion->is_checked, + 'indent' => $indent, + 'text' => $criterion->text, + ]; + + self::flattenItems($items, $grouped, $criterion->id, $indent + 1); + } + } + /** * @param array $value * @return array{ diff --git a/app/Actions/Tickets/UpdateProjectTicket.php b/app/Actions/Tickets/UpdateProjectTicket.php index 9b21690..cf83e3c 100644 --- a/app/Actions/Tickets/UpdateProjectTicket.php +++ b/app/Actions/Tickets/UpdateProjectTicket.php @@ -52,9 +52,12 @@ class UpdateProjectTicket array $validated, ): bool { $fieldValueAttributes = Arr::get($validated, 'field_values', []); + $nextAcceptanceCriteria = TicketAcceptanceCriteriaData::normalize( + Arr::get($validated, 'acceptance_criteria'), + ); $ticketAttributes = $this->buildTicketAttributes($ticket, $validated); - return DB::transaction(function () use ($actor, $fieldValueAttributes, $ticket, $ticketAttributes): bool { + return DB::transaction(function () use ($actor, $fieldValueAttributes, $nextAcceptanceCriteria, $ticket, $ticketAttributes): bool { if (! $ticketAttributes['is_blocked']) { $ticketAttributes['blocked_comment'] = null; } @@ -62,6 +65,7 @@ class UpdateProjectTicket $ticket->loadMissing([ 'assignee', 'author', + 'acceptanceCriteria', 'fieldValues.ticketFieldDefinition', 'fieldValues.userValue', 'parentTicket', @@ -85,14 +89,10 @@ class UpdateProjectTicket /** @var Collection $existingFieldValues */ $existingFieldValues = $ticket->fieldValues ->keyBy('ticket_field_definition_id'); - $currentAcceptanceCriteria = TicketAcceptanceCriteriaData::fromStored( - $ticket->acceptance_criteria_data, + $currentAcceptanceCriteria = TicketAcceptanceCriteriaData::fromModels( + $ticket->acceptanceCriteria, $ticket->acceptance_criteria, ); - $nextAcceptanceCriteria = TicketAcceptanceCriteriaData::fromStored( - $ticketAttributes['acceptance_criteria_data'], - $ticketAttributes['acceptance_criteria'], - ); $changes = []; $summaryLabels = []; @@ -198,6 +198,10 @@ class UpdateProjectTicket $ticket->save(); } + if ($currentAcceptanceCriteria !== $nextAcceptanceCriteria) { + TicketAcceptanceCriteriaData::syncTicket($ticket, $nextAcceptanceCriteria); + } + $fieldValueChangeSet = $this->syncProjectTicketFieldValues->handle( $ticket, $fieldDefinitions, @@ -252,14 +256,6 @@ class UpdateProjectTicket * } $validated * @return array{ * acceptance_criteria: string|null, - * acceptance_criteria_data: array{ - * summary: string|null, - * items: array - * }|null, * assignee_user_id: int|null, * author_user_id: int|null, * blocked_comment: string|null, @@ -280,7 +276,6 @@ class UpdateProjectTicket return [ 'acceptance_criteria' => TicketAcceptanceCriteriaData::toLegacyText($acceptanceCriteria), - 'acceptance_criteria_data' => TicketAcceptanceCriteriaData::forStorage($acceptanceCriteria), 'assignee_user_id' => Arr::exists($validated, 'assignee_user_id') ? Arr::get($validated, 'assignee_user_id') : $ticket->assignee_user_id, diff --git a/app/Http/Resources/TicketResource.php b/app/Http/Resources/TicketResource.php index e4dee66..1b38660 100644 --- a/app/Http/Resources/TicketResource.php +++ b/app/Http/Resources/TicketResource.php @@ -17,8 +17,10 @@ class TicketResource extends JsonResource { return [ 'id' => $this->id, - 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromStored( - $this->acceptance_criteria_data, + 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromModels( + $this->relationLoaded('acceptanceCriteria') + ? $this->acceptanceCriteria + : $this->acceptanceCriteria()->get(), $this->acceptance_criteria, ), 'acceptance_criteria_text' => $this->acceptance_criteria, diff --git a/app/Mcp/Support/KanbanGitMcp.php b/app/Mcp/Support/KanbanGitMcp.php index 6f0acec..85444a4 100644 --- a/app/Mcp/Support/KanbanGitMcp.php +++ b/app/Mcp/Support/KanbanGitMcp.php @@ -227,8 +227,10 @@ class KanbanGitMcp public function updatePayload(Ticket $ticket): array { return [ - 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromStored( - $ticket->acceptance_criteria_data, + 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromModels( + $ticket->relationLoaded('acceptanceCriteria') + ? $ticket->acceptanceCriteria + : $ticket->acceptanceCriteria()->get(), $ticket->acceptance_criteria, ), 'assignee_user_id' => $ticket->assignee_user_id, diff --git a/app/Models/AcceptanceCriterion.php b/app/Models/AcceptanceCriterion.php new file mode 100644 index 0000000..e1b112d --- /dev/null +++ b/app/Models/AcceptanceCriterion.php @@ -0,0 +1,45 @@ + + */ + protected function casts(): array + { + return [ + 'is_checked' => 'boolean', + ]; + } + + public function children(): HasMany + { + return $this->hasMany(self::class, 'parent_acceptance_criterion_id') + ->orderBy('position') + ->orderBy('id'); + } + + public function parent(): BelongsTo + { + return $this->belongsTo(self::class, 'parent_acceptance_criterion_id'); + } + + public function ticket(): BelongsTo + { + return $this->belongsTo(Ticket::class); + } +} diff --git a/app/Models/Ticket.php b/app/Models/Ticket.php index 5936e56..d82a367 100644 --- a/app/Models/Ticket.php +++ b/app/Models/Ticket.php @@ -11,7 +11,6 @@ use Illuminate\Database\Eloquent\Relations\HasMany; #[Fillable([ 'acceptance_criteria', - 'acceptance_criteria_data', 'assignee_user_id', 'author_user_id', 'blocked_comment', @@ -38,11 +37,17 @@ class Ticket extends Model protected function casts(): array { return [ - 'acceptance_criteria_data' => 'array', 'is_blocked' => 'boolean', ]; } + public function acceptanceCriteria(): HasMany + { + return $this->hasMany(AcceptanceCriterion::class) + ->orderBy('position') + ->orderBy('id'); + } + public function assignee(): BelongsTo { return $this->belongsTo(User::class, 'assignee_user_id'); diff --git a/database/migrations/2026_04_24_203500_add_acceptance_criteria_data_to_tickets_table.php b/database/migrations/2026_04_24_203500_add_acceptance_criteria_data_to_tickets_table.php deleted file mode 100644 index d74bd70..0000000 --- a/database/migrations/2026_04_24_203500_add_acceptance_criteria_data_to_tickets_table.php +++ /dev/null @@ -1,28 +0,0 @@ -json('acceptance_criteria_data')->nullable(); - }); - } - - /** - * Reverse the migrations. - */ - public function down(): void - { - Schema::table('tickets', function (Blueprint $table): void { - $table->dropColumn('acceptance_criteria_data'); - }); - } -}; diff --git a/database/migrations/2026_04_24_203500_create_acceptance_criteria_table.php b/database/migrations/2026_04_24_203500_create_acceptance_criteria_table.php new file mode 100644 index 0000000..d073fdc --- /dev/null +++ b/database/migrations/2026_04_24_203500_create_acceptance_criteria_table.php @@ -0,0 +1,39 @@ +id(); + $table->foreignId('ticket_id') + ->constrained() + ->cascadeOnDelete(); + $table->foreignId('parent_acceptance_criterion_id') + ->nullable() + ->constrained('acceptance_criteria') + ->cascadeOnDelete(); + $table->unsignedInteger('position')->default(0); + $table->text('text'); + $table->boolean('is_checked')->default(false); + $table->timestamps(); + + $table->index(['ticket_id', 'parent_acceptance_criterion_id', 'position']); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::dropIfExists('acceptance_criteria'); + } +}; diff --git a/tests/Feature/Projects/ProjectTicketControllerTest.php b/tests/Feature/Projects/ProjectTicketControllerTest.php index 696367a..7dc0ba7 100644 --- a/tests/Feature/Projects/ProjectTicketControllerTest.php +++ b/tests/Feature/Projects/ProjectTicketControllerTest.php @@ -527,9 +527,34 @@ class ProjectTicketControllerTest extends TestCase 'ticket_priority_id' => $highPriority->id, ]); - $ticket->refresh(); + $ticket->refresh()->load('acceptanceCriteria'); - $this->assertEquals($updatedAcceptanceCriteria, $ticket->acceptance_criteria_data); + $this->assertEquals( + $updatedAcceptanceCriteria, + \App\Actions\Tickets\Support\TicketAcceptanceCriteriaData::fromModels( + $ticket->acceptanceCriteria, + $ticket->acceptance_criteria, + ), + ); + $this->assertDatabaseHas('acceptance_criteria', [ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => null, + 'position' => 0, + 'text' => 'Shows success banner', + 'is_checked' => false, + ]); + $rootCriterionId = \App\Models\AcceptanceCriterion::query() + ->where('ticket_id', $ticket->id) + ->where('text', 'Shows success banner') + ->value('id'); + $this->assertNotNull($rootCriterionId); + $this->assertDatabaseHas('acceptance_criteria', [ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => $rootCriterionId, + 'position' => 0, + 'text' => 'Includes nested confirmations', + 'is_checked' => true, + ]); $this->assertDatabaseHas('ticket_field_values', [ 'ticket_id' => $ticket->id, 'ticket_field_definition_id' => $reviewerField->id, From 689db9e41b904811a58a0549ecc7951012abb395 Mon Sep 17 00:00:00 2001 From: Koda Date: Tue, 19 May 2026 22:44:28 +0000 Subject: [PATCH 2/6] Simplify acceptance criteria checklist model --- app/Actions/Tickets/CreateProjectTicket.php | 21 +- ...ta.php => AcceptanceCriteriaChecklist.php} | 207 ++++-------------- app/Actions/Tickets/UpdateProjectTicket.php | 48 ++-- .../Requests/StoreProjectTicketRequest.php | 14 +- .../Requests/UpdateProjectTicketRequest.php | 14 +- app/Http/Resources/TicketResource.php | 5 +- app/Mcp/Support/KanbanGitMcp.php | 9 +- resources/js/lib/acceptance-criteria.ts | 33 +-- resources/js/pages/projects/Ticket.vue | 29 +-- resources/js/pages/projects/TicketCreate.vue | 27 +-- .../types/models/acceptance-criteria-model.ts | 5 +- resources/js/types/models/ticket-model.ts | 1 - tests/Feature/Api/TicketApiTest.php | 37 ++-- .../TicketCreatedNotificationDispatchTest.php | 2 +- tests/Feature/Projects/ProjectBacklogTest.php | 6 +- .../Projects/ProjectTicketControllerTest.php | 45 ++-- 16 files changed, 140 insertions(+), 363 deletions(-) rename app/Actions/Tickets/Support/{TicketAcceptanceCriteriaData.php => AcceptanceCriteriaChecklist.php} (54%) diff --git a/app/Actions/Tickets/CreateProjectTicket.php b/app/Actions/Tickets/CreateProjectTicket.php index 2598012..d4e69bf 100644 --- a/app/Actions/Tickets/CreateProjectTicket.php +++ b/app/Actions/Tickets/CreateProjectTicket.php @@ -4,7 +4,7 @@ namespace App\Actions\Tickets; use App\Actions\Notifications\BuildTicketCreatedPayload; use App\Actions\Notifications\TicketCreated; -use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData; +use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist; use App\Enums\TicketWorklogEntryType; use App\Models\Project; use App\Models\Ticket; @@ -23,14 +23,11 @@ class CreateProjectTicket /** * @param array{ - * acceptance_criteria: array{ - * summary: string|null, - * items: array - * }, + * acceptance_criteria: array, * assignee_user_id?: int|null, * description: string|null, * story_points?: int|null, @@ -43,7 +40,7 @@ class CreateProjectTicket public function handle(Project $project, array $attributes, ?User $actor = null): Ticket { return DB::transaction(function () use ($actor, $attributes, $project): Ticket { - $acceptanceCriteria = TicketAcceptanceCriteriaData::normalize( + $acceptanceCriteria = AcceptanceCriteriaChecklist::normalize( Arr::get($attributes, 'acceptance_criteria'), ); $nextSequenceNumber = (int) Ticket::query() @@ -54,7 +51,7 @@ class CreateProjectTicket ->findOrFail(Arr::get($attributes, 'ticket_status_id')); $ticket = Ticket::query()->create([ - 'acceptance_criteria' => TicketAcceptanceCriteriaData::toLegacyText($acceptanceCriteria), + 'acceptance_criteria' => AcceptanceCriteriaChecklist::toText($acceptanceCriteria), 'assignee_user_id' => Arr::get($attributes, 'assignee_user_id'), 'author_user_id' => $actor?->id, 'description' => Arr::get($attributes, 'description'), @@ -68,7 +65,7 @@ class CreateProjectTicket 'story_points' => Arr::get($attributes, 'story_points'), ]); - TicketAcceptanceCriteriaData::syncTicket($ticket, $acceptanceCriteria); + AcceptanceCriteriaChecklist::syncTicket($ticket, $acceptanceCriteria); $occurredAt = CarbonImmutable::now(); diff --git a/app/Actions/Tickets/Support/TicketAcceptanceCriteriaData.php b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklist.php similarity index 54% rename from app/Actions/Tickets/Support/TicketAcceptanceCriteriaData.php rename to app/Actions/Tickets/Support/AcceptanceCriteriaChecklist.php index 3378505..0157c66 100644 --- a/app/Actions/Tickets/Support/TicketAcceptanceCriteriaData.php +++ b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklist.php @@ -7,24 +7,16 @@ use App\Models\Ticket; use Illuminate\Database\Eloquent\Collection as EloquentCollection; use Illuminate\Support\Arr; -class TicketAcceptanceCriteriaData +class AcceptanceCriteriaChecklist { public const MAX_INDENT_LEVEL = 8; /** * @param iterable $criteria - * @return array{ - * summary: string|null, - * items: array - * } + * @return array */ - public static function fromModels(iterable $criteria, ?string $legacyText): array + public static function fromModels(iterable $criteria, ?string $legacyText = null): array { - $summary = self::normalize($legacyText)['summary']; $models = ($criteria instanceof EloquentCollection ? $criteria : collect($criteria)) ->sort(function (AcceptanceCriterion $left, AcceptanceCriterion $right): int { return [ @@ -43,27 +35,17 @@ class TicketAcceptanceCriteriaData return self::normalize($legacyText); } - /** @var array> $grouped */ + /** @var array> $grouped */ $grouped = $models->groupBy('parent_acceptance_criterion_id')->all(); $items = []; self::flattenItems($items, $grouped, null, 0); - return [ - 'summary' => $summary, - 'items' => $items, - ]; + return $items; } /** - * @return array{ - * summary: string|null, - * items: array - * } + * @return array */ public static function normalize(mixed $value): array { @@ -75,50 +57,27 @@ class TicketAcceptanceCriteriaData return self::normalizeFromString($value); } - return self::empty(); - } - - /** - * @param array{ - * summary: string|null, - * items: array - * } $acceptanceCriteria - */ - public static function hasContent(array $acceptanceCriteria): bool - { - return $acceptanceCriteria['summary'] !== null - || $acceptanceCriteria['items'] !== []; + return []; } /** * @param Ticket $ticket - * @param array{ - * summary: string|null, - * items: array - * } $acceptanceCriteria + * @param array $items */ - public static function syncTicket(Ticket $ticket, array $acceptanceCriteria): void + public static function syncTicket(Ticket $ticket, array $items): void { - $normalized = self::normalize($acceptanceCriteria); + $normalized = self::normalize($items); $ticket->acceptanceCriteria()->delete(); - if ($normalized['items'] === []) { + if ($normalized === []) { return; } $parentStack = []; $positionsByParent = []; - foreach ($normalized['items'] as $item) { + foreach ($normalized as $item) { $indent = min($item['indent'], count($parentStack)); $parentId = $indent === 0 ? null : $parentStack[$indent - 1]; $positionKey = $parentId ?? 'root'; @@ -140,60 +99,25 @@ class TicketAcceptanceCriteriaData } /** - * @param array{ - * summary: string|null, - * items: array - * } $acceptanceCriteria + * @param array $items */ - public static function toLegacyText(array $acceptanceCriteria): ?string + public static function toText(array $items): ?string { - $normalized = self::normalize($acceptanceCriteria); - $lines = []; + $normalized = self::normalize($items); - if ($normalized['summary'] !== null) { - $lines[] = $normalized['summary']; + if ($normalized === []) { + return null; } - if ($normalized['items'] !== []) { - if ($normalized['summary'] !== null) { - $lines[] = ''; - } - - foreach ($normalized['items'] as $item) { - $lines[] = sprintf( - '%s- [%s] %s', - str_repeat(' ', $item['indent']), - $item['checked'] ? 'x' : ' ', - $item['text'], - ); - } - } - - $legacyText = trim(implode("\n", $lines)); - - return $legacyText === '' ? null : $legacyText; - } - - /** - * @return array{ - * summary: string|null, - * items: array - * } - */ - private static function empty(): array - { - return [ - 'summary' => null, - 'items' => [], - ]; + return implode("\n", array_map( + static fn (array $item): string => sprintf( + '%s- [%s] %s', + str_repeat(' ', $item['indent']), + $item['checked'] ? 'x' : ' ', + $item['text'], + ), + $normalized, + )); } /** @@ -214,23 +138,15 @@ class TicketAcceptanceCriteriaData } /** - * @param array $value - * @return array{ - * summary: string|null, - * items: array - * } + * @param array $value + * @return array */ private static function normalizeFromArray(array $value): array { - $summary = trim((string) Arr::get($value, 'summary', '')); - $rawItems = Arr::get($value, 'items', []); + $rawItems = Arr::isAssoc($value) ? Arr::get($value, 'items', []) : $value; if (! is_array($rawItems)) { - $rawItems = []; + return []; } $items = []; @@ -246,78 +162,37 @@ class TicketAcceptanceCriteriaData continue; } - $rawChecked = Arr::get($rawItem, 'checked', false); - $rawIndent = Arr::get($rawItem, 'indent', 0); - $items[] = [ - 'checked' => filter_var($rawChecked, FILTER_VALIDATE_BOOLEAN), - 'indent' => self::clampIndent((int) $rawIndent), + 'checked' => filter_var(Arr::get($rawItem, 'checked', false), FILTER_VALIDATE_BOOLEAN), + 'indent' => self::clampIndent((int) Arr::get($rawItem, 'indent', 0)), 'text' => $text, ]; } - return [ - 'summary' => $summary === '' ? null : $summary, - 'items' => $items, - ]; + return $items; } /** - * @return array{ - * summary: string|null, - * items: array - * } + * @return array */ private static function normalizeFromString(?string $value): array { $text = trim((string) $value); if ($text === '') { - return self::empty(); + return []; } $lines = preg_split('/\R/u', $text); if (! is_array($lines)) { - return [ - 'summary' => $text, - 'items' => [], - ]; + return []; } - $summary = null; - $items = []; - - foreach ($lines as $line) { - $trimmedLine = trim($line); - - if ($trimmedLine === '') { - continue; - } - - if ($summary === null) { - $summary = $trimmedLine; - - continue; - } - - $parsedItem = self::parseChecklistItem($line); - - if ($parsedItem === null) { - continue; - } - - $items[] = $parsedItem; - } - - return [ - 'summary' => $summary, - 'items' => $items, - ]; + return array_values(array_filter(array_map( + static fn (string $line): ?array => self::parseChecklistItem($line), + $lines, + ))); } /** @@ -348,6 +223,8 @@ class TicketAcceptanceCriteriaData $content = trim((string) Arr::get($checkboxMatches, 'text', '')); } elseif (preg_match('/^[-*+]\s+(?.+)$/u', $content, $bulletMatches)) { $content = trim((string) Arr::get($bulletMatches, 'text', '')); + } else { + return null; } if ($content === '') { diff --git a/app/Actions/Tickets/UpdateProjectTicket.php b/app/Actions/Tickets/UpdateProjectTicket.php index cf83e3c..ea2969f 100644 --- a/app/Actions/Tickets/UpdateProjectTicket.php +++ b/app/Actions/Tickets/UpdateProjectTicket.php @@ -2,7 +2,7 @@ namespace App\Actions\Tickets; -use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData; +use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist; use App\Enums\TicketWorklogEntryType; use App\Models\Ticket; use App\Models\TicketFieldDefinition; @@ -21,14 +21,11 @@ class UpdateProjectTicket /** * @param array{ - * acceptance_criteria: array{ - * summary: string|null, - * items: array - * }, + * acceptance_criteria: array, * assignee_user_id?: int|null, * author_user_id?: int|null, * blocked_comment: string|null, @@ -52,7 +49,7 @@ class UpdateProjectTicket array $validated, ): bool { $fieldValueAttributes = Arr::get($validated, 'field_values', []); - $nextAcceptanceCriteria = TicketAcceptanceCriteriaData::normalize( + $nextAcceptanceCriteria = AcceptanceCriteriaChecklist::normalize( Arr::get($validated, 'acceptance_criteria'), ); $ticketAttributes = $this->buildTicketAttributes($ticket, $validated); @@ -89,7 +86,7 @@ class UpdateProjectTicket /** @var Collection $existingFieldValues */ $existingFieldValues = $ticket->fieldValues ->keyBy('ticket_field_definition_id'); - $currentAcceptanceCriteria = TicketAcceptanceCriteriaData::fromModels( + $currentAcceptanceCriteria = AcceptanceCriteriaChecklist::fromModels( $ticket->acceptanceCriteria, $ticket->acceptance_criteria, ); @@ -199,7 +196,7 @@ class UpdateProjectTicket } if ($currentAcceptanceCriteria !== $nextAcceptanceCriteria) { - TicketAcceptanceCriteriaData::syncTicket($ticket, $nextAcceptanceCriteria); + AcceptanceCriteriaChecklist::syncTicket($ticket, $nextAcceptanceCriteria); } $fieldValueChangeSet = $this->syncProjectTicketFieldValues->handle( @@ -235,14 +232,11 @@ class UpdateProjectTicket /** * @param array{ - * acceptance_criteria: array{ - * summary: string|null, - * items: array - * }, + * acceptance_criteria: array, * assignee_user_id?: int|null, * author_user_id?: int|null, * blocked_comment: string|null, @@ -270,12 +264,12 @@ class UpdateProjectTicket */ private function buildTicketAttributes(Ticket $ticket, array $validated): array { - $acceptanceCriteria = TicketAcceptanceCriteriaData::normalize( + $acceptanceCriteria = AcceptanceCriteriaChecklist::normalize( Arr::get($validated, 'acceptance_criteria'), ); return [ - 'acceptance_criteria' => TicketAcceptanceCriteriaData::toLegacyText($acceptanceCriteria), + 'acceptance_criteria' => AcceptanceCriteriaChecklist::toText($acceptanceCriteria), 'assignee_user_id' => Arr::exists($validated, 'assignee_user_id') ? Arr::get($validated, 'assignee_user_id') : $ticket->assignee_user_id, @@ -298,14 +292,8 @@ class UpdateProjectTicket /** * @param array $changes * @param array $summaryLabels - * @param array{ - * summary: string|null, - * items: array - * } $currentValue - * @param array{ - * summary: string|null, - * items: array - * } $newValue + * @param array $currentValue + * @param array $newValue */ private function trackAcceptanceCriteriaChange( array &$changes, diff --git a/app/Http/Requests/StoreProjectTicketRequest.php b/app/Http/Requests/StoreProjectTicketRequest.php index cf5e886..2fd9d8b 100644 --- a/app/Http/Requests/StoreProjectTicketRequest.php +++ b/app/Http/Requests/StoreProjectTicketRequest.php @@ -2,7 +2,7 @@ namespace App\Http\Requests; -use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData; +use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist; use App\Models\Project; use Illuminate\Contracts\Validation\ValidationRule; use Illuminate\Foundation\Http\FormRequest; @@ -24,7 +24,7 @@ class StoreProjectTicketRequest extends FormRequest protected function prepareForValidation(): void { $this->merge([ - 'acceptance_criteria' => TicketAcceptanceCriteriaData::normalize( + 'acceptance_criteria' => AcceptanceCriteriaChecklist::normalize( $this->input('acceptance_criteria'), ), ]); @@ -42,12 +42,10 @@ class StoreProjectTicketRequest extends FormRequest return [ 'acceptance_criteria' => ['nullable', 'array'], - 'acceptance_criteria.summary' => ['nullable', 'string', 'max:255'], - 'acceptance_criteria.items' => ['array'], - 'acceptance_criteria.items.*' => ['array'], - 'acceptance_criteria.items.*.text' => ['required', 'string'], - 'acceptance_criteria.items.*.checked' => ['required', 'boolean'], - 'acceptance_criteria.items.*.indent' => ['required', 'integer', 'min:0', 'max:'.TicketAcceptanceCriteriaData::MAX_INDENT_LEVEL], + 'acceptance_criteria.*' => ['array'], + 'acceptance_criteria.*.text' => ['required', 'string'], + 'acceptance_criteria.*.checked' => ['required', 'boolean'], + 'acceptance_criteria.*.indent' => ['required', 'integer', 'min:0', 'max:'.AcceptanceCriteriaChecklist::MAX_INDENT_LEVEL], 'assignee_user_id' => ['nullable', 'integer', Rule::exists('users', 'id')], 'description' => ['nullable', 'string'], 'ticket_priority_id' => [ diff --git a/app/Http/Requests/UpdateProjectTicketRequest.php b/app/Http/Requests/UpdateProjectTicketRequest.php index a8d08e7..d5796f0 100644 --- a/app/Http/Requests/UpdateProjectTicketRequest.php +++ b/app/Http/Requests/UpdateProjectTicketRequest.php @@ -3,7 +3,7 @@ namespace App\Http\Requests; use App\Actions\Permissions\PermissionService; -use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData; +use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist; use App\Enums\Permission; use App\Models\Project; use App\Models\Ticket; @@ -30,7 +30,7 @@ class UpdateProjectTicketRequest extends FormRequest protected function prepareForValidation(): void { $this->merge([ - 'acceptance_criteria' => TicketAcceptanceCriteriaData::normalize( + 'acceptance_criteria' => AcceptanceCriteriaChecklist::normalize( $this->input('acceptance_criteria'), ), ]); @@ -48,12 +48,10 @@ class UpdateProjectTicketRequest extends FormRequest return [ 'acceptance_criteria' => ['nullable', 'array'], - 'acceptance_criteria.summary' => ['nullable', 'string', 'max:255'], - 'acceptance_criteria.items' => ['array'], - 'acceptance_criteria.items.*' => ['array'], - 'acceptance_criteria.items.*.text' => ['required', 'string'], - 'acceptance_criteria.items.*.checked' => ['required', 'boolean'], - 'acceptance_criteria.items.*.indent' => ['required', 'integer', 'min:0', 'max:'.TicketAcceptanceCriteriaData::MAX_INDENT_LEVEL], + 'acceptance_criteria.*' => ['array'], + 'acceptance_criteria.*.text' => ['required', 'string'], + 'acceptance_criteria.*.checked' => ['required', 'boolean'], + 'acceptance_criteria.*.indent' => ['required', 'integer', 'min:0', 'max:'.AcceptanceCriteriaChecklist::MAX_INDENT_LEVEL], 'assignee_user_id' => ['nullable', 'integer', Rule::exists('users', 'id')], 'author_user_id' => ['nullable', 'integer', Rule::exists('users', 'id')], 'blocked_comment' => ['nullable', 'string'], diff --git a/app/Http/Resources/TicketResource.php b/app/Http/Resources/TicketResource.php index 1b38660..bb18f5a 100644 --- a/app/Http/Resources/TicketResource.php +++ b/app/Http/Resources/TicketResource.php @@ -2,7 +2,7 @@ namespace App\Http\Resources; -use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData; +use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist; use Illuminate\Http\Request; use Illuminate\Http\Resources\Json\JsonResource; @@ -17,13 +17,12 @@ class TicketResource extends JsonResource { return [ 'id' => $this->id, - 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromModels( + 'acceptance_criteria' => AcceptanceCriteriaChecklist::fromModels( $this->relationLoaded('acceptanceCriteria') ? $this->acceptanceCriteria : $this->acceptanceCriteria()->get(), $this->acceptance_criteria, ), - 'acceptance_criteria_text' => $this->acceptance_criteria, 'assignee' => $this->whenLoaded( 'assignee', fn (): ?array => $this->assignee === null diff --git a/app/Mcp/Support/KanbanGitMcp.php b/app/Mcp/Support/KanbanGitMcp.php index 85444a4..a6fc846 100644 --- a/app/Mcp/Support/KanbanGitMcp.php +++ b/app/Mcp/Support/KanbanGitMcp.php @@ -3,7 +3,7 @@ namespace App\Mcp\Support; use App\Actions\Permissions\PermissionService; -use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData; +use App\Actions\Tickets\Support\AcceptanceCriteriaChecklist; use App\Enums\Permission; use App\Http\Resources\ProjectResource; use App\Http\Resources\TicketResource; @@ -208,10 +208,7 @@ class KanbanGitMcp /** * @return array{ - * acceptance_criteria: array{ - * summary: string|null, - * items: array - * }, + * acceptance_criteria: array, * assignee_user_id: int|null, * author_user_id: int|null, * blocked_comment: string|null, @@ -227,7 +224,7 @@ class KanbanGitMcp public function updatePayload(Ticket $ticket): array { return [ - 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromModels( + 'acceptance_criteria' => AcceptanceCriteriaChecklist::fromModels( $ticket->relationLoaded('acceptanceCriteria') ? $ticket->acceptanceCriteria : $ticket->acceptanceCriteria()->get(), diff --git a/resources/js/lib/acceptance-criteria.ts b/resources/js/lib/acceptance-criteria.ts index e3b5243..e08fe36 100644 --- a/resources/js/lib/acceptance-criteria.ts +++ b/resources/js/lib/acceptance-criteria.ts @@ -7,24 +7,15 @@ const MAX_INDENT_LEVEL = 8; export type AcceptanceCriteriaEditorState = { checklist: string; - summary: string; }; -export const emptyAcceptanceCriteria = (): AcceptanceCriteriaModel => ({ - summary: null, - items: [], -}); +export const emptyAcceptanceCriteria = (): AcceptanceCriteriaModel => []; export const normalizeAcceptanceCriteria = ( value: AcceptanceCriteriaModel | null | undefined, ): AcceptanceCriteriaModel => { - if (value === null || value === undefined) { - return emptyAcceptanceCriteria(); - } - - const summary = value.summary?.trim() ?? null; - const items = Array.isArray(value.items) - ? value.items + const items = Array.isArray(value) + ? value .map((item) => ({ checked: item.checked === true, indent: clampIndent(item.indent), @@ -33,10 +24,7 @@ export const normalizeAcceptanceCriteria = ( .filter((item) => item.text !== '') : []; - return { - summary: summary === '' ? null : summary, - items, - }; + return items; }; export const parseChecklistLines = ( @@ -69,15 +57,9 @@ export const formatChecklistLines = ( }; export const buildAcceptanceCriteria = ( - summaryInput: string, checklistInput: string, ): AcceptanceCriteriaModel => { - const summary = summaryInput.trim(); - - return normalizeAcceptanceCriteria({ - summary: summary === '' ? null : summary, - items: parseChecklistLines(checklistInput), - }); + return normalizeAcceptanceCriteria(parseChecklistLines(checklistInput)); }; export const acceptanceCriteriaToEditorState = ( @@ -86,8 +68,7 @@ export const acceptanceCriteriaToEditorState = ( const normalized = normalizeAcceptanceCriteria(value); return { - checklist: formatChecklistLines(normalized.items), - summary: normalized.summary ?? '', + checklist: formatChecklistLines(normalized), }; }; @@ -123,6 +104,8 @@ const parseChecklistLine = ( if (bulletMatch?.groups !== undefined) { content = bulletMatch.groups.text.trim(); + } else { + return null; } } diff --git a/resources/js/pages/projects/Ticket.vue b/resources/js/pages/projects/Ticket.vue index b03ec0a..84753f7 100644 --- a/resources/js/pages/projects/Ticket.vue +++ b/resources/js/pages/projects/Ticket.vue @@ -111,14 +111,10 @@ const breadcrumbs: BreadcrumbItem[] = [ const acceptanceCriteriaEditorState = acceptanceCriteriaToEditorState( props.ticket.acceptance_criteria, ); -const acceptanceCriteriaSummary = ref(acceptanceCriteriaEditorState.summary); const acceptanceCriteriaChecklist = ref(acceptanceCriteriaEditorState.checklist); const form = useForm({ - acceptance_criteria: buildAcceptanceCriteria( - acceptanceCriteriaSummary.value, - acceptanceCriteriaChecklist.value, - ), + acceptance_criteria: buildAcceptanceCriteria(acceptanceCriteriaChecklist.value), assignee_user_id: props.ticket.assignee_user_id ?? '', author_user_id: props.ticket.author_user_id ?? '', blocked_comment: props.ticket.blocked_comment ?? '', @@ -151,10 +147,8 @@ const acceptanceCriteriaPreviewItems = computed(() => const acceptanceCriteriaError = computed(() => { return ( form.errors.acceptance_criteria ?? - form.errors['acceptance_criteria.summary'] ?? - form.errors['acceptance_criteria.items'] ?? Object.entries(form.errors).find(([key]) => - key.startsWith('acceptance_criteria.items.'), + key.startsWith('acceptance_criteria.'), )?.[1] ?? '' ); @@ -348,10 +342,7 @@ const submit = (): void => { return; } - form.acceptance_criteria = buildAcceptanceCriteria( - acceptanceCriteriaSummary.value, - acceptanceCriteriaChecklist.value, - ); + form.acceptance_criteria = buildAcceptanceCriteria(acceptanceCriteriaChecklist.value); form.submit( ProjectTicketController.update({ @@ -625,20 +616,6 @@ const formatDateTime = (value: string): string => {
-
- - -
-
-
- - -
-
{ class="border-border rounded-xl border p-4 text-sm" >

- Checklist preview + Saved checklist

  • - {{ item.checked ? '☑' : '☐' }} - {{ item.text }} + +
diff --git a/resources/js/types/models/acceptance-criteria-model.ts b/resources/js/types/models/acceptance-criteria-model.ts index bb65bd4..9b2bba3 100644 --- a/resources/js/types/models/acceptance-criteria-model.ts +++ b/resources/js/types/models/acceptance-criteria-model.ts @@ -1,4 +1,5 @@ export type AcceptanceCriteriaItemModel = { + id?: number; checked: boolean; indent: number; text: string; diff --git a/tests/Feature/Projects/ProjectTicketControllerTest.php b/tests/Feature/Projects/ProjectTicketControllerTest.php index 3a35809..e74504a 100644 --- a/tests/Feature/Projects/ProjectTicketControllerTest.php +++ b/tests/Feature/Projects/ProjectTicketControllerTest.php @@ -579,6 +579,88 @@ class ProjectTicketControllerTest extends TestCase ]); } + public function test_ticket_update_reuses_acceptance_criteria_ids_when_provided(): void + { + $editor = User::factory()->create(); + $project = Project::factory()->create([ + 'key' => 'CORE', + ]); + + app(InitializeProjectTicketMetadata::class)->handle($project); + + $priority = $project->ticketPriorities()->firstOrFail(); + $ticketType = $this->nonParentType($project); + $ticket = Ticket::factory()->create([ + 'project_id' => $project->id, + 'sequence_number' => 1, + 'ticket_key' => 'CORE-1', + 'ticket_priority_id' => $priority->id, + 'ticket_type_id' => $ticketType->id, + 'title' => 'Acceptance criteria update test', + ]); + $rootCriterion = AcceptanceCriterion::query()->create([ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => null, + 'position' => 0, + 'text' => 'Original root item', + 'is_checked' => false, + ]); + $childCriterion = AcceptanceCriterion::query()->create([ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => $rootCriterion->id, + 'position' => 0, + 'text' => 'Original child item', + 'is_checked' => false, + ]); + + $this->syncProjectPermissions($project, $editor, ['write', 'view']); + + $this->actingAs($editor) + ->patch(route('projects.tickets.update', [$project, $ticket]), [ + 'acceptance_criteria' => [ + [ + 'id' => $rootCriterion->id, + 'text' => 'Updated root item', + 'checked' => true, + 'indent' => 0, + ], + [ + 'id' => $childCriterion->id, + 'text' => 'Updated child item', + 'checked' => false, + 'indent' => 1, + ], + ], + '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' => $priority->id, + 'ticket_type_id' => $ticketType->id, + 'title' => $ticket->title, + ]) + ->assertRedirect(route('projects.tickets.show', [$project, $ticket])); + + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $rootCriterion->id, + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => null, + 'position' => 0, + 'text' => 'Updated root item', + 'is_checked' => true, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $childCriterion->id, + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => $rootCriterion->id, + 'position' => 0, + 'text' => 'Updated child item', + 'is_checked' => false, + ]); + } + /** * Ensure non-parent tickets cannot be assigned as a parent target. */