fix(charts list): do not trigger ListViewError exception for anonymous user

superset-frontend/src/views/CRUD/chart/ChartCard.tsx

@@ -43,7 +43,7 @@ interface ChartCardProps {
  saveFavoriteStatus: (id: number, isStarred: boolean) => void;
  favoriteStatus: boolean;
  chartFilter?: string;
- userId?: number;
+ userId?: string | number;
  showThumbnails?: boolean;
  handleBulkChartExport: (chartsToExport: Chart[]) => void;
 }
@@ -165,11 +165,13 @@ export default function ChartCard({
  e.preventDefault();
  }}
  >
- <FaveStar
- itemId={chart.id}
- saveFaveStar={saveFavoriteStatus}
- isStarred={favoriteStatus}
- />
+ {userId && (
+ <FaveStar
+ itemId={chart.id}
+ saveFaveStar={saveFavoriteStatus}
+ isStarred={favoriteStatus}
+ />
+ )}
  <AntdDropdown overlay={menu}>
  <Icons.MoreVert iconColor={theme.colors.grayscale.base} />
  </AntdDropdown>

superset-frontend/src/views/CRUD/chart/ChartList.test.jsx

@@ -17,6 +17,7 @@
  * under the License.
  */
 import React from 'react';
+import { MemoryRouter } from 'react-router-dom';
 import thunk from 'redux-thunk';
 import configureStore from 'redux-mock-store';
 import { Provider } from 'react-redux';
@@ -34,6 +35,9 @@ import ConfirmStatusChange from 'src/components/ConfirmStatusChange';
 import ListView from 'src/components/ListView';
 import PropertiesModal from 'src/explore/components/PropertiesModal';
 import ListViewCard from 'src/components/ListViewCard';
+import FaveStar from 'src/components/FaveStar';
+import TableCollection from 'src/components/TableCollection';
+import CardCollection from 'src/components/ListView/CardCollection';
 // store needed for withToasts(ChartTable)
 const mockStore = configureStore([thunk]);
 const store = mockStore({});
@@ -105,13 +109,17 @@ describe('ChartList', () => {
  });
  const mockedProps = {};
 
- const wrapper = mount(
- <Provider store={store}>
- <ChartList {...mockedProps} user={mockUser} />
- </Provider>,
- );
+ let wrapper;
 
  beforeAll(async () => {
+ wrapper = mount(
+ <MemoryRouter>
+ <Provider store={store}>
+ <ChartList {...mockedProps} user={mockUser} />
+ </Provider>
+ </MemoryRouter>,
+ );
+
  await waitForComponentToPaint(wrapper);
  });
 
@@ -159,6 +167,18 @@ describe('ChartList', () => {
  await waitForComponentToPaint(wrapper);
  expect(wrapper.find(ConfirmStatusChange)).toExist();
  });
+
+ it('renders the Favorite Star column in list view for logged in user', async () => {
+ wrapper.find('[aria-label="list-view"]').first().simulate('click');
+ await waitForComponentToPaint(wrapper);
+ expect(wrapper.find(TableCollection).find(FaveStar)).toExist();
+ });
+
+ it('renders the Favorite Star in card view for logged in user', async () => {
+ wrapper.find('[aria-label="card-view"]').first().simulate('click');
+ await waitForComponentToPaint(wrapper);
+ expect(wrapper.find(CardCollection).find(FaveStar)).toExist();
+ });
 });
 
 describe('RTL', () => {
@@ -201,3 +221,39 @@ describe('RTL', () => {
  expect(importTooltip).toBeInTheDocument();
  });
 });
+
+describe('ChartList - anonymous view', () => {
+ const mockedProps = {};
+ const mockUserLoggedOut = {};
+ let wrapper;
+
+ beforeAll(async () => {
+ fetchMock.resetHistory();
+ wrapper = mount(
+ <MemoryRouter>
+ <Provider store={store}>
+ <ChartList {...mockedProps} user={mockUserLoggedOut} />
+ </Provider>
+ </MemoryRouter>,
+ );
+
+ await waitForComponentToPaint(wrapper);
+ });
+
+ afterAll(() => {
+ cleanup();
+ fetch.resetMocks();
+ });
+
+ it('does not render the Favorite Star column in list view for anonymous user', async () => {
+ wrapper.find('[aria-label="list-view"]').first().simulate('click');
+ await waitForComponentToPaint(wrapper);
+ expect(wrapper.find(TableCollection).find(FaveStar)).not.toExist();
+ });
+
+ it('does not render the Favorite Star in card view for anonymous user', async () => {
+ wrapper.find('[aria-label="card-view"]').first().simulate('click');
+ await waitForComponentToPaint(wrapper);
+ expect(wrapper.find(CardCollection).find(FaveStar)).not.toExist();
+ });
+});

