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

[Discover] Refactor and simplify state container #142131

Closed
5 tasks done
kertal opened this issue Sep 28, 2022 · 1 comment · Fixed by #153528
Closed
5 tasks done

[Discover] Refactor and simplify state container #142131

kertal opened this issue Sep 28, 2022 · 1 comment · Fixed by #153528
Assignees
Labels
chore Feature:Discover Discover Application impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.

Comments

@kertal
Copy link
Member

kertal commented Sep 28, 2022

Summary

Discover's state management is scattered around the code base (saved search, appState, dataState via observables, access and usage of search source, data view list state, ...). It should be cleaned up and centralized. Targets:

  • A clearer structure
  • Better separation from UI,
  • Removing of hook dependencies making it hard to debug the code (recent implementations like ad-hoc data views and the Log explorer effort clearly showed it's pain points)
  • Omit regressions
  • Making it ready for next steps like migrating to redux-toolkit

There's already a state container in the code (discover_state.ts), which exports a function getState. it's primary usage is the providing appStateContainer, which is a redux like state container that's synced with the URL. This should be extended to be Discover's main state container by adding a "savedSearchContainer" and "dataStateContainer" and by providing functions to transform the state in the user interface. It could look like this:

export interface DiscoverStateContainer {
  /**
   * App state, the _a part of the URL - This is btw a redux like state
   */
  appStateContainer: ReduxLikeStateContainer<AppState>;
  /**
  * New, to access savedSearch related functionality (also for searchSource) 
  **/
  savedSearchContainer
 /**
 * New, data fetching related state migrated from `use_saved_search.ts`
 **/
  dataStateContainer
...
some more stuff from the old container, what's more we should have way to reload data views
...
 /**
functionality triggered from UI
**/
  actions: {
    resetSavedSearch: (id: string) => void;
    onOpenSavedSearch: (newSavedSearchId: string) => void;
    onUpdateQuery: (
      payload: { dateRange: TimeRange; query?: Query | AggregateQuery },
      isUpdate?: boolean
    ) => void;
    updateSavedQueryId: (newSavedQueryId: string | undefined) => void;
    /**
     * Pause the auto refresh interval without pushing an entry to history
     */
    pauseAutoRefreshInterval: () => Promise<void>;
    fetch: (reset?: boolean) => void;

  }
}
@kertal kertal added chore Feature:Discover Discover Application Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Sep 28, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Feature:Discover Discover Application impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants