From 3eaa53c7756e31fabd9d26b878c0a2208053d364 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Wed, 2 Nov 2022 11:14:36 +0100 Subject: [PATCH] Add UT --- .../src/dashboard/actions/dashboardInfo.ts | 67 +++++---- .../FilterBarLocationSelect.test.tsx | 131 ++++++++++++++++-- .../FilterBarLocationSelect/index.tsx | 6 +- superset-frontend/src/dashboard/types.ts | 10 +- .../dashboard/util/permissionUtils.test.ts | 2 +- .../src/dashboard/util/permissionUtils.ts | 2 +- superset-frontend/src/types/Dashboard.ts | 2 - 7 files changed, 167 insertions(+), 53 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/dashboardInfo.ts b/superset-frontend/src/dashboard/actions/dashboardInfo.ts index e12cb7624d1c6..19035a2b22033 100644 --- a/superset-frontend/src/dashboard/actions/dashboardInfo.ts +++ b/superset-frontend/src/dashboard/actions/dashboardInfo.ts @@ -130,7 +130,7 @@ export function setFilterBarLocation(filterBarLocation: FilterBarLocation) { } export function saveFilterBarLocation(location: FilterBarLocation) { - return (dispatch: Dispatch, getState: () => RootState) => { + return async (dispatch: Dispatch, getState: () => RootState) => { const { id, metadata } = getState().dashboardInfo; const updateDashboard = makeApi< Partial, @@ -139,40 +139,39 @@ export function saveFilterBarLocation(location: FilterBarLocation) { method: 'PUT', endpoint: `/api/v1/dashboard/${id}`, }); - return updateDashboard({ - json_metadata: JSON.stringify({ - ...metadata, - filter_bar_location: location, - }), - }) - .then(response => { - const updatedDashboard = response.result; - const lastModifiedTime = response.last_modified_time; - if (updatedDashboard.json_metadata) { - const metadata = JSON.parse(updatedDashboard.json_metadata); - if (metadata.filter_bar_location) { - dispatch(setFilterBarLocation(metadata.filter_bar_location)); - } - } - if (lastModifiedTime) { - dispatch(onSave(lastModifiedTime)); + try { + const response = await updateDashboard({ + json_metadata: JSON.stringify({ + ...metadata, + filter_bar_location: location, + }), + }); + const updatedDashboard = response.result; + const lastModifiedTime = response.last_modified_time; + if (updatedDashboard.json_metadata) { + const metadata = JSON.parse(updatedDashboard.json_metadata); + if (metadata.filter_bar_location) { + dispatch(setFilterBarLocation(metadata.filter_bar_location)); } - }) - .catch(async errorObject => { - const { error, message } = await getClientErrorObject(errorObject); - let errorText = t('Sorry, an unknown error occurred.'); + } + if (lastModifiedTime) { + dispatch(onSave(lastModifiedTime)); + } + } catch (errorObject) { + const { error, message } = await getClientErrorObject(errorObject); + let errorText = t('Sorry, an unknown error occurred.'); - if (error) { - errorText = t( - 'Sorry, there was an error saving this dashboard: %s', - error, - ); - } - if (typeof message === 'string' && message === 'Forbidden') { - errorText = t('You do not have permission to edit this dashboard'); - } - dispatch(addDangerToast(errorText)); - throw errorObject; - }); + if (error) { + errorText = t( + 'Sorry, there was an error saving this dashboard: %s', + error, + ); + } + if (typeof message === 'string' && message === 'Forbidden') { + errorText = t('You do not have permission to edit this dashboard'); + } + dispatch(addDangerToast(errorText)); + throw errorObject; + } }; } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarLocationSelect/FilterBarLocationSelect.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarLocationSelect/FilterBarLocationSelect.test.tsx index 480e583c41ccb..90b640a2c1678 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarLocationSelect/FilterBarLocationSelect.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarLocationSelect/FilterBarLocationSelect.test.tsx @@ -18,12 +18,15 @@ */ import React from 'react'; -import { render, screen } from 'spec/helpers/testing-library'; -import { FilterBarLocation } from 'src/dashboard/types'; -import { RootState } from 'src/dashboard/types'; +import fetchMock from 'fetch-mock'; +import { waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { render, screen, within } from 'spec/helpers/testing-library'; +import { DashboardInfo, FilterBarLocation } from 'src/dashboard/types'; +import * as mockedMessageActions from 'src/components/MessageToasts/actions'; import { FilterBarLocationSelect } from './index'; -const initialState: Partial = { +const initialState: { dashboardInfo: DashboardInfo } = { dashboardInfo: { id: 1, userId: '1', @@ -47,14 +50,126 @@ const initialState: Partial = { }, }; -const setup = (initialStateOverride: Partial = {}) => { - return render(, { +const setup = (dashboardInfoOverride: Partial = {}) => + render(, { useRedux: true, - initialState: { ...initialState, ...initialStateOverride }, + initialState: { + ...initialState, + dashboardInfo: { + ...initialState.dashboardInfo, + ...dashboardInfoOverride, + }, + }, }); -}; test('Dropdown trigger renders', () => { setup(); expect(screen.getByLabelText('gear')).toBeVisible(); }); + +test('Popover opens with "Vertical" selected', async () => { + setup(); + userEvent.click(screen.getByLabelText('gear')); + expect(await screen.findByText('Vertical (Left)')).toBeInTheDocument(); + expect(screen.getByText('Horizontal (Top)')).toBeInTheDocument(); + expect( + within(screen.getAllByRole('menuitem')[0]).getByLabelText('check'), + ).toBeInTheDocument(); +}); + +test('Popover opens with "Horizontal" selected', async () => { + setup({ filterBarLocation: FilterBarLocation.HORIZONTAL }); + userEvent.click(screen.getByLabelText('gear')); + expect(await screen.findByText('Vertical (Left)')).toBeInTheDocument(); + expect(screen.getByText('Horizontal (Top)')).toBeInTheDocument(); + expect( + within(screen.getAllByRole('menuitem')[1]).getByLabelText('check'), + ).toBeInTheDocument(); +}); + +test('On selection change, send request and update checked value', async () => { + fetchMock.reset(); + fetchMock.put('glob:*/api/v1/dashboard/1', { + result: { + json_metadata: JSON.stringify({ + ...initialState.dashboardInfo.metadata, + filter_bar_location: 'HORIZONTAL', + }), + }, + }); + + setup(); + userEvent.click(screen.getByLabelText('gear')); + + expect( + within(screen.getAllByRole('menuitem')[0]).getByLabelText('check'), + ).toBeInTheDocument(); + expect( + within(screen.getAllByRole('menuitem')[1]).queryByLabelText('check'), + ).not.toBeInTheDocument(); + + userEvent.click(await screen.findByText('Horizontal (Top)')); + + // 1st check - checkmark appears immediately after click + expect( + await within(screen.getAllByRole('menuitem')[1]).findByLabelText('check'), + ).toBeInTheDocument(); + expect( + within(screen.getAllByRole('menuitem')[0]).queryByLabelText('check'), + ).not.toBeInTheDocument(); + + // successful query + await waitFor(() => + expect(fetchMock.lastCall()?.[1]?.body).toEqual( + JSON.stringify({ + json_metadata: JSON.stringify({ + ...initialState.dashboardInfo.metadata, + filter_bar_location: 'HORIZONTAL', + }), + }), + ), + ); + + // 2nd check - checkmark stays after successful query + expect( + await within(screen.getAllByRole('menuitem')[1]).findByLabelText('check'), + ).toBeInTheDocument(); + expect( + within(screen.getAllByRole('menuitem')[0]).queryByLabelText('check'), + ).not.toBeInTheDocument(); + + fetchMock.reset(); +}); + +test('On failed request, restore previous selection', async () => { + fetchMock.reset(); + fetchMock.put('glob:*/api/v1/dashboard/1', 400); + + const dangerToastSpy = jest.spyOn(mockedMessageActions, 'addDangerToast'); + + setup(); + userEvent.click(screen.getByLabelText('gear')); + + expect( + within(screen.getAllByRole('menuitem')[0]).getByLabelText('check'), + ).toBeInTheDocument(); + expect( + within(screen.getAllByRole('menuitem')[1]).queryByLabelText('check'), + ).not.toBeInTheDocument(); + + userEvent.click(await screen.findByText('Horizontal (Top)')); + + // checkmark gets rolled back to the original selection after successful query + expect( + await within(screen.getAllByRole('menuitem')[0]).findByLabelText('check'), + ).toBeInTheDocument(); + expect( + within(screen.getAllByRole('menuitem')[1]).queryByLabelText('check'), + ).not.toBeInTheDocument(); + + expect(dangerToastSpy).toHaveBeenCalledWith( + 'Sorry, there was an error saving this dashboard: Unknown Error', + ); + + fetchMock.reset(); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarLocationSelect/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarLocationSelect/index.tsx index d79661be6e6c6..82d4ba92e6ec0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarLocationSelect/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarLocationSelect/index.tsx @@ -36,7 +36,7 @@ export const FilterBarLocationSelect = () => { useState(filterBarLocation); const toggleFilterBarLocation = useCallback( - ( + async ( selection: Parameters< Required>['onSelect'] >[0], @@ -47,7 +47,9 @@ export const FilterBarLocationSelect = () => { setSelectedFilterBarLocation(selectedKey); try { // save selection in Redux and backend - dispatch(saveFilterBarLocation(selection.key as FilterBarLocation)); + await dispatch( + saveFilterBarLocation(selection.key as FilterBarLocation), + ); } catch { // revert local state in case of error when saving setSelectedFilterBarLocation(filterBarLocation); diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index c8ec6eb7f861a..023120f62fbcd 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -52,6 +52,11 @@ export type Chart = ChartState & { }; }; +export enum FilterBarLocation { + VERTICAL = 'VERTICAL', + HORIZONTAL = 'HORIZONTAL', +} + export type ActiveTabs = string[]; export type DashboardLayout = { [key: string]: LayoutItem }; export type DashboardLayoutState = { present: DashboardLayout }; @@ -175,8 +180,3 @@ export type EmbeddedDashboard = { dashboard_id: string; allowed_domains: string[]; }; - -export enum FilterBarLocation { - VERTICAL = 'VERTICAL', - HORIZONTAL = 'HORIZONTAL', -} diff --git a/superset-frontend/src/dashboard/util/permissionUtils.test.ts b/superset-frontend/src/dashboard/util/permissionUtils.test.ts index d448db3845c68..a1aa8c1ca4259 100644 --- a/superset-frontend/src/dashboard/util/permissionUtils.test.ts +++ b/superset-frontend/src/dashboard/util/permissionUtils.test.ts @@ -20,7 +20,7 @@ import { UndefinedUser, UserWithPermissionsAndRoles, } from 'src/types/bootstrapTypes'; -import Dashboard from 'src/types/Dashboard'; +import { Dashboard } from 'src/types/Dashboard'; import Owner from 'src/types/Owner'; import { canUserEditDashboard, isUserAdmin } from './permissionUtils'; diff --git a/superset-frontend/src/dashboard/util/permissionUtils.ts b/superset-frontend/src/dashboard/util/permissionUtils.ts index 7348c3526b293..4980480cee0b6 100644 --- a/superset-frontend/src/dashboard/util/permissionUtils.ts +++ b/superset-frontend/src/dashboard/util/permissionUtils.ts @@ -21,7 +21,7 @@ import { UndefinedUser, UserWithPermissionsAndRoles, } from 'src/types/bootstrapTypes'; -import Dashboard from 'src/types/Dashboard'; +import { Dashboard } from 'src/types/Dashboard'; import { findPermission } from 'src/utils/findPermission'; // this should really be a config value, diff --git a/superset-frontend/src/types/Dashboard.ts b/superset-frontend/src/types/Dashboard.ts index 96a92d567f124..faecc0bc4acf7 100644 --- a/superset-frontend/src/types/Dashboard.ts +++ b/superset-frontend/src/types/Dashboard.ts @@ -36,5 +36,3 @@ export interface Dashboard { owners: Owner[]; roles: Role[]; } - -export default Dashboard;