[WIP] feature(dashboard): Drill to detail modal

[codyml]

SUMMARY

This PR introduces a drill-to-detail modal and adds a "Drill to detail" menu item to dashboard charts' contextual menus. The drill-to-detail modal allows users to inspect the data that is backing a chart from the dashboard that the chart is on. Future PRs will support right-clicking on a part of a chart to view the data filtered based on where the user clicked, but this PR only adds support for showing the drill-to-detail modal for all of a chart's data.

More details:

• Unlike the Samples pane, the drill-to-detail modal content is server-paginated and allows paging through all rows of data.
• All columns of the dataset are displayed.
• Native dashboard filters, cross-filters, ad-hoc chart filters and time chart filters are transparently respected; only rows matching those filters are displayed in the modal table.
• The menu item appears when the DRILL_TO_DETAIL feature flag is enabled.

WIP: This PR should not be merged until server-side support (à la 0ac33f9b4d709c2eee5b24fc794d4fb321129938) has been merged and this PR has been rebased over it, with 0ac33f9b4d709c2eee5b24fc794d4fb321129938 excluded.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Menu item:

Modal:

Modal w/ native filters:

TESTING INSTRUCTIONS

• Ensure no change to contextual menu without feature flag set
• With feature flag set, compare expected datasource results to drill to detail results in the following circumstances:
  • No filters from chart or dashboard context
  • Chart time filters
  • Chart ad-hoc filters
  • Dashboard native filters
  • Dashboard cross-filters
• Ensure that paginated results in modal work as expected in the following circumstances:
  • No results
  • One page of results
  • Multiple pages of results
  • Few columns
  • Lots of columns

ADDITIONAL INFORMATION

• [ ] Has associated issue:
• [x] Required feature flags: DRILL_TO_DETAIL, DASHBOARD_CROSS_FILTERS optional to test cross-filter support.
• [x] 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
• [x] Introduces new feature or API
• [ ] Removes existing feature or API

[codecov[bot]]

Codecov Report

Merging #20728 (0bf4e56) into master (0bf4e56) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 0bf4e56 differs from pull request most recent head 2310b03. Consider uploading reports for the commit 2310b03 to get more accurate results

@@           Coverage Diff           @@
##           master   #20728   +/-   ##
=======================================
  Coverage   66.16%   66.16%           
=======================================
  Files        1773     1773           
  Lines       67689    67689           
  Branches     7218     7218           
=======================================
  Hits        44786    44786           
  Misses      21059    21059           
  Partials     1844     1844           

Flag	Coverage Δ
javascript	52.12% <0.00%> (ø)
mysql	80.98% <0.00%> (ø)
postgres	81.04% <0.00%> (ø)
python	81.15% <0.00%> (ø)
sqlite	79.64% <0.00%> (ø)

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

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[michael-s-molina]

Hi @codyml. We need to account for charts and dashboard configurations when displaying the drilling details panel. One example is the chart below:

In this chart, we should consider the controls configuration that impacts the query like is_software_dev = 1, last_yr_income <= 200000, and time range = 2018-03-01 <= col < 2022-07-25. We also need to consider that each visualization has different controls.

I believe the samples pane in Explore should also account for these filters. If I set is_software_dev = 1 I don't expect to see samples where this condition is not true.

The drilling details panel should also be influenced by dashboard configurations like developer_type in the example below:

To make things more complicated, dashboard filters can come from the filters panel or cross-filters.

I don't know if I covered everything. We may have other sources that impact a chart. Tagging some folks to see if I'm missing anything @rusackas @lauderbaugh @zhaoyongjie @kgabryje @geido

[zhaoyongjie]

I believe the samples pane in Explore should also account for these filters. If I set is_software_dev = 1 I don't expect to see samples where this condition is not true.

It's not true. The SamplesPane implies sampling the data source and has no relation to the chart. V1 API is at here, legacy API is at here.

---

It is a point worth discussing why the original implementation did not reset the filters field in the QueryObject? IMO, It's a bug.

The filters should be applied where clause or having clause in the query. The metrics has been reset on the QueryObject, so 1) we did not have a chance to send a query with having clause. 2) the time range has been reset so that no chance to send a query with a time partition.

[codyml]

Closed to refocus on additional work required given clarified requirements.

[codyml]

Reopening!

[michael-s-molina]

Can we display the modal with a fixed height/width? I find it weird that it starts small and grows after the results are rendered.

[zhaoyongjie]

@codyml Thanks for the new feature, I have tested this feature in my local. there are 2 issues.

1. The drill detail modal always sends force query. the force query means that don't use data cache in the backend, so every force query will send actual SQL to the underlying database, usually, it's an "expensive" operation.

2. The drill detail request is 2 times when I click drill to detail.

https://user-images.githubusercontent.com/2016594/183557696-584bea48-e9ce-4f7c-b1aa-79ddeb88713c.mov

[zhaoyongjie]

/testenv up FEATURE_DRILL_TO_DETAIL=true

[github-actions[bot]]

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

[codyml]

@codyml Thanks for the new feature, I have tested this feature in my local. there are 2 issues.

1. The drill detail modal always sends force query. the force query means that don't use data cache in the backend, so every force query will send actual SQL to the underlying database, usually, it's an "expensive" operation.

2. The drill detail request is 2 times when I click drill to detail.

drill.to.detail.mov

@zhaoyongjie Thanks for looking it over again. I fixed (2), so it should only send one request. For (1), do you think we should have force=false for all of these requests? Should we be worried about getting stale data? I could also have it only do force=true if you click the reload button in the upper right of the modal.

