Skip to content

Commit

Permalink
feat: Drill by error management (apache#23724)
Browse files Browse the repository at this point in the history
  • Loading branch information
kgabryje authored and sebastianliebscher committed Apr 28, 2023
1 parent 68eb1aa commit 424c8b8
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
} from '@superset-ui/core';
import Icons from 'src/components/Icons';
import { Input } from 'src/components/Input';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import {
cachedSupersetGet,
supersetGetCache,
Expand Down Expand Up @@ -80,12 +81,12 @@ export const DrillByMenuItems = ({
...rest
}: DrillByMenuItemsProps) => {
const theme = useTheme();
const { addDangerToast } = useToasts();
const [searchInput, setSearchInput] = useState('');
const [dataset, setDataset] = useState<Dataset>();
const [columns, setColumns] = useState<Column[]>([]);
const [showModal, setShowModal] = useState(false);
const [currentColumn, setCurrentColumn] = useState();

const handleSelection = useCallback(
(event, column) => {
onClick(event);
Expand Down Expand Up @@ -143,9 +144,11 @@ export const DrillByMenuItems = ({
})
.catch(() => {
supersetGetCache.delete(`/api/v1/dataset/${datasetId}`);
addDangerToast(t('Failed to load dimensions for drill by'));
});
}
}, [
addDangerToast,
excludedColumns,
formData,
groupbyFieldName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ beforeEach(() => {
.post(CHART_DATA_ENDPOINT, { body: {} }, {})
.post(FORM_DATA_KEY_ENDPOINT, { key: '123' });
});
afterEach(fetchMock.restore);
afterEach(() => fetchMock.restore());

test('should render the title', async () => {
await renderModal();
Expand All @@ -127,10 +127,22 @@ test('should close the modal', async () => {
});

test('should render loading indicator', async () => {
await renderModal();
await waitFor(() =>
expect(screen.getByLabelText('Loading')).toBeInTheDocument(),
fetchMock.post(
CHART_DATA_ENDPOINT,
{ body: {} },
// delay is missing in fetch-mock types
// @ts-ignore
{ overwriteRoutes: true, delay: 1000 },
);
await renderModal();
expect(screen.getByLabelText('Loading')).toBeInTheDocument();
});

test('should render alert banner when results fail to load', async () => {
await renderModal();
expect(
await screen.findByText('There was an error loading the chart data'),
).toBeInTheDocument();
});

test('should generate Explore url', async () => {
Expand Down
48 changes: 39 additions & 9 deletions superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ import { noOp } from 'src/utils/common';
import { simpleFilterToAdhoc } from 'src/utils/simpleFilterToAdhoc';
import { useDatasetMetadataBar } from 'src/features/datasets/metadataBar/useDatasetMetadataBar';
import { SingleQueryResultPane } from 'src/explore/components/DataTablesPane/components/SingleQueryResultPane';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import Alert from 'src/components/Alert';
import { Dataset, DrillByType } from '../types';
import DrillByChart from './DrillByChart';
import { ContextMenuItem } from '../ChartContextMenu/ChartContextMenu';
Expand All @@ -65,6 +67,7 @@ interface ModalFooterProps {
}

const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
const { addDangerToast } = useToasts();
const [url, setUrl] = useState('');
const dashboardPageId = useContext(DashboardPageIdContext);
const [datasource_id, datasource_type] = formData.datasource.split('__');
Expand All @@ -75,13 +78,24 @@ const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => {
`/explore/?form_data_key=${key}&dashboard_page_id=${dashboardPageId}`,
);
})
.catch(e => {
console.log(e);
.catch(() => {
addDangerToast(t('Failed to generate chart edit URL'));
});
}, [dashboardPageId, datasource_id, datasource_type, formData]);
}, [
addDangerToast,
dashboardPageId,
datasource_id,
datasource_type,
formData,
]);
return (
<>
<Button buttonStyle="secondary" buttonSize="small" onClick={noOp}>
<Button
buttonStyle="secondary"
buttonSize="small"
onClick={noOp}
disabled={!url}
>
<Link
css={css`
&:hover {
Expand Down Expand Up @@ -126,6 +140,8 @@ export default function DrillByModal({
onHideModal,
}: DrillByModalProps) {
const theme = useTheme();
const { addDangerToast } = useToasts();
const [isChartDataLoading, setIsChartDataLoading] = useState(true);

const initialGroupbyColumns = useMemo(
() =>
Expand Down Expand Up @@ -278,14 +294,22 @@ export default function DrillByModal({

useEffect(() => {
if (drilledFormData) {
setIsChartDataLoading(true);
setChartDataResult(undefined);
getChartDataRequest({
formData: drilledFormData,
}).then(({ json }) => {
setChartDataResult(json.result);
});
})
.then(({ json }) => {
setChartDataResult(json.result);
})
.catch(() => {
addDangerToast(t('Failed to load chart data.'));
})
.finally(() => {
setIsChartDataLoading(false);
});
}
}, [drilledFormData]);
}, [addDangerToast, drilledFormData]);
const { metadataBar } = useDatasetMetadataBar({ dataset });

return (
Expand Down Expand Up @@ -323,7 +347,13 @@ export default function DrillByModal({
{metadataBar}
{breadcrumbs}
{displayModeToggle}
{!chartDataResult && <Loading />}
{isChartDataLoading && <Loading />}
{!isChartDataLoading && !chartDataResult && (
<Alert
type="error"
message={t('There was an error loading the chart data')}
/>
)}
{drillByDisplayMode === DrillByType.Chart && chartDataResult && (
<DrillByChart
formData={drilledFormData}
Expand Down

0 comments on commit 424c8b8

Please sign in to comment.