fix(core): Delete data tables on project delete (no-changelog) (#19683)

Co-authored-by: Jaakko Husso <jaakko@n8n.io>
This commit is contained in:
Charlie Kolb 2025-09-19 16:36:31 +02:00 committed by GitHub
parent d1b17d908b
commit e5e7c0bf45
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 301 additions and 30 deletions

View File

@ -2683,4 +2683,166 @@ describe('dataStore', () => {
await expect(result).rejects.toThrow(DataStoreValidationError);
});
});
describe('transferDataStoresByProjectId', () => {
it('should transfer all data stores from one project to another', async () => {
// ARRANGE
const { id: dataStoreId1, name: dataStoreName1 } = await dataStoreService.createDataStore(
project1.id,
{
name: 'dataStore1',
columns: [{ name: 'col1', type: 'string' }],
},
);
const { id: dataStoreId2, name: dataStoreName2 } = await dataStoreService.createDataStore(
project1.id,
{
name: 'dataStore2',
columns: [{ name: 'col1', type: 'string' }],
},
);
// ACT
await dataStoreService.transferDataStoresByProjectId(project1.id, project2.id);
// ASSERT
const dataStore1 = await dataStoreRepository.findOneByOrFail({
id: dataStoreId1,
});
expect(dataStore1).toBeDefined();
expect(dataStore1.projectId).toBe(project2.id);
expect(dataStore1.name).toBe(dataStoreName1);
const dataStore2 = await dataStoreRepository.findOneByOrFail({
id: dataStoreId2,
});
expect(dataStore2).toBeDefined();
expect(dataStore2.projectId).toBe(project2.id);
expect(dataStore2.name).toBe(dataStoreName2);
});
it('should not affect data stores in other projects', async () => {
// ARRANGE
const { id: dataStoreId1 } = await dataStoreService.createDataStore(project1.id, {
name: 'dataStore1',
columns: [{ name: 'col1', type: 'string' }],
});
const { id: dataStoreId2 } = await dataStoreService.createDataStore(project2.id, {
name: 'dataStore2',
columns: [{ name: 'col1', type: 'string' }],
});
// ACT
await dataStoreService.transferDataStoresByProjectId(project1.id, project2.id);
// ASSERT
const dataStore1 = await dataStoreRepository.findOneByOrFail({
id: dataStoreId1,
});
expect(dataStore1).toBeDefined();
expect(dataStore1.projectId).toBe(project2.id);
const dataStore2 = await dataStoreRepository.findOneByOrFail({
id: dataStoreId2,
project: {
id: project2.id,
},
});
expect(dataStore2).toBeDefined();
expect(dataStore2.projectId).toBe(project2.id);
});
it('should rename data stores if name conflict occurs in target project', async () => {
// ARRANGE
const { id: dataStoreId1 } = await dataStoreService.createDataStore(project1.id, {
name: 'dataStore',
columns: [{ name: 'col1', type: 'string' }],
});
await dataStoreService.createDataStore(project2.id, {
name: 'dataStore',
columns: [{ name: 'col1', type: 'string' }],
});
// ACT
await dataStoreService.transferDataStoresByProjectId(project1.id, project2.id);
// ASSERT
const dataStore1 = await dataStoreRepository.findOneByOrFail({
id: dataStoreId1,
});
expect(dataStore1).toBeDefined();
expect(dataStore1.projectId).toBe(project2.id);
expect(dataStore1.name).toBe(`dataStore (${project1.name})`);
});
});
describe('deleteDataStoreByProjectId', () => {
it('should delete all data stores for a given project ID', async () => {
// ARRANGE
const { id: dataStoreId1 } = await dataStoreService.createDataStore(project1.id, {
name: 'dataStore1',
columns: [{ name: 'col1', type: 'string' }],
});
const { id: dataStoreId2 } = await dataStoreService.createDataStore(project1.id, {
name: 'dataStore2',
columns: [{ name: 'col1', type: 'string' }],
});
// ACT
await dataStoreService.deleteDataStoreByProjectId(project1.id);
// ASSERT
await expect(
dataStoreRepository.findOneByOrFail({
id: dataStoreId1,
project: {
id: project1.id,
},
}),
).rejects.toThrow();
await expect(
dataStoreRepository.findOneByOrFail({
id: dataStoreId2,
project: {
id: project1.id,
},
}),
).rejects.toThrow();
});
it('should not delete data stores for other projects', async () => {
// ARRANGE
const { id: dataStoreId1 } = await dataStoreService.createDataStore(project1.id, {
name: 'dataStoreA',
columns: [{ name: 'col1', type: 'string' }],
});
const { id: dataStoreId2 } = await dataStoreService.createDataStore(project2.id, {
name: 'dataStoreB',
columns: [{ name: 'col1', type: 'string' }],
});
// ACT
await dataStoreService.deleteDataStoreByProjectId(project1.id);
// ASSERT
await expect(
dataStoreRepository.findOneByOrFail({
id: dataStoreId1,
project: {
id: project1.id,
},
}),
).rejects.toThrow();
const dataStore2 = await dataStoreRepository.findOneByOrFail({
id: dataStoreId2,
project: {
id: project2.id,
},
});
expect(dataStore2).toBeDefined();
expect(dataStore2.id).toBe(dataStoreId2);
});
});
});

