[Cases] Delete Cases API Guardrails #160846
Conversation
Tests. Documentation
adcoelho
changed the title
adcoelho
added
Feature:CI
|
Pinging @elastic/response-ops (Team:ResponseOps) |
|
Pinging @elastic/response-ops-cases (Feature:Cases) |
| @@ -106,6 +106,7 @@ export const MAX_CONCURRENT_SEARCHES = 10 as const; | |||
| export const MAX_BULK_GET_CASES = 1000 as const; | |||
| export const MAX_COMMENTS_PER_PAGE = 100 as const; | |||
| export const MAX_CATEGORY_FILTER_LENGTH = 100 as const; | |||
| export const MAX_DELETE_IDS_LENGTH = 100 as const; | |||
There was a problem hiding this comment.
super nit: I think we should move it to Validation section below.
| @@ -95,6 +95,22 @@ describe('delete', () => { | |||
| }); | |||
| }); | |||
| }); | |||
|
|
|||
| describe('errors', () => { | |||
| it(`throws 400 when trying to delete more than ${MAX_DELETE_IDS_LENGTH} files at a time`, async () => { | |||
There was a problem hiding this comment.
This should be testing the MAX_DELETE_IDS_LENGTH, right?
There was a problem hiding this comment.
Yup, do you mean the test name? I left files there by mistake 😅
There was a problem hiding this comment.
Yes, for some reason I thought the MAX_DELETE_IDS_LENGTH was only for the files.
| @@ -26,7 +28,10 @@ import { createFileEntities, deleteFiles } from '../files'; | |||
| /** | |||
| * Deletes the specified cases and their attachments. | |||
| */ | |||
| export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): Promise<void> { | |||
| export async function deleteCases( | |||
There was a problem hiding this comment.
I noticed we also define a params schema in x-pack/plugins/cases/server/routes/api/cases/delete_cases.ts. Can you also put the validation there? (using the @kbn/schema)
There was a problem hiding this comment.
params: {
query: schema.object({
ids: schema.arrayOf(schema.string()),
}),
}
What do you mean with
(using the @kbn/schema)
?
I am not familiar with this '@kbn/config-schema';. I see some routes use it for the parameters(not all though) is it used for validation anywhere, how does it work?
I guess I can just replace this with the limitedArraySchema definition I created.
There was a problem hiding this comment.
We discuss it offline and we decided that we are going to remove the schema defined in all cases routes and rely only on the validation inside the cases client.
💚 Build Succeeded
Metrics [docs]Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @adcoelho |
Connected to #146945
Summary
Checklist
Delete any items that are not applicable to this PR.
Release notes
The Delete cases API now limits the number of cases to be deleted to 100.