fix: dashboard performance (#28609)

Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>
Co-authored-by: Joe Li <joe@preset.io>

pyproject.toml

@@ -45,7 +45,7 @@ dependencies = [
     "cryptography>=42.0.4, <43.0.0",
     "deprecation>=2.1.0, <2.2.0",
     "flask>=2.2.5, <3.0.0",
-    "flask-appbuilder>=4.4.1, <5.0.0",
+    "flask-appbuilder>=4.5.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

@@ -15,8 +15,6 @@ apispec[yaml]==6.3.0
     # via flask-appbuilder
 apsw==3.46.0.0
     # via shillelagh
-async-timeout==4.0.3
-    # via redis
 attrs==23.2.0
     # via
     #   cattrs
@@ -93,8 +91,6 @@ dnspython==2.6.1
     # via email-validator
 email-validator==2.1.1
     # via flask-appbuilder
-exceptiongroup==1.2.1
-    # via cattrs
 flask==2.3.3
     # via
     #   apache-superset
@@ -109,7 +105,7 @@ flask==2.3.3
     #   flask-session
     #   flask-sqlalchemy
     #   flask-wtf
-flask-appbuilder==4.4.1
+flask-appbuilder==4.5.0
     # via apache-superset
 flask-babel==2.0.0
     # via flask-appbuilder
@@ -360,7 +356,6 @@ typing-extensions==4.12.0
     # via
     #   alembic
     #   apache-superset
-    #   cattrs
     #   flask-limiter
     #   limits
     #   shillelagh

requirements/development.txt

@@ -10,6 +10,8 @@
     # via
     #   -r requirements/base.in
     #   -r requirements/development.in
+appnope==0.1.4
+    # via ipython
 astroid==3.1.0
     # via pylint
 boto3==1.34.112
@@ -175,7 +177,9 @@ protobuf==4.23.0
 psycopg2-binary==2.9.6
     # via apache-superset
 pure-sasl==0.6.2
-    # via thrift-sasl
+    # via
+    #   pyhive
+    #   thrift-sasl
 pydata-google-auth==1.7.0
     # via pandas-gbq
 pydruid==0.6.9
@@ -184,7 +188,7 @@ pyee==11.0.1
     # via playwright
 pyfakefs==5.3.5
     # via apache-superset
-pyhive[presto]==0.7.0
+pyhive[hive_pure_sasl]==0.7.0
     # via apache-superset
 pyinstrument==4.4.0
     # via apache-superset
@@ -228,10 +232,9 @@ tableschema==1.20.10
 thrift==0.16.0
     # via
     #   apache-superset
+    #   pyhive
     #   thrift-sasl
 thrift-sasl==0.4.3
-    # via apache-superset
-tomli==2.0.1
     # via
     #   build
     #   coverage

superset-frontend/src/features/dashboards/DashboardCard.test.tsx

@@ -0,0 +1,158 @@
+/**
+ * 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 React from 'react';
+import { MemoryRouter } from 'react-router-dom';
+import { FeatureFlag, SupersetClient } from '@superset-ui/core';
+import * as uiCore from '@superset-ui/core';
+
+import { render, screen, waitFor } from 'spec/helpers/testing-library';
+
+import DashboardCard from './DashboardCard';
+
+const mockDashboard = {
+  id: 1,
+  dashboard_title: 'Sample Dashboard',
+  certified_by: 'John Doe',
+  certification_details: 'Certified on 2022-01-01',
+  published: true,
+  url: '/dashboard/1',
+  thumbnail_url: '/thumbnails/1.png',
+  changed_on_delta_humanized: '2 days ago',
+  owners: [
+    { id: 1, name: 'Alice', first_name: 'Alice', last_name: 'Doe' },
+    { id: 2, name: 'Bob', first_name: 'Bob', last_name: 'Smith' },
+  ],
+  changed_by_name: 'John Doe',
+  changed_by: 'john.doe@example.com',
+};
+
+const mockHasPerm = jest.fn().mockReturnValue(true);
+const mockOpenDashboardEditModal = jest.fn();
+const mockSaveFavoriteStatus = jest.fn();
+const mockHandleBulkDashboardExport = jest.fn();
+const mockOnDelete = jest.fn();
+
+let isFeatureEnabledMock: jest.MockInstance<boolean, [feature: FeatureFlag]>;
+
+beforeAll(() => {
+  isFeatureEnabledMock = jest
+    .spyOn(uiCore, 'isFeatureEnabled')
+    .mockImplementation(() => true);
+});
+
+afterAll(() => {
+  // @ts-ignore
+  isFeatureEnabledMock.mockClear();
+});
+
+beforeEach(() => {
+  render(
+    <MemoryRouter>
+      <DashboardCard
+        dashboard={mockDashboard}
+        hasPerm={mockHasPerm}
+        bulkSelectEnabled={false}
+        loading={false}
+        openDashboardEditModal={mockOpenDashboardEditModal}
+        saveFavoriteStatus={mockSaveFavoriteStatus}
+        favoriteStatus={false}
+        handleBulkDashboardExport={mockHandleBulkDashboardExport}
+        onDelete={mockOnDelete}
+      />
+    </MemoryRouter>,
+  );
+});
+
+it('Renders the dashboard title', () => {
+  const titleElement = screen.getByText('Sample Dashboard');
+  expect(titleElement).toBeInTheDocument();
+});
+
+it('Renders the certification details', () => {
+  const certificationDetailsElement = screen.getByLabelText(/certified/i);
+  expect(certificationDetailsElement).toBeInTheDocument();
+});
+
+it('Renders the published status', () => {
+  const publishedElement = screen.getByText(/published/i);
+  expect(publishedElement).toBeInTheDocument();
+});
+
+it('Renders the modified date', () => {
+  const modifiedDateElement = screen.getByText('Modified 2 days ago');
+  expect(modifiedDateElement).toBeInTheDocument();
+});
+
+it('should fetch thumbnail when dashboard has no thumbnail URL and feature flag is enabled', async () => {
+  const mockGet = jest.spyOn(SupersetClient, 'get').mockResolvedValue({
+    response: new Response(
+      JSON.stringify({ thumbnail_url: '/new-thumbnail.png' }),
+    ),
+    json: () => Promise.resolve({ thumbnail_url: '/new-thumbnail.png' }),
+  });
+  const { rerender } = render(
+    <DashboardCard
+      dashboard={{
+        id: 1,
+        thumbnail_url: '',
+        changed_by_name: '',
+        changed_by: '',
+        dashboard_title: '',
+        published: false,
+        url: '',
+        owners: [],
+      }}
+      hasPerm={() => true}
+      bulkSelectEnabled={false}
+      loading={false}
+      saveFavoriteStatus={() => {}}
+      favoriteStatus={false}
+      handleBulkDashboardExport={() => {}}
+      onDelete={() => {}}
+    />,
+  );
+  await waitFor(() => {
+    expect(mockGet).toHaveBeenCalledWith({
+      endpoint: '/api/v1/dashboard/1',
+    });
+  });
+  rerender(
+    <DashboardCard
+      dashboard={{
+        id: 1,
+        thumbnail_url: '/new-thumbnail.png',
+        changed_by_name: '',
+        changed_by: '',
+        dashboard_title: '',
+        published: false,
+        url: '',
+        owners: [],
+      }}
+      hasPerm={() => true}
+      bulkSelectEnabled={false}
+      loading={false}
+      saveFavoriteStatus={() => {}}
+      favoriteStatus={false}
+      handleBulkDashboardExport={() => {}}
+      onDelete={() => {}}
+    />,
+  );
+  mockGet.mockRestore();
+});

superset-frontend/src/features/dashboards/DashboardCard.tsx

@@ -16,9 +16,15 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React from 'react';
+import React, { useEffect, useState } from 'react';
 import { Link, useHistory } from 'react-router-dom';
-import { isFeatureEnabled, FeatureFlag, t, useTheme } from '@superset-ui/core';
+import {
+  isFeatureEnabled,
+  FeatureFlag,
+  t,
+  useTheme,
+  SupersetClient,
+} from '@superset-ui/core';
 import { CardStyles } from 'src/views/CRUD/utils';
 import { AntdDropdown } from 'src/components';
 import { Menu } from 'src/components/Menu';
@@ -62,6 +68,35 @@ function DashboardCard({
   const canExport = hasPerm('can_export');
 
   const theme = useTheme();
+
+  const [thumbnailUrl, setThumbnailUrl] = useState<string | null>(null);
+  const [fetchingThumbnail, setFetchingThumbnail] = useState<boolean>(false);
+
+  useEffect(() => {
+    // fetch thumbnail only if it's not already fetched
+    if (
+      !fetchingThumbnail &&
+      dashboard.id &&
+      (thumbnailUrl === undefined || thumbnailUrl === null) &&
+      isFeatureEnabled(FeatureFlag.Thumbnails)
+    ) {
+      // fetch thumbnail
+      if (dashboard.thumbnail_url) {
+        // set to empty string if null so that we don't
+        // keep fetching the thumbnail
+        setThumbnailUrl(dashboard.thumbnail_url || '');
+        return;
+      }
+      setFetchingThumbnail(true);
+      SupersetClient.get({
+        endpoint: `/api/v1/dashboard/${dashboard.id}`,
+      }).then(({ json = {} }) => {
+        setThumbnailUrl(json.thumbnail_url || '');
+        setFetchingThumbnail(false);
+      });
+    }
+  }, [dashboard, thumbnailUrl]);
+
   const menu = (
     <Menu>
       {canEdit && openDashboardEditModal && (

superset-frontend/src/pages/DashboardList/DashboardList.test.jsx

@@ -162,7 +162,7 @@ describe('DashboardList', () => {
     const callsD = fetchMock.calls(/dashboard\/\?q/);
     expect(callsD).toHaveLength(1);
     expect(callsD[0][0]).toMatchInlineSnapshot(
-      `"http://localhost/api/v1/dashboard/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25)"`,
+      `"http://localhost/api/v1/dashboard/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25,select_columns:!(id,dashboard_title,published,url,slug,changed_by,changed_on_delta_humanized,owners.id,owners.first_name,owners.last_name,owners,tags.id,tags.name,tags.type,status,certified_by,certification_details,changed_on))"`,
     );
   });
 

superset-frontend/src/pages/DashboardList/index.tsx

@@ -111,6 +111,27 @@ const Actions = styled.div`
   color: ${({ theme }) => theme.colors.grayscale.base};
 `;
 
+const DASHBOARD_COLUMNS_TO_FETCH = [
+  'id',
+  'dashboard_title',
+  'published',
+  'url',
+  'slug',
+  'changed_by',
+  'changed_on_delta_humanized',
+  'owners.id',
+  'owners.first_name',
+  'owners.last_name',
+  'owners',
+  'tags.id',
+  'tags.name',
+  'tags.type',
+  'status',
+  'certified_by',
+  'certification_details',
+  'changed_on',
+];
+
 function DashboardList(props: DashboardListProps) {
   const { addDangerToast, addSuccessToast, user } = props;
 
@@ -135,6 +156,11 @@ function DashboardList(props: DashboardListProps) {
     'dashboard',
     t('dashboard'),
     addDangerToast,
+    undefined,
+    undefined,
+    undefined,
+    undefined,
+    DASHBOARD_COLUMNS_TO_FETCH,
   );
   const dashboardIds = useMemo(() => dashboards.map(d => d.id), [dashboards]);
   const [saveFavoriteStatus, favoriteStatus] = useFavoriteStatus(

superset-frontend/src/views/CRUD/hooks.test.tsx

@@ -0,0 +1,105 @@
+/**
+ * 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 { renderHook } from '@testing-library/react-hooks';
+import { JsonResponse, SupersetClient } from '@superset-ui/core';
+
+import { useListViewResource } from './hooks';
+
+describe('useListViewResource', () => {
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('should fetch data with correct query parameters', async () => {
+    const pageIndex = 0; // Declare and initialize the pageIndex variable
+    const pageSize = 10; // Declare and initialize the pageSize variable
+    const baseFilters = [{ id: 'status', operator: 'equals', value: 'active' }];
+
+    const fetchSpy = jest.spyOn(SupersetClient, 'get').mockResolvedValue({
+      json: {
+        result: {
+          dashboard_title: 'New Title',
+          slug: '/new',
+          json_metadata: '{"something":"foo"}',
+          owners: [{ id: 1, first_name: 'Al', last_name: 'Pacino' }],
+          roles: [],
+        },
+      },
+    } as unknown as JsonResponse);
+
+    const { result } = renderHook(() =>
+      useListViewResource('example', 'Example', jest.fn()),
+    );
+    result.current.fetchData({
+      pageIndex,
+      pageSize,
+      sortBy: [{ id: 'foo' }], // Change the type of sortBy from string to SortColumn[]
+      filters: baseFilters,
+    });
+
+    expect(fetchSpy).toHaveBeenNthCalledWith(2, {
+      endpoint:
+        '/api/v1/example/?q=(filters:!((col:status,opr:equals,value:active)),order_column:foo,order_direction:asc,page:0,page_size:10)',
+    });
+  });
+
+  it('should pass the selectColumns to the fetch call', async () => {
+    const pageIndex = 0; // Declare and initialize the pageIndex variable
+    const pageSize = 10; // Declare and initialize the pageSize variable
+    const baseFilters = [{ id: 'status', operator: 'equals', value: 'active' }];
+    const selectColumns = ['id', 'name'];
+
+    const fetchSpy = jest.spyOn(SupersetClient, 'get').mockResolvedValue({
+      json: {
+        result: {
+          dashboard_title: 'New Title',
+          slug: '/new',
+          json_metadata: '{"something":"foo"}',
+          owners: [{ id: 1, first_name: 'Al', last_name: 'Pacino' }],
+          roles: [],
+        },
+      },
+    } as unknown as JsonResponse);
+
+    const { result } = renderHook(() =>
+      useListViewResource(
+        'example',
+        'Example',
+        jest.fn(),
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        selectColumns,
+      ),
+    );
+
+    result.current.fetchData({
+      pageIndex,
+      pageSize,
+      sortBy: [{ id: 'foo' }], // Change the type of sortBy from string to SortColumn[]
+      filters: baseFilters,
+    });
+
+    expect(fetchSpy).toHaveBeenNthCalledWith(2, {
+      endpoint:
+        '/api/v1/example/?q=(filters:!((col:status,opr:equals,value:active)),order_column:foo,order_direction:asc,page:0,page_size:10,select_columns:!(id,name))',
+    });
+  });
+});

superset-frontend/src/views/CRUD/hooks.ts

@@ -77,6 +77,7 @@ export function useListViewResource<D extends object = any>(
   defaultCollectionValue: D[] = [],
   baseFilters?: FilterValue[], // must be memoized
   initialLoadingState = true,
+  selectColumns?: string[],
 ) {
   const [state, setState] = useState<ListViewResourceState<D>>({
     count: 0,
@@ -162,6 +163,7 @@ export function useListViewResource<D extends object = any>(
         page: pageIndex,
         page_size: pageSize,
         ...(filterExps.length ? { filters: filterExps } : {}),
+        ...(selectColumns?.length ? { select_columns: selectColumns } : {}),
       });
 
       return SupersetClient.get({