Skip to content

Commit

Permalink
fix(dashboard list): do not show favorite star for anonymous users #1…
Browse files Browse the repository at this point in the history
…8210 (#19409)

* fix: Only show favorite star on dashboard list if user is logged in #18210

* Fix linter errors

(cherry picked from commit b8891ac)
  • Loading branch information
dudasaron authored and villebro committed Apr 4, 2022
1 parent f44ed06 commit dba4610
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 45 deletions.
2 changes: 1 addition & 1 deletion superset-frontend/src/components/ListViewCard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)`
Expand Down
14 changes: 8 additions & 6 deletions superset-frontend/src/views/CRUD/dashboard/DashboardCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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>
Expand Down
68 changes: 61 additions & 7 deletions superset-frontend/src/views/CRUD/dashboard/DashboardList.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down Expand Up @@ -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);
});

Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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();
});
});
79 changes: 49 additions & 30 deletions superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -96,7 +96,11 @@ const Actions = styled.div`
`;

function DashboardList(props: DashboardListProps) {
const { addDangerToast, addSuccessToast } = props;
const {
addDangerToast,
addSuccessToast,
user: { userId },
} = props;

const {
state: {
Expand Down Expand Up @@ -144,7 +148,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);

Expand Down Expand Up @@ -233,27 +236,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: {
Expand Down Expand Up @@ -423,10 +424,15 @@ function DashboardList(props: DashboardListProps) {
},
],
[
userId,
canEdit,
canDelete,
canExport,
...(props.user.userId ? [favoriteStatus] : []),
saveFavoriteStatus,
favoriteStatus,
refreshData,
addSuccessToast,
addDangerToast,
],
);

Expand Down Expand Up @@ -501,7 +507,7 @@ function DashboardList(props: DashboardListProps) {
{ label: t('Draft'), value: false },
],
},
...(props.user.userId ? [favoritesFilter] : []),
...(userId ? [favoritesFilter] : []),
{
Header: t('Certified'),
id: 'id',
Expand Down Expand Up @@ -545,8 +551,8 @@ function DashboardList(props: DashboardListProps) {
},
];

function renderCard(dashboard: Dashboard) {
return (
const renderCard = useCallback(
(dashboard: Dashboard) => (
<DashboardCard
dashboard={dashboard}
hasPerm={hasPerm}
Expand All @@ -557,6 +563,7 @@ function DashboardList(props: DashboardListProps) {
? userKey.thumbnails
: isFeatureEnabled(FeatureFlag.THUMBNAILS)
}
userId={userId}
loading={loading}
addDangerToast={addDangerToast}
addSuccessToast={addSuccessToast}
Expand All @@ -565,8 +572,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) {
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
Expand Down

0 comments on commit dba4610

Please sign in to comment.