diff --git a/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php index 0a9c40d..5d08552 100644 --- a/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php +++ b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php @@ -16,6 +16,162 @@ class AcceptanceCriteriaChecklistMapper * @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 { @@ -39,108 +195,31 @@ class AcceptanceCriteriaChecklistMapper $grouped = $models->groupBy('parent_acceptance_criterion_id')->all(); $items = []; - self::flattenItems($items, $grouped, null, 0); + self::flattenItems($items, $grouped, null, 0, $includeIds); return $items; } /** - * @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); - - $ticket->acceptanceCriteria()->delete(); - - if ($normalized === []) { - return; - } - - $parentStack = []; - $positionsByParent = []; - - foreach ($normalized 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'); - } - /** - * @param array $items + * @param array $items * @param array> $grouped */ - private static function flattenItems(array &$items, array $grouped, ?int $parentId, int $indent): void + private static function flattenItems(array &$items, array $grouped, ?int $parentId, int $indent, bool $includeIds): void { foreach ($grouped[$parentId] ?? [] as $criterion) { - $items[] = [ + $item = [ 'checked' => $criterion->is_checked, 'indent' => $indent, 'text' => $criterion->text, ]; - self::flattenItems($items, $grouped, $criterion->id, $indent + 1); - } - } - - /** - * @param array $value - * @return array - */ - private static function normalizeFromArray(array $value): array - { - $items = []; - - foreach ($value as $rawItem) { - if (! is_array($rawItem)) { - continue; + if ($includeIds) { + $item['id'] = $criterion->id; } - $text = trim((string) Arr::get($rawItem, 'text', '')); + $items[] = $item; - if ($text === '') { - continue; - } - - $items[] = [ - 'checked' => filter_var(Arr::get($rawItem, 'checked', false), FILTER_VALIDATE_BOOLEAN), - 'indent' => self::clampIndent((int) Arr::get($rawItem, 'indent', 0)), - 'text' => $text, - ]; + self::flattenItems($items, $grouped, $criterion->id, $indent + 1, $includeIds); } - - return $items; - } - - private static function clampIndent(int $indent): int - { - return max(0, min(self::MAX_INDENT_LEVEL, $indent)); } } diff --git a/app/Http/Requests/StoreProjectTicketRequest.php b/app/Http/Requests/StoreProjectTicketRequest.php index 4f6aa82..505a267 100644 --- a/app/Http/Requests/StoreProjectTicketRequest.php +++ b/app/Http/Requests/StoreProjectTicketRequest.php @@ -49,6 +49,7 @@ class StoreProjectTicketRequest extends FormRequest return [ 'acceptance_criteria' => ['nullable', 'array'], '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], diff --git a/app/Http/Requests/UpdateProjectTicketRequest.php b/app/Http/Requests/UpdateProjectTicketRequest.php index 32f7290..1db69e5 100644 --- a/app/Http/Requests/UpdateProjectTicketRequest.php +++ b/app/Http/Requests/UpdateProjectTicketRequest.php @@ -55,6 +55,7 @@ class UpdateProjectTicketRequest extends FormRequest return [ 'acceptance_criteria' => ['nullable', 'array'], '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], diff --git a/app/Http/Resources/TicketResource.php b/app/Http/Resources/TicketResource.php index a10d8f3..faacda9 100644 --- a/app/Http/Resources/TicketResource.php +++ b/app/Http/Resources/TicketResource.php @@ -17,7 +17,7 @@ class TicketResource extends JsonResource { return [ 'id' => $this->id, - 'acceptance_criteria' => AcceptanceCriteriaChecklistMapper::fromModels( + 'acceptance_criteria' => AcceptanceCriteriaChecklistMapper::fromModelsWithIds( $this->relationLoaded('acceptanceCriteria') ? $this->acceptanceCriteria : $this->acceptanceCriteria()->get(), diff --git a/resources/js/lib/acceptance-criteria.ts b/resources/js/lib/acceptance-criteria.ts index e08fe36..897562b 100644 --- a/resources/js/lib/acceptance-criteria.ts +++ b/resources/js/lib/acceptance-criteria.ts @@ -17,6 +17,9 @@ export const normalizeAcceptanceCriteria = ( 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(), diff --git a/resources/js/pages/projects/Ticket.vue b/resources/js/pages/projects/Ticket.vue index 84753f7..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'; @@ -112,6 +114,11 @@ const acceptanceCriteriaEditorState = acceptanceCriteriaToEditorState( props.ticket.acceptance_criteria, ); 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(acceptanceCriteriaChecklist.value), @@ -141,7 +148,13 @@ const form = useForm({ }); const acceptanceCriteriaPreviewItems = computed(() => - parseChecklistLines(acceptanceCriteriaChecklist.value), + savedAcceptanceCriteria.value, +); + +const hasUnsavedAcceptanceCriteriaDraft = computed( + () => + acceptanceCriteriaChecklist.value !== + formatChecklistLines(savedAcceptanceCriteria.value), ); const acceptanceCriteriaError = computed(() => { @@ -337,12 +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(acceptanceCriteriaChecklist.value); + form.acceptance_criteria = buildAcceptanceCriteriaForSubmit(); form.submit( ProjectTicketController.update({ @@ -632,10 +750,20 @@ const formatDateTime = (value: string): string => { />

Use [ ] / [x] for checklist items. Indent with two spaces for nested steps. + Text changes are saved with the main ticket update. +

+

+ Save checklist text changes before toggling completion directly.

+
{ 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. */