fix(dashboard list): do not show favorite star for anonymous users ap…

…ache#18210 (apache#19409) * fix: Only show favorite star on dashboard list if user is logged in apache#18210 * Fix linter errors
Powerarena-Limited · Dec 27, 2022 · ee9147a · ee9147a
1 parent 20c6501
commit ee9147a
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 45 deletions.
diff --git a/superset-frontend/src/components/ListViewCard/index.tsx b/superset-frontend/src/components/ListViewCard/index.tsx
@@ -26,7 +26,7 @@ import CertifiedIcon from '../CertifiedIcon';
 const ActionsWrapper = styled.div`
   width: 64px;
   display: flex;
-  justify-content: space-between;
+  justify-content: flex-end;
 `;
 
 const StyledCard = styled(AntdCard)`

diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardCard.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardCard.tsx
@@ -43,7 +43,7 @@ interface DashboardCardProps {
   saveFavoriteStatus: (id: number, isStarred: boolean) => void;
   favoriteStatus: boolean;
   dashboardFilter?: string;
-  userId?: number;
+  userId?: string | number;
   showThumbnails?: boolean;
   handleBulkDashboardExport: (dashboardsToExport: Dashboard[]) => void;
 }
@@ -170,11 +170,13 @@ function DashboardCard({
               e.preventDefault();
             }}
           >
-            <FaveStar
-              itemId={dashboard.id}
-              saveFaveStar={saveFavoriteStatus}
-              isStarred={favoriteStatus}
-            />
+            {userId && (
+              <FaveStar
+                itemId={dashboard.id}
+                saveFaveStar={saveFavoriteStatus}
+                isStarred={favoriteStatus}
+              />
+            )}
             <Dropdown overlay={menu}>
               <Icons.MoreVert iconColor={theme.colors.grayscale.base} />
             </Dropdown>

diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.test.jsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.test.jsx
@@ -36,6 +36,9 @@ import DashboardList from 'src/views/CRUD/dashboard/DashboardList';
 import ListView from 'src/components/ListView';
 import ListViewCard from 'src/components/ListViewCard';
 import PropertiesModal from 'src/dashboard/components/PropertiesModal';
+import FaveStar from 'src/components/FaveStar';
+// import TableCollection from 'src/components/TableCollection';
+import CardCollection from 'src/components/ListView/CardCollection';
 
 // store needed for withToasts(DashboardTable)
 const mockStore = configureStore([thunk]);
@@ -104,15 +107,18 @@ describe('DashboardList', () => {
   });
 
   const mockedProps = {};
-  const wrapper = mount(
-    <MemoryRouter>
-      <Provider store={store}>
-        <DashboardList {...mockedProps} user={mockUser} />
-      </Provider>
-    </MemoryRouter>,
-  );
+  let wrapper;
 
   beforeAll(async () => {
+    fetchMock.resetHistory();
+    wrapper = mount(
+      <MemoryRouter>
+        <Provider store={store}>
+          <DashboardList {...mockedProps} user={mockUser} />
+        </Provider>
+      </MemoryRouter>,
+    );
+
     await waitForComponentToPaint(wrapper);
   });
 
@@ -178,6 +184,18 @@ describe('DashboardList', () => {
     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', () => {
@@ -222,3 +240,39 @@ describe('RTL', () => {
     expect(importTooltip).toBeInTheDocument();
   });
 });
+
+describe('DashboardList - anonymous view', () => {
+  const mockedProps = {};
+  const mockUserLoggedOut = {};
+  let wrapper;
+
+  beforeAll(async () => {
+    fetchMock.resetHistory();
+    wrapper = mount(
+      <MemoryRouter>
+        <Provider store={store}>
+          <DashboardList {...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();
+  });
+});
diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 import { styled, SupersetClient, t } from '@superset-ui/core';
-import React, { useState, useMemo } from 'react';
+import React, { useState, useMemo, useCallback } from 'react';
 import { Link } from 'react-router-dom';
 import rison from 'rison';
 import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
@@ -96,7 +96,11 @@ const Actions = styled.div`
 `;
 
 function DashboardList(props: DashboardListProps) {
-  const { addDangerToast, addSuccessToast } = props;
+  const {
+    addDangerToast,
+    addSuccessToast,
+    user: { userId },
+  } = props;
 
   const {
     state: {
@@ -143,7 +147,7 @@ function DashboardList(props: DashboardListProps) {
     refreshData();
   };
 
-  const { userId } = props.user;
+  // TODO: Fix usage of localStorage keying on the user id
   const userKey = getFromLocalStorage(userId?.toString(), null);
 
   const canCreate = hasPerm('can_write');
@@ -231,27 +235,25 @@ function DashboardList(props: DashboardListProps) {
 
   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: 'xs',
-            },
-          ]
-        : []),
+      {
+        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: {
@@ -421,10 +423,15 @@ function DashboardList(props: DashboardListProps) {
       },
     ],
     [
+      userId,
       canEdit,
       canDelete,
       canExport,
-      ...(props.user.userId ? [favoriteStatus] : []),
+      saveFavoriteStatus,
+      favoriteStatus,
+      refreshData,
+      addSuccessToast,
+      addDangerToast,
     ],
   );
 
@@ -499,7 +506,7 @@ function DashboardList(props: DashboardListProps) {
           { label: t('Draft'), value: false },
         ],
       },
-      ...(props.user.userId ? [favoritesFilter] : []),
+      ...(userId ? [favoritesFilter] : []),
       {
         Header: t('Certified'),
         id: 'id',
@@ -543,8 +550,8 @@ function DashboardList(props: DashboardListProps) {
     },
   ];
 
-  function renderCard(dashboard: Dashboard) {
-    return (
+  const renderCard = useCallback(
+    (dashboard: Dashboard) => (
       <DashboardCard
         dashboard={dashboard}
         hasPerm={hasPerm}
@@ -555,6 +562,7 @@ function DashboardList(props: DashboardListProps) {
             ? userKey.thumbnails
             : isFeatureEnabled(FeatureFlag.THUMBNAILS)
         }
+        userId={userId}
         loading={loading}
         addDangerToast={addDangerToast}
         addSuccessToast={addSuccessToast}
@@ -563,8 +571,20 @@ function DashboardList(props: DashboardListProps) {
         favoriteStatus={favoriteStatus[dashboard.id]}
         handleBulkDashboardExport={handleBulkDashboardExport}
       />
-    );
-  }
+    ),
+    [
+      addDangerToast,
+      addSuccessToast,
+      bulkSelectEnabled,
+      favoriteStatus,
+      hasPerm,
+      loading,
+      userId,
+      refreshData,
+      saveFavoriteStatus,
+      userKey,
+    ],
+  );
 
   const subMenuButtons: SubMenuProps['buttons'] = [];
   if (canDelete || canExport) {

diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx
@@ -260,7 +260,7 @@ export function handleDashboardDelete(
   addSuccessToast: (arg0: string) => void,
   addDangerToast: (arg0: string) => void,
   dashboardFilter?: string,
-  userId?: number,
+  userId?: string | number,
 ) {
   return SupersetClient.delete({
     endpoint: `/api/v1/dashboard/${id}`,