-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Cases] Change the max files that can be bulk deleted. #160716
Conversation
Pinging @elastic/response-ops (Team:ResponseOps) |
Pinging @elastic/response-ops-cases (Feature:Cases) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's modify this test: https://github.com/elastic/kibana/blob/main/x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/bulk_delete_file_attachments.ts#L142 to say and use 11 file ids.
await expect( | ||
bulkDeleteFileAttachments({ caseId: 'mock-id-4', fileIds }, clientArgs, casesClient) | ||
).rejects.toThrowError( | ||
'Failed to delete file attachments for case: mock-id-4: Error: array must be of length <= 10' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here about the message. It should be a bit more explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure if that can be done.
The schema/type has a message for when it fails but it has no knowledge of the variable name that is failing. So I cannot really say FileIds error: array must be etc etc
.
One solution would be something like creating limitedFileIdsArray
just so it could have a specific error message but it kind of defeats the purpose of having a generic limitedArray
type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can pass a message
variable to the limitedArray
function to customize the message. I am talking about the 'array must be of length >= ${min}'
which seems is being set in the limitedArray
function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also pass a field name to schema and include in message. I am currently doing like this The length of the ${field} is too long. The maximum length is ${max}.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh even better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the PR with this logic 🙌
💚 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 with #146945
Summary
Checklist