fix(core): Update data table delete row endpoint to accept filter as string (no-changelog) (#19673)

This commit is contained in:
Daria
2025-09-17 21:56:35 +03:00
committed by GitHub
parent 2598e735b1
commit 51b8f8c7dc
7 changed files with 179 additions and 27 deletions

View File

@@ -1,11 +1,64 @@
import { jsonParse } from 'n8n-workflow';
import { z } from 'zod';
import { Z } from 'zod-class';
import { dataTableFilterSchema } from '../../schemas/data-table-filter.schema';
const dataTableFilterQueryValidator = z.string().transform((val, ctx) => {
if (!val) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: 'Filter is required for delete operations',
path: ['filter'],
});
return z.NEVER;
}
try {
const parsed: unknown = jsonParse(val);
try {
// Parse with the schema which applies defaults
const result = dataTableFilterSchema.parse(parsed);
// Ensure filters array is not empty
if (!result.filters || result.filters.length === 0) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: 'At least one filter condition is required for delete operations',
path: ['filter'],
});
return z.NEVER;
}
return result;
} catch (e) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: 'Invalid filter fields',
path: ['filter'],
});
return z.NEVER;
}
} catch (e) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: 'Invalid filter format',
path: ['filter'],
});
return z.NEVER;
}
});
const returnDataValidator = z
.union([z.string(), z.boolean()])
.optional()
.transform((val) => {
if (typeof val === 'string') {
return val === 'true';
}
return val ?? false;
});
const deleteDataTableRowsShape = {
filter: dataTableFilterSchema.optional(),
returnData: z.boolean().optional().default(false),
filter: dataTableFilterQueryValidator,
returnData: returnDataValidator,
};
export class DeleteDataTableRowsDto extends Z.class(deleteDataTableRowsShape) {}

View File

