From b08d399906046fffb7e7e0637c6319f1905c3891 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 6 Jan 2025 15:19:15 -0500 Subject: [PATCH 01/16] rechecking everything - step 1 --- packages/manager/src/MainContent.tsx | 13 ------------- .../routes/placementGroups/PlacementGroupsRoute.tsx | 6 +++++- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/packages/manager/src/MainContent.tsx b/packages/manager/src/MainContent.tsx index dcdae2ec607..d576bc0b103 100644 --- a/packages/manager/src/MainContent.tsx +++ b/packages/manager/src/MainContent.tsx @@ -32,7 +32,6 @@ import { switchAccountSessionContext } from './context/switchAccountSessionConte import { useIsACLPEnabled } from './features/CloudPulse/Utils/utils'; import { useIsDatabasesEnabled } from './features/Databases/utilities'; import { useIsIAMEnabled } from './features/IAM/Shared/utilities'; -import { useIsPlacementGroupsEnabled } from './features/PlacementGroups/utils'; import { useGlobalErrors } from './hooks/useGlobalErrors'; import { useAccountSettings } from './queries/account/settings'; import { useProfile } from './queries/profile/profile'; @@ -180,11 +179,6 @@ const AccountActivationLanding = React.lazy( const Firewalls = React.lazy(() => import('src/features/Firewalls')); const Databases = React.lazy(() => import('src/features/Databases')); const VPC = React.lazy(() => import('src/features/VPCs')); -const PlacementGroups = React.lazy(() => - import('src/features/PlacementGroups').then((module) => ({ - default: module.PlacementGroups, - })) -); const CloudPulse = React.lazy(() => import('src/features/CloudPulse/CloudPulseLanding').then((module) => ({ @@ -230,7 +224,6 @@ export const MainContent = () => { const username = profile?.username || ''; const { isDatabasesEnabled } = useIsDatabasesEnabled(); - const { isPlacementGroupsEnabled } = useIsPlacementGroupsEnabled(); const { data: accountSettings } = useAccountSettings(); const defaultRoot = accountSettings?.managed ? '/managed' : '/linodes'; @@ -328,12 +321,6 @@ export const MainContent = () => { }> - {isPlacementGroupsEnabled && ( - - )} { + const { isPlacementGroupsEnabled } = useIsPlacementGroupsEnabled(); + return ( }> - + {isPlacementGroupsEnabled ? : } ); }; From c4cb913841ffe5f950e408f2e5e892176352e1e7 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 6 Jan 2025 15:25:51 -0500 Subject: [PATCH 02/16] steps 2-4 --- .../PlacementGroupsDetail.tsx | 13 ----- .../PlacementGroupsLanding.tsx | 7 --- .../src/features/PlacementGroups/index.tsx | 54 ------------------- packages/manager/src/routes/index.tsx | 1 + .../src/routes/placementGroups/index.ts | 48 ++++++++--------- .../placementGroupsLazyRoutes.ts | 22 ++++++++ 6 files changed, 47 insertions(+), 98 deletions(-) delete mode 100644 packages/manager/src/features/PlacementGroups/index.tsx create mode 100644 packages/manager/src/routes/placementGroups/placementGroupsLazyRoutes.ts diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx index 67d1cb570d1..06535fa9eda 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx @@ -1,6 +1,5 @@ import { PLACEMENT_GROUP_TYPES } from '@linode/api-v4'; import { CircleProgress, Notice } from '@linode/ui'; -import { createLazyRoute } from '@tanstack/react-router'; import * as React from 'react'; import { useParams } from 'react-router-dom'; @@ -134,15 +133,3 @@ export const PlacementGroupsDetail = () => { ); }; - -export const placementGroupsDetailLazyRoute = createLazyRoute( - '/placement-groups/$id' -)({ - component: PlacementGroupsDetail, -}); - -export const placementGroupsUnassignLazyRoute = createLazyRoute( - '/placement-groups/$id/linodes/unassign/$linodeId' -)({ - component: PlacementGroupsDetail, -}); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx index f6c3a75f986..5628d7158db 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx @@ -1,6 +1,5 @@ import { CircleProgress } from '@linode/ui'; import { useMediaQuery, useTheme } from '@mui/material'; -import { createLazyRoute } from '@tanstack/react-router'; import * as React from 'react'; import { useParams } from 'react-router-dom'; import { useHistory } from 'react-router-dom'; @@ -281,9 +280,3 @@ export const PlacementGroupsLanding = React.memo(() => { ); }); - -export const placementGroupsLandingLazyRoute = createLazyRoute( - '/placement-groups' -)({ - component: PlacementGroupsLanding, -}); diff --git a/packages/manager/src/features/PlacementGroups/index.tsx b/packages/manager/src/features/PlacementGroups/index.tsx deleted file mode 100644 index aae3d8f8c7d..00000000000 --- a/packages/manager/src/features/PlacementGroups/index.tsx +++ /dev/null @@ -1,54 +0,0 @@ -import * as React from 'react'; -import { Route, Switch, useRouteMatch } from 'react-router-dom'; - -import { DocumentTitleSegment } from 'src/components/DocumentTitle'; -import { ProductInformationBanner } from 'src/components/ProductInformationBanner/ProductInformationBanner'; -import { SuspenseLoader } from 'src/components/SuspenseLoader'; - -const PlacementGroupsLanding = React.lazy(() => - import('./PlacementGroupsLanding/PlacementGroupsLanding').then((module) => ({ - default: module.PlacementGroupsLanding, - })) -); - -const PlacementGroupsDetail = React.lazy(() => - import('./PlacementGroupsDetail/PlacementGroupsDetail').then((module) => ({ - default: module.PlacementGroupsDetail, - })) -); - -export const PlacementGroups = () => { - const { path } = useRouteMatch(); - - return ( - }> - - - - - - - - - - - - - - ); -}; diff --git a/packages/manager/src/routes/index.tsx b/packages/manager/src/routes/index.tsx index 9aa1261c7a6..10f2cecc5cf 100644 --- a/packages/manager/src/routes/index.tsx +++ b/packages/manager/src/routes/index.tsx @@ -87,6 +87,7 @@ declare module '@tanstack/react-router' { export const migrationRouteTree = migrationRootRoute.addChildren([ betaRouteTree, domainsRouteTree, + placementGroupsRouteTree, volumesRouteTree, ]); export type MigrationRouteTree = typeof migrationRouteTree; diff --git a/packages/manager/src/routes/placementGroups/index.ts b/packages/manager/src/routes/placementGroups/index.ts index 9088af62b15..95f62bc597f 100644 --- a/packages/manager/src/routes/placementGroups/index.ts +++ b/packages/manager/src/routes/placementGroups/index.ts @@ -13,18 +13,18 @@ const placementGroupsIndexRoute = createRoute({ getParentRoute: () => placementGroupsRoute, path: '/', }).lazy(() => - import( - 'src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding' - ).then((m) => m.placementGroupsLandingLazyRoute) + import('./placementGroupsLazyRoutes').then( + (m) => m.placementGroupsLandingLazyRoute + ) ); const placementGroupsCreateRoute = createRoute({ getParentRoute: () => placementGroupsRoute, path: 'create', }).lazy(() => - import( - 'src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding' - ).then((m) => m.placementGroupsLandingLazyRoute) + import('./placementGroupsLazyRoutes').then( + (m) => m.placementGroupsLandingLazyRoute + ) ); const placementGroupsEditRoute = createRoute({ @@ -34,9 +34,9 @@ const placementGroupsEditRoute = createRoute({ }), path: 'edit/$id', }).lazy(() => - import( - 'src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding' - ).then((m) => m.placementGroupsLandingLazyRoute) + import('./placementGroupsLazyRoutes').then( + (m) => m.placementGroupsLandingLazyRoute + ) ); const placementGroupsDeleteRoute = createRoute({ @@ -46,9 +46,9 @@ const placementGroupsDeleteRoute = createRoute({ }), path: 'delete/$id', }).lazy(() => - import( - 'src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding' - ).then((m) => m.placementGroupsLandingLazyRoute) + import('./placementGroupsLazyRoutes').then( + (m) => m.placementGroupsLandingLazyRoute + ) ); const placementGroupsDetailRoute = createRoute({ @@ -58,27 +58,27 @@ const placementGroupsDetailRoute = createRoute({ }), path: '$id', }).lazy(() => - import( - 'src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail' - ).then((m) => m.placementGroupsDetailLazyRoute) + import('./placementGroupsLazyRoutes').then( + (m) => m.placementGroupsDetailLazyRoute + ) ); const placementGroupsDetailLinodesRoute = createRoute({ getParentRoute: () => placementGroupsDetailRoute, path: 'linodes', }).lazy(() => - import( - 'src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail' - ).then((m) => m.placementGroupsDetailLazyRoute) + import('./placementGroupsLazyRoutes').then( + (m) => m.placementGroupsDetailLazyRoute + ) ); const placementGroupsAssignRoute = createRoute({ getParentRoute: () => placementGroupsDetailLinodesRoute, path: 'assign', }).lazy(() => - import( - 'src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail' - ).then((m) => m.placementGroupsUnassignLazyRoute) + import('./placementGroupsLazyRoutes').then( + (m) => m.placementGroupsUnassignLazyRoute + ) ); const placementGroupsUnassignRoute = createRoute({ @@ -88,9 +88,9 @@ const placementGroupsUnassignRoute = createRoute({ }), path: 'unassign/$linodeId', }).lazy(() => - import( - 'src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail' - ).then((m) => m.placementGroupsUnassignLazyRoute) + import('./placementGroupsLazyRoutes').then( + (m) => m.placementGroupsUnassignLazyRoute + ) ); export const placementGroupsRouteTree = placementGroupsRoute.addChildren([ diff --git a/packages/manager/src/routes/placementGroups/placementGroupsLazyRoutes.ts b/packages/manager/src/routes/placementGroups/placementGroupsLazyRoutes.ts new file mode 100644 index 00000000000..6521e756fc7 --- /dev/null +++ b/packages/manager/src/routes/placementGroups/placementGroupsLazyRoutes.ts @@ -0,0 +1,22 @@ +import { createLazyRoute } from '@tanstack/react-router'; + +import { PlacementGroupsDetail } from 'src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail'; +import { PlacementGroupsLanding } from 'src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding'; + +export const placementGroupsDetailLazyRoute = createLazyRoute( + '/placement-groups/$id' +)({ + component: PlacementGroupsDetail, +}); + +export const placementGroupsUnassignLazyRoute = createLazyRoute( + '/placement-groups/$id/linodes/unassign/$linodeId' +)({ + component: PlacementGroupsDetail, +}); + +export const placementGroupsLandingLazyRoute = createLazyRoute( + '/placement-groups' +)({ + component: PlacementGroupsLanding, +}); From 3e7cace72ea6fb83c8d70194d072c8aae9eca22c Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 6 Jan 2025 15:27:03 -0500 Subject: [PATCH 03/16] step 5 (after this gets a bit finnicky i think) --- packages/manager/.eslintrc.cjs | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/manager/.eslintrc.cjs b/packages/manager/.eslintrc.cjs index cc85bbb89d0..2543d144a41 100644 --- a/packages/manager/.eslintrc.cjs +++ b/packages/manager/.eslintrc.cjs @@ -91,6 +91,7 @@ module.exports = { // for each new features added to the migration router, add its directory here 'src/features/Betas/**/*', 'src/features/Domains/**/*', + 'src/features/PlacementGroups/**/*', 'src/features/Volumes/**/*', ], rules: { From 3c2127a1686db8a3eaca728fd2f6688f06623225 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 6 Jan 2025 15:42:24 -0500 Subject: [PATCH 04/16] step 6 initial changes - 23f, 29p --- ...lacementGroupsAssignLinodesDrawer.test.tsx | 16 +++++--- .../PlacementGroupsCreateDrawer.test.tsx | 28 ++++++++------ .../PlacementGroupsDeleteModal.test.tsx | 10 +++-- .../PlacementGroupsDetail.test.tsx | 36 ++++++++++-------- .../PlacementGroupsLinodes.test.tsx | 10 ++--- .../PlacementGroupsLinodesTable.test.tsx | 18 ++++----- .../PlacementGroupsLinodesTableRow.test.tsx | 6 +-- .../PlacementGroupsSummary.test.tsx | 6 +-- .../PlacementGroupsDetailPanel.test.tsx | 18 ++++----- .../PlacementGroupsEditDrawer.test.tsx | 10 +++-- .../PlacementGroupsLanding.test.tsx | 38 ++++++++++++------- .../PlacementGroupsRow.test.tsx | 12 ++++-- .../PlacementGroupsUnassignModal.test.tsx | 6 +-- 13 files changed, 125 insertions(+), 89 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsAssignLinodesDrawer.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsAssignLinodesDrawer.test.tsx index 756eda226fa..5876631c0e4 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsAssignLinodesDrawer.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsAssignLinodesDrawer.test.tsx @@ -6,7 +6,7 @@ import { placementGroupFactory, regionFactory, } from 'src/factories'; -import { renderWithTheme } from 'src/utilities/testHelpers'; +import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PlacementGroupsAssignLinodesDrawer } from './PlacementGroupsAssignLinodesDrawer'; @@ -42,12 +42,12 @@ vi.mock('src/queries/placementGroups', async () => { }); describe('PlacementGroupsAssignLinodesDrawer', () => { - it('should render the error state', () => { + it('should render the error state', async () => { queryMocks.useAllLinodesQuery.mockReturnValue({ error: [{ reason: 'Not found' }], }); - const { container } = renderWithTheme( + const { container } = await renderWithThemeAndRouter( { expect(container).toBeEmptyDOMElement(); }); - it('should render the drawer components', () => { + it('should render the drawer components', async () => { queryMocks.useAllLinodesQuery.mockReturnValue({ data: [ linodeFactory.build({ id: 1, label: 'Linode-1', region: 'us-east' }), @@ -117,11 +117,15 @@ describe('PlacementGroupsAssignLinodesDrawer', () => { }) ); - const { getByPlaceholderText, getByRole, getByText } = renderWithTheme( + const { + getByPlaceholderText, + getByRole, + getByText, + } = await renderWithThemeAndRouter( { }); describe('PlacementGroupsCreateDrawer', () => { - it('should render and have its fields enabled', () => { - const { getAllByRole, getByLabelText, getByText } = renderWithTheme( + it('should render and have its fields enabled', async () => { + const { + getAllByRole, + getByLabelText, + getByText, + } = await renderWithThemeAndRouter( ); @@ -47,7 +51,7 @@ describe('PlacementGroupsCreateDrawer', () => { }); it('Placement Group Type select should have the correct options', async () => { - const { getByPlaceholderText, getByText } = renderWithTheme( + const { getByPlaceholderText, getByText } = await renderWithThemeAndRouter( ); @@ -62,15 +66,13 @@ describe('PlacementGroupsCreateDrawer', () => { }); it('should populate the region select with the selected region prop', async () => { - const { getByText } = renderWithTheme( + const { getByText } = await renderWithThemeAndRouter( , { - MemoryRouter: { - initialEntries: ['/linodes/create'], - }, + initialRoute: 'linodes/create', } ); @@ -85,7 +87,9 @@ describe('PlacementGroupsCreateDrawer', () => { getByPlaceholderText, getByRole, getByText, - } = renderWithTheme(); + } = await renderWithThemeAndRouter( + + ); fireEvent.change(getByLabelText('Label'), { target: { value: 'my-label' }, @@ -107,9 +111,9 @@ describe('PlacementGroupsCreateDrawer', () => { expect( queryMocks.useCreatePlacementGroup().mutateAsync ).toHaveBeenCalledWith({ - placement_group_type: 'anti_affinity:local', - placement_group_policy: 'strict', label: 'my-label', + placement_group_policy: 'strict', + placement_group_type: 'anti_affinity:local', region: 'us-east', }); }); @@ -120,7 +124,7 @@ describe('PlacementGroupsCreateDrawer', () => { data: [placementGroupFactory.build({ region: 'us-west' })], }); const regionWithoutCapacity = 'US, Fremont, CA (us-west)'; - const { getByPlaceholderText, getByText } = renderWithTheme( + const { getByPlaceholderText, getByText } = await renderWithThemeAndRouter( ); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx index e95dd1e6c83..f91552263e4 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx @@ -2,7 +2,7 @@ import { userEvent } from '@testing-library/user-event'; import * as React from 'react'; import { linodeFactory, placementGroupFactory } from 'src/factories'; -import { renderWithTheme } from 'src/utilities/testHelpers'; +import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PlacementGroupsDeleteModal } from './PlacementGroupsDeleteModal'; @@ -50,7 +50,11 @@ describe('PlacementGroupsDeleteModal', () => { data: preference, }); - const { getByRole, getByTestId, getByText } = renderWithTheme( + const { + getByRole, + getByTestId, + getByText, + } = await renderWithThemeAndRouter( { data: preference, }); - const { getByRole, getByTestId } = renderWithTheme( + const { getByRole, getByTestId } = await renderWithThemeAndRouter( { }); describe('PlacementGroupsLanding', () => { - it('renders a error page', () => { - const { getByText } = renderWithTheme(); + it('renders a error page', async () => { + const { getByText } = await renderWithThemeAndRouter( + + ); expect(getByText('Not Found')).toBeInTheDocument(); }); - it('renders a loading state', () => { + it('renders a loading state', async () => { queryMocks.usePlacementGroupQuery.mockReturnValue({ data: placementGroupFactory.build({ id: 1, @@ -80,30 +82,32 @@ describe('PlacementGroupsLanding', () => { ], }); - const { getByRole } = renderWithTheme(, { - MemoryRouter: { - initialEntries: [{ pathname: '/placement-groups/1' }], - }, - }); + const { getByRole } = await renderWithThemeAndRouter( + , + { + initialRoute: '/placement-groups/1', + } + ); expect(getByRole('progressbar')).toBeInTheDocument(); }); - it('renders breadcrumbs, docs link and tabs', () => { + it('renders breadcrumbs, docs link and tabs', async () => { queryMocks.usePlacementGroupQuery.mockReturnValue({ data: placementGroupFactory.build({ - placement_group_type: 'anti_affinity:local', id: 1, is_compliant: true, label: 'My first PG', + placement_group_type: 'anti_affinity:local', }), }); - const { getByText } = renderWithTheme(, { - MemoryRouter: { - initialEntries: [{ pathname: '/placement-groups/1' }], - }, - }); + const { getByText } = await renderWithThemeAndRouter( + , + { + initialRoute: '/placement-groups/1', + } + ); expect(getByText(/my first pg/i)).toBeInTheDocument(); expect(getByText(/docs/i)).toBeInTheDocument(); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx index 16e06911d3d..90a233b97d0 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx @@ -1,14 +1,14 @@ import * as React from 'react'; import { placementGroupFactory } from 'src/factories'; -import { renderWithTheme } from 'src/utilities/testHelpers'; +import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PLACEMENT_GROUP_LINODES_ERROR_MESSAGE } from '../../constants'; import { PlacementGroupsLinodes } from './PlacementGroupsLinodes'; describe('PlacementGroupsLinodes', () => { - it('renders an error state if placement groups are undefined', () => { - const { getByText } = renderWithTheme( + it('renders an error state if placement groups are undefined', async () => { + const { getByText } = await renderWithThemeAndRouter( { ).toBeInTheDocument(); }); - it('features the linodes table, a filter field, a create button and a docs link', () => { + it('features the linodes table, a filter field, a create button and a docs link', async () => { const placementGroup = placementGroupFactory.build({ members: [ { @@ -33,7 +33,7 @@ describe('PlacementGroupsLinodes', () => { ], }); - const { getByPlaceholderText, getByRole } = renderWithTheme( + const { getByPlaceholderText, getByRole } = await renderWithThemeAndRouter( { - it('renders an error state when encountering an API error', () => { - const { getByText } = renderWithTheme( + it('renders an error state when encountering an API error', async () => { + const { getByText } = await renderWithThemeAndRouter( { expect(getByText(/not found/i)).toBeInTheDocument(); }); - it('renders a loading skeleton based on the loading prop', () => { - const { getByTestId } = renderWithTheme( + it('renders a loading skeleton based on the loading prop', async () => { + const { getByTestId } = await renderWithThemeAndRouter( ); expect(getByTestId('table-row-loading')).toBeInTheDocument(); }); - it('should have the correct number of columns', () => { - const { getAllByRole } = renderWithTheme( + it('should have the correct number of columns', async () => { + const { getAllByRole } = await renderWithThemeAndRouter( ); expect(getAllByRole('columnheader')).toHaveLength(3); }); - it('should have the correct number of rows', () => { - const { getAllByTestId } = renderWithTheme( + it('should have the correct number of rows', async () => { + const { getAllByTestId } = await renderWithThemeAndRouter( ); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx index c4d32c1caa9..527b5a83a3b 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import { linodeFactory } from 'src/factories'; import { wrapWithTableBody } from 'src/utilities/testHelpers'; -import { renderWithTheme } from 'src/utilities/testHelpers'; +import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PlacementGroupsLinodesTableRow } from './PlacementGroupsLinodesTableRow'; @@ -15,8 +15,8 @@ const defaultProps = { }; describe('PlacementGroupsLinodesTableRow', () => { - it('should feature the right table row data', () => { - const { getAllByRole } = renderWithTheme( + it('should feature the right table row data', async () => { + const { getAllByRole } = await renderWithThemeAndRouter( wrapWithTableBody() ); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx index 73b8a19223e..a22f4d6f729 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx @@ -1,13 +1,13 @@ import * as React from 'react'; import { placementGroupFactory, regionFactory } from 'src/factories'; -import { renderWithTheme } from 'src/utilities/testHelpers'; +import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PlacementGroupsSummary } from './PlacementGroupsSummary'; describe('PlacementGroups Summary', () => { - it('renders the placement group detail summary panel', () => { - const { getByTestId, getByText } = renderWithTheme( + it('renders the placement group detail summary panel', async () => { + const { getByTestId, getByText } = await renderWithThemeAndRouter( { }); }); - it('should have its select disabled and no create PG button on initial render', () => { - const { getByRole, queryByRole } = renderWithTheme( + it('should have its select disabled and no create PG button on initial render', async () => { + const { getByRole, queryByRole } = await renderWithThemeAndRouter( ); @@ -89,8 +89,8 @@ describe('PlacementGroupsDetailPanel', () => { ).not.toBeInTheDocument(); }); - it('should have its select enabled and a create PG button when provided a region', () => { - const { getByRole } = renderWithTheme( + it('should have its select enabled and a create PG button when provided a region', async () => { + const { getByRole } = await renderWithThemeAndRouter( { ).toBeEnabled(); }); - it('should have its select disabled and no create PG button when provided a region without PG capability', () => { - const { getByRole, getByTestId, queryByRole } = renderWithTheme( + it('should have its select disabled and no create PG button when provided a region without PG capability', async () => { + const { getByRole, getByTestId, queryByRole } = await renderWithThemeAndRouter( { ).not.toBeInTheDocument(); }); - it('should have its PG select enabled and Create Placement Group button disabled if the region has reached its PG capacity', () => { - const { getByPlaceholderText, getByRole } = renderWithTheme( + it('should have its PG select enabled and Create Placement Group button disabled if the region has reached its PG capacity', async () => { + const { getByPlaceholderText, getByRole } = await renderWithThemeAndRouter( { it('should render, have the proper fields populated with PG values, and have uneditable fields disabled', async () => { queryMocks.useParams.mockReturnValue({ id: '1' }); - const { getByLabelText, getByRole, getByText } = renderWithTheme( + const { + getByLabelText, + getByRole, + getByText, + } = await renderWithThemeAndRouter( { }); describe('PlacementGroupsLanding', () => { - it('renders loading state', () => { + it('renders loading state', async () => { queryMocks.usePlacementGroupsQuery.mockReturnValue({ isLoading: true, }); - const { getByRole } = renderWithTheme(); + const { getByRole } = await renderWithThemeAndRouter( + + ); expect(getByRole('progressbar')).toBeInTheDocument(); }); - it('renders error state', () => { + it('renders error state', async () => { queryMocks.usePlacementGroupsQuery.mockReturnValue({ error: [{ reason: 'Not found' }], }); - const { getByText } = renderWithTheme(); + const { getByText } = await renderWithThemeAndRouter( + + ); expect(getByText(/not found/i)).toBeInTheDocument(); }); - it('renders docs link and create button', () => { + it('renders docs link and create button', async () => { queryMocks.usePlacementGroupsQuery.mockReturnValue({ data: { data: [ @@ -51,13 +55,15 @@ describe('PlacementGroupsLanding', () => { }, }); - const { getByText } = renderWithTheme(); + const { getByText } = await renderWithThemeAndRouter( + + ); expect(getByText(/create placement group/i)).toBeInTheDocument(); expect(getByText(/docs/i)).toBeInTheDocument(); }); - it('renders placement groups', () => { + it('renders placement groups', async () => { queryMocks.usePlacementGroupsQuery.mockReturnValue({ data: { data: [ @@ -72,13 +78,15 @@ describe('PlacementGroupsLanding', () => { }, }); - const { getByText } = renderWithTheme(); + const { getByText } = await renderWithThemeAndRouter( + + ); expect(getByText(/group 1/i)).toBeInTheDocument(); expect(getByText(/group 2/i)).toBeInTheDocument(); }); - it('should render placement group landing with empty state', () => { + it('should render placement group landing with empty state', async () => { queryMocks.usePlacementGroupsQuery.mockReturnValue({ data: { data: [], @@ -86,12 +94,14 @@ describe('PlacementGroupsLanding', () => { }, }); - const { getByText } = renderWithTheme(); + const { getByText } = await renderWithThemeAndRouter( + + ); expect(getByText(headers.description)).toBeInTheDocument(); }); - it('should render placement group Getting Started Guides on landing page with empty state', () => { + it('should render placement group Getting Started Guides on landing page with empty state', async () => { queryMocks.usePlacementGroupsQuery.mockReturnValue({ data: { data: [], @@ -99,7 +109,9 @@ describe('PlacementGroupsLanding', () => { }, }); - const { getByText } = renderWithTheme(); + const { getByText } = await renderWithThemeAndRouter( + + ); expect(getByText('Getting Started Guides')).toBeInTheDocument(); }); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.test.tsx index 54ba5906a06..ae029ef4683 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.test.tsx @@ -6,7 +6,7 @@ import { regionFactory, } from 'src/factories'; import { - renderWithTheme, + renderWithThemeAndRouter, resizeScreenSize, wrapWithTableBody, } from 'src/utilities/testHelpers'; @@ -22,7 +22,6 @@ const linode = linodeFactory.build({ }); const placementGroup = placementGroupFactory.build({ - placement_group_type: 'anti_affinity:local', id: 1, is_compliant: false, label: 'group 1', @@ -32,6 +31,7 @@ const placementGroup = placementGroupFactory.build({ linode_id: 1, }, ], + placement_group_type: 'anti_affinity:local', region: 'us-east', }); @@ -43,9 +43,13 @@ const region = regionFactory.build({ }); describe('PlacementGroupsRow', () => { - it('renders the columns with proper data', () => { + it('renders the columns with proper data', async () => { resizeScreenSize(1200); - const { getByRole, getByTestId, getByText } = renderWithTheme( + const { + getByRole, + getByTestId, + getByText, + } = await renderWithThemeAndRouter( wrapWithTableBody( { }); describe('PlacementGroupsUnassignModal', () => { - it('should render and have the proper content and CTAs', () => { + it('should render and have the proper content and CTAs', async () => { queryMocks.useLinodeQuery.mockReturnValue({ data: linodeFactory.build({ id: 1, @@ -39,7 +39,7 @@ describe('PlacementGroupsUnassignModal', () => { linodeId: '1', }); - const { getByLabelText, getByRole } = renderWithTheme( + const { getByLabelText, getByRole } = await renderWithThemeAndRouter( null} open From 0f4c92c02f138385570e0cb71796558bb3a6aea8 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Mon, 6 Jan 2025 20:44:04 -0500 Subject: [PATCH 05/16] steps 6-7 ish --- .../PlacementGroupsCreateDrawer.tsx | 6 +- .../PlacementGroupsDetail.tsx | 4 +- .../PlacementGroupsLinodes.tsx | 30 +++--- .../PlacementGroupsLinodesTableRow.tsx | 6 +- .../PlacementGroupsEditDrawer.tsx | 8 +- .../PlacementGroupsLanding.tsx | 92 ++++++++++++++----- .../PlacementGroupsUnassignModal.tsx | 17 ++-- .../src/features/PlacementGroups/constants.ts | 6 ++ .../src/routes/placementGroups/index.ts | 61 ++++++++---- 9 files changed, 155 insertions(+), 75 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.tsx index ccd9a0058ca..5a440a3eebb 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.tsx @@ -8,10 +8,10 @@ import { Typography, } from '@linode/ui'; import { createPlacementGroupSchema } from '@linode/validation'; +import { useLocation } from '@tanstack/react-router'; import { useFormik } from 'formik'; import { useSnackbar } from 'notistack'; import * as React from 'react'; -import { useLocation } from 'react-router-dom'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { DescriptionList } from 'src/components/DescriptionList/DescriptionList'; @@ -73,7 +73,9 @@ export const PlacementGroupsCreateDrawer = ( const location = useLocation(); const isFromLinodeCreate = location.pathname.includes('/linodes/create'); - const queryParams = getQueryParamsFromQueryString(location.search); + const queryParams = getQueryParamsFromQueryString( + location.pathname.split('?')[1] // todo connie - fix this to be more robust, but just getting rid of type errors for now + ); const handleRegionSelect = (region: Region['id']) => { setFieldValue('region', region); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx index 06535fa9eda..e59399b4e12 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx @@ -1,7 +1,7 @@ import { PLACEMENT_GROUP_TYPES } from '@linode/api-v4'; import { CircleProgress, Notice } from '@linode/ui'; import * as React from 'react'; -import { useParams } from 'react-router-dom'; +import { useParams } from '@tanstack/react-router'; import { DocumentTitleSegment } from 'src/components/DocumentTitle'; import { ErrorState } from 'src/components/ErrorState/ErrorState'; @@ -22,7 +22,7 @@ import { PlacementGroupsLinodes } from './PlacementGroupsLinodes/PlacementGroups import { PlacementGroupsSummary } from './PlacementGroupsSummary/PlacementGroupsSummary'; export const PlacementGroupsDetail = () => { - const { id } = useParams<{ id: string }>(); + const { id } = useParams({ from: '/placement-groups/$id' }); const placementGroupId = +id; const { diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx index c89332c6e19..f2ef1a19687 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx @@ -1,7 +1,7 @@ import { Button, Stack } from '@linode/ui'; import Grid from '@mui/material/Unstable_Grid2/Grid2'; +import { useLocation, useNavigate } from '@tanstack/react-router'; import * as React from 'react'; -import { useHistory } from 'react-router-dom'; import { DebouncedSearchTextField } from 'src/components/DebouncedSearchTextField'; import { ErrorState } from 'src/components/ErrorState/ErrorState'; @@ -33,7 +33,8 @@ export const PlacementGroupsLinodes = (props: Props) => { placementGroup, region, } = props; - const history = useHistory(); + const navigate = useNavigate(); + const location = useLocation(); const [searchText, setSearchText] = React.useState(''); const [selectedLinode, setSelectedLinode] = React.useState< Linode | undefined @@ -63,24 +64,27 @@ export const PlacementGroupsLinodes = (props: Props) => { }); const handleAssignLinodesDrawer = () => { - history.replace(`/placement-groups/${placementGroup.id}/linodes/assign`); + navigate({ + params: { id: placementGroup.id }, + to: '/placement-groups/$id/linodes/assign', + }); }; const handleUnassignLinodeModal = (linode: Linode) => { setSelectedLinode(linode); - history.replace( - `/placement-groups/${placementGroup.id}/linodes/unassign/${linode.id}` - ); + navigate({ + params: { id: placementGroup.id, linodeId: linode.id }, + to: '/placement-groups/$id/linodes/unassign/$linodeId', + }); }; const handleCloseDrawer = () => { setSelectedLinode(undefined); - history.replace(`/placement-groups/${placementGroup.id}/linodes`); + navigate({ + params: { id: placementGroup.id }, + to: '/placement-groups/$id', + }); }; - const isAssignLinodesDrawerOpen = history.location.pathname.includes( - '/assign' - ); - const isUnassignLinodesDrawerOpen = history.location.pathname.includes( - '/unassign' - ); + const isAssignLinodesDrawerOpen = location.pathname.includes('/assign'); + const isUnassignLinodesDrawerOpen = location.pathname.includes('/unassign'); return ( diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.tsx index 5b53be099bc..6aafac59542 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.tsx @@ -1,5 +1,5 @@ +import { useParams } from '@tanstack/react-router'; import * as React from 'react'; -import { useParams } from 'react-router-dom'; import { InlineMenuAction } from 'src/components/InlineMenuAction/InlineMenuAction'; import { Link } from 'src/components/Link'; @@ -36,7 +36,9 @@ type MigrationType = 'inbound' | 'outbound' | null; export const PlacementGroupsLinodesTableRow = React.memo((props: Props) => { const { handleUnassignLinodeModal, linode } = props; const { label, status } = linode; - const { id: placementGroupId } = useParams<{ id: string }>(); + const { id: placementGroupId } = useParams({ + from: '/placement-groups/$id', // todo connie - check about $id/linode + }); const notificationContext = React.useContext(notificationCenterContext); const isLinodeMigrating = Boolean(linode.placement_group?.migrating_to); const { data: placementGroup } = usePlacementGroupQuery( diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx index 728ae6759e9..9e142f92247 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx @@ -7,7 +7,7 @@ import { updatePlacementGroupSchema } from '@linode/validation'; import { useFormik } from 'formik'; import { useSnackbar } from 'notistack'; import * as React from 'react'; -import { useParams } from 'react-router-dom'; +import { useParams } from '@tanstack/react-router'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { DescriptionList } from 'src/components/DescriptionList/DescriptionList'; @@ -36,13 +36,15 @@ export const PlacementGroupsEditDrawer = ( region, selectedPlacementGroup: placementGroupFromProps, } = props; - const { id } = useParams<{ id: string }>(); + // todo connie - try to consolidate params/remove params from drawers? + // figure out error with NaN + const params = useParams({ strict: false }); const { data: placementGroupFromParam, isFetching, status, } = usePlacementGroupQuery( - Number(id), + Number(params.id), open && placementGroupFromProps === undefined ); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx index 5628d7158db..ff26a01fdf9 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx @@ -1,8 +1,12 @@ import { CircleProgress } from '@linode/ui'; import { useMediaQuery, useTheme } from '@mui/material'; +import { + useLocation, + useNavigate, + useParams, + useSearch, +} from '@tanstack/react-router'; import * as React from 'react'; -import { useParams } from 'react-router-dom'; -import { useHistory } from 'react-router-dom'; import { DebouncedSearchTextField } from 'src/components/DebouncedSearchTextField'; import { ErrorState } from 'src/components/ErrorState/ErrorState'; @@ -17,15 +21,21 @@ import { TableRow } from 'src/components/TableRow'; import { TableRowEmpty } from 'src/components/TableRowEmpty/TableRowEmpty'; import { TableSortCell } from 'src/components/TableSortCell/TableSortCell'; import { getRestrictedResourceText } from 'src/features/Account/utils'; -import { useOrder } from 'src/hooks/useOrder'; -import { usePagination } from 'src/hooks/usePagination'; +import { useOrderV2 } from 'src/hooks/useOrderV2'; +import { usePaginationV2 } from 'src/hooks/usePaginationV2'; import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck'; import { useAllLinodesQuery } from 'src/queries/linodes/linodes'; import { usePlacementGroupsQuery } from 'src/queries/placementGroups'; import { useRegionsQuery } from 'src/queries/regions/regions'; import { getAPIErrorOrDefault } from 'src/utilities/errorUtils'; -import { PLACEMENT_GROUPS_DOCS_LINK } from '../constants'; +import { + PG_LANDING_TABLE_DEFAULT_ORDER, + PG_LANDING_TABLE_DEFAULT_ORDER_BY, + PG_LANDING_TABLE_PREFERENCE_KEY, + PLACEMENT_GROUPS_DOCS_LINK, + PLACEMENT_GROUPS_LANDING_ROUTE, +} from '../constants'; import { PlacementGroupsCreateDrawer } from '../PlacementGroupsCreateDrawer'; import { PlacementGroupsDeleteModal } from '../PlacementGroupsDeleteModal'; import { PlacementGroupsEditDrawer } from '../PlacementGroupsEditDrawer'; @@ -34,23 +44,36 @@ import { PlacementGroupsLandingEmptyState } from './PlacementGroupsLandingEmptyS import { PlacementGroupsRow } from './PlacementGroupsRow'; import type { Filter, PlacementGroup } from '@linode/api-v4'; - -const preferenceKey = 'placement-groups'; +import type { PlacementGroupsSearchParams } from 'src/routes/placementGroups'; export const PlacementGroupsLanding = React.memo(() => { - const history = useHistory(); - const pagination = usePagination(1, preferenceKey); - const { id } = useParams<{ id: string }>(); + const navigate = useNavigate(); + const location = useLocation(); + const pagination = usePaginationV2({ + currentRoute: PLACEMENT_GROUPS_LANDING_ROUTE, + preferenceKey: PG_LANDING_TABLE_PREFERENCE_KEY, + searchParams: (prev) => ({ + ...prev, + query: search.query, + }), + }); + const params = useParams({ strict: false }); + const search: PlacementGroupsSearchParams = useSearch({ + from: PLACEMENT_GROUPS_LANDING_ROUTE, + }); + const { query } = search; const theme = useTheme(); - const [query, setQuery] = React.useState(''); const matchesSmDown = useMediaQuery(theme.breakpoints.down('md')); - const { handleOrderChange, order, orderBy } = useOrder( - { - order: 'asc', - orderBy: 'label', + const { handleOrderChange, order, orderBy } = useOrderV2({ + initialRoute: { + defaultOrder: { + order: PG_LANDING_TABLE_DEFAULT_ORDER, + orderBy: PG_LANDING_TABLE_DEFAULT_ORDER_BY, + }, + from: PLACEMENT_GROUPS_LANDING_ROUTE, }, - `${preferenceKey}-order` - ); + preferenceKey: `${PG_LANDING_TABLE_PREFERENCE_KEY}-order`, + }); const filter: Filter = { ['+order']: order, @@ -72,7 +95,7 @@ export const PlacementGroupsLanding = React.memo(() => { ); const selectedPlacementGroup = placementGroups?.data.find( - (pg) => pg.id === Number(id) + (pg) => pg.id === Number(params.id) ); const allLinodeIDsAssigned = placementGroups?.data.reduce( @@ -103,25 +126,44 @@ export const PlacementGroupsLanding = React.memo(() => { }); const handleCreatePlacementGroup = () => { - history.push('/placement-groups/create'); + navigate({ search: (prev) => prev, to: '/placement-groups/create' }); }; const handleEditPlacementGroup = (placementGroup: PlacementGroup) => { - history.push(`/placement-groups/edit/${placementGroup.id}`); + navigate({ + params: { action: 'edit', id: placementGroup.id }, + search: (prev) => prev, + to: '/placement-groups/$action/$id', + }); }; const handleDeletePlacementGroup = (placementGroup: PlacementGroup) => { - history.push(`/placement-groups/delete/${placementGroup.id}`); + navigate({ + params: { action: 'delete', id: placementGroup.id }, + search: (prev) => prev, + to: '/placement-groups/$action/$id', + }); }; const onClosePlacementGroupDrawer = () => { - history.push('/placement-groups'); + navigate({ search: (prev) => prev, to: PLACEMENT_GROUPS_LANDING_ROUTE }); }; const isPlacementGroupCreateDrawerOpen = location.pathname.endsWith('create'); const isPlacementGroupDeleteModalOpen = location.pathname.includes('delete'); const isPlacementGroupEditDrawerOpen = location.pathname.includes('edit'); + const onSearch = (searchString: string) => { + navigate({ + search: (prev) => ({ + ...prev, + page: undefined, + query: searchString || undefined, + }), + to: PLACEMENT_GROUPS_LANDING_ROUTE, + }); + }; + if (placementGroupsLoading) { return ; } @@ -163,7 +205,7 @@ export const PlacementGroupsLanding = React.memo(() => { resourceType: 'Placement Groups', }), }} - breadcrumbProps={{ pathname: '/placement-groups' }} + breadcrumbProps={{ pathname: PLACEMENT_GROUPS_LANDING_ROUTE }} disabledCreateButton={isLinodeReadOnly} docsLink={PLACEMENT_GROUPS_DOCS_LINK} entity="Placement Group" @@ -176,10 +218,10 @@ export const PlacementGroupsLanding = React.memo(() => { hideLabel isSearching={isFetching} label="Search" - onSearch={setQuery} + onSearch={onSearch} placeholder="Search Placement Groups" sx={{ mb: 4 }} - value={query} + value={query ?? ''} /> diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx index e7c03e7bcd4..a0b59cce925 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx @@ -1,7 +1,7 @@ import { CircleProgress, Notice, Typography } from '@linode/ui'; import { useSnackbar } from 'notistack'; import * as React from 'react'; -import { useParams } from 'react-router-dom'; +import { useParams } from '@tanstack/react-router'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { ConfirmationDialog } from 'src/components/ConfirmationDialog/ConfirmationDialog'; @@ -25,10 +25,9 @@ export const PlacementGroupsUnassignModal = (props: Props) => { const { onClose, open, selectedLinode } = props; const { enqueueSnackbar } = useSnackbar(); - const { id: placementGroupId, linodeId } = useParams<{ - id: string; - linodeId: string; - }>(); + const { id: placementGroupId, linodeId } = useParams({ + strict: false, + }); const [linode, setLinode] = React.useState( selectedLinode @@ -38,10 +37,12 @@ export const PlacementGroupsUnassignModal = (props: Props) => { error, isPending, mutateAsync: unassignLinodes, - } = useUnassignLinodesFromPlacementGroup(+placementGroupId); + } = useUnassignLinodesFromPlacementGroup( + placementGroupId ? +placementGroupId : -1 + ); const { data: linodeFromQuery, isFetching } = useLinodeQuery( - +linodeId, + linodeId ? +linodeId : -1, open && selectedLinode === undefined ); @@ -69,7 +70,7 @@ export const PlacementGroupsUnassignModal = (props: Props) => { const isLinodeReadOnly = useIsResourceRestricted({ grantLevel: 'read_write', grantType: 'linode', - id: +linodeId, + id: linodeId ? +linodeId : -1, }); const actions = ( diff --git a/packages/manager/src/features/PlacementGroups/constants.ts b/packages/manager/src/features/PlacementGroups/constants.ts index 506556f8120..2c73079f10f 100644 --- a/packages/manager/src/features/PlacementGroups/constants.ts +++ b/packages/manager/src/features/PlacementGroups/constants.ts @@ -45,3 +45,9 @@ export const PLACEMENT_GROUP_MIGRATION_INBOUND_MESSAGE = export const PLACEMENT_GROUP_MIGRATION_OUTBOUND_MESSAGE = 'This Linode is being migrated. It will be removed from this placement group after the migration completes.'; + +export const PLACEMENT_GROUPS_LANDING_ROUTE = '/placement-groups'; +// default order constants +export const PG_LANDING_TABLE_DEFAULT_ORDER = 'asc'; +export const PG_LANDING_TABLE_DEFAULT_ORDER_BY = 'label'; +export const PG_LANDING_TABLE_PREFERENCE_KEY = 'placement-groups'; diff --git a/packages/manager/src/routes/placementGroups/index.ts b/packages/manager/src/routes/placementGroups/index.ts index 95f62bc597f..2a2af0e1c5e 100644 --- a/packages/manager/src/routes/placementGroups/index.ts +++ b/packages/manager/src/routes/placementGroups/index.ts @@ -1,8 +1,21 @@ -import { createRoute } from '@tanstack/react-router'; +import { createRoute, redirect } from '@tanstack/react-router'; import { rootRoute } from '../root'; import { PlacementGroupsRoute } from './PlacementGroupsRoute'; +import type { TableSearchParams } from '../types'; + +export interface PlacementGroupsSearchParams extends TableSearchParams { + query?: string; +} + +const placementGroupAction = { + delete: 'delete', + edit: 'edit', +} as const; + +export type PlacementGroupAction = typeof placementGroupAction[keyof typeof placementGroupAction]; + export const placementGroupsRoute = createRoute({ component: PlacementGroupsRoute, getParentRoute: () => rootRoute, @@ -12,6 +25,7 @@ export const placementGroupsRoute = createRoute({ const placementGroupsIndexRoute = createRoute({ getParentRoute: () => placementGroupsRoute, path: '/', + validateSearch: (search: PlacementGroupsSearchParams) => search, }).lazy(() => import('./placementGroupsLazyRoutes').then( (m) => m.placementGroupsLandingLazyRoute @@ -27,24 +41,33 @@ const placementGroupsCreateRoute = createRoute({ ) ); -const placementGroupsEditRoute = createRoute({ - getParentRoute: () => placementGroupsRoute, - parseParams: (params) => ({ - id: Number(params.id), - }), - path: 'edit/$id', -}).lazy(() => - import('./placementGroupsLazyRoutes').then( - (m) => m.placementGroupsLandingLazyRoute - ) -); +type PlacementGroupActionRouteParams

= { + action: PlacementGroupAction; + id: P; +}; -const placementGroupsDeleteRoute = createRoute({ +const placementGroupActionRoute = createRoute({ + beforeLoad: async ({ params }) => { + if (!(params.action in placementGroupAction)) { + throw redirect({ + search: () => ({}), + to: '/placement-groups', + }); + } + }, getParentRoute: () => placementGroupsRoute, - parseParams: (params) => ({ - id: Number(params.id), - }), - path: 'delete/$id', + params: { + parse: ({ action, id }: PlacementGroupActionRouteParams) => ({ + action, + id: Number(id), + }), + stringify: ({ action, id }: PlacementGroupActionRouteParams) => ({ + action, + id: String(id), + }), + }, + path: '$action/$id', + validateSearch: (search: PlacementGroupsSearchParams) => search, }).lazy(() => import('./placementGroupsLazyRoutes').then( (m) => m.placementGroupsLandingLazyRoute @@ -94,10 +117,8 @@ const placementGroupsUnassignRoute = createRoute({ ); export const placementGroupsRouteTree = placementGroupsRoute.addChildren([ - placementGroupsIndexRoute, + placementGroupsIndexRoute.addChildren([placementGroupActionRoute]), placementGroupsCreateRoute, - placementGroupsEditRoute, - placementGroupsDeleteRoute, placementGroupsDetailRoute.addChildren([ placementGroupsDetailLinodesRoute, placementGroupsAssignRoute, From 53d20f68a4dbf590141df7a170b27c3518b3548e Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Tue, 7 Jan 2025 10:52:50 -0500 Subject: [PATCH 06/16] switch PG create back to react router dom (used in LinodeCreate as well) getting closer revert rest of createDrawer router changes maybe a bit closer?? fix landing state prevent query from firing when NaN trying to add routing for search and order by pg linodes table --- .../PlacementGroupsCreateDrawer.test.tsx | 27 ++- .../PlacementGroupsCreateDrawer.tsx | 7 +- .../PlacementGroupsDetail.test.tsx | 16 +- .../PlacementGroupsDetail.tsx | 2 +- .../PlacementGroupsLinodes.test.tsx | 14 ++ .../PlacementGroupsLinodes.tsx | 36 +++- .../PlacementGroupsLinodesTable.test.tsx | 14 ++ .../PlacementGroupsLinodesTable.tsx | 185 +++++++++++------- .../PlacementGroupsLinodesTableRow.test.tsx | 12 ++ .../PlacementGroupsDetailPanel.test.tsx | 28 ++- .../PlacementGroupsEditDrawer.test.tsx | 8 +- .../PlacementGroupsEditDrawer.tsx | 9 +- .../PlacementGroupsLanding.test.tsx | 48 ++++- .../PlacementGroupsLanding.tsx | 2 +- .../PlacementGroupsUnassignModal.test.tsx | 4 +- .../PlacementGroupsUnassignModal.tsx | 2 +- .../src/features/PlacementGroups/constants.ts | 2 + .../src/features/PlacementGroups/types.ts | 2 +- 18 files changed, 297 insertions(+), 121 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.test.tsx index a132d0a5195..f0102f7a4b9 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.test.tsx @@ -2,7 +2,8 @@ import { fireEvent, waitFor } from '@testing-library/react'; import * as React from 'react'; import { placementGroupFactory } from 'src/factories'; -import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; +// eslint-disable-next-line no-restricted-imports +import { renderWithTheme } from 'src/utilities/testHelpers'; import { PlacementGroupsCreateDrawer } from './PlacementGroupsCreateDrawer'; @@ -31,12 +32,8 @@ vi.mock('src/queries/placementGroups', async () => { }); describe('PlacementGroupsCreateDrawer', () => { - it('should render and have its fields enabled', async () => { - const { - getAllByRole, - getByLabelText, - getByText, - } = await renderWithThemeAndRouter( + it('should render and have its fields enabled', () => { + const { getAllByRole, getByLabelText, getByText } = renderWithTheme( ); @@ -50,8 +47,8 @@ describe('PlacementGroupsCreateDrawer', () => { expect(radioInputs[0]).toBeChecked(); }); - it('Placement Group Type select should have the correct options', async () => { - const { getByPlaceholderText, getByText } = await renderWithThemeAndRouter( + it('Placement Group Type select should have the correct options', () => { + const { getByPlaceholderText, getByText } = renderWithTheme( ); @@ -66,13 +63,15 @@ describe('PlacementGroupsCreateDrawer', () => { }); it('should populate the region select with the selected region prop', async () => { - const { getByText } = await renderWithThemeAndRouter( + const { getByText } = renderWithTheme( , { - initialRoute: 'linodes/create', + MemoryRouter: { + initialEntries: ['/linodes/create'], + }, } ); @@ -87,9 +86,7 @@ describe('PlacementGroupsCreateDrawer', () => { getByPlaceholderText, getByRole, getByText, - } = await renderWithThemeAndRouter( - - ); + } = renderWithTheme(); fireEvent.change(getByLabelText('Label'), { target: { value: 'my-label' }, @@ -124,7 +121,7 @@ describe('PlacementGroupsCreateDrawer', () => { data: [placementGroupFactory.build({ region: 'us-west' })], }); const regionWithoutCapacity = 'US, Fremont, CA (us-west)'; - const { getByPlaceholderText, getByText } = await renderWithThemeAndRouter( + const { getByPlaceholderText, getByText } = renderWithTheme( ); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.tsx index 5a440a3eebb..5d52c888a7d 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsCreateDrawer.tsx @@ -8,10 +8,11 @@ import { Typography, } from '@linode/ui'; import { createPlacementGroupSchema } from '@linode/validation'; -import { useLocation } from '@tanstack/react-router'; import { useFormik } from 'formik'; import { useSnackbar } from 'notistack'; import * as React from 'react'; +// eslint-disable-next-line no-restricted-imports +import { useLocation } from 'react-router-dom'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { DescriptionList } from 'src/components/DescriptionList/DescriptionList'; @@ -73,9 +74,7 @@ export const PlacementGroupsCreateDrawer = ( const location = useLocation(); const isFromLinodeCreate = location.pathname.includes('/linodes/create'); - const queryParams = getQueryParamsFromQueryString( - location.pathname.split('?')[1] // todo connie - fix this to be more robust, but just getting rid of type errors for now - ); + const queryParams = getQueryParamsFromQueryString(location.search); const handleRegionSelect = (region: Region['id']) => { setFieldValue('region', region); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx index c6f1b6f2a05..f91ca836baf 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx @@ -11,6 +11,7 @@ import { PlacementGroupsDetail } from './PlacementGroupsDetail'; const queryMocks = vi.hoisted(() => ({ useAllLinodesQuery: vi.fn().mockReturnValue({}), + useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), useParams: vi.fn().mockReturnValue({}), usePlacementGroupQuery: vi.fn().mockReturnValue({}), useRegionsQuery: vi.fn().mockReturnValue({}), @@ -32,11 +33,19 @@ vi.mock('src/queries/linodes/linodes', async () => { }; }); +vi.mock('@tanstack/react-router', async () => { + const actual = await vi.importActual('@tanstack/react-router'); + return { + ...actual, + useParams: queryMocks.useParams, + }; +}); + vi.mock('react-router-dom', async () => { const actual = await vi.importActual('react-router-dom'); return { ...actual, - useParams: queryMocks.useParams, + useLocation: queryMocks.useLocation, }; }); @@ -51,7 +60,10 @@ vi.mock('src/queries/regions/regions', async () => { describe('PlacementGroupsLanding', () => { it('renders a error page', async () => { const { getByText } = await renderWithThemeAndRouter( - + , + { + initialRoute: '/placement-groups/1', + } ); expect(getByText('Not Found')).toBeInTheDocument(); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx index e59399b4e12..86c6d0e56af 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx @@ -1,7 +1,7 @@ import { PLACEMENT_GROUP_TYPES } from '@linode/api-v4'; import { CircleProgress, Notice } from '@linode/ui'; -import * as React from 'react'; import { useParams } from '@tanstack/react-router'; +import * as React from 'react'; import { DocumentTitleSegment } from 'src/components/DocumentTitle'; import { ErrorState } from 'src/components/ErrorState/ErrorState'; diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx index 90a233b97d0..72ed4fbecab 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx @@ -6,6 +6,20 @@ import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PLACEMENT_GROUP_LINODES_ERROR_MESSAGE } from '../../constants'; import { PlacementGroupsLinodes } from './PlacementGroupsLinodes'; +const queryMocks = vi.hoisted(() => ({ + useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), + useParams: vi.fn().mockReturnValue({}), +})); + +vi.mock('@tanstack/react-router', async () => { + const actual = await vi.importActual('@tanstack/react-router'); + return { + ...actual, + useLocation: queryMocks.useLocation, + useParams: queryMocks.useParams, + }; +}); + describe('PlacementGroupsLinodes', () => { it('renders an error state if placement groups are undefined', async () => { const { getByText } = await renderWithThemeAndRouter( diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx index f2ef1a19687..4569f519b9c 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx @@ -1,6 +1,6 @@ import { Button, Stack } from '@linode/ui'; import Grid from '@mui/material/Unstable_Grid2/Grid2'; -import { useLocation, useNavigate } from '@tanstack/react-router'; +import { useLocation, useNavigate, useSearch } from '@tanstack/react-router'; import * as React from 'react'; import { DebouncedSearchTextField } from 'src/components/DebouncedSearchTextField'; @@ -10,12 +10,14 @@ import { hasPlacementGroupReachedCapacity } from 'src/features/PlacementGroups/u import { MAX_NUMBER_OF_LINODES_IN_PLACEMENT_GROUP_MESSAGE, PLACEMENT_GROUP_LINODES_ERROR_MESSAGE, + PLACEMENT_GROUPS_DETAILS_ROUTE, } from '../../constants'; import { PlacementGroupsAssignLinodesDrawer } from '../../PlacementGroupsAssignLinodesDrawer'; import { PlacementGroupsUnassignModal } from '../../PlacementGroupsUnassignModal'; import { PlacementGroupsLinodesTable } from './PlacementGroupsLinodesTable'; import type { Linode, PlacementGroup, Region } from '@linode/api-v4'; +import type { PlacementGroupsSearchParams } from 'src/routes/placementGroups'; interface Props { assignedLinodes: Linode[] | undefined; @@ -25,6 +27,8 @@ interface Props { region: Region | undefined; } +type PlacementGroupLinodesSearchParams = PlacementGroupsSearchParams; + export const PlacementGroupsLinodes = (props: Props) => { const { assignedLinodes, @@ -35,7 +39,10 @@ export const PlacementGroupsLinodes = (props: Props) => { } = props; const navigate = useNavigate(); const location = useLocation(); - const [searchText, setSearchText] = React.useState(''); + const search: PlacementGroupLinodesSearchParams = useSearch({ + from: PLACEMENT_GROUPS_DETAILS_ROUTE, + }); + const { query } = search; const [selectedLinode, setSelectedLinode] = React.useState< Linode | undefined >(); @@ -49,15 +56,27 @@ export const PlacementGroupsLinodes = (props: Props) => { return []; } - if (searchText) { + if (query) { return assignedLinodes.filter((linode: Linode) => { - return linode.label.toLowerCase().includes(searchText.toLowerCase()); + return linode.label.toLowerCase().includes(query.toLowerCase()); }); } return assignedLinodes; }; + const onSearch = (searchString: string) => { + navigate({ + params: { id: placementGroup.id }, + search: (prev) => ({ + ...prev, + page: undefined, + query: searchString || undefined, + }), + to: PLACEMENT_GROUPS_DETAILS_ROUTE, + }); + }; + const hasReachedCapacity = hasPlacementGroupReachedCapacity({ placementGroup, region, @@ -66,6 +85,7 @@ export const PlacementGroupsLinodes = (props: Props) => { const handleAssignLinodesDrawer = () => { navigate({ params: { id: placementGroup.id }, + search: (prev) => prev, to: '/placement-groups/$id/linodes/assign', }); }; @@ -73,6 +93,7 @@ export const PlacementGroupsLinodes = (props: Props) => { setSelectedLinode(linode); navigate({ params: { id: placementGroup.id, linodeId: linode.id }, + search: (prev) => prev, to: '/placement-groups/$id/linodes/unassign/$linodeId', }); }; @@ -80,7 +101,8 @@ export const PlacementGroupsLinodes = (props: Props) => { setSelectedLinode(undefined); navigate({ params: { id: placementGroup.id }, - to: '/placement-groups/$id', + search: (prev) => prev, + to: PLACEMENT_GROUPS_DETAILS_ROUTE, }); }; const isAssignLinodesDrawerOpen = location.pathname.includes('/assign'); @@ -95,9 +117,9 @@ export const PlacementGroupsLinodes = (props: Props) => { debounceTime={250} hideLabel label="Search Linodes" - onSearch={setSearchText} + onSearch={onSearch} placeholder="Search Linodes" - value={searchText} + value={query ?? ''} /> diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.test.tsx index a8e42ece979..3257b09d46a 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.test.tsx @@ -5,6 +5,20 @@ import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PlacementGroupsLinodesTable } from './PlacementGroupsLinodesTable'; +const queryMocks = vi.hoisted(() => ({ + useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), + useParams: vi.fn().mockReturnValue({}), +})); + +vi.mock('@tanstack/react-router', async () => { + const actual = await vi.importActual('@tanstack/react-router'); + return { + ...actual, + useLocation: queryMocks.useLocation, + useParams: queryMocks.useParams, + }; +}); + const defaultProps = { error: [], handleUnassignLinodeModal: vi.fn(), diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.tsx index ca59ee861c0..e2418cfb6b3 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.tsx @@ -1,6 +1,8 @@ +// yeah we're gonna have to get rid of that equals +import { equals } from 'ramda'; import * as React from 'react'; -import OrderBy from 'src/components/OrderBy'; +import { sortData } from 'src/components/OrderBy'; import Paginate from 'src/components/Paginate'; import { PaginationFooter } from 'src/components/PaginationFooter/PaginationFooter'; import { Table } from 'src/components/Table'; @@ -10,9 +12,16 @@ import { TableContentWrapper } from 'src/components/TableContentWrapper/TableCon import { TableHead } from 'src/components/TableHead'; import { TableRow } from 'src/components/TableRow'; import { TableSortCell } from 'src/components/TableSortCell'; +import { useOrderV2 } from 'src/hooks/useOrderV2'; +import { usePrevious } from 'src/hooks/usePrevious'; import { getAPIErrorOrDefault } from 'src/utilities/errorUtils'; -import { PLACEMENT_GROUP_LINODES_ERROR_MESSAGE } from '../../constants'; +import { + PG_LANDING_TABLE_DEFAULT_ORDER, + PG_LINODES_TABLE_PREFERENCE_KEY, + PLACEMENT_GROUP_LINODES_ERROR_MESSAGE, + PLACEMENT_GROUPS_DETAILS_ROUTE, +} from '../../constants'; import { PlacementGroupsLinodesTableRow } from './PlacementGroupsLinodesTableRow'; import type { APIError, Linode } from '@linode/api-v4'; @@ -35,79 +44,115 @@ export const PlacementGroupsLinodesTable = React.memo((props: Props) => { const orderLinodeKey = 'label'; const orderStatusKey = 'status'; + const { handleOrderChange, order, orderBy } = useOrderV2({ + initialRoute: { + defaultOrder: { + order: PG_LANDING_TABLE_DEFAULT_ORDER, + orderBy: orderLinodeKey, + }, + from: PLACEMENT_GROUPS_DETAILS_ROUTE, + }, + preferenceKey: `${PG_LINODES_TABLE_PREFERENCE_KEY}-order`, + }); + + // definitely gonna look at this but currently trying to see what works + // ----------- + // Stash a copy of the previous data for equality check. + const prevData = usePrevious(linodes); + + // Our working copy of the data to be sorted. + const dataToSort = React.useRef(linodes); + + // If `props.data` has changed, that's the data we should sort. + // + // Note: I really don't like this equality check that runs every render, but + // I have yet to find a another solution. + if (!equals(linodes, prevData)) { + dataToSort.current = linodes; + } + + // SORT THE DATA! + const sortedData = sortData(orderBy, order)(dataToSort.current); + + // Save this – this is what will be sorted next time around, if e.g. the order + // or orderBy keys change. In that case we don't want to start from scratch + // and sort `props.data`. That might result in odd UI behavior depending on + // the data. See: https://github.com/linode/manager/pull/6855. + dataToSort.current = sortedData; + // ----------- const _error = error ? getAPIErrorOrDefault(error, PLACEMENT_GROUP_LINODES_ERROR_MESSAGE) : undefined; return ( - - {({ data: orderedData, handleOrderChange, order, orderBy }) => ( - - {({ - count, - data: paginatedAndOrderedLinodes, - handlePageChange, - handlePageSizeChange, - page, - pageSize, - }) => ( - <> -

- - - - Linode - - - Status - - - - - - - {paginatedAndOrderedLinodes.map((linode) => ( - - ))} - - -
- - - )} - + // + // {({ data: orderedData, handleOrderChange, order, orderBy }) => ( + + {({ + count, + data: paginatedAndOrderedLinodes, + handlePageChange, + handlePageSizeChange, + page, + pageSize, + }) => ( + <> + + + + + Linode + + + Status + + + + + + + {paginatedAndOrderedLinodes.map((linode) => ( + + ))} + + +
+ + )} - + + // )} + // ); }); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx index 527b5a83a3b..ed3c04a3b8a 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx @@ -6,6 +6,18 @@ import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PlacementGroupsLinodesTableRow } from './PlacementGroupsLinodesTableRow'; +const queryMocks = vi.hoisted(() => ({ + useParams: vi.fn().mockReturnValue({}), +})); + +vi.mock('@tanstack/react-router', async () => { + const actual = await vi.importActual('@tanstack/react-router'); + return { + ...actual, + useParams: queryMocks.useParams, + }; +}); + const defaultProps = { handleUnassignLinodeModal: vi.fn(), linode: linodeFactory.build({ diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx index cb4b6c3e5d0..cba19912aaf 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx @@ -12,9 +12,27 @@ const defaultProps = { const queryMocks = vi.hoisted(() => ({ useAllPlacementGroupsQuery: vi.fn().mockReturnValue({}), + useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), + useParams: vi.fn().mockReturnValue({}), useRegionsQuery: vi.fn().mockReturnValue({}), })); +vi.mock('react-router-dom', async () => { + const actual = await vi.importActual('react-router-dom'); + return { + ...actual, + useLocation: queryMocks.useLocation, + }; +}); + +vi.mock('@tanstack/react-router', async () => { + const actual = await vi.importActual('@tanstack/react-router'); + return { + ...actual, + useParams: queryMocks.useParams, + }; +}); + vi.mock('src/queries/regions/regions', async () => { const actual = await vi.importActual('src/queries/regions/regions'); return { @@ -61,10 +79,8 @@ describe('PlacementGroupsDetailPanel', () => { queryMocks.useAllPlacementGroupsQuery.mockReturnValue({ data: [ placementGroupFactory.build({ - placement_group_type: 'affinity:local', id: 1, is_compliant: true, - placement_group_policy: 'strict', label: 'my-placement-group', members: [ { @@ -72,6 +88,8 @@ describe('PlacementGroupsDetailPanel', () => { linode_id: 1, }, ], + placement_group_policy: 'strict', + placement_group_type: 'affinity:local', region: 'us-west', }), ], @@ -104,7 +122,11 @@ describe('PlacementGroupsDetailPanel', () => { }); it('should have its select disabled and no create PG button when provided a region without PG capability', async () => { - const { getByRole, getByTestId, queryByRole } = await renderWithThemeAndRouter( + const { + getByRole, + getByTestId, + queryByRole, + } = await renderWithThemeAndRouter( ({ useParams: vi.fn().mockReturnValue({}), })); -vi.mock('react-router-dom', async () => { - const actual = await vi.importActual('react-router-dom'); +vi.mock('@tanstack/react-router', async () => { + const actual = await vi.importActual('@tanstack/react-router'); return { ...actual, useParams: queryMocks.useParams, @@ -66,7 +66,7 @@ describe('PlacementGroupsCreateDrawer', () => { const editButton = getByRole('button', { name: 'Edit' }); expect(editButton).toBeEnabled(); - fireEvent.click(editButton); + await userEvent.click(editButton); expect(queryMocks.useMutatePlacementGroup).toHaveBeenCalled(); }); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx index 9e142f92247..4d6790c6a5d 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx @@ -4,10 +4,10 @@ import { } from '@linode/api-v4'; import { CircleProgress, Divider, Notice, Stack, TextField } from '@linode/ui'; import { updatePlacementGroupSchema } from '@linode/validation'; +import { useParams } from '@tanstack/react-router'; import { useFormik } from 'formik'; import { useSnackbar } from 'notistack'; import * as React from 'react'; -import { useParams } from '@tanstack/react-router'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { DescriptionList } from 'src/components/DescriptionList/DescriptionList'; @@ -36,8 +36,7 @@ export const PlacementGroupsEditDrawer = ( region, selectedPlacementGroup: placementGroupFromProps, } = props; - // todo connie - try to consolidate params/remove params from drawers? - // figure out error with NaN + // todo connie - try to consolidate params/remove params from drawers / switch to dialog props const params = useParams({ strict: false }); const { data: placementGroupFromParam, @@ -45,7 +44,9 @@ export const PlacementGroupsEditDrawer = ( status, } = usePlacementGroupQuery( Number(params.id), - open && placementGroupFromProps === undefined + open && + placementGroupFromProps === undefined && + !Number.isNaN(Number(params.id)) ); const placementGroup = React.useMemo( diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx index 7484e71fb03..a7dd0490e78 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx @@ -7,9 +7,27 @@ import { PlacementGroupsLanding } from './PlacementGroupsLanding'; import { headers } from './PlacementGroupsLandingEmptyStateData'; const queryMocks = vi.hoisted(() => ({ + useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups' }), + useParams: vi.fn().mockReturnValue({}), usePlacementGroupsQuery: vi.fn().mockReturnValue({}), })); +vi.mock('@tanstack/react-router', async () => { + const actual = await vi.importActual('@tanstack/react-router'); + return { + ...actual, + useParams: queryMocks.useParams, + }; +}); + +vi.mock('react-router-dom', async () => { + const actual = await vi.importActual('react-router-dom'); + return { + ...actual, + useLocation: queryMocks.useLocation, + }; +}); + vi.mock('src/queries/placementGroups', async () => { const actual = await vi.importActual('src/queries/placementGroups'); return { @@ -25,7 +43,10 @@ describe('PlacementGroupsLanding', () => { }); const { getByRole } = await renderWithThemeAndRouter( - + , + { + initialRoute: '/placement-groups', + } ); expect(getByRole('progressbar')).toBeInTheDocument(); @@ -37,7 +58,10 @@ describe('PlacementGroupsLanding', () => { }); const { getByText } = await renderWithThemeAndRouter( - + , + { + initialRoute: '/placement-groups', + } ); expect(getByText(/not found/i)).toBeInTheDocument(); @@ -56,7 +80,10 @@ describe('PlacementGroupsLanding', () => { }); const { getByText } = await renderWithThemeAndRouter( - + , + { + initialRoute: '/placement-groups', + } ); expect(getByText(/create placement group/i)).toBeInTheDocument(); @@ -79,7 +106,10 @@ describe('PlacementGroupsLanding', () => { }); const { getByText } = await renderWithThemeAndRouter( - + , + { + initialRoute: '/placement-groups', + } ); expect(getByText(/group 1/i)).toBeInTheDocument(); @@ -95,7 +125,10 @@ describe('PlacementGroupsLanding', () => { }); const { getByText } = await renderWithThemeAndRouter( - + , + { + initialRoute: '/placement-groups', + } ); expect(getByText(headers.description)).toBeInTheDocument(); @@ -110,7 +143,10 @@ describe('PlacementGroupsLanding', () => { }); const { getByText } = await renderWithThemeAndRouter( - + , + { + initialRoute: '/placement-groups', + } ); expect(getByText('Getting Started Guides')).toBeInTheDocument(); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx index ff26a01fdf9..984ef269996 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx @@ -168,7 +168,7 @@ export const PlacementGroupsLanding = React.memo(() => { return ; } - if (placementGroups?.results === 0 && query === '') { + if (placementGroups?.results === 0 && !query) { return ( <> ({ useParams: vi.fn().mockReturnValue({}), })); -vi.mock('react-router-dom', async () => { - const actual = await vi.importActual('react-router-dom'); +vi.mock('@tanstack/react-router', async () => { + const actual = await vi.importActual('@tanstack/react-router'); return { ...actual, useParams: queryMocks.useParams, diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx index a0b59cce925..4b58a5c5211 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx @@ -1,7 +1,7 @@ import { CircleProgress, Notice, Typography } from '@linode/ui'; +import { useParams } from '@tanstack/react-router'; import { useSnackbar } from 'notistack'; import * as React from 'react'; -import { useParams } from '@tanstack/react-router'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { ConfirmationDialog } from 'src/components/ConfirmationDialog/ConfirmationDialog'; diff --git a/packages/manager/src/features/PlacementGroups/constants.ts b/packages/manager/src/features/PlacementGroups/constants.ts index 2c73079f10f..5605877fbed 100644 --- a/packages/manager/src/features/PlacementGroups/constants.ts +++ b/packages/manager/src/features/PlacementGroups/constants.ts @@ -47,7 +47,9 @@ export const PLACEMENT_GROUP_MIGRATION_OUTBOUND_MESSAGE = 'This Linode is being migrated. It will be removed from this placement group after the migration completes.'; export const PLACEMENT_GROUPS_LANDING_ROUTE = '/placement-groups'; +export const PLACEMENT_GROUPS_DETAILS_ROUTE = '/placement-groups/$id'; // default order constants export const PG_LANDING_TABLE_DEFAULT_ORDER = 'asc'; export const PG_LANDING_TABLE_DEFAULT_ORDER_BY = 'label'; export const PG_LANDING_TABLE_PREFERENCE_KEY = 'placement-groups'; +export const PG_LINODES_TABLE_PREFERENCE_KEY = 'placement-groups-linodes'; diff --git a/packages/manager/src/features/PlacementGroups/types.ts b/packages/manager/src/features/PlacementGroups/types.ts index 15e78d12e3b..2363ad85216 100644 --- a/packages/manager/src/features/PlacementGroups/types.ts +++ b/packages/manager/src/features/PlacementGroups/types.ts @@ -1,4 +1,4 @@ -import { PlacementGroup, Region } from '@linode/api-v4'; +import type { PlacementGroup, Region } from '@linode/api-v4'; export interface PlacementGroupsDrawerPropsBase { onClose: () => void; From c65f6882a7627a8cce71e6a374351c67b83f7227 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Wed, 8 Jan 2025 21:55:08 -0500 Subject: [PATCH 07/16] drawer/modal for pg landing --- .../PlacementGroupsDeleteModal.test.tsx | 2 ++ .../PlacementGroupsDeleteModal.tsx | 35 +++---------------- .../PlacementGroupsEditDrawer.test.tsx | 1 + .../PlacementGroupsEditDrawer.tsx | 32 +++-------------- .../PlacementGroupsLanding.tsx | 30 ++++++++++------ .../src/features/PlacementGroups/types.ts | 1 + 6 files changed, 33 insertions(+), 68 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx index f91552263e4..43bce0ff666 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.test.tsx @@ -77,6 +77,7 @@ describe('PlacementGroupsDeleteModal', () => { region: 'us-east', })} disableUnassignButton={false} + isFetching={false} /> ); @@ -119,6 +120,7 @@ describe('PlacementGroupsDeleteModal', () => { placement_group_type: 'anti_affinity:local', })} disableUnassignButton={false} + isFetching={false} /> ); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.tsx index 31a2111eb91..3f5892ad665 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.tsx @@ -1,15 +1,7 @@ -import { - Button, - CircleProgress, - List, - ListItem, - Notice, - Typography, -} from '@linode/ui'; +import { Button, List, ListItem, Notice, Typography } from '@linode/ui'; import { useSnackbar } from 'notistack'; import * as React from 'react'; -import { ConfirmationDialog } from 'src/components/ConfirmationDialog/ConfirmationDialog'; import { RemovableSelectionsList } from 'src/components/RemovableSelectionsList/RemovableSelectionsList'; import { TypeToConfirmDialog } from 'src/components/TypeToConfirmDialog/TypeToConfirmDialog'; import { @@ -28,6 +20,7 @@ import type { ButtonProps } from '@linode/ui'; interface Props { disableUnassignButton: boolean; + isFetching: boolean; linodes: Linode[] | undefined; onClose: () => void; open: boolean; @@ -37,6 +30,7 @@ interface Props { export const PlacementGroupsDeleteModal = (props: Props) => { const { disableUnassignButton, + isFetching, linodes, onClose, open, @@ -105,28 +99,6 @@ export const PlacementGroupsDeleteModal = (props: Props) => { return null; } - if (!assignedLinodes) { - return ( - .MuiDialogContent-root > div': { - maxHeight: 300, - padding: 4, - }, - maxHeight: 500, - width: 500, - }, - }} - onClose={handleClose} - open={open} - title="Delete Placement Group" - > - - - ); - } - return ( { disableTypeToConfirmInput={isDisabled} disableTypeToConfirmSubmit={isDisabled} expand + isFetching={isFetching} label="Placement Group" loading={deletePlacementLoading} onClick={onDelete} diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.test.tsx index 7bd881f1a70..a6d03dcce4c 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.test.tsx @@ -47,6 +47,7 @@ describe('PlacementGroupsCreateDrawer', () => { region: 'us-east', })} disableEditButton={false} + isFetching={false} onClose={vi.fn()} onPlacementGroupEdit={vi.fn()} open={true} diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx index 4d6790c6a5d..91f8940ce3c 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.tsx @@ -2,9 +2,8 @@ import { PLACEMENT_GROUP_POLICIES, PLACEMENT_GROUP_TYPES, } from '@linode/api-v4'; -import { CircleProgress, Divider, Notice, Stack, TextField } from '@linode/ui'; +import { Divider, Notice, Stack, TextField } from '@linode/ui'; import { updatePlacementGroupSchema } from '@linode/validation'; -import { useParams } from '@tanstack/react-router'; import { useFormik } from 'formik'; import { useSnackbar } from 'notistack'; import * as React from 'react'; @@ -14,10 +13,7 @@ import { DescriptionList } from 'src/components/DescriptionList/DescriptionList' import { Drawer } from 'src/components/Drawer'; import { NotFound } from 'src/components/NotFound'; import { useFormValidateOnChange } from 'src/hooks/useFormValidateOnChange'; -import { - useMutatePlacementGroup, - usePlacementGroupQuery, -} from 'src/queries/placementGroups'; +import { useMutatePlacementGroup } from 'src/queries/placementGroups'; import { getFormikErrorsFromAPIErrors } from 'src/utilities/formikErrorUtils'; import { scrollErrorIntoView } from 'src/utilities/scrollErrorIntoView'; @@ -30,30 +26,13 @@ export const PlacementGroupsEditDrawer = ( ) => { const { disableEditButton, + isFetching, onClose, onPlacementGroupEdit, open, region, - selectedPlacementGroup: placementGroupFromProps, + selectedPlacementGroup: placementGroup, } = props; - // todo connie - try to consolidate params/remove params from drawers / switch to dialog props - const params = useParams({ strict: false }); - const { - data: placementGroupFromParam, - isFetching, - status, - } = usePlacementGroupQuery( - Number(params.id), - open && - placementGroupFromProps === undefined && - !Number.isNaN(Number(params.id)) - ); - - const placementGroup = React.useMemo( - () => - open ? placementGroupFromProps ?? placementGroupFromParam : undefined, - [open, placementGroupFromProps, placementGroupFromParam] - ); const { error, mutateAsync } = useMutatePlacementGroup( placementGroup?.id ?? -1 @@ -127,6 +106,7 @@ export const PlacementGroupsEditDrawer = ( ? `Edit Placement Group ${placementGroup.label}` : 'Edit Placement Group' } + isFetching={isFetching} onClose={handleClose} open={open} > @@ -192,8 +172,6 @@ export const PlacementGroupsEditDrawer = ( - ) : isFetching ? ( - ) : status === 'error' ? ( ) : null} diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx index 984ef269996..6246a6fdd9c 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx @@ -21,11 +21,15 @@ import { TableRow } from 'src/components/TableRow'; import { TableRowEmpty } from 'src/components/TableRowEmpty/TableRowEmpty'; import { TableSortCell } from 'src/components/TableSortCell/TableSortCell'; import { getRestrictedResourceText } from 'src/features/Account/utils'; +import { useDialogData } from 'src/hooks/useDialogData'; import { useOrderV2 } from 'src/hooks/useOrderV2'; import { usePaginationV2 } from 'src/hooks/usePaginationV2'; import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck'; import { useAllLinodesQuery } from 'src/queries/linodes/linodes'; -import { usePlacementGroupsQuery } from 'src/queries/placementGroups'; +import { + usePlacementGroupQuery, + usePlacementGroupsQuery, +} from 'src/queries/placementGroups'; import { useRegionsQuery } from 'src/queries/regions/regions'; import { getAPIErrorOrDefault } from 'src/utilities/errorUtils'; @@ -94,10 +98,6 @@ export const PlacementGroupsLanding = React.memo(() => { filter ); - const selectedPlacementGroup = placementGroups?.data.find( - (pg) => pg.id === Number(params.id) - ); - const allLinodeIDsAssigned = placementGroups?.data.reduce( (acc, placementGroup) => { return acc.concat( @@ -107,13 +107,23 @@ export const PlacementGroupsLanding = React.memo(() => { [] as number[] ); - const { data: linodes } = useAllLinodesQuery( + const { data: linodes, isFetching: isFetchingLinodes } = useAllLinodesQuery( {}, { '+or': allLinodeIDsAssigned?.map((linodeId) => ({ id: linodeId })), } ); + const { + data: selectedPlacementGroup, + isFetching: isFetchingPlacementGroup, + } = useDialogData({ + enabled: !!params.id, + paramKey: 'id', + queryHook: usePlacementGroupQuery, + redirectToOnNotFound: '/placement-groups', + }); + const { data: regions } = useRegionsQuery(); const getPlacementGroupRegion = ( placementGroup: PlacementGroup | undefined @@ -150,8 +160,6 @@ export const PlacementGroupsLanding = React.memo(() => { }; const isPlacementGroupCreateDrawerOpen = location.pathname.endsWith('create'); - const isPlacementGroupDeleteModalOpen = location.pathname.includes('delete'); - const isPlacementGroupEditDrawerOpen = location.pathname.includes('edit'); const onSearch = (searchString: string) => { navigate({ @@ -307,16 +315,18 @@ export const PlacementGroupsLanding = React.memo(() => { /> diff --git a/packages/manager/src/features/PlacementGroups/types.ts b/packages/manager/src/features/PlacementGroups/types.ts index 2363ad85216..dc264c25074 100644 --- a/packages/manager/src/features/PlacementGroups/types.ts +++ b/packages/manager/src/features/PlacementGroups/types.ts @@ -15,6 +15,7 @@ export interface PlacementGroupsCreateDrawerProps { export interface PlacementGroupsEditDrawerProps { disableEditButton: boolean; + isFetching: boolean; onClose: PlacementGroupsDrawerPropsBase['onClose']; onPlacementGroupEdit?: (placementGroup: PlacementGroup) => void; open: PlacementGroupsDrawerPropsBase['open']; From d7fea4683787271968c1c1f6513c534010dc3d63 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Thu, 9 Jan 2025 11:39:00 -0500 Subject: [PATCH 08/16] feeling a lot better about this updated order by pg linodes table --- .../PlacementGroupsDetail.tsx | 15 ----- .../PlacementGroupsLinodes.test.tsx | 4 -- .../PlacementGroupsLinodes.tsx | 65 ++++++++++++------- .../PlacementGroupsLinodesTable.test.tsx | 7 ++ .../PlacementGroupsLinodesTable.tsx | 63 ++++-------------- 5 files changed, 58 insertions(+), 96 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx index 86c6d0e56af..8c6615aaf15 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx @@ -9,7 +9,6 @@ import { LandingHeader } from 'src/components/LandingHeader'; import { NotFound } from 'src/components/NotFound'; import { getRestrictedResourceText } from 'src/features/Account/utils'; import { useRestrictedGlobalGrantCheck } from 'src/hooks/useRestrictedGlobalGrantCheck'; -import { useAllLinodesQuery } from 'src/queries/linodes/linodes'; import { useMutatePlacementGroup, usePlacementGroupQuery, @@ -30,14 +29,6 @@ export const PlacementGroupsDetail = () => { error: placementGroupError, isLoading, } = usePlacementGroupQuery(placementGroupId); - const { data: linodes, isFetching: isFetchingLinodes } = useAllLinodesQuery( - {}, - { - '+or': placementGroup?.members.map((member) => ({ - id: member.linode_id, - })), - } - ); const { data: regions } = useRegionsQuery(); const region = regions?.find( @@ -70,10 +61,6 @@ export const PlacementGroupsDetail = () => { ); } - const assignedLinodes = linodes?.filter((linode) => - placementGroup?.members.some((pgLinode) => pgLinode.linode_id === linode.id) - ); - const { label, placement_group_type } = placementGroup; const resetEditableLabel = () => { @@ -124,8 +111,6 @@ export const PlacementGroupsDetail = () => { )} { it('renders an error state if placement groups are undefined', async () => { const { getByText } = await renderWithThemeAndRouter( { const { getByPlaceholderText, getByRole } = await renderWithThemeAndRouter( { - const { - assignedLinodes, - isFetchingLinodes, - isLinodeReadOnly, - placementGroup, - region, - } = props; + const { isLinodeReadOnly, placementGroup, region } = props; const navigate = useNavigate(); const location = useLocation(); + const search: PlacementGroupLinodesSearchParams = useSearch({ from: PLACEMENT_GROUPS_DETAILS_ROUTE, }); const { query } = search; + + const { handleOrderChange, order, orderBy } = useOrderV2({ + initialRoute: { + defaultOrder: { + order: PG_LANDING_TABLE_DEFAULT_ORDER, + orderBy: PG_LANDING_TABLE_DEFAULT_ORDER_BY, + }, + from: PLACEMENT_GROUPS_DETAILS_ROUTE, + }, + preferenceKey: `${PG_LINODES_TABLE_PREFERENCE_KEY}-order`, + }); + + const filter: Filter = { + ['+or']: placementGroup?.members.map((member) => ({ + id: member.linode_id, + })), + ['+order']: order, + ['+order_by']: orderBy, + ...(query && { label: { '+contains': query } }), + }; + + const { data: linodes, isFetching: isFetchingLinodes } = useAllLinodesQuery( + {}, + filter + ); + + const assignedLinodes = linodes?.filter((linode) => + placementGroup?.members.some((pgLinode) => pgLinode.linode_id === linode.id) + ); + const [selectedLinode, setSelectedLinode] = React.useState< Linode | undefined >(); @@ -51,20 +79,6 @@ export const PlacementGroupsLinodes = (props: Props) => { return ; } - const getLinodesList = () => { - if (!assignedLinodes) { - return []; - } - - if (query) { - return assignedLinodes.filter((linode: Linode) => { - return linode.label.toLowerCase().includes(query.toLowerCase()); - }); - } - - return assignedLinodes; - }; - const onSearch = (searchString: string) => { navigate({ params: { id: placementGroup.id }, @@ -141,7 +155,8 @@ export const PlacementGroupsLinodes = (props: Props) => { ({ useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), useParams: vi.fn().mockReturnValue({}), @@ -24,6 +26,11 @@ const defaultProps = { handleUnassignLinodeModal: vi.fn(), isFetchingLinodes: false, linodes: linodeFactory.buildList(5), + orderByProps: { + handleOrderChange: vi.fn(), + order: 'asc' as Order, + orderBy: 'label', + }, }; describe('PlacementGroupsLinodesTable', () => { diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.tsx index e2418cfb6b3..15ec77dcedf 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.tsx @@ -1,8 +1,5 @@ -// yeah we're gonna have to get rid of that equals -import { equals } from 'ramda'; import * as React from 'react'; -import { sortData } from 'src/components/OrderBy'; import Paginate from 'src/components/Paginate'; import { PaginationFooter } from 'src/components/PaginationFooter/PaginationFooter'; import { Table } from 'src/components/Table'; @@ -12,25 +9,24 @@ import { TableContentWrapper } from 'src/components/TableContentWrapper/TableCon import { TableHead } from 'src/components/TableHead'; import { TableRow } from 'src/components/TableRow'; import { TableSortCell } from 'src/components/TableSortCell'; -import { useOrderV2 } from 'src/hooks/useOrderV2'; -import { usePrevious } from 'src/hooks/usePrevious'; import { getAPIErrorOrDefault } from 'src/utilities/errorUtils'; -import { - PG_LANDING_TABLE_DEFAULT_ORDER, - PG_LINODES_TABLE_PREFERENCE_KEY, - PLACEMENT_GROUP_LINODES_ERROR_MESSAGE, - PLACEMENT_GROUPS_DETAILS_ROUTE, -} from '../../constants'; +import { PLACEMENT_GROUP_LINODES_ERROR_MESSAGE } from '../../constants'; import { PlacementGroupsLinodesTableRow } from './PlacementGroupsLinodesTableRow'; import type { APIError, Linode } from '@linode/api-v4'; +import type { Order } from 'src/hooks/useOrderV2'; export interface Props { error?: APIError[]; handleUnassignLinodeModal: (linode: Linode) => void; isFetchingLinodes: boolean; linodes: Linode[]; + orderByProps: { + handleOrderChange: (newOrderBy: string, newOrder: Order) => void; + order: Order; + orderBy: string; + }; } export const PlacementGroupsLinodesTable = React.memo((props: Props) => { @@ -39,55 +35,20 @@ export const PlacementGroupsLinodesTable = React.memo((props: Props) => { handleUnassignLinodeModal, isFetchingLinodes, linodes, + orderByProps, } = props; + const { handleOrderChange, order, orderBy } = orderByProps; + const orderLinodeKey = 'label'; const orderStatusKey = 'status'; - const { handleOrderChange, order, orderBy } = useOrderV2({ - initialRoute: { - defaultOrder: { - order: PG_LANDING_TABLE_DEFAULT_ORDER, - orderBy: orderLinodeKey, - }, - from: PLACEMENT_GROUPS_DETAILS_ROUTE, - }, - preferenceKey: `${PG_LINODES_TABLE_PREFERENCE_KEY}-order`, - }); - - // definitely gonna look at this but currently trying to see what works - // ----------- - // Stash a copy of the previous data for equality check. - const prevData = usePrevious(linodes); - - // Our working copy of the data to be sorted. - const dataToSort = React.useRef(linodes); - - // If `props.data` has changed, that's the data we should sort. - // - // Note: I really don't like this equality check that runs every render, but - // I have yet to find a another solution. - if (!equals(linodes, prevData)) { - dataToSort.current = linodes; - } - - // SORT THE DATA! - const sortedData = sortData(orderBy, order)(dataToSort.current); - - // Save this – this is what will be sorted next time around, if e.g. the order - // or orderBy keys change. In that case we don't want to start from scratch - // and sort `props.data`. That might result in odd UI behavior depending on - // the data. See: https://github.com/linode/manager/pull/6855. - dataToSort.current = sortedData; - // ----------- const _error = error ? getAPIErrorOrDefault(error, PLACEMENT_GROUP_LINODES_ERROR_MESSAGE) : undefined; return ( - // - // {({ data: orderedData, handleOrderChange, order, orderBy }) => ( - + {({ count, data: paginatedAndOrderedLinodes, @@ -152,7 +113,5 @@ export const PlacementGroupsLinodesTable = React.memo((props: Props) => { )} - // )} - // ); }); From 610ed4b371cab1b0d3b7b5e7a1e93270dde7254f Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Fri, 10 Jan 2025 10:08:53 -0500 Subject: [PATCH 09/16] pg linodes table - pagination and dialog data --- .../PlacementGroupsLinodes.tsx | 53 +++++++-- .../PlacementGroupsLinodesTable.tsx | 106 +++++++----------- .../PlacementGroupsUnassignModal.test.tsx | 1 + .../PlacementGroupsUnassignModal.tsx | 45 +------- 4 files changed, 88 insertions(+), 117 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx index cd77d1f3ab9..6cb78154a4c 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx @@ -1,13 +1,24 @@ import { Button, Stack } from '@linode/ui'; import Grid from '@mui/material/Unstable_Grid2/Grid2'; -import { useLocation, useNavigate, useSearch } from '@tanstack/react-router'; +import { + useLocation, + useNavigate, + useParams, + useSearch, +} from '@tanstack/react-router'; import * as React from 'react'; import { DebouncedSearchTextField } from 'src/components/DebouncedSearchTextField'; import { ErrorState } from 'src/components/ErrorState/ErrorState'; +import { PaginationFooter } from 'src/components/PaginationFooter/PaginationFooter'; import { hasPlacementGroupReachedCapacity } from 'src/features/PlacementGroups/utils'; +import { useDialogData } from 'src/hooks/useDialogData'; import { useOrderV2 } from 'src/hooks/useOrderV2'; -import { useAllLinodesQuery } from 'src/queries/linodes/linodes'; +import { usePaginationV2 } from 'src/hooks/usePaginationV2'; +import { + useAllLinodesQuery, + useLinodeQuery, +} from 'src/queries/linodes/linodes'; import { MAX_NUMBER_OF_LINODES_IN_PLACEMENT_GROUP_MESSAGE, @@ -36,6 +47,7 @@ export const PlacementGroupsLinodes = (props: Props) => { const { isLinodeReadOnly, placementGroup, region } = props; const navigate = useNavigate(); const location = useLocation(); + const params = useParams({ strict: false }); const search: PlacementGroupLinodesSearchParams = useSearch({ from: PLACEMENT_GROUPS_DETAILS_ROUTE, @@ -53,6 +65,15 @@ export const PlacementGroupsLinodes = (props: Props) => { preferenceKey: `${PG_LINODES_TABLE_PREFERENCE_KEY}-order`, }); + const pagination = usePaginationV2({ + currentRoute: PLACEMENT_GROUPS_DETAILS_ROUTE, + preferenceKey: PG_LINODES_TABLE_PREFERENCE_KEY, + searchParams: (prev) => ({ + ...prev, + query: search.query, + }), + }); + const filter: Filter = { ['+or']: placementGroup?.members.map((member) => ({ id: member.linode_id, @@ -63,7 +84,10 @@ export const PlacementGroupsLinodes = (props: Props) => { }; const { data: linodes, isFetching: isFetchingLinodes } = useAllLinodesQuery( - {}, + { + page: pagination.page, + page_size: pagination.pageSize, + }, filter ); @@ -71,9 +95,12 @@ export const PlacementGroupsLinodes = (props: Props) => { placementGroup?.members.some((pgLinode) => pgLinode.linode_id === linode.id) ); - const [selectedLinode, setSelectedLinode] = React.useState< - Linode | undefined - >(); + const { data: selectedLinode, isFetching: isFetchingLinode } = useDialogData({ + enabled: !!params.linodeId, + paramKey: 'linodeId', + queryHook: useLinodeQuery, + redirectToOnNotFound: '/placement-groups/$id', + }); if (!placementGroup) { return ; @@ -103,22 +130,23 @@ export const PlacementGroupsLinodes = (props: Props) => { to: '/placement-groups/$id/linodes/assign', }); }; + const handleUnassignLinodeModal = (linode: Linode) => { - setSelectedLinode(linode); navigate({ params: { id: placementGroup.id, linodeId: linode.id }, search: (prev) => prev, to: '/placement-groups/$id/linodes/unassign/$linodeId', }); }; + const handleCloseDrawer = () => { - setSelectedLinode(undefined); navigate({ params: { id: placementGroup.id }, search: (prev) => prev, to: PLACEMENT_GROUPS_DETAILS_ROUTE, }); }; + const isAssignLinodesDrawerOpen = location.pathname.includes('/assign'); const isUnassignLinodesDrawerOpen = location.pathname.includes('/unassign'); @@ -158,6 +186,14 @@ export const PlacementGroupsLinodes = (props: Props) => { linodes={assignedLinodes ?? []} orderByProps={{ handleOrderChange, order, orderBy }} /> + { selectedPlacementGroup={placementGroup} /> { const { handleOrderChange, order, orderBy } = orderByProps; const orderLinodeKey = 'label'; - const orderStatusKey = 'status'; const _error = error ? getAPIErrorOrDefault(error, PLACEMENT_GROUP_LINODES_ERROR_MESSAGE) : undefined; return ( - - {({ - count, - data: paginatedAndOrderedLinodes, - handlePageChange, - handlePageSizeChange, - page, - pageSize, - }) => ( - <> - - - - - Linode - - - Status - - - - - - - {paginatedAndOrderedLinodes.map((linode) => ( - - ))} - - -
- - - )} - + + + + + Linode + + + Status + + + + + + + {linodes.map((linode) => ( + + ))} + + +
); }); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.test.tsx index 1ea69515107..97e527c9c96 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.test.tsx @@ -41,6 +41,7 @@ describe('PlacementGroupsUnassignModal', () => { const { getByLabelText, getByRole } = await renderWithThemeAndRouter( null} open selectedLinode={undefined} diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx index 4b58a5c5211..02e748e8c5b 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx @@ -1,13 +1,11 @@ -import { CircleProgress, Notice, Typography } from '@linode/ui'; +import { Notice, Typography } from '@linode/ui'; import { useParams } from '@tanstack/react-router'; import { useSnackbar } from 'notistack'; import * as React from 'react'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { ConfirmationDialog } from 'src/components/ConfirmationDialog/ConfirmationDialog'; -import { NotFound } from 'src/components/NotFound'; import { useIsResourceRestricted } from 'src/hooks/useIsResourceRestricted'; -import { useLinodeQuery } from 'src/queries/linodes/linodes'; import { useUnassignLinodesFromPlacementGroup } from 'src/queries/placementGroups'; import type { @@ -16,23 +14,20 @@ import type { } from '@linode/api-v4'; interface Props { + isFetching: boolean; onClose: () => void; open: boolean; selectedLinode: Linode | undefined; } export const PlacementGroupsUnassignModal = (props: Props) => { - const { onClose, open, selectedLinode } = props; + const { isFetching, onClose, open, selectedLinode: linode } = props; const { enqueueSnackbar } = useSnackbar(); const { id: placementGroupId, linodeId } = useParams({ strict: false, }); - const [linode, setLinode] = React.useState( - selectedLinode - ); - const { error, isPending, @@ -41,17 +36,6 @@ export const PlacementGroupsUnassignModal = (props: Props) => { placementGroupId ? +placementGroupId : -1 ); - const { data: linodeFromQuery, isFetching } = useLinodeQuery( - linodeId ? +linodeId : -1, - open && selectedLinode === undefined - ); - - React.useEffect(() => { - if (open) { - setLinode(selectedLinode ?? linodeFromQuery); - } - }, [selectedLinode, linodeFromQuery, open]); - const payload: UnassignLinodesFromPlacementGroupPayload = { linodes: [linode?.id ?? -1], }; @@ -89,32 +73,11 @@ export const PlacementGroupsUnassignModal = (props: Props) => { /> ); - if (!linode) { - return ( - .MuiDialogContent-root > div': { - maxHeight: 300, - padding: 4, - }, - maxHeight: 500, - width: 500, - }, - }} - onClose={onClose} - open={open} - title="Delete Placement Group" - > - {isFetching ? : } - - ); - } - return ( Date: Fri, 10 Jan 2025 10:37:56 -0500 Subject: [PATCH 10/16] update failing e2e --- .../core/placementGroups/delete-placement-groups.spec.ts | 7 ++++++- .../placement-groups-linode-assignment.spec.ts | 1 + .../placementGroups/update-placement-group-label.spec.ts | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts b/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts index d065bf1140f..7d3451f3a74 100644 --- a/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts +++ b/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts @@ -5,6 +5,7 @@ import { mockGetAccount } from 'support/intercepts/account'; import { mockDeletePlacementGroup, + mockGetPlacementGroup, mockGetPlacementGroups, mockUnassignPlacementGroupLinodes, mockDeletePlacementGroupError, @@ -62,6 +63,7 @@ describe('Placement Group deletion', () => { }); mockGetPlacementGroups([mockPlacementGroup]).as('getPlacementGroups'); + mockGetPlacementGroup(mockPlacementGroup).as('getPlacementGroup'); cy.visitWithLogin('/placement-groups'); cy.wait('@getPlacementGroups'); @@ -135,7 +137,7 @@ describe('Placement Group deletion', () => { * - Confirms that UI automatically updates to reflect deleted Placement Group. * - Confirms that user can retry and continue with unassignment when unexpected error happens. */ - it('can delete with Linodes assigned when unexpected error show up and retry', () => { + it.only('can delete with Linodes assigned when unexpected error show up and retry', () => { const mockPlacementGroupRegion = chooseRegion(); // Linodes that are assigned to the Placement Group being deleted. @@ -172,6 +174,7 @@ describe('Placement Group deletion', () => { mockGetPlacementGroups([mockPlacementGroup, secondMockPlacementGroup]).as( 'getPlacementGroups' ); + mockGetPlacementGroup(mockPlacementGroup).as('getPlacementGroup'); cy.visitWithLogin('/placement-groups'); cy.wait(['@getPlacementGroups', '@getLinodes']); @@ -363,6 +366,7 @@ describe('Placement Group deletion', () => { }); mockGetPlacementGroups([mockPlacementGroup]).as('getPlacementGroups'); + mockGetPlacementGroup(mockPlacementGroup).as('getPlacementGroup'); cy.visitWithLogin('/placement-groups'); cy.wait('@getPlacementGroups'); @@ -488,6 +492,7 @@ describe('Placement Group deletion', () => { mockGetPlacementGroups([mockPlacementGroup, secondMockPlacementGroup]).as( 'getPlacementGroups' ); + mockGetPlacementGroup(mockPlacementGroup).as('getPlacementGroup'); cy.visitWithLogin('/placement-groups'); cy.wait(['@getPlacementGroups', '@getLinodes']); diff --git a/packages/manager/cypress/e2e/core/placementGroups/placement-groups-linode-assignment.spec.ts b/packages/manager/cypress/e2e/core/placementGroups/placement-groups-linode-assignment.spec.ts index 0c52a148848..3f0bd28aa7d 100644 --- a/packages/manager/cypress/e2e/core/placementGroups/placement-groups-linode-assignment.spec.ts +++ b/packages/manager/cypress/e2e/core/placementGroups/placement-groups-linode-assignment.spec.ts @@ -378,6 +378,7 @@ describe('Placement Groups Linode assignment', () => { mockGetRegions(mockRegions); mockGetLinodes(mockLinodes); + mockGetLinodeDetails(mockLinodeUnassigned.id, mockLinodeUnassigned); mockGetPlacementGroups([mockPlacementGroup]); mockGetPlacementGroup(mockPlacementGroup).as('getPlacementGroup'); diff --git a/packages/manager/cypress/e2e/core/placementGroups/update-placement-group-label.spec.ts b/packages/manager/cypress/e2e/core/placementGroups/update-placement-group-label.spec.ts index b857c69e1ca..de6c903c887 100644 --- a/packages/manager/cypress/e2e/core/placementGroups/update-placement-group-label.spec.ts +++ b/packages/manager/cypress/e2e/core/placementGroups/update-placement-group-label.spec.ts @@ -4,6 +4,7 @@ import { randomLabel, randomNumber } from 'support/util/random'; import { + mockGetPlacementGroup, mockGetPlacementGroups, mockUpdatePlacementGroup, mockUpdatePlacementGroupError, @@ -48,6 +49,7 @@ describe('Placement Group update label flow', () => { }; mockGetPlacementGroups([mockPlacementGroup]).as('getPlacementGroups'); + mockGetPlacementGroup(mockPlacementGroup).as('getPlacementGroup'); mockUpdatePlacementGroup( mockPlacementGroup.id, @@ -114,6 +116,7 @@ describe('Placement Group update label flow', () => { }; mockGetPlacementGroups([mockPlacementGroup]).as('getPlacementGroups'); + mockGetPlacementGroup(mockPlacementGroup).as('getPlacementGroup'); mockUpdatePlacementGroupError( mockPlacementGroup.id, From 5e70be57cb871d04598402f3389a5cbf59417335 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Fri, 10 Jan 2025 11:38:42 -0500 Subject: [PATCH 11/16] more unit tests passing than ever before :) --- packages/manager/.eslintrc.cjs | 6 ----- .../delete-placement-groups.spec.ts | 2 +- ...lacementGroupsAssignLinodesDrawer.test.tsx | 14 ++++------- .../PlacementGroupsCreateDrawer.test.tsx | 1 - .../PlacementGroupsDeleteModal.test.tsx | 12 +++------ .../PlacementGroupsLinodesTable.test.tsx | 18 ++++++------- .../PlacementGroupsLinodesTableRow.test.tsx | 6 ++--- .../PlacementGroupsSummary.test.tsx | 8 +++--- .../PlacementGroupsDetailPanel.test.tsx | 22 +++++++--------- .../PlacementGroupsEditDrawer.test.tsx | 8 ++---- .../PlacementGroupsRow.test.tsx | 10 +++----- .../PlacementGroupsUnassignModal.test.tsx | 25 ++++++------------- 12 files changed, 47 insertions(+), 85 deletions(-) diff --git a/packages/manager/.eslintrc.cjs b/packages/manager/.eslintrc.cjs index 2543d144a41..73ce5518bd7 100644 --- a/packages/manager/.eslintrc.cjs +++ b/packages/manager/.eslintrc.cjs @@ -123,12 +123,6 @@ module.exports = { 'Please use routing utilities from @tanstack/react-router.', name: 'react-router-dom', }, - { - importNames: ['renderWithTheme'], - message: - 'Please use the wrapWithThemeAndRouter helper function for testing components being migrated to TanStack Router.', - name: 'src/utilities/testHelpers', - }, ], }, ], diff --git a/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts b/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts index 7d3451f3a74..cc8fba5d5b9 100644 --- a/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts +++ b/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts @@ -137,7 +137,7 @@ describe('Placement Group deletion', () => { * - Confirms that UI automatically updates to reflect deleted Placement Group. * - Confirms that user can retry and continue with unassignment when unexpected error happens. */ - it.only('can delete with Linodes assigned when unexpected error show up and retry', () => { + it('can delete with Linodes assigned when unexpected error show up and retry', () => { const mockPlacementGroupRegion = chooseRegion(); // Linodes that are assigned to the Placement Group being deleted. diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsAssignLinodesDrawer.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsAssignLinodesDrawer.test.tsx index 5876631c0e4..f45b85219a3 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsAssignLinodesDrawer.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsAssignLinodesDrawer.test.tsx @@ -6,7 +6,7 @@ import { placementGroupFactory, regionFactory, } from 'src/factories'; -import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; +import { renderWithTheme } from 'src/utilities/testHelpers'; import { PlacementGroupsAssignLinodesDrawer } from './PlacementGroupsAssignLinodesDrawer'; @@ -42,12 +42,12 @@ vi.mock('src/queries/placementGroups', async () => { }); describe('PlacementGroupsAssignLinodesDrawer', () => { - it('should render the error state', async () => { + it('should render the error state', () => { queryMocks.useAllLinodesQuery.mockReturnValue({ error: [{ reason: 'Not found' }], }); - const { container } = await renderWithThemeAndRouter( + const { container } = renderWithTheme( { expect(container).toBeEmptyDOMElement(); }); - it('should render the drawer components', async () => { + it('should render the drawer components', () => { queryMocks.useAllLinodesQuery.mockReturnValue({ data: [ linodeFactory.build({ id: 1, label: 'Linode-1', region: 'us-east' }), @@ -117,11 +117,7 @@ describe('PlacementGroupsAssignLinodesDrawer', () => { }) ); - const { - getByPlaceholderText, - getByRole, - getByText, - } = await renderWithThemeAndRouter( + const { getByPlaceholderText, getByRole, getByText } = renderWithTheme( { - it('should render the right form elements', async () => { + it('should render the right form elements', () => { queryMocks.usePreferences.mockReturnValue({ data: preference, }); - const { - getByRole, - getByTestId, - getByText, - } = await renderWithThemeAndRouter( + const { getByRole, getByTestId, getByText } = renderWithTheme( { data: preference, }); - const { getByRole, getByTestId } = await renderWithThemeAndRouter( + const { getByRole, getByTestId } = renderWithTheme( { - it('renders an error state when encountering an API error', async () => { - const { getByText } = await renderWithThemeAndRouter( + it('renders an error state when encountering an API error', () => { + const { getByText } = renderWithTheme( { expect(getByText(/not found/i)).toBeInTheDocument(); }); - it('renders a loading skeleton based on the loading prop', async () => { - const { getByTestId } = await renderWithThemeAndRouter( + it('renders a loading skeleton based on the loading prop', () => { + const { getByTestId } = renderWithTheme( ); expect(getByTestId('table-row-loading')).toBeInTheDocument(); }); - it('should have the correct number of columns', async () => { - const { getAllByRole } = await renderWithThemeAndRouter( + it('should have the correct number of columns', () => { + const { getAllByRole } = renderWithTheme( ); expect(getAllByRole('columnheader')).toHaveLength(3); }); - it('should have the correct number of rows', async () => { - const { getAllByTestId } = await renderWithThemeAndRouter( + it('should have the correct number of rows', () => { + const { getAllByTestId } = renderWithTheme( ); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx index ed3c04a3b8a..befbd550601 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTableRow.test.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import { linodeFactory } from 'src/factories'; import { wrapWithTableBody } from 'src/utilities/testHelpers'; -import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; +import { renderWithTheme } from 'src/utilities/testHelpers'; import { PlacementGroupsLinodesTableRow } from './PlacementGroupsLinodesTableRow'; @@ -27,8 +27,8 @@ const defaultProps = { }; describe('PlacementGroupsLinodesTableRow', () => { - it('should feature the right table row data', async () => { - const { getAllByRole } = await renderWithThemeAndRouter( + it('should feature the right table row data', () => { + const { getAllByRole } = renderWithTheme( wrapWithTableBody() ); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx index a22f4d6f729..27922913f5c 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsSummary/PlacementGroupsSummary.test.tsx @@ -1,16 +1,15 @@ import * as React from 'react'; import { placementGroupFactory, regionFactory } from 'src/factories'; -import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; +import { renderWithTheme } from 'src/utilities/testHelpers'; import { PlacementGroupsSummary } from './PlacementGroupsSummary'; describe('PlacementGroups Summary', () => { - it('renders the placement group detail summary panel', async () => { - const { getByTestId, getByText } = await renderWithThemeAndRouter( + it('renders the placement group detail summary panel', () => { + const { getByTestId, getByText } = renderWithTheme( { linode_id: 10, }, ], + placement_group_type: 'affinity:local', region: 'us-east', })} region={regionFactory.build({ diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx index cba19912aaf..59535b7c2b5 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { placementGroupFactory, regionFactory } from 'src/factories'; -import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; +import { renderWithTheme } from 'src/utilities/testHelpers'; import { PlacementGroupsDetailPanel } from './PlacementGroupsDetailPanel'; @@ -96,8 +96,8 @@ describe('PlacementGroupsDetailPanel', () => { }); }); - it('should have its select disabled and no create PG button on initial render', async () => { - const { getByRole, queryByRole } = await renderWithThemeAndRouter( + it('should have its select disabled and no create PG button on initial render', () => { + const { getByRole, queryByRole } = renderWithTheme( ); @@ -107,8 +107,8 @@ describe('PlacementGroupsDetailPanel', () => { ).not.toBeInTheDocument(); }); - it('should have its select enabled and a create PG button when provided a region', async () => { - const { getByRole } = await renderWithThemeAndRouter( + it('should have its select enabled and a create PG button when provided a region', () => { + const { getByRole } = renderWithTheme( { ).toBeEnabled(); }); - it('should have its select disabled and no create PG button when provided a region without PG capability', async () => { - const { - getByRole, - getByTestId, - queryByRole, - } = await renderWithThemeAndRouter( + it('should have its select disabled and no create PG button when provided a region without PG capability', () => { + const { getByRole, getByTestId, queryByRole } = renderWithTheme( { ).not.toBeInTheDocument(); }); - it('should have its PG select enabled and Create Placement Group button disabled if the region has reached its PG capacity', async () => { - const { getByPlaceholderText, getByRole } = await renderWithThemeAndRouter( + it('should have its PG select enabled and Create Placement Group button disabled if the region has reached its PG capacity', () => { + const { getByPlaceholderText, getByRole } = renderWithTheme( { it('should render, have the proper fields populated with PG values, and have uneditable fields disabled', async () => { queryMocks.useParams.mockReturnValue({ id: '1' }); - const { - getByLabelText, - getByRole, - getByText, - } = await renderWithThemeAndRouter( + const { getByLabelText, getByRole, getByText } = renderWithTheme( { - it('renders the columns with proper data', async () => { + it('renders the columns with proper data', () => { resizeScreenSize(1200); - const { - getByRole, - getByTestId, - getByText, - } = await renderWithThemeAndRouter( + const { getByRole, getByTestId, getByText } = renderWithTheme( wrapWithTableBody( ({ - useLinodeQuery: vi.fn().mockReturnValue({}), useParams: vi.fn().mockReturnValue({}), })); @@ -18,33 +17,23 @@ vi.mock('@tanstack/react-router', async () => { }; }); -vi.mock('src/queries/linodes/linodes', async () => { - const actual = await vi.importActual('src/queries/linodes/linodes'); - return { - ...actual, - useLinodeQuery: queryMocks.useLinodeQuery, - }; -}); - describe('PlacementGroupsUnassignModal', () => { - it('should render and have the proper content and CTAs', async () => { - queryMocks.useLinodeQuery.mockReturnValue({ - data: linodeFactory.build({ - id: 1, - label: 'test-linode', - }), + it('should render and have the proper content and CTAs', () => { + const linode = linodeFactory.build({ + id: 1, + label: 'test-linode', }); queryMocks.useParams.mockReturnValue({ id: '1', linodeId: '1', }); - const { getByLabelText, getByRole } = await renderWithThemeAndRouter( + const { getByLabelText, getByRole } = renderWithTheme( null} open - selectedLinode={undefined} + selectedLinode={linode} /> ); From 589f7e8a19e011472436ca0568fd7b5f5425b3b3 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Fri, 10 Jan 2025 12:04:12 -0500 Subject: [PATCH 12/16] testing pls send help --- .../PlacementGroupsLinodes.test.tsx | 11 ++++++++--- .../PlacementGroupsLanding.test.tsx | 9 +++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx index 1171b580c82..ab6dd9db0b2 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx @@ -5,10 +5,11 @@ import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PLACEMENT_GROUP_LINODES_ERROR_MESSAGE } from '../../constants'; import { PlacementGroupsLinodes } from './PlacementGroupsLinodes'; +import { migrationRouteTree } from 'src/routes'; const queryMocks = vi.hoisted(() => ({ useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), - useParams: vi.fn().mockReturnValue({}), + useParams: vi.fn().mockReturnValue({ id: 1 }), })); vi.mock('@tanstack/react-router', async () => { @@ -21,13 +22,17 @@ vi.mock('@tanstack/react-router', async () => { }); describe('PlacementGroupsLinodes', () => { - it('renders an error state if placement groups are undefined', async () => { + it.only('renders an error state if placement groups are undefined', async () => { const { getByText } = await renderWithThemeAndRouter( + />, + { + routeTree: migrationRouteTree, + initialRoute: '/placement-groups/1' + } ); expect( diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx index a7dd0490e78..f28c8bebaef 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx @@ -5,6 +5,7 @@ import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PlacementGroupsLanding } from './PlacementGroupsLanding'; import { headers } from './PlacementGroupsLandingEmptyStateData'; +import { TanstackLink } from 'src/components/TanstackLinks'; const queryMocks = vi.hoisted(() => ({ useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups' }), @@ -36,6 +37,14 @@ vi.mock('src/queries/placementGroups', async () => { }; }); +vi.mock('src/components/Link', async () => { + const actual = await vi.importActual('src/components/Link'); + return { + ...actual, + Link: TanstackLink, + }; +}); + describe('PlacementGroupsLanding', () => { it('renders loading state', async () => { queryMocks.usePlacementGroupsQuery.mockReturnValue({ From b2c89202e752038318019d8d61f1a36ade7ae104 Mon Sep 17 00:00:00 2001 From: Alban Bailly Date: Fri, 10 Jan 2025 13:23:33 -0500 Subject: [PATCH 13/16] test cleanup --- .../PlacementGroupsDetail.test.tsx | 39 ++++----- .../PlacementGroupsDetail.tsx | 3 +- .../PlacementGroupsLinodes.test.tsx | 13 ++- .../PlacementGroupsLanding.test.tsx | 17 ---- packages/manager/src/routes/routes.test.tsx | 87 ------------------- .../manager/src/utilities/testHelpers.tsx | 5 +- 6 files changed, 27 insertions(+), 137 deletions(-) delete mode 100644 packages/manager/src/routes/routes.test.tsx diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx index f91ca836baf..eb41720fde0 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx @@ -12,9 +12,11 @@ import { PlacementGroupsDetail } from './PlacementGroupsDetail'; const queryMocks = vi.hoisted(() => ({ useAllLinodesQuery: vi.fn().mockReturnValue({}), useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), - useParams: vi.fn().mockReturnValue({}), + useNavigate: vi.fn(), + useParams: vi.fn().mockReturnValue({ id: 1 }), usePlacementGroupQuery: vi.fn().mockReturnValue({}), useRegionsQuery: vi.fn().mockReturnValue({}), + useSearch: vi.fn().mockReturnValue({ query: undefined }), })); vi.mock('src/queries/placementGroups', async () => { @@ -37,15 +39,9 @@ vi.mock('@tanstack/react-router', async () => { const actual = await vi.importActual('@tanstack/react-router'); return { ...actual, + useNavigate: queryMocks.useNavigate, useParams: queryMocks.useParams, - }; -}); - -vi.mock('react-router-dom', async () => { - const actual = await vi.importActual('react-router-dom'); - return { - ...actual, - useLocation: queryMocks.useLocation, + useSearch: queryMocks.useSearch, }; }); @@ -57,13 +53,10 @@ vi.mock('src/queries/regions/regions', async () => { }; }); -describe('PlacementGroupsLanding', () => { +describe('PlacementGroupsDetail', () => { it('renders a error page', async () => { const { getByText } = await renderWithThemeAndRouter( - , - { - initialRoute: '/placement-groups/1', - } + ); expect(getByText('Not Found')).toBeInTheDocument(); @@ -74,7 +67,6 @@ describe('PlacementGroupsLanding', () => { data: placementGroupFactory.build({ id: 1, }), - isLoading: true, }); queryMocks.useAllLinodesQuery.mockReturnValue({ @@ -95,10 +87,7 @@ describe('PlacementGroupsLanding', () => { }); const { getByRole } = await renderWithThemeAndRouter( - , - { - initialRoute: '/placement-groups/1', - } + ); expect(getByRole('progressbar')).toBeInTheDocument(); @@ -113,12 +102,16 @@ describe('PlacementGroupsLanding', () => { placement_group_type: 'anti_affinity:local', }), }); + queryMocks.useAllLinodesQuery.mockReturnValue({ + data: [], + isLoading: false, + page: 1, + pages: 1, + results: 0, + }); const { getByText } = await renderWithThemeAndRouter( - , - { - initialRoute: '/placement-groups/1', - } + ); expect(getByText(/my first pg/i)).toBeInTheDocument(); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx index 8c6615aaf15..8c222d02f25 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.tsx @@ -21,8 +21,7 @@ import { PlacementGroupsLinodes } from './PlacementGroupsLinodes/PlacementGroups import { PlacementGroupsSummary } from './PlacementGroupsSummary/PlacementGroupsSummary'; export const PlacementGroupsDetail = () => { - const { id } = useParams({ from: '/placement-groups/$id' }); - const placementGroupId = +id; + const { id: placementGroupId } = useParams({ from: '/placement-groups/$id' }); const { data: placementGroup, diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx index ab6dd9db0b2..f87ff9b79de 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx @@ -5,11 +5,12 @@ import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PLACEMENT_GROUP_LINODES_ERROR_MESSAGE } from '../../constants'; import { PlacementGroupsLinodes } from './PlacementGroupsLinodes'; -import { migrationRouteTree } from 'src/routes'; const queryMocks = vi.hoisted(() => ({ useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), + useNavigate: vi.fn(), useParams: vi.fn().mockReturnValue({ id: 1 }), + useSearch: vi.fn().mockReturnValue({ query: undefined }), })); vi.mock('@tanstack/react-router', async () => { @@ -17,22 +18,20 @@ vi.mock('@tanstack/react-router', async () => { return { ...actual, useLocation: queryMocks.useLocation, + useNavigate: queryMocks.useNavigate, useParams: queryMocks.useParams, + useSearch: queryMocks.useSearch, }; }); describe('PlacementGroupsLinodes', () => { - it.only('renders an error state if placement groups are undefined', async () => { + it('renders an error state if placement groups are undefined', async () => { const { getByText } = await renderWithThemeAndRouter( , - { - routeTree: migrationRouteTree, - initialRoute: '/placement-groups/1' - } + /> ); expect( diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx index f28c8bebaef..60fc0f40a4f 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.test.tsx @@ -5,7 +5,6 @@ import { renderWithThemeAndRouter } from 'src/utilities/testHelpers'; import { PlacementGroupsLanding } from './PlacementGroupsLanding'; import { headers } from './PlacementGroupsLandingEmptyStateData'; -import { TanstackLink } from 'src/components/TanstackLinks'; const queryMocks = vi.hoisted(() => ({ useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups' }), @@ -21,14 +20,6 @@ vi.mock('@tanstack/react-router', async () => { }; }); -vi.mock('react-router-dom', async () => { - const actual = await vi.importActual('react-router-dom'); - return { - ...actual, - useLocation: queryMocks.useLocation, - }; -}); - vi.mock('src/queries/placementGroups', async () => { const actual = await vi.importActual('src/queries/placementGroups'); return { @@ -37,14 +28,6 @@ vi.mock('src/queries/placementGroups', async () => { }; }); -vi.mock('src/components/Link', async () => { - const actual = await vi.importActual('src/components/Link'); - return { - ...actual, - Link: TanstackLink, - }; -}); - describe('PlacementGroupsLanding', () => { it('renders loading state', async () => { queryMocks.usePlacementGroupsQuery.mockReturnValue({ diff --git a/packages/manager/src/routes/routes.test.tsx b/packages/manager/src/routes/routes.test.tsx deleted file mode 100644 index b760499ec50..00000000000 --- a/packages/manager/src/routes/routes.test.tsx +++ /dev/null @@ -1,87 +0,0 @@ -import { RouterProvider } from '@tanstack/react-router'; -import { screen, waitFor } from '@testing-library/react'; -import React from 'react'; - -import { renderWithTheme } from 'src/utilities/testHelpers'; - -import { migrationRouter } from './index'; -import { getAllRoutePaths } from './utils/allPaths'; - -import type { useQuery } from '@tanstack/react-query'; -// TODO: Tanstack Router - replace AnyRouter once migration is complete. -import type { AnyRouter } from '@tanstack/react-router'; - -vi.mock('@tanstack/react-query', async () => { - const actual = await vi.importActual('@tanstack/react-query'); - return { - ...actual, - useQuery: vi - .fn() - .mockImplementation((...args: Parameters) => { - const actualResult = (actual.useQuery as typeof useQuery)(...args); - return { - ...actualResult, - isLoading: false, - }; - }), - }; -}); - -const allMigrationPaths = getAllRoutePaths(migrationRouter); - -describe('Migration Router', () => { - const renderWithRouter = (initialEntry: string) => { - migrationRouter.invalidate(); - migrationRouter.navigate({ replace: true, to: initialEntry }); - - return renderWithTheme( - , - { - flags: { - selfServeBetas: true, - }, - } - ); - }; - - /** - * This test is meant to incrementally test all routes being added to the migration router. - * It will hopefully catch any issues with routes not being added or set up correctly: - * - Route is not found in the router - * - Route is found in the router but the component is not rendered - * - Route is found in the router and the component is rendered but missing a heading (which should be a requirement for all routes) - */ - test.each(allMigrationPaths)('route: %s', async (path) => { - renderWithRouter(path); - - await waitFor( - async () => { - const migrationRouter = screen.getByTestId('migration-router'); - const h1 = screen.getByRole('heading', { level: 1 }); - expect(migrationRouter).toBeInTheDocument(); - expect(h1).toBeInTheDocument(); - expect(h1).not.toHaveTextContent('Not Found'); - }, - { - timeout: 5000, - } - ); - }); - - it('should render the NotFound component for broken routes', async () => { - renderWithRouter('/broken-route'); - - await waitFor( - async () => { - const migrationRouter = screen.getByTestId('migration-router'); - const h1 = screen.getByRole('heading', { level: 1 }); - expect(migrationRouter).toBeInTheDocument(); - expect(h1).toBeInTheDocument(); - expect(h1).toHaveTextContent('Not Found'); - }, - { - timeout: 5000, - } - ); - }); -}); diff --git a/packages/manager/src/utilities/testHelpers.tsx b/packages/manager/src/utilities/testHelpers.tsx index 0ae3424c2bf..43918a2acbc 100644 --- a/packages/manager/src/utilities/testHelpers.tsx +++ b/packages/manager/src/utilities/testHelpers.tsx @@ -18,6 +18,7 @@ import { Provider } from 'react-redux'; import { MemoryRouter, Route } from 'react-router-dom'; import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; +import { BrowserRouter } from 'react-router-dom'; import { LinodeThemeWrapper } from 'src/LinodeThemeWrapper'; import { queryClientFactory } from 'src/queries/base'; @@ -206,7 +207,9 @@ export const wrapWithThemeAndRouter = ( options={{ bootstrap: options.flags }} > - + + + From 059f300c7ce700f80bbf32d73fed6c7e7d26412b Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Fri, 10 Jan 2025 16:18:41 -0500 Subject: [PATCH 14/16] some cleanup pt1 + address initial feedback --- .../PlacementGroupsDetail.test.tsx | 3 -- .../PlacementGroupsLinodes.test.tsx | 6 --- .../PlacementGroupsLinodes.tsx | 27 ++++------ .../PlacementGroupsLinodesTable.test.tsx | 2 - .../PlacementGroupsDetailPanel.test.tsx | 18 ------- .../PlacementGroupsEditDrawer.test.tsx | 13 +---- .../PlacementGroupsLanding.test.tsx | 1 - .../PlacementGroupsUnassignModal.tsx | 2 +- .../src/routes/placementGroups/index.ts | 54 +++++++++++++------ .../placementGroupsLazyRoutes.ts | 6 --- 10 files changed, 50 insertions(+), 82 deletions(-) diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx index eb41720fde0..450f318a971 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsDetail.test.tsx @@ -11,8 +11,6 @@ import { PlacementGroupsDetail } from './PlacementGroupsDetail'; const queryMocks = vi.hoisted(() => ({ useAllLinodesQuery: vi.fn().mockReturnValue({}), - useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), - useNavigate: vi.fn(), useParams: vi.fn().mockReturnValue({ id: 1 }), usePlacementGroupQuery: vi.fn().mockReturnValue({}), useRegionsQuery: vi.fn().mockReturnValue({}), @@ -39,7 +37,6 @@ vi.mock('@tanstack/react-router', async () => { const actual = await vi.importActual('@tanstack/react-router'); return { ...actual, - useNavigate: queryMocks.useNavigate, useParams: queryMocks.useParams, useSearch: queryMocks.useSearch, }; diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx index f87ff9b79de..70f7d217cf6 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.test.tsx @@ -7,9 +7,6 @@ import { PLACEMENT_GROUP_LINODES_ERROR_MESSAGE } from '../../constants'; import { PlacementGroupsLinodes } from './PlacementGroupsLinodes'; const queryMocks = vi.hoisted(() => ({ - useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), - useNavigate: vi.fn(), - useParams: vi.fn().mockReturnValue({ id: 1 }), useSearch: vi.fn().mockReturnValue({ query: undefined }), })); @@ -17,9 +14,6 @@ vi.mock('@tanstack/react-router', async () => { const actual = await vi.importActual('@tanstack/react-router'); return { ...actual, - useLocation: queryMocks.useLocation, - useNavigate: queryMocks.useNavigate, - useParams: queryMocks.useParams, useSearch: queryMocks.useSearch, }; }); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx index 6cb78154a4c..6378e35032c 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodes.tsx @@ -1,11 +1,6 @@ import { Button, Stack } from '@linode/ui'; import Grid from '@mui/material/Unstable_Grid2/Grid2'; -import { - useLocation, - useNavigate, - useParams, - useSearch, -} from '@tanstack/react-router'; +import { useNavigate, useParams, useSearch } from '@tanstack/react-router'; import * as React from 'react'; import { DebouncedSearchTextField } from 'src/components/DebouncedSearchTextField'; @@ -46,7 +41,6 @@ type PlacementGroupLinodesSearchParams = PlacementGroupsSearchParams; export const PlacementGroupsLinodes = (props: Props) => { const { isLinodeReadOnly, placementGroup, region } = props; const navigate = useNavigate(); - const location = useLocation(); const params = useParams({ strict: false }); const search: PlacementGroupLinodesSearchParams = useSearch({ @@ -125,17 +119,21 @@ export const PlacementGroupsLinodes = (props: Props) => { const handleAssignLinodesDrawer = () => { navigate({ - params: { id: placementGroup.id }, + params: { action: 'assign', id: placementGroup.id }, search: (prev) => prev, - to: '/placement-groups/$id/linodes/assign', + to: '/placement-groups/$id/linodes/$action', }); }; const handleUnassignLinodeModal = (linode: Linode) => { navigate({ - params: { id: placementGroup.id, linodeId: linode.id }, + params: { + action: 'unassign', + id: placementGroup.id, + linodeId: linode.id, + }, search: (prev) => prev, - to: '/placement-groups/$id/linodes/unassign/$linodeId', + to: '/placement-groups/$id/linodes/$action/$linodeId', }); }; @@ -147,9 +145,6 @@ export const PlacementGroupsLinodes = (props: Props) => { }); }; - const isAssignLinodesDrawerOpen = location.pathname.includes('/assign'); - const isUnassignLinodesDrawerOpen = location.pathname.includes('/unassign'); - return ( @@ -196,14 +191,14 @@ export const PlacementGroupsLinodes = (props: Props) => { /> diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.test.tsx index 703dbba9f9f..56a41adce9e 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetail/PlacementGroupsLinodes/PlacementGroupsLinodesTable.test.tsx @@ -8,7 +8,6 @@ import { PlacementGroupsLinodesTable } from './PlacementGroupsLinodesTable'; import type { Order } from 'src/hooks/useOrderV2'; const queryMocks = vi.hoisted(() => ({ - useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), useParams: vi.fn().mockReturnValue({}), })); @@ -16,7 +15,6 @@ vi.mock('@tanstack/react-router', async () => { const actual = await vi.importActual('@tanstack/react-router'); return { ...actual, - useLocation: queryMocks.useLocation, useParams: queryMocks.useParams, }; }); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx index 59535b7c2b5..29eea22abf1 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsDetailPanel.test.tsx @@ -12,27 +12,9 @@ const defaultProps = { const queryMocks = vi.hoisted(() => ({ useAllPlacementGroupsQuery: vi.fn().mockReturnValue({}), - useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups/1' }), - useParams: vi.fn().mockReturnValue({}), useRegionsQuery: vi.fn().mockReturnValue({}), })); -vi.mock('react-router-dom', async () => { - const actual = await vi.importActual('react-router-dom'); - return { - ...actual, - useLocation: queryMocks.useLocation, - }; -}); - -vi.mock('@tanstack/react-router', async () => { - const actual = await vi.importActual('@tanstack/react-router'); - return { - ...actual, - useParams: queryMocks.useParams, - }; -}); - vi.mock('src/queries/regions/regions', async () => { const actual = await vi.importActual('src/queries/regions/regions'); return { diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.test.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.test.tsx index fbaf9e13794..47b4ce0a0dc 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.test.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsEditDrawer.test.tsx @@ -11,17 +11,8 @@ const queryMocks = vi.hoisted(() => ({ mutateAsync: vi.fn().mockResolvedValue({}), reset: vi.fn(), }), - useParams: vi.fn().mockReturnValue({}), })); -vi.mock('@tanstack/react-router', async () => { - const actual = await vi.importActual('@tanstack/react-router'); - return { - ...actual, - useParams: queryMocks.useParams, - }; -}); - vi.mock('src/queries/placementGroups', async () => { const actual = await vi.importActual('src/queries/placementGroups'); return { @@ -30,10 +21,8 @@ vi.mock('src/queries/placementGroups', async () => { }; }); -describe('PlacementGroupsCreateDrawer', () => { +describe('PlacementGroupsEditDrawer', () => { it('should render, have the proper fields populated with PG values, and have uneditable fields disabled', async () => { - queryMocks.useParams.mockReturnValue({ id: '1' }); - const { getByLabelText, getByRole, getByText } = renderWithTheme( ({ - useLocation: vi.fn().mockReturnValue({ pathname: '/placement-groups' }), useParams: vi.fn().mockReturnValue({}), usePlacementGroupsQuery: vi.fn().mockReturnValue({}), })); diff --git a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx index 02e748e8c5b..8cd16484355 100644 --- a/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx +++ b/packages/manager/src/features/PlacementGroups/PlacementGroupsUnassignModal.tsx @@ -54,7 +54,7 @@ export const PlacementGroupsUnassignModal = (props: Props) => { const isLinodeReadOnly = useIsResourceRestricted({ grantLevel: 'read_write', grantType: 'linode', - id: linodeId ? +linodeId : -1, + id: linodeId ? linodeId : -1, }); const actions = ( diff --git a/packages/manager/src/routes/placementGroups/index.ts b/packages/manager/src/routes/placementGroups/index.ts index 2a2af0e1c5e..d15820870c7 100644 --- a/packages/manager/src/routes/placementGroups/index.ts +++ b/packages/manager/src/routes/placementGroups/index.ts @@ -14,7 +14,13 @@ const placementGroupAction = { edit: 'edit', } as const; +const placementGroupLinodeAction = { + assign: 'assign', + unassign: 'unassign', +} as const; + export type PlacementGroupAction = typeof placementGroupAction[keyof typeof placementGroupAction]; +export type PlacementGroupLinodesAction = typeof placementGroupLinodeAction[keyof typeof placementGroupLinodeAction]; export const placementGroupsRoute = createRoute({ component: PlacementGroupsRoute, @@ -86,33 +92,47 @@ const placementGroupsDetailRoute = createRoute({ ) ); -const placementGroupsDetailLinodesRoute = createRoute({ +type PlacementGroupLinodesActionRouteParams

= { + action: PlacementGroupLinodesAction; +}; + +const placementGroupLinodesActionBaseRoute = createRoute({ + beforeLoad: async ({ params }) => { + if (!(params.action in placementGroupLinodeAction)) { + throw redirect({ + search: () => ({}), + to: `/placement-groups/${params.id}`, + }); + } + }, getParentRoute: () => placementGroupsDetailRoute, - path: 'linodes', + params: { + parse: ({ action }: PlacementGroupLinodesActionRouteParams) => ({ + action, + }), + stringify: ({ + action, + }: PlacementGroupLinodesActionRouteParams) => ({ + action, + }), + }, + path: 'linodes/$action', + validateSearch: (search: PlacementGroupsSearchParams) => search, }).lazy(() => import('./placementGroupsLazyRoutes').then( (m) => m.placementGroupsDetailLazyRoute ) ); -const placementGroupsAssignRoute = createRoute({ - getParentRoute: () => placementGroupsDetailLinodesRoute, - path: 'assign', -}).lazy(() => - import('./placementGroupsLazyRoutes').then( - (m) => m.placementGroupsUnassignLazyRoute - ) -); - const placementGroupsUnassignRoute = createRoute({ - getParentRoute: () => placementGroupsDetailLinodesRoute, + getParentRoute: () => placementGroupLinodesActionBaseRoute, parseParams: (params) => ({ linodeId: Number(params.linodeId), }), - path: 'unassign/$linodeId', + path: '$linodeId', }).lazy(() => import('./placementGroupsLazyRoutes').then( - (m) => m.placementGroupsUnassignLazyRoute + (m) => m.placementGroupsDetailLazyRoute ) ); @@ -120,8 +140,8 @@ export const placementGroupsRouteTree = placementGroupsRoute.addChildren([ placementGroupsIndexRoute.addChildren([placementGroupActionRoute]), placementGroupsCreateRoute, placementGroupsDetailRoute.addChildren([ - placementGroupsDetailLinodesRoute, - placementGroupsAssignRoute, - placementGroupsUnassignRoute, + placementGroupLinodesActionBaseRoute.addChildren([ + placementGroupsUnassignRoute, + ]), ]), ]); diff --git a/packages/manager/src/routes/placementGroups/placementGroupsLazyRoutes.ts b/packages/manager/src/routes/placementGroups/placementGroupsLazyRoutes.ts index 6521e756fc7..a49a13a3d87 100644 --- a/packages/manager/src/routes/placementGroups/placementGroupsLazyRoutes.ts +++ b/packages/manager/src/routes/placementGroups/placementGroupsLazyRoutes.ts @@ -9,12 +9,6 @@ export const placementGroupsDetailLazyRoute = createLazyRoute( component: PlacementGroupsDetail, }); -export const placementGroupsUnassignLazyRoute = createLazyRoute( - '/placement-groups/$id/linodes/unassign/$linodeId' -)({ - component: PlacementGroupsDetail, -}); - export const placementGroupsLandingLazyRoute = createLazyRoute( '/placement-groups' )({ From 8fac66d607d31a161690544ba01507ac15dc89b7 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Fri, 10 Jan 2025 17:17:13 -0500 Subject: [PATCH 15/16] Added changeset: Refactor routing for Placement Groups to use Tanstack Router --- .../.changeset/pr-11474-tech-stories-1736547433123.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-11474-tech-stories-1736547433123.md diff --git a/packages/manager/.changeset/pr-11474-tech-stories-1736547433123.md b/packages/manager/.changeset/pr-11474-tech-stories-1736547433123.md new file mode 100644 index 00000000000..713ea203d09 --- /dev/null +++ b/packages/manager/.changeset/pr-11474-tech-stories-1736547433123.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Tech Stories +--- + +Refactor routing for Placement Groups to use Tanstack Router ([#11474](https://github.com/linode/manager/pull/11474)) From 073cce2068983dbf2bab3fb247dc21dbc8b3bab2 Mon Sep 17 00:00:00 2001 From: Connie Liu Date: Fri, 10 Jan 2025 17:34:53 -0500 Subject: [PATCH 16/16] fix e2e delete test --- .../manager/cypress/e2e/core/linodes/resize-linode.spec.ts | 2 +- .../e2e/core/placementGroups/delete-placement-groups.spec.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/manager/cypress/e2e/core/linodes/resize-linode.spec.ts b/packages/manager/cypress/e2e/core/linodes/resize-linode.spec.ts index 4242ba01f9d..77d99e42a30 100644 --- a/packages/manager/cypress/e2e/core/linodes/resize-linode.spec.ts +++ b/packages/manager/cypress/e2e/core/linodes/resize-linode.spec.ts @@ -155,7 +155,7 @@ describe('resize linode', () => { }); }); - it.only('resizes a linode by decreasing size', () => { + it('resizes a linode by decreasing size', () => { // Use `vlan_no_internet` security method. // This works around an issue where the Linode API responds with a 400 // when attempting to interact with it shortly after booting up when the diff --git a/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts b/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts index cc8fba5d5b9..314d24969e0 100644 --- a/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts +++ b/packages/manager/cypress/e2e/core/placementGroups/delete-placement-groups.spec.ts @@ -299,6 +299,9 @@ describe('Placement Group deletion', () => { placementGroupAfterUnassignment, secondMockPlacementGroup, ]).as('getPlacementGroups'); + mockGetPlacementGroup(placementGroupAfterUnassignment).as( + 'getPlacementGroups' + ); cy.findByText(mockLinode.label) .should('be.visible')