chore: Improve performance to load the chart properties modal (#34079)

pyproject.toml

@@ -45,7 +45,7 @@ dependencies = [
     "cryptography>=42.0.4, <45.0.0",
     "deprecation>=2.1.0, <2.2.0",
     "flask>=2.2.5, <3.0.0",
-    "flask-appbuilder>=4.7.0, <5.0.0",
+    "flask-appbuilder>=4.8.0, <5.0.0",
     "flask-caching>=2.1.0, <3",
     "flask-compress>=1.13, <2.0",
     "flask-talisman>=1.0.0, <2.0",

requirements/base.txt

@@ -112,7 +112,7 @@ flask==2.3.3
     #   flask-session
     #   flask-sqlalchemy
     #   flask-wtf
-flask-appbuilder==4.7.0
+flask-appbuilder==4.8.0
     # via apache-superset (pyproject.toml)
 flask-babel==2.0.0
     # via flask-appbuilder
@@ -154,6 +154,7 @@ greenlet==3.1.1
     # via
     #   apache-superset (pyproject.toml)
     #   shillelagh
+    #   sqlalchemy
 gunicorn==23.0.0
     # via apache-superset (pyproject.toml)
 h11==0.16.0

requirements/development.txt

@@ -151,7 +151,7 @@ flask==2.3.3
     #   flask-sqlalchemy
     #   flask-testing
     #   flask-wtf
-flask-appbuilder==4.7.0
+flask-appbuilder==4.8.0
     # via
     #   apache-superset (pyproject.toml)
     #   apache-superset
@@ -257,6 +257,7 @@ greenlet==3.1.1
     #   apache-superset
     #   gevent
     #   shillelagh
+    #   sqlalchemy
 grpcio==1.71.0
     # via
     #   apache-superset

requirements/translations.txt

@@ -109,7 +109,7 @@ flask==2.3.3
     #   flask-session
     #   flask-sqlalchemy
     #   flask-wtf
-flask-appbuilder==4.7.0
+flask-appbuilder==4.8.0
     # via apache-superset (pyproject.toml)
 flask-babel==2.0.0
     # via flask-appbuilder
@@ -149,6 +149,7 @@ greenlet==3.1.1
     # via
     #   apache-superset (pyproject.toml)
     #   shillelagh
+    #   sqlalchemy
 gunicorn==23.0.0
     # via apache-superset (pyproject.toml)
 h11==0.16.0

superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx

@@ -17,7 +17,6 @@
  * under the License.
  */
 
-import { VizType } from '@superset-ui/core';
 import {
   render,
   screen,
@@ -25,8 +24,14 @@ import {
   waitFor,
 } from 'spec/helpers/testing-library';
 import fetchMock from 'fetch-mock';
+import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core';
 import PropertiesModal, { PropertiesModalProps } from '.';
 
+jest.mock('@superset-ui/core', () => ({
+  ...jest.requireActual('@superset-ui/core'),
+  isFeatureEnabled: jest.fn(),
+}));
+
 const createProps = () =>
   ({
     slice: {
@@ -44,59 +49,51 @@ const createProps = () =>
     addSuccessToast: jest.fn(),
   }) as PropertiesModalProps;
 
-fetchMock.get('glob:*/api/v1/chart/318', {
+fetchMock.get('glob:*/api/v1/chart/318*', {
   body: {
     description_columns: {},
     id: 318,
     label_columns: {
-      cache_timeout: 'Cache Timeout',
-      'dashboards.dashboard_title': 'Dashboards Dashboard Title',
-      'dashboards.id': 'Dashboards Id',
-      description: 'Description',
       'owners.first_name': 'Owners First Name',
       'owners.id': 'Owners Id',
       'owners.last_name': 'Owners Last Name',
-      'owners.username': 'Owners Username',
-      params: 'Params',
-      slice_name: 'Slice Name',
-      viz_type: 'Viz Type',
+      'tags.id': 'Tags Id',
+      'tags.name': 'Tags Name',
+      'tags.type': 'Tags Type',
     },
     result: {
-      cache_timeout: null,
-      certified_by: 'John Doe',
-      certification_details: 'Sample certification',
-      dashboards: [
-        {
-          dashboard_title: 'FCC New Coder Survey 2018',
-          id: 23,
-        },
-      ],
-      description: null,
       owners: [
         {
           first_name: 'Superset',
           id: 1,
           last_name: 'Admin',
-          username: 'admin',
         },
       ],
-      params:
-        '{"adhoc_filters": [], "all_columns_x": ["age"], "color_scheme": "supersetColors", "datasource": "42__table", "granularity_sqla": "time_start", "groupby": null, "label_colors": {}, "link_length": "25", "queryFields": {"groupby": "groupby"}, "row_limit": 10000, "slice_id": 1380, "time_range": "No filter", "url_params": {}, "viz_type": "histogram", "x_axis_label": "age", "y_axis_label": "count"}',
-      slice_name: 'Age distribution of respondents',
-      viz_type: VizType.Histogram,
+      tags: [
+        {
+          id: 1,
+          name: 'type:chart',
+          type: 2,
+        },
+        {
+          id: 2,
+          name: 'owner:1',
+          type: 3,
+        },
+        {
+          id: 3,
+          name: 'my test tag',
+          type: 1,
+        },
+      ],
     },
     show_columns: [
-      'cache_timeout',
-      'dashboards.dashboard_title',
-      'dashboards.id',
-      'description',
-      'owners.first_name',
       'owners.id',
+      'owners.first_name',
       'owners.last_name',
-      'owners.username',
-      'params',
-      'slice_name',
-      'viz_type',
+      'tags.id',
+      'tags.name',
+      'tags.type',
     ],
     show_title: 'Show Slice',
   },
@@ -422,3 +419,32 @@ test('"Certification details" should not be empty when saved', async () => {
     );
   });
 });
+
+test('Should display only custom tags when tagging system is enabled', async () => {
+  const mockIsFeatureEnabled = isFeatureEnabled as jest.MockedFunction<
+    typeof isFeatureEnabled
+  >;
+  mockIsFeatureEnabled.mockImplementation(
+    flag => flag === FeatureFlag.TaggingSystem,
+  );
+
+  const props = createProps();
+  renderModal(props);
+
+  await waitFor(async () => {
+    expect(
+      await screen.findByRole('heading', { name: 'Tags' }),
+    ).toBeInTheDocument();
+    expect(
+      await screen.findByRole('combobox', { name: 'Tags' }),
+    ).toBeInTheDocument();
+  });
+
+  await waitFor(async () => {
+    expect(await screen.findByText('my test tag')).toBeInTheDocument();
+    expect(screen.queryByText('type:chart')).not.toBeInTheDocument();
+    expect(screen.queryByText('owner:1')).not.toBeInTheDocument();
+  });
+
+  mockIsFeatureEnabled.mockRestore();
+});

superset-frontend/src/explore/components/PropertiesModal/index.tsx

@@ -47,7 +47,6 @@ import Chart, { Slice } from 'src/types/Chart';
 import withToasts from 'src/components/MessageToasts/withToasts';
 import { type TagType } from 'src/components';
 import { loadTags } from 'src/components/Tag/utils';
-import { fetchTags, OBJECT_TYPES } from 'src/features/tags/tags';
 
 export type PropertiesModalProps = {
   slice: Slice;
@@ -101,11 +100,21 @@ function PropertiesModal({
     });
   }
 
-  const fetchChartOwners = useCallback(
-    async function fetchChartOwners() {
+  const fetchChartProperties = useCallback(
+    async function fetchChartProperties() {
+      const queryParams = rison.encode({
+        select_columns: [
+          'owners.id',
+          'owners.first_name',
+          'owners.last_name',
+          'tags.id',
+          'tags.name',
+          'tags.type',
+        ],
+      });
       try {
         const response = await SupersetClient.get({
-          endpoint: `/api/v1/chart/${slice.slice_id}`,
+          endpoint: `/api/v1/chart/${slice.slice_id}?q=${queryParams}`,
         });
         const chart = response.json.result;
         setSelectedOwners(
@@ -114,6 +123,12 @@ function PropertiesModal({
             label: `${owner.first_name} ${owner.last_name}`,
           })),
         );
+        if (isFeatureEnabled(FeatureFlag.TaggingSystem)) {
+          const customTags = chart.tags?.filter(
+            (tag: TagType) => tag.type === 1,
+          );
+          setTags(customTags);
+        }
       } catch (response) {
         const clientError = await getClientErrorObject(response);
         showError(clientError);
@@ -206,33 +221,14 @@ function PropertiesModal({
 
   // get the owners of this slice
   useEffect(() => {
-    fetchChartOwners();
-  }, [fetchChartOwners]);
+    fetchChartProperties();
+  }, [slice.slice_id]);
 
   // update name after it's changed in another modal
   useEffect(() => {
     setName(slice.slice_name || '');
   }, [slice.slice_name]);
 
-  useEffect(() => {
-    if (!isFeatureEnabled(FeatureFlag.TaggingSystem)) return;
-    try {
-      fetchTags(
-        {
-          objectType: OBJECT_TYPES.CHART,
-          objectId: slice.slice_id,
-          includeTypes: false,
-        },
-        (tags: TagType[]) => setTags(tags),
-        error => {
-          showError(error);
-        },
-      );
-    } catch (error) {
-      showError(error);
-    }
-  }, [slice.slice_id]);
-
   const handleChangeTags = (tags: { label: string; value: number }[]) => {
     const parsedTags: TagType[] = ensureIsArray(tags).map(r => ({
       id: r.value,