This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 68e8b00cde feat(dashboard): Support changing filter bar location
(#22004)
68e8b00cde is described below
commit 68e8b00cdec21db491995567be0aedbe26ea9482
Author: Kamil Gabryjelski <[email protected]>
AuthorDate: Thu Nov 3 15:50:34 2022 +0100
feat(dashboard): Support changing filter bar location (#22004)
---
.../src/dashboard/actions/dashboardInfo.ts | 68 +++++++-
.../src/dashboard/actions/dashboardState.js | 2 +-
superset-frontend/src/dashboard/actions/hydrate.js | 3 +
.../FilterBarLocationSelect.test.tsx | 175 +++++++++++++++++++++
.../FilterBar/FilterBarLocationSelect/index.tsx | 80 ++++++++++
.../nativeFilters/FilterBar/Header/index.tsx | 23 +--
.../src/dashboard/reducers/dashboardInfo.js | 10 +-
superset-frontend/src/dashboard/types.ts | 6 +
.../src/dashboard/util/permissionUtils.test.ts | 2 +-
.../src/dashboard/util/permissionUtils.ts | 2 +-
superset-frontend/src/types/Dashboard.ts | 2 -
superset/dashboards/schemas.py | 1 +
12 files changed, 345 insertions(+), 29 deletions(-)
diff --git a/superset-frontend/src/dashboard/actions/dashboardInfo.ts
b/superset-frontend/src/dashboard/actions/dashboardInfo.ts
index 9a769101cf..19035a2b22 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<DashboardInfo>,
+ { result: Partial<DashboardInfo>; 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 701eab7845..027288cdbc 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 da16e2f6f6..5e3b97247a 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 0000000000..90b640a2c1
--- /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<DashboardInfo> = {}) =>
+ render(<FilterBarLocationSelect />, {
+ 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 0000000000..82d4ba92e6
--- /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<RootState, FilterBarLocation>(
+ ({ dashboardInfo }) => dashboardInfo.filterBarLocation,
+ );
+ const [selectedFilterBarLocation, setSelectedFilterBarLocation] =
+ useState(filterBarLocation);
+
+ const toggleFilterBarLocation = useCallback(
+ async (
+ selection: Parameters<
+ Required<Pick<MenuProps, 'onSelect'>>['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 (
+ <DropdownSelectableIcon
+ onSelect={toggleFilterBarLocation}
+ info={t('Placement of filter bar')}
+ icon={<Icons.Gear name="gear" iconColor={theme.colors.grayscale.base} />}
+ 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 1f5440d639..a20d6a61f7 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;
@@ -100,26 +100,7 @@ const Header: FC<HeaderProps> = ({ toggleFiltersBar }) => {
<Wrapper>
<TitleArea>
<span>{t('Filters')}</span>
- {canSetHorizontalFilterBar && (
- <DropdownSelectableIcon
- onSelect={item => console.log('Selected item', item)}
- info={t('Placement of filter bar')}
- icon={
- <Icons.Gear name="gear" iconColor={theme.colors.grayscale.base}
/>
- }
- menuItems={[
- {
- key: 'vertical',
- label: t('Vertical (Left)'),
- },
- {
- key: 'horizontal',
- label: t('Horizontal (Top)'),
- },
- ]}
- selectedKeys={['vertical']}
- />
- )}
+ {canSetHorizontalFilterBar && <FilterBarLocationSelect />}
<HeaderButton
{...getFilterBarTestId('collapse-button')}
buttonStyle="link"
diff --git a/superset-frontend/src/dashboard/reducers/dashboardInfo.js
b/superset-frontend/src/dashboard/reducers/dashboardInfo.js
index fdd39fae12..8a01e6122c 100644
--- a/superset-frontend/src/dashboard/reducers/dashboardInfo.js
+++ b/superset-frontend/src/dashboard/reducers/dashboardInfo.js
@@ -17,7 +17,10 @@
* under the License.
*/
-import { DASHBOARD_INFO_UPDATED } from '../actions/dashboardInfo';
+import {
+ DASHBOARD_INFO_UPDATED,
+ SET_FILTER_BAR_LOCATION,
+} from '../actions/dashboardInfo';
import { HYDRATE_DASHBOARD } from '../actions/hydrate';
export default function dashboardStateReducer(state = {}, action) {
@@ -35,6 +38,11 @@ export default function dashboardStateReducer(state = {},
action) {
...action.data.dashboardInfo,
// set async api call data
};
+ case SET_FILTER_BAR_LOCATION:
+ return {
+ ...state,
+ filterBarLocation: action.filterBarLocation,
+ };
default:
return state;
}
diff --git a/superset-frontend/src/dashboard/types.ts
b/superset-frontend/src/dashboard/types.ts
index 57a317bb83..023120f62f 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 };
@@ -92,6 +97,7 @@ export type DashboardInfo = {
label_colors: JsonObject;
shared_label_colors: JsonObject;
};
+ filterBarLocation: FilterBarLocation;
};
export type ChartsState = { [key: string]: Chart };
diff --git a/superset-frontend/src/dashboard/util/permissionUtils.test.ts
b/superset-frontend/src/dashboard/util/permissionUtils.test.ts
index d448db3845..a1aa8c1ca4 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 7348c3526b..4980480cee 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 96a92d567f..faecc0bc4a 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;
diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py
index c8912d1433..e6db0b5688 100644
--- a/superset/dashboards/schemas.py
+++ b/superset/dashboards/schemas.py
@@ -133,6 +133,7 @@ class DashboardJSONMetadataSchema(Schema):
# used for v0 import/export
import_time = fields.Integer()
remote_id = fields.Integer()
+ filter_bar_location = fields.Str(allow_none=True)
class UserSchema(Schema):
