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

[Infrastructure UI] Saved object hooks that use the SO client are removed in favour of the new endpoints #154725

Closed
Tracked by #152617
tonyghiani opened this issue Apr 11, 2023 · 1 comment · Fixed by #155827
Assignees
Labels
blocked Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@tonyghiani
Copy link
Contributor

tonyghiani commented Apr 11, 2023

📓 Summary

Depends on #152617

As a consequence of the creation of domain endpoints for handling the CRUD operation related to the inventory-view and metrics-explorer-view saved objects, we need to update the client-side logic to migrate from the savedObjectClient to consume these new endpoints.

✔️ Acceptance criteria

  • Implement new useInventoryView and useMetricExplorerView custom hooks
  • useCreateSavedObject, useGetSavedObject, useFindSavedObject, useUpdateSavedObject, useDeleteSavedObject are removed/modified to consume the new API.
  • useBulkGetSavedObject is not used anywhere and should be removed.
  • useDataView should be updated accordingly to consume the data views services
@botelastic botelastic bot added the needs-team Issues missing a team label label Apr 11, 2023
@tonyghiani tonyghiani changed the title Saved object hooks that use the SO client are removed in favour of the new endpoints: useCreateSavedObject, useGetSavedObject, useBulkGetSavedObject, useUpdateSavedObject, useDeleteSavedObject [Infrastructure UI] Saved object hooks that use the SO client are removed in favour of the new endpoints Apr 12, 2023
@tonyghiani tonyghiani self-assigned this Apr 12, 2023
@tonyghiani tonyghiani added blocked Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Apr 12, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Apr 12, 2023
tonyghiani added a commit that referenced this issue Apr 25, 2023
… hooks (#155174)

## 📓  Summary

Part of #152617 

This PR does some refactoring on the presentational components used to
render the saved views on both Infra Inventory and Metrics Explorer,
preparing for the additional work required once [[Infrastructure UI]
Saved object hooks that use the SO client are removed in favour of the
new endpoints#154725](#154725)
will be implemented.

## 🐞 Bug fixes

While working on this code, some pre-existing issues have been
discovered and fixed:
- "Make default" star icon was not aligned correctly when rendered
alone:
<img width="751" alt="Screenshot 2023-04-19 at 15 22 24"
src="https://user-images.githubusercontent.com/34506779/233088425-34992395-4d18-46bc-9124-5d99101406ce.png">

- Delete view confirm prompt not closed after removing a view:


https://user-images.githubusercontent.com/34506779/233088780-9b1bfe57-170c-4e66-9303-f41448eb8447.mov

---------

Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
tonyghiani added a commit that referenced this issue Apr 26, 2023
…ient (#155126)

## 📓 Summary

Depends on #154900
Closes #155110 

This PR implements the `InventoryViewsService` and
`InventoryViewsClient`, injecting an instance of the client in the
KibanaContextForPlugin and exposing so a set of utilities to
retrieve/update inventory views:
- `findInventoryViews`
- `getInventoryView`
- `createInventoryView`
- `updateInventoryView`
- `deleteInventoryView`

## 👣 Next steps
- Implement #154725 to consume the service

---------

Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
tonyghiani added a commit that referenced this issue Apr 27, 2023
…lorerViewsClient (#155878)

## 📓 Summary

Depends on #154900
Closes #155112  

This PR implements the `InventoryViewsService` and
`InventoryViewsClient`, injecting an instance of the client in the
KibanaContextForPlugin and exposing so a set of utilities to
retrieve/update inventory views:
- `findMetricsExplorerViews`
- `getMetricsExplorerView`
- `createMetricsExplorerView`
- `updateMetricsExplorerView`
- `deleteMetricsExplorerView`

## 👣 Next steps
- Implement #154725 to consume the service

---------

Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com>
tonyghiani added a commit that referenced this issue May 2, 2023
## 📓 Summary

Part of #152617
Closes #106650
Closes #154725

This PR replace the existing client React logic for handling saved views
on the `infra` plugin with a new state management implementation that
interacts with the newly created API.
It brings the following changes:
- Implement `useInventoryViews` and `useMetricsExplorerViews` custom
hooks.
- Adopt `@tanstack/react-query` for the above hooks implementation as it
was already used across the plugin and simplifies the server state
management. Extract the provider for the react query cache.
- Update server services to fix an issue while updating views, which was
preventing the unset of properties from the view.
- Update Saved Views components to integrate the new hooks.
- The `Load views` option has been removed accordingly to the decision
made with the UX team, since it wasn't adding any value that wasn't
already present in the `Manage views` option.

Even if we are duplicating similar logic to handle the Inventory and
Metrics Explorer views, we decided to keep them separated to easily
control their behaviour and avoid coupled logic that can be painful to
split in future.

## 🐞 Bug fixes

This implementation also fixed some existing bugs in production:
- As reported in [this
comment](#155174 (review)),
when the current view is deleted, the selector doesn't fallback on
another view and keeps showing the same view title.
It has been fixed and the selected view fallbacks to the default view.
- When refreshing the page after a view was selected, the view was not
correctly recovered and shown. The implemented changes fix this
behaviour.
- The "include time" option for creating/updating a saved view was not
working and never removed the time property if disabled.
- Minor visual adjustments such as action button type and alignment.

## 👨‍💻 Review hints

The best way to verify all the interactions and loadings work correctly
as a user expects, running the branch locally with an oblt cluster is
recommended.
In both the inventory and metrics explorer pages, the user should be
able to:
- Access and manage the saved views, select and load a view, delete a
view, and set a view as default.
- Save a new view.
- Update the currently used view, except for the static **Default
view**.
- Show an error when trying to create/update a view with a name already
held by another view.
- Restore the view with the following priority order
  - Use from the URL the stored view id to restore the view
- Use the default view id stored in the source configuration as a user
preference
  - Use the static **Default view**

## 👣 Following steps

- [ ] #155117

---------

Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: jennypavlova <jennypavlova94@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants