Skip to content
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] Restrict searchFields, sortField and remove rootSearchFields from find_cases API #162245

Merged

Conversation

js-jankisalvi
Copy link
Contributor

Summary

This PR modifies find_cases API with below changes

  • Restrict searchFields only to the title and the description
  • Restrict sortField to: title, category, createdAt, updatedAt, status, and severity
  • Remove rootSearchFields

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@js-jankisalvi js-jankisalvi added release_note:breaking Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.10.0 labels Jul 19, 2023
@js-jankisalvi js-jankisalvi self-assigned this Jul 19, 2023
@js-jankisalvi js-jankisalvi requested a review from a team as a code owner July 19, 2023 13:20
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops-cases (Feature:Cases)

@js-jankisalvi js-jankisalvi requested a review from a team as a code owner July 19, 2023 14:04
Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I left a comment about searching by the case ID. We should keep this functionality and not remove it.

@@ -35,7 +35,6 @@ type FindOptions = Pick<
| 'searchFields'
| 'sortField'
| 'sortOrder'
| 'rootSearchFields'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is needed because is used by SavedObjectFindOptionsKueryNode and the results of constructSearch is being passed to functions that use the SavedObjectFindOptionsKueryNode type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh, this common for all services and not only for find cases.

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! Tested locally and the search by case ID and the sorting is working as expected.

@@ -394,7 +394,7 @@ export default ({ getService }: FtrProviderContext): void => {
expect(cases.cases[0].title).to.equal(uuid);
});

it('should successfully find a case with a valid uuid in title', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why were these two tests with the same name not throwing a listing error!?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no idea why 😆

Copy link
Contributor

@adcoelho adcoelho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 🔥

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #7 / Alert details expandable flyout right panel overview tab insights section should display threat intelligence section
  • [job] [logs] Defend Workflows Endpoint Cypress Tests #1 / Artifact pages Trusted applications "after all" hook for "should update Endpoint Policy on Endpoint when adding Trusted application name"
  • [job] [logs] Defend Workflows Endpoint Cypress Tests #1 / Artifact pages Trusted applications "before all" hook for "should update Endpoint Policy on Endpoint when adding Trusted application name"
  • [job] [logs] Defend Workflows Cypress Tests #1 / Endpoint Policy Response from Fleet Agent Details page should display policy response with errors

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
cases 148.3KB 147.6KB -644.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @js-jankisalvi

Copy link
Member

@machadoum machadoum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explore changes LGTM!

@js-jankisalvi js-jankisalvi merged commit 0af4e7e into elastic:main Jul 24, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 24, 2023
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
…from find_cases API (elastic#162245)

## Summary

This PR modifies` find_cases API` with below changes
- [x] Restrict `searchFields` only to the `title` and the `description`
- [x] Restrict `sortField` to: `title`, `category`, `createdAt`,
`updatedAt`, `status`, and `severity`
- [x] Remove `rootSearchFields`

### Checklist

Delete any items that are not applicable to this PR.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@js-jankisalvi js-jankisalvi deleted the restrict-params-find-cases-api branch February 19, 2024 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Cases Cases feature release_note:breaking Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants