chore(explore): Get Explore data from endpoint instead of bootstrap_data #20519

kgabryje · 2022-06-28T09:38:02Z

SUMMARY

This PR detaches Explore page from bootstrap data.
Getting slice, form_data and datasource is done by calling the new /v1/explore endpoint.
This is a prerequisite to Explore SPA project.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Everything should work like before (though a bit slower until we finish the rest of Explore work).
Make sure to test different initial urls (go to explore from charts list, dashboard, permalink...)

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

codecov · 2022-06-28T10:49:47Z

Codecov Report

Merging #20519 (04e17e4) into master (cadd259) will increase coverage by 0.01%.
The diff coverage is 51.07%.

❗ Current head 04e17e4 differs from pull request most recent head 99a1ebc. Consider uploading reports for the commit 99a1ebc to get more accurate results

@@            Coverage Diff             @@
##           master   #20519      +/-   ##
==========================================
+ Coverage   66.78%   66.80%   +0.01%     
==========================================
  Files        1752     1754       +2     
  Lines       65461    65562     +101     
  Branches     6916     6932      +16     
==========================================
+ Hits        43719    43799      +80     
- Misses      19994    20009      +15     
- Partials     1748     1754       +6

Flag	Coverage Δ
javascript	`51.82% <52.06%> (+0.01%)`	⬆️

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

Impacted Files	Coverage Δ
...d/packages/superset-ui-chart-controls/src/types.ts	`100.00% <ø> (ø)`
...lugin-chart-echarts/src/BigNumber/BigNumberViz.tsx	`0.00% <ø> (ø)`
superset-frontend/src/SqlLab/types.ts	`57.14% <ø> (ø)`
...rset-frontend/src/components/Chart/chartReducer.ts	`25.00% <0.00%> (ø)`
...t-frontend/src/dashboard/actions/dashboardState.js	`36.54% <ø> (ø)`
superset-frontend/src/dashboard/actions/hydrate.js	`2.06% <ø> (ø)`
...hboard/components/nativeFilters/FilterBar/state.ts	`67.85% <0.00%> (ø)`
...ConfigModal/FiltersConfigForm/FilterScope/utils.ts	`87.03% <ø> (ø)`
...shboard/util/charts/getFormDataWithExtraFilters.ts	`94.44% <ø> (ø)`
...d/src/dashboard/util/logging/childChartsDidLoad.js	`0.00% <0.00%> (ø)`
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cadd259...99a1ebc. Read the comment docs.

michael-s-molina

Thank you for the PR @kgabryje. I left some first-pass comments.

superset-frontend/src/explore/ExplorePage.tsx

superset-frontend/src/explore/actions/hydrateExplore.ts

superset-frontend/src/explore/exploreUtils/getParsedExploreURLParams.test.ts

michael-s-molina · 2022-06-28T12:54:56Z

superset-frontend/src/explore/actions/hydrateExplore.ts

+
+export const HYDRATE_EXPLORE = 'HYDRATE_EXPLORE';
+export const hydrateExplore =
+  ({ form_data, slice, dataset: datasource }: ExplorePageInitialData) =>


The endpoint also returns a message attribute that contains information related to the request. One example is when we request /explore with an invalid form_data_key. In this case, message will contain Form data not found in cache, reverting to chart metadata. and this should be presented to the user in a toast.

I think some other part of the code is already handling displaying the message in toast - it shows up when form_data_key is wrong. When I manually added danger toast with message, it was displayed twice

Can you confirm what part? I'm only asking because the attribute message didn't exist previously, I added it to the return type with the new endpoint 🤔

It's in common.flash_messages

stephenLYZ · 2022-06-28T15:41:39Z

/testenv up

github-actions · 2022-06-28T16:58:02Z

@stephenLYZ Ephemeral environment spinning up at http://34.216.144.176:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

jinghua-qa · 2022-06-29T07:27:12Z

found a bug: blank page when create chart from the + button

Screen.Recording.2022-06-29.at.12.19.30.AM.mov

zhaoyongjie

thanks for your hard work, leave some comments.

zhaoyongjie · 2022-06-29T06:53:58Z

superset-frontend/src/explore/ExplorePage.tsx

+    const fetchExploreData = async () => {
+      const { result } = await makeApi<{}, ExploreResponsePayload>({
+        method: 'GET',
+        endpoint: 'api/v1/explore/',
+      })(exploreUrlParams);
+      dispatch(hydrateExplore(result));
+      setIsLoaded(true);
+    };


We have to process the response that contains the error code, 1) should return a default explore state 2) hide the loading spin.

using the following snippet to debug this case.

(superset) yongjie.zhao@:incubator-superset$ git diff diff --git a/superset/explore/api.py b/superset/explore/api.py index 7cce592d3..b697c4a17 100644 --- a/superset/explore/api.py +++ b/superset/explore/api.py @@ -105,6 +105,7 @@ class ExploreRestApi(BaseApi): 500: $ref: '#/components/responses/500' """ + return self.response_404() try: params = CommandParameters( actor=g.user,

Good point!

zhaoyongjie · 2022-06-29T07:37:01Z

superset-frontend/src/explore/actions/hydrateExplore.ts

+export const hydrateExplore =
+  ({ form_data, slice, dataset: datasource }: ExplorePageInitialData) =>
+  (dispatch: Dispatch, getState: () => ExplorePageState) => {
+    const initialFormData = form_data;


I think we should construct an appropriate/default formData here so that supports to open dataset.

const initialFormData = slice?.form_data ? slice.form_data : form_data; const initialData = { form_data, slice, datasource }; const initialControls = getControlsState( initialData, initialFormData, ) as ControlStateMapping; const { user, datasources, charts } = getState(); const exploreState = { .... .... controls: initialControls, .... } .... .... const sliceFormData = slice ? getFormDataFromControls(initialControls) : null;

kgabryje · 2022-06-29T16:53:35Z

/testenv up

github-actions · 2022-06-29T17:09:01Z

@kgabryje Ephemeral environment spinning up at http://52.12.110.64:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

zhaoyongjie · 2022-06-30T03:51:16Z

@kgabryje @jinghua-qa I leave a few common use cases so that we can test this PR

#	Test case	Result
1	Open an existing chart	success
2	Open an existing virtual/physical dataset	failure
3	Edit a Chart when clicking `edit chart` from Dashboard	success
4	Create a dataset from SQLLab	failure
5	Open an existing chart, and then switch to different datasets	success
6	Open an existing chart and the current user is admin; the user should permit editing the current dataset	failure
7	Manually construct an exception response, the explore page should show default interface	success

jinghua-qa · 2022-06-30T06:41:12Z

@kgabryje @jinghua-qa I leave a few common use cases so that we can test this PR

Test case Result

1 Open an existing chart success
2 Open an existing virtual/physical dataset failure
3 Edit a Chart when clicking edit chart from Dashboard success
4 Create a dataset from SQLLab failure
5 Open an existing chart, and then switch to different datasets success
6 Open an existing chart and the current user is admin; the user should permit editing the current dataset failure
7 Manually construct an exception response, the explore page should show default interface success

Also see issue 2 and 4, issue 6 is current issue on master

kgabryje · 2022-06-30T07:03:15Z

Thank you so much @zhaoyongjie @jinghua-qa ! I'll fix issues 2 and 4, let's address issue 6 in separate PR since it's already on master

github-actions · 2022-06-30T09:03:13Z

Ephemeral environment shutdown and build artifacts deleted.

geido · 2022-06-30T09:03:59Z

@kgabryje @jinghua-qa I leave a few common use cases so that we can test this PR

Test case Result

1 Open an existing chart success
2 Open an existing virtual/physical dataset failure
3 Edit a Chart when clicking edit chart from Dashboard success
4 Create a dataset from SQLLab failure
5 Open an existing chart, and then switch to different datasets success
6 Open an existing chart and the current user is admin; the user should permit editing the current dataset failure
7 Manually construct an exception response, the explore page should show default interface success

Wondering if we should add these cases to Cypress as these seems all scenarios that should have failed in CI.

The code LGTM. I am going to pull this branch as I need it as a base to test another PR and I will report all eventual issues that I find, if any.

P.S. Sorry didn't mean to close the PR :)

kgabryje · 2022-06-30T09:37:34Z

Wondering if we should add these cases to Cypress as these seems all scenarios that should have failed in CI.

Agreed. I'll create a task for the next sprint

kgabryje · 2022-06-30T12:13:05Z

/testenv up

github-actions · 2022-06-30T12:15:16Z

@kgabryje Ephemeral environment spinning up at http://18.236.74.190:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

zhaoyongjie

the main process test passed. continue to polish in the future.

jinghua-qa · 2022-06-30T16:05:10Z

Wondering if we should add these cases to Cypress as these seems all scenarios that should have failed in CI.

Good idea!!

michael-s-molina

LGTM. Thank you for all the hard work @kgabryje! This is amazing and greatly simplifies the application. Thank you for addressing all the comments and bugs as well.

Let's just get QA's approval before merging it because of the high risk 😉

codyml · 2022-06-30T16:50:40Z

Tried all of the entrypoints to Explore I could think of and nothing broke!

geido

Code LGTM! Also manual testing didn't show any significant issue. With the OK of QA I think this is ready to go

jinghua-qa

QA team has check the related regression test and we did not see blocker issue. LGTM!
https://docs.google.com/spreadsheets/d/1aVEM4f4slnwAA-Bv9CMcQyQ9NSfT1vw5V0cXsNMmFUA/edit#gid=0

github-actions · 2022-06-30T19:07:16Z

Ephemeral environment shutdown and build artifacts deleted.

…ata (apache#20519) * feat(explore): Use v1/explore endpoint data instead of bootstrapData * Add tests * Fix ci * Remove redundant dependency * Use form_data_key in cypress tests * Add auth headers to for data request * Address comments * Remove displaying danger toast * Conditionally add auth headers * Address comments * Fix typing bug * fix * Fix opening dataset * Fix sqllab chart create * Run queries in parallel * Fix dashboard id autofill * Fix lint * Fix test

john-bodley · 2022-10-03T17:57:07Z

/testenv up

github-actions · 2022-10-03T17:57:19Z

@john-bodley Ephemeral environment creation is currently limited to committers.

kgabryje requested review from zhaoyongjie, stephenLYZ, geido, michael-s-molina and jinghua-qa June 28, 2022 09:38

superset-github-bot bot added the preset-io label Jun 28, 2022

pull-request-size bot added the size/XL label Jun 28, 2022

michael-s-molina reviewed Jun 28, 2022

View reviewed changes

zhaoyongjie reviewed Jun 29, 2022

View reviewed changes

pull-request-size bot added size/XXL and removed size/XL labels Jun 29, 2022

kgabryje added 12 commits June 29, 2022 19:33

feat(explore): Use v1/explore endpoint data instead of bootstrapData

7b6c424

Add tests

308745b

Fix ci

b47a41f

Remove redundant dependency

d687d3e

Use form_data_key in cypress tests

c05882a

Add auth headers to for data request

a152491

Address comments

bc3ce1c

Remove displaying danger toast

fe9b97b

Conditionally add auth headers

4e95594

Address comments

d2edbd7

Fix typing bug

7e3e8d7

fix

24bc86b

kgabryje force-pushed the feat/explore-v1 branch from 7c67a6f to 24bc86b Compare June 29, 2022 18:00

Fix opening dataset

d2e3679

geido closed this Jun 30, 2022

geido reopened this Jun 30, 2022

Fix sqllab chart create

d77c547

kgabryje added 4 commits June 30, 2022 12:06

Run queries in parallel

c3f151a

Fix dashboard id autofill

3c21b3a

Fix lint

1c3f2ef

Fix test

99a1ebc

zhaoyongjie approved these changes Jun 30, 2022

View reviewed changes

michael-s-molina approved these changes Jun 30, 2022

View reviewed changes

geido approved these changes Jun 30, 2022

View reviewed changes

jinghua-qa approved these changes Jun 30, 2022

View reviewed changes

kgabryje merged commit b30f6a5 into apache:master Jun 30, 2022

michael-s-molina mentioned this pull request Jul 1, 2022

refactor: Renders Explore in SPA #20572

Merged

9 tasks

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(explore): Get Explore data from endpoint instead of bootstrap_data #20519

chore(explore): Get Explore data from endpoint instead of bootstrap_data #20519

kgabryje commented Jun 28, 2022 •

edited

Loading

codecov bot commented Jun 28, 2022 •

edited

Loading

michael-s-molina left a comment

michael-s-molina Jun 28, 2022

kgabryje Jun 28, 2022

michael-s-molina Jun 28, 2022

kgabryje Jun 28, 2022

stephenLYZ commented Jun 28, 2022

github-actions bot commented Jun 28, 2022

jinghua-qa commented Jun 29, 2022

zhaoyongjie left a comment

zhaoyongjie Jun 29, 2022 •

edited

Loading

kgabryje Jun 29, 2022

zhaoyongjie Jun 29, 2022

kgabryje commented Jun 29, 2022

github-actions bot commented Jun 29, 2022

zhaoyongjie commented Jun 30, 2022 •

edited

Loading

jinghua-qa commented Jun 30, 2022

Test case Result

kgabryje commented Jun 30, 2022 •

edited

Loading

github-actions bot commented Jun 30, 2022

geido commented Jun 30, 2022 •

edited

Loading

Test case Result

kgabryje commented Jun 30, 2022

kgabryje commented Jun 30, 2022

github-actions bot commented Jun 30, 2022

zhaoyongjie left a comment

jinghua-qa commented Jun 30, 2022 •

edited

Loading

michael-s-molina left a comment

codyml commented Jun 30, 2022

geido left a comment

jinghua-qa left a comment

github-actions bot commented Jun 30, 2022

john-bodley commented Oct 3, 2022

github-actions bot commented Oct 3, 2022

chore(explore): Get Explore data from endpoint instead of bootstrap_data #20519

chore(explore): Get Explore data from endpoint instead of bootstrap_data #20519

Conversation

kgabryje commented Jun 28, 2022 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Jun 28, 2022 • edited Loading

Codecov Report

michael-s-molina left a comment

Choose a reason for hiding this comment

michael-s-molina Jun 28, 2022

Choose a reason for hiding this comment

kgabryje Jun 28, 2022

Choose a reason for hiding this comment

michael-s-molina Jun 28, 2022

Choose a reason for hiding this comment

kgabryje Jun 28, 2022

Choose a reason for hiding this comment

stephenLYZ commented Jun 28, 2022

github-actions bot commented Jun 28, 2022

jinghua-qa commented Jun 29, 2022

zhaoyongjie left a comment

Choose a reason for hiding this comment

zhaoyongjie Jun 29, 2022 • edited Loading

Choose a reason for hiding this comment

kgabryje Jun 29, 2022

Choose a reason for hiding this comment

zhaoyongjie Jun 29, 2022

Choose a reason for hiding this comment

kgabryje commented Jun 29, 2022

github-actions bot commented Jun 29, 2022

zhaoyongjie commented Jun 30, 2022 • edited Loading

jinghua-qa commented Jun 30, 2022

Test case Result

kgabryje commented Jun 30, 2022 • edited Loading

github-actions bot commented Jun 30, 2022

geido commented Jun 30, 2022 • edited Loading

Test case Result

kgabryje commented Jun 30, 2022

kgabryje commented Jun 30, 2022

github-actions bot commented Jun 30, 2022

zhaoyongjie left a comment

Choose a reason for hiding this comment

jinghua-qa commented Jun 30, 2022 • edited Loading

michael-s-molina left a comment

Choose a reason for hiding this comment

codyml commented Jun 30, 2022

geido left a comment

Choose a reason for hiding this comment

jinghua-qa left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 30, 2022

john-bodley commented Oct 3, 2022

github-actions bot commented Oct 3, 2022

kgabryje commented Jun 28, 2022 •

edited

Loading

codecov bot commented Jun 28, 2022 •

edited

Loading

zhaoyongjie Jun 29, 2022 •

edited

Loading

zhaoyongjie commented Jun 30, 2022 •

edited

Loading

kgabryje commented Jun 30, 2022 •

edited

Loading

geido commented Jun 30, 2022 •

edited

Loading

jinghua-qa commented Jun 30, 2022 •

edited

Loading