mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 13:39:44 +02:00
feat(editor): Add bulk state actions to the migration rule detail (no-changelog) (#40780)
Co-authored-by: Michael Siega <michael.siega@n8n.io> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
parent
27aa660a74
commit
7dae1b64de
|
|
@ -0,0 +1,23 @@
|
|||
import { UpdateMigrationFindingStatusesRequestDto } from '../update-migration-finding-statuses-request.dto';
|
||||
|
||||
describe('UpdateMigrationFindingStatusesRequestDto', () => {
|
||||
test.each(['open', 'wont_fix'])('should accept the status %s', (status) => {
|
||||
expect(
|
||||
UpdateMigrationFindingStatusesRequestDto.safeParse({ workflowIds: ['wf-1', 'wf-2'], status })
|
||||
.success,
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
test.each([
|
||||
{
|
||||
name: 'a status that only the scan sets',
|
||||
request: { workflowIds: ['wf-1'], status: 'fixed' },
|
||||
},
|
||||
{ name: 'an empty workflow list', request: { workflowIds: [], status: 'wont_fix' } },
|
||||
{ name: 'an empty workflow id', request: { workflowIds: [''], status: 'wont_fix' } },
|
||||
{ name: 'a missing workflow list', request: { status: 'wont_fix' } },
|
||||
{ name: 'a missing status', request: { workflowIds: ['wf-1'] } },
|
||||
])('should reject $name', ({ request }) => {
|
||||
expect(UpdateMigrationFindingStatusesRequestDto.safeParse(request).success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,9 @@
|
|||
import { z } from 'zod';
|
||||
|
||||
import { migrationFindingTriageStatusSchema } from '../../schemas/breaking-changes.schema';
|
||||
import { Z } from '../../zod-class';
|
||||
|
||||
export class UpdateMigrationFindingStatusesRequestDto extends Z.class({
|
||||
workflowIds: z.array(z.string().min(1)).min(1),
|
||||
status: migrationFindingTriageStatusSchema,
|
||||
}) {}
|
||||
|
|
@ -54,6 +54,7 @@ export type { DiscoverDataPublic } from './discover/discover-public.dto';
|
|||
|
||||
export { BreakingChangeReportQueryDto } from './breaking-changes/breaking-change-report-query.dto';
|
||||
export { UpdateMigrationFindingStatusRequestDto } from './breaking-changes/update-migration-finding-status-request.dto';
|
||||
export { UpdateMigrationFindingStatusesRequestDto } from './breaking-changes/update-migration-finding-statuses-request.dto';
|
||||
export { AssignMigrationOwnerDto } from './breaking-changes/assign-migration-owner.dto';
|
||||
|
||||
export {
|
||||
|
|
|
|||
|
|
@ -333,6 +333,36 @@ describe('BreakingChangesController', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('PATCH /report/:ruleId/workflows', () => {
|
||||
it('passes the rule, workflows and status to the triage service and returns nothing', async () => {
|
||||
triageService.setStatuses.mockResolvedValue(undefined);
|
||||
|
||||
const result = await controller.updateFindingStatuses(req, res, 'removed-nodes-v3', {
|
||||
workflowIds: ['wf-1', 'wf-2'],
|
||||
status: 'wont_fix',
|
||||
});
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
expect(triageService.setStatuses).toHaveBeenCalledWith(
|
||||
'removed-nodes-v3',
|
||||
['wf-1', 'wf-2'],
|
||||
'wont_fix',
|
||||
);
|
||||
expect(syncService.syncIfStale).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('requires the global breakingChanges:migrate scope', () => {
|
||||
const metadata = Container.get(ControllerRegistryMetadata).getControllerMetadata(
|
||||
BreakingChangesController as Controller,
|
||||
);
|
||||
|
||||
expect(metadata.routes.get('updateFindingStatuses')?.accessScope).toEqual({
|
||||
scope: 'breakingChanges:migrate',
|
||||
globalOnly: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('PATCH /report/:ruleId/workflows/:workflowId', () => {
|
||||
it('passes the rule, workflow and status to the triage service and returns nothing', async () => {
|
||||
triageService.setStatus.mockResolvedValue(undefined);
|
||||
|
|
|
|||
|
|
@ -666,6 +666,101 @@ describe('MigrationFindingRepository', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('setTriageStatusForWorkflows', () => {
|
||||
const RULE_ID = 'removed-nodes-v3';
|
||||
|
||||
test('sets the status on the findings of the given workflows and moves statusChangedAt', async () => {
|
||||
const [first, second] = await Promise.all([createWorkflow(), createWorkflow()]);
|
||||
await insertWithStatusChangedAt(first.id, PAST);
|
||||
await insertWithStatusChangedAt(second.id, PAST);
|
||||
|
||||
await findingRepository.setTriageStatusForWorkflows(
|
||||
'v3',
|
||||
RULE_ID,
|
||||
[first.id, second.id],
|
||||
'wont_fix',
|
||||
ctx,
|
||||
);
|
||||
|
||||
const rows = await findingRepository.listForWorkflows('v3', [first.id, second.id], ctx);
|
||||
expect(rows.map((row) => row.status)).toEqual(['wont_fix', 'wont_fix']);
|
||||
for (const row of rows) {
|
||||
expect(row.statusChangedAt.getTime()).toBeGreaterThan(PAST.getTime());
|
||||
}
|
||||
});
|
||||
|
||||
test('keeps statusChangedAt on a finding that already has the status', async () => {
|
||||
const workflow = await createWorkflow();
|
||||
const id = await insertWithStatusChangedAt(workflow.id, PAST);
|
||||
await findingRepository.update({ id }, { status: 'wont_fix', statusChangedAt: PAST });
|
||||
|
||||
await findingRepository.setTriageStatusForWorkflows(
|
||||
'v3',
|
||||
RULE_ID,
|
||||
[workflow.id],
|
||||
'wont_fix',
|
||||
ctx,
|
||||
);
|
||||
|
||||
const [after] = await findingRepository.listForWorkflows('v3', [workflow.id], ctx);
|
||||
expect(after.status).toBe('wont_fix');
|
||||
expect(after.statusChangedAt.getTime()).toBe(PAST.getTime());
|
||||
});
|
||||
|
||||
test('keeps a fixed finding fixed and still updates the others', async () => {
|
||||
const [fixed, open] = await Promise.all([createWorkflow(), createWorkflow()]);
|
||||
const fixedId = await insertWithStatusChangedAt(fixed.id, PAST);
|
||||
await insertWithStatusChangedAt(open.id, PAST);
|
||||
await findingRepository.markFixedForIds([fixedId], ctx);
|
||||
|
||||
await findingRepository.setTriageStatusForWorkflows(
|
||||
'v3',
|
||||
RULE_ID,
|
||||
[fixed.id, open.id],
|
||||
'wont_fix',
|
||||
ctx,
|
||||
);
|
||||
|
||||
const rows = await findingRepository.find();
|
||||
expect(rows.find((row) => row.workflowId === fixed.id)?.status).toBe('fixed');
|
||||
expect(rows.find((row) => row.workflowId === open.id)?.status).toBe('wont_fix');
|
||||
});
|
||||
|
||||
test('changes only the findings of the given version, rule and workflows', async () => {
|
||||
const [target, other] = await Promise.all([createWorkflow(), createWorkflow()]);
|
||||
await findingRepository.insertMany(
|
||||
[
|
||||
finding(target.id, RULE_ID),
|
||||
finding(target.id, RULE_ID, 'v2'),
|
||||
finding(target.id, 'rule-b'),
|
||||
finding(other.id, RULE_ID),
|
||||
],
|
||||
ctx,
|
||||
);
|
||||
|
||||
await findingRepository.setTriageStatusForWorkflows(
|
||||
'v3',
|
||||
RULE_ID,
|
||||
[target.id],
|
||||
'wont_fix',
|
||||
ctx,
|
||||
);
|
||||
|
||||
const rows = await findingRepository.find();
|
||||
const statusOf = (workflowId: string, ruleId: string, targetVersion: string) =>
|
||||
rows.find(
|
||||
(row) =>
|
||||
row.workflowId === workflowId &&
|
||||
row.ruleId === ruleId &&
|
||||
row.targetVersion === targetVersion,
|
||||
)?.status;
|
||||
expect(statusOf(target.id, RULE_ID, 'v3')).toBe('wont_fix');
|
||||
expect(statusOf(target.id, RULE_ID, 'v2')).toBe('open');
|
||||
expect(statusOf(target.id, 'rule-b', 'v3')).toBe('open');
|
||||
expect(statusOf(other.id, RULE_ID, 'v3')).toBe('open');
|
||||
});
|
||||
});
|
||||
|
||||
describe('cascade delete', () => {
|
||||
test('deletes the findings of a deleted workflow and keeps the others', async () => {
|
||||
const [deleted, kept] = await Promise.all([createWorkflow(), createWorkflow()]);
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import {
|
|||
BreakingChangeVersion,
|
||||
MigrationWorkflowOwnerResponse,
|
||||
UpdateMigrationFindingStatusRequestDto,
|
||||
UpdateMigrationFindingStatusesRequestDto,
|
||||
WorkflowMigrationResult,
|
||||
} from '@n8n/api-types';
|
||||
import { EventService, WorkflowSharingService } from '@n8n/backend-services';
|
||||
|
|
@ -140,6 +141,18 @@ export class BreakingChangesController {
|
|||
return await this.queryService.getRuleFindings(version, ruleId, scope);
|
||||
}
|
||||
|
||||
/** Sets the status a user picks for the findings of one rule on many workflows. */
|
||||
@Patch('/report/:ruleId/workflows')
|
||||
@GlobalScope('breakingChanges:migrate')
|
||||
async updateFindingStatuses(
|
||||
_req: AuthenticatedRequest,
|
||||
_res: Response,
|
||||
@Param('ruleId') ruleId: string,
|
||||
@Body body: UpdateMigrationFindingStatusesRequestDto,
|
||||
): Promise<void> {
|
||||
await this.triageService.setStatuses(ruleId, body.workflowIds, body.status);
|
||||
}
|
||||
|
||||
/** Sets the status a user picks for the finding of one rule on one workflow. */
|
||||
@Patch('/report/:ruleId/workflows/:workflowId')
|
||||
@GlobalScope('breakingChanges:migrate')
|
||||
|
|
|
|||
|
|
@ -196,6 +196,31 @@ export class MigrationFindingRepository extends BaseRepository<MigrationFinding>
|
|||
return (result.affected ?? 0) > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the status a user picked on the findings of one rule on many workflows.
|
||||
* Findings in a status only the scan sets, for example `fixed`, stay as they are.
|
||||
* `statusChangedAt` moves only on the findings whose status changes.
|
||||
*/
|
||||
async setTriageStatusForWorkflows(
|
||||
targetVersion: BreakingChangeVersion,
|
||||
ruleId: string,
|
||||
workflowIds: string[],
|
||||
status: MigrationFindingTriageStatus,
|
||||
ctx: OperationContext,
|
||||
): Promise<void> {
|
||||
const otherTriageStatuses = TRIAGE_STATUSES.filter((triageStatus) => triageStatus !== status);
|
||||
// One transaction, so a failed chunk does not leave the earlier chunks changed.
|
||||
await this.runInTransaction(ctx, async (manager) => {
|
||||
for (const chunk of chunkIds(workflowIds)) {
|
||||
await manager.update(
|
||||
MigrationFinding,
|
||||
{ targetVersion, ruleId, workflowId: In(chunk), status: In(otherTriageStatuses) },
|
||||
{ status, statusChangedAt: new Date() },
|
||||
);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Inserts the findings as `open`. A finding that exists for the same workflow,
|
||||
* rule and target version is left as it is, so two syncs that run at the same
|
||||
|
|
|
|||
|
|
@ -77,4 +77,40 @@ describe('MigrationFindingTriageService', () => {
|
|||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
describe('setStatuses', () => {
|
||||
it("sets the status on the given workflows for the rule's version", async () => {
|
||||
registerRule('removed-nodes-v3', 'v3', 'workflow');
|
||||
|
||||
await expect(
|
||||
service.setStatuses('removed-nodes-v3', ['wf-1', 'wf-2'], 'wont_fix'),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(findingRepository.setTriageStatusForWorkflows).toHaveBeenCalledWith(
|
||||
'v3',
|
||||
'removed-nodes-v3',
|
||||
['wf-1', 'wf-2'],
|
||||
'wont_fix',
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects an unknown rule with not-found before writing', async () => {
|
||||
ruleRegistry.getRule.mockReturnValue(undefined);
|
||||
|
||||
await expect(service.setStatuses('unknown', ['wf-1'], 'open')).rejects.toBeInstanceOf(
|
||||
NotFoundError,
|
||||
);
|
||||
expect(findingRepository.setTriageStatusForWorkflows).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects an instance rule with not-found before writing', async () => {
|
||||
registerRule('docker-only-deployment-v3', 'v3', 'instance');
|
||||
|
||||
await expect(
|
||||
service.setStatuses('docker-only-deployment-v3', ['wf-1'], 'open'),
|
||||
).rejects.toBeInstanceOf(NotFoundError);
|
||||
expect(findingRepository.setTriageStatusForWorkflows).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -14,21 +14,13 @@ export class MigrationFindingTriageService {
|
|||
private readonly findingRepository: MigrationFindingRepository,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Sets the status of the finding of one workflow rule on one workflow. The
|
||||
* rule decides the target version, as on the rule detail route.
|
||||
*/
|
||||
/** Sets the status of the finding of one workflow rule on one workflow. */
|
||||
async setStatus(
|
||||
ruleId: string,
|
||||
workflowId: string,
|
||||
status: MigrationFindingTriageStatus,
|
||||
): Promise<void> {
|
||||
const rule = this.ruleRegistry.getRule(ruleId);
|
||||
if (!rule || !isWorkflowLevelRule(rule)) {
|
||||
throw new NotFoundError(`Breaking change rule with ID '${ruleId}' not found.`);
|
||||
}
|
||||
|
||||
const version = rule.getMetadata().version;
|
||||
const version = this.getWorkflowRuleVersion(ruleId);
|
||||
const updated = await this.findingRepository.setTriageStatus(
|
||||
version,
|
||||
ruleId,
|
||||
|
|
@ -43,4 +35,33 @@ export class MigrationFindingTriageService {
|
|||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the status of the findings of one workflow rule on many workflows. A
|
||||
* workflow without a finding in a status a user can set is skipped, so one
|
||||
* fixed finding does not fail the others.
|
||||
*/
|
||||
async setStatuses(
|
||||
ruleId: string,
|
||||
workflowIds: string[],
|
||||
status: MigrationFindingTriageStatus,
|
||||
): Promise<void> {
|
||||
const version = this.getWorkflowRuleVersion(ruleId);
|
||||
await this.findingRepository.setTriageStatusForWorkflows(
|
||||
version,
|
||||
ruleId,
|
||||
workflowIds,
|
||||
status,
|
||||
{},
|
||||
);
|
||||
}
|
||||
|
||||
/** The rule decides the target version, as on the rule detail route. */
|
||||
private getWorkflowRuleVersion(ruleId: string) {
|
||||
const rule = this.ruleRegistry.getRule(ruleId);
|
||||
if (!rule || !isWorkflowLevelRule(rule)) {
|
||||
throw new NotFoundError(`Breaking change rule with ID '${ruleId}' not found.`);
|
||||
}
|
||||
return rule.getMetadata().version;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4657,6 +4657,8 @@
|
|||
"settings.migrationReport.detail.state.wontFix": "Won't fix",
|
||||
"settings.migrationReport.detail.state.wontFix.description": "Counts as resolved without a fix",
|
||||
"settings.migrationReport.detail.state.error.title": "Could not change the state",
|
||||
"settings.migrationReport.detail.bulk.wontFix": "Mark won't fix",
|
||||
"settings.migrationReport.detail.bulk.reopen": "Reopen",
|
||||
"settings.migrationReport.detail.search.placeholder": "Search workflows...",
|
||||
"settings.migrationReport.detail.quickFilter.assignedToMe": "Assigned to me {count}",
|
||||
"settings.migrationReport.detail.quickFilter.open": "Open {count}",
|
||||
|
|
|
|||
|
|
@ -0,0 +1,27 @@
|
|||
import type { IRestApiContext } from '../../types';
|
||||
import * as utils from '../../utils';
|
||||
import { updateFindingStatuses } from '../breaking-changes';
|
||||
|
||||
vi.mock('../../utils');
|
||||
|
||||
const context = {} as IRestApiContext;
|
||||
|
||||
describe('updateFindingStatuses', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('sends a PATCH to the rule workflows endpoint with the workflow IDs and status', async () => {
|
||||
vi.mocked(utils.makeRestApiRequest).mockResolvedValueOnce(undefined);
|
||||
|
||||
await updateFindingStatuses(context, 'removed-nodes-v3', ['wf-1', 'wf-2'], 'wont_fix');
|
||||
|
||||
expect(utils.makeRestApiRequest).toHaveBeenCalledTimes(1);
|
||||
expect(utils.makeRestApiRequest).toHaveBeenCalledWith(
|
||||
context,
|
||||
'PATCH',
|
||||
'/breaking-changes/report/removed-nodes-v3/workflows',
|
||||
{ workflowIds: ['wf-1', 'wf-2'], status: 'wont_fix' },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -88,3 +88,15 @@ export async function updateFindingStatus(
|
|||
{ status },
|
||||
);
|
||||
}
|
||||
|
||||
export async function updateFindingStatuses(
|
||||
context: IRestApiContext,
|
||||
ruleId: string,
|
||||
workflowIds: string[],
|
||||
status: MigrationFindingTriageStatus,
|
||||
): Promise<void> {
|
||||
await makeRestApiRequest(context, 'PATCH', `/breaking-changes/report/${ruleId}/workflows`, {
|
||||
workflowIds,
|
||||
status,
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ vi.mock('@n8n/rest-api-client/api/breaking-changes', () => ({
|
|||
getReportForRule: vi.fn(),
|
||||
migrateWorkflowForRule: vi.fn(),
|
||||
updateFindingStatus: vi.fn(),
|
||||
updateFindingStatuses: vi.fn(),
|
||||
assignWorkflowOwner: vi.fn(),
|
||||
unassignWorkflowOwner: vi.fn(),
|
||||
}));
|
||||
|
|
@ -211,7 +212,8 @@ describe('MigrationRuleDetail', () => {
|
|||
const titles = screen
|
||||
.getAllByRole('columnheader')
|
||||
.map((th) => th.textContent?.replace(/[↑↓]/g, '').trim());
|
||||
expect(titles).toEqual(['Name', 'Nodes affected', 'Usage', 'Owner', 'State', '']);
|
||||
// The first column holds the select-all checkbox.
|
||||
expect(titles).toEqual(['', 'Name', 'Nodes affected', 'Usage', 'Owner', 'State', '']);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -353,6 +355,155 @@ describe('MigrationRuleDetail', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('bulk selection', () => {
|
||||
const getRowCheckbox = (workflowName: string) => {
|
||||
const row = screen.getByText(workflowName).closest('tr');
|
||||
if (!row) throw new Error('Row not found');
|
||||
return within(row).getByRole('checkbox');
|
||||
};
|
||||
|
||||
const getStateSelect = (workflowName: string) => {
|
||||
const row = screen.getByText(workflowName).closest('tr');
|
||||
if (!row) throw new Error('Row not found');
|
||||
return within(row).getByTestId('migration-finding-state-select');
|
||||
};
|
||||
|
||||
it('should show the selection bar only while rows are selected', async () => {
|
||||
renderComponent({ props: { migrationRuleId: 'rule-1' } });
|
||||
await screen.findByText('2 affected');
|
||||
expect(screen.queryByTestId('selected-items-info')).not.toBeInTheDocument();
|
||||
|
||||
await userEvent.click(getRowCheckbox('Test Workflow 1'));
|
||||
|
||||
expect(await screen.findByTestId('selected-items-info')).toHaveTextContent('1 row selected');
|
||||
|
||||
await userEvent.click(screen.getByTestId('clear-selection-button'));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(screen.queryByTestId('selected-items-info')).not.toBeInTheDocument(),
|
||||
);
|
||||
});
|
||||
|
||||
it("should mark the selected rows as won't fix in one request and clear the selection", async () => {
|
||||
vi.mocked(breakingChangesApi.updateFindingStatuses).mockResolvedValue();
|
||||
renderComponent({ props: { migrationRuleId: 'rule-1' } });
|
||||
await screen.findByText('2 affected');
|
||||
|
||||
await userEvent.click(getRowCheckbox('Test Workflow 1'));
|
||||
await userEvent.click(getRowCheckbox('Test Workflow 2'));
|
||||
await userEvent.click(screen.getByTestId('migration-bulk-wont-fix-button'));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(getStateSelect('Test Workflow 1')).toHaveTextContent("Won't fix");
|
||||
expect(getStateSelect('Test Workflow 2')).toHaveTextContent("Won't fix");
|
||||
expect(screen.getByText('0 affected')).toBeInTheDocument();
|
||||
expect(screen.queryByTestId('selected-items-info')).not.toBeInTheDocument();
|
||||
});
|
||||
expect(breakingChangesApi.updateFindingStatuses).toHaveBeenCalledTimes(1);
|
||||
const [, ruleId, workflowIds, status] = vi.mocked(breakingChangesApi.updateFindingStatuses)
|
||||
.mock.calls[0];
|
||||
expect(ruleId).toBe('rule-1');
|
||||
expect([...workflowIds].sort()).toEqual(['workflow-1', 'workflow-2']);
|
||||
expect(status).toBe('wont_fix');
|
||||
});
|
||||
|
||||
it('should keep a row that the user selects while the request is in flight', async () => {
|
||||
let finishSave = () => {};
|
||||
vi.mocked(breakingChangesApi.updateFindingStatuses).mockImplementation(
|
||||
async () => await new Promise<void>((resolve) => (finishSave = resolve)),
|
||||
);
|
||||
renderComponent({ props: { migrationRuleId: 'rule-1' } });
|
||||
await screen.findByText('2 affected');
|
||||
|
||||
await userEvent.click(getRowCheckbox('Test Workflow 1'));
|
||||
await userEvent.click(screen.getByTestId('migration-bulk-wont-fix-button'));
|
||||
await userEvent.click(getRowCheckbox('Test Workflow 2'));
|
||||
finishSave();
|
||||
|
||||
await waitFor(() =>
|
||||
expect(screen.getByTestId('selected-items-info')).toHaveTextContent('1 row selected'),
|
||||
);
|
||||
expect(getRowCheckbox('Test Workflow 2')).toBeChecked();
|
||||
expect(getRowCheckbox('Test Workflow 1')).not.toBeChecked();
|
||||
});
|
||||
|
||||
it('should reopen only the selected rows that are not open', async () => {
|
||||
vi.mocked(breakingChangesApi.getReportForRule).mockResolvedValue(
|
||||
createMockRuleResult({
|
||||
affectedWorkflows: [
|
||||
mockWorkflowWithIssue,
|
||||
{ ...mockWorkflowWithMultipleNodes, status: 'wont_fix' },
|
||||
],
|
||||
}),
|
||||
);
|
||||
vi.mocked(breakingChangesApi.updateFindingStatuses).mockResolvedValue();
|
||||
renderComponent({ props: { migrationRuleId: 'rule-1' } });
|
||||
await screen.findByText('1 affected');
|
||||
|
||||
await userEvent.click(getRowCheckbox('Test Workflow 1'));
|
||||
await userEvent.click(getRowCheckbox('Test Workflow 2'));
|
||||
await userEvent.click(screen.getByTestId('migration-bulk-reopen-button'));
|
||||
|
||||
await waitFor(() => expect(screen.getByText('2 affected')).toBeInTheDocument());
|
||||
expect(breakingChangesApi.updateFindingStatuses).toHaveBeenCalledWith(
|
||||
rootStore.restApiContext,
|
||||
'rule-1',
|
||||
['workflow-2'],
|
||||
'open',
|
||||
);
|
||||
});
|
||||
|
||||
it('should disable an action that would change none of the selected rows', async () => {
|
||||
renderComponent({ props: { migrationRuleId: 'rule-1' } });
|
||||
await screen.findByText('2 affected');
|
||||
|
||||
await userEvent.click(getRowCheckbox('Test Workflow 1'));
|
||||
|
||||
expect(await screen.findByTestId('migration-bulk-reopen-button')).toBeDisabled();
|
||||
expect(screen.getByTestId('migration-bulk-wont-fix-button')).toBeEnabled();
|
||||
});
|
||||
|
||||
it('should revert the rows, keep the selection and show an error when the save fails', async () => {
|
||||
const error = new Error('Request failed');
|
||||
vi.mocked(breakingChangesApi.updateFindingStatuses).mockRejectedValue(error);
|
||||
renderComponent({ props: { migrationRuleId: 'rule-1' } });
|
||||
await screen.findByText('2 affected');
|
||||
|
||||
await userEvent.click(getRowCheckbox('Test Workflow 1'));
|
||||
await userEvent.click(screen.getByTestId('migration-bulk-wont-fix-button'));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(showError).toHaveBeenCalledWith(error, 'Could not change the state');
|
||||
});
|
||||
expect(getStateSelect('Test Workflow 1')).toHaveTextContent('Open');
|
||||
expect(screen.getByText('2 affected')).toBeInTheDocument();
|
||||
expect(screen.getByTestId('selected-items-info')).toHaveTextContent('1 row selected');
|
||||
});
|
||||
|
||||
it('should drop rows that a search hides from the selection', async () => {
|
||||
const user = userEvent.setup({ delay: null });
|
||||
renderComponent({ props: { migrationRuleId: 'rule-1' } });
|
||||
await screen.findByText('2 affected');
|
||||
|
||||
await user.click(getRowCheckbox('Test Workflow 1'));
|
||||
await user.click(getRowCheckbox('Test Workflow 2'));
|
||||
await user.type(screen.getByPlaceholderText('Search workflows...'), 'workflow 1');
|
||||
|
||||
await vi.waitFor(
|
||||
() => expect(screen.getByTestId('selected-items-info')).toHaveTextContent('1 row selected'),
|
||||
{ timeout: 1000 },
|
||||
);
|
||||
});
|
||||
|
||||
it('should not offer selection to a user without the migrate scope', async () => {
|
||||
rbacStore.hasScope.mockReturnValue(false);
|
||||
renderComponent({ props: { migrationRuleId: 'rule-1' } });
|
||||
await screen.findByText('2 affected');
|
||||
|
||||
expect(screen.queryAllByRole('checkbox')).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('migration', () => {
|
||||
it('should not render a Migrate button when the rule is not migratable', async () => {
|
||||
vi.mocked(breakingChangesApi.getReportForRule).mockResolvedValue(mockRuleResult);
|
||||
|
|
@ -526,7 +677,14 @@ describe('MigrationRuleDetail', () => {
|
|||
// The first row's search hangs; the second row's answers at once.
|
||||
await userEvent.click(pickers[0]);
|
||||
await userEvent.click(pickers[1]);
|
||||
await waitFor(() => expect(usersApi.getUsers).toHaveBeenCalledTimes(2));
|
||||
// An open picker also runs its debounced remote search, so a slow run can see
|
||||
// more calls. Wait for the second row's search instead of a call count.
|
||||
await waitFor(() =>
|
||||
expect(usersApi.getUsers).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({ filter: { projectId: 'project-2' } }),
|
||||
),
|
||||
);
|
||||
await waitFor(() => {
|
||||
expect(baseElement.querySelector('#user-select-option-id-user-2')).not.toBeNull();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ import {
|
|||
N8nInput,
|
||||
N8nLink,
|
||||
N8nLoading,
|
||||
N8nSelectedItemsInfo,
|
||||
N8nSettingsLayout,
|
||||
N8nText,
|
||||
N8nUserSelect,
|
||||
|
|
@ -35,7 +36,7 @@ import { useRootStore } from '@n8n/stores/useRootStore';
|
|||
import { createEventBus } from '@n8n/utils/event-bus';
|
||||
import { useAsyncState, useDebounceFn } from '@vueuse/core';
|
||||
import orderBy from 'lodash/orderBy';
|
||||
import { computed, nextTick, ref } from 'vue';
|
||||
import { computed, nextTick, ref, watch } from 'vue';
|
||||
import { useRouter } from 'vue-router';
|
||||
import { I18nT } from 'vue-i18n';
|
||||
import FindingStateSelect from './components/FindingStateSelect.vue';
|
||||
|
|
@ -296,15 +297,25 @@ const openCount = computed(
|
|||
const savingWorkflowIds = ref<Set<string>>(new Set());
|
||||
|
||||
// The state is a shallow ref, so replace the list to make the table update.
|
||||
function setFindingStatus(workflowId: string, status: MigrationFindingTriageStatus) {
|
||||
function setFindingStatuses(statuses: Map<string, MigrationFindingTriageStatus>) {
|
||||
state.value = {
|
||||
...state.value,
|
||||
affectedWorkflows: state.value.affectedWorkflows.map((workflow) =>
|
||||
workflow.id === workflowId ? { ...workflow, status } : workflow,
|
||||
),
|
||||
affectedWorkflows: state.value.affectedWorkflows.map((workflow) => {
|
||||
const status = statuses.get(workflow.id);
|
||||
return status ? { ...workflow, status } : workflow;
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
function setSaving(workflowIds: string[], saving: boolean) {
|
||||
const next = new Set(savingWorkflowIds.value);
|
||||
for (const workflowId of workflowIds) {
|
||||
if (saving) next.add(workflowId);
|
||||
else next.delete(workflowId);
|
||||
}
|
||||
savingWorkflowIds.value = next;
|
||||
}
|
||||
|
||||
async function onFindingStatusChange(
|
||||
workflow: AffectedWorkflow,
|
||||
status: MigrationFindingTriageStatus,
|
||||
|
|
@ -312,8 +323,8 @@ async function onFindingStatusChange(
|
|||
const previousStatus = workflow.status;
|
||||
if (status === previousStatus || savingWorkflowIds.value.has(workflow.id)) return;
|
||||
|
||||
setFindingStatus(workflow.id, status);
|
||||
savingWorkflowIds.value = new Set(savingWorkflowIds.value).add(workflow.id);
|
||||
setFindingStatuses(new Map([[workflow.id, status]]));
|
||||
setSaving([workflow.id], true);
|
||||
try {
|
||||
await breakingChangesApi.updateFindingStatus(
|
||||
rootStore.restApiContext,
|
||||
|
|
@ -322,12 +333,59 @@ async function onFindingStatusChange(
|
|||
status,
|
||||
);
|
||||
} catch (error) {
|
||||
setFindingStatus(workflow.id, previousStatus);
|
||||
setFindingStatuses(new Map([[workflow.id, previousStatus]]));
|
||||
toast.showError(error, i18n.baseText('settings.migrationReport.detail.state.error.title'));
|
||||
} finally {
|
||||
const saving = new Set(savingWorkflowIds.value);
|
||||
saving.delete(workflow.id);
|
||||
savingWorkflowIds.value = saving;
|
||||
setSaving([workflow.id], false);
|
||||
}
|
||||
}
|
||||
|
||||
const selectedWorkflowIds = ref<string[]>([]);
|
||||
|
||||
const selectedWorkflows = computed(() => {
|
||||
const selected = new Set(selectedWorkflowIds.value);
|
||||
return state.value.affectedWorkflows.filter((workflow) => selected.has(workflow.id));
|
||||
});
|
||||
|
||||
// A migrated row has no state to change, so it cannot be selected.
|
||||
function isSelectable(workflow: AffectedWorkflow): boolean {
|
||||
return !migratedWorkflowIds.value.has(workflow.id);
|
||||
}
|
||||
|
||||
/** The selected rows that a bulk change to `status` would change. */
|
||||
function bulkTargets(status: MigrationFindingTriageStatus): AffectedWorkflow[] {
|
||||
return selectedWorkflows.value.filter(
|
||||
(workflow) =>
|
||||
workflow.status !== status &&
|
||||
isSelectable(workflow) &&
|
||||
!savingWorkflowIds.value.has(workflow.id),
|
||||
);
|
||||
}
|
||||
|
||||
async function onBulkStatusChange(status: MigrationFindingTriageStatus) {
|
||||
const workflows = bulkTargets(status);
|
||||
if (workflows.length === 0) return;
|
||||
|
||||
const workflowIds = workflows.map((workflow) => workflow.id);
|
||||
setFindingStatuses(new Map(workflowIds.map((workflowId) => [workflowId, status])));
|
||||
setSaving(workflowIds, true);
|
||||
try {
|
||||
await breakingChangesApi.updateFindingStatuses(
|
||||
rootStore.restApiContext,
|
||||
props.migrationRuleId,
|
||||
workflowIds,
|
||||
status,
|
||||
);
|
||||
// Keep the rows that the user selected while the request was in flight.
|
||||
const changed = new Set(workflowIds);
|
||||
selectedWorkflowIds.value = selectedWorkflowIds.value.filter(
|
||||
(workflowId) => !changed.has(workflowId),
|
||||
);
|
||||
} catch (error) {
|
||||
setFindingStatuses(new Map(workflows.map((workflow) => [workflow.id, workflow.status])));
|
||||
toast.showError(error, i18n.baseText('settings.migrationReport.detail.state.error.title'));
|
||||
} finally {
|
||||
setSaving(workflowIds, false);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -458,6 +516,14 @@ const filteredWorkflows = computed(() => {
|
|||
);
|
||||
});
|
||||
|
||||
// A bulk action must not change rows the user cannot see, so a search or a
|
||||
// filter drops the hidden rows from the selection.
|
||||
watch(filteredWorkflows, (workflows) => {
|
||||
const visible = new Set(workflows.map((workflow) => workflow.id));
|
||||
const kept = selectedWorkflowIds.value.filter((workflowId) => visible.has(workflowId));
|
||||
if (kept.length !== selectedWorkflowIds.value.length) selectedWorkflowIds.value = kept;
|
||||
});
|
||||
|
||||
// The owner cell shows a label, so it sorts by that label and not by the owner object.
|
||||
// Dates come from JSON as strings, and a workflow that never ran has no date.
|
||||
// A timestamp sorts them right, with a missing date as the oldest.
|
||||
|
|
@ -617,6 +683,9 @@ const sortedWorkflows = computed(() => {
|
|||
<N8nDataTableServer
|
||||
:key="String(state.migratable)"
|
||||
v-model:sort-by="sortBy"
|
||||
v-model:selection="selectedWorkflowIds"
|
||||
:show-select="canMigrate"
|
||||
:item-selectable="isSelectable"
|
||||
:items-per-page="sortedWorkflows.length + 1"
|
||||
:items="sortedWorkflows"
|
||||
:items-length="sortedWorkflows.length"
|
||||
|
|
@ -743,6 +812,30 @@ const sortedWorkflows = computed(() => {
|
|||
</div>
|
||||
</template>
|
||||
</N8nDataTableServer>
|
||||
|
||||
<div v-if="selectedWorkflowIds.length > 0" :class="$style.selectionBar">
|
||||
<N8nSelectedItemsInfo
|
||||
:selected-count="selectedWorkflowIds.length"
|
||||
@clear-selection="selectedWorkflowIds = []"
|
||||
>
|
||||
<template #actions>
|
||||
<N8nButton
|
||||
variant="subtle"
|
||||
:label="i18n.baseText('settings.migrationReport.detail.bulk.wontFix')"
|
||||
:disabled="bulkTargets('wont_fix').length === 0"
|
||||
data-test-id="migration-bulk-wont-fix-button"
|
||||
@click="onBulkStatusChange('wont_fix')"
|
||||
/>
|
||||
<N8nButton
|
||||
variant="subtle"
|
||||
:label="i18n.baseText('settings.migrationReport.detail.bulk.reopen')"
|
||||
:disabled="bulkTargets('open').length === 0"
|
||||
data-test-id="migration-bulk-reopen-button"
|
||||
@click="onBulkStatusChange('open')"
|
||||
/>
|
||||
</template>
|
||||
</N8nSelectedItemsInfo>
|
||||
</div>
|
||||
</N8nSettingsLayout>
|
||||
</template>
|
||||
|
||||
|
|
@ -786,6 +879,15 @@ const sortedWorkflows = computed(() => {
|
|||
gap: var(--spacing--sm);
|
||||
}
|
||||
|
||||
/* Sticks to the bottom of the viewport, so the floating selection bar stays in
|
||||
view while a long table scrolls. The height holds the bar and its bottom
|
||||
offset, so the bar does not cover the rows of a short table. */
|
||||
.selectionBar {
|
||||
position: sticky;
|
||||
bottom: 0;
|
||||
height: calc(var(--spacing--3xl) + var(--spacing--2xl));
|
||||
}
|
||||
|
||||
.toolbar {
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user