Fix sunburst color assignment that causes pie chart crashing (#49183)

* pie chart crashes when light or dark accents of instance colors overriden

* Update Loki Snapshots

---------

Co-authored-by: Metabase Automation <github-automation@metabase.com>

e2e/test/scenarios/visualizations-charts/visualizations-charts-reproductions.cy.spec.js

@@ -7,6 +7,7 @@ import {
   cartesianChartCircle,
   chartPathWithFillColor,
   createQuestion,
+  describeEE,
   echartsContainer,
   editDashboard,
   filter,
@@ -24,9 +25,11 @@ import {
   saveDashboard,
   saveSavedQuestion,
   selectFilterOperator,
+  setTokenFeatures,
   sidebar,
   summarize,
   testPairedTooltipValues,
+  updateSetting,
   visitAlias,
   visitDashboard,
   visitQuestionAdhoc,
@@ -1207,3 +1210,55 @@ describe("issue 43077", () => {
     cy.wait(100).then(() => expect(cardRequestSpy).not.to.have.been.called);
   });
 });
+
+describeEE("issue 49160", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsAdmin();
+    setTokenFeatures("all");
+  });
+
+  it("pie chart should have a placeholder", () => {
+    visitQuestionAdhoc({
+      dataset_query: {
+        type: "query",
+        query: {
+          "source-table": PRODUCTS_ID,
+        },
+        database: SAMPLE_DB_ID,
+      },
+      display: "pie",
+    });
+
+    // Shows pie placeholder
+    echartsContainer().findByText("18,760").should("be.visible");
+    cy.findByTestId("qb-header-action-panel").findByText("Summarize").click();
+
+    cy.findByLabelText("Rating").click();
+    echartsContainer().findByText("200").should("be.visible");
+    echartsContainer().findByText("TOTAL").should("be.visible");
+  });
+
+  it("pie chart should work when instance colors have overrides", () => {
+    updateSetting("application-colors", { "accent0-light": "#98b4ce" });
+
+    visitQuestionAdhoc({
+      dataset_query: {
+        type: "query",
+        query: {
+          "source-table": PRODUCTS_ID,
+          aggregation: [["count"]],
+          breakout: [["field", PRODUCTS.CATEGORY, null]],
+        },
+        database: SAMPLE_DB_ID,
+      },
+      display: "pie",
+    });
+
+    echartsContainer().findByText("200").should("be.visible");
+
+    cy.findByTestId("viz-settings-button").click();
+
+    leftSidebar().findByText("Gizmo");
+  });
+});

frontend/src/metabase/visualizations/echarts/pie/model/index.ts

