added status restriction

2026-04-14 19:07:15 +02:00
parent 9927a91327
commit fb10ecd535
12 changed files with 570 additions and 69 deletions
@@ -51,9 +51,9 @@ class TicketStatusTest extends TestCase
    }

    /**
-     * Ensure new statuses can move to each other through the transition relation.
+     * Ensure new statuses do not allow moves until transitions are configured.
     */
-    public function test_ticket_statuses_create_allowed_transition_relations(): void
+    public function test_ticket_statuses_do_not_create_allowed_transition_relations_by_default(): void
    {
        $project = Project::factory()->create();
        $backlogStatus = TicketStatus::factory()->for($project)->create([
@@ -65,8 +65,38 @@ class TicketStatusTest extends TestCase
            'position' => 2,
        ]);

-        $this->assertTrue($backlogStatus->allowedTargetStatuses()->whereKey($readyStatus->id)->exists());
-        $this->assertTrue($readyStatus->allowedTargetStatuses()->whereKey($backlogStatus->id)->exists());
+        $this->assertFalse($backlogStatus->allowedTargetStatuses()->whereKey($readyStatus->id)->exists());
+        $this->assertFalse($readyStatus->allowedTargetStatuses()->whereKey($backlogStatus->id)->exists());
+        $this->assertCount(0, $backlogStatus->allowedTargetStatuses);
+        $this->assertCount(0, $readyStatus->allowedTargetStatuses);
+    }
+
+    /**
+     * Ensure allowed status moves are controlled by the transition relation.
+     */
+    public function test_ticket_statuses_sync_allowed_transition_relations(): void
+    {
+        $project = Project::factory()->create();
+        $backlogStatus = TicketStatus::factory()->for($project)->create([
+            'name' => 'Backlog',
+            'position' => 1,
+        ]);
+        $readyStatus = TicketStatus::factory()->for($project)->create([
+            'name' => 'Ready',
+            'position' => 2,
+        ]);
+
+        $backlogStatus->allowedTargetStatuses()->sync([$readyStatus->id]);
+
+        $this->assertSame(
+            [$readyStatus->id],
+            $backlogStatus->allowedTargetStatuses()->get()->pluck('id')->all(),
+        );
+        $this->assertFalse($readyStatus->allowedTargetStatuses()->whereKey($backlogStatus->id)->exists());
+
+        $backlogStatus->allowedTargetStatuses()->sync([]);
+
+        $this->assertCount(0, $backlogStatus->allowedTargetStatuses()->get());
    }

    /**
@@ -119,6 +119,7 @@ class ProjectBoardControllerTest extends TestCase
        ]);

        $this->syncProjectPermissions($project, $user, ['edit', 'view']);
+        $this->allowMove($backlogStatus, $todoStatus);

        $this->actingAs($user)
            ->patch(route('projects.tickets.status.update', [$project, $ticket]), [
@@ -180,6 +181,7 @@ class ProjectBoardControllerTest extends TestCase
        ]);

        $this->syncProjectPermissions($project, $user, ['edit', 'view']);
+        $this->allowMove($todoStatus, $doneStatus);

        $this->actingAs($user)
            ->patch(route('projects.tickets.status.update', [$project, $ticket]), [
@@ -237,6 +239,7 @@ class ProjectBoardControllerTest extends TestCase
        ]);

        $this->syncProjectPermissions($project, $user, ['edit', 'view']);
+        $this->allowMove($doingStatus, $backlogStatus);

        $this->actingAs($user)
            ->patch(route('projects.tickets.status.update', [$project, $ticket]), [
@@ -328,7 +331,6 @@ class ProjectBoardControllerTest extends TestCase
        ]);
        $todoStatus = $this->createColumnStatus($todoColumn);
        $backlogStatus = $this->backlogStatus($project);
-        $backlogStatus->allowedTargetStatuses()->detach($todoStatus->id);

        $priority = TicketPriority::factory()->for($project)->create([
            'name' => 'High',
@@ -364,6 +366,126 @@ class ProjectBoardControllerTest extends TestCase
        $this->assertDatabaseCount('ticket_worklog_entries', 0);
    }

+    /**
+     * Ensure configured status moves can model a restricted workflow.
+     */
+    public function test_users_with_edit_permission_can_follow_configured_status_transitions(): void
+    {
+        $user = User::factory()->create();
+        $project = Project::factory()->create([
+            'key' => 'CORE',
+        ]);
+
+        app(InitializeProjectTicketMetadata::class)->handle($project);
+
+        $progressColumn = BoardColumn::factory()->for($project)->create([
+            'name' => 'Progress',
+            'position' => 1,
+        ]);
+        $reviewColumn = BoardColumn::factory()->for($project)->create([
+            'name' => 'Review',
+            'position' => 2,
+        ]);
+        $doneColumn = BoardColumn::factory()->for($project)->create([
+            'name' => 'Done',
+            'position' => 3,
+        ]);
+        $progressStatus = $this->createColumnStatus($progressColumn);
+        $reviewStatus = $this->createColumnStatus($reviewColumn);
+        $doneStatus = $this->createColumnStatus($doneColumn);
+        $this->allowMove($progressStatus, $reviewStatus);
+        $this->allowMove($reviewStatus, $doneStatus);
+        $this->allowMove($reviewStatus, $progressStatus);
+
+        $priority = TicketPriority::factory()->for($project)->create([
+            'name' => 'High',
+            'position' => 2,
+        ]);
+
+        $ticket = Ticket::factory()->create([
+            'project_id' => $project->id,
+            'sequence_number' => 1,
+            'ticket_key' => 'CORE-1',
+            'ticket_priority_id' => $priority->id,
+            'ticket_status_id' => $progressStatus->id,
+            'ticket_type_id' => $project->ticketTypes()->firstOrFail()->id,
+            'title' => 'Board ticket',
+        ]);
+
+        $this->syncProjectPermissions($project, $user, ['edit', 'view']);
+
+        $this->actingAs($user)
+            ->patch(route('projects.tickets.status.update', [$project, $ticket]), [
+                'redirect_to' => 'board',
+                'ticket_status_id' => $reviewStatus->id,
+            ])
+            ->assertRedirect(route('projects.board.index', $project))
+            ->assertSessionHas('status', 'CORE-1 moved to Review.');
+
+        $ticket->refresh();
+
+        $this->actingAs($user)
+            ->patch(route('projects.tickets.status.update', [$project, $ticket]), [
+                'redirect_to' => 'board',
+                'ticket_status_id' => $progressStatus->id,
+            ])
+            ->assertRedirect(route('projects.board.index', $project))
+            ->assertSessionHas('status', 'CORE-1 moved to Progress.');
+
+        $ticket->refresh();
+
+        $this->actingAs($user)
+            ->from(route('projects.board.index', $project))
+            ->patch(route('projects.tickets.status.update', [$project, $ticket]), [
+                'redirect_to' => 'board',
+                'ticket_status_id' => $doneStatus->id,
+            ])
+            ->assertRedirect(route('projects.board.index', $project))
+            ->assertSessionHasErrors('ticket_status_id');
+
+        $this->assertDatabaseHas('tickets', [
+            'id' => $ticket->id,
+            'ticket_status_id' => $progressStatus->id,
+        ]);
+
+        $this->assertDatabaseCount('ticket_worklog_entries', 2);
+
+        $this->actingAs($user)
+            ->patch(route('projects.tickets.status.update', [$project, $ticket]), [
+                'redirect_to' => 'board',
+                'ticket_status_id' => $reviewStatus->id,
+            ])
+            ->assertRedirect(route('projects.board.index', $project));
+
+        $ticket->refresh();
+
+        $this->actingAs($user)
+            ->patch(route('projects.tickets.status.update', [$project, $ticket]), [
+                'redirect_to' => 'board',
+                'ticket_status_id' => $doneStatus->id,
+            ])
+            ->assertRedirect(route('projects.board.index', $project))
+            ->assertSessionHas('status', 'CORE-1 moved to Done.');
+
+        $ticket->refresh();
+
+        $this->actingAs($user)
+            ->from(route('projects.board.index', $project))
+            ->patch(route('projects.tickets.status.update', [$project, $ticket]), [
+                'redirect_to' => 'board',
+                'ticket_status_id' => $reviewStatus->id,
+            ])
+            ->assertRedirect(route('projects.board.index', $project))
+            ->assertSessionHasErrors('ticket_status_id');
+
+        $this->assertDatabaseHas('tickets', [
+            'id' => $ticket->id,
+            'ticket_status_id' => $doneStatus->id,
+        ]);
+
+        $this->assertDatabaseCount('ticket_worklog_entries', 4);
+    }
+
    private function backlogStatus(Project $project): TicketStatus
    {
        return $project->ticketStatuses()
@@ -381,4 +503,9 @@ class ProjectBoardControllerTest extends TestCase
                'position' => 1,
            ]);
    }