View File

@ -4,6 +4,7 @@ import {
type ListDataStoreQueryDto,
} from '@n8n/api-types';
import { GlobalConfig } from '@n8n/config';
import { Project } from '@n8n/db';
import { Service } from '@n8n/di';
import { DataSource, EntityManager, Repository, SelectQueryBuilder } from '@n8n/typeorm';
import { UnexpectedError } from 'n8n-workflow';
@ -12,6 +13,7 @@ import { DataStoreRowsRepository } from './data-store-rows.repository';
import { DataStoreUserTableName, DataTablesSizeData } from './data-store.types';
import { DataTableColumn } from './data-table-column.entity';
import { DataTable } from './data-table.entity';
import { DataStoreNameConflictError } from './errors/data-store-name-conflict.error';
import { DataStoreValidationError } from './errors/data-store-validation.error';
import { isValidColumnName, toTableId, toTableName } from './utils/sql-utils';
@ -77,9 +79,8 @@ export class DataStoreRepository extends Repository<DataTable> {
return createdDataStore;
}
async deleteDataStore(dataStoreId: string, entityManager?: EntityManager) {
const executor = entityManager ?? this.manager;
return await executor.transaction(async (em) => {
async deleteDataStore(dataStoreId: string, tx?: EntityManager) {
const run = async (em: EntityManager) => {
const queryRunner = em.queryRunner;
if (!queryRunner) {
throw new UnexpectedError('QueryRunner is not available');
@ -87,8 +88,51 @@ export class DataStoreRepository extends Repository<DataTable> {
await em.delete(DataTable, { id: dataStoreId });
await this.dataStoreRowsRepository.dropTable(dataStoreId, queryRunner);
return true;
};
if (tx) {
return await run(tx);
}
return await this.manager.transaction(run);
}
async transferDataStoreByProjectId(fromProjectId: string, toProjectId: string) {
if (fromProjectId === toProjectId) return false;
return await this.manager.transaction(async (em) => {
const existingTables = await em.findBy(DataTable, { projectId: fromProjectId });
let transferred = false;
for (const existing of existingTables) {
let name = existing.name;
const hasNameClash = await em.existsBy(DataTable, {
name,
projectId: toProjectId,
});
if (hasNameClash) {
const project = await em.findOneByOrFail(Project, { id: fromProjectId });
name = `${existing.name} (${project.name})`;
const stillHasNameClash = await em.existsBy(DataTable, {
name,
projectId: toProjectId,
});
if (stillHasNameClash) {
throw new DataStoreNameConflictError(
`Failed to transfer data store "${existing.name}" to the target project "${toProjectId}". A data table with the same name already exists in the target project.`,
);
}
}
await em.update(DataTable, { id: existing.id }, { name, projectId: toProjectId });
transferred = true;
}
return transferred;
});
}

View File

@ -66,6 +66,10 @@ export class DataStoreService {
return true;
}
async transferDataStoresByProjectId(fromProjectId: string, toProjectId: string) {
return await this.dataStoreRepository.transferDataStoreByProjectId(fromProjectId, toProjectId);
}
async deleteDataStoreByProjectId(projectId: string) {
return await this.dataStoreRepository.deleteDataStoreByProjectId(projectId);
}

View File

@ -1,4 +1,5 @@
import type { ProjectRelation } from '@n8n/api-types';
import type { ModuleRegistry } from '@n8n/backend-common';
import type { DatabaseConfig } from '@n8n/config';
import {
type Project,
@ -23,6 +24,7 @@ describe('ProjectService', () => {
const roleService = mock<RoleService>();
const sharedCredentialsRepository = mock<SharedCredentialsRepository>();
const cacheService = mock<CacheService>();
const moduleRegistry = mock<ModuleRegistry>({ entities: [] });
const projectService = new ProjectService(
mock(),
projectRepository,
@ -32,6 +34,7 @@ describe('ProjectService', () => {
cacheService,
mock(),
mock<DatabaseConfig>({ type: 'postgresdb' }),
moduleRegistry,
);
describe('addUsersToProject', () => {

View File

@ -1,5 +1,5 @@
import type { CreateProjectDto, ProjectType, UpdateProjectDto } from '@n8n/api-types';
import { LicenseState } from '@n8n/backend-common';
import { LicenseState, ModuleRegistry } from '@n8n/backend-common';
import { DatabaseConfig } from '@n8n/config';
import { UNLIMITED_LICENSE_QUOTA } from '@n8n/constants';
import type { User } from '@n8n/db';
@ -73,6 +73,7 @@ export class ProjectService {
private readonly cacheService: CacheService,
private readonly licenseState: LicenseState,
private readonly databaseConfig: DatabaseConfig,
private readonly moduleRegistry: ModuleRegistry,
) {}
private get workflowService() {
@ -93,6 +94,12 @@ export class ProjectService {
);
}
private get dataTableService() {
return import('@/modules/data-table/data-store.service').then(({ DataStoreService }) =>
Container.get(DataStoreService),
);
}
async deleteProject(
user: User,
projectId: string,
@ -115,11 +122,12 @@ export class ProjectService {
targetProject = await this.getProjectWithScope(user, migrateToProject, [
'credential:create',
'workflow:create',
'dataStore:create',
]);
if (!targetProject) {
throw new NotFoundError(
`Could not find project to migrate to. ID: ${targetProject}. You may lack permissions to create workflow and credentials in the target project.`,
`Could not find project to migrate to. ID: ${targetProject}. You may lack permissions to create workflow, credentials or data tables in the target project.`,
);
}
}
@ -176,10 +184,21 @@ export class ProjectService {
// 5. delete shared workflows into this project
// Cascading deletes take care of this.
// 6. delete project
// 6. delete or migrate associated data tables
if (this.moduleRegistry.isActive('data-table')) {
const dataTableService = await this.dataTableService;
if (targetProject) {
await dataTableService.transferDataStoresByProjectId(project.id, targetProject.id);
} else {
await dataTableService.deleteDataStoreByProjectId(project.id);
}
}
// 7. delete project
await this.projectRepository.remove(project);
// 7. delete project relations
// 8. delete project relations
// Cascading deletes take care of this.
}

View File

@ -3063,10 +3063,10 @@
"projects.settings.role.viewer": "@:_reusableBaseText.roles.viewer",
"projects.settings.delete.title": "Delete \"{projectName}\" Project?",
"projects.settings.delete.message": "What should we do with the project data?",
"projects.settings.delete.message.empty": "There are no workflows or credentials in this project.",
"projects.settings.delete.question.transfer.label": "Transfer its workflows and credentials to another project or user",
"projects.settings.delete.message.empty": "There are no workflows, credentials or data tables in this project.",
"projects.settings.delete.question.transfer.label": "Transfer its workflows, credentials and data tables to another project or user",
"projects.settings.delete.question.transfer.title": "Project or user to transfer to",
"projects.settings.delete.question.wipe.label": "Delete its workflows and credentials",
"projects.settings.delete.question.wipe.label": "Delete its workflows, credentials and data tables",
"projects.settings.delete.question.wipe.title": "Type \"delete all data\" to confirm",
"projects.settings.delete.question.wipe.placeholder": "delete all data",
"projects.settings.delete.confirm": "Yes, I am sure",

View File

@ -25,8 +25,12 @@ describe('ProjectDeleteDialog', () => {
props: {
currentProject,
projects: [],
isCurrentProjectEmpty: true,
modelValue: true,
resourceCounts: {
credentials: 0,
workflows: 0,
dataTables: 0,
},
},
});
@ -43,8 +47,12 @@ describe('ProjectDeleteDialog', () => {
props: {
currentProject,
projects: [],
isCurrentProjectEmpty: false,
modelValue: true,
resourceCounts: {
credentials: 0,
workflows: 1,
dataTables: 0,
},
},
});

View File

@ -3,11 +3,12 @@ import { ref, computed } from 'vue';
import type { Project, ProjectListItem, ProjectSharingData } from '@/types/projects.types';
import ProjectSharing from '@/components/Projects/ProjectSharing.vue';
import { useI18n } from '@n8n/i18n';
import type { ResourceCounts } from '@/stores/projects.store';
type Props = {
currentProject: Project | null;
projects: ProjectListItem[];
isCurrentProjectEmpty: boolean;
resourceCounts: ResourceCounts;
};
const props = defineProps<Props>();
@ -21,13 +22,20 @@ const locale = useI18n();
const selectedProject = ref<ProjectSharingData | null>(null);
const operation = ref<'transfer' | 'wipe' | null>(null);
const wipeConfirmText = ref('');
const hasMovableResources = computed(
() =>
props.resourceCounts.credentials +
props.resourceCounts.workflows +
props.resourceCounts.dataTables >
0,
);
const isValid = computed(() => {
const expectedWipeConfirmation = locale.baseText(
'projects.settings.delete.question.wipe.placeholder',
);
return (
props.isCurrentProjectEmpty ||
!hasMovableResources.value ||
(operation.value === 'transfer' && !!selectedProject.value) ||
(operation.value === 'wipe' && wipeConfirmText.value === expectedWipeConfirmation)
);
@ -53,12 +61,12 @@ const onDelete = () => {
interpolate: { projectName: props.currentProject?.name ?? '' },
})
"
width="500"
width="650"
>
<n8n-text v-if="isCurrentProjectEmpty" color="text-base">{{
<n8n-text v-if="!hasMovableResources" color="text-base">{{
locale.baseText('projects.settings.delete.message.empty')
}}</n8n-text>
<div v-else>
<div v-else-if="hasMovableResources">
<n8n-text color="text-base">{{
locale.baseText('projects.settings.delete.message')
}}</n8n-text>

View File

@ -2,6 +2,7 @@ import { defineStore } from 'pinia';
import { ref, watch, computed } from 'vue';
import { useRoute } from 'vue-router';
import { useRootStore } from '@n8n/stores/useRootStore';
import * as dataStoreApi from '@/features/dataStore/dataStore.api';
import * as projectsApi from '@/api/projects.api';
import * as workflowsApi from '@/api/workflows';
import * as workflowsEEApi from '@/api/workflows.ee';
@ -19,6 +20,12 @@ import { getResourcePermissions } from '@n8n/permissions';
import type { CreateProjectDto, UpdateProjectDto } from '@n8n/api-types';
import { useSourceControlStore } from '@/stores/sourceControl.store';
export type ResourceCounts = {
credentials: number;
workflows: number;
dataTables: number;
};
export const useProjectsStore = defineStore(STORES.PROJECTS, () => {
const route = useRoute();
const rootStore = useRootStore();
@ -193,13 +200,18 @@ export const useProjectsStore = defineStore(STORES.PROJECTS, () => {
}
};
const isProjectEmpty = async (projectId: string) => {
const [credentials, workflows] = await Promise.all([
const getResourceCounts = async (projectId: string): Promise<ResourceCounts> => {
const [credentials, workflows, dataTables] = await Promise.all([
credentialsApi.getAllCredentials(rootStore.restApiContext, { projectId }),
workflowsApi.getWorkflows(rootStore.restApiContext, { projectId }),
dataStoreApi.fetchDataStoresApi(rootStore.restApiContext, projectId),
]);
return credentials.length === 0 && workflows.count === 0;
return {
credentials: credentials.length,
workflows: workflows.count,
dataTables: dataTables.count,
};
};
watch(
@ -263,6 +275,6 @@ export const useProjectsStore = defineStore(STORES.PROJECTS, () => {
getProjectsCount,
setProjectNavActiveIdByWorkflowHomeProject,
moveResourceToProject,
isProjectEmpty,
getResourceCounts,
};
});

View File

@ -264,7 +264,11 @@ describe('ProjectSettings', () => {
it('deletes project with transfer or wipe based on modal selection', async () => {
projectsStore.deleteProject.mockResolvedValue();
projectsStore.isProjectEmpty.mockResolvedValue(false);
projectsStore.getResourceCounts.mockResolvedValue({
credentials: 0,
workflows: 1,
dataTables: 0,
});
const r1 = renderComponent();
const deleteButton = r1.getByTestId('project-settings-delete-button');
@ -290,7 +294,12 @@ describe('ProjectSettings', () => {
// Case 2: Empty project, wiping directly (no transfer)
projectsStore.deleteProject.mockClear();
projectsStore.isProjectEmpty.mockResolvedValue(true);
projectsStore.getResourceCounts.mockResolvedValue({
credentials: 0,
workflows: 0,
dataTables: 0,
});
// unmount previous instance and re-render to reset dialog internal state
r1.unmount();
const r2 = renderComponent();

View File

@ -8,7 +8,7 @@ import { useDebounceFn } from '@vueuse/core';
import { useUsersStore } from '@/stores/users.store';
import type { IUser } from '@n8n/rest-api-client/api/users';
import { useI18n } from '@n8n/i18n';
import { useProjectsStore } from '@/stores/projects.store';
import { type ResourceCounts, useProjectsStore } from '@/stores/projects.store';
import type { Project, ProjectRelation } from '@/types/projects.types';
import { useToast } from '@/composables/useToast';
import { VIEWS } from '@/constants';
@ -49,7 +49,11 @@ const upgradeDialogVisible = ref(false);
const isDirty = ref(false);
const isValid = ref(false);
const isCurrentProjectEmpty = ref(true);
const resourceCounts = ref<ResourceCounts>({
credentials: -1,
dataTables: -1,
workflows: -1,
});
const formData = ref<Pick<Project, 'name' | 'description' | 'relations'>>({
name: '',
description: '',
@ -346,9 +350,7 @@ const onDelete = async () => {
await projectsStore.getAvailableProjects();
if (projectsStore.currentProjectId) {
isCurrentProjectEmpty.value = await projectsStore.isProjectEmpty(
projectsStore.currentProjectId,
);
resourceCounts.value = await projectsStore.getResourceCounts(projectsStore.currentProjectId);
}
dialogVisible.value = true;
@ -607,7 +609,7 @@ onMounted(() => {
<ProjectDeleteDialog
v-model="dialogVisible"
:current-project="projectsStore.currentProject"
:is-current-project-empty="isCurrentProjectEmpty"
:resource-counts="resourceCounts"
:projects="projects"
@confirm-delete="onConfirmDelete"
/>