superset-frontend/src/views/CRUD/chart/ChartList.tsx

@@ -22,7 +22,7 @@ import {
  SupersetClient,
  t,
 } from '@superset-ui/core';
-import React, { useMemo, useState } from 'react';
+import React, { useState, useMemo, useCallback } from 'react';
 import rison from 'rison';
 import { uniqBy } from 'lodash';
 import moment from 'moment';
@@ -146,7 +146,11 @@ const Actions = styled.div`
 `;
 
 function ChartList(props: ChartListProps) {
- const { addDangerToast, addSuccessToast } = props;
+ const {
+ addDangerToast,
+ addSuccessToast,
+ user: { userId },
+ } = props;
 
  const {
  state: {
@@ -180,7 +184,6 @@ function ChartList(props: ChartListProps) {
  const [passwordFields, setPasswordFields] = useState<string[]>([]);
  const [preparingExport, setPreparingExport] = useState<boolean>(false);
 
- const { userId } = props.user;
  // TODO: Fix usage of localStorage keying on the user id
  const userSettings = dangerouslyGetItemDoNotUse(userId?.toString(), null) as {
  thumbnails: boolean;
@@ -235,27 +238,25 @@ function ChartList(props: ChartListProps) {
 
  const columns = useMemo(
  () => [
- ...(props.user.userId
- ? [
- {
- Cell: ({
- row: {
- original: { id },
- },
- }: any) => (
- <FaveStar
- itemId={id}
- saveFaveStar={saveFavoriteStatus}
- isStarred={favoriteStatus[id]}
- />
- ),
- Header: '',
- id: 'id',
- disableSortBy: true,
- size: 'sm',
- },
- ]
- : []),
+ {
+ Cell: ({
+ row: {
+ original: { id },
+ },
+ }: any) =>
+ userId && (
+ <FaveStar
+ itemId={id}
+ saveFaveStar={saveFavoriteStatus}
+ isStarred={favoriteStatus[id]}
+ />
+ ),
+ Header: '',
+ id: 'id',
+ disableSortBy: true,
+ size: 'xs',
+ hidden: !userId,
+ },
  {
  Cell: ({
  row: {
@@ -451,10 +452,15 @@ function ChartList(props: ChartListProps) {
  },
  ],
  [
+ userId,
  canEdit,
  canDelete,
  canExport,
- ...(props.user.userId ? [favoriteStatus] : []),
+ saveFavoriteStatus,
+ favoriteStatus,
+ refreshData,
+ addSuccessToast,
+ addDangerToast,
  ],
  );
 
@@ -552,7 +558,7 @@ function ChartList(props: ChartListProps) {
  fetchSelects: createFetchDatasets,
  paginate: true,
  },
- ...(props.user.userId ? [favoritesFilter] : []),
+ ...(userId ? [favoritesFilter] : []),
  {
  Header: t('Certified'),
  id: 'id',
@@ -596,8 +602,8 @@ function ChartList(props: ChartListProps) {
  },
  ];
 
- function renderCard(chart: Chart) {
- return (
+ const renderCard = useCallback(
+ (chart: Chart) => (
  <ChartCard
  chart={chart}
  showThumbnails={
@@ -611,13 +617,23 @@ function ChartList(props: ChartListProps) {
  addDangerToast={addDangerToast}
  addSuccessToast={addSuccessToast}
  refreshData={refreshData}
+ userId={userId}
  loading={loading}
  favoriteStatus={favoriteStatus[chart.id]}
  saveFavoriteStatus={saveFavoriteStatus}
  handleBulkChartExport={handleBulkChartExport}
  />
- );
- }
+ ),
+ [
+ addDangerToast,
+ addSuccessToast,
+ bulkSelectEnabled,
+ favoriteStatus,
+ hasPerm,
+ loading,
+ ],
+ );
+
  const subMenuButtons: SubMenuProps['buttons'] = [];
  if (canDelete || canExport) {
  subMenuButtons.push({

superset-frontend/src/views/CRUD/utils.tsx

@@ -246,7 +246,7 @@ export function handleChartDelete(
  addDangerToast: (arg0: string) => void,
  refreshData: (arg0?: FetchDataConfig | null) => void,
  chartFilter?: string,
- userId?: number,
+ userId?: string | number,
 ) {
  const filters = {
  pageIndex: 0,