From 31c502518572cea7d2044dc9da986d95781c9d64 Mon Sep 17 00:00:00 2001 From: Koda Date: Wed, 20 May 2026 13:46:50 +0000 Subject: [PATCH] Fix false child acceptance criteria sync --- .../AcceptanceCriteriaChecklistMapper.php | 26 +--- resources/js/lib/acceptance-criteria.ts | 5 +- .../Projects/ProjectTicketControllerTest.php | 134 +++++++++++++++++- 3 files changed, 134 insertions(+), 31 deletions(-) diff --git a/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php index 75d600d..edd9efc 100644 --- a/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php +++ b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php @@ -35,9 +35,7 @@ class AcceptanceCriteriaChecklistMapper public static function normalize(mixed $value): array { if (is_array($value)) { - return self::syncDescendantCheckedState( - self::normalizeFromArray($value), - ); + return self::normalizeFromArray($value); } return []; @@ -168,28 +166,6 @@ class AcceptanceCriteriaChecklistMapper return max(0, min(self::MAX_INDENT_LEVEL, $indent)); } - /** - * @param array $items - * @return array - */ - private static function syncDescendantCheckedState(array $items): array - { - $checkedStateByIndent = []; - - foreach ($items as $index => $item) { - $indent = $item['indent']; - - if ($indent > 0 && array_key_exists($indent - 1, $checkedStateByIndent)) { - $items[$index]['checked'] = $checkedStateByIndent[$indent - 1]; - } - - $checkedStateByIndent[$indent] = $items[$index]['checked']; - $checkedStateByIndent = array_slice($checkedStateByIndent, 0, $indent + 1, true); - } - - return $items; - } - /** * @param iterable $criteria * @return array diff --git a/resources/js/lib/acceptance-criteria.ts b/resources/js/lib/acceptance-criteria.ts index cef658f..3224291 100644 --- a/resources/js/lib/acceptance-criteria.ts +++ b/resources/js/lib/acceptance-criteria.ts @@ -75,17 +75,20 @@ export const syncAcceptanceCriteriaDescendantCheckedState = ( } const parentIndent = items[index]?.indent ?? 0; + let isWithinDescendantRange = true; return items.map((item, itemIndex) => { if (itemIndex === index) { return { ...item, checked }; } - if (itemIndex < index) { + if (itemIndex < index || !isWithinDescendantRange) { return item; } if (item.indent <= parentIndent) { + isWithinDescendantRange = false; + return item; } diff --git a/tests/Feature/Projects/ProjectTicketControllerTest.php b/tests/Feature/Projects/ProjectTicketControllerTest.php index 4101cb4..a6903ab 100644 --- a/tests/Feature/Projects/ProjectTicketControllerTest.php +++ b/tests/Feature/Projects/ProjectTicketControllerTest.php @@ -708,7 +708,7 @@ class ProjectTicketControllerTest extends TestCase ]); } - public function test_ticket_update_syncs_child_acceptance_criteria_when_parent_is_toggled(): void + public function test_ticket_update_persists_explicit_child_acceptance_criteria_toggle_states(): void { $editor = User::factory()->create(); $project = Project::factory()->create([ @@ -769,13 +769,13 @@ class ProjectTicketControllerTest extends TestCase [ 'id' => $childCriterion->id, 'text' => 'Child item', - 'checked' => false, + 'checked' => true, 'indent' => 1, ], [ 'id' => $grandchildCriterion->id, 'text' => 'Grandchild item', - 'checked' => false, + 'checked' => true, 'indent' => 2, ], ], @@ -812,13 +812,13 @@ class ProjectTicketControllerTest extends TestCase [ 'id' => $childCriterion->id, 'text' => 'Child item', - 'checked' => true, + 'checked' => false, 'indent' => 1, ], [ 'id' => $grandchildCriterion->id, 'text' => 'Grandchild item', - 'checked' => true, + 'checked' => false, 'indent' => 2, ], ], @@ -843,6 +843,130 @@ class ProjectTicketControllerTest extends TestCase ]); } + public function test_ticket_update_does_not_sync_later_indented_items_outside_the_toggled_subtree(): 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 false child test', + ]); + $loginCriterion = AcceptanceCriterion::query()->create([ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => null, + 'position' => 0, + 'text' => 'Login', + 'is_checked' => false, + ]); + $stayLoggedInCriterion = AcceptanceCriterion::query()->create([ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => $loginCriterion->id, + 'position' => 0, + 'text' => 'stay logged in button', + 'is_checked' => false, + ]); + $registerCriterion = AcceptanceCriterion::query()->create([ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => null, + 'position' => 1, + 'text' => 'Register', + 'is_checked' => true, + ]); + $passwordResetCriterion = AcceptanceCriterion::query()->create([ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => null, + 'position' => 2, + 'text' => 'Password reset', + 'is_checked' => true, + ]); + $rateLimitCriterion = AcceptanceCriterion::query()->create([ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => $passwordResetCriterion->id, + 'position' => 0, + 'text' => 'max 1x pro 10 min', + 'is_checked' => false, + ]); + + $this->syncProjectPermissions($project, $editor, ['write', 'view']); + + $this->actingAs($editor) + ->withHeaders([ + 'Accept' => 'application/json', + 'X-Requested-With' => 'XMLHttpRequest', + ]) + ->patch(route('projects.tickets.update', [$project, $ticket]), $this->updatePayload($ticket, [ + 'acceptance_criteria' => [ + [ + 'id' => $loginCriterion->id, + 'text' => 'Login', + 'checked' => true, + 'indent' => 0, + ], + [ + 'id' => $stayLoggedInCriterion->id, + 'text' => 'stay logged in button', + 'checked' => true, + 'indent' => 1, + ], + [ + 'id' => $registerCriterion->id, + 'text' => 'Register', + 'checked' => true, + 'indent' => 0, + ], + [ + 'id' => $passwordResetCriterion->id, + 'text' => 'Password reset', + 'checked' => true, + 'indent' => 0, + ], + [ + 'id' => $rateLimitCriterion->id, + 'text' => 'max 1x pro 10 min', + 'checked' => false, + 'indent' => 1, + ], + ], + ])) + ->assertOk() + ->assertJson([ + 'message' => 'CORE-1 was updated.', + 'updated' => true, + ]); + + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $loginCriterion->id, + 'is_checked' => true, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $stayLoggedInCriterion->id, + 'is_checked' => true, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $registerCriterion->id, + 'is_checked' => true, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $passwordResetCriterion->id, + 'is_checked' => true, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $rateLimitCriterion->id, + 'is_checked' => false, + ]); + } + /** * Ensure non-parent tickets cannot be assigned as a parent target. */