+
+    private function allowMove(TicketStatus $sourceStatus, TicketStatus $targetStatus): void
+    {
+        $sourceStatus->allowedTargetStatuses()->syncWithoutDetaching([$targetStatus->id]);
+    }
 }
@@ -40,7 +40,8 @@ class ProjectBoardSettingsControllerTest extends TestCase
                ->component('projects/settings/Board')
                ->where('project.id', $project->id)
                ->where('backlogTicketStatuses.0.name', 'Backlog')
-                ->where('boardColumns.0.name', 'Todo'),
+                ->where('boardColumns.0.name', 'Todo')
+                ->where('ticketStatuses.0.name', 'Backlog'),
            );
    }

@@ -205,7 +206,7 @@ class ProjectBoardSettingsControllerTest extends TestCase

        $this->assertSame($boardColumn->id, $newStatus->board_column_id);
        $this->assertSame('ready-for-qa', $newStatus->slug);
-        $this->assertTrue($defaultStatus->allowedTargetStatuses()->whereKey($newStatus->id)->exists());
+        $this->assertFalse($defaultStatus->allowedTargetStatuses()->whereKey($newStatus->id)->exists());

        $this->actingAs($user)
            ->patch(route('projects.settings.board.statuses.update', [$project, $newStatus]), [
@@ -238,6 +239,94 @@ class ProjectBoardSettingsControllerTest extends TestCase
        ]);
    }

