(superset) 11/45: fix(explore): handle boolean false values correctly in control rendering (#39172)

[mi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

michaelsmolina pushed a commit to branch 6.1
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 1e4ad1ba5290a92658f23a1b56359c94ffd3fc8c
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Tue Apr 7 18:23:03 2026 -0700

    fix(explore): handle boolean false values correctly in control rendering (#39172)
    
    Co-authored-by: Claude Opus 4.6 <no...@anthropic.com>
    (cherry picked from commit 6ba90968701fbce377d6479842329fed86c31e4f)
---
 .../ConditionalFormattingControl.test.tsx          | 83 ++++++++++++++++++++++
 .../ConditionalFormattingControl.tsx               |  5 ++
 ...AdhocFilterEditPopoverSimpleTabContent.test.tsx | 20 ++++++
 .../index.tsx                                      |  5 +-
 .../exploreUtils/getSimpleSQLExpression.test.ts    | 12 ++++
 .../src/explore/exploreUtils/index.ts              |  3 +-
 6 files changed, 125 insertions(+), 3 deletions(-)

diff --git a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.test.tsx b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.test.tsx
new file mode 100644
index 00000000000..11716aa43a1
--- /dev/null
+++ b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.test.tsx
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, screen } from 'spec/helpers/testing-library';
+import { Comparator } from '@superset-ui/chart-controls';
+import { GenericDataType } from '@apache-superset/core/common';
+import ConditionalFormattingControl from './ConditionalFormattingControl';
+import { ConditionalFormattingConfig } from './types';
+
+const columnOptions = [
+  { label: 'My Column', value: 'my_col', dataType: GenericDataType.Boolean },
+];
+
+const defaultProps = {
+  columnOptions,
+  verboseMap: {} as Record<string, string>,
+  removeIrrelevantConditions: false,
+  label: 'Conditional Formatting',
+  description: 'Test',
+  name: 'conditional_formatting',
+  onChange: jest.fn(),
+};
+
+test('renders "is false" operator label without trailing undefined', () => {
+  const value: ConditionalFormattingConfig[] = [
+    { column: 'my_col', operator: Comparator.IsFalse, colorScheme: 'colorSuccess' },
+  ];
+  render(<ConditionalFormattingControl {...defaultProps} value={value} />);
+  expect(screen.getByText('my_col is false')).toBeInTheDocument();
+});
+
+test('renders "is true" operator label without trailing undefined', () => {
+  const value: ConditionalFormattingConfig[] = [
+    { column: 'my_col', operator: Comparator.IsTrue, colorScheme: 'colorSuccess' },
+  ];
+  render(<ConditionalFormattingControl {...defaultProps} value={value} />);
+  expect(screen.getByText('my_col is true')).toBeInTheDocument();
+});
+
+test('renders "is null" operator label without trailing undefined', () => {
+  const value: ConditionalFormattingConfig[] = [
+    { column: 'my_col', operator: Comparator.IsNull, colorScheme: 'colorSuccess' },
+  ];
+  render(<ConditionalFormattingControl {...defaultProps} value={value} />);
+  expect(screen.getByText('my_col is null')).toBeInTheDocument();
+});
+
+test('renders "is not null" operator label without trailing undefined', () => {
+  const value: ConditionalFormattingConfig[] = [
+    { column: 'my_col', operator: Comparator.IsNotNull, colorScheme: 'colorSuccess' },
+  ];
+  render(<ConditionalFormattingControl {...defaultProps} value={value} />);
+  expect(screen.getByText('my_col is not null')).toBeInTheDocument();
+});
+
+test('renders verbose column name when available', () => {
+  const value: ConditionalFormattingConfig[] = [
+    { column: 'my_col', operator: Comparator.IsFalse, colorScheme: 'colorSuccess' },
+  ];
+  render(
+    <ConditionalFormattingControl
+      {...defaultProps}
+      verboseMap={{ my_col: 'My Column' }}
+      value={value}
+    />,
+  );
+  expect(screen.getByText('My Column is false')).toBeInTheDocument();
+});
diff --git a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.tsx b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.tsx
index c476cc67772..52cb263f503 100644
--- a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.tsx
+++ b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.tsx
@@ -136,6 +136,11 @@ const ConditionalFormattingControl = ({
         return `${targetValueLeft} ${Comparator.LessOrEqual} ${columnName} ${Comparator.LessThan} ${targetValueRight}`;
       case Comparator.BetweenOrRightEqual:
         return `${targetValueLeft} ${Comparator.LessThan} ${columnName} ${Comparator.LessOrEqual} ${targetValueRight}`;
+      case Comparator.IsTrue:
+      case Comparator.IsFalse:
+      case Comparator.IsNull:
+      case Comparator.IsNotNull:
+        return `${columnName} ${operator}`;
       default:
         return `${columnName} ${operator} ${targetValue}`;
     }
diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx
index 1c0cb00f7e6..a238ca38f28 100644
--- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx
+++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx
@@ -269,6 +269,26 @@ test('will convert from individual comparator to array if the operator changes t
   ).toEqual(Operators.In);
 });
 
+test('will preserve boolean false comparator when converting to multi operator', () => {
+  const booleanFalseFilter = new AdhocFilter({
+    expressionType: ExpressionTypes.Simple,
+    subject: 'value',
+    operatorId: Operators.Equals,
+    operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.Equals].operation,
+    comparator: false,
+    clause: Clauses.Where,
+  });
+  const props = setup({ adhocFilter: booleanFalseFilter });
+  const { onOperatorChange } = useSimpleTabFilterProps(
+    props as unknown as Props,
+  );
+  onOperatorChange(Operators.In);
+  expect(
+    props.onChange.mock.calls[props.onChange.mock.calls.length - 1][0]
+      .comparator,
+  ).toEqual([false]);
+});
+
 test('will convert from array to individual comparators if the operator changes from multi', () => {
   const props = setup({
     adhocFilter: simpleMultiAdhocFilter,
diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx
index ff9c4db9274..64a590a01d8 100644
--- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx
+++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx
@@ -199,7 +199,7 @@ export const useSimpleTabFilterProps = (props: Props) => {
     if (MULTI_OPERATORS.has(operatorId)) {
       newComparator = Array.isArray(currentComparator)
         ? currentComparator
-        : [currentComparator].filter(element => element);
+        : [currentComparator].filter(element => element != null);
     } else {
       newComparator = Array.isArray(currentComparator)
         ? currentComparator[0]
@@ -396,7 +396,8 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
   };
 
   const comparatorHasValue =
-    comparator &&
+    comparator != null &&
+    comparator !== '' &&
     (Array.isArray(comparator)
       ? comparator.length > 0
       : String(comparator).length > 0);
diff --git a/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts b/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts
index b13b0853035..d2319cf3268 100644
--- a/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts
+++ b/superset-frontend/src/explore/exploreUtils/getSimpleSQLExpression.test.ts
@@ -70,3 +70,15 @@ test('Should return correct string when subject and operator are valid values',
     ]),
   ).toBe("subject operator 'comparator', 'comparator-2'");
 });
+
+test('Should handle boolean false comparator as a string value', () => {
+  expect(getSimpleSQLExpression(params.subject, params.operator, false)).toBe(
+    "subject operator 'FALSE'",
+  );
+});
+
+test('Should handle boolean true comparator as a string value', () => {
+  expect(getSimpleSQLExpression(params.subject, params.operator, true)).toBe(
+    "subject operator 'TRUE'",
+  );
+});
diff --git a/superset-frontend/src/explore/exploreUtils/index.ts b/superset-frontend/src/explore/exploreUtils/index.ts
index d67e5f8474b..e0207f22dd7 100644
--- a/superset-frontend/src/explore/exploreUtils/index.ts
+++ b/superset-frontend/src/explore/exploreUtils/index.ts
@@ -458,7 +458,8 @@ export const getSimpleSQLExpression = (
       isMulti && Array.isArray(comparator) ? comparator[0] : comparator;
     const comparatorArray = ensureIsArray(comparator);
     const isString =
-      firstValue !== undefined && Number.isNaN(Number(firstValue));
+      firstValue !== undefined &&
+      (typeof firstValue === 'boolean' || Number.isNaN(Number(firstValue)));
     const quote = isString ? "'" : '';
     const [prefix, suffix] = isMulti ? ['(', ')'] : ['', ''];
     if (comparatorArray.length > 0 && showComparator) {