fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user * add test + fix types * fix lint errors
apache · Jul 9, 2021 · 7ec6bdf · 7ec6bdf
1 parent 2ebba51
commit 7ec6bdf
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 5 deletions.
diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx
@@ -32,12 +32,13 @@ const createProps = () => ({
     dash_edit_perm: false,
     dash_save_perm: false,
     dash_share_perm: false,
-    userId: 1,
+    userId: '1',
     metadata: {},
     common: {
       conf: {},
     },
   },
+  userId: 1,
   dashboardTitle: 'Dashboard Title',
   charts: {},
   layout: {},
@@ -248,6 +249,22 @@ test('should render the selected fave icon', () => {
   ).toBeInTheDocument();
 });
 
+test('should NOT render the fave icon on anonymous user', () => {
+  const mockedProps = createProps();
+  const anonymousUserProps = {
+    ...mockedProps,
+    userId: undefined,
+  };
+  render(setup(anonymousUserProps));
+  expect(mockedProps.fetchFaveStar).not.toHaveBeenCalled();
+  expect(() =>
+    screen.getByRole('img', { name: 'favorite-unselected' }),
+  ).toThrowError('Unable to find');
+  expect(() =>
+    screen.getByRole('img', { name: 'favorite-selected' }),
+  ).toThrowError('Unable to find');
+});
+
 test('should fave', async () => {
   const mockedProps = createProps();
   render(setup(mockedProps));

diff --git a/...tend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx b/...tend/src/dashboard/components/Header/HeaderActionsDropdown/HeaderActionsDropdown.test.tsx
@@ -32,7 +32,7 @@ const createProps = () => ({
     id: 1,
     dash_edit_perm: true,
     dash_save_perm: true,
-    userId: 1,
+    userId: '1',
     metadata: {},
     common: {
       conf: {},

diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx
@@ -51,6 +51,7 @@ const propTypes = {
   addSuccessToast: PropTypes.func.isRequired,
   addDangerToast: PropTypes.func.isRequired,
   addWarningToast: PropTypes.func.isRequired,
+  userId: PropTypes.number,
   dashboardInfo: PropTypes.object.isRequired,
   dashboardTitle: PropTypes.string.isRequired,
   dataMask: PropTypes.object.isRequired,
@@ -366,6 +367,7 @@ class Header extends React.PureComponent {
       updateCss,
       editMode,
       isPublished,
+      userId,
       dashboardInfo,
       hasUnsavedChanges,
       isLoading,
@@ -404,7 +406,7 @@ class Header extends React.PureComponent {
             canEdit={userCanEdit}
             canSave={userCanSaveAs}
           />
-          {dashboardInfo.userId && (
+          {userId && (
             <FaveStar
               itemId={dashboardInfo.id}
               fetchFaveStar={this.props.fetchFaveStar}

diff --git a/superset-frontend/src/dashboard/components/Header/types.ts b/superset-frontend/src/dashboard/components/Header/types.ts
@@ -22,7 +22,7 @@ import { ChartState } from 'src/explore/types';
 
 interface DashboardInfo {
   id: number;
-  userId: number;
+  userId: string | undefined;
   dash_edit_perm: boolean;
   dash_save_perm: boolean;
   metadata?: Record<string, any>;
@@ -65,6 +65,7 @@ export interface HeaderProps {
   charts: ChartState | {};
   colorScheme?: string;
   customCss: string;
+  userId: number | undefined;
   dashboardInfo: DashboardInfo;
   dashboardTitle: string;
   setColorSchemeAndUnsavedChanges: () => void;

diff --git a/superset-frontend/src/dashboard/containers/DashboardHeader.jsx b/superset-frontend/src/dashboard/containers/DashboardHeader.jsx
@@ -63,6 +63,7 @@ function mapStateToProps({
   dashboardInfo,
   charts,
   dataMask,
+  user,
 }) {
   return {
     dashboardInfo,
@@ -80,7 +81,7 @@ function mapStateToProps({
     colorScheme: dashboardState.colorScheme,
     charts,
     dataMask,
-    userId: dashboardInfo.userId,
+    userId: user.userId,
     isStarred: !!dashboardState.isStarred,
     isPublished: !!dashboardState.isPublished,
     isLoading: isDashboardLoading(charts),