diff --git a/superset-frontend/src/dashboard/actions/dashboardInfo.ts b/superset-frontend/src/dashboard/actions/dashboardInfo.ts index 9a769101cfdca..19035a2b22033 100644 --- a/superset-frontend/src/dashboard/actions/dashboardInfo.ts +++ b/superset-frontend/src/dashboard/actions/dashboardInfo.ts @@ -17,9 +17,17 @@ * under the License. */ import { Dispatch } from 'redux'; -import { makeApi, CategoricalColorNamespace } from '@superset-ui/core'; +import { makeApi, CategoricalColorNamespace, t } from '@superset-ui/core'; import { isString } from 'lodash'; -import { ChartConfiguration, DashboardInfo } from '../reducers/types'; +import { getClientErrorObject } from 'src/utils/getClientErrorObject'; +import { addDangerToast } from 'src/components/MessageToasts/actions'; +import { + DashboardInfo, + FilterBarLocation, + RootState, +} from 'src/dashboard/types'; +import { ChartConfiguration } from 'src/dashboard/reducers/types'; +import { onSave } from './dashboardState'; export const DASHBOARD_INFO_UPDATED = 'DASHBOARD_INFO_UPDATED'; @@ -111,3 +119,59 @@ export const setChartConfiguration = dispatch({ type: SET_CHART_CONFIG_FAIL, chartConfiguration }); } }; + +export const SET_FILTER_BAR_LOCATION = 'SET_FILTER_BAR_LOCATION'; +export interface SetFilterBarLocation { + type: typeof SET_FILTER_BAR_LOCATION; + filterBarLocation: FilterBarLocation; +} +export function setFilterBarLocation(filterBarLocation: FilterBarLocation) { + return { type: SET_FILTER_BAR_LOCATION, filterBarLocation }; +} + +export function saveFilterBarLocation(location: FilterBarLocation) { + return async (dispatch: Dispatch, getState: () => RootState) => { + const { id, metadata } = getState().dashboardInfo; + const updateDashboard = makeApi< + Partial, + { result: Partial; last_modified_time: number } + >({ + method: 'PUT', + endpoint: `/api/v1/dashboard/${id}`, + }); + 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)); + } + } + 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; + } + }; +} diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 701eab78454d4..027288cdbc9b4 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -284,7 +284,7 @@ export function saveDashboardRequest(data, id, saveType) { const onUpdateSuccess = response => { const updatedDashboard = response.json.result; const lastModifiedTime = response.json.last_modified_time; - // synching with the backend transformations of the metadata + // syncing with the backend transformations of the metadata if (updatedDashboard.json_metadata) { const metadata = JSON.parse(updatedDashboard.json_metadata); dispatch( diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index da16e2f6f6945..5e3b97247a88c 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -57,6 +57,7 @@ import getNativeFilterConfig from '../util/filterboxMigrationHelper'; import { updateColorSchema } from './dashboardInfo'; import { getChartIdsInFilterScope } from '../util/getChartIdsInFilterScope'; import updateComponentParentsList from '../util/updateComponentParentsList'; +import { FilterBarLocation } from '../types'; export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD'; @@ -428,6 +429,8 @@ export const hydrateDashboard = flash_messages: common?.flash_messages, conf: common?.conf, }, + filterBarLocation: + metadata.filter_bar_location ?? FilterBarLocation.VERTICAL, }, dataMask, dashboardFilters, 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 new file mode 100644 index 0000000000000..90b640a2c1678 --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarLocationSelect/FilterBarLocationSelect.test.tsx @@ -0,0 +1,175 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +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: { dashboardInfo: DashboardInfo } = { + dashboardInfo: { + id: 1, + userId: '1', + metadata: { + native_filter_configuration: {}, + show_native_filters: true, + chart_configuration: {}, + color_scheme: '', + color_namespace: '', + color_scheme_domain: [], + label_colors: {}, + shared_label_colors: {}, + }, + json_metadata: '', + dash_edit_perm: true, + filterBarLocation: FilterBarLocation.VERTICAL, + common: { + conf: {}, + flash_messages: [], + }, + }, +}; + +const setup = (dashboardInfoOverride: Partial = {}) => + render(, { + useRedux: true, + 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 new file mode 100644 index 0000000000000..82d4ba92e6ec0 --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarLocationSelect/index.tsx @@ -0,0 +1,80 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React, { useCallback, useState } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { t, useTheme } from '@superset-ui/core'; +import { MenuProps } from 'src/components/Menu'; +import { FilterBarLocation, RootState } from 'src/dashboard/types'; +import { saveFilterBarLocation } from 'src/dashboard/actions/dashboardInfo'; +import Icons from 'src/components/Icons'; +import DropdownSelectableIcon from 'src/components/DropdownSelectableIcon'; + +export const FilterBarLocationSelect = () => { + const dispatch = useDispatch(); + const theme = useTheme(); + const filterBarLocation = useSelector( + ({ dashboardInfo }) => dashboardInfo.filterBarLocation, + ); + const [selectedFilterBarLocation, setSelectedFilterBarLocation] = + useState(filterBarLocation); + + const toggleFilterBarLocation = useCallback( + async ( + selection: Parameters< + Required>['onSelect'] + >[0], + ) => { + const selectedKey = selection.key as FilterBarLocation; + if (selectedKey !== filterBarLocation) { + // set displayed selection in local state for immediate visual response after clicking + setSelectedFilterBarLocation(selectedKey); + try { + // save selection in Redux and backend + await dispatch( + saveFilterBarLocation(selection.key as FilterBarLocation), + ); + } catch { + // revert local state in case of error when saving + setSelectedFilterBarLocation(filterBarLocation); + } + } + }, + [dispatch, filterBarLocation], + ); + + return ( + } + menuItems={[ + { + key: FilterBarLocation.VERTICAL, + label: t('Vertical (Left)'), + }, + { + key: FilterBarLocation.HORIZONTAL, + label: t('Horizontal (Top)'), + }, + ]} + selectedKeys={[selectedFilterBarLocation]} + /> + ); +}; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx index a9ae82cc7c7ce..a20d6a61f75c2 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx @@ -32,8 +32,8 @@ import { useSelector } from 'react-redux'; import FilterConfigurationLink from 'src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink'; import { useFilters } from 'src/dashboard/components/nativeFilters/FilterBar/state'; import { RootState } from 'src/dashboard/types'; -import DropdownSelectableIcon from 'src/components/DropdownSelectableIcon'; import { getFilterBarTestId } from '..'; +import { FilterBarLocationSelect } from '../FilterBarLocationSelect'; const TitleArea = styled.h4` display: flex; @@ -93,34 +93,14 @@ const Header: FC = ({ toggleFiltersBar }) => { const dashboardId = useSelector( ({ dashboardInfo }) => dashboardInfo.id, ); - const canSetHorizontalFilterBar = isFeatureEnabled( - FeatureFlag.HORIZONTAL_FILTER_BAR, - ); + const canSetHorizontalFilterBar = + canEdit && isFeatureEnabled(FeatureFlag.HORIZONTAL_FILTER_BAR); return ( {t('Filters')} - {canSetHorizontalFilterBar && ( - console.log('Selected item', item)} - info={t('Placement of filter bar')} - icon={ - - } - menuItems={[ - { - key: 'vertical', - label: t('Vertical (Left)'), - }, - { - key: 'horizontal', - label: t('Horizontal (Top)'), - }, - ]} - selectedKeys={['vertical']} - /> - )} + {canSetHorizontalFilterBar && }