+    /**
+     * Ensure authorized users can configure the allowed move targets for a status.
+     */
+    public function test_users_with_edit_permission_can_update_ticket_status_transitions(): void
+    {
+        $user = User::factory()->create();
+        $project = Project::factory()->create([
+            'key' => 'CORE',
+        ]);
+        $this->syncProjectPermissions($project, $user, ['edit']);
+
+        $sourceStatus = TicketStatus::factory()->for($project)->create([
+            'name' => 'Progress',
+            'position' => 1,
+        ]);
+        $reviewStatus = TicketStatus::factory()->for($project)->create([
+            'name' => 'Review',
+            'position' => 2,
+        ]);
+        $doneStatus = TicketStatus::factory()->for($project)->create([
+            'name' => 'Done',
+            'position' => 3,
+        ]);
+
+        $this->actingAs($user)
+            ->patch(route('projects.settings.board.statuses.transitions.update', [$project, $sourceStatus]), [
+                'allowed_ticket_status_ids' => [$reviewStatus->id, $doneStatus->id],
+            ])
+            ->assertRedirect(route('projects.settings.board', $project))
+            ->assertSessionHas('status', 'Progress move targets were updated.');
+
+        $this->assertEqualsCanonicalizing(
+            [$reviewStatus->id, $doneStatus->id],
+            $sourceStatus->allowedTargetStatuses()->get()->pluck('id')->all(),
+        );
+
+        $this->actingAs($user)
+            ->patch(route('projects.settings.board.statuses.transitions.update', [$project, $sourceStatus]), [])
+            ->assertRedirect(route('projects.settings.board', $project))
+            ->assertSessionHas('status', 'Progress move targets were updated.');
+
+        $this->assertCount(0, $sourceStatus->allowedTargetStatuses()->get());
+    }
+
+    /**
+     * Ensure transition updates only accept targets from the same project and never the source status itself.
+     */
+    public function test_ticket_status_transition_updates_reject_foreign_and_self_targets(): void
+    {
+        $user = User::factory()->create();
+        $project = Project::factory()->create([
+            'key' => 'CORE',
+        ]);
+        $otherProject = Project::factory()->create([
+            'key' => 'EXT',
+        ]);
+        $this->syncProjectPermissions($project, $user, ['edit']);
+
+        $sourceStatus = TicketStatus::factory()->for($project)->create([
+            'name' => 'Progress',
+            'position' => 1,
+        ]);
+        $foreignStatus = TicketStatus::factory()->for($otherProject)->create([
+            'name' => 'External review',
+            'position' => 1,
+        ]);
+
+        $this->actingAs($user)
+            ->from(route('projects.settings.board', $project))
+            ->patch(route('projects.settings.board.statuses.transitions.update', [$project, $sourceStatus]), [
+                'allowed_ticket_status_ids' => [$foreignStatus->id],
+            ])
+            ->assertRedirect(route('projects.settings.board', $project))
+            ->assertSessionHasErrors('allowed_ticket_status_ids.0');
+
+        $this->assertCount(0, $sourceStatus->allowedTargetStatuses()->get());
+
+        $this->actingAs($user)
+            ->from(route('projects.settings.board', $project))
+            ->patch(route('projects.settings.board.statuses.transitions.update', [$project, $sourceStatus]), [
+                'allowed_ticket_status_ids' => [$sourceStatus->id],
+            ])
+            ->assertRedirect(route('projects.settings.board', $project))
+            ->assertSessionHasErrors('allowed_ticket_status_ids');
+
+        $this->assertCount(0, $sourceStatus->allowedTargetStatuses()->get());
+    }
+
    /**
     * Ensure the last status in a column or backlog cannot be deleted.
     */