@@ -330,11 +330,11 @@ export function getPieChartModel(
     if (!hasMultipleRings) {
       return hexColor;
     }
-    const accentNumber = hexToAccentColorMap.get(hexColor);
-    if (accentNumber == null) {
+    const accentKey = hexToAccentColorMap.get(hexColor);
+    if (accentKey == null) {
       return hexColor;
     }
-    return renderingContext.getColor(getRingColorAlias(accentNumber, ring));
+    return renderingContext.getColor(getRingColorAlias(accentKey, ring));
   }
 
   // Create sliceTree, fill out the innermost slice ring

frontend/src/metabase/visualizations/echarts/pie/util/colors.ts

 import { aliases, colors } from "metabase/lib/colors";
-import { checkNumber } from "metabase/lib/types";
 
 const ACCENT_KEY_PREFIX = "accent";
 
+type AccentKey = string;
+
+const isAccentColorKey = (key: string) => key.startsWith(ACCENT_KEY_PREFIX);
+
+const extractAccentKey = (input: string): AccentKey => {
+  const withoutPrefix = input.slice(ACCENT_KEY_PREFIX.length);
+  return withoutPrefix.split("-")[0] ?? null;
+};
+
 export function createHexToAccentNumberMap() {
-  const hexToAccentNumber = new Map<string, number>();
+  const hexToAccentNumber = new Map<string, AccentKey>();
 
-  for (const [key, hex] of Object.entries(colors)) {
-    if (!key.startsWith(ACCENT_KEY_PREFIX)) {
+  for (const [colorKey, hex] of Object.entries(colors)) {
+    if (!isAccentColorKey(colorKey)) {
       continue;
     }
-
-    const accentNumber = checkNumber(
-      Number(key.slice(ACCENT_KEY_PREFIX.length)),
-    );
-
-    hexToAccentNumber.set(hex, accentNumber);
+    const accentKey = extractAccentKey(colorKey);
+    if (accentKey) {
+      hexToAccentNumber.set(hex, accentKey);
+    }
   }
 
-  for (const [key, hexGetter] of Object.entries(aliases)) {
-    if (!key.startsWith(ACCENT_KEY_PREFIX)) {
+  for (const [colorKey, hexGetter] of Object.entries(aliases)) {
+    if (!isAccentColorKey(colorKey)) {
       continue;
     }
 
-    const accentNumber = checkNumber(
-      Number(key.slice(ACCENT_KEY_PREFIX.length, ACCENT_KEY_PREFIX.length + 1)),
-    );
-    const hex = hexGetter(colors);
-
-    hexToAccentNumber.set(hex, accentNumber);
+    const accentKey = extractAccentKey(colorKey);
+    if (accentKey) {
+      hexToAccentNumber.set(hexGetter(colors), accentKey);
+    }
   }
 
   return hexToAccentNumber;
 }
 
 export function getRingColorAlias(
-  accentColorNumber: number,
+  accentKey: AccentKey,
   ring: "inner" | "middle" | "outer",
 ) {
   let suffix = "";
@@ -45,9 +49,9 @@ export function getRingColorAlias(
     suffix = "-light";
   }
 
-  return `${ACCENT_KEY_PREFIX}${accentColorNumber}${suffix}`;
+  return `${ACCENT_KEY_PREFIX}${accentKey}${suffix}`;
 }
 
-export function getPickerColorAlias(accentNumber: number) {
-  return `${ACCENT_KEY_PREFIX}${accentNumber}-dark`;
+export function getPickerColorAlias(accentKey: AccentKey) {
+  return `${ACCENT_KEY_PREFIX}${accentKey}-dark`;
 }

frontend/src/metabase/visualizations/visualizations/PieChart/PieChart.tsx

@@ -31,6 +31,8 @@ export function PieChart(props: VisualizationProps) {
     onRender,
     isDashboard,
     isFullscreen,
+    isPlaceholder,
+    series: transformedSeries,
   } = props;
   const hoveredIndex = props.hovered?.index;
   const hoveredSliceKeyPath = props.hovered?.pieSliceKeyPath;
@@ -57,22 +59,22 @@ export function PieChart(props: VisualizationProps) {
     () => extractRemappings(rawSeries),
     [rawSeries],
   );
+
+  const seriesToRender = useMemo(
+    () => (isPlaceholder ? transformedSeries : rawSeriesWithRemappings),
+    [isPlaceholder, transformedSeries, rawSeriesWithRemappings],
+  );
+
   const chartModel = useMemo(
     () =>
       getPieChartModel(
-        rawSeriesWithRemappings,
+        seriesToRender,
         settings,
         Array.from(hiddenSlices),
         renderingContext,
         showWarning,
       ),
-    [
-      rawSeriesWithRemappings,
-      settings,
-      hiddenSlices,
-      renderingContext,
-      showWarning,
-    ],
+    [seriesToRender, settings, hiddenSlices, renderingContext, showWarning],
   );
   const formatters = useMemo(
     () => getPieChartFormatters(chartModel, settings),

frontend/src/metabase/visualizations/visualizations/PieChart/PieRowsPicker.tsx

@@ -41,11 +41,11 @@ export function PieRowsPicker({
     if (!hasMultipleRings || hexColor == null) {
       return hexColor;
     }
-    const accentNumber = hexToAccentNumberMap.get(hexColor);
-    if (accentNumber == null) {
+    const accentKey = hexToAccentNumberMap.get(hexColor);
+    if (accentKey == null) {
       return hexColor;
     }
-    return color(getPickerColorAlias(accentNumber));
+    return color(getPickerColorAlias(accentKey));
   };
 
   const onChangeSeriesColor = (sliceKey: string, color: string) =>

frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts

@@ -77,7 +77,11 @@ export const PIE_CHART_DEFINITION: VisualizationDefinition = {
     if (rows.length < 1) {
       throw new MinRowsError(1, 0);
     }
-    if (!settings["pie.dimension"] || !settings["pie.metric"]) {
+    const isDimensionMissing =
+      !settings["pie.dimension"] ||
+      (Array.isArray(settings["pie.dimension"]) &&
+        settings["pie.dimension"].every(col => col == null));
+    if (isDimensionMissing || !settings["pie.metric"]) {
       throw new ChartSettingsError(t`Which columns do you want to use?`, {
         section: `Data`,
       });