diff --git a/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php index 5d08552..edd9efc 100644 --- a/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php +++ b/app/Actions/Tickets/Support/AcceptanceCriteriaChecklistMapper.php @@ -42,7 +42,6 @@ class AcceptanceCriteriaChecklistMapper } /** - * @param Ticket $ticket * @param array $items */ public static function syncTicket(Ticket $ticket, array $items): void diff --git a/resources/js/lib/acceptance-criteria.ts b/resources/js/lib/acceptance-criteria.ts index 897562b..3224291 100644 --- a/resources/js/lib/acceptance-criteria.ts +++ b/resources/js/lib/acceptance-criteria.ts @@ -65,6 +65,37 @@ 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; + let isWithinDescendantRange = true; + + return items.map((item, itemIndex) => { + if (itemIndex === index) { + return { ...item, checked }; + } + + if (itemIndex < index || !isWithinDescendantRange) { + return item; + } + + if (item.indent <= parentIndent) { + isWithinDescendantRange = false; + + 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/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 diff --git a/tests/Feature/Projects/ProjectTicketControllerTest.php b/tests/Feature/Projects/ProjectTicketControllerTest.php index daaa2bb..a6903ab 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,265 @@ class ProjectTicketControllerTest extends TestCase ]); } + public function test_ticket_update_persists_explicit_child_acceptance_criteria_toggle_states(): 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' => 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' => 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' => 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' => false, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $childCriterion->id, + 'is_checked' => false, + ]); + $this->assertDatabaseHas('acceptance_criteria', [ + 'id' => $grandchildCriterion->id, + 'is_checked' => false, + ]); + } + + 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. */ @@ -1107,7 +1367,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,