villebro commented on a change in pull request #13576:
URL: https://github.com/apache/superset/pull/13576#discussion_r594181636
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FilterSetUnit.tsx
##########
@@ -41,36 +40,38 @@ const IconsBlock = styled.div`
`;
type FilterSetUnitProps = {
- filters: Filter[];
editMode?: boolean;
isApplied?: boolean;
filterSet?: FilterSet;
filterSetName?: string;
- dataMaskApplied?: DataMaskUnit;
+ dataMaskSelected?: DataMaskUnit;
setFilterSetName?: (name: string) => void;
onDelete?: HandlerFunction;
onEdit?: HandlerFunction;
+ onInvalidate?: HandlerFunction;
};
const FilterSetUnit: FC<FilterSetUnitProps> = ({
- filters,
editMode,
setFilterSetName,
onDelete,
onEdit,
filterSetName,
- dataMaskApplied,
+ dataMaskSelected,
filterSet,
isApplied,
+ onInvalidate,
}) => {
const menu = (
<Menu>
<Menu.Item onClick={onEdit}>{t('Edit')}</Menu.Item>
+ <Menu.Item onClick={onInvalidate}>{t('Invalidate')}</Menu.Item>
Review comment:
I think "Invalidate" might be a misleading name - I'd propose removing
this option if the metadata is ok, and calling it "Rebuild", "Refresh" or
something similar with a tooltip that explains that it will remove invalid
filters/settings.
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FiltersHeader.tsx
##########
@@ -53,45 +55,74 @@ const StyledCollapse = styled(Collapse)`
`;
type FiltersHeaderProps = {
- filters: Filter[];
dataMask?: DataMaskUnit;
- expanded: boolean;
+ filterSet?: FilterSet;
};
-const FiltersHeader: FC<FiltersHeaderProps> = ({
- filters,
- dataMask,
- expanded,
-}) => {
+const FiltersHeader: FC<FiltersHeaderProps> = ({ dataMask, filterSet }) => {
+ const filters = useFilters();
+ const filterValues = Object.values(filters);
+
+ let resultFilters = filterValues ?? [];
+ if (filterSet?.nativeFilters) {
+ resultFilters = Object.values(filterSet?.nativeFilters);
+ }
+
const getFiltersHeader = () => (
<FilterHeader>
<Typography.Text type="secondary">
- {t('Filters (%d)', filters.length)}
+ {t('Filters (%d)', resultFilters.length)}
</Typography.Text>
</FilterHeader>
);
+
+ const getFilterRow = ({ id, name }: { id: string; name: string }) => {
+ const changedFilter =
+ filterSet &&
+ !areObjectsEqual(filters[id], filterSet?.nativeFilters?.[id]);
+ const nonExistedFilter = !Object.keys(filters).includes(id);
Review comment:
nit: `removedFilter` or `nonExistentFilter` might be clearer
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FilterSets.tsx
##########
@@ -133,21 +144,59 @@ const FilterSets: React.FC<FilterSetsProps> = ({
);
};
+ const handleInvalidate = (id: string) => {
+ const filtersSet = filterSets[id];
+ // We need remove invalid filters from filter set
+ const newFilters = Object.values(filtersSet?.dataMask?.nativeFilters ?? [])
+ .filter(dataMask => {
+ const { id } = dataMask as MaskWithId;
+ // if we have extra filters in filter set don't add them to selected
data mask || if we have filters with changed metadata not apply them
+ return !(
+ !filterValues.find(filter => filter?.id === id) ||
+ !areObjectsEqual(filters[id], filtersSet?.nativeFilters?.[id])
+ );
Review comment:
Could we move this to a helper function to avoid duplication (same logic
used on lines 133-136).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
