From 561d1ac9f2f3861619e319cf69befb57d6559485 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Mon, 8 Nov 2021 10:31:43 -0800 Subject: [PATCH] feat(metrics): Provide override for disabling ad-hoc metrics (#17202) Co-authored-by: John Bodley --- .../components/ExploreContentPopover.tsx | 6 - .../DndMetricSelect.tsx | 6 +- .../AdhocFilterEditPopover/index.jsx | 128 ++++++++---------- .../controls/FixedOrMetricControl/index.jsx | 1 + .../AdhocMetricEditPopover.test.tsx | 56 +++++++- .../AdhocMetricEditPopover/index.jsx | 98 ++++++++++---- .../MetricControl/AdhocMetricOption.jsx | 6 +- .../AdhocMetricPopoverTrigger.tsx | 8 +- .../MetricControl/MetricDefinitionValue.jsx | 6 +- .../controls/MetricControl/MetricsControl.jsx | 9 +- superset-frontend/src/explore/controls.jsx | 4 +- 11 files changed, 195 insertions(+), 133 deletions(-) diff --git a/superset-frontend/src/explore/components/ExploreContentPopover.tsx b/superset-frontend/src/explore/components/ExploreContentPopover.tsx index 6a91c6b4ca29f..93ee96a24e977 100644 --- a/superset-frontend/src/explore/components/ExploreContentPopover.tsx +++ b/superset-frontend/src/explore/components/ExploreContentPopover.tsx @@ -29,10 +29,4 @@ export const ExplorePopoverContent = styled.div` .filter-sql-editor { border: ${({ theme }) => theme.colors.grayscale.light2} solid thin; } - .custom-sql-disabled-message { - color: ${({ theme }) => theme.colors.grayscale.light1}; - font-size: ${({ theme }) => theme.typography.sizes.xs}px; - text-align: center; - margin-top: ${({ theme }) => theme.gridUnit * 15}px; - } `; diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx index c733ad69b10c2..1936eb8995135 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx @@ -284,7 +284,7 @@ export const DndMetricSelect = (props: any) => { columns={props.columns} savedMetrics={props.savedMetrics} savedMetricsOptions={getSavedMetricOptionsForMetric(index)} - datasourceType={props.datasourceType} + datasource={props.datasource} onMoveLabel={moveLabel} onDropLabel={handleDropLabel} type={`${DndItemType.AdhocMetricOption}_${props.name}_${props.label}`} @@ -299,7 +299,7 @@ export const DndMetricSelect = (props: any) => { onMetricEdit, onRemoveMetric, props.columns, - props.datasourceType, + props.datasource, props.label, props.name, props.savedMetrics, @@ -396,7 +396,7 @@ export const DndMetricSelect = (props: any) => { columns={props.columns} savedMetricsOptions={newSavedMetricOptions} savedMetric={EMPTY_OBJECT as savedMetricType} - datasourceType={props.datasourceType} + datasource={props.datasource} isControlledComponent visible={newMetricPopoverVisible} togglePopover={togglePopover} diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx index afe501ec5e691..097cdb4122db9 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx @@ -19,6 +19,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import Button from 'src/components/Button'; +import { Tooltip } from 'src/components/Tooltip'; import { styled, t } from '@superset-ui/core'; import ErrorBoundary from 'src/components/ErrorBoundary'; @@ -56,11 +57,6 @@ const ResizeIcon = styled.i` const startingWidth = 320; const startingHeight = 240; -const SectionWrapper = styled.div` - .ant-select { - margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; - } -`; const FilterPopoverContentContainer = styled.div` .adhoc-filter-edit-tabs > .nav-tabs { @@ -173,51 +169,15 @@ export default class AdhocFilterEditPopover extends React.Component { onResize, datasource, partitionColumn, - sections = ['SIMPLE', 'CUSTOM_SQL'], theme, operators, ...popoverProps } = this.props; const { adhocFilter } = this.state; - - const resultSections = - datasource?.type === 'druid' - ? sections.filter(s => s !== 'CUSTOM_SQL') - : sections; - const stateIsValid = adhocFilter.isValid(); const hasUnsavedChanges = !adhocFilter.equals(propsAdhocFilter); - const sectionRenders = {}; - - sectionRenders.CUSTOM_SQL = ( - - - - ); - - sectionRenders.SIMPLE = ( - - - - ); - return ( - {resultSections.length > 1 ? ( - + + + + + + + {t('Custom SQL')} + + ) : ( + t('Custom SQL') + ) + } + disabled={datasource?.type === 'druid'} > - {resultSections.includes('SIMPLE') && ( - - {sectionRenders.SIMPLE} - - )} - {resultSections.includes('CUSTOM_SQL') && ( - - {sectionRenders.CUSTOM_SQL} - - )} - - ) : ( - {sectionRenders[resultSections[0]]} - )} + + + + +
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx index 8d4ef8a624656..d665befcca6ad 100644 --- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx @@ -41,7 +41,10 @@ const createProps = () => ({ }, ], adhocMetric: new AdhocMetric({ isNew: true }), - datasourceType: 'table', + datasource: { + extra: '{}', + type: 'table', + }, columns: [ { id: 1342, @@ -62,18 +65,61 @@ test('Should render', () => { test('Should render correct elements', () => { const props = createProps(); render(); - expect(screen.getByRole('tablist')).toBeVisible(); + expect(screen.getByRole('button', { name: 'Resize' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'Save' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'Close' })).toBeVisible(); +}); +test('Should render correct elements for SQL', () => { + const props = createProps(); + render(); expect(screen.getByRole('tab', { name: 'Custom SQL' })).toBeVisible(); expect(screen.getByRole('tab', { name: 'Simple' })).toBeVisible(); expect(screen.getByRole('tab', { name: 'Saved' })).toBeVisible(); + expect(screen.getByRole('tabpanel', { name: 'Saved' })).toBeVisible(); +}); + +test('Should render correct elements for native Druid', () => { + const props = { ...createProps(), datasource: { type: 'druid' } }; + render(); + expect(screen.getByRole('tab', { name: 'Custom SQL' })).toHaveAttribute( + 'aria-disabled', + 'true', + ); + expect(screen.getByRole('tab', { name: 'Simple' })).toBeEnabled(); + expect(screen.getByRole('tab', { name: 'Saved' })).toBeEnabled(); + expect(screen.getByRole('tabpanel', { name: 'Saved' })).toBeVisible(); +}); +test('Should render correct elements for allow ad-hoc metrics', () => { + const props = { + ...createProps(), + datasource: { extra: '{"disallow_adhoc_metrics": false}' }, + }; + render(); + expect(screen.getByRole('tab', { name: 'Custom SQL' })).toBeEnabled(); + expect(screen.getByRole('tab', { name: 'Simple' })).toBeEnabled(); + expect(screen.getByRole('tab', { name: 'Saved' })).toBeEnabled(); expect(screen.getByRole('tabpanel', { name: 'Saved' })).toBeVisible(); +}); - expect(screen.getByRole('button', { name: 'Resize' })).toBeVisible(); - expect(screen.getByRole('button', { name: 'Save' })).toBeVisible(); - expect(screen.getByRole('button', { name: 'Close' })).toBeVisible(); +test('Should render correct elements for disallow ad-hoc metrics', () => { + const props = { + ...createProps(), + datasource: { extra: '{"disallow_adhoc_metrics": true}' }, + }; + render(); + expect(screen.getByRole('tab', { name: 'Custom SQL' })).toHaveAttribute( + 'aria-disabled', + 'true', + ); + expect(screen.getByRole('tab', { name: 'Simple' })).toHaveAttribute( + 'aria-disabled', + 'true', + ); + expect(screen.getByRole('tab', { name: 'Saved' })).toBeEnabled(); + expect(screen.getByRole('tabpanel', { name: 'Saved' })).toBeVisible(); }); test('Clicking on "Close" should call onClose', () => { diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx index 59760e9766ba0..e997e8c7ff04a 100644 --- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx @@ -22,6 +22,7 @@ import PropTypes from 'prop-types'; import Tabs from 'src/components/Tabs'; import Button from 'src/components/Button'; import { Select } from 'src/components'; +import { Tooltip } from 'src/components/Tooltip'; import { t, styled } from '@superset-ui/core'; import { Form, FormItem } from 'src/components/Form'; @@ -50,7 +51,7 @@ const propTypes = { columns: PropTypes.arrayOf(columnType), savedMetricsOptions: PropTypes.arrayOf(savedMetricType), savedMetric: savedMetricType, - datasourceType: PropTypes.string, + datasource: PropTypes.object, }; const defaultProps = { @@ -277,7 +278,7 @@ export default class AdhocMetricEditPopover extends React.PureComponent { onChange, onClose, onResize, - datasourceType, + datasource, ...popoverProps } = this.props; const { adhocMetric, savedMetric } = this.state; @@ -322,7 +323,10 @@ export default class AdhocMetricEditPopover extends React.PureComponent { autoFocus: true, }; - if (this.props.datasourceType === 'druid' && aggregateSelectProps.options) { + if ( + this.props.datasource?.type === 'druid' && + aggregateSelectProps.options + ) { aggregateSelectProps.options = aggregateSelectProps.options.filter( aggregate => aggregate !== 'AVG', ); @@ -337,6 +341,13 @@ export default class AdhocMetricEditPopover extends React.PureComponent { ) && savedMetric?.metric_name !== propsSavedMetric?.metric_name); + let extra = {}; + if (datasource?.extra) { + try { + extra = JSON.parse(datasource.extra); + } catch {} // eslint-disable-line no-empty + } + return (
- + + {t('Simple')} + + ) : ( + t('Simple') + ) + } + disabled={extra.disallow_adhoc_metrics} + >