You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/10/20 18:29:12 UTC

(superset) branch master updated: fix(dashboard): stop Edit Properties hanging on dashboards with many charts (#45354)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 1811fe5167f fix(dashboard): stop Edit Properties hanging on dashboards with many charts (#45354)
1811fe5167f is described below

commit 1811fe5167f442e6a972ef7600c0188e113159e3
Author: Joe Li <jo...@preset.io>
AuthorDate: Tue Oct 20 11:29:04 2026 -0700

    fix(dashboard): stop Edit Properties hanging on dashboards with many charts (#45354)
    
    Co-authored-by: Claude Sonnet 5.5 <no...@anthropic.com>
---
 .../PropertiesModal.manyCharts.test.tsx            | 107 +++++++++++++++++++++
 .../dashboard/components/PropertiesModal/index.tsx |   9 +-
 2 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.manyCharts.test.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.manyCharts.test.tsx
new file mode 100644
index 00000000000..9a992d1fd95
--- /dev/null
+++ b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.manyCharts.test.tsx
@@ -0,0 +1,107 @@
+/**
+ * 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, waitFor } from 'spec/helpers/testing-library';
+import fetchMock from 'fetch-mock';
+import { Form } from '@superset-ui/core/components';
+import PropertiesModal from '.';
+
+jest.mock('@superset-ui/core', () => ({
+  ...jest.requireActual('@superset-ui/core'),
+  isFeatureEnabled: jest.fn(() => false),
+  getCategoricalSchemeRegistry: jest.fn(() => ({
+    keys: () => ['supersetColors'],
+    get: () => ['#FFFFFF', '#000000'],
+    getDefaultKey: () => 'supersetColors',
+    getMap: () => ({}),
+  })),
+}));
+
+fetchMock.get('glob:*/api/v1/theme/*', { body: { result: [] } });
+
+afterAll(() => {
+  fetchMock.clearHistory().removeRoutes();
+});
+
+// Hydration gives every interactive chart a `chartsInScope` list of all the
+// other charts, so `chart_configuration` grows with the square of the chart
+// count.
+const buildDashboardInfo = (chartCount: number) => {
+  const chartIds = Array.from({ length: chartCount }, (_, i) => i + 1);
+  const chartConfiguration = Object.fromEntries(
+    chartIds.map(id => [
+      id,
+      {
+        id,
+        crossFilters: {
+          scope: 'global',
+          chartsInScope: chartIds.filter(other => other !== id),
+        },
+      },
+    ]),
+  );
+  return {
+    id: 26,
+    dashboard_title: 'Many charts',
+    slug: null,
+    css: '',
+    certified_by: '',
+    certification_details: '',
+    editors: [],
+    viewers: [],
+    metadata: {
+      refresh_frequency: 0,
+      chart_configuration: chartConfiguration,
+      global_chart_configuration: {
+        scope: { rootPath: ['ROOT_ID'], excluded: [] },
+        chartsInScope: chartIds,
+      },
+    },
+  };
+};
+
+test('does not copy dashboard metadata into the form store on dashboards with many charts', async () => {
+  // antd's form store deep-merges every value it is given, which is cubic in
+  // the chart count for `chart_configuration`; no form field reads `metadata`.
+  const useFormSpy = jest.spyOn(Form, 'useForm');
+
+  render(
+    <PropertiesModal
+      dashboardId={26}
+      dashboardInfo={buildDashboardInfo(333)}
+      dashboardTitle="Renamed inline"
+      show
+      onlyApply
+      onHide={jest.fn()}
+      onSubmit={jest.fn()}
+      addSuccessToast={jest.fn()}
+    />,
+    { useRedux: true },
+  );
+
+  await waitFor(() =>
+    expect(screen.getByTestId('dashboard-title-input')).toHaveValue(
+      'Renamed inline',
+    ),
+  );
+
+  const [form] = useFormSpy.mock.results[0].value;
+  expect(Object.keys(form.getFieldsValue(true))).not.toContain('metadata');
+  expect(form.getFieldValue('title')).toBe('Renamed inline');
+  useFormSpy.mockRestore();
+});
diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
index a10228ce274..b7939652f6a 100644
--- a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
+++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
@@ -102,7 +102,6 @@ type DashboardInfo = {
   certifiedBy: string;
   certificationDetails: string;
   isManagedExternally: boolean;
-  metadata: Record<string, any>;
   common?: {
     conf?: {
       SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT?: number;
@@ -213,7 +212,13 @@ const PropertiesModal = ({
         certificationDetails: certification_details || '',
         isManagedExternally: is_managed_externally || false,
         css: css || '',
-        metadata,
+        // `metadata` is deliberately left out: this object feeds the form
+        // store, which deep-merges every value it is given, and no form field
+        // reads metadata. On dashboards with many charts it holds a
+        // chart_configuration whose chartsInScope lists grow with the square
+        // of the chart count, which made the merge block the main thread.
+        // The raw metadata is kept in `jsonMetadata` and
+        // `originalDashboardMetadata` instead.
       };
 
       form.setFieldsValue(dashboardInfo);