From 34820b4677118cff69667b9af41594e8693e7ab0 Mon Sep 17 00:00:00 2001 From: Aron Dudas <6893357+dudasaron@users.noreply.github.com> Date: Tue, 29 Mar 2022 14:54:30 +0200 Subject: [PATCH 1/2] fix: Only show favorite star on dashboard list if user is logged in #18210 --- .../src/components/ListViewCard/index.tsx | 2 +- .../views/CRUD/dashboard/DashboardCard.tsx | 14 +-- .../CRUD/dashboard/DashboardList.test.jsx | 68 ++++++++++-- .../views/CRUD/dashboard/DashboardList.tsx | 79 ++++++++------ superset-frontend/src/views/CRUD/utils.tsx | 101 +++++++++--------- 5 files changed, 168 insertions(+), 96 deletions(-) diff --git a/superset-frontend/src/components/ListViewCard/index.tsx b/superset-frontend/src/components/ListViewCard/index.tsx index 79616c86aca5..153b06489be4 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 25a012f86b32..2da9f4551533 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(); }} > - + {userId && ( + + )} diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.test.jsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.test.jsx index 3561cafdbce6..e42ba92ff292 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( - - - - - , - ); + let wrapper; beforeAll(async () => { + fetchMock.resetHistory(); + wrapper = mount( + + + + + , + ); + 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( + + + + + , + ); + + 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 b3da8ee8e353..c7c6fe00aae7 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) => ( - - ), - Header: '', - id: 'id', - disableSortBy: true, - size: 'xs', - }, - ] - : []), + { + Cell: ({ + row: { + original: { id }, + }, + }: any) => + userId ? ( + + ) : null, + 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) => ( - ); - } + ), + [ + 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 6606fd459f87..41646b36d7b8 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -64,60 +64,57 @@ import { Dashboard, Filters } from './types'; risonRef.next_id = new RegExp(idrx, 'g'); })(); -const createFetchResourceMethod = - (method: string) => - ( - resource: string, - relation: string, - handleError: (error: Response) => void, - user?: { userId: string | number; firstName: string; lastName: string }, - ) => - async (filterValue = '', page: number, pageSize: number) => { - const resourceEndpoint = `/api/v1/${resource}/${method}/${relation}`; - const queryParams = rison.encode_uri({ - filter: filterValue, - page, - page_size: pageSize, - }); - const { json = {} } = await SupersetClient.get({ - endpoint: `${resourceEndpoint}?q=${queryParams}`, - }); - - let fetchedLoggedUser = false; - const loggedUser = user - ? { - label: `${user.firstName} ${user.lastName}`, - value: user.userId, - } - : undefined; - - const data: { label: string; value: string | number }[] = []; - json?.result?.forEach( - ({ text, value }: { text: string; value: string | number }) => { - if ( - loggedUser && - value === loggedUser.value && - text === loggedUser.label - ) { - fetchedLoggedUser = true; - } else { - data.push({ - label: text, - value, - }); - } - }, - ); +const createFetchResourceMethod = (method: string) => ( + resource: string, + relation: string, + handleError: (error: Response) => void, + user?: { userId: string | number; firstName: string; lastName: string }, +) => async (filterValue = '', page: number, pageSize: number) => { + const resourceEndpoint = `/api/v1/${resource}/${method}/${relation}`; + const queryParams = rison.encode_uri({ + filter: filterValue, + page, + page_size: pageSize, + }); + const { json = {} } = await SupersetClient.get({ + endpoint: `${resourceEndpoint}?q=${queryParams}`, + }); - if (loggedUser && (!filterValue || fetchedLoggedUser)) { - data.unshift(loggedUser); - } + let fetchedLoggedUser = false; + const loggedUser = user + ? { + label: `${user.firstName} ${user.lastName}`, + value: user.userId, + } + : undefined; + + const data: { label: string; value: string | number }[] = []; + json?.result?.forEach( + ({ text, value }: { text: string; value: string | number }) => { + if ( + loggedUser && + value === loggedUser.value && + text === loggedUser.label + ) { + fetchedLoggedUser = true; + } else { + data.push({ + label: text, + value, + }); + } + }, + ); - return { - data, - totalCount: json?.count, - }; + if (loggedUser && (!filterValue || fetchedLoggedUser)) { + data.unshift(loggedUser); + } + + return { + data, + totalCount: json?.count, }; +}; export const PAGE_SIZE = 5; const getParams = (filters?: Array) => { @@ -285,7 +282,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}`, From ced4cea8ad6ae342b95eea0729c4c2f85421f2b2 Mon Sep 17 00:00:00 2001 From: Aron Dudas <6893357+dudasaron@users.noreply.github.com> Date: Thu, 31 Mar 2022 09:43:30 +0200 Subject: [PATCH 2/2] Fix linter errors --- .../views/CRUD/dashboard/DashboardList.tsx | 4 +- superset-frontend/src/views/CRUD/utils.tsx | 99 ++++++++++--------- 2 files changed, 53 insertions(+), 50 deletions(-) diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index c7c6fe00aae7..a00ceefbc876 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -241,13 +241,13 @@ function DashboardList(props: DashboardListProps) { original: { id }, }, }: any) => - userId ? ( + userId && ( - ) : null, + ), Header: '', id: 'id', disableSortBy: true, diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index 41646b36d7b8..a0b10241e7d2 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -64,57 +64,60 @@ import { Dashboard, Filters } from './types'; risonRef.next_id = new RegExp(idrx, 'g'); })(); -const createFetchResourceMethod = (method: string) => ( - resource: string, - relation: string, - handleError: (error: Response) => void, - user?: { userId: string | number; firstName: string; lastName: string }, -) => async (filterValue = '', page: number, pageSize: number) => { - const resourceEndpoint = `/api/v1/${resource}/${method}/${relation}`; - const queryParams = rison.encode_uri({ - filter: filterValue, - page, - page_size: pageSize, - }); - const { json = {} } = await SupersetClient.get({ - endpoint: `${resourceEndpoint}?q=${queryParams}`, - }); - - let fetchedLoggedUser = false; - const loggedUser = user - ? { - label: `${user.firstName} ${user.lastName}`, - value: user.userId, - } - : undefined; - - const data: { label: string; value: string | number }[] = []; - json?.result?.forEach( - ({ text, value }: { text: string; value: string | number }) => { - if ( - loggedUser && - value === loggedUser.value && - text === loggedUser.label - ) { - fetchedLoggedUser = true; - } else { - data.push({ - label: text, - value, - }); - } - }, - ); +const createFetchResourceMethod = + (method: string) => + ( + resource: string, + relation: string, + handleError: (error: Response) => void, + user?: { userId: string | number; firstName: string; lastName: string }, + ) => + async (filterValue = '', page: number, pageSize: number) => { + const resourceEndpoint = `/api/v1/${resource}/${method}/${relation}`; + const queryParams = rison.encode_uri({ + filter: filterValue, + page, + page_size: pageSize, + }); + const { json = {} } = await SupersetClient.get({ + endpoint: `${resourceEndpoint}?q=${queryParams}`, + }); + + let fetchedLoggedUser = false; + const loggedUser = user + ? { + label: `${user.firstName} ${user.lastName}`, + value: user.userId, + } + : undefined; + + const data: { label: string; value: string | number }[] = []; + json?.result?.forEach( + ({ text, value }: { text: string; value: string | number }) => { + if ( + loggedUser && + value === loggedUser.value && + text === loggedUser.label + ) { + fetchedLoggedUser = true; + } else { + data.push({ + label: text, + value, + }); + } + }, + ); - if (loggedUser && (!filterValue || fetchedLoggedUser)) { - data.unshift(loggedUser); - } + if (loggedUser && (!filterValue || fetchedLoggedUser)) { + data.unshift(loggedUser); + } - return { - data, - totalCount: json?.count, + return { + data, + totalCount: json?.count, + }; }; -}; export const PAGE_SIZE = 5; const getParams = (filters?: Array) => {