diff --git a/packages/manager/.changeset/pr-13056-fixed-1762431727811.md b/packages/manager/.changeset/pr-13056-fixed-1762431727811.md new file mode 100644 index 00000000000..ce6999d3ffd --- /dev/null +++ b/packages/manager/.changeset/pr-13056-fixed-1762431727811.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Fixed +--- + +Race condition with Preferences overrides in PrimaryNav ([#13056](https://github.com/linode/manager/pull/13056)) diff --git a/packages/manager/src/components/PrimaryNav/PrimaryNav.test.tsx b/packages/manager/src/components/PrimaryNav/PrimaryNav.test.tsx index c560e1d0815..6036cfccd13 100644 --- a/packages/manager/src/components/PrimaryNav/PrimaryNav.test.tsx +++ b/packages/manager/src/components/PrimaryNav/PrimaryNav.test.tsx @@ -8,7 +8,6 @@ import { renderWithTheme, wrapWithTheme } from 'src/utilities/testHelpers'; import PrimaryNav from './PrimaryNav'; -import type { ManagerPreferences } from '@linode/utilities'; import type { Flags } from 'src/featureFlags'; const props = { @@ -28,6 +27,8 @@ const queryMocks = vi.hoisted(() => ({ isIAMEnabled: false, })), usePreferences: vi.fn().mockReturnValue({}), + useAccount: vi.fn().mockReturnValue({}), + useAccountSettings: vi.fn().mockReturnValue({}), })); vi.mock('src/features/IAM/hooks/useIsIAMEnabled', () => ({ @@ -39,28 +40,42 @@ vi.mock('@linode/queries', async () => { return { ...actual, usePreferences: queryMocks.usePreferences, + useAccount: queryMocks.useAccount, + useAccountSettings: queryMocks.useAccountSettings, }; }); describe('PrimaryNav', () => { - const preference: ManagerPreferences['collapsedSideNavProductFamilies'] = []; + beforeEach(() => { + queryMocks.usePreferences.mockReturnValue({ + data: { + collapsedSideNavProductFamilies: [], + }, + isLoading: false, + error: null, + }); + }); it('only contains a "Managed" menu link if the user has Managed services.', async () => { - server.use( - http.get('*/account/maintenance', () => { - return HttpResponse.json({ managed: false }); - }) - ); + queryMocks.useAccountSettings.mockReturnValue({ + data: { + managed: false, + }, + isLoading: false, + error: null, + }); const { findByTestId, getByTestId, queryByTestId, rerender } = renderWithTheme(, { queryClient }); expect(queryByTestId(queryString)).not.toBeInTheDocument(); - server.use( - http.get('*/account/maintenance', () => { - return HttpResponse.json({ managed: true }); - }) - ); + queryMocks.useAccountSettings.mockReturnValue({ + data: { + managed: true, + }, + isLoading: false, + error: null, + }); rerender(wrapWithTheme(, { queryClient })); @@ -70,6 +85,13 @@ describe('PrimaryNav', () => { }); it('should have aria-current attribute for accessible links', () => { + queryMocks.useAccountSettings.mockReturnValue({ + data: { + managed: true, + }, + isLoading: false, + error: null, + }); const { getByTestId } = renderWithTheme(, { queryClient, }); @@ -78,19 +100,15 @@ describe('PrimaryNav', () => { }); it('should show Databases menu item if the user has the account capability V1', async () => { - queryMocks.usePreferences.mockReturnValue({ - data: preference, - }); - const account = accountFactory.build({ capabilities: ['Managed Databases'], }); - server.use( - http.get('*/account', () => { - return HttpResponse.json(account); - }) - ); + queryMocks.useAccount.mockReturnValue({ + data: account, + isLoading: false, + error: null, + }); const flags: Partial = { dbaasV2: { @@ -99,33 +117,26 @@ describe('PrimaryNav', () => { }, }; - const { findByTestId, queryByTestId } = renderWithTheme( - , - { - flags, - } - ); + renderWithTheme(, { + flags, + }); - const databaseNavItem = await findByTestId('menu-item-Databases'); + const databaseNavItem = screen.getByTestId('menu-item-Databases'); expect(databaseNavItem).toBeVisible(); - expect(queryByTestId('betaChip')).toBeNull(); + expect(screen.queryByTestId('betaChip')).toBeNull(); }); it('should show Databases menu item if the user has the account capability V2 Beta', async () => { - queryMocks.usePreferences.mockReturnValue({ - data: preference, - }); - const account = accountFactory.build({ capabilities: ['Managed Databases Beta'], }); - server.use( - http.get('*/account', () => { - return HttpResponse.json(account); - }) - ); + queryMocks.useAccount.mockReturnValue({ + data: account, + isLoading: false, + error: null, + }); const flags: Partial = { dbaasV2: { @@ -146,19 +157,15 @@ describe('PrimaryNav', () => { }); it('should show Databases menu item if the user has the account capability V2', async () => { - queryMocks.usePreferences.mockReturnValue({ - data: preference, - }); - const account = accountFactory.build({ capabilities: ['Managed Databases'], }); - server.use( - http.get('*/account', () => { - return HttpResponse.json(account); - }) - ); + queryMocks.useAccount.mockReturnValue({ + data: account, + isLoading: false, + error: null, + }); const flags: Partial = { dbaasV2: { @@ -181,19 +188,15 @@ describe('PrimaryNav', () => { }); it('should show Databases menu item if the user has the account capability V2', async () => { - queryMocks.usePreferences.mockReturnValue({ - data: preference, - }); - const account = accountFactory.build({ capabilities: ['Managed Databases Beta'], }); - server.use( - http.get('*/account', () => { - return HttpResponse.json(account); - }) - ); + queryMocks.useAccount.mockReturnValue({ + data: account, + isLoading: false, + error: null, + }); const flags: Partial = { dbaasV2: { @@ -216,11 +219,11 @@ describe('PrimaryNav', () => { capabilities: ['Akamai Cloud Pulse'], }); - server.use( - http.get('*/account', () => { - return HttpResponse.json(account); - }) - ); + queryMocks.useAccount.mockReturnValue({ + data: account, + isLoading: false, + error: null, + }); const flags = { aclp: { diff --git a/packages/manager/src/components/PrimaryNav/PrimaryNav.tsx b/packages/manager/src/components/PrimaryNav/PrimaryNav.tsx index 4c14e98bbf3..867e17878a0 100644 --- a/packages/manager/src/components/PrimaryNav/PrimaryNav.tsx +++ b/packages/manager/src/components/PrimaryNav/PrimaryNav.tsx @@ -96,6 +96,7 @@ export const PrimaryNav = (props: PrimaryNavProps) => { const location = useLocation(); const { data: accountSettings } = useAccountSettings(); + const isManaged = accountSettings?.managed ?? false; const { isACLPEnabled } = useIsACLPEnabled(); @@ -114,16 +115,18 @@ export const PrimaryNav = (props: PrimaryNavProps) => { const { isIAMBeta, isIAMEnabled } = useIsIAMEnabled(); const { - data: collapsedSideNavPreference, + data: preferences, error: preferencesError, isLoading: preferencesLoading, - } = usePreferences( - (preferences) => preferences?.collapsedSideNavProductFamilies - ); + } = usePreferences(); - const collapsedAccordions = collapsedSideNavPreference ?? [ - 1, 2, 3, 4, 5, 6, 7, - ]; // by default, we collapse all categories if no preference is set; + const collapsedSideNavPreference = + preferences?.collapsedSideNavProductFamilies; + + const collapsedAccordions = React.useMemo( + () => collapsedSideNavPreference ?? [1, 2, 3, 4, 5, 6, 7], // by default, we collapse all categories if no preference is set; + [collapsedSideNavPreference] + ); const { mutateAsync: updatePreferences } = useMutatePreferences(); @@ -337,22 +340,22 @@ export const PrimaryNav = (props: PrimaryNavProps) => { ] ); - const accordionClicked = (index: number) => { - let updatedCollapsedAccordions: number[] = [1, 2, 3, 4, 5, 6, 7]; - if (collapsedAccordions.includes(index)) { - updatedCollapsedAccordions = collapsedAccordions.filter( - (accIndex) => accIndex !== index - ); - updatePreferences({ - collapsedSideNavProductFamilies: updatedCollapsedAccordions, - }); - } else { - updatedCollapsedAccordions = [...collapsedAccordions, index]; + const accordionClicked = React.useCallback( + (index: number) => { + let updatedCollapsedAccordions: number[]; + if (collapsedAccordions.includes(index)) { + updatedCollapsedAccordions = collapsedAccordions.filter( + (accIndex) => accIndex !== index + ); + } else { + updatedCollapsedAccordions = [...collapsedAccordions, index]; + } updatePreferences({ collapsedSideNavProductFamilies: updatedCollapsedAccordions, }); - } - }; + }, + [collapsedAccordions, updatePreferences] + ); const checkOverflow = React.useCallback(() => { if (navItemsRef.current && primaryNavRef.current) { @@ -404,10 +407,17 @@ export const PrimaryNav = (props: PrimaryNavProps) => { // When a user lands on a page and does not have any preference set, // we want to expand the accordion that contains the active link for convenience and discoverability React.useEffect(() => { + // Wait for preferences to load or if there's an error if (preferencesLoading || preferencesError) { return; } + // Wait for preferences data to be available (not just the field, but the whole object) + if (!preferences) { + return; + } + + // If user has already set collapsedSideNavProductFamilies preference, don't override it if (collapsedSideNavPreference) { return; } @@ -424,13 +434,13 @@ export const PrimaryNav = (props: PrimaryNavProps) => { if (activeGroupIndex !== -1) { accordionClicked(activeGroupIndex); } - - // eslint-disable-next-line react-hooks/exhaustive-deps }, [ + accordionClicked, location.pathname, location.search, productFamilyLinkGroups, collapsedSideNavPreference, + preferences, preferencesLoading, preferencesError, ]); diff --git a/packages/manager/src/dev-tools/DevTools.tsx b/packages/manager/src/dev-tools/DevTools.tsx index c3454d40852..22de6e63c1e 100644 --- a/packages/manager/src/dev-tools/DevTools.tsx +++ b/packages/manager/src/dev-tools/DevTools.tsx @@ -57,7 +57,7 @@ export const DevTools = (props: Props) => { }; const handleGoToPreferences = () => { - window.location.assign('/profile/settings?preferenceEditor=true'); + window.location.assign('/profile/preferences?preferenceEditor=true'); }; React.useEffect(() => {