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.

Sign up for GitHub

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

Jump to bottom

[ResponseOps][AlertsTable] Migrate API hooks to React Query #186448

Closed
2 tasks done
cnasikas opened this issue Jun 19, 2024 · 2 comments
Closed
2 tasks done

[ResponseOps][AlertsTable] Migrate API hooks to React Query #186448

cnasikas opened this issue Jun 19, 2024 · 2 comments
Assignees
@umbopepato
Labels
Feature:Alerting/AlertsTable Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@cnasikas
Copy link
Member

cnasikas commented Jun 19, 2024 •
edited
Loading

The alerts table uses various hooks to make API calls. Most of the hooks are using React Query to perform these API calls, The useFetchAlerts and useFetchBrowserFieldCapabilities do not. We should migrate the rest of the hook to use React Query.

DoD
Preview Give feedback
@botelastic botelastic bot added the needs-team Issues missing a team label label Jun 19, 2024
@cnasikas cnasikas added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/AlertsTable and removed needs-team Issues missing a team label labels Jun 19, 2024
@elasticmachine
Copy link
Contributor

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

@cnasikas cnasikas mentioned this issue Jun 20, 2024
Open
This was referenced Jul 10, 2024
Merged
Merged
@umbopepato
Copy link
Member

umbopepato commented Jul 15, 2024 •
edited
Loading

While doing this refactor I'm taking the occasion to try and standardize the naming and shape of TanStack Query-based hooks in the alerts area.

Naming rule

   use     +     FetchAlerts   +    Query/Mutation
    |                 |                    |
   hook          name of the      to highlight the fact
                corresponding    that it returns a query
                fetch function    or mutation object

Arguments

  • params for the parameters that contribute to the data fetching and computation of the query key,
  • options for any options overrides the user wants to apply to the query.

Return

The whole query or mutation object, to better standardize the return types across different data fetching hooks and to give the user the freedom to access loading states, refetch functions etc.

@cnasikas cnasikas mentioned this issue Jul 15, 2024
Open
umbopepato added a commit that referenced this issue Jul 25, 2024
@umbopepato @kibanamachine
[ResponseOps][Alerts] Migrate alerts fetching to TanStack Query (#186978
bd3032b 
)

## Summary

Implements a new `useSearchAlertsQuery` hook based on TanStack Query to
replace the `useFetchAlerts` hook, following [this organizational
logic](#186448 (comment)).

This PR focuses mainly on the fetching logic itself, leaving the
surrounding API surface mostly unchanged since it will be likely
addressed in subsequent PRs.

## To verify

1. Create rules that fire alerts in different solutions
2. Check that the alerts table usages work correctly ({O11y, Security,
Stack} alerts and rule details pages, ...)
1. Check that the alerts displayed in the table are coherent with the
solution, KQL query, time filter, pagination
    2. Check that pagination changes are reflected in the table
3. Check that changing the query when in pages > 0 resets the pagination
to the first page

Closes point 1 of #186448
Should fix #171738

### Checklist

- [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

---------

Co-authored-by: kibanamachine <[email protected]>
umbopepato added a commit that referenced this issue Jul 29, 2024
@umbopepato @kibanamachine
[ResponseOps][Alerts] Migrate alerts fields fetching to TanStack Query (
f24e0cd 
#188320)

## Summary

Migrates the `useFetchBrowserFieldCapabilities` hook (renamed to
`useFetchAlertsFieldsQuery`) to TanStack Query, following [this
organizational
logic](#186448 (comment)).

This PR focuses mainly on the fetching logic itself, leaving the
surrounding API surface mostly unchanged since it will be likely
addressed in subsequent PRs.

## To verify

1. Create rules that fire alerts in different solutions
2. Check that the alerts table usages work correctly ({O11y, Security,
Stack} alerts and rule details pages, ...)
1. Check that the alerts displayed in the table are coherent with the
solution, KQL query, time filter, pagination
    2. Check that pagination changes are reflected in the table
3. Check that changing the query when in pages > 0 resets the pagination
to the first page
4. Check that the fields browser shows and works correctly (`Fields`
button in the alerts table header)

Closes point 2 of #186448

### Checklist

- [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

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@umbopepato umbopepato

Labels
Feature:Alerting/AlertsTable Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@cnasikas @elasticmachine @umbopepato