You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2022/04/04 07:39:03 UTC

[superset] branch master updated: fix(dashboard list): do not show favorite star for anonymous users #18210 (#19409)

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

villebro 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 b8891acf4a fix(dashboard list): do not show favorite star for anonymous users  #18210 (#19409)
b8891acf4a is described below

commit b8891acf4a478da8883bd168272715023d6a3351
Author: dudasaron <68...@users.noreply.github.com>
AuthorDate: Mon Apr 4 09:38:44 2022 +0200

    fix(dashboard list): do not show favorite star for anonymous users  #18210 (#19409)
    
    * fix: Only show favorite star on dashboard list if user is logged in #18210
    
    * Fix linter errors
---
 .../src/components/ListViewCard/index.tsx          |  2 +-
 .../src/views/CRUD/dashboard/DashboardCard.tsx     | 14 ++--
 .../views/CRUD/dashboard/DashboardList.test.jsx    | 68 +++++++++++++++++--
 .../src/views/CRUD/dashboard/DashboardList.tsx     | 79 ++++++++++++++--------
 superset-frontend/src/views/CRUD/utils.tsx         |  2 +-
 5 files changed, 120 insertions(+), 45 deletions(-)

diff --git a/superset-frontend/src/components/ListViewCard/index.tsx b/superset-frontend/src/components/ListViewCard/index.tsx
index 79616c86ac..153b06489b 100644
--- a/superset-frontend/src/components/ListViewCard/index.tsx
+++ b/superset-frontend/src/components/ListViewCard/index.tsx
@@ -26,7 +26,7 @@ import CertifiedBadge from '../CertifiedBadge';
 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
index 25a012f86b..2da9f45515 100644
--- a/superset-frontend/src/views/CRUD/dashboard/DashboardCard.tsx
+++ b/superset-frontend/src/views/CRUD/dashboard/DashboardCard.tsx
@@ -44,7 +44,7 @@ interface DashboardCardProps {
   saveFavoriteStatus: (id: number, isStarred: boolean) => void;
   favoriteStatus: boolean;
   dashboardFilter?: string;
-  userId?: number;
+  userId?: string | number;
   showThumbnails?: boolean;
   handleBulkDashboardExport: (dashboardsToExport: Dashboard[]) => void;
 }
@@ -171,11 +171,13 @@ function DashboardCard({
               e.preventDefault();
             }}
           >
-            <FaveStar
-              itemId={dashboard.id}
-              saveFaveStar={saveFavoriteStatus}
-              isStarred={favoriteStatus}
-            />
+            {userId && (
+              <FaveStar
+                itemId={dashboard.id}
+                saveFaveStar={saveFavoriteStatus}
+                isStarred={favoriteStatus}
+              />
+            )}
             <AntdDropdown overlay={menu}>
               <Icons.MoreVert iconColor={theme.colors.grayscale.base} />
             </AntdDropdown>
diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.test.jsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.test.jsx
index 3561cafdbc..e42ba92ff2 100644
--- 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
index b3da8ee8e3..a00ceefbc8 100644
--- 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';
@@ -95,7 +95,11 @@ const Actions = styled.div`
 `;
 
 function DashboardList(props: DashboardListProps) {
-  const { addDangerToast, addSuccessToast } = props;
+  const {
+    addDangerToast,
+    addSuccessToast,
+    user: { userId },
+  } = props;
 
   const {
     state: {
@@ -143,7 +147,6 @@ function DashboardList(props: DashboardListProps) {
     addSuccessToast(t('Dashboard imported'));
   };
 
-  const { userId } = props.user;
   // TODO: Fix usage of localStorage keying on the user id
   const userKey = dangerouslyGetItemDoNotUse(userId?.toString(), null);
 
@@ -232,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: {
@@ -422,10 +423,15 @@ function DashboardList(props: DashboardListProps) {
       },
     ],
     [
+      userId,
       canEdit,
       canDelete,
       canExport,
-      ...(props.user.userId ? [favoriteStatus] : []),
+      saveFavoriteStatus,
+      favoriteStatus,
+      refreshData,
+      addSuccessToast,
+      addDangerToast,
     ],
   );
 
@@ -500,7 +506,7 @@ function DashboardList(props: DashboardListProps) {
           { label: t('Draft'), value: false },
         ],
       },
-      ...(props.user.userId ? [favoritesFilter] : []),
+      ...(userId ? [favoritesFilter] : []),
       {
         Header: t('Certified'),
         id: 'id',
@@ -544,8 +550,8 @@ function DashboardList(props: DashboardListProps) {
     },
   ];
 
-  function renderCard(dashboard: Dashboard) {
-    return (
+  const renderCard = useCallback(
+    (dashboard: Dashboard) => (
       <DashboardCard
         dashboard={dashboard}
         hasPerm={hasPerm}
@@ -556,6 +562,7 @@ function DashboardList(props: DashboardListProps) {
             ? userKey.thumbnails
             : isFeatureEnabled(FeatureFlag.THUMBNAILS)
         }
+        userId={userId}
         loading={loading}
         addDangerToast={addDangerToast}
         addSuccessToast={addSuccessToast}
@@ -564,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
index 3449d764ab..7f069a3e7c 100644
--- a/superset-frontend/src/views/CRUD/utils.tsx
+++ b/superset-frontend/src/views/CRUD/utils.tsx
@@ -285,7 +285,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}`,