From 24993fe588ff915833f9a49e11bb3d5160bed032 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 25 Jan 2024 17:08:54 -0500 Subject: [PATCH 1/6] Dsiable pop-up when tenancy is not enabled or security backend has default tenant set Signed-off-by: Derek Ho --- public/apps/account/account-app.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/public/apps/account/account-app.tsx b/public/apps/account/account-app.tsx index ebcd15d5a..81a81b08f 100644 --- a/public/apps/account/account-app.tsx +++ b/public/apps/account/account-app.tsx @@ -28,6 +28,7 @@ import { } from '../../utils/storage-utils'; import { constructErrorMessageAndLog } from '../error-utils'; import { fetchCurrentAuthType } from '../../utils/logout-utils'; +import { getDashboardsInfoSafe } from '../../utils/dashboards-info-utils'; function tenantSpecifiedInUrl() { return ( @@ -71,7 +72,14 @@ export async function setupTopNavButton(coreStart: CoreStart, config: ClientConf let shouldShowTenantPopup = true; - if (tenantSpecifiedInUrl() || getShouldShowTenantPopup() === false) { + const dashboardsInfo = await getDashboardsInfoSafe(coreStart.http); + const multitenancyEnabled = dashboardsInfo?.multitenancy_enabled && config.multitenancy.enabled; + const defaultTenantSet = dashboardsInfo?.default_tenant !== '' + console.log(dashboardsInfo) + console.log(multitenancyEnabled) + console.log(defaultTenantSet) +; + if (tenantSpecifiedInUrl() || getShouldShowTenantPopup() === false || !multitenancyEnabled || defaultTenantSet ) { shouldShowTenantPopup = false; } else { // Switch to previous tenant based on localStorage, it may fail due to From fec47347a4c17ab1ead7d7b604efda4ee27bd7a6 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 25 Jan 2024 17:09:29 -0500 Subject: [PATCH 2/6] Remove console logs Signed-off-by: Derek Ho --- public/apps/account/account-app.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/public/apps/account/account-app.tsx b/public/apps/account/account-app.tsx index 81a81b08f..8dc9f0c50 100644 --- a/public/apps/account/account-app.tsx +++ b/public/apps/account/account-app.tsx @@ -75,9 +75,6 @@ export async function setupTopNavButton(coreStart: CoreStart, config: ClientConf const dashboardsInfo = await getDashboardsInfoSafe(coreStart.http); const multitenancyEnabled = dashboardsInfo?.multitenancy_enabled && config.multitenancy.enabled; const defaultTenantSet = dashboardsInfo?.default_tenant !== '' - console.log(dashboardsInfo) - console.log(multitenancyEnabled) - console.log(defaultTenantSet) ; if (tenantSpecifiedInUrl() || getShouldShowTenantPopup() === false || !multitenancyEnabled || defaultTenantSet ) { shouldShowTenantPopup = false; From 099e5ce0d3173433b4c6298d36a031f8dcd7f2da Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 25 Jan 2024 17:21:16 -0500 Subject: [PATCH 3/6] Fix lint issues Signed-off-by: Derek Ho --- public/apps/account/account-app.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/public/apps/account/account-app.tsx b/public/apps/account/account-app.tsx index 8dc9f0c50..879eb5a93 100644 --- a/public/apps/account/account-app.tsx +++ b/public/apps/account/account-app.tsx @@ -74,9 +74,13 @@ export async function setupTopNavButton(coreStart: CoreStart, config: ClientConf const dashboardsInfo = await getDashboardsInfoSafe(coreStart.http); const multitenancyEnabled = dashboardsInfo?.multitenancy_enabled && config.multitenancy.enabled; - const defaultTenantSet = dashboardsInfo?.default_tenant !== '' -; - if (tenantSpecifiedInUrl() || getShouldShowTenantPopup() === false || !multitenancyEnabled || defaultTenantSet ) { + const defaultTenantSet = dashboardsInfo?.default_tenant !== ''; + if ( + tenantSpecifiedInUrl() || + getShouldShowTenantPopup() === false || + !multitenancyEnabled || + defaultTenantSet + ) { shouldShowTenantPopup = false; } else { // Switch to previous tenant based on localStorage, it may fail due to From 602a2bfd9afa0ff9dbf031f332d74471e0539db3 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 25 Jan 2024 17:59:27 -0500 Subject: [PATCH 4/6] Add a test proving it fixes the above scenarios Signed-off-by: Derek Ho --- public/apps/account/test/account-app.test.tsx | 68 ++++++++++++++++--- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/public/apps/account/test/account-app.test.tsx b/public/apps/account/test/account-app.test.tsx index 21e61d375..3c7e1a744 100644 --- a/public/apps/account/test/account-app.test.tsx +++ b/public/apps/account/test/account-app.test.tsx @@ -13,17 +13,12 @@ * permissions and limitations under the License. */ -import { shallow } from 'enzyme'; -import React from 'react'; import { setupTopNavButton } from '../account-app'; -import { - getShouldShowTenantPopup, - setShouldShowTenantPopup, - getSavedTenant, -} from '../../../utils/storage-utils'; +import { setShouldShowTenantPopup, getSavedTenant } from '../../../utils/storage-utils'; import { fetchAccountInfoSafe } from '../utils'; import { fetchCurrentAuthType } from '../../../utils/logout-utils'; -import { fetchCurrentTenant, selectTenant } from '../../configuration/utils/tenant-utils'; +import { fetchCurrentTenant } from '../../configuration/utils/tenant-utils'; +import { getDashboardsInfoSafe } from '../../../utils/dashboards-info-utils'; jest.mock('../../../utils/storage-utils', () => ({ getShouldShowTenantPopup: jest.fn(), @@ -35,6 +30,10 @@ jest.mock('../utils', () => ({ fetchAccountInfoSafe: jest.fn(), })); +jest.mock('../../../utils/dashboards-info-utils.tsx', () => ({ + getDashboardsInfoSafe: jest.fn(), +})); + jest.mock('../../../utils/logout-utils', () => ({ fetchCurrentAuthType: jest.fn(), })); @@ -56,6 +55,7 @@ describe('Account app', () => { const mockConfig = { multitenancy: { enable_aggregation_view: true, + enabled: true, }, }; @@ -67,12 +67,18 @@ describe('Account app', () => { }, }; + const mockDashboardsInfo = { + default_tenant: '', + multitenancy_enabled: true, + }; + const mockTenant = 'test1'; beforeAll(() => { (fetchAccountInfoSafe as jest.Mock).mockResolvedValue(mockAccountInfo); (fetchCurrentAuthType as jest.Mock).mockResolvedValue('dummy'); (fetchCurrentTenant as jest.Mock).mockResolvedValue(mockTenant); + (getDashboardsInfoSafe as jest.Mock).mockResolvedValue(mockDashboardsInfo); }); it('Should skip if auto swich if securitytenant in url', (done) => { @@ -116,4 +122,50 @@ describe('Account app', () => { done(); }); }); + + it('Should not show tenant selection popup when multitenancy is disabled from security index', (done) => { + (getDashboardsInfoSafe as jest.Mock).mockResolvedValueOnce({ + default_tenant: '', + multitenancy_enabled: false, + }); + + setupTopNavButton(mockCoreStart, mockConfig as any); + + process.nextTick(() => { + expect(getSavedTenant).toBeCalledTimes(0); + expect(setShouldShowTenantPopup).toBeCalledWith(false); + done(); + }); + }); + + it('Should not show tenant selection popup when multitenancy is disabled dynamically', (done) => { + const multiTenancyDisabledConfig = { + multitenancy: { + enable_aggregation_view: true, + enabled: false, + }, + }; + setupTopNavButton(mockCoreStart, multiTenancyDisabledConfig as any); + + process.nextTick(() => { + expect(getSavedTenant).toBeCalledTimes(0); + expect(setShouldShowTenantPopup).toBeCalledWith(false); + done(); + }); + }); + + it('Should not show tenant selection popup when default tenant is set from security index', (done) => { + (getDashboardsInfoSafe as jest.Mock).mockResolvedValueOnce({ + default_tenant: 'Global', + multitenancy_enabled: true, + }); + + setupTopNavButton(mockCoreStart, mockConfig as any); + + process.nextTick(() => { + expect(getSavedTenant).toBeCalledTimes(0); + expect(setShouldShowTenantPopup).toBeCalledWith(false); + done(); + }); + }); }); From 2e44a083f12977721f643bebb4d00fe69d0918cd Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 29 Jan 2024 16:33:55 -0500 Subject: [PATCH 5/6] Remove unecessary logic and fix test Signed-off-by: Derek Ho --- public/apps/account/account-nav-button.tsx | 2 +- public/apps/account/test/account-nav-button.test.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/public/apps/account/account-nav-button.tsx b/public/apps/account/account-nav-button.tsx index 02e04f3af..486316695 100644 --- a/public/apps/account/account-nav-button.tsx +++ b/public/apps/account/account-nav-button.tsx @@ -85,7 +85,7 @@ export function AccountNavButton(props: { }, [props.coreStart.http]); // Check if the tenant modal should be shown on load - if (isMultiTenancyEnabled && getShouldShowTenantPopup() && props.config.multitenancy.enabled) { + if (getShouldShowTenantPopup()) { setShouldShowTenantPopup(false); showTenantSwitchPanel(); } diff --git a/public/apps/account/test/account-nav-button.test.tsx b/public/apps/account/test/account-nav-button.test.tsx index ff20b0f25..27f2fb7c0 100644 --- a/public/apps/account/test/account-nav-button.test.tsx +++ b/public/apps/account/test/account-nav-button.test.tsx @@ -153,7 +153,7 @@ describe('Account navigation button, multitenancy disabled', () => { jest.clearAllMocks(); }); - it('should not set modal when show popup is true', () => { + it('should not set modal when show popup is false', () => { (getDashboardsInfo as jest.Mock).mockImplementation(() => { return { multitenancy_enabled: false, @@ -161,7 +161,7 @@ describe('Account navigation button, multitenancy disabled', () => { default_tenant: '', }; }); - (getShouldShowTenantPopup as jest.Mock).mockReturnValueOnce(true); + (getShouldShowTenantPopup as jest.Mock).mockReturnValueOnce(false); shallow( Date: Tue, 30 Jan 2024 09:11:18 -0500 Subject: [PATCH 6/6] Remove line and change test name Signed-off-by: Derek Ho --- public/apps/account/test/account-nav-button.test.tsx | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/public/apps/account/test/account-nav-button.test.tsx b/public/apps/account/test/account-nav-button.test.tsx index 27f2fb7c0..c6d3aab40 100644 --- a/public/apps/account/test/account-nav-button.test.tsx +++ b/public/apps/account/test/account-nav-button.test.tsx @@ -153,14 +153,7 @@ describe('Account navigation button, multitenancy disabled', () => { jest.clearAllMocks(); }); - it('should not set modal when show popup is false', () => { - (getDashboardsInfo as jest.Mock).mockImplementation(() => { - return { - multitenancy_enabled: false, - private_tenant_enabled: false, - default_tenant: '', - }; - }); + it('should not set modal when getShouldShowTenantPopup is false', () => { (getShouldShowTenantPopup as jest.Mock).mockReturnValueOnce(false); shallow(