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

[Data Explorer] Persisted state and syncing, shared context, linking state to component + smaller changes #4678

Conversation

ashwin-pc
Copy link
Member

Description

This PR might look big but its actually just 3 changes and a few cleanup tasks that simplify the app structure.

Old PR #4652

Changes:

  1. Adds persistent storage using the URL for the following and sets them up to sync
    1. global state _g
    2. app state _a
    3. query state _q
  2. Adds a shared context in Data Explorer's view service to share context between the panel and canvas view components. Discover uses this to share the data fetching observable.
  3. Wired up the doc table, sidenav and the querybar to the application state.

Other minor changes include:

  1. renaming docViewExpand to expandedHit to make it clearer what it contains
  2. minor styling fixes for the sidebar
  3. Simplified the Doc Table structure by eliminating similar components
  4. Added back the DocViewLinks that was deleted in an older commit. (This is still WIP but added to keep the API intact)
  5. Adds the initial logic to handle routing changes from legacy discover to data explorer. Currently this only migrates older discover state to data explorer state and is limited to src/plugins/discover/public/migrate_state.ts and where ever its referenced.
  6. Disabling functional test for now until merge to main since they will anyways fail and will be reactivated before a merge to main.

Note: The commit history has changes that have already been merged but the diff is accurate.

Issues Resolved

closes #4225
closes #4227
closes #4228

Screenshot

Ignore the layout issues, this will be fixed later

Screen.Recording.2023-08-01.at.4.15.59.AM.mov

Testing the changes

To test the state syncing:

  1. for global state, modify the time range and refresh to see that the state persists. The data in the doc table should also update
  2. for app state, add colums and see the URL update. Again this should persist on refresh and the doc table should change
  3. For query state, update the search query and do the same as the above

To test the shared context, updating the columns should update the table and vice versa and only one network request should fire for an update in the netwprk tab to the opensearch route

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Ashwin P Chandran <[email protected]>
Signed-off-by: Ashwin P Chandran <[email protected]>
@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #4678 (4b7498b) into feature/data-explorer (521f306) will decrease coverage by 0.04%.
The diff coverage is 44.82%.

@@                    Coverage Diff                    @@
##           feature/data-explorer    #4678      +/-   ##
=========================================================
- Coverage                  66.50%   66.47%   -0.04%     
=========================================================
  Files                       3289     3292       +3     
  Lines                      62821    62860      +39     
  Branches                    9788     9793       +5     
=========================================================
+ Hits                       41782    41788       +6     
- Misses                     18658    18696      +38     
+ Partials                    2381     2376       -5     
Flag Coverage Δ
Windows 66.47% <44.82%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
.../data_explorer/public/components/app_container.tsx 40.00% <0.00%> (ø)
.../data_explorer/public/components/sidebar/index.tsx 3.44% <0.00%> (ø)
...tion/components/sidebar/discover_field_details.tsx 80.00% <ø> (ø)
...over/public/application/components/table/table.tsx 84.61% <ø> (ø)
src/plugins/discover/public/build_services.ts 0.00% <ø> (ø)
src/plugins/discover/public/migrate_state.ts 0.00% <0.00%> (ø)
src/plugins/discover/public/plugin.ts 0.00% <0.00%> (ø)
...ta_explorer/public/utils/state_management/store.ts 20.68% <20.00%> (+0.68%) ⬆️
...pplication/components/sidebar/lib/group_fields.tsx 84.61% <25.00%> (-10.84%) ⬇️
...pplication/components/sidebar/discover_sidebar.tsx 66.66% <33.33%> (-11.72%) ⬇️
... and 9 more

... and 1 file with indirect coverage changes

Signed-off-by: Ashwin P Chandran <[email protected]>
Signed-off-by: Ashwin P Chandran <[email protected]>
@ashwin-pc ashwin-pc merged commit 47f257d into opensearch-project:feature/data-explorer Aug 7, 2023
17 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data explorer Issues related to the Data Explorer project de-angular de-angularize work discover for discover reinvent distinguished-contributor
Projects
Development

Successfully merging this pull request may close these issues.

2 participants