Skip to content

Commit

Permalink
Add groupby col name
Browse files Browse the repository at this point in the history
  • Loading branch information
kgabryje committed Mar 28, 2023
1 parent c73bd93 commit d793393
Show file tree
Hide file tree
Showing 14 changed files with 153 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ export interface ContextMenuFilters {
isCurrentValueSelected?: boolean;
};
drillToDetail?: BinaryQueryObjectFilterClause[];
drillBy?: BinaryQueryObjectFilterClause[];
drillBy?: {
filters: BinaryQueryObjectFilterClause[];
groupbyFieldName: string;
adhocFilterFieldName?: string;
};
}

export enum AppSection {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ function WorldMap(element, props) {
const val =
countryFieldtype === 'name' ? mapData[key]?.name : mapData[key]?.country;
let drillToDetailFilters;
let drillBy;
let drillByFilters;
if (val) {
drillToDetailFilters = [
{
Expand All @@ -182,7 +182,7 @@ function WorldMap(element, props) {
formattedVal: val,
},
];
drillBy = [
drillByFilters = [
{
col: entity,
op: '==',
Expand All @@ -193,7 +193,7 @@ function WorldMap(element, props) {
onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
drillToDetail: drillToDetailFilters,
crossFilter: getCrossFilterDataMask(source),
drillBy,
drillBy: { filters: drillByFilters, groupbyFieldName: 'entity' },
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,15 @@ export default function EchartsGraph({
const drillToDetailFilters =
e.dataType === 'node' ? handleNodeClick(data) : handleEdgeClick(data);
const node = data.find(item => item.id === e.data.id);

onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
drillToDetail: drillToDetailFilters,
crossFilter: getCrossFilterDataMask(node),
drillBy: node && [{ col: node.col, op: '==', val: node.name }],
drillBy: node && {
filters: [{ col: node.col, op: '==', val: node.name }],
groupbyFieldName:
node.col === formData.source ? 'source' : 'target',
},
});
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,10 @@ export default function EchartsMixedTimeseries({
const pointerEvent = eventParams.event.event;
const drillToDetailFilters: BinaryQueryObjectFilterClause[] = [];
const drillByFilters: BinaryQueryObjectFilterClause[] = [];
const isFirst = isFirstQuery(seriesIndex);
const values = [
...(eventParams.name ? [eventParams.name] : []),
...(isFirstQuery(seriesIndex) ? labelMap : labelMapB)[
eventParams.seriesName
],
...(isFirst ? labelMap : labelMapB)[eventParams.seriesName],
];
if (data && xAxis.type === AxisType.time) {
drillToDetailFilters.push({
Expand All @@ -152,7 +151,7 @@ export default function EchartsMixedTimeseries({
}
[
...(data && xAxis.type === AxisType.category ? [xAxis.label] : []),
...(isFirstQuery(seriesIndex) ? formData.groupby : formData.groupbyB),
...(isFirst ? formData.groupby : formData.groupbyB),
].forEach((dimension, i) =>
drillToDetailFilters.push({
col: dimension,
Expand All @@ -162,19 +161,22 @@ export default function EchartsMixedTimeseries({
}),
);

[
...(isFirstQuery(seriesIndex) ? formData.groupby : formData.groupbyB),
].forEach((dimension, i) =>
drillByFilters.push({
col: dimension,
op: '==',
val: values[i],
}),
[...(isFirst ? formData.groupby : formData.groupbyB)].forEach(
(dimension, i) =>
drillByFilters.push({
col: dimension,
op: '==',
val: values[i],
}),
);
onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
drillToDetail: drillToDetailFilters,
crossFilter: getCrossFilterDataMask(seriesName, seriesIndex),
drillBy: drillByFilters,
drillBy: {
filters: drillByFilters,
groupbyFieldName: isFirst ? 'groupby' : 'groupby_b',
adhocFilterFieldName: isFirst ? 'adhoc_filters' : 'adhoc_filters_b',
},
});
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export default function EchartsSunburst(props: SunburstTransformedProps) {
onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
drillToDetail: drillToDetailFilters,
crossFilter: getCrossFilterDataMask(treePathInfo),
drillBy: drillByFilters,
drillBy: { filters: drillByFilters, groupbyFieldName: 'columns' },
});
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ export default function EchartsTimeseries({
onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
drillToDetail: drillToDetailFilters,
crossFilter: getCrossFilterDataMask(seriesName),
drillBy: drillByFilters,
drillBy: { filters: drillByFilters, groupbyFieldName: 'groupby' },
});
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export default function EchartsTreemap({
onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
drillToDetail: drillToDetailFilters,
crossFilter: getCrossFilterDataMask(data, treePathInfo),
drillBy: drillByFilters,
drillBy: { filters: drillByFilters, groupbyFieldName: 'groupby' },
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export const contextMenuEventHandler =
onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
drillToDetail: drillFilters,
crossFilter: getCrossFilterDataMask(e.name),
drillBy: drillFilters,
drillBy: { filters: drillFilters, groupbyFieldName: 'groupby' },
});
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -478,17 +478,27 @@ export default function PivotTableChart(props: PivotTableProps) {
onContextMenu(e.clientX, e.clientY, {
drillToDetail: drillToDetailFilters,
crossFilter: getCrossFilterDataMask(dataPoint),
drillBy: [
{
col: Object.keys(dataPoint)[0],
op: '==',
val: Object.values(dataPoint)[0],
},
],
drillBy: {
filters: [
{
col: Object.keys(dataPoint)[0],
op: '==',
val: Object.values(dataPoint)[0],
},
],
groupbyFieldName: rowKey ? 'groupbyRows' : 'groupbyColumns',
},
});
}
},
[cols, dateFormatters, onContextMenu, rows, timeGrainSqla],
[
cols,
dateFormatters,
getCrossFilterDataMask,
onContextMenu,
rows,
timeGrainSqla,
],
);

return (
Expand Down
17 changes: 10 additions & 7 deletions superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,16 @@ export default function TableChart<D extends DataRecord = DataRecord>(
: getCrossFilterDataMask(cellPoint.key, cellPoint.value),
drillBy: cellPoint.isMetric
? undefined
: [
{
col: cellPoint.key,
op: '==',
val: cellPoint.value as string | number | boolean,
},
],
: {
filters: [
{
col: cellPoint.key,
op: '==',
val: cellPoint.value as string | number | boolean,
},
],
groupbyFieldName: 'groupby',
},
});
}
: undefined;
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/components/Chart/ChartContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ const ChartContextMenu = (
}
menuItems.push(
<DrillByMenuItems
filters={filters?.drillBy}
filters={filters?.drillBy?.filters}
groupbyFieldName={filters?.drillBy?.groupbyFieldName}
formData={formData}
contextMenuY={clientY}
submenuIndex={submenuIndex}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import fetchMock from 'fetch-mock';
import { render, screen, within, waitFor } from 'spec/helpers/testing-library';
import chartQueries, { sliceId } from 'spec/fixtures/mockChartQueries';
import { Menu } from 'src/components/Menu';
import { supersetGetCache } from 'src/utils/cachedSupersetGet';
import { DrillByMenuItems, DrillByMenuItemsProps } from './DrillByMenuItems';

/* eslint jest/expect-expect: ["warn", { "assertFunctionNames": ["expect*"] }] */
Expand Down Expand Up @@ -65,6 +66,7 @@ const renderMenu = ({
<DrillByMenuItems
formData={formData ?? defaultFormData}
filters={filters}
groupbyFieldName="groupby"
/>
</Menu>,
{ useRouter: true, useRedux: true },
Expand All @@ -89,7 +91,9 @@ const expectDrillByEnabled = async () => {
name: 'Drill by',
});
expect(drillByMenuItem).toBeInTheDocument();
expect(drillByMenuItem).not.toHaveAttribute('aria-disabled');
await waitFor(() =>
expect(drillByMenuItem).not.toHaveAttribute('aria-disabled'),
);
const tooltipTrigger =
within(drillByMenuItem).queryByTestId('tooltip-trigger');
expect(tooltipTrigger).not.toBeInTheDocument();
Expand All @@ -110,64 +114,77 @@ getChartMetadataRegistry().registerValue(
}),
);

test('render disabled menu item for unsupported chart', async () => {
renderMenu({ formData: { ...defaultFormData, viz_type: 'unsupported_viz' } });
await expectDrillByDisabled(
'Drill by is not yet supported for this chart type',
);
});

test('render disabled menu item for supported chart, no filters', async () => {
renderMenu({ filters: [] });
await expectDrillByDisabled('Drill by is not available for this data point');
});
describe('Drill by menu items', () => {
afterEach(() => {
supersetGetCache.clear();
fetchMock.restore();
});

test('render disabled menu item for supported chart, no columns', async () => {
renderMenu({});
await expectDrillByDisabled('No dimensions available for drill by');
});
test('render disabled menu item for unsupported chart', async () => {
renderMenu({
formData: { ...defaultFormData, viz_type: 'unsupported_viz' },
});
await expectDrillByDisabled(
'Drill by is not yet supported for this chart type',
);
});

test('render menu item with submenu without searchbox', async () => {
const slicedColumns = defaultColumns.slice(0, 9);
fetchMock.get(datasetEndpointMatcher, { result: { columns: slicedColumns } });
renderMenu({});
await waitFor(() => fetchMock.called(datasetEndpointMatcher));
await expectDrillByEnabled();
slicedColumns.forEach(column => {
expect(screen.getByText(column.column_name)).toBeInTheDocument();
test('render disabled menu item for supported chart, no filters', async () => {
renderMenu({ filters: [] });
await expectDrillByDisabled(
'Drill by is not available for this data point',
);
});
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
fetchMock.restore();
});

test('render menu item with submenu and searchbox', async () => {
fetchMock.get(datasetEndpointMatcher, {
result: { columns: defaultColumns },
test('render disabled menu item for supported chart, no columns', async () => {
fetchMock.get(datasetEndpointMatcher, { result: { columns: [] } });
renderMenu({});
await waitFor(() => fetchMock.called(datasetEndpointMatcher));
await expectDrillByDisabled('No dimensions available for drill by');
});
renderMenu({});
await waitFor(() => fetchMock.called(datasetEndpointMatcher));
await expectDrillByEnabled();
defaultColumns.forEach(column => {
expect(screen.getByText(column.column_name)).toBeInTheDocument();

test('render menu item with submenu without searchbox', async () => {
const slicedColumns = defaultColumns.slice(0, 9);
fetchMock.get(datasetEndpointMatcher, {
result: { columns: slicedColumns },
});
renderMenu({});
await waitFor(() => fetchMock.called(datasetEndpointMatcher));
await expectDrillByEnabled();
slicedColumns.forEach(column => {
expect(screen.getByText(column.column_name)).toBeInTheDocument();
});
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
});

const searchbox = screen.getByRole('textbox');
expect(searchbox).toBeInTheDocument();
test('render menu item with submenu and searchbox', async () => {
fetchMock.get(datasetEndpointMatcher, {
result: { columns: defaultColumns },
});
renderMenu({});
await waitFor(() => fetchMock.called(datasetEndpointMatcher));
await expectDrillByEnabled();
defaultColumns.forEach(column => {
expect(screen.getByText(column.column_name)).toBeInTheDocument();
});

userEvent.type(searchbox, 'col1');
const searchbox = screen.getByRole('textbox');
expect(searchbox).toBeInTheDocument();

await screen.findByText('col1');
userEvent.type(searchbox, 'col1');

const expectedFilteredColumnNames = ['col1', 'col10', 'col11'];
await screen.findByText('col1');

defaultColumns
.filter(col => !expectedFilteredColumnNames.includes(col.column_name))
.forEach(col => {
expect(screen.queryByText(col.column_name)).not.toBeInTheDocument();
});
const expectedFilteredColumnNames = ['col1', 'col10', 'col11'];

expectedFilteredColumnNames.forEach(colName => {
expect(screen.getByText(colName)).toBeInTheDocument();
defaultColumns
.filter(col => !expectedFilteredColumnNames.includes(col.column_name))
.forEach(col => {
expect(screen.queryByText(col.column_name)).not.toBeInTheDocument();
});

expectedFilteredColumnNames.forEach(colName => {
expect(screen.getByText(colName)).toBeInTheDocument();
});
});
fetchMock.restore();
});
Loading

0 comments on commit d793393

Please sign in to comment.