From ce3ba67cf63e90059d94e2aa956982ad4ea44d1e Mon Sep 17 00:00:00 2001
From: Stepan <66589759+Always-prog@users.noreply.github.com>
Date: Thu, 30 Mar 2023 20:10:31 +0300
Subject: [PATCH] fix(conditional formatting): controls looses on save (#23137)

---
 .../src/plugin/controlPanel.tsx               |  4 +-
 .../plugin-chart-table/src/controlPanel.tsx   |  2 +
 .../ConditionalFormattingControl.tsx          | 37 ++++++++-----------
 .../ConditionalFormattingControl/types.ts     |  1 +
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx
index 1b01e37a4c..d099406c55 100644
--- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx
@@ -384,7 +384,7 @@ const config: ControlPanelConfig = {
               renderTrigger: true,
               label: t('Conditional formatting'),
               description: t('Apply conditional color formatting to metrics'),
-              mapStateToProps(explore) {
+              mapStateToProps(explore, _, chart) {
                 const values =
                   (explore?.controls?.metrics?.value as QueryFormMetric[]) ??
                   [];
@@ -393,6 +393,7 @@ const config: ControlPanelConfig = {
                 )
                   ? (explore?.datasource as Dataset)?.verbose_map
                   : explore?.datasource?.columns ?? {};
+                const chartStatus = chart?.chartStatus;
                 const metricColumn = values.map(value => {
                   if (typeof value === 'string') {
                     return { value, label: verboseMap[value] ?? value };
@@ -400,6 +401,7 @@ const config: ControlPanelConfig = {
                   return { value: value.label, label: value.label };
                 });
                 return {
+                  removeIrrelevantConditions: chartStatus === 'success',
                   columnOptions: metricColumn,
                   verboseMap,
                 };
diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
index 35c5d3970a..9ca5ce63b1 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
@@ -510,6 +510,7 @@ const config: ControlPanelConfig = {
                 )
                   ? (explore?.datasource as Dataset)?.verbose_map
                   : explore?.datasource?.columns ?? {};
+                const chartStatus = chart?.chartStatus;
                 const { colnames, coltypes } =
                   chart?.queriesResponse?.[0] ?? {};
                 const numericColumns =
@@ -525,6 +526,7 @@ const config: ControlPanelConfig = {
                         }))
                     : [];
                 return {
+                  removeIrrelevantConditions: chartStatus === 'success',
                   columnOptions: numericColumns,
                   verboseMap,
                 };
diff --git a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.tsx b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.tsx
index 8fb9b27453..2b9050c3bd 100644
--- a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.tsx
+++ b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.tsx
@@ -16,14 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useCallback, useEffect, useState } from 'react';
-import {
-  styled,
-  css,
-  t,
-  useTheme,
-  useComponentDidUpdate,
-} from '@superset-ui/core';
+import React, { useEffect, useState } from 'react';
+import { styled, css, t, useTheme } from '@superset-ui/core';
 import Icons from 'src/components/Icons';
 import ControlHeader from 'src/explore/components/ControlHeader';
 import { FormattingPopover } from './FormattingPopover';
@@ -76,6 +70,7 @@ const ConditionalFormattingControl = ({
   onChange,
   columnOptions,
   verboseMap,
+  removeIrrelevantConditions,
   ...props
 }: ConditionalFormattingControlProps) => {
   const theme = useTheme();
@@ -88,20 +83,20 @@ const ConditionalFormattingControl = ({
     }
   }, [conditionalFormattingConfigs, onChange]);
 
-  // remove formatter when corresponding column is removed from controls
-  const removeFormattersWhenColumnsChange = useCallback(() => {
-    const newFormattingConfigs = conditionalFormattingConfigs.filter(config =>
-      columnOptions.some(option => option?.value === config?.column),
-    );
-    if (
-      newFormattingConfigs.length !== conditionalFormattingConfigs.length &&
-      onChange
-    ) {
-      setConditionalFormattingConfigs(newFormattingConfigs);
-      onChange(newFormattingConfigs);
+  useEffect(() => {
+    if (removeIrrelevantConditions) {
+      // remove formatter when corresponding column is removed from controls
+      const newFormattingConfigs = conditionalFormattingConfigs.filter(config =>
+        columnOptions.some((option: any) => option?.value === config?.column),
+      );
+      if (
+        newFormattingConfigs.length !== conditionalFormattingConfigs.length &&
+        removeIrrelevantConditions
+      ) {
+        setConditionalFormattingConfigs(newFormattingConfigs);
+      }
     }
-  }, [JSON.stringify(columnOptions)]);
-  useComponentDidUpdate(removeFormattersWhenColumnsChange);
+  }, [conditionalFormattingConfigs, columnOptions, removeIrrelevantConditions]);
 
   const onDelete = (index: number) => {
     setConditionalFormattingConfigs(prevConfigs =>
diff --git a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/types.ts b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/types.ts
index cd89828daa..5d4c511459 100644
--- a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/types.ts
+++ b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/types.ts
@@ -55,6 +55,7 @@ export type ConditionalFormattingControlProps = ControlComponentProps<
   ConditionalFormattingConfig[]
 > & {
   columnOptions: { label: string; value: string }[];
+  removeIrrelevantConditions: boolean;
   verboseMap: Record<string, string>;
   label: string;
   description: string;