-
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] Create internal API to filter cases by custom fields #170297
[Cases] Create internal API to filter cases by custom fields #170297
Conversation
Pinging @elastic/response-ops (Team:ResponseOps) |
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.
lgtm!
Also played around a bit in Kibana to check _find
's behavior wasn't affected. It was fine 👍
…isalvi/kibana into filter-custom-fields-api
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.
Great work! I really like the abstraction with the mapping 🚀. I left some comments.
x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/search_cases.ts
Outdated
Show resolved
Hide resolved
x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/search_cases.ts
Outdated
Show resolved
Hide resolved
x-pack/test/cases_api_integration/security_and_spaces/tests/common/internal/search_cases.ts
Outdated
Show resolved
Hide resolved
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.
Great work @js-jankisalvi 🚀 ! Thank you for your patience with the review.
if (!paramArgs.owner || !paramArgs.owner.length) { | ||
throw Boom.badRequest('In order to filter cases by customFields, you must provide owner.'); | ||
} | ||
|
||
if (isArray(paramArgs.owner) && paramArgs.owner.length > 1) { | ||
throw Boom.badRequest( | ||
'In order to filter cases by customFields, you must provide only one owner.' | ||
); | ||
} |
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.
What do you think of combining the two messages?
if (isArray(paramArgs.owner) && paramArgs.owner.length > 1) {
throw Boom.badRequest(
'Owner must be provided. Multiple owners are not supported.'
);
}
* throw error if request body does not have owner | ||
*/ | ||
if (!Object.hasOwn(options, 'owner')) { | ||
throw Boom.badRequest('Search cases must have a valid owner.'); |
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.
nit: What about Owner is required.
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: |
Summary
Implements #167740
This PR
search_cases
internal route with POSTexample of search cases body:
{ "customFields": { "custom_field_key": [true] } }
Checklist
For maintainers