diff --git a/app/Actions/Notifications/MatchNotificationFilters.php b/app/Actions/Notifications/MatchNotificationFilters.php index 1c620a5..9553370 100644 --- a/app/Actions/Notifications/MatchNotificationFilters.php +++ b/app/Actions/Notifications/MatchNotificationFilters.php @@ -190,6 +190,10 @@ class MatchNotificationFilters $fromStatusLabel = property_exists($payload, 'fromStatusLabel') ? $payload->fromStatusLabel : null; $toStatusId = property_exists($payload, 'toStatusId') ? $payload->toStatusId : null; $toStatusLabel = property_exists($payload, 'toStatusLabel') ? $payload->toStatusLabel : null; + $fromAssigneeId = property_exists($payload, 'fromAssigneeId') ? $payload->fromAssigneeId : null; + $fromAssigneeName = property_exists($payload, 'fromAssigneeName') ? $payload->fromAssigneeName : null; + $toAssigneeId = property_exists($payload, 'toAssigneeId') ? $payload->toAssigneeId : null; + $toAssigneeName = property_exists($payload, 'toAssigneeName') ? $payload->toAssigneeName : null; return [ 'project' => [ @@ -209,12 +213,12 @@ class MatchNotificationFilters ], 'assignee' => [ 'from' => [ - 'id' => $payload->fromAssigneeId, - 'name' => $payload->fromAssigneeName, + 'id' => $fromAssigneeId, + 'name' => $fromAssigneeName, ], 'to' => [ - 'id' => $payload->toAssigneeId, - 'name' => $payload->toAssigneeName, + 'id' => $toAssigneeId, + 'name' => $toAssigneeName, ], ], 'status' => [ diff --git a/app/Actions/Tickets/CreateProjectTicket.php b/app/Actions/Tickets/CreateProjectTicket.php index 2f8c551..a2c26a5 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\AcceptanceCriteriaChecklistMapper; 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 = AcceptanceCriteriaChecklistMapper::normalize( Arr::get($attributes, 'acceptance_criteria'), ); $nextSequenceNumber = (int) Ticket::query() @@ -54,8 +51,6 @@ class CreateProjectTicket ->findOrFail(Arr::get($attributes, 'ticket_status_id')); $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 +64,8 @@ class CreateProjectTicket 'story_points' => Arr::get($attributes, 'story_points'), ]); + AcceptanceCriteriaChecklistMapper::syncTicket($ticket, $acceptanceCriteria); + $occurredAt = CarbonImmutable::now(); if ($actor !== null) { diff --git a/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php new file mode 100644 index 0000000..5d08552 --- /dev/null +++ b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php @@ -0,0 +1,225 @@ + $criteria + * @return array + */ + public static function fromModels(iterable $criteria): array + { + return self::flattenModels($criteria, false); + } + + /** + * @param iterable $criteria + * @return array + */ + public static function fromModelsWithIds(iterable $criteria): array + { + return self::flattenModels($criteria, true); + } + + /** + * @return array + */ + public static function normalize(mixed $value): array + { + if (is_array($value)) { + return self::normalizeFromArray($value); + } + + return []; + } + + /** + * @param Ticket $ticket + * @param array $items + */ + public static function syncTicket(Ticket $ticket, array $items): void + { + $normalized = self::normalize($items); + $existingCriteria = $ticket->acceptanceCriteria()->get()->keyBy('id'); + + if ($normalized === []) { + $ticket->acceptanceCriteria()->delete(); + $ticket->unsetRelation('acceptanceCriteria'); + + return; + } + + $keptCriterionIds = []; + $parentStack = []; + $positionsByParent = []; + $fallbackCriteria = $existingCriteria->values(); + $fallbackIndex = 0; + + foreach ($normalized as $item) { + $indent = min($item['indent'], count($parentStack)); + $parentId = $indent === 0 ? null : $parentStack[$indent - 1]; + $positionKey = $parentId ?? 'root'; + $position = $positionsByParent[$positionKey] ?? 0; + $criterionId = $item['id'] ?? null; + $criterion = $criterionId === null + ? null + : $existingCriteria->get($criterionId); + + while ( + $criterion === null + && $fallbackIndex < $fallbackCriteria->count() + ) { + /** @var AcceptanceCriterion $candidate */ + $candidate = $fallbackCriteria[$fallbackIndex]; + $fallbackIndex++; + + if (in_array($candidate->id, $keptCriterionIds, true)) { + continue; + } + + $criterion = $candidate; + } + + if ($criterion === null) { + $criterion = $ticket->acceptanceCriteria()->create([ + 'parent_acceptance_criterion_id' => $parentId, + 'position' => $position, + 'text' => $item['text'], + 'is_checked' => $item['checked'], + ]); + } else { + $criterion->fill([ + 'parent_acceptance_criterion_id' => $parentId, + 'position' => $position, + 'text' => $item['text'], + 'is_checked' => $item['checked'], + ]); + + if ($criterion->isDirty()) { + $criterion->save(); + } + } + + $keptCriterionIds[] = $criterion->id; + $positionsByParent[$positionKey] = $position + 1; + $parentStack[$indent] = $criterion->id; + $parentStack = array_slice($parentStack, 0, $indent + 1); + } + + $idsToDelete = $existingCriteria->keys()->diff($keptCriterionIds); + + if ($idsToDelete->isNotEmpty()) { + $ticket->acceptanceCriteria() + ->whereIn('id', $idsToDelete->all()) + ->delete(); + } + + $ticket->unsetRelation('acceptanceCriteria'); + } + + /** + * @param array $value + * @return array + */ + private static function normalizeFromArray(array $value): array + { + $items = []; + + foreach ($value as $rawItem) { + if (! is_array($rawItem)) { + continue; + } + + $text = trim((string) Arr::get($rawItem, 'text', '')); + + if ($text === '') { + continue; + } + + $normalizedItem = [ + 'checked' => filter_var(Arr::get($rawItem, 'checked', false), FILTER_VALIDATE_BOOLEAN), + 'indent' => self::clampIndent((int) Arr::get($rawItem, 'indent', 0)), + 'text' => $text, + ]; + + $id = filter_var(Arr::get($rawItem, 'id'), FILTER_VALIDATE_INT); + + if (is_int($id) && $id > 0) { + $normalizedItem['id'] = $id; + } + + $items[] = $normalizedItem; + } + + return $items; + } + + private static function clampIndent(int $indent): int + { + return max(0, min(self::MAX_INDENT_LEVEL, $indent)); + } + + /** + * @param iterable $criteria + * @return array + */ + private static function flattenModels(iterable $criteria, bool $includeIds): array + { + $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 ($models->isEmpty()) { + return []; + } + + /** @var array> $grouped */ + $grouped = $models->groupBy('parent_acceptance_criterion_id')->all(); + $items = []; + + self::flattenItems($items, $grouped, null, 0, $includeIds); + + return $items; + } + + /** + * @param array $items + * @param array> $grouped + */ + private static function flattenItems(array &$items, array $grouped, ?int $parentId, int $indent, bool $includeIds): void + { + foreach ($grouped[$parentId] ?? [] as $criterion) { + $item = [ + 'checked' => $criterion->is_checked, + 'indent' => $indent, + 'text' => $criterion->text, + ]; + + if ($includeIds) { + $item['id'] = $criterion->id; + } + + $items[] = $item; + + self::flattenItems($items, $grouped, $criterion->id, $indent + 1, $includeIds); + } + } +} diff --git a/app/Actions/Tickets/Support/TicketAcceptanceCriteriaData.php b/app/Actions/Tickets/Support/TicketAcceptanceCriteriaData.php deleted file mode 100644 index 9a066f3..0000000 --- a/app/Actions/Tickets/Support/TicketAcceptanceCriteriaData.php +++ /dev/null @@ -1,316 +0,0 @@ - - * } - */ - public static function fromStored(mixed $storedData, ?string $legacyText): array - { - $normalizedStoredData = is_array($storedData) - ? self::normalize($storedData) - : self::empty(); - - if (self::hasContent($normalizedStoredData)) { - return $normalizedStoredData; - } - - return self::normalize($legacyText); - } - - /** - * @return array{ - * summary: string|null, - * items: array - * } - */ - public static function normalize(mixed $value): array - { - if (is_array($value)) { - return self::normalizeFromArray($value); - } - - if (is_string($value) || $value === null) { - 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'] !== []; - } - - /** - * @param array{ - * summary: string|null, - * items: array - * } $acceptanceCriteria - * @return array{ - * summary: string|null, - * items: array - * }|null - */ - public static function forStorage(array $acceptanceCriteria): ?array - { - $normalized = self::normalize($acceptanceCriteria); - - return self::hasContent($normalized) - ? $normalized - : null; - } - - /** - * @param array{ - * summary: string|null, - * items: array - * } $acceptanceCriteria - */ - public static function toLegacyText(array $acceptanceCriteria): ?string - { - $normalized = self::normalize($acceptanceCriteria); - $lines = []; - - if ($normalized['summary'] !== null) { - $lines[] = $normalized['summary']; - } - - 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' => [], - ]; - } - - /** - * @param array $value - * @return array{ - * summary: string|null, - * items: array - * } - */ - private static function normalizeFromArray(array $value): array - { - $summary = trim((string) Arr::get($value, 'summary', '')); - $rawItems = Arr::get($value, 'items', []); - - if (! is_array($rawItems)) { - $rawItems = []; - } - - $items = []; - - foreach ($rawItems as $rawItem) { - if (! is_array($rawItem)) { - continue; - } - - $text = trim((string) Arr::get($rawItem, 'text', '')); - - if ($text === '') { - 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), - 'text' => $text, - ]; - } - - return [ - 'summary' => $summary === '' ? null : $summary, - 'items' => $items, - ]; - } - - /** - * @return array{ - * summary: string|null, - * items: array - * } - */ - private static function normalizeFromString(?string $value): array - { - $text = trim((string) $value); - - if ($text === '') { - return self::empty(); - } - - $lines = preg_split('/\R/u', $text); - - if (! is_array($lines)) { - return [ - 'summary' => $text, - 'items' => [], - ]; - } - - $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{checked: bool, indent: int, text: string}|null - */ - private static function parseChecklistItem(string $line): ?array - { - $line = rtrim(str_replace("\t", ' ', $line)); - - if ($line === '') { - return null; - } - - if (! preg_match('/^(?\s*)(?.+)$/u', $line, $matches)) { - return null; - } - - $content = trim((string) Arr::get($matches, 'content', '')); - - if ($content === '') { - return null; - } - - $checked = false; - - if (preg_match('/^(?:[-*+]\s+)?\[(?[xX ])\]\s*(?.+)$/u', $content, $checkboxMatches)) { - $checked = strtolower((string) Arr::get($checkboxMatches, 'state', '')) === 'x'; - $content = trim((string) Arr::get($checkboxMatches, 'text', '')); - } elseif (preg_match('/^[-*+]\s+(?.+)$/u', $content, $bulletMatches)) { - $content = trim((string) Arr::get($bulletMatches, 'text', '')); - } - - if ($content === '') { - return null; - } - - return [ - 'checked' => $checked, - 'indent' => self::indentLevel((string) Arr::get($matches, 'indent', '')), - 'text' => $content, - ]; - } - - private static function indentLevel(string $leadingWhitespace): int - { - $normalizedWhitespace = str_replace("\t", ' ', $leadingWhitespace); - - return self::clampIndent(intdiv(strlen($normalizedWhitespace), 2)); - } - - private static function clampIndent(int $indent): int - { - return max(0, min(self::MAX_INDENT_LEVEL, $indent)); - } -} diff --git a/app/Actions/Tickets/UpdateProjectTicket.php b/app/Actions/Tickets/UpdateProjectTicket.php index f1e598a..5207647 100644 --- a/app/Actions/Tickets/UpdateProjectTicket.php +++ b/app/Actions/Tickets/UpdateProjectTicket.php @@ -4,7 +4,7 @@ namespace App\Actions\Tickets; use App\Actions\Notifications\BuildTicketAssigneeChangedPayload; use App\Actions\Notifications\TicketAssigneeChanged; -use App\Actions\Tickets\Support\TicketAcceptanceCriteriaData; +use App\Actions\Tickets\Support\AcceptanceCriteriaChecklistMapper; use App\Enums\TicketWorklogEntryType; use App\Models\Ticket; use App\Models\TicketFieldDefinition; @@ -25,14 +25,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, @@ -56,9 +53,12 @@ class UpdateProjectTicket array $validated, ): bool { $fieldValueAttributes = Arr::get($validated, 'field_values', []); + $nextAcceptanceCriteria = AcceptanceCriteriaChecklistMapper::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; } @@ -66,6 +66,7 @@ class UpdateProjectTicket $ticket->loadMissing([ 'assignee', 'author', + 'acceptanceCriteria', 'fieldValues.ticketFieldDefinition', 'fieldValues.userValue', 'parentTicket', @@ -90,14 +91,7 @@ class UpdateProjectTicket /** @var Collection $existingFieldValues */ $existingFieldValues = $ticket->fieldValues ->keyBy('ticket_field_definition_id'); - $currentAcceptanceCriteria = TicketAcceptanceCriteriaData::fromStored( - $ticket->acceptance_criteria_data, - $ticket->acceptance_criteria, - ); - $nextAcceptanceCriteria = TicketAcceptanceCriteriaData::fromStored( - $ticketAttributes['acceptance_criteria_data'], - $ticketAttributes['acceptance_criteria'], - ); + $currentAcceptanceCriteria = AcceptanceCriteriaChecklistMapper::fromModels($ticket->acceptanceCriteria); $changes = []; $summaryLabels = []; @@ -214,6 +208,10 @@ class UpdateProjectTicket $ticket->save(); } + if ($currentAcceptanceCriteria !== $nextAcceptanceCriteria) { + AcceptanceCriteriaChecklistMapper::syncTicket($ticket, $nextAcceptanceCriteria); + } + $fieldValueChangeSet = $this->syncProjectTicketFieldValues->handle( $ticket, $fieldDefinitions, @@ -262,14 +260,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, @@ -282,15 +277,6 @@ class UpdateProjectTicket * title: string * } $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, @@ -305,13 +291,7 @@ class UpdateProjectTicket */ private function buildTicketAttributes(Ticket $ticket, array $validated): array { - $acceptanceCriteria = TicketAcceptanceCriteriaData::normalize( - Arr::get($validated, 'acceptance_criteria'), - ); - 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, @@ -334,14 +314,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..505a267 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\AcceptanceCriteriaChecklistMapper; use App\Models\Project; use Illuminate\Contracts\Validation\ValidationRule; use Illuminate\Foundation\Http\FormRequest; @@ -23,9 +23,15 @@ class StoreProjectTicketRequest extends FormRequest */ protected function prepareForValidation(): void { + $acceptanceCriteria = $this->input('acceptance_criteria'); + + if ($acceptanceCriteria !== null && ! is_array($acceptanceCriteria)) { + return; + } + $this->merge([ - 'acceptance_criteria' => TicketAcceptanceCriteriaData::normalize( - $this->input('acceptance_criteria'), + 'acceptance_criteria' => AcceptanceCriteriaChecklistMapper::normalize( + $acceptanceCriteria, ), ]); } @@ -42,12 +48,11 @@ 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.*.id' => ['nullable', 'integer'], + 'acceptance_criteria.*.text' => ['required', 'string'], + 'acceptance_criteria.*.checked' => ['required', 'boolean'], + 'acceptance_criteria.*.indent' => ['required', 'integer', 'min:0', 'max:'.AcceptanceCriteriaChecklistMapper::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..1db69e5 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\AcceptanceCriteriaChecklistMapper; use App\Enums\Permission; use App\Models\Project; use App\Models\Ticket; @@ -29,9 +29,15 @@ class UpdateProjectTicketRequest extends FormRequest */ protected function prepareForValidation(): void { + $acceptanceCriteria = $this->input('acceptance_criteria'); + + if ($acceptanceCriteria !== null && ! is_array($acceptanceCriteria)) { + return; + } + $this->merge([ - 'acceptance_criteria' => TicketAcceptanceCriteriaData::normalize( - $this->input('acceptance_criteria'), + 'acceptance_criteria' => AcceptanceCriteriaChecklistMapper::normalize( + $acceptanceCriteria, ), ]); } @@ -48,12 +54,11 @@ 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.*.id' => ['nullable', 'integer'], + 'acceptance_criteria.*.text' => ['required', 'string'], + 'acceptance_criteria.*.checked' => ['required', 'boolean'], + 'acceptance_criteria.*.indent' => ['required', 'integer', 'min:0', 'max:'.AcceptanceCriteriaChecklistMapper::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 e4dee66..faacda9 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\AcceptanceCriteriaChecklistMapper; use Illuminate\Http\Request; use Illuminate\Http\Resources\Json\JsonResource; @@ -17,11 +17,11 @@ class TicketResource extends JsonResource { return [ 'id' => $this->id, - 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromStored( - $this->acceptance_criteria_data, - $this->acceptance_criteria, + 'acceptance_criteria' => AcceptanceCriteriaChecklistMapper::fromModelsWithIds( + $this->relationLoaded('acceptanceCriteria') + ? $this->acceptanceCriteria + : $this->acceptanceCriteria()->get(), ), - '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 6f0acec..c57cbd4 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\AcceptanceCriteriaChecklistMapper; 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,9 +224,10 @@ class KanbanGitMcp public function updatePayload(Ticket $ticket): array { return [ - 'acceptance_criteria' => TicketAcceptanceCriteriaData::fromStored( - $ticket->acceptance_criteria_data, - $ticket->acceptance_criteria, + 'acceptance_criteria' => AcceptanceCriteriaChecklistMapper::fromModels( + $ticket->relationLoaded('acceptanceCriteria') + ? $ticket->acceptanceCriteria + : $ticket->acceptanceCriteria()->get(), ), 'assignee_user_id' => $ticket->assignee_user_id, 'author_user_id' => $ticket->author_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..692faca 100644 --- a/app/Models/Ticket.php +++ b/app/Models/Ticket.php @@ -10,8 +10,6 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasMany; #[Fillable([ - 'acceptance_criteria', - 'acceptance_criteria_data', 'assignee_user_id', 'author_user_id', 'blocked_comment', @@ -38,11 +36,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/factories/TicketFactory.php b/database/factories/TicketFactory.php index 3dd26b8..401d387 100644 --- a/database/factories/TicketFactory.php +++ b/database/factories/TicketFactory.php @@ -22,7 +22,6 @@ class TicketFactory extends Factory public function definition(): array { return [ - 'acceptance_criteria' => fake()->paragraph(), 'assignee_user_id' => null, 'author_user_id' => User::factory(), 'blocked_comment' => null, 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/database/migrations/2026_04_24_203500_add_acceptance_criteria_data_to_tickets_table.php b/database/migrations/2026_05_19_230500_drop_acceptance_criteria_from_tickets_table.php similarity index 60% rename from database/migrations/2026_04_24_203500_add_acceptance_criteria_data_to_tickets_table.php rename to database/migrations/2026_05_19_230500_drop_acceptance_criteria_from_tickets_table.php index d74bd70..fc5a47b 100644 --- a/database/migrations/2026_04_24_203500_add_acceptance_criteria_data_to_tickets_table.php +++ b/database/migrations/2026_05_19_230500_drop_acceptance_criteria_from_tickets_table.php @@ -11,8 +11,12 @@ return new class extends Migration */ public function up(): void { + if (! Schema::hasColumn('tickets', 'acceptance_criteria')) { + return; + } + Schema::table('tickets', function (Blueprint $table): void { - $table->json('acceptance_criteria_data')->nullable(); + $table->dropColumn('acceptance_criteria'); }); } @@ -21,8 +25,12 @@ return new class extends Migration */ public function down(): void { + if (Schema::hasColumn('tickets', 'acceptance_criteria')) { + return; + } + Schema::table('tickets', function (Blueprint $table): void { - $table->dropColumn('acceptance_criteria_data'); + $table->text('acceptance_criteria')->nullable()->after('description'); }); } }; diff --git a/database/seeders/TicketSeeder.php b/database/seeders/TicketSeeder.php index 2528980..54ecfc4 100644 --- a/database/seeders/TicketSeeder.php +++ b/database/seeders/TicketSeeder.php @@ -42,7 +42,11 @@ class TicketSeeder extends Seeder ->firstOrFail(); $createProjectTicket->handle($project, [ - 'acceptance_criteria' => 'The seeded ticket should be visible in the backlog list.', + 'acceptance_criteria' => [[ + 'checked' => false, + 'indent' => 0, + 'text' => 'The seeded ticket should be visible in the backlog list.', + ]], 'assignee_user_id' => $assignee?->id, 'description' => 'Seeded backlog item for demonstrating the first ticket workflow.', 'story_points' => 3, @@ -53,7 +57,11 @@ class TicketSeeder extends Seeder ], $author); $createProjectTicket->handle($project, [ - 'acceptance_criteria' => 'Duplicate titles must be allowed for project-local ticket keys.', + 'acceptance_criteria' => [[ + 'checked' => false, + 'indent' => 0, + 'text' => 'Duplicate titles must be allowed for project-local ticket keys.', + ]], 'assignee_user_id' => null, 'description' => 'Second seeded backlog item using the same title on purpose.', 'story_points' => 5, diff --git a/resources/js/lib/acceptance-criteria.ts b/resources/js/lib/acceptance-criteria.ts index e3b5243..897562b 100644 --- a/resources/js/lib/acceptance-criteria.ts +++ b/resources/js/lib/acceptance-criteria.ts @@ -7,25 +7,19 @@ 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) => ({ + ...(typeof item.id === 'number' && item.id > 0 + ? { id: item.id } + : {}), checked: item.checked === true, indent: clampIndent(item.indent), text: item.text.trim(), @@ -33,10 +27,7 @@ export const normalizeAcceptanceCriteria = ( .filter((item) => item.text !== '') : []; - return { - summary: summary === '' ? null : summary, - items, - }; + return items; }; export const parseChecklistLines = ( @@ -69,15 +60,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 +71,7 @@ export const acceptanceCriteriaToEditorState = ( const normalized = normalizeAcceptanceCriteria(value); return { - checklist: formatChecklistLines(normalized.items), - summary: normalized.summary ?? '', + checklist: formatChecklistLines(normalized), }; }; @@ -123,6 +107,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..35b7118 100644 --- a/resources/js/pages/projects/Ticket.vue +++ b/resources/js/pages/projects/Ticket.vue @@ -5,6 +5,8 @@ import { computed, ref, watch } from 'vue'; import { acceptanceCriteriaToEditorState, buildAcceptanceCriteria, + formatChecklistLines, + normalizeAcceptanceCriteria, parseChecklistLines, } from '@/lib/acceptance-criteria'; import ProjectTicketBranchController from '@/actions/App/Http/Controllers/ProjectTicketBranchController'; @@ -111,14 +113,15 @@ const breadcrumbs: BreadcrumbItem[] = [ const acceptanceCriteriaEditorState = acceptanceCriteriaToEditorState( props.ticket.acceptance_criteria, ); -const acceptanceCriteriaSummary = ref(acceptanceCriteriaEditorState.summary); const acceptanceCriteriaChecklist = ref(acceptanceCriteriaEditorState.checklist); +const savedAcceptanceCriteria = ref( + normalizeAcceptanceCriteria(props.ticket.acceptance_criteria), +); +const isPersistingAcceptanceCriteriaToggle = ref(false); +const acceptanceCriteriaToggleError = ref(''); 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 ?? '', @@ -145,16 +148,20 @@ const form = useForm({ }); const acceptanceCriteriaPreviewItems = computed(() => - parseChecklistLines(acceptanceCriteriaChecklist.value), + savedAcceptanceCriteria.value, +); + +const hasUnsavedAcceptanceCriteriaDraft = computed( + () => + acceptanceCriteriaChecklist.value !== + formatChecklistLines(savedAcceptanceCriteria.value), ); 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] ?? '' ); @@ -343,15 +350,117 @@ const submitBranchCreation = (): void => { ); }; +const buildAcceptanceCriteriaForSubmit = () => { + const draftItems = buildAcceptanceCriteria(acceptanceCriteriaChecklist.value); + + return draftItems.map((item, index) => ({ + ...item, + ...(savedAcceptanceCriteria.value[index]?.id !== undefined + ? { id: savedAcceptanceCriteria.value[index].id } + : {}), + })); +}; + +const buildSavedTicketPayload = (acceptanceCriteria = savedAcceptanceCriteria.value) => ({ + acceptance_criteria: acceptanceCriteria, + assignee_user_id: props.ticket.assignee_user_id, + author_user_id: props.ticket.author_user_id, + blocked_comment: props.ticket.blocked_comment, + description: props.ticket.description, + field_values: (props.ticket.field_values ?? []).map((fieldValue) => ({ + ticket_field_definition_id: fieldValue.ticket_field_definition_id, + value_number: fieldValue.value_number, + value_user_id: fieldValue.value_user_id, + })), + is_blocked: props.ticket.is_blocked, + parent_ticket_id: props.ticket.parent_ticket_id, + story_points: props.ticket.story_points, + ticket_priority_id: props.ticket.ticket_priority_id, + ticket_type_id: props.ticket.ticket_type_id, + title: props.ticket.title, +}); + +const persistAcceptanceCriteriaToggle = async ( + index: number, + checked: boolean, +): Promise => { + if ( + !props.canEditTicket || + isPersistingAcceptanceCriteriaToggle.value || + hasUnsavedAcceptanceCriteriaDraft.value + ) { + return; + } + + const nextAcceptanceCriteria = savedAcceptanceCriteria.value.map((item, itemIndex) => + itemIndex === index ? { ...item, checked } : item, + ); + const previousChecklist = formatChecklistLines(savedAcceptanceCriteria.value); + const nextChecklist = formatChecklistLines(nextAcceptanceCriteria); + const csrfToken = document + .querySelector('meta[name="csrf-token"]') + ?.getAttribute('content'); + + isPersistingAcceptanceCriteriaToggle.value = true; + acceptanceCriteriaToggleError.value = ''; + + try { + const response = await fetch( + ProjectTicketController.update.url({ + project: props.project.id, + ticket: props.ticket.id, + }), + { + method: 'PATCH', + credentials: 'same-origin', + headers: { + Accept: 'text/html, application/xhtml+xml', + 'Content-Type': 'application/json', + 'X-CSRF-TOKEN': csrfToken ?? '', + 'X-Requested-With': 'XMLHttpRequest', + }, + body: JSON.stringify( + buildSavedTicketPayload(nextAcceptanceCriteria), + ), + }, + ); + + if (!response.ok) { + throw new Error('Checklist toggle update failed.'); + } + + savedAcceptanceCriteria.value = nextAcceptanceCriteria; + + if (acceptanceCriteriaChecklist.value === previousChecklist) { + acceptanceCriteriaChecklist.value = nextChecklist; + } + } catch (_error) { + acceptanceCriteriaToggleError.value = + 'Checklist status could not be saved right now.'; + } finally { + isPersistingAcceptanceCriteriaToggle.value = false; + } +}; + +const onAcceptanceCriteriaCheckboxChange = ( + index: number, + event: Event, +): void => { + const target = event.target; + + if (!(target instanceof HTMLInputElement)) { + return; + } + + void persistAcceptanceCriteriaToggle(index, target.checked); +}; + const submit = (): void => { if (!props.canEditTicket) { return; } - form.acceptance_criteria = buildAcceptanceCriteria( - acceptanceCriteriaSummary.value, - acceptanceCriteriaChecklist.value, - ); + form.acceptance_criteria = buildAcceptanceCriteriaForSubmit(); form.submit( ProjectTicketController.update({ @@ -625,20 +734,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/pages/projects/TicketCreate.vue b/resources/js/pages/projects/TicketCreate.vue index 63630cf..442bfb0 100644 --- a/resources/js/pages/projects/TicketCreate.vue +++ b/resources/js/pages/projects/TicketCreate.vue @@ -61,14 +61,10 @@ const hasTicketMetadata = props.ticketPriorities.length > 0 && props.ticketStatusOptions.length > 0; -const acceptanceCriteriaSummary = ref(''); const acceptanceCriteriaChecklist = ref(''); const form = useForm({ - acceptance_criteria: buildAcceptanceCriteria( - acceptanceCriteriaSummary.value, - acceptanceCriteriaChecklist.value, - ), + acceptance_criteria: buildAcceptanceCriteria(acceptanceCriteriaChecklist.value), assignee_user_id: '', description: '', story_points: '', @@ -87,20 +83,15 @@ 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] ?? '' ); }); const submit = (): void => { - form.acceptance_criteria = buildAcceptanceCriteria( - acceptanceCriteriaSummary.value, - acceptanceCriteriaChecklist.value, - ); + form.acceptance_criteria = buildAcceptanceCriteria(acceptanceCriteriaChecklist.value); form.submit(ProjectTicketController.store({ project: props.project.id }), { preserveScroll: true, @@ -197,18 +188,6 @@ const ticketStatusLabel = (ticketStatus: TicketStatusModel): string => {
-
- - -
-