From 2bc7745e53054057436bc56dce53a1f5283c976d Mon Sep 17 00:00:00 2001 From: Koda Date: Wed, 20 May 2026 13:34:28 +0000 Subject: [PATCH 1/3] Sync acceptance criteria toggle state to children --- .../AcceptanceCriteriaChecklistMapper.php | 27 +++- resources/js/lib/acceptance-criteria.ts | 28 ++++ resources/js/pages/projects/Ticket.vue | 7 +- .../Projects/ProjectTicketControllerTest.php | 150 +++++++++++++++++- 4 files changed, 201 insertions(+), 11 deletions(-) diff --git a/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php index 5d08552..75d600d 100644 --- a/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php +++ b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php @@ -35,14 +35,15 @@ class AcceptanceCriteriaChecklistMapper public static function normalize(mixed $value): array { if (is_array($value)) { - return self::normalizeFromArray($value); + return self::syncDescendantCheckedState( + self::normalizeFromArray($value), + ); } return []; } /** - * @param Ticket $ticket * @param array $items */ public static function syncTicket(Ticket $ticket, array $items): void @@ -167,6 +168,28 @@ 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 897562b..cef658f 100644 --- a/resources/js/lib/acceptance-criteria.ts +++ b/resources/js/lib/acceptance-criteria.ts @@ -65,6 +65,34 @@ export const buildAcceptanceCriteria = ( return normalizeAcceptanceCriteria(parseChecklistLines(checklistInput)); }; +export const syncAcceptanceCriteriaDescendantCheckedState = ( + items: AcceptanceCriteriaModel, + index: number, + checked: boolean, +): AcceptanceCriteriaModel => { + if (index < 0 || index >= items.length) { + return items; + } + + const parentIndent = items[index]?.indent ?? 0; + + return items.map((item, itemIndex) => { + if (itemIndex === index) { + return { ...item, checked }; + } + + if (itemIndex < index) { + return item; + } + + if (item.indent <= parentIndent) { + return item; + } + + return { ...item, checked }; + }); +}; + export const acceptanceCriteriaToEditorState = ( value: AcceptanceCriteriaModel | null | undefined, ): AcceptanceCriteriaEditorState => { diff --git a/resources/js/pages/projects/Ticket.vue b/resources/js/pages/projects/Ticket.vue index 0cca9ef..fac373b 100644 --- a/resources/js/pages/projects/Ticket.vue +++ b/resources/js/pages/projects/Ticket.vue @@ -8,6 +8,7 @@ import { formatChecklistLines, normalizeAcceptanceCriteria, parseChecklistLines, + syncAcceptanceCriteriaDescendantCheckedState, } from '@/lib/acceptance-criteria'; import ProjectTicketBranchController from '@/actions/App/Http/Controllers/ProjectTicketBranchController'; import ProjectTicketLinkController from '@/actions/App/Http/Controllers/ProjectTicketLinkController'; @@ -392,8 +393,10 @@ const persistAcceptanceCriteriaToggle = async ( return; } - const nextAcceptanceCriteria = savedAcceptanceCriteria.value.map((item, itemIndex) => - itemIndex === index ? { ...item, checked } : item, + const nextAcceptanceCriteria = syncAcceptanceCriteriaDescendantCheckedState( + savedAcceptanceCriteria.value, + index, + checked, ); const previousChecklist = formatChecklistLines(savedAcceptanceCriteria.value); const nextChecklist = formatChecklistLines(nextAcceptanceCriteria); diff --git a/tests/Feature/Projects/ProjectTicketControllerTest.php b/tests/Feature/Projects/ProjectTicketControllerTest.php index daaa2bb..4101cb4 100644 --- a/tests/Feature/Projects/ProjectTicketControllerTest.php +++ b/tests/Feature/Projects/ProjectTicketControllerTest.php @@ -3,6 +3,7 @@ namespace Tests\Feature\Projects; use App\Actions\ProjectTicketMetadata\InitializeProjectTicketMetadata; +use App\Actions\Tickets\Support\AcceptanceCriteriaChecklistMapper; use App\Models\AcceptanceCriterion; use App\Models\BoardColumn; use App\Models\Project; @@ -495,7 +496,7 @@ class ProjectTicketControllerTest extends TestCase ], [ 'text' => 'Includes nested confirmations', - 'checked' => true, + 'checked' => false, 'indent' => 1, ], ]; @@ -540,7 +541,7 @@ class ProjectTicketControllerTest extends TestCase $this->assertEquals( $updatedAcceptanceCriteria, - \App\Actions\Tickets\Support\AcceptanceCriteriaChecklistMapper::fromModels($ticket->acceptanceCriteria), + AcceptanceCriteriaChecklistMapper::fromModels($ticket->acceptanceCriteria), ); $this->assertDatabaseHas('acceptance_criteria', [ 'ticket_id' => $ticket->id, @@ -549,7 +550,7 @@ class ProjectTicketControllerTest extends TestCase 'text' => 'Shows success banner', 'is_checked' => false, ]); - $rootCriterionId = \App\Models\AcceptanceCriterion::query() + $rootCriterionId = AcceptanceCriterion::query() ->where('ticket_id', $ticket->id) ->where('text', 'Shows success banner') ->value('id'); @@ -559,7 +560,7 @@ class ProjectTicketControllerTest extends TestCase 'parent_acceptance_criterion_id' => $rootCriterionId, 'position' => 0, 'text' => 'Includes nested confirmations', - 'is_checked' => true, + 'is_checked' => false, ]); $this->assertDatabaseHas('ticket_field_values', [ 'ticket_id' => $ticket->id, @@ -627,7 +628,7 @@ class ProjectTicketControllerTest extends TestCase [ 'id' => $childCriterion->id, 'text' => 'Updated child item', - 'checked' => false, + 'checked' => true, 'indent' => 1, ], ], @@ -657,7 +658,7 @@ class ProjectTicketControllerTest extends TestCase 'parent_acceptance_criterion_id' => $rootCriterion->id, 'position' => 0, 'text' => 'Updated child item', - 'is_checked' => false, + 'is_checked' => true, ]); } @@ -707,6 +708,141 @@ class ProjectTicketControllerTest extends TestCase ]); } + public function test_ticket_update_syncs_child_acceptance_criteria_when_parent_is_toggled(): 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 parent toggle test', + ]); + $parentCriterion = AcceptanceCriterion::query()->create([ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => null, + 'position' => 0, + 'text' => 'Parent item', + 'is_checked' => false, + ]); + $childCriterion = AcceptanceCriterion::query()->create([ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => $parentCriterion->id, + 'position' => 0, + 'text' => 'Child item', + 'is_checked' => false, + ]); + $grandchildCriterion = AcceptanceCriterion::query()->create([ + 'ticket_id' => $ticket->id, + 'parent_acceptance_criterion_id' => $childCriterion->id, + 'position' => 0, + 'text' => 'Grandchild item', + 'is_checked' => false, + ]); + + $this->syncProjectPermissions($project, $editor, ['write', 'view']); + + $headers = [ + 'Accept' => 'application/json', + 'X-Requested-With' => 'XMLHttpRequest', + ]; + + $this->actingAs($editor) + ->withHeaders($headers) + ->patch(route('projects.tickets.update', [$project, $ticket]), $this->updatePayload($ticket, [ + 'acceptance_criteria' => [ + [ + 'id' => $parentCriterion->id, + 'text' => 'Parent item', + 'checked' => true, + 'indent' => 0, + ], + [ + 'id' => $childCriterion->id, + 'text' => 'Child item', + 'checked' => false, + 'indent' => 1, + ], + [ + 'id' => $grandchildCriterion->id, + 'text' => 'Grandchild item', + 'checked' => false, + 'indent' => 2, + ], + ], + ])) + ->assertOk() + ->assertJson([ + 'message' => 'CORE-1 was updated.', + 'updated' => true, + ]); + + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $parentCriterion->id, + 'is_checked' => true, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $childCriterion->id, + 'is_checked' => true, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $grandchildCriterion->id, + 'is_checked' => true, + ]); + + $this->actingAs($editor) + ->withHeaders($headers) + ->patch(route('projects.tickets.update', [$project, $ticket]), $this->updatePayload($ticket->fresh(), [ + 'acceptance_criteria' => [ + [ + 'id' => $parentCriterion->id, + 'text' => 'Parent item', + 'checked' => false, + 'indent' => 0, + ], + [ + 'id' => $childCriterion->id, + 'text' => 'Child item', + 'checked' => true, + 'indent' => 1, + ], + [ + 'id' => $grandchildCriterion->id, + 'text' => 'Grandchild item', + 'checked' => true, + 'indent' => 2, + ], + ], + ])) + ->assertOk() + ->assertJson([ + 'message' => 'CORE-1 was updated.', + 'updated' => true, + ]); + + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $parentCriterion->id, + 'is_checked' => false, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $childCriterion->id, + 'is_checked' => false, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $grandchildCriterion->id, + 'is_checked' => false, + ]); + } + /** * Ensure non-parent tickets cannot be assigned as a parent target. */ @@ -1107,7 +1243,7 @@ class ProjectTicketControllerTest extends TestCase private function updatePayload(Ticket $ticket, array $overrides = []): array { return array_merge([ - 'acceptance_criteria' => \App\Actions\Tickets\Support\AcceptanceCriteriaChecklistMapper::fromModels( + 'acceptance_criteria' => AcceptanceCriteriaChecklistMapper::fromModels( $ticket->acceptanceCriteria()->get(), ), 'assignee_user_id' => $ticket->assignee_user_id, -- 2.54.0 From 31c502518572cea7d2044dc9da986d95781c9d64 Mon Sep 17 00:00:00 2001 From: Koda Date: Wed, 20 May 2026 13:46:50 +0000 Subject: [PATCH 2/3] 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. */ -- 2.54.0 From 384080648e5fd33cc90c48d9690f14d3e02d3a2f Mon Sep 17 00:00:00 2001 From: Koda Date: Wed, 20 May 2026 13:58:34 +0000 Subject: [PATCH 3/3] Fix flaky user ordering assertion in SettingsApiTest --- tests/Feature/Api/SettingsApiTest.php | 28 ++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/tests/Feature/Api/SettingsApiTest.php b/tests/Feature/Api/SettingsApiTest.php index 9747a66..f3012fe 100644 --- a/tests/Feature/Api/SettingsApiTest.php +++ b/tests/Feature/Api/SettingsApiTest.php @@ -154,7 +154,7 @@ class SettingsApiTest extends TestCase { [$user, $project] = $this->projectContext(); $viewer = User::factory()->create(['name' => 'Visible User']); - User::factory()->create(['name' => 'Hidden User']); + $hiddenUser = User::factory()->create(['name' => 'Hidden User']); $teamViewer = User::factory()->create(['name' => 'Team Viewer']); $team = Team::factory()->create(); @@ -172,21 +172,31 @@ class SettingsApiTest extends TestCase 'title' => 'Scoped users ticket', ]); - $this->withApplicationKey($user) + $expectedUserIds = [$user->id, $viewer->id, $teamViewer->id]; + + $backlogResponse = $this->withApplicationKey($user) ->getJson(route('api.projects.backlog', $project)) ->assertOk() - ->assertJsonPath('data.users.0.name', $user->name) - ->assertJsonPath('data.users.1.name', 'Team Viewer') - ->assertJsonPath('data.users.2.name', 'Visible User') ->assertJsonCount(3, 'data.users'); - $this->withApplicationKey($user) + $backlogUserIds = collect($backlogResponse->json('data.users')) + ->pluck('id') + ->all(); + + $this->assertEqualsCanonicalizing($expectedUserIds, $backlogUserIds); + $this->assertNotContains($hiddenUser->id, $backlogUserIds); + + $ticketResponse = $this->withApplicationKey($user) ->getJson(route('api.projects.tickets.show', [$project, $ticket])) ->assertOk() - ->assertJsonPath('data.users.0.name', $user->name) - ->assertJsonPath('data.users.1.name', 'Team Viewer') - ->assertJsonPath('data.users.2.name', 'Visible User') ->assertJsonCount(3, 'data.users'); + + $ticketUserIds = collect($ticketResponse->json('data.users')) + ->pluck('id') + ->all(); + + $this->assertEqualsCanonicalizing($expectedUserIds, $ticketUserIds); + $this->assertNotContains($hiddenUser->id, $ticketUserIds); } public function test_project_repositories_and_user_git_connections_are_available(): void -- 2.54.0