feat(dashboard): Support changing filter bar location (#22004)

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;
+    }
+  };
+}

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(

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,

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();
+});

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]}
+    />
+  );
+};