From a31f6e6c87f0da7d3935d9dab7f6c7f91c4cce1c Mon Sep 17 00:00:00 2001 From: Vitor Avila Date: Thu, 4 Jul 2024 13:26:20 -0300 Subject: [PATCH 1/4] chore(Home): Avoid firing API requests when a custom Home is used --- superset-frontend/src/pages/Home/index.tsx | 173 +++++++++++---------- 1 file changed, 92 insertions(+), 81 deletions(-) diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx index 5661451a42df..35e023d353cc 100644 --- a/superset-frontend/src/pages/Home/index.tsx +++ b/superset-frontend/src/pages/Home/index.tsx @@ -217,89 +217,100 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { ]; }, []); - useEffect(() => { - if (!otherTabFilters) { - return; - } - const activeTab = getItem(LocalStorageKeys.HomepageActivityFilter, null); - setActiveState(collapseState.length > 0 ? collapseState : DEFAULT_TAB_ARR); - getRecentActivityObjs(user.userId!, recent, addDangerToast, otherTabFilters) - .then(res => { - const data: ActivityData | null = {}; - data[TableTab.Other] = res.other; - if (res.viewed) { - const filtered = reject(res.viewed, ['item_url', null]).map(r => r); - data[TableTab.Viewed] = filtered; - if (!activeTab && data[TableTab.Viewed]) { - setActiveChild(TableTab.Viewed); - } else if (!activeTab && !data[TableTab.Viewed]) { - setActiveChild(TableTab.Created); - } else setActiveChild(activeTab || TableTab.Created); - } else if (!activeTab) setActiveChild(TableTab.Created); - else setActiveChild(activeTab); - setActivityData(activityData => ({ ...activityData, ...data })); - }) - .catch( - createErrorHandler((errMsg: unknown) => { - setActivityData(activityData => ({ - ...activityData, - [TableTab.Viewed]: [], - })); - addDangerToast( - t('There was an issue fetching your recent activity: %s', errMsg), - ); - }), + if (!WelcomeTopExtension && !WelcomeMainExtension) { + useEffect(() => { + if (!otherTabFilters) { + return; + } + const activeTab = getItem(LocalStorageKeys.HomepageActivityFilter, null); + setActiveState( + collapseState.length > 0 ? collapseState : DEFAULT_TAB_ARR, ); - - // Sets other activity data in parallel with recents api call - const ownSavedQueryFilters = [ - { - col: 'created_by', - opr: 'rel_o_m', - value: `${id}`, - }, - ]; - Promise.all([ - getUserOwnedObjects(id, 'dashboard') - .then(r => { - setDashboardData(r); - return Promise.resolve(); + getRecentActivityObjs( + user.userId!, + recent, + addDangerToast, + otherTabFilters, + ) + .then(res => { + const data: ActivityData | null = {}; + data[TableTab.Other] = res.other; + if (res.viewed) { + const filtered = reject(res.viewed, ['item_url', null]).map(r => r); + data[TableTab.Viewed] = filtered; + if (!activeTab && data[TableTab.Viewed]) { + setActiveChild(TableTab.Viewed); + } else if (!activeTab && !data[TableTab.Viewed]) { + setActiveChild(TableTab.Created); + } else setActiveChild(activeTab || TableTab.Created); + } else if (!activeTab) setActiveChild(TableTab.Created); + else setActiveChild(activeTab); + setActivityData(activityData => ({ ...activityData, ...data })); }) - .catch((err: unknown) => { - setDashboardData([]); - addDangerToast( - t('There was an issue fetching your dashboards: %s', err), - ); - return Promise.resolve(); - }), - getUserOwnedObjects(id, 'chart') - .then(r => { - setChartData(r); - return Promise.resolve(); - }) - .catch((err: unknown) => { - setChartData([]); - addDangerToast(t('There was an issue fetching your chart: %s', err)); - return Promise.resolve(); - }), - canReadSavedQueries - ? getUserOwnedObjects(id, 'saved_query', ownSavedQueryFilters) - .then(r => { - setQueryData(r); - return Promise.resolve(); - }) - .catch((err: unknown) => { - setQueryData([]); - addDangerToast( - t('There was an issue fetching your saved queries: %s', err), - ); - return Promise.resolve(); - }) - : Promise.resolve(), - ]).then(() => { - setIsFetchingActivityData(false); - }); - }, [otherTabFilters]); + .catch( + createErrorHandler((errMsg: unknown) => { + setActivityData(activityData => ({ + ...activityData, + [TableTab.Viewed]: [], + })); + addDangerToast( + t('There was an issue fetching your recent activity: %s', errMsg), + ); + }), + ); + + // Sets other activity data in parallel with recents api call + const ownSavedQueryFilters = [ + { + col: 'created_by', + opr: 'rel_o_m', + value: `${id}`, + }, + ]; + Promise.all([ + getUserOwnedObjects(id, 'dashboard') + .then(r => { + setDashboardData(r); + return Promise.resolve(); + }) + .catch((err: unknown) => { + setDashboardData([]); + addDangerToast( + t('There was an issue fetching your dashboards: %s', err), + ); + return Promise.resolve(); + }), + getUserOwnedObjects(id, 'chart') + .then(r => { + setChartData(r); + return Promise.resolve(); + }) + .catch((err: unknown) => { + setChartData([]); + addDangerToast( + t('There was an issue fetching your chart: %s', err), + ); + return Promise.resolve(); + }), + canReadSavedQueries + ? getUserOwnedObjects(id, 'saved_query', ownSavedQueryFilters) + .then(r => { + setQueryData(r); + return Promise.resolve(); + }) + .catch((err: unknown) => { + setQueryData([]); + addDangerToast( + t('There was an issue fetching your saved queries: %s', err), + ); + return Promise.resolve(); + }) + : Promise.resolve(), + ]).then(() => { + setIsFetchingActivityData(false); + }); + }, [otherTabFilters]); + } const handleToggle = () => { setChecked(!checked); From 2cfc4141c5ebfd0faccdce13a6de2df5ad3fa72c Mon Sep 17 00:00:00 2001 From: Vitor Avila Date: Thu, 4 Jul 2024 15:49:49 -0300 Subject: [PATCH 2/4] chore(Home): Avoid firing API requests when a custom Home is used --- superset-frontend/src/pages/Home/index.tsx | 173 ++++++++++----------- 1 file changed, 81 insertions(+), 92 deletions(-) diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx index 35e023d353cc..56ff1f6f8c7a 100644 --- a/superset-frontend/src/pages/Home/index.tsx +++ b/superset-frontend/src/pages/Home/index.tsx @@ -217,100 +217,89 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { ]; }, []); - if (!WelcomeTopExtension && !WelcomeMainExtension) { - useEffect(() => { - if (!otherTabFilters) { - return; - } - const activeTab = getItem(LocalStorageKeys.HomepageActivityFilter, null); - setActiveState( - collapseState.length > 0 ? collapseState : DEFAULT_TAB_ARR, + useEffect(() => { + if (!otherTabFilters || (WelcomeTopExtension && WelcomeMainExtension)) { + return; + } + const activeTab = getItem(LocalStorageKeys.HomepageActivityFilter, null); + setActiveState(collapseState.length > 0 ? collapseState : DEFAULT_TAB_ARR); + getRecentActivityObjs(user.userId!, recent, addDangerToast, otherTabFilters) + .then(res => { + const data: ActivityData | null = {}; + data[TableTab.Other] = res.other; + if (res.viewed) { + const filtered = reject(res.viewed, ['item_url', null]).map(r => r); + data[TableTab.Viewed] = filtered; + if (!activeTab && data[TableTab.Viewed]) { + setActiveChild(TableTab.Viewed); + } else if (!activeTab && !data[TableTab.Viewed]) { + setActiveChild(TableTab.Created); + } else setActiveChild(activeTab || TableTab.Created); + } else if (!activeTab) setActiveChild(TableTab.Created); + else setActiveChild(activeTab); + setActivityData(activityData => ({ ...activityData, ...data })); + }) + .catch( + createErrorHandler((errMsg: unknown) => { + setActivityData(activityData => ({ + ...activityData, + [TableTab.Viewed]: [], + })); + addDangerToast( + t('There was an issue fetching your recent activity: %s', errMsg), + ); + }), ); - getRecentActivityObjs( - user.userId!, - recent, - addDangerToast, - otherTabFilters, - ) - .then(res => { - const data: ActivityData | null = {}; - data[TableTab.Other] = res.other; - if (res.viewed) { - const filtered = reject(res.viewed, ['item_url', null]).map(r => r); - data[TableTab.Viewed] = filtered; - if (!activeTab && data[TableTab.Viewed]) { - setActiveChild(TableTab.Viewed); - } else if (!activeTab && !data[TableTab.Viewed]) { - setActiveChild(TableTab.Created); - } else setActiveChild(activeTab || TableTab.Created); - } else if (!activeTab) setActiveChild(TableTab.Created); - else setActiveChild(activeTab); - setActivityData(activityData => ({ ...activityData, ...data })); - }) - .catch( - createErrorHandler((errMsg: unknown) => { - setActivityData(activityData => ({ - ...activityData, - [TableTab.Viewed]: [], - })); - addDangerToast( - t('There was an issue fetching your recent activity: %s', errMsg), - ); - }), - ); - // Sets other activity data in parallel with recents api call - const ownSavedQueryFilters = [ - { - col: 'created_by', - opr: 'rel_o_m', - value: `${id}`, - }, - ]; - Promise.all([ - getUserOwnedObjects(id, 'dashboard') - .then(r => { - setDashboardData(r); - return Promise.resolve(); - }) - .catch((err: unknown) => { - setDashboardData([]); - addDangerToast( - t('There was an issue fetching your dashboards: %s', err), - ); - return Promise.resolve(); - }), - getUserOwnedObjects(id, 'chart') - .then(r => { - setChartData(r); - return Promise.resolve(); - }) - .catch((err: unknown) => { - setChartData([]); - addDangerToast( - t('There was an issue fetching your chart: %s', err), - ); - return Promise.resolve(); - }), - canReadSavedQueries - ? getUserOwnedObjects(id, 'saved_query', ownSavedQueryFilters) - .then(r => { - setQueryData(r); - return Promise.resolve(); - }) - .catch((err: unknown) => { - setQueryData([]); - addDangerToast( - t('There was an issue fetching your saved queries: %s', err), - ); - return Promise.resolve(); - }) - : Promise.resolve(), - ]).then(() => { - setIsFetchingActivityData(false); - }); - }, [otherTabFilters]); - } + // Sets other activity data in parallel with recents api call + const ownSavedQueryFilters = [ + { + col: 'created_by', + opr: 'rel_o_m', + value: `${id}`, + }, + ]; + Promise.all([ + getUserOwnedObjects(id, 'dashboard') + .then(r => { + setDashboardData(r); + return Promise.resolve(); + }) + .catch((err: unknown) => { + setDashboardData([]); + addDangerToast( + t('There was an issue fetching your dashboards: %s', err), + ); + return Promise.resolve(); + }), + getUserOwnedObjects(id, 'chart') + .then(r => { + setChartData(r); + return Promise.resolve(); + }) + .catch((err: unknown) => { + setChartData([]); + addDangerToast(t('There was an issue fetching your chart: %s', err)); + return Promise.resolve(); + }), + canReadSavedQueries + ? getUserOwnedObjects(id, 'saved_query', ownSavedQueryFilters) + .then(r => { + setQueryData(r); + return Promise.resolve(); + }) + .catch((err: unknown) => { + setQueryData([]); + addDangerToast( + t('There was an issue fetching your saved queries: %s', err), + ); + return Promise.resolve(); + }) + : Promise.resolve(), + ]).then(() => { + setIsFetchingActivityData(false); + }); + }, [otherTabFilters]); const handleToggle = () => { setChecked(!checked); From f13dabd3eff78ea30db3d20263c0bf4532b723b8 Mon Sep 17 00:00:00 2001 From: Vitor Avila Date: Thu, 4 Jul 2024 16:13:13 -0300 Subject: [PATCH 3/4] Adding tests --- .../src/pages/Home/Home.test.tsx | 23 +++++++++++++++++++ superset-frontend/src/pages/Home/index.tsx | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/pages/Home/Home.test.tsx b/superset-frontend/src/pages/Home/Home.test.tsx index 6be4dc186ed7..10c1960a9c3c 100644 --- a/superset-frontend/src/pages/Home/Home.test.tsx +++ b/superset-frontend/src/pages/Home/Home.test.tsx @@ -228,3 +228,26 @@ test('Should render a submenu extension component if one is supplied', async () expect(screen.getByText('submenu extension')).toBeInTheDocument(); }); + +test('Should not make data fetch calls if `welcome.main.replacement` is defined', async () => { + const extensionsRegistry = getExtensionsRegistry(); + + // Clean up + extensionsRegistry.set('welcome.banner', () => null); + extensionsRegistry.set('welcome.main.replacement', () => ( + <>welcome.main.replacement extension component + )); + + setupExtensions(); + + await renderWelcome(); + + expect( + screen.getByText('welcome.main.replacement extension component'), + ).toBeInTheDocument(); + + expect(fetchMock.calls(chartsEndpoint)).toHaveLength(0); + expect(fetchMock.calls(dashboardsEndpoint)).toHaveLength(0); + expect(fetchMock.calls(recentActivityEndpoint)).toHaveLength(0); + expect(fetchMock.calls(savedQueryEndpoint)).toHaveLength(0); +}); diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx index 56ff1f6f8c7a..b92e84f486dc 100644 --- a/superset-frontend/src/pages/Home/index.tsx +++ b/superset-frontend/src/pages/Home/index.tsx @@ -218,7 +218,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { }, []); useEffect(() => { - if (!otherTabFilters || (WelcomeTopExtension && WelcomeMainExtension)) { + if (!otherTabFilters || WelcomeMainExtension) { return; } const activeTab = getItem(LocalStorageKeys.HomepageActivityFilter, null); From f7070e9be570afb0dfa377efe0ea1b2d5f9ec9c4 Mon Sep 17 00:00:00 2001 From: Vitor Avila Date: Fri, 5 Jul 2024 16:38:23 -0300 Subject: [PATCH 4/4] Addressing PR feedback --- superset-frontend/src/pages/Home/Home.test.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/superset-frontend/src/pages/Home/Home.test.tsx b/superset-frontend/src/pages/Home/Home.test.tsx index 10c1960a9c3c..f470e9ea7380 100644 --- a/superset-frontend/src/pages/Home/Home.test.tsx +++ b/superset-frontend/src/pages/Home/Home.test.tsx @@ -234,6 +234,8 @@ test('Should not make data fetch calls if `welcome.main.replacement` is defined' // Clean up extensionsRegistry.set('welcome.banner', () => null); + + // Set up extensionsRegistry.set('welcome.main.replacement', () => ( <>welcome.main.replacement extension component ));