diff --git a/resources/js/components/projects/WorkflowTransitionMatrix.vue b/resources/js/components/projects/WorkflowTransitionMatrix.vue index 51176d3..070575d 100644 --- a/resources/js/components/projects/WorkflowTransitionMatrix.vue +++ b/resources/js/components/projects/WorkflowTransitionMatrix.vue @@ -1,91 +1,282 @@ diff --git a/resources/js/locales/en.ts b/resources/js/locales/en.ts index 40a0200..e7feea4 100644 --- a/resources/js/locales/en.ts +++ b/resources/js/locales/en.ts @@ -46,6 +46,22 @@ export const en = { 'workflow.transitions': 'Status moves', 'workflow.transitionsDescription': 'Choose exactly where each status is allowed to move next.', + 'workflow.sourceStatus': 'From status', + 'workflow.directionHelp': + 'Rules apply in one direction. Select another source to configure the return move.', + 'workflow.targetsFrom': 'Allowed destinations from {name}', + 'workflow.targetHelp': + 'Checked destinations are allowed. Unchecked destinations are forbidden. Save to apply these rules to ticket moves and board drop targets.', + 'workflow.allowed': 'Allowed', + 'workflow.notAllowed': 'Not allowed', + 'workflow.noMovesHelp': + 'With no destinations selected, tickets cannot leave this status after you save.', + 'workflow.noStatuses': + 'Add a status to configure its allowed destinations.', + 'workflow.unsavedMoves': 'Unsaved changes', + 'workflow.savedMoves': 'Saved destinations: {count}', + 'workflow.resetMoves': 'Reset changes', + 'workflow.saveMoves': 'Save moves', 'workflow.noTargets': 'No other statuses exist yet.', 'workflow.confirmDelete': 'Delete', 'security.title': 'Security settings', diff --git a/tests/Feature/Projects/ProjectBoardSettingsControllerTest.php b/tests/Feature/Projects/ProjectBoardSettingsControllerTest.php index 37ca35f..c1d0332 100644 --- a/tests/Feature/Projects/ProjectBoardSettingsControllerTest.php +++ b/tests/Feature/Projects/ProjectBoardSettingsControllerTest.php @@ -298,6 +298,79 @@ class ProjectBoardSettingsControllerTest extends TestCase $this->assertCount(0, $sourceStatus->allowedTargetStatuses()->get()); } + /** + * Ensure saved outgoing targets reach both consumers without changing the reverse direction. + */ + public function test_saved_transition_subsets_and_empty_targets_are_reflected_in_ticket_and_board_responses(): void + { + $user = User::factory()->create(); + $project = Project::factory()->create(['key' => 'CORE']); + $this->syncProjectPermissions($project, $user, ['edit']); + $column = BoardColumn::factory()->for($project)->create(['position' => 1]); + $sourceStatus = TicketStatus::factory()->for($project)->for($column)->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, + ]); + $ticket = Ticket::factory()->for($project)->create(['ticket_status_id' => $sourceStatus->id]); + $sourceStatus->allowedTargetStatuses()->sync([$reviewStatus->id, $doneStatus->id]); + $reviewStatus->allowedTargetStatuses()->sync([$sourceStatus->id]); + + $this->actingAs($user) + ->patch(route('projects.settings.board.statuses.transitions.update', [$project, $sourceStatus]), [ + 'allowed_ticket_status_ids' => [$reviewStatus->id], + ]) + ->assertRedirect(route('projects.settings.board', $project)) + ->assertSessionHasNoErrors(); + + $this->assertSame([$reviewStatus->id], $sourceStatus->allowedTargetStatuses()->get()->modelKeys()); + $this->assertSame([$sourceStatus->id], $reviewStatus->allowedTargetStatuses()->get()->modelKeys()); + + $this->withApplicationKey($user) + ->getJson(route('api.projects.tickets.status-targets', [$project, $ticket])) + ->assertOk() + ->assertJsonPath('data.current_status.id', $sourceStatus->id) + ->assertJsonCount(1, 'data.target_statuses') + ->assertJsonPath('data.target_statuses.0.id', $reviewStatus->id); + + $this->get(route('projects.board.index', $project)) + ->assertOk() + ->assertInertia(fn (Assert $page) => $page + ->component('projects/Board') + ->where('boardColumns.0.tickets.0.id', $ticket->id) + ->has('boardColumns.0.tickets.0.ticket_status.allowed_target_statuses', 1) + ->where('boardColumns.0.tickets.0.ticket_status.allowed_target_statuses.0.id', $reviewStatus->id), + ); + + $this->patch(route('projects.settings.board.statuses.transitions.update', [$project, $sourceStatus]), [ + 'allowed_ticket_status_ids' => [], + ]) + ->assertRedirect(route('projects.settings.board', $project)) + ->assertSessionHasNoErrors(); + + $this->assertCount(0, $sourceStatus->allowedTargetStatuses()->get()); + $this->assertSame([$sourceStatus->id], $reviewStatus->allowedTargetStatuses()->get()->modelKeys()); + + $this->getJson(route('api.projects.tickets.status-targets', [$project, $ticket])) + ->assertOk() + ->assertJsonPath('data.target_statuses', []); + + $this->get(route('projects.board.index', $project)) + ->assertOk() + ->assertInertia(fn (Assert $page) => $page + ->component('projects/Board') + ->where('boardColumns.0.tickets.0.id', $ticket->id) + ->has('boardColumns.0.tickets.0.ticket_status.allowed_target_statuses', 0), + ); + } + /** * Ensure transition updates only accept targets from the same project and never the source status itself. */ diff --git a/tests/Fixtures/project-board-settings-browser.mjs b/tests/Fixtures/project-board-settings-browser.mjs index f0a4511..d32801f 100644 --- a/tests/Fixtures/project-board-settings-browser.mjs +++ b/tests/Fixtures/project-board-settings-browser.mjs @@ -254,23 +254,6 @@ try { .getByText('Building', { exact: true }) .waitFor(); assert.equal(await button('Edit column In progress').count(), 0); - await page.locator('summary').filter({ hasText: 'Status moves' }).click(); - const transition = page.locator('details form').first(); - const checkbox = transition.getByRole('checkbox').first(); - await checkbox.check(); - const move = page.waitForRequest((request) => request.method() === 'POST'); - await transition.getByRole('button', { name: 'Save', exact: true }).click(); - assert.equal( - new URL((await move).url()).pathname, - '/projects/1/settings/board/statuses/1/transitions', - ); - assert.equal( - new URL((await move).url()).searchParams.get('_method'), - 'PATCH', - ); - assert.deepEqual((await move).postDataJSON(), { - allowed_ticket_status_ids: [await checkbox.getAttribute('value')], - }); assert.deepEqual(errors, []); console.log( 'Workflow browser checks passed: CRUD payloads, server refresh, order, deletion protection, validation, retries, dirty forms, keyboard, responsive widths and both themes.', diff --git a/tests/Fixtures/project-board-settings.mjs b/tests/Fixtures/project-board-settings.mjs index 73b06d3..ddb19ca 100644 --- a/tests/Fixtures/project-board-settings.mjs +++ b/tests/Fixtures/project-board-settings.mjs @@ -35,6 +35,7 @@ function initial() { const ready = makeStatus(2, 'Ready', 10); const active = makeStatus(3, 'Building', 20); const review = makeStatus(4, 'Review', 20, 2); + backlog.allowed_target_statuses = [ready]; const columns = [ { id: 10, name: 'Todo', position: 1, ticket_statuses: [ready] }, { @@ -134,13 +135,17 @@ if (serve) { return; } if (result === 'validation') { - errors = - request.method === 'DELETE' - ? { - ticket_status_id: - 'This group needs at least one status.', - } - : { name: 'Choose a different name.' }; + errors = url.pathname.endsWith('/transitions') + ? { + 'allowed_ticket_status_ids.0': + 'Choose a destination from this project.', + } + : request.method === 'DELETE' + ? { + ticket_status_id: + 'This group needs at least one status.', + } + : { name: 'Choose a different name.' }; } else if (result === 'position') { errors = { position: 'The position must be at least 1.' }; } else { @@ -188,7 +193,16 @@ if (serve) { column.position = index + 1; }); } else if (url.pathname.endsWith('/transitions')) { - // The browser assertion checks the preserved transition endpoint and payload. + const sourceId = Number(segments.at(-2)); + const statuses = [ + ...data.backlogTicketStatuses, + ...columns.flatMap((column) => column.ticket_statuses), + ]; + statuses.find( + (status) => status.id === sourceId, + ).allowed_target_statuses = ( + submitted.allowed_ticket_status_ids ?? [] + ).map((id) => ({ id: Number(id) })); } else if (request.method === 'POST') { const group = submitted.board_column_id === null @@ -294,6 +308,16 @@ if (serve) { html.indexOf('data-column-id="10"') < html.indexOf('data-column-id="20"'), ); + assert.match(html, /From status/); + assert.match(html, /Saved destinations: 1/); + assert.match( + await render({ ticketStatuses: [] }), + /Add a status to configure/, + ); + assert.match( + await render({ ticketStatuses: [makeStatus(1, 'Backlog', null)] }), + /No other statuses exist yet/, + ); const buttons = html.match(/]*>[\s\S]*?<\/button>/g); for (const name of ['Backlog', 'Ready']) { assert.match( diff --git a/tests/Fixtures/workflow-transitions-browser.mjs b/tests/Fixtures/workflow-transitions-browser.mjs new file mode 100644 index 0000000..0be8edc --- /dev/null +++ b/tests/Fixtures/workflow-transitions-browser.mjs @@ -0,0 +1,164 @@ +import assert from 'node:assert/strict'; +import process from 'node:process'; +const { chromium } = await import( + process.env.PLAYWRIGHT_MODULE ?? 'playwright' +); +const browser = await chromium.launch({ channel: 'chrome', headless: true }); +const base = 'http://127.0.0.1:4188'; +const context = await browser.newContext(); +const page = await context.newPage(); +page.setDefaultTimeout(10000); +const errors = []; +page.on('pageerror', (error) => errors.push(error.message)); +const editor = page.getByRole('region', { name: 'Status moves', exact: true }); +const source = () => editor.getByLabel('From status', { exact: true }); +const check = (name) => + editor.getByRole('checkbox', { name: new RegExp(name) }); +const button = (name) => editor.getByRole('button', { name, exact: true }); +const state = (name) => editor.locator('[data-form-state="' + name + '"]'); +const result = (value) => + context.addCookies([{ name: 'workflow_test_result', value, url: base }]); +const visit = async (query = '?reset') => { + await result('success'); + await page.goto(base + '/projects/1/settings/board' + query); + await source().waitFor(); +}; +const noOverflow = async () => + assert.equal( + await page.evaluate( + () => document.documentElement.scrollWidth <= window.innerWidth, + ), + true, + ); +const save = async (id, targets) => { + const sent = page.waitForRequest((request) => request.method() === 'PATCH'); + await button('Save moves').click(); + const request = await sent; + assert.equal( + new URL(request.url()).pathname, + '/projects/1/settings/board/statuses/' + id + '/transitions', + ); + assert.deepEqual(request.postDataJSON(), { + allowed_ticket_status_ids: targets, + }); +}; +try { + for (const width of [1440, 768, 390, 320]) { + await page.setViewportSize({ width, height: 1000 }); + for (const theme of ['light', 'dark']) { + await page.emulateMedia({ colorScheme: theme }); + await visit(); + assert.equal(await check('Ready').isChecked(), true); + assert.equal(await check('Building').isChecked(), false); + assert.equal(await check('Backlog').count(), 0); + assert.equal(await button('Save moves').isDisabled(), true); + await check('Building').focus(); + await page.keyboard.press('Space'); + assert.equal(await check('Building').isChecked(), true); + await editor + .getByText('Unsaved changes', { exact: true }) + .waitFor(); + await noOverflow(); + await button('Reset changes').click(); + assert.equal(await check('Building').isChecked(), false); + assert.equal(await check('Ready').isChecked(), true); + if ([1440, 390].includes(width)) { + await editor.scrollIntoViewIfNeeded(); + await page.screenshot({ + path: '/private/tmp/KAN-41-' + width + '-' + theme + '.png', + }); + } + } + } + await visit(); + await check('Ready').uncheck(); + await check('Review').check(); + await result('validation'); + await save(1, [4]); + await state('validation').waitFor(); + await editor + .getByText('Choose a destination from this project.', { exact: true }) + .waitFor(); + assert.equal(await check('Review').isChecked(), true); + assert.equal(await check('Ready').isChecked(), false); + assert.equal( + await editor.getByRole('group').getAttribute('aria-invalid'), + 'true', + ); + await result('server'); + await save(1, [4]); + await state('server').waitFor(); + await result('success'); + await button('Try again').click(); + await state('success').waitFor(); + assert.equal(await button('Save moves').isDisabled(), true); + assert.equal(await check('Review').isChecked(), true); + await page.reload(); + await source().waitFor(); + assert.equal(await check('Review').isChecked(), true); + assert.equal(await check('Ready').isChecked(), false); + await source().selectOption('4'); + assert.equal(await check('Backlog').isChecked(), false); + assert.equal(await check('Review').count(), 0); + await check('Backlog').check(); + page.once('dialog', (prompt) => prompt.dismiss()); + await source().selectOption('1'); + assert.equal(await source().inputValue(), '4'); + assert.equal(await check('Backlog').isChecked(), true); + page.once('dialog', (prompt) => prompt.accept()); + await source().selectOption('1'); + assert.equal(await source().inputValue(), '1'); + assert.equal(await check('Review').isChecked(), true); + await check('Review').uncheck(); + await editor.getByText(/With no destinations selected/).waitFor(); + await save(1, []); + await state('success').waitFor(); + await page.reload(); + await source().waitFor(); + assert.equal(await editor.locator('input:checked').count(), 0); + await editor.getByText('Saved destinations: 0', { exact: true }).waitFor(); + // A disconnected request retains the draft and supports a single retry. + await check('Building').check(); + await page.route('**/statuses/1/transitions', (route) => + route.abort('failed'), + ); + await save(1, [3]); + await state('network').waitFor(); + assert.equal(await check('Building').isChecked(), true); + await page.unroute('**/statuses/1/transitions'); + await button('Try again').click(); + await state('success').waitFor(); + // Pending saves disable inputs, source changes and duplicate submits. + let release; + const blocked = new Promise((resolve) => { + release = resolve; + }); + await page.route('**/statuses/1/transitions', async (route) => { + await blocked; + await route.continue(); + }); + await check('Review').check(); + await save(1, [3, 4]); + await state('loading').waitFor(); + assert.equal(await source().isDisabled(), true); + assert.equal(await check('Building').isDisabled(), true); + assert.equal( + await editor.locator('button[type=submit]').isDisabled(), + true, + ); + release(); + await state('success').waitFor(); + await page.unroute('**/statuses/1/transitions'); + await visit('?reset&empty'); + await editor + .getByText('No other statuses exist yet.', { exact: true }) + .waitFor(); + assert.equal(await editor.getByRole('checkbox').count(), 0); + await noOverflow(); + assert.deepEqual(errors, []); + console.log( + 'Transition browser checks passed: directed rules, persisted selection, clearing all targets, reset, source-change protection, validation/network/server errors, retry, pending saves, keyboard, responsive widths and both themes.', + ); +} finally { + await browser.close(); +}