@@ -2507,6 +2507,12 @@ describe('DELETE /projects/:projectId/data-tables/:dataStoreId/rows', () => {
test('should not delete rows when project does not exist', async () => {
await authOwnerAgent
.delete('/projects/non-existing-id/data-tables/some-data-store-id/rows')
.query({
filter: JSON.stringify({
type: 'and',
filters: [{ columnName: 'first', condition: 'eq', value: 'test value' }],
}),
})
.expect(403);
});
@@ -2515,9 +2521,53 @@ describe('DELETE /projects/:projectId/data-tables/:dataStoreId/rows', () => {
await authOwnerAgent
.delete(`/projects/${project.id}/data-tables/non-existing-id/rows`)
.query({
filter: JSON.stringify({
type: 'and',
filters: [{ columnName: 'first', condition: 'eq', value: 'test value' }],
}),
})
.expect(404);
});
test('should not delete rows when no filter is provided', async () => {
const project = await createTeamProject('test project', owner);
const dataStore = await createDataStore(project, {
columns: [
{
name: 'first',
type: 'string',
},
],
});
await authOwnerAgent
.delete(`/projects/${project.id}/data-tables/${dataStore.id}/rows`)
.expect(400);
});
test('should not delete rows when filter has empty filters array', async () => {
const project = await createTeamProject('test project', owner);
const dataStore = await createDataStore(project, {
columns: [
{
name: 'first',
type: 'string',
},
],
});
await authOwnerAgent
.delete(`/projects/${project.id}/data-tables/${dataStore.id}/rows`)
.query({
filter: {
type: 'and',
filters: [],
},
})
.expect(400);
});
test("should not delete rows in another user's personal project data store", async () => {
const dataStore = await createDataStore(ownerProject, {
columns: [
@@ -2540,6 +2590,12 @@ describe('DELETE /projects/:projectId/data-tables/:dataStoreId/rows', () => {
await authMemberAgent
.delete(`/projects/${ownerProject.id}/data-tables/${dataStore.id}/rows`)
.query({
filter: JSON.stringify({
type: 'and',
filters: [{ columnName: 'first', condition: 'eq', value: 'test value' }],
}),
})
.expect(403);
const rowsInDb = await dataStoreRowsRepository.getManyAndCount(dataStore.id, {});
@@ -2570,6 +2626,12 @@ describe('DELETE /projects/:projectId/data-tables/:dataStoreId/rows', () => {
await authMemberAgent
.delete(`/projects/${project.id}/data-tables/${dataStore.id}/rows`)
.query({
filter: JSON.stringify({
type: 'and',
filters: [{ columnName: 'first', condition: 'eq', value: 'test value' }],
}),
})
.expect(403);
const rowsInDb = await dataStoreRowsRepository.getManyAndCount(dataStore.id, {});
@@ -2609,14 +2671,14 @@ describe('DELETE /projects/:projectId/data-tables/:dataStoreId/rows', () => {
await authMemberAgent
.delete(`/projects/${project.id}/data-tables/${dataStore.id}/rows`)
.send({
filter: {
.query({
filter: JSON.stringify({
type: 'or',
filters: [
{ columnName: 'first', condition: 'eq', value: 'test value 1' },
{ columnName: 'first', condition: 'eq', value: 'test value 3' },
],
},
}),
})
.expect(200);
@@ -2657,11 +2719,11 @@ describe('DELETE /projects/:projectId/data-tables/:dataStoreId/rows', () => {
await authAdminAgent
.delete(`/projects/${project.id}/data-tables/${dataStore.id}/rows`)
.send({
filter: {
.query({
filter: JSON.stringify({
type: 'and',
filters: [{ columnName: 'first', condition: 'eq', value: 'test value 2' }],
},
}),
})
.expect(200);
@@ -2701,11 +2763,11 @@ describe('DELETE /projects/:projectId/data-tables/:dataStoreId/rows', () => {
await authOwnerAgent
.delete(`/projects/${project.id}/data-tables/${dataStore.id}/rows`)
.send({
filter: {
.query({
filter: JSON.stringify({
type: 'and',
filters: [{ columnName: 'first', condition: 'eq', value: 'test value 2' }],
},
}),
})
.expect(200);
@@ -2744,11 +2806,11 @@ describe('DELETE /projects/:projectId/data-tables/:dataStoreId/rows', () => {
await authMemberAgent
.delete(`/projects/${memberProject.id}/data-tables/${dataStore.id}/rows`)
.send({
filter: {
.query({
filter: JSON.stringify({
type: 'and',
filters: [{ columnName: 'first', condition: 'eq', value: 'test value 2' }],
},
}),
})
.expect(200);
@@ -2787,11 +2849,11 @@ describe('DELETE /projects/:projectId/data-tables/:dataStoreId/rows', () => {
const result = await authMemberAgent
.delete(`/projects/${memberProject.id}/data-tables/${dataStore.id}/rows`)
.send({
filter: {
.query({
filter: JSON.stringify({
type: 'and',
filters: [{ columnName: 'first', condition: 'eq', value: 'test value 3' }],
},
}),
returnData: true,
});

View File

@@ -1813,7 +1813,7 @@ describe('dataStore', () => {
expect(count).toBe(1);
});
it('should delete all rows when no filter is provided', async () => {
it('should not delete all rows when no filter is provided', async () => {
// ARRANGE
const dataStore = await dataStoreService.createDataStore(project1.id, {
name: 'dataStore',
@@ -1828,13 +1828,42 @@ describe('dataStore', () => {
]);
// ACT
const result = await dataStoreService.deleteRows(dataStoreId, project1.id, {});
const result = dataStoreService.deleteRows(dataStoreId, project1.id, {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
filter: undefined as any,
});
// ASSERT
expect(result).toBe(true);
await expect(result).rejects.toThrow(
new DataStoreValidationError(
'Filter is required for delete operations to prevent accidental deletion of all data',
),
);
});
const { count } = await dataStoreService.getManyRowsAndCount(dataStoreId, project1.id, {});
expect(count).toBe(0);
it('should not delete all rows when no filter is empty', async () => {
// ARRANGE
const dataStore = await dataStoreService.createDataStore(project1.id, {
name: 'dataStore',
columns: [{ name: 'name', type: 'string' }],
});
const { id: dataStoreId } = dataStore;
await dataStoreService.insertRows(dataStoreId, project1.id, [
{ name: 'Alice' },
{ name: 'Bob' },
{ name: 'Charlie' },
]);
// ACT
const result = dataStoreService.deleteRows(dataStoreId, project1.id, {
filter: { type: 'and', filters: [] },
});
await expect(result).rejects.toThrow(
new DataStoreValidationError(
'Filter is required for delete operations to prevent accidental deletion of all data',
),
);
});
});

View File

@@ -334,7 +334,7 @@ export class DataStoreController {
req: AuthenticatedRequest<{ projectId: string }>,
_res: Response,
@Param('dataTableId') dataTableId: string,
@Body dto: DeleteDataTableRowsDto,
@Query dto: DeleteDataTableRowsDto,
) {
try {
return await this.dataStoreService.deleteRows(

View File

@@ -288,10 +288,14 @@ export class DataStoreService {
);
}
if (dto.filter?.filters && dto.filter.filters.length !== 0) {
this.validateAndTransformFilters(dto.filter, columns);
if (!dto.filter?.filters || dto.filter.filters.length === 0) {
throw new DataStoreValidationError(
'Filter is required for delete operations to prevent accidental deletion of all data',
);
}
this.validateAndTransformFilters(dto.filter, columns);
const result = await this.dataStoreRowsRepository.deleteRows(
dataStoreId,
columns,

View File

@@ -50,6 +50,10 @@ export async function execute(
const filter = getSelectFilter(this, index);
if (filter.filters.length === 0) {
throw new NodeOperationError(this.getNode(), 'At least one condition is required');
}
if (dryRun) {
const { data: rowsToDelete } = await dataStoreProxy.getManyRowsAndCount({ filter });
return rowsToDelete.map((json) => ({ json }));

View File

@@ -68,7 +68,7 @@ export type UpsertDataStoreRowOptions = {
};
export type DeleteDataTableRowsOptions = {
filter?: DataTableFilter;
filter: DataTableFilter;
};
export type MoveDataStoreColumnOptions = {