villebro commented on a change in pull request #13576:
URL: https://github.com/apache/superset/pull/13576#discussion_r594254930
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FilterSetUnit.tsx
##########
@@ -65,7 +66,11 @@ const FilterSetUnit: FC<FilterSetUnitProps> = ({
const menu = (
<Menu>
<Menu.Item onClick={onEdit}>{t('Edit')}</Menu.Item>
- <Menu.Item onClick={onInvalidate}>{t('Invalidate')}</Menu.Item>
+ <Menu.Item onClick={onInvalidate}>
Review comment:
Can we rename the handler to `onRebuild`
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FilterSetUnit.tsx
##########
@@ -65,7 +66,11 @@ const FilterSetUnit: FC<FilterSetUnitProps> = ({
const menu = (
<Menu>
<Menu.Item onClick={onEdit}>{t('Edit')}</Menu.Item>
- <Menu.Item onClick={onInvalidate}>{t('Invalidate')}</Menu.Item>
+ <Menu.Item onClick={onInvalidate}>
+ <Tooltip placement="right" title={t('It will remove invalid filters')}>
Review comment:
I think "Remove invalid filters" is sufficient
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FilterSets.tsx
##########
@@ -125,15 +129,14 @@ const FilterSets: React.FC<FilterSetsProps> = ({
if (!id) {
return;
}
+
const filtersSet = filterSets[id];
+
Review comment:
bycatch: let's rename this to `filterSet`
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FilterSets.tsx
##########
@@ -151,10 +154,7 @@ const FilterSets: React.FC<FilterSetsProps> = ({
.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 !(
Review comment:
also remove the comment above
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FilterSets.tsx
##########
@@ -125,15 +129,14 @@ const FilterSets: React.FC<FilterSetsProps> = ({
if (!id) {
return;
}
+
const filtersSet = filterSets[id];
+
Object.values(filtersSet?.dataMask?.nativeFilters ?? []).forEach(
dataMask => {
const { extraFormData, currentState, 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
- if (
Review comment:
I think we can remove the comment on the line above (the helper seems
fairly self-explanatory) - alternatively rename the helper to something so
obvious the comment won't be needed (`isFilterMissingOrContainsInvalidMetadata`)
----------------------------------------------------------------
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]
