amaannawab923 commented on code in PR #33831:
URL: https://github.com/apache/superset/pull/33831#discussion_r2239792058
##########
superset-frontend/src/dashboard/actions/dashboardInfo.ts:
##########
@@ -188,43 +288,403 @@ export function saveFilterBarOrientation(orientation:
FilterBarOrientation) {
dispatch(onSave(lastModifiedTime));
}
} catch (errorObject) {
- const errorText = await getErrorText(errorObject, 'dashboard');
- dispatch(addDangerToast(errorText));
+ const { error } = await getClientErrorObject(errorObject);
+ dispatch(
+ addDangerToast(
+ t(
+ 'Sorry, there was an error saving this dashboard: %s',
+ error || 'Bad Request',
+ ),
+ ),
+ );
throw errorObject;
}
};
}
export function saveCrossFiltersSetting(crossFiltersEnabled: boolean) {
- return async (dispatch: Dispatch, getState: () => RootState) => {
+ return async function saveCrossFiltersSettingThunk(
+ 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}`,
- });
+ dispatch(setCrossFiltersEnabled(crossFiltersEnabled));
+ const updateDashboard = createUpdateDashboardApi(id);
+
try {
const response = await updateDashboard({
json_metadata: JSON.stringify({
...metadata,
cross_filters_enabled: crossFiltersEnabled,
}),
});
+ dispatch(
+ dashboardInfoChanged({
+ metadata: JSON.parse(response.result.json_metadata || '{}'),
+ }),
+ );
+ return response;
+ } catch (err) {
+ dispatch(addDangerToast(t('Failed to save cross-filters setting')));
+ throw err;
+ }
+ };
+}
+
+export function saveChartCustomization(
+ chartCustomizationItems: ChartCustomizationSavePayload[],
+): ThunkAction<Promise<any>, RootState, null, AnyAction> {
+ return async function (
+ dispatch: ThunkDispatch<RootState, null, AnyAction>,
+ getState: () => RootState,
+ ) {
+ const { id, metadata, json_metadata } = getState().dashboardInfo;
+
+ const currentState = getState();
+ const currentChartCustomizationItems =
+ currentState.dashboardInfo.metadata?.chart_customization_config || [];
+
+ const existingItemsMap = new Map(
+ currentChartCustomizationItems.map(item => [item.id, item]),
+ );
+
+ const updatedItemsMap = new Map(existingItemsMap);
+
+ chartCustomizationItems.forEach(newItem => {
+ if (newItem.removed) {
+ updatedItemsMap.delete(newItem.id);
+ } else {
+ const chartCustomizationItem: ChartCustomizationItem = {
+ id: newItem.id,
+ title: newItem.title,
+ removed: newItem.removed,
+ chartId: newItem.chartId,
+ customization: newItem.customization,
+ };
+ updatedItemsMap.set(newItem.id, chartCustomizationItem);
+ }
+ });
+
+ const simpleItems = Array.from(updatedItemsMap.values());
+
+ const updateDashboard = createUpdateDashboardApi(id);
+
+ try {
+ let parsedMetadata: any = {};
+ try {
+ parsedMetadata = json_metadata ? JSON.parse(json_metadata) : metadata;
+ } catch (e) {
+ console.error('Error parsing json_metadata:', e);
+ parsedMetadata = metadata || {};
+ }
+
+ const updatedMetadata = {
+ ...parsedMetadata,
+ native_filter_configuration: (
+ parsedMetadata.native_filter_configuration || []
+ ).filter(
+ (item: any) =>
+ !(
+ item.type === 'CHART_CUSTOMIZATION' &&
+ item.id === 'chart_customization_groupby'
+ ),
+ ),
+ chart_customization_config: simpleItems,
+ };
+
+ const response = await updateDashboard({
+ json_metadata: JSON.stringify(updatedMetadata),
+ });
+
const updatedDashboard = response.result;
const lastModifiedTime = response.last_modified_time;
+
if (updatedDashboard.json_metadata) {
- const metadata = JSON.parse(updatedDashboard.json_metadata);
- dispatch(setCrossFiltersEnabled(metadata.cross_filters_enabled));
+ dispatch(setChartCustomization(simpleItems));
+
+ const removedItems = currentChartCustomizationItems.filter(
+ existingItem => !updatedItemsMap.has(existingItem.id),
+ );
+
+ removedItems.forEach(removedItem => {
+ const customizationFilterId =
`chart_customization_${removedItem.id}`;
+ dispatch(removeDataMask(customizationFilterId));
+ });
+
+ simpleItems.forEach(item => {
+ const customizationFilterId = `chart_customization_${item.id}`;
+
+ if (item.customization?.column) {
+ dispatch(removeDataMask(customizationFilterId));
+
+ const dataMask = {
+ extraFormData: {},
+ filterState: {
+ value:
+ item.customization?.defaultDataMask?.filterState?.value ||
[],
+ },
+ ownState: {
+ column: item.customization.column,
+ },
+ };
+ dispatch(updateDataMask(customizationFilterId, dataMask));
+ } else {
+ dispatch(removeDataMask(customizationFilterId));
+ }
+ });
+
+ const state = getState();
+ const affectedChartIds = getAffectedChartIdsFromCustomization(
+ simpleItems,
+ state,
+ );
+
+ removedItems.forEach(removedItem => {
+ if (removedItem.chartId) {
+ affectedChartIds.push(removedItem.chartId);
+ }
+ });
+
+ const uniqueAffectedChartIds = [...new Set(affectedChartIds)];
+
+ uniqueAffectedChartIds.forEach(chartId => {
+ const chart = state.charts[chartId];
+ if (
+ chart?.latestQueryFormData &&
+ Object.keys(chart.latestQueryFormData).length > 0
+ ) {
+ dispatch(refreshChart(chartId));
Review Comment:
Why are we refreshing Charts from here
superset-frontend/src/dashboard/components/Dashboard.jsx
The native filters get affected chart ids in dashboard.jsx when they compare
activeFilters & appliedFilters .... Is it safe to put that logic here .... I
mean this is not the convention ... Since we extended native filters logic to
groupby's the existing native filter way should have been fine
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
