graceguo-supercat commented on a change in pull request #14933:
URL: https://github.com/apache/superset/pull/14933#discussion_r650331375
##########
File path:
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
##########
@@ -39,17 +43,47 @@ const DashboardContainer: FC<DashboardContainerProps> = ({
topLevelTabs }) => {
const dashboardLayout = useSelector<RootState, DashboardLayout>(
state => state.dashboardLayout.present,
);
+ const nativeFilters = useSelector<RootState, Filters>(
+ state => state.nativeFilters.filters,
+ );
const directPathToChild = useSelector<RootState, string[]>(
state => state.dashboardState.directPathToChild,
);
const [tabIndex, setTabIndex] = useState(
getRootLevelTabIndex(dashboardLayout, directPathToChild),
);
+ const dispatch = useDispatch();
+
useEffect(() => {
setTabIndex(getRootLevelTabIndex(dashboardLayout, directPathToChild));
}, [getLeafComponentIdFromPath(directPathToChild)]);
+ // recalculate charts and tabs in scopes of native filters only when a scope
or dashboard layout changes
+ const nativeFiltersValues = Object.values(nativeFilters);
+ const scopes = nativeFiltersValues.map(filter => filter.scope);
+ useEffect(() => {
+ nativeFiltersValues.forEach(filter => {
+ const filterScope = filter.scope;
+ const chartsInScope = getChartIdsInFilterScope({
+ filterScope: {
+ scope: filterScope.rootPath,
+ // @ts-ignore
+ immune: filterScope.excluded,
+ },
+ });
+ const tabsInScope = findTabsWithChartsInScope(
+ dashboardLayout,
+ chartsInScope,
+ );
+ Object.assign(filter, {
+ chartsInScope,
+ tabsInScope: Array.from(tabsInScope),
+ });
+ });
+ dispatch(setFilterConfiguration(nativeFiltersValues));
Review comment:
@kgabryje This update function is called every time user **_open_** a
dashboard without any change. This is not a correct behavior. cc @junlincc
##########
File path:
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
##########
@@ -39,17 +43,47 @@ const DashboardContainer: FC<DashboardContainerProps> = ({
topLevelTabs }) => {
const dashboardLayout = useSelector<RootState, DashboardLayout>(
state => state.dashboardLayout.present,
);
+ const nativeFilters = useSelector<RootState, Filters>(
+ state => state.nativeFilters.filters,
+ );
const directPathToChild = useSelector<RootState, string[]>(
state => state.dashboardState.directPathToChild,
);
const [tabIndex, setTabIndex] = useState(
getRootLevelTabIndex(dashboardLayout, directPathToChild),
);
+ const dispatch = useDispatch();
+
useEffect(() => {
setTabIndex(getRootLevelTabIndex(dashboardLayout, directPathToChild));
}, [getLeafComponentIdFromPath(directPathToChild)]);
+ // recalculate charts and tabs in scopes of native filters only when a scope
or dashboard layout changes
+ const nativeFiltersValues = Object.values(nativeFilters);
+ const scopes = nativeFiltersValues.map(filter => filter.scope);
+ useEffect(() => {
+ nativeFiltersValues.forEach(filter => {
+ const filterScope = filter.scope;
+ const chartsInScope = getChartIdsInFilterScope({
+ filterScope: {
+ scope: filterScope.rootPath,
+ // @ts-ignore
+ immune: filterScope.excluded,
+ },
+ });
+ const tabsInScope = findTabsWithChartsInScope(
+ dashboardLayout,
+ chartsInScope,
+ );
+ Object.assign(filter, {
+ chartsInScope,
+ tabsInScope: Array.from(tabsInScope),
+ });
+ });
+ dispatch(setFilterConfiguration(nativeFiltersValues));
Review comment:
@kgabryje This update function is **_called every time_** user
**_open_** a dashboard without any change. This is not a correct behavior. cc
@junlincc
##########
File path:
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
##########
@@ -39,17 +43,47 @@ const DashboardContainer: FC<DashboardContainerProps> = ({
topLevelTabs }) => {
const dashboardLayout = useSelector<RootState, DashboardLayout>(
state => state.dashboardLayout.present,
);
+ const nativeFilters = useSelector<RootState, Filters>(
+ state => state.nativeFilters.filters,
+ );
const directPathToChild = useSelector<RootState, string[]>(
state => state.dashboardState.directPathToChild,
);
const [tabIndex, setTabIndex] = useState(
getRootLevelTabIndex(dashboardLayout, directPathToChild),
);
+ const dispatch = useDispatch();
+
useEffect(() => {
setTabIndex(getRootLevelTabIndex(dashboardLayout, directPathToChild));
}, [getLeafComponentIdFromPath(directPathToChild)]);
+ // recalculate charts and tabs in scopes of native filters only when a scope
or dashboard layout changes
+ const nativeFiltersValues = Object.values(nativeFilters);
+ const scopes = nativeFiltersValues.map(filter => filter.scope);
+ useEffect(() => {
+ nativeFiltersValues.forEach(filter => {
+ const filterScope = filter.scope;
+ const chartsInScope = getChartIdsInFilterScope({
+ filterScope: {
+ scope: filterScope.rootPath,
+ // @ts-ignore
+ immune: filterScope.excluded,
+ },
+ });
+ const tabsInScope = findTabsWithChartsInScope(
+ dashboardLayout,
+ chartsInScope,
+ );
+ Object.assign(filter, {
+ chartsInScope,
+ tabsInScope: Array.from(tabsInScope),
+ });
+ });
+ dispatch(setFilterConfiguration(nativeFiltersValues));
Review comment:
@kgabryje This update function is **_called every time_** a user
**_opens_** a dashboard without any change. This is not a correct behavior. cc
@junlincc
##########
File path:
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
##########
@@ -39,17 +43,47 @@ const DashboardContainer: FC<DashboardContainerProps> = ({
topLevelTabs }) => {
const dashboardLayout = useSelector<RootState, DashboardLayout>(
state => state.dashboardLayout.present,
);
+ const nativeFilters = useSelector<RootState, Filters>(
+ state => state.nativeFilters.filters,
+ );
const directPathToChild = useSelector<RootState, string[]>(
state => state.dashboardState.directPathToChild,
);
const [tabIndex, setTabIndex] = useState(
getRootLevelTabIndex(dashboardLayout, directPathToChild),
);
+ const dispatch = useDispatch();
+
useEffect(() => {
setTabIndex(getRootLevelTabIndex(dashboardLayout, directPathToChild));
}, [getLeafComponentIdFromPath(directPathToChild)]);
+ // recalculate charts and tabs in scopes of native filters only when a scope
or dashboard layout changes
+ const nativeFiltersValues = Object.values(nativeFilters);
+ const scopes = nativeFiltersValues.map(filter => filter.scope);
+ useEffect(() => {
+ nativeFiltersValues.forEach(filter => {
+ const filterScope = filter.scope;
+ const chartsInScope = getChartIdsInFilterScope({
+ filterScope: {
+ scope: filterScope.rootPath,
+ // @ts-ignore
+ immune: filterScope.excluded,
+ },
+ });
+ const tabsInScope = findTabsWithChartsInScope(
+ dashboardLayout,
+ chartsInScope,
+ );
+ Object.assign(filter, {
+ chartsInScope,
+ tabsInScope: Array.from(tabsInScope),
+ });
+ });
+ dispatch(setFilterConfiguration(nativeFiltersValues));
Review comment:
@kgabryje This update function is **_called every time_** a user
**_opens_** a dashboard, even without any change. This is not a correct
behavior. cc @junlincc
##########
File path:
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
##########
@@ -39,17 +43,47 @@ const DashboardContainer: FC<DashboardContainerProps> = ({
topLevelTabs }) => {
const dashboardLayout = useSelector<RootState, DashboardLayout>(
state => state.dashboardLayout.present,
);
+ const nativeFilters = useSelector<RootState, Filters>(
+ state => state.nativeFilters.filters,
+ );
const directPathToChild = useSelector<RootState, string[]>(
state => state.dashboardState.directPathToChild,
);
const [tabIndex, setTabIndex] = useState(
getRootLevelTabIndex(dashboardLayout, directPathToChild),
);
+ const dispatch = useDispatch();
+
useEffect(() => {
setTabIndex(getRootLevelTabIndex(dashboardLayout, directPathToChild));
}, [getLeafComponentIdFromPath(directPathToChild)]);
+ // recalculate charts and tabs in scopes of native filters only when a scope
or dashboard layout changes
+ const nativeFiltersValues = Object.values(nativeFilters);
+ const scopes = nativeFiltersValues.map(filter => filter.scope);
+ useEffect(() => {
+ nativeFiltersValues.forEach(filter => {
+ const filterScope = filter.scope;
+ const chartsInScope = getChartIdsInFilterScope({
+ filterScope: {
+ scope: filterScope.rootPath,
+ // @ts-ignore
+ immune: filterScope.excluded,
+ },
+ });
+ const tabsInScope = findTabsWithChartsInScope(
+ dashboardLayout,
+ chartsInScope,
+ );
+ Object.assign(filter, {
+ chartsInScope,
+ tabsInScope: Array.from(tabsInScope),
+ });
+ });
+ dispatch(setFilterConfiguration(nativeFiltersValues));
Review comment:
@kgabryje This update function is **_called every time_** a user
**_opens_** a dashboard, even without any change. This is not a correct
behavior.
And, update dashboard should check user permission and get user confirm.
This call is secretly updated dashboard json_metadata without user confirm.
This will cause serious issue. Please fix asap. Otherwise this PR should be
reverted.
cc @junlincc
##########
File path:
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
##########
@@ -39,17 +43,47 @@ const DashboardContainer: FC<DashboardContainerProps> = ({
topLevelTabs }) => {
const dashboardLayout = useSelector<RootState, DashboardLayout>(
state => state.dashboardLayout.present,
);
+ const nativeFilters = useSelector<RootState, Filters>(
+ state => state.nativeFilters.filters,
+ );
const directPathToChild = useSelector<RootState, string[]>(
state => state.dashboardState.directPathToChild,
);
const [tabIndex, setTabIndex] = useState(
getRootLevelTabIndex(dashboardLayout, directPathToChild),
);
+ const dispatch = useDispatch();
+
useEffect(() => {
setTabIndex(getRootLevelTabIndex(dashboardLayout, directPathToChild));
}, [getLeafComponentIdFromPath(directPathToChild)]);
+ // recalculate charts and tabs in scopes of native filters only when a scope
or dashboard layout changes
+ const nativeFiltersValues = Object.values(nativeFilters);
+ const scopes = nativeFiltersValues.map(filter => filter.scope);
+ useEffect(() => {
+ nativeFiltersValues.forEach(filter => {
+ const filterScope = filter.scope;
+ const chartsInScope = getChartIdsInFilterScope({
+ filterScope: {
+ scope: filterScope.rootPath,
+ // @ts-ignore
+ immune: filterScope.excluded,
+ },
+ });
+ const tabsInScope = findTabsWithChartsInScope(
+ dashboardLayout,
+ chartsInScope,
+ );
+ Object.assign(filter, {
+ chartsInScope,
+ tabsInScope: Array.from(tabsInScope),
+ });
+ });
+ dispatch(setFilterConfiguration(nativeFiltersValues));
Review comment:
@kgabryje This update function is **_called every time_** a user
**_opens_** a dashboard, even without any change. This is not a correct
behavior.
And, update dashboard should check user permission and get user confirm.
This call is secretly changed dashboard json_metadata without user confirm.
This will cause serious issue. Please fix asap. Otherwise this PR should be
reverted.
cc @junlincc
--
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
