From 985af72ac364377264618358544ba89a79d0ecc5 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Wed, 14 Jul 2021 14:12:20 -0700 Subject: [PATCH] fix(dashboard): Make the View Chart In Explore menu option a link (#15668) * hey look, it's a real anchor tag! * get the explore chart url into the link * add doc comments to the functions * remove pointless test * update weird tests Co-authored-by: Evan Rusackas --- .../SliceHeader/SliceHeader.test.tsx | 12 +++--- .../components/SliceHeader/index.tsx | 38 ++++------------- .../SliceHeaderControls.test.tsx | 12 ------ .../components/SliceHeaderControls/index.tsx | 41 ++++++++++++------- .../components/gridComponents/Chart.jsx | 13 +++--- .../src/explore/exploreUtils/index.js | 9 ++++ 6 files changed, 56 insertions(+), 69 deletions(-) diff --git a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx index 3416fef38..6acf88844 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx @@ -57,7 +57,7 @@ jest.mock('src/dashboard/components/SliceHeaderControls', () => ({ @@ -155,7 +155,7 @@ const createProps = () => ({ updateSliceName: jest.fn(), toggleExpandSlice: jest.fn(), forceRefresh: jest.fn(), - exploreChart: jest.fn(), + logExploreChart: jest.fn(), exportCSV: jest.fn(), formData: {}, }); @@ -176,7 +176,7 @@ test('Should render - default props', () => { // @ts-ignore delete props.toggleExpandSlice; // @ts-ignore - delete props.exploreChart; + delete props.logExploreChart; // @ts-ignore delete props.exportCSV; // @ts-ignore @@ -218,7 +218,7 @@ test('Should render default props and "call" actions', () => { // @ts-ignore delete props.toggleExpandSlice; // @ts-ignore - delete props.exploreChart; + delete props.logExploreChart; // @ts-ignore delete props.exportCSV; // @ts-ignore @@ -378,9 +378,9 @@ test('Correct actions to "SliceHeaderControls"', () => { userEvent.click(screen.getByTestId('forceRefresh')); expect(props.forceRefresh).toBeCalledTimes(1); - expect(props.exploreChart).toBeCalledTimes(0); + expect(props.logExploreChart).toBeCalledTimes(0); userEvent.click(screen.getByTestId('exploreChart')); - expect(props.exploreChart).toBeCalledTimes(1); + expect(props.logExploreChart).toBeCalledTimes(1); expect(props.exportCSV).toBeCalledTimes(0); userEvent.click(screen.getByTestId('exportCSV')); diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx index 32eac7df0..a0cdb94be 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx @@ -21,48 +21,24 @@ import { styled, t } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; import { useDispatch, useSelector } from 'react-redux'; import EditableTitle from 'src/components/EditableTitle'; -import SliceHeaderControls from 'src/dashboard/components/SliceHeaderControls'; +import SliceHeaderControls, { + SliceHeaderControlsProps, +} from 'src/dashboard/components/SliceHeaderControls'; import FiltersBadge from 'src/dashboard/components/FiltersBadge'; import Icons from 'src/components/Icons'; import { RootState } from 'src/dashboard/types'; import FilterIndicator from 'src/dashboard/components/FiltersBadge/FilterIndicator'; import { clearDataMask } from 'src/dataMask/actions'; -type SliceHeaderProps = { +type SliceHeaderProps = SliceHeaderControlsProps & { innerRef?: string; - slice: { - description: string; - viz_type: string; - slice_name: string; - slice_id: number; - slice_description: string; - }; - isExpanded?: boolean; - isCached?: boolean[]; - cachedDttm?: string[]; - updatedDttm?: number; updateSliceName?: (arg0: string) => void; - toggleExpandSlice?: () => void; - forceRefresh?: () => void; - exploreChart?: () => void; - exportCSV?: () => void; - exportFullCSV?: () => void; editMode?: boolean; - isFullSize?: boolean; annotationQuery?: object; annotationError?: object; sliceName?: string; - supersetCanExplore?: boolean; - supersetCanShare?: boolean; - supersetCanCSV?: boolean; - sliceCanEdit?: boolean; - componentId: string; - dashboardId: number; filters: object; - addSuccessToast: () => void; - addDangerToast: () => void; handleToggleFullSize: () => void; - chartStatus: string; formData: object; }; @@ -81,7 +57,8 @@ const SliceHeader: FC = ({ forceRefresh = () => ({}), updateSliceName = () => ({}), toggleExpandSlice = () => ({}), - exploreChart = () => ({}), + logExploreChart = () => ({}), + exploreUrl = '#', exportCSV = () => ({}), editMode = false, annotationQuery = {}, @@ -184,7 +161,8 @@ const SliceHeader: FC = ({ updatedDttm={updatedDttm} toggleExpandSlice={toggleExpandSlice} forceRefresh={forceRefresh} - exploreChart={exploreChart} + logExploreChart={logExploreChart} + exploreUrl={exploreUrl} exportCSV={exportCSV} exportFullCSV={exportFullCSV} supersetCanExplore={supersetCanExplore} diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx index d3a57d511..158bd99e8 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx @@ -193,18 +193,6 @@ test('Should not show export full CSV if report is not table', () => { ); }); -test('Should "View chart in Explore"', () => { - const props = createProps(); - render(, { useRedux: true }); - - expect(props.exploreChart).toBeCalledTimes(0); - userEvent.click( - screen.getByRole('menuitem', { name: 'View chart in Explore' }), - ); - expect(props.exploreChart).toBeCalledTimes(1); - expect(props.exploreChart).toBeCalledWith(371); -}); - test('Should "Toggle chart description"', () => { const props = createProps(); render(, { useRedux: true }); diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx index 28d42dd67..ffa9b589e 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx @@ -80,7 +80,8 @@ const VerticalDotsTrigger = () => ( ); -interface Props { + +export interface SliceHeaderControlsProps { slice: { description: string; viz_type: string; @@ -88,35 +89,43 @@ interface Props { slice_id: number; slice_description: string; }; + componentId: string; - chartStatus: string; dashboardId: number; - addDangerToast: () => void; + chartStatus: string; isCached: boolean[]; cachedDttm: string[] | null; isExpanded?: boolean; updatedDttm: number | null; - supersetCanExplore: boolean; - supersetCanShare: boolean; - supersetCanCSV: boolean; - sliceCanEdit: boolean; isFullSize?: boolean; formData: object; - toggleExpandSlice?: (sliceId: number) => void; + exploreUrl?: string; + forceRefresh: (sliceId: number, dashboardId: number) => void; - exploreChart?: (sliceId: number) => void; + logExploreChart?: (sliceId: number) => void; + toggleExpandSlice?: (sliceId: number) => void; exportCSV?: (sliceId: number) => void; exportFullCSV?: (sliceId: number) => void; - addSuccessToast: (message: string) => void; handleToggleFullSize: () => void; + + addDangerToast: (message: string) => void; + addSuccessToast: (message: string) => void; + + supersetCanExplore?: boolean; + supersetCanShare?: boolean; + supersetCanCSV?: boolean; + sliceCanEdit?: boolean; } interface State { showControls: boolean; showCrossFilterScopingModal: boolean; } -class SliceHeaderControls extends React.PureComponent { - constructor(props: Props) { +class SliceHeaderControls extends React.PureComponent< + SliceHeaderControlsProps, + State +> { + constructor(props: SliceHeaderControlsProps) { super(props); this.toggleControls = this.toggleControls.bind(this); this.refreshChart = this.refreshChart.bind(this); @@ -164,8 +173,8 @@ class SliceHeaderControls extends React.PureComponent { break; case MENU_KEYS.EXPLORE_CHART: // eslint-disable-next-line no-unused-expressions - this.props.exploreChart && - this.props.exploreChart(this.props.slice.slice_id); + this.props.logExploreChart && + this.props.logExploreChart(this.props.slice.slice_id); break; case MENU_KEYS.EXPORT_CSV: // eslint-disable-next-line no-unused-expressions @@ -272,7 +281,9 @@ class SliceHeaderControls extends React.PureComponent { {this.props.supersetCanExplore && ( - {t('View chart in Explore')} + + {t('View chart in Explore')} + )} diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index 1b492d255..e0a8cc5c8 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx @@ -21,7 +21,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { styled } from '@superset-ui/core'; -import { exploreChart, exportChart } from 'src/explore/exploreUtils'; +import { exportChart, getExploreLongUrl } from 'src/explore/exploreUtils'; import ChartContainer from 'src/chart/ChartContainer'; import { LOG_ACTIONS_CHANGE_DASHBOARD_FILTER, @@ -110,7 +110,6 @@ export default class Chart extends React.Component { this.changeFilter = this.changeFilter.bind(this); this.handleFilterMenuOpen = this.handleFilterMenuOpen.bind(this); this.handleFilterMenuClose = this.handleFilterMenuClose.bind(this); - this.exploreChart = this.exploreChart.bind(this); this.exportCSV = this.exportCSV.bind(this); this.exportFullCSV = this.exportFullCSV.bind(this); this.forceRefresh = this.forceRefresh.bind(this); @@ -221,13 +220,14 @@ export default class Chart extends React.Component { this.props.unsetFocusedFilterField(chartId, column); } - exploreChart() { + logExploreChart = () => { this.props.logEvent(LOG_ACTIONS_EXPLORE_DASHBOARD_CHART, { slice_id: this.props.slice.slice_id, is_cached: this.props.isCached, }); - exploreChart(this.props.formData); - } + }; + + getChartUrl = () => getExploreLongUrl(this.props.formData); exportCSV(isFullCSV = false) { this.props.logEvent(LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART, { @@ -327,7 +327,8 @@ export default class Chart extends React.Component { forceRefresh={this.forceRefresh} editMode={editMode} annotationQuery={chart.annotationQuery} - exploreChart={this.exploreChart} + logExploreChart={this.logExploreChart} + exploreUrl={this.getChartUrl()} exportCSV={this.exportCSV} exportFullCSV={this.exportFullCSV} updateSliceName={updateSliceName} diff --git a/superset-frontend/src/explore/exploreUtils/index.js b/superset-frontend/src/explore/exploreUtils/index.js index d5dca456c..ddcd95b82 100644 --- a/superset-frontend/src/explore/exploreUtils/index.js +++ b/superset-frontend/src/explore/exploreUtils/index.js @@ -89,6 +89,10 @@ export function getURIDirectory(endpointType = 'base') { return '/superset/explore/'; } +/** + * This gets the url of the explore page, with all the form data included explicitly. + * This includes any form data overrides from the dashboard. + */ export function getExploreLongUrl( formData, endpointType, @@ -138,6 +142,11 @@ export function getChartDataUri({ path, qs, allowDomainSharding = false }) { return uri; } +/** + * This gets the minimal url for the given form data. + * If there are dashboard overrides present in the form data, + * they will not be included in the url. + */ export function getExploreUrl({ formData, endpointType = 'base',