Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(Dashboard): Native filters form update endpoint #30609

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions docs/static/resources/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -3008,6 +3008,33 @@
},
"type": "object"
},
"DashboardNativeFiltersConfigUpdateSchema": {
"properties": {
"deleted": {
"description": "List of filter ids to delete",
"items": {
"type": "integer"
},
"type": "array"
},
"modified": {
"description": "List of filter objects to update",
"items": {
"type": "object"
},
"type": "array"
},
"reordered": {
"description": "List of filter ids in the new order",
"items": {
"type": "integer"
},
"type": "array"
}
},
"required": ["deleted", "modified", "reordered"],
"type": "object"
},
"DashboardCopySchema": {
"properties": {
"css": {
Expand Down Expand Up @@ -13495,6 +13522,72 @@
"tags": ["Dashboards"]
}
},
"/api/v1/dashboard/{id_or_slug}/filters/": {
"put": {
"description": "Update the filters for a given dashboard",
"parameters": [
{
"in": "path",
"name": "id_or_slug",
"required": true,
"schema": {
"type": "string"
}
}
],
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/DashboardNativeFiltersConfigUpdateSchema"
}
}
},
"required": true
},
"responses": {
"200": {
"content": {
"application/json": {
"schema": {
"properties": {
"result": {
"type": "array",
"items": {
"type": "object"
}
}
},
"type": "object"
}
}
},
"description": "Successfully updated the filters"
},
"400": {
"$ref": "#/components/responses/400"
},
"401": {
"$ref": "#/components/responses/401"
},
"403": {
"$ref": "#/components/responses/403"
},
"404": {
"$ref": "#/components/responses/404"
},
"500": {
"$ref": "#/components/responses/500"
}
},
"security": [
{
"jwt": []
}
],
"tags": ["Dashboards"]
}
},
"/api/v1/dashboard/{id_or_slug}/copy/": {
"post": {
"parameters": [
Expand Down
6 changes: 6 additions & 0 deletions superset-frontend/src/dashboard/actions/dashboardInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,17 @@ import {
import { onSave } from './dashboardState';

export const DASHBOARD_INFO_UPDATED = 'DASHBOARD_INFO_UPDATED';
export const DASHBOARD_INFO_FILTERS_CHANGED = 'DASHBOARD_INFO_FILTERS_CHANGED';

// updates partially changed dashboard info
export function dashboardInfoChanged(newInfo: { metadata: any }) {
return { type: DASHBOARD_INFO_UPDATED, newInfo };
}

export function nativeFiltersConfigChanged(newInfo: Record<string, any>) {
return { type: DASHBOARD_INFO_FILTERS_CHANGED, newInfo };
}

export const SAVE_CHART_CONFIG_BEGIN = 'SAVE_CHART_CONFIG_BEGIN';
export const SAVE_CHART_CONFIG_COMPLETE = 'SAVE_CHART_CONFIG_COMPLETE';
export const SAVE_CHART_CONFIG_FAIL = 'SAVE_CHART_CONFIG_FAIL';
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/dashboard/actions/dashboardState.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ import {
} from './dashboardInfo';
import { fetchDatasourceMetadata, setDatasources } from './datasources';
import { updateDirectPathToFilter } from './dashboardFilters';
import { SET_FILTER_CONFIG_COMPLETE } from './nativeFilters';
import { SET_IN_SCOPE_STATUS_OF_FILTERS } from './nativeFilters';
import getOverwriteItems from '../util/getOverwriteItems';
import {
applyColors,
Expand Down Expand Up @@ -337,7 +337,7 @@ export function saveDashboardRequest(data, id, saveType) {
}
if (metadata.native_filter_configuration) {
dispatch({
type: SET_FILTER_CONFIG_COMPLETE,
type: SET_IN_SCOPE_STATUS_OF_FILTERS,
filterConfig: metadata.native_filter_configuration,
});
}
Expand Down
115 changes: 54 additions & 61 deletions superset-frontend/src/dashboard/actions/nativeFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,39 @@
* specific language governing permissions and limitations
* under the License.
*/
import { FilterConfiguration, Filters, makeApi } from '@superset-ui/core';
import {
Filter,
FilterConfiguration,
Filters,
makeApi,
} from '@superset-ui/core';
import { Dispatch } from 'redux';
import { cloneDeep } from 'lodash';
import {
SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL,
setDataMaskForFilterConfigComplete,
} from 'src/dataMask/actions';
import { setDataMaskForFilterChangesComplete } from 'src/dataMask/actions';
import { HYDRATE_DASHBOARD } from './hydrate';
import { dashboardInfoChanged } from './dashboardInfo';
import { DashboardInfo } from '../types';
import {
dashboardInfoChanged,
nativeFiltersConfigChanged,
} from './dashboardInfo';
import { SaveFilterChangesType } from '../components/nativeFilters/FiltersConfigModal/types';

export const SET_FILTER_CONFIG_BEGIN = 'SET_FILTER_CONFIG_BEGIN';
export interface SetFilterConfigBegin {
type: typeof SET_FILTER_CONFIG_BEGIN;
export const SET_NATIVE_FILTERS_CONFIG_BEGIN =
'SET_NATIVE_FILTERS_CONFIG_BEGIN';
export interface SetNativeFiltersConfigBegin {
type: typeof SET_NATIVE_FILTERS_CONFIG_BEGIN;
filterConfig: FilterConfiguration;
}

export const SET_FILTER_CONFIG_COMPLETE = 'SET_FILTER_CONFIG_COMPLETE';
export interface SetFilterConfigComplete {
type: typeof SET_FILTER_CONFIG_COMPLETE;
filterConfig: FilterConfiguration;
export const SET_NATIVE_FILTERS_CONFIG_COMPLETE =
'SET_NATIVE_FILTERS_CONFIG_COMPLETE';
export interface SetNativeFiltersConfigComplete {
type: typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE;
filterChanges: Filter[];
}
export const SET_FILTER_CONFIG_FAIL = 'SET_FILTER_CONFIG_FAIL';
export interface SetFilterConfigFail {
type: typeof SET_FILTER_CONFIG_FAIL;

export const SET_NATIVE_FILTERS_CONFIG_FAIL = 'SET_NATIVE_FILTERS_CONFIG_FAIL';
export interface SetNativeFiltersConfigFail {
type: typeof SET_NATIVE_FILTERS_CONFIG_FAIL;
filterConfig: FilterConfiguration;
}
export const SET_IN_SCOPE_STATUS_OF_FILTERS = 'SET_IN_SCOPE_STATUS_OF_FILTERS';
Expand All @@ -49,60 +57,45 @@ export interface SetInScopeStatusOfFilters {
filterConfig: FilterConfiguration;
}

const isFilterChangesEmpty = (filterChanges: SaveFilterChangesType) =>
Object.values(filterChanges).every(
array => Array.isArray(array) && !array.length,
);

export const setFilterConfiguration =
(filterConfig: FilterConfiguration) =>
(filterChanges: SaveFilterChangesType) =>
async (dispatch: Dispatch, getState: () => any) => {
if (isFilterChangesEmpty(filterChanges)) {
return;
}

const { id } = getState().dashboardInfo;
const oldFilters = getState().nativeFilters?.filters;

dispatch({
type: SET_FILTER_CONFIG_BEGIN,
filterConfig,
type: SET_NATIVE_FILTERS_CONFIG_BEGIN,
filterChanges,
});
const { id, metadata } = getState().dashboardInfo;
const oldFilters = getState().nativeFilters?.filters;

// TODO extract this out when makeApi supports url parameters
const updateDashboard = makeApi<
Partial<DashboardInfo>,
{ result: DashboardInfo }
const updateFilters = makeApi<
SaveFilterChangesType,
{ result: SaveFilterChangesType }
>({
method: 'PUT',
endpoint: `/api/v1/dashboard/${id}`,
endpoint: `/api/v1/dashboard/${id}/filters`,
});

const mergedFilterConfig = filterConfig.map(filter => {
const oldFilter = oldFilters[filter.id];
if (!oldFilter) {
return filter;
}
return { ...oldFilter, ...filter };
});

try {
const response = await updateDashboard({
json_metadata: JSON.stringify({
...metadata,
native_filter_configuration: mergedFilterConfig,
}),
});
dispatch(
dashboardInfoChanged({
metadata: JSON.parse(response.result.json_metadata),
}),
);
const response = await updateFilters(filterChanges);
dispatch(nativeFiltersConfigChanged(response.result));
dispatch({
type: SET_FILTER_CONFIG_COMPLETE,
filterConfig: mergedFilterConfig,
type: SET_NATIVE_FILTERS_CONFIG_COMPLETE,
filterChanges: response.result,
});
dispatch(
setDataMaskForFilterConfigComplete(mergedFilterConfig, oldFilters),
);
dispatch(setDataMaskForFilterChangesComplete(filterChanges, oldFilters));
} catch (err) {
dispatch({
type: SET_FILTER_CONFIG_FAIL,
filterConfig: mergedFilterConfig,
});
dispatch({
type: SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL,
filterConfig: mergedFilterConfig,
type: SET_NATIVE_FILTERS_CONFIG_FAIL,
filterConfig: filterChanges,
});
}
};
Expand Down Expand Up @@ -221,9 +214,9 @@ export function updateCascadeParentIds(
}

export type AnyFilterAction =
| SetFilterConfigBegin
| SetFilterConfigComplete
| SetFilterConfigFail
| SetNativeFiltersConfigBegin
| SetNativeFiltersConfigComplete
| SetNativeFiltersConfigFail
| SetInScopeStatusOfFilters
| SetBootstrapData
| SetFocusedNativeFilter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ import { ReactNode, FC, useCallback, useState, memo } from 'react';
import { useDispatch } from 'react-redux';
import { setFilterConfiguration } from 'src/dashboard/actions/nativeFilters';
import Button from 'src/components/Button';
import { FilterConfiguration, styled } from '@superset-ui/core';
import { styled } from '@superset-ui/core';
import FiltersConfigModal from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal';
import { getFilterBarTestId } from '../utils';
import { SaveFilterChangesType } from '../../FiltersConfigModal/types';

export interface FCBProps {
createNewOnOpen?: boolean;
Expand All @@ -46,14 +47,13 @@ export const FilterConfigurationLink: FC<FCBProps> = ({
}) => {
const dispatch = useDispatch();
const [isOpen, setOpen] = useState(false);

const close = useCallback(() => {
setOpen(false);
}, [setOpen]);

const submit = useCallback(
async (filterConfig: FilterConfiguration) => {
dispatch(await setFilterConfiguration(filterConfig));
async (filterChanges: SaveFilterChangesType) => {
dispatch(await setFilterConfiguration(filterChanges));
close();
},
[dispatch, close],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ const FilterValue: FC<FilterControlProps> = ({
if (isFeatureEnabled(FeatureFlag.GlobalAsyncQueries)) {
// deal with getChartDataRequest transforming the response data
const result = 'result' in json ? json.result[0] : json;

if (response.status === 200) {
setState([result]);
handleFilterLoadFinish();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe('FilterScope', () => {
activeFilterPanelKeys: `DefaultFilterId-${FilterPanels.configuration.key}`,
isActive: true,
validateDependencies: jest.fn(),
onModifyFilter: jest.fn(),
};

const MockModal = ({ scope }: { scope?: object }) => {
Expand Down
Loading
Loading