mikebridge commented on code in PR #42540:
URL: https://github.com/apache/superset/pull/42540#discussion_r3897890884
##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx:
##########
@@ -392,7 +464,42 @@ const ColumnSelectPopover = ({
selectedMetric?.metric_name !== undefined ||
adhocColumn?.sqlExpression !== initialAdhocColumn?.sqlExpression;
- const savedExpressionsLabel = t('Saved expressions');
+ // With Saved classification, a value that can no longer be committed keeps
+ // Save disabled until the user explicitly picks a compatible dimension.
+ const invalidSelectionFeedback = useMemo(() => {
+ if (!savedClassification) {
+ return null;
+ }
+ if (adhocColumn) {
+ return t(
+ 'Custom column values are not supported here. Select a saved dimension
to replace this value.',
+ );
+ }
+ if (
+ selectedCalculatedColumn &&
+ compatibleDimensions != null &&
+ !compatibleDimensions.includes(selectedCalculatedColumn.column_name)
+ ) {
Review Comment:
Addressed in 4342bcae89 (metric-keyed compatibility).
##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx:
##########
@@ -446,9 +582,41 @@ const ColumnSelectPopover = ({
column_name: calculatedColumn.column_name,
verbose_name:
calculatedColumn.verbose_name ?? '',
- }),
- )}
- optionFilterProps={['column_name', 'verbose_name']}
+ disabled:
+ savedClassification &&
+ compatibleDimensions != null &&
+ !compatibleDimensions.includes(
+ calculatedColumn.column_name,
+ ),
+ })),
+ ...(savedClassification
+ ? availableMetrics.map(metric => ({
+ value: metric.metric_name,
Review Comment:
Fixed in 4342bcae89: metric options are now disabled via
selectCompatibleMetricNames instead of the dimension list; a regression test
uses a fixture where the dimension list contains the other metric's name, so
keying off the wrong list fails both assertions.
##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx:
##########
@@ -446,9 +582,41 @@ const ColumnSelectPopover = ({
column_name: calculatedColumn.column_name,
verbose_name:
calculatedColumn.verbose_name ?? '',
- }),
- )}
- optionFilterProps={['column_name', 'verbose_name']}
+ disabled:
+ savedClassification &&
+ compatibleDimensions != null &&
+ !compatibleDimensions.includes(
+ calculatedColumn.column_name,
+ ),
+ })),
+ ...(savedClassification
+ ? availableMetrics.map(metric => ({
+ value: metric.metric_name,
+ label: (
+ <MetricOptionContainer>
+ <MetricIcon>ƒ</MetricIcon>
+ <MetricLabel>
+ {metric.verbose_name ||
+ metric.metric_name}
+ </MetricLabel>
+ </MetricOptionContainer>
+ ),
+ key: `metric-${metric.metric_name}`,
+ metric_name: metric.metric_name,
+ verbose_name: metric.verbose_name ?? '',
+ disabled:
+ compatibleDimensions != null &&
+ !compatibleDimensions.includes(
+ metric.metric_name,
+ ),
Review Comment:
Addressed in 4342bcae89 (metric-keyed compatibility).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
