mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-12 02:19:43 +00:00
fix(core): Bypass archived workflows in activation (#30517)
This commit is contained in:
@@ -78,7 +78,7 @@ export class WorkflowRepository extends Repository<WorkflowEntity> {
|
||||
async getAllActiveIds() {
|
||||
const result = await this.find({
|
||||
select: { id: true },
|
||||
where: { activeVersionId: Not(IsNull()) },
|
||||
where: { activeVersionId: Not(IsNull()), isArchived: false },
|
||||
});
|
||||
|
||||
return result.map(({ id }) => id);
|
||||
@@ -1294,20 +1294,23 @@ export class WorkflowRepository extends Repository<WorkflowEntity> {
|
||||
* @param versionId - The ID of the version to publish (optional; if not provided, uses the current version)
|
||||
* */
|
||||
async publishVersion(workflowId: string, versionId?: string) {
|
||||
let versionIdToPublish = versionId;
|
||||
if (!versionIdToPublish) {
|
||||
const workflow = await this.findOne({
|
||||
where: { id: workflowId },
|
||||
select: ['id', 'versionId'],
|
||||
});
|
||||
const workflow = await this.findOne({
|
||||
where: { id: workflowId },
|
||||
select: ['id', 'versionId', 'isArchived'],
|
||||
});
|
||||
|
||||
if (!workflow) {
|
||||
throw new UserError(`Workflow "${workflowId}" not found.`);
|
||||
}
|
||||
|
||||
versionIdToPublish = workflow.versionId;
|
||||
if (!workflow) {
|
||||
throw new UserError(`Workflow "${workflowId}" not found.`);
|
||||
}
|
||||
|
||||
if (workflow.isArchived) {
|
||||
throw new UserError('Cannot publish archived Workflow', {
|
||||
extra: { workflowId },
|
||||
});
|
||||
}
|
||||
|
||||
const versionIdToPublish = versionId ?? workflow.versionId;
|
||||
|
||||
const version = await this.workflowHistoryRepository.findOneBy({
|
||||
workflowId,
|
||||
versionId: versionIdToPublish,
|
||||
|
||||
@@ -120,6 +120,31 @@ describe('ActiveWorkflowManager', () => {
|
||||
expect(added).toEqual({ triggersAndPollers: false, webhooks: false });
|
||||
},
|
||||
);
|
||||
|
||||
test.each<[WorkflowActivateMode]>([['init'], ['leadershipChange'], ['activate']])(
|
||||
'should skip archived workflow in `%s` activation mode',
|
||||
async (mode) => {
|
||||
const addWebhooksSpy = jest.spyOn(activeWorkflowManager, 'addWebhooks');
|
||||
const addTriggersAndPollersSpy = jest.spyOn(
|
||||
activeWorkflowManager,
|
||||
'addTriggersAndPollers',
|
||||
);
|
||||
workflowRepository.findById.mockResolvedValue(
|
||||
mock<WorkflowEntity>({
|
||||
id: 'archived-id',
|
||||
active: true,
|
||||
activeVersionId: 'v1',
|
||||
isArchived: true,
|
||||
}),
|
||||
);
|
||||
|
||||
const added = await activeWorkflowManager.add('archived-id', mode);
|
||||
|
||||
expect(addWebhooksSpy).not.toHaveBeenCalled();
|
||||
expect(addTriggersAndPollersSpy).not.toHaveBeenCalled();
|
||||
expect(added).toEqual({ triggersAndPollers: false, webhooks: false });
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -652,6 +652,11 @@ export class ActiveWorkflowManager {
|
||||
});
|
||||
}
|
||||
|
||||
if (dbWorkflow.isArchived) {
|
||||
this.logger.debug('Cannot publish archived Workflow', { workflowId: dbWorkflow.id });
|
||||
return added;
|
||||
}
|
||||
|
||||
if (this.instanceSettings.isMultiMain && shouldPublish) {
|
||||
if (!dbWorkflow?.activeVersionId) {
|
||||
throw new UnexpectedError('Active version ID not found for workflow', {
|
||||
|
||||
+50
@@ -544,6 +544,56 @@ describe('SourceControlImportService', () => {
|
||||
expect(workflowService.activateWorkflow).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should clear active state when local workflow is archived with a lingering active version', async () => {
|
||||
const mockUserId = 'user-id-123';
|
||||
const mockUser = Object.assign(new User(), { id: mockUserId });
|
||||
const mockWorkflowFile = '/mock/workflow1.json';
|
||||
const mockWorkflowData = {
|
||||
id: 'workflow1',
|
||||
name: 'Workflow',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
versionId: 'v2',
|
||||
parentFolderId: null,
|
||||
};
|
||||
const candidates = [mock<SourceControlledFile>({ file: mockWorkflowFile, id: 'workflow1' })];
|
||||
|
||||
userRepository.findOne.mockResolvedValue(mockUser);
|
||||
projectRepository.getPersonalProjectForUserOrFail.mockResolvedValue(
|
||||
Object.assign(new Project(), { id: 'project1', type: 'personal' }),
|
||||
);
|
||||
workflowRepository.findByIds.mockResolvedValue([
|
||||
Object.assign(new WorkflowEntity(), {
|
||||
id: 'workflow1',
|
||||
name: 'Workflow',
|
||||
active: true,
|
||||
activeVersionId: 'v1',
|
||||
isArchived: true,
|
||||
}),
|
||||
]);
|
||||
folderRepository.find.mockResolvedValue([]);
|
||||
sharedWorkflowRepository.findWithFields.mockResolvedValue([]);
|
||||
workflowRepository.upsert.mockResolvedValue({
|
||||
identifiers: [{ id: 'workflow1' }],
|
||||
generatedMaps: [],
|
||||
raw: [],
|
||||
});
|
||||
|
||||
fsReadFile.mockResolvedValue(JSON.stringify(mockWorkflowData));
|
||||
|
||||
await service.importWorkflowFromWorkFolder(candidates, mockUserId, 'none');
|
||||
|
||||
expect(workflowRepository.upsert).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
id: 'workflow1',
|
||||
active: false,
|
||||
activeVersionId: null,
|
||||
}),
|
||||
['id'],
|
||||
);
|
||||
expect(workflowService.activateWorkflow).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should unpublish archived workflows even if they were previously published', async () => {
|
||||
const mockUserId = 'user-id-123';
|
||||
const mockUser = Object.assign(new User(), { id: mockUserId });
|
||||
|
||||
@@ -683,7 +683,7 @@ export class SourceControlImportService {
|
||||
const personalProject = await this.projectRepository.getPersonalProjectForUserOrFail(userId);
|
||||
const candidateIds = candidates.map((c) => c.id);
|
||||
const existingWorkflows = await this.workflowRepository.findByIds(candidateIds, {
|
||||
fields: ['id', 'name', 'versionId', 'active', 'activeVersionId'],
|
||||
fields: ['id', 'name', 'versionId', 'active', 'activeVersionId', 'isArchived'],
|
||||
});
|
||||
|
||||
const folders = await this.folderRepository.find({ select: ['id'] });
|
||||
@@ -1761,7 +1761,7 @@ export class SourceControlImportService {
|
||||
|
||||
this.resolvePublishedStatus(
|
||||
importedWorkflow,
|
||||
existingWorkflow?.activeVersionId,
|
||||
existingWorkflow,
|
||||
mustUnpublishLocal,
|
||||
unpublishedLocal,
|
||||
);
|
||||
@@ -1773,7 +1773,7 @@ export class SourceControlImportService {
|
||||
* Resolves the publish status for the upsert of the imported workflow.
|
||||
* We set active to false here and handle publishing after upsert.
|
||||
* @param importedWorkflow The imported workflow.
|
||||
* @param existingWorkflowActiveVersionId The existing workflow active version id, if it exists.
|
||||
* @param existingWorkflow The existing workflow entity, if it exists.
|
||||
* @param mustUnpublishLocal Whether the local workflow must be unpublished.
|
||||
* @param unpublishedLocal Whether the local workflow was unpublished.
|
||||
*/
|
||||
@@ -1781,12 +1781,17 @@ export class SourceControlImportService {
|
||||
// Note: Workflow's active status is not saved in the remote workflow files,
|
||||
// and the field is missing despite IWorkflowToImport having it typed as boolean.
|
||||
importedWorkflow: IWorkflowToImport,
|
||||
existingWorkflowActiveVersionId: string | null | undefined,
|
||||
existingWorkflow: WorkflowEntity | undefined,
|
||||
mustUnpublishLocal: boolean,
|
||||
unpublishedLocal: boolean,
|
||||
) {
|
||||
const existingWorkflowActiveVersionId = existingWorkflow?.activeVersionId;
|
||||
const isExistingArchived = !!existingWorkflow?.isArchived;
|
||||
|
||||
const shouldPreserve =
|
||||
!!existingWorkflowActiveVersionId && (!mustUnpublishLocal || !unpublishedLocal);
|
||||
!isExistingArchived &&
|
||||
!!existingWorkflowActiveVersionId &&
|
||||
(!mustUnpublishLocal || !unpublishedLocal);
|
||||
|
||||
if (shouldPreserve) {
|
||||
importedWorkflow.active = !!existingWorkflowActiveVersionId;
|
||||
|
||||
@@ -148,6 +148,27 @@ describe('WorkflowRepository', () => {
|
||||
expect(updatedWorkflow?.activeVersionId).toBe(workflow.versionId);
|
||||
expect(updatedWorkflow?.active).toBe(true);
|
||||
});
|
||||
|
||||
it('should throw when workflow does not exist', async () => {
|
||||
const workflowRepository = Container.get(WorkflowRepository);
|
||||
|
||||
await expect(workflowRepository.publishVersion('non-existent-id')).rejects.toThrow(
|
||||
'Workflow "non-existent-id" not found.',
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw when workflow is archived', async () => {
|
||||
const workflowRepository = Container.get(WorkflowRepository);
|
||||
const workflow = await createWorkflowWithTriggerAndHistory({ isArchived: true });
|
||||
|
||||
await expect(workflowRepository.publishVersion(workflow.id)).rejects.toThrow(
|
||||
'Cannot publish archived Workflow',
|
||||
);
|
||||
|
||||
const updatedWorkflow = await getWorkflowById(workflow.id);
|
||||
expect(updatedWorkflow?.activeVersionId).toBeNull();
|
||||
expect(updatedWorkflow?.active).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('unpublishAll', () => {
|
||||
@@ -227,6 +248,19 @@ describe('WorkflowRepository', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('getAllActiveIds', () => {
|
||||
it('should exclude archived workflows', async () => {
|
||||
const workflowRepository = Container.get(WorkflowRepository);
|
||||
const activeNonArchived = await createActiveWorkflow();
|
||||
const activeArchived = await createActiveWorkflow();
|
||||
await workflowRepository.update(activeArchived.id, { isArchived: true });
|
||||
|
||||
const activeIds = await workflowRepository.getAllActiveIds();
|
||||
|
||||
expect(activeIds).toEqual([activeNonArchived.id]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getWorkflowsWithEvaluationCount', () => {
|
||||
it('should return 0 when no workflows have test runs', async () => {
|
||||
//
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
linkUserToProject,
|
||||
createWorkflowWithHistory,
|
||||
randomCredentialPayload,
|
||||
setActiveVersion,
|
||||
testDb,
|
||||
mockInstance,
|
||||
} from '@n8n/backend-test-utils';
|
||||
@@ -1932,5 +1933,37 @@ describe('SourceControlImportService', () => {
|
||||
expect(workflowInDb).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('archived workflows', () => {
|
||||
it('should not preserve active state when existing workflow is archived', async () => {
|
||||
const importingUser = await getGlobalOwner();
|
||||
const workflowId = nanoid();
|
||||
const versionId = nanoid();
|
||||
|
||||
await createWorkflowWithHistory(
|
||||
{
|
||||
id: workflowId,
|
||||
name: 'Archived Workflow',
|
||||
versionId,
|
||||
active: true,
|
||||
isArchived: true,
|
||||
},
|
||||
importingUser,
|
||||
);
|
||||
await setActiveVersion(workflowId, versionId);
|
||||
|
||||
const imported = makeWorkflowImport({ id: workflowId, isArchived: false });
|
||||
const file = putWorkflowFile(workflowId, imported);
|
||||
|
||||
await service.importWorkflowFromWorkFolder(
|
||||
[mock<SourceControlledFile>({ id: workflowId, file })],
|
||||
importingUser.id,
|
||||
);
|
||||
|
||||
const post = await workflowRepository.findOne({ where: { id: workflowId } });
|
||||
expect(post?.active).toBe(false);
|
||||
expect(post?.activeVersionId).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user