Skip to content

Commit

Permalink
refactor: Removes Apply to all panels filters scope configuration (#3…
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-s-molina authored Jan 9, 2025
1 parent 3a6fdf8 commit 5acd038
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ describe('FilterScope', () => {
it('select tree values with 1 excluded', async () => {
render(<MockModal />);
fireEvent.click(screen.getByText('Scoping'));
fireEvent.click(screen.getByLabelText('Apply to specific panels'));
expect(screen.getByRole('tree')).toBeInTheDocument();
fireEvent.click(getTreeSwitcher(2));
fireEvent.click(screen.getByText('CHART_ID2'));
Expand All @@ -99,7 +98,6 @@ describe('FilterScope', () => {
it('select 1 value only', async () => {
render(<MockModal />);
fireEvent.click(screen.getByText('Scoping'));
fireEvent.click(screen.getByLabelText('Apply to specific panels'));
expect(screen.getByRole('tree')).toBeInTheDocument();
fireEvent.click(getTreeSwitcher(2));
fireEvent.click(screen.getByText('CHART_ID2'));
Expand All @@ -124,7 +122,6 @@ describe('FilterScope', () => {
/>,
);
fireEvent.click(screen.getByText('Scoping'));
fireEvent.click(screen.getByLabelText('Apply to specific panels'));

await waitFor(() => {
expect(screen.getByRole('tree')).toBeInTheDocument();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,18 @@
* under the License.
*/

import { FC, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { NativeFilterScope, styled, t } from '@superset-ui/core';
import { Radio } from 'src/components/Radio';
import { AntdForm, Typography } from 'src/components';
import { ScopingType } from './types';
import { FC, useCallback, useEffect, useMemo, useState } from 'react';
import { NativeFilterScope, styled } from '@superset-ui/core';
import { AntdForm } from 'src/components';
import ScopingTree from './ScopingTree';
import { getDefaultScopeValue, isScopingAll } from './utils';
import { getDefaultScopeValue } from './utils';

type FilterScopeProps = {
pathToFormValue?: string[];
updateFormValues: (values: any, triggerFormChange?: boolean) => void;
formFilterScope?: NativeFilterScope;
forceUpdate: Function;
filterScope?: NativeFilterScope;
formScopingType?: ScopingType;
chartId?: number;
initiallyExcludedCharts?: number[];
};
Expand All @@ -51,7 +48,6 @@ const CleanFormItem = styled(AntdForm.Item)`

const FilterScope: FC<FilterScopeProps> = ({
pathToFormValue = [],
formScopingType,
formFilterScope,
forceUpdate,
filterScope,
Expand All @@ -63,25 +59,14 @@ const FilterScope: FC<FilterScopeProps> = ({
() => filterScope || getDefaultScopeValue(chartId, initiallyExcludedCharts),
[chartId, filterScope, initiallyExcludedCharts],
);
const lastSpecificScope = useRef(initialFilterScope);
const initialScopingType = useMemo(
() =>
isScopingAll(initialFilterScope, chartId)
? ScopingType.All
: ScopingType.Specific,
[chartId, initialFilterScope],
);
const [hasScopeBeenModified, setHasScopeBeenModified] = useState(false);

const onUpdateFormValues = useCallback(
(formValues: any) => {
if (formScopingType === ScopingType.Specific) {
lastSpecificScope.current = formValues.scope;
}
updateFormValues(formValues);
setHasScopeBeenModified(true);
},
[formScopingType, updateFormValues],
[updateFormValues],
);

const updateScopes = useCallback(
Expand All @@ -98,49 +83,20 @@ const FilterScope: FC<FilterScopeProps> = ({
useEffect(() => {
const updatedFormValues = {
scope: initialFilterScope,
scoping: initialScopingType,
};
updateScopes(updatedFormValues);
}, [initialFilterScope, initialScopingType, updateScopes]);
}, [initialFilterScope, updateScopes]);

return (
<Wrapper>
<CleanFormItem
name={[...pathToFormValue, 'scoping']}
initialValue={initialScopingType}
>
<Radio.Group
onChange={({ target: { value } }) => {
const scope =
value === ScopingType.All
? getDefaultScopeValue(chartId)
: lastSpecificScope.current;
updateFormValues({ scope });
setHasScopeBeenModified(true);
forceUpdate();
}}
>
<Radio value={ScopingType.All}>{t('Apply to all panels')}</Radio>
<Radio value={ScopingType.Specific}>
{t('Apply to specific panels')}
</Radio>
</Radio.Group>
</CleanFormItem>
<Typography.Text type="secondary">
{(formScopingType ?? initialScopingType) === ScopingType.Specific
? t('Only selected panels will be affected by this filter')
: t('All panels with this column will be affected by this filter')}
</Typography.Text>
{(formScopingType ?? initialScopingType) === ScopingType.Specific && (
<ScopingTree
updateFormValues={onUpdateFormValues}
initialScope={initialFilterScope}
formScope={formFilterScope}
forceUpdate={forceUpdate}
chartId={chartId}
initiallyExcludedCharts={initiallyExcludedCharts}
/>
)}
<ScopingTree
updateFormValues={onUpdateFormValues}
initialScope={initialFilterScope}
formScope={formFilterScope}
forceUpdate={forceUpdate}
chartId={chartId}
initiallyExcludedCharts={initiallyExcludedCharts}
/>
<CleanFormItem
name={[...pathToFormValue, 'scope']}
hidden
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@

import { ReactNode } from 'react';

export enum ScopingType {
All,
Specific,
}

/** UI Ant tree type */
export type TreeItem = {
children: TreeItem[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,3 @@ export const getDefaultScopeValue = (
? [chartId, ...initiallyExcludedCharts]
: initiallyExcludedCharts,
});

export const isScopingAll = (scope: NativeFilterScope, chartId?: number) =>
!scope ||
(scope.rootPath[0] === DASHBOARD_ROOT_ID &&
!scope.excluded.filter(item => item !== chartId).length);
Original file line number Diff line number Diff line change
Expand Up @@ -1353,7 +1353,6 @@ const FiltersConfigForm = (
forceUpdate={forceUpdate}
filterScope={filterToEdit?.scope}
formFilterScope={formFilter?.scope}
formScopingType={formFilter?.scoping}
initiallyExcludedCharts={initiallyExcludedCharts}
/>
</TabPane>
Expand Down

0 comments on commit 5acd038

Please sign in to comment.