geido commented on code in PR #33831:
URL: https://github.com/apache/superset/pull/33831#discussion_r2166856705
##########
superset-frontend/src/dashboard/actions/dashboardInfo.ts:
##########
@@ -113,7 +151,56 @@ export function
setCrossFiltersEnabled(crossFiltersEnabled: boolean) {
return { type: SET_CROSS_FILTERS_ENABLED, crossFiltersEnabled };
}
-export function saveFilterBarOrientation(orientation: FilterBarOrientation) {
+export const SAVE_CHART_CUSTOMIZATION_COMPLETE =
+ 'SAVE_CHART_CUSTOMIZATION_COMPLETE';
+
+function getAffectedChartIdsFromCustomization(
+ chartCustomization: ChartCustomizationItem[],
+ state: any,
Review Comment:
Curious if there is a better type definition that we can re-use for this
##########
superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/ChartCustomizationModal.tsx:
##########
@@ -0,0 +1,524 @@
+/**
+ * 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 { useState, useEffect, useMemo, useCallback, memo } from 'react';
+import { t, styled, css } from '@superset-ui/core';
+import { useSelector } from 'react-redux';
+import { isEmpty, isEqual, sortBy, debounce } from 'lodash';
+import { StyledModal, Form } from '@superset-ui/core/components';
+import { ErrorBoundary } from 'src/components/ErrorBoundary';
+import Footer from
'src/dashboard/components/nativeFilters/FiltersConfigModal/Footer/Footer';
+import { CancelConfirmationAlert } from
'src/dashboard/components/nativeFilters/FiltersConfigModal/Footer/CancelConfirmationAlert';
+import { DatasourcesState, ChartsState, RootState } from 'src/dashboard/types';
+import { mostUsedDataset } from
'../FiltersConfigModal/FiltersConfigForm/utils';
+import ChartCustomizationTitlePane from './ChartCustomizationTitlePane';
+import ChartCustomizationForm from './ChartCustomizationForm';
+import { createDefaultChartCustomizationItem } from './utils';
+import { ChartCustomizationItem } from './types';
+import RemovedFilter from
'../FiltersConfigModal/FiltersConfigForm/RemovedFilter';
+import { selectChartCustomizationItems } from './selectors';
+
+const MIN_WIDTH = 880;
+const MODAL_MARGIN = 16;
+
+const StyledModalWrapper = styled(StyledModal)<{ expanded: boolean }>`
Review Comment:
@LevisNgigi wasn't the plan to re-use the existing native filters modal and
form instead of creating two separate components that almost do the same thing?
Wouldn't composability help here?
##########
superset-frontend/src/dashboard/actions/dashboardInfo.ts:
##########
@@ -142,43 +229,272 @@ 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(error || t('Failed to save filter bar orientation')),
+ );
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;
+ dispatch(setCrossFiltersEnabled(crossFiltersEnabled));
const updateDashboard = makeApi<
Partial<DashboardInfo>,
- { result: Partial<DashboardInfo>; last_modified_time: number }
+ { result: DashboardInfo }
>({
method: 'PUT',
endpoint: `/api/v1/dashboard/${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 simpleItems = chartCustomizationItems
+ .filter(item => !item.removed)
+ .map(item => ({
+ id: item.id,
+ title: item.title || '[untitled]',
+ chartId: item.chartId, // Include chart ID in saved data
+ customization: {
+ name: item.customization?.name || '',
+ dataset: item.customization?.dataset || null,
+ datasetInfo: item.customization?.datasetInfo,
+ column: item.customization?.column || null,
+ description: item.customization?.description,
+ sortFilter: !!item.customization?.sortFilter,
+ sortAscending: item.customization?.sortAscending !== false,
+ sortMetric: item.customization?.sortMetric || undefined,
+ hasDefaultValue: !!item.customization?.hasDefaultValue,
+ defaultValue: item.customization?.defaultValue,
+ isRequired: !!item.customization?.isRequired,
+ selectFirst: !!item.customization?.selectFirst,
+ defaultDataMask: item.customization?.defaultDataMask,
+ defaultValueQueriesData: item.customization?.defaultValueQueriesData,
+ },
+ }));
+
+ const updateDashboard = makeApi<
+ Partial<DashboardInfo>,
+ { result: Partial<DashboardInfo>; last_modified_time: number }
Review Comment:
It seems this was defined before as well, can we dry up?
##########
superset-frontend/src/dashboard/components/GroupByBadge/index.tsx:
##########
@@ -0,0 +1,179 @@
+/**
+ * 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 { memo, useMemo, useState, useRef } from 'react';
+import { useSelector } from 'react-redux';
+import { styled, t } from '@superset-ui/core';
+import { Icons, Badge, Tooltip } from '@superset-ui/core/components';
+import { ChartCustomizationItem } from
'../nativeFilters/ChartCustomization/types';
+import { RootState } from '../../types';
+
+export interface GroupByBadgeProps {
+ chartId: number;
+}
+
+const StyledGroupByCount = styled.div`
Review Comment:
It seems like Ant Design might offer something like this that we can re-use
with minimal style customizations if any at all?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