[zhaoyongjie]

/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DRILL_TO_DETAIL=true

[github-actions[bot]]

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

[zhaoyongjie]

@codyml Thanks for the new feature, I have tested this feature in my local. there are 2 issues.

1. The drill detail modal always sends force query. the force query means that don't use data cache in the backend, so every force query will send actual SQL to the underlying database, usually, it's an "expensive" operation.

2. The drill detail request is 2 times when I click drill to detail.

drill.to.detail.mov

@zhaoyongjie Thanks for looking it over again. I fixed (2), so it should only send one request. For (1), do you think we should have force=false for all of these requests? Should we be worried about getting stale data? I could also have it only do force=true if you click the reload button in the upper right of the modal.

For the ideal, the force is sent when the force refresh is selected in three-dot menu. IMO, this can be optimized later, just keep force=false for all of requests.

[zhaoyongjie]

Hi @codyml, I made a new Dashboard on the ephemeral environment.

The time filter on the drill payload looks incorrect because the time filter is different from other filters in the current filter design. Ideally, these things should be applied under the filters field, but this part still needs a lot of work.

https://user-images.githubusercontent.com/2016594/184063120-78ea29a1-5f5d-40f7-a81c-d6260313fb79.mov

The drill-detail payload has such fields.

{
  extras: { time_grain_sqla }  <---- how to aggregate time column, for example: P1Y('2022-03-01') => '2022-01-01'
  granularity <----- used for time column
  time_range <----- used for time value, it's a pairs value, like the `between` value in SQL
}

Fortunately, the time_range field supports lots of time expression. The time value(timestamp represented by integer) and time_grain_sqla should be converted to time expression. For example, the time value is 1643673600000 and time_grain_sqla is P1Y.

1. convert the time value from the integer to the time string,
2. apply time expression into time string and get a time_range value

if the time_grain_sqla is P1Y, it will generate a SQL like 2022-01-01 <= col < 2023-01-01.

datetrunc(datetime('2022-02-01'), year) : dateadd(datetrunc(datetime('2022-02-01'), year), 1, year)

if the time_grain_sqla is P1M, it will generate a SQL like 2022-02-01 <= col < 2022-03-01.

datetrunc(datetime('2022-02-01'), month) : dateadd(datetrunc(datetime('2022-02-01'), month), 1, month)

if the time_grain_sqla is P1W, it will generate a SQL like 2022-01-31 <= col < 2022-02-07.

datetrunc(datetime('2022-02-01'), week) : dateadd(datetrunc(datetime('2022-02-01'), week), 1, week)

[geido]

Some more comments from manual testing:

• When resizing the modal, the pagination should stick to the bottom. Also, there are 2 vertical scrolls there. I believe we should only have 1 vertical scroll

• Let's set a minimum size for the modal as the user should never resize it to the point that is unusable

• When refreshing, should we move the user to page 1? I am thinking of an edge case where we have 2 pages initially, but a bunch of data is deleted or the underlying dataset has changed, now you only have 1 page, but the request will still try to look for page 2 causing a scenario like the one depicted below

• In the example above you can also see some weird layout for the no results which needs improvement

• I think we can consider this problem for a next phase, but I believe the user should be able to select exclusively all the content of the table without selecting any other area of the application

[codyml]

For the ideal, the force is sent when the force refresh is selected in three-dot menu. IMO, this can be optimized later, just keep force=false for all of requests.

@zhaoyongjie Sounds good, thanks!

[codyml]

Some more comments from manual testing:

• When resizing the modal, the pagination should stick to the bottom. Also, there are 2 vertical scrolls there. I believe we should only have 1 vertical scroll
• Let's set a minimum size for the modal as the user should never resize it to the point that is unusable
• When refreshing, should we move the user to page 1? I am thinking of an edge case where we have 2 pages initially, but a bunch of data is deleted or the underlying dataset has changed, now you only have 1 page, but the request will still try to look for page 2 causing a scenario like the one depicted below
• In the example above you can also see some weird layout for the no results which needs improvement
• I think we can consider this problem for a next phase, but I believe the user should be able to select exclusively all the content of the table without selecting any other area of the application

@geido Good call on the modal resizing and good catch on the extra scrollbars, will fix! Agreed on the reset to page 1 on refresh, will update. For the selection, maybe we should reintroduce the Copy button if it's clear that it'll only copy the one page? But sure, let's tackle separately.

[geido]

/testenv up FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DRILL_TO_DETAIL=true

[github-actions[bot]]

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

[codyml]

All issues should now be fixed, with the exception of D2D on values containing commas, which is being worked on in separate PRs (#21151, #21124).

[jinghua-qa]

/testenv up

[github-actions[bot]]

@jinghua-qa Ephemeral environment spinning up at http://54.191.88.138:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

[michael-s-molina]

All issues should now be fixed, with the exception of D2D on values containing commas, which is being worked on in separate PRs (#21151, #21124).

@codyml Both PRs are now merged in case you want to rebase.

[jinghua-qa]

Dashboard Regression Test LGTM

[github-actions[bot]]

Ephemeral environment shutdown and build artifacts deleted.

[JunTech]

how to use in 2.0.0 branch

[codyml]

how to use in 2.0.0 branch

Hi @JunTech! This feature wasn't included in the 2.0.0 release, so using it would require running Superset from your own non-release branch that includes this commit (and previous supporting commits), then enabling the DRILL_TO_DETAIL feature flag. This feature is very much still a work-in-progress and you're likely to run into incomplete support for viz plugins and the potential for bugs when enabled. It'll be included in a release when it's more complete!