From 5d8d563530ad56dc3b2896fa3b1cd5770bd06751 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Tue, 10 Oct 2023 12:56:56 -0400 Subject: [PATCH 1/5] initial edit flow and e2e test --- .../load-balancer-routes.spec.ts | 73 ++++++++++++++++++- .../LoadBalancerDetail/LoadBalancerRoutes.tsx | 19 ++++- .../Routes/AddRuleDrawer.tsx | 34 +++++++-- .../LoadBalancerDetail/RulesTable.test.tsx | 5 +- .../LoadBalancerDetail/RulesTable.tsx | 16 ++-- packages/manager/src/mocks/serverHandlers.ts | 5 +- 6 files changed, 131 insertions(+), 21 deletions(-) diff --git a/packages/manager/cypress/e2e/core/loadBalancers/load-balancer-routes.spec.ts b/packages/manager/cypress/e2e/core/loadBalancers/load-balancer-routes.spec.ts index 24d8f206664..81a7980815c 100644 --- a/packages/manager/cypress/e2e/core/loadBalancers/load-balancer-routes.spec.ts +++ b/packages/manager/cypress/e2e/core/loadBalancers/load-balancer-routes.spec.ts @@ -217,7 +217,78 @@ describe('Akamai Global Load Balancer routes page', () => { cy.wait('@updateRoute'); }); - it('surfaces API errors in the Add Rule Drawer', () => { + it('can edit a HTTP rule', () => { + const loadbalancer = loadbalancerFactory.build(); + const routes = routeFactory.buildList(1, { protocol: 'http' }); + const serviceTargets = serviceTargetFactory.buildList(3); + + mockAppendFeatureFlags({ + aglb: makeFeatureFlagData(true), + }).as('getFeatureFlags'); + mockGetFeatureFlagClientstream().as('getClientStream'); + mockGetLoadBalancer(loadbalancer).as('getLoadBalancer'); + mockGetLoadBalancerRoutes(loadbalancer.id, routes).as('getRoutes'); + mockGetLoadBalancerServiceTargets(loadbalancer.id, serviceTargets).as( + 'getServiceTargets' + ); + + cy.visitWithLogin(`/loadbalancers/${loadbalancer.id}/routes`); + cy.wait([ + '@getFeatureFlags', + '@getClientStream', + '@getLoadBalancer', + '@getRoutes', + ]); + + cy.findByLabelText(`route-${routes[0].id} expand row`).click(); + + ui.actionMenu.findByTitle('Action Menu for Rule 0').click(); + + ui.actionMenuItem.findByTitle('Edit').click(); + + mockUpdateRoute(loadbalancer, routes[0]).as('updateRoute'); + + ui.drawer + .findByTitle('Edit Rule') + .should('be.visible') + .within(() => { + cy.findByLabelText('Hostname') + .should('have.value', routes[0].rules[0].match_condition.hostname) + .clear() + .type('example.com'); + + cy.findByLabelText('Match Type') + .should('be.visible') + .click() + .clear() + .type('Header'); + + ui.autocompletePopper + .findByTitle('HTTP Header') + .should('be.visible') + .click(); + + cy.findByLabelText('Match Value') + .should('have.value', routes[0].rules[0].match_condition.match_value) + .clear() + .type('x-header=my-header-value'); + + ui.buttonGroup + .findButtonByTitle('Save') + .should('be.visible') + .should('be.enabled') + .click(); + }); + + cy.wait('@updateRoute'); + + // Verify the table updates after the drawer saves and closes + cy.findByLabelText('Rule 0').within(() => { + cy.findByText('x-header=my-header-value'); + cy.findByText('HTTP Header'); + }); + }); + it('surfaces API errors in the Add Rule Drawer for an HTTP route', () => { const loadbalancer = loadbalancerFactory.build(); const routes = routeFactory.buildList(1, { protocol: 'http' }); const serviceTargets = serviceTargetFactory.buildList(3); diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerRoutes.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerRoutes.tsx index b9d6d4d5f4f..4e79ad13076 100644 --- a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerRoutes.tsx +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerRoutes.tsx @@ -39,6 +39,7 @@ export const LoadBalancerRoutes = () => { const [query, setQuery] = useState(); const [isDeleteDialogOpen, setIsDeleteDialogOpen] = useState(false); const [selectedRouteId, setSelectedRouteId] = useState(); + const [selectedRuleIndex, setSelectedRuleIndex] = useState(); const pagination = usePagination(1, PREFERENCE_KEY); @@ -78,6 +79,12 @@ export const LoadBalancerRoutes = () => { setSelectedRouteId(route.id); }; + const onEditRule = (route: Route, ruleIndex: number) => { + setIsAddRuleDrawerOpen(true); + setSelectedRouteId(route.id); + setSelectedRuleIndex(ruleIndex); + }; + const onDeleteRoute = (route: Route) => { setIsDeleteDialogOpen(true); setSelectedRouteId(route.id); @@ -124,7 +131,11 @@ export const LoadBalancerRoutes = () => { ); const InnerTable = ( - + onEditRule(route, index)} + route={route} + /> ); return { @@ -217,10 +228,14 @@ export const LoadBalancerRoutes = () => { pageSize={pagination.pageSize} /> { + setIsAddRuleDrawerOpen(false); + setSelectedRuleIndex(undefined); + }} loadbalancerId={Number(loadbalancerId)} - onClose={() => setIsAddRuleDrawerOpen(false)} open={isAddRuleDrawerOpen} route={selectedRoute} + ruleIndexToEdit={selectedRuleIndex} /> void; open: boolean; route: Route | undefined; + ruleIndexToEdit: number | undefined; } export const AddRuleDrawer = (props: Props) => { - const { loadbalancerId, onClose: _onClose, open, route } = props; + const { + loadbalancerId, + onClose: _onClose, + open, + route, + ruleIndexToEdit, + } = props; + + const ruleIndex = ruleIndexToEdit ?? route?.rules.length ?? 0; - const ruleIndex = route?.rules.length ?? 0; + const isEditMode = ruleIndexToEdit !== undefined; const validationSchema = route?.protocol === 'tcp' ? TCPRuleSchema : HTTPRuleSchema; @@ -56,19 +65,25 @@ export const AddRuleDrawer = (props: Props) => { error, mutateAsync: updateRule, reset, + isLoading, } = useLoadBalancerRouteUpdateMutation(loadbalancerId, route?.id ?? -1); const [ttlUnit, setTTLUnit] = useState('hour'); const formik = useFormik({ enableReinitialize: true, - initialValues, + initialValues: isEditMode ? route!.rules[ruleIndexToEdit] : initialValues, async onSubmit(rule) { try { const existingRules = route?.rules ?? []; + + if (isEditMode) { + existingRules[ruleIndexToEdit] = rule; + } + await updateRule({ protocol: route?.protocol, - rules: [...existingRules, rule], + rules: isEditMode ? existingRules : [...existingRules, rule], }); onClose(); } catch (errors) { @@ -167,7 +182,12 @@ export const AddRuleDrawer = (props: Props) => { .join(', '); return ( - +
{/** * @todo: AGLB update copy @@ -464,8 +484,8 @@ export const AddRuleDrawer = (props: Props) => { { it('renders table headers', () => { const { getByText } = renderWithTheme( - + ); expect(getByText('Execution')).toBeInTheDocument(); expect(getByText('Match Value')).toBeInTheDocument(); @@ -34,6 +34,7 @@ describe('RulesTable', () => { const { getByText } = renderWithTheme( ); @@ -42,7 +43,7 @@ describe('RulesTable', () => { it('renders rules correctly', () => { const { getByText } = renderWithTheme( - + ); expect(getByText('First')).toBeInTheDocument(); diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx index 40c278b7d01..19ed15a8e52 100644 --- a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx @@ -28,6 +28,7 @@ import type { MatchField, Route } from '@linode/api-v4'; interface Props { loadbalancerId: number; + onEditRule: (ruleIndex: number) => void; route: Route; } @@ -42,7 +43,7 @@ const matchFieldMap: Record = { const screenReaderMessage = 'Some screen readers may require you to enter focus mode to interact with Loadbalancer rule list items. In focus mode, press spacebar to begin a drag or tab to access item actions.'; -export const RulesTable = ({ loadbalancerId, route }: Props) => { +export const RulesTable = ({ loadbalancerId, route, onEditRule }: Props) => { const { label, protocol, rules } = route; const theme = useTheme(); const { enqueueSnackbar } = useSnackbar(); @@ -146,10 +147,7 @@ export const RulesTable = ({ loadbalancerId, route }: Props) => { > {(provided) => (
  • { {...provided.dragHandleProps} > @@ -270,7 +267,10 @@ export const RulesTable = ({ loadbalancerId, route }: Props) => { {/** TODO: AGLB: The Edit and Delete Action menu behavior should be implemented in future AGLB tickets. */} null, title: 'Edit' }, + { + onClick: () => onEditRule(index), + title: 'Edit', + }, { disabled: index === 0, onClick: () => handleMoveUp(index), @@ -283,7 +283,7 @@ export const RulesTable = ({ loadbalancerId, route }: Props) => { }, { onClick: () => null, title: 'Remove' }, ]} - ariaLabel={`Action Menu for Rule ${rule.match_condition.match_value}`} + ariaLabel={`Action Menu for Rule ${index}`} /> diff --git a/packages/manager/src/mocks/serverHandlers.ts b/packages/manager/src/mocks/serverHandlers.ts index 7417e168484..cd16329e708 100644 --- a/packages/manager/src/mocks/serverHandlers.ts +++ b/packages/manager/src/mocks/serverHandlers.ts @@ -362,7 +362,10 @@ const aglb = [ rest.put('*/v4beta/aglb/:id/routes/:routeId', (req, res, ctx) => { const id = Number(req.params.routeId); const body = req.body as any; - return res(ctx.json(createRouteFactory.build({ id, ...body }))); + return res( + ctx.delay(2000), + ctx.json(createRouteFactory.build({ id, ...body })) + ); }), rest.delete('*/v4beta/aglb/:id/routes/:routeId', (req, res, ctx) => { return res(ctx.json({})); From 97a00a7b575d9edd7bc07e8108195a18474cbd66 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Tue, 10 Oct 2023 13:19:57 -0400 Subject: [PATCH 2/5] fix notice regression --- packages/manager/src/components/Notice/Notice.styles.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/manager/src/components/Notice/Notice.styles.ts b/packages/manager/src/components/Notice/Notice.styles.ts index 2b45a97cc28..b27f9b9a8a5 100644 --- a/packages/manager/src/components/Notice/Notice.styles.ts +++ b/packages/manager/src/components/Notice/Notice.styles.ts @@ -28,6 +28,7 @@ export const useStyles = makeStyles< [`& .${classes.noticeText}`]: { fontFamily: theme.font.normal, }, + backgroundColor: theme.bg.bgPaper, }, info: { [`&.${classes.important}`]: { @@ -56,11 +57,7 @@ export const useStyles = makeStyles< [`& .${classes.error}`]: { borderLeftColor: theme.color.red, }, - [`& .${classes.important}`]: { - backgroundColor: theme.bg.bgPaper, - }, alignItems: 'center', - backgroundColor: theme.bg.bgPaper, borderRadius: 1, display: 'flex', fontSize: '1rem', From b9cf65abe6acfaab50dac0c47db50d9c7e28b8a4 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Tue, 10 Oct 2023 13:48:07 -0400 Subject: [PATCH 3/5] a few small fixes --- .../LoadBalancerDetail/LoadBalancerRoutes.tsx | 4 ++-- .../Routes/{AddRuleDrawer.tsx => RuleDrawer.tsx} | 14 +++++++++++--- .../LoadBalancerDetail/RulesTable.tsx | 4 ++-- packages/manager/src/mocks/serverHandlers.ts | 4 ++-- 4 files changed, 17 insertions(+), 9 deletions(-) rename packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/{AddRuleDrawer.tsx => RuleDrawer.tsx} (97%) diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerRoutes.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerRoutes.tsx index 4e79ad13076..ee313df1e6d 100644 --- a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerRoutes.tsx +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerRoutes.tsx @@ -24,8 +24,8 @@ import { useOrder } from 'src/hooks/useOrder'; import { usePagination } from 'src/hooks/usePagination'; import { useLoadBalancerRoutesQuery } from 'src/queries/aglb/routes'; -import { AddRuleDrawer } from './Routes/AddRuleDrawer'; import { DeleteRouteDialog } from './Routes/DeleteRouteDialog'; +import { RuleDrawer } from './Routes/RuleDrawer'; import { RulesTable } from './RulesTable'; import type { Filter, Route } from '@linode/api-v4'; @@ -227,7 +227,7 @@ export const LoadBalancerRoutes = () => { page={pagination.page} pageSize={pagination.pageSize} /> - { setIsAddRuleDrawerOpen(false); setSelectedRuleIndex(undefined); diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/AddRuleDrawer.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/RuleDrawer.tsx similarity index 97% rename from packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/AddRuleDrawer.tsx rename to packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/RuleDrawer.tsx index 9c66430310f..1ffe7b6a017 100644 --- a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/AddRuleDrawer.tsx +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/Routes/RuleDrawer.tsx @@ -45,7 +45,10 @@ interface Props { ruleIndexToEdit: number | undefined; } -export const AddRuleDrawer = (props: Props) => { +/** + * Drawer used for *adding* and *editing* AGLB rules + */ +export const RuleDrawer = (props: Props) => { const { loadbalancerId, onClose: _onClose, @@ -63,26 +66,31 @@ export const AddRuleDrawer = (props: Props) => { const { error, + isLoading, mutateAsync: updateRule, reset, - isLoading, } = useLoadBalancerRouteUpdateMutation(loadbalancerId, route?.id ?? -1); const [ttlUnit, setTTLUnit] = useState('hour'); const formik = useFormik({ enableReinitialize: true, - initialValues: isEditMode ? route!.rules[ruleIndexToEdit] : initialValues, + initialValues: isEditMode + ? route?.rules[ruleIndexToEdit] ?? initialValues + : initialValues, async onSubmit(rule) { try { const existingRules = route?.rules ?? []; + // If we are editing, update the rule with the form data. if (isEditMode) { existingRules[ruleIndexToEdit] = rule; } await updateRule({ protocol: route?.protocol, + // If we are editing, send the updated rules, otherwise + // append a new rule to the end. rules: isEditMode ? existingRules : [...existingRules, rule], }); onClose(); diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx index 19ed15a8e52..56d1bd19639 100644 --- a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx @@ -43,7 +43,7 @@ const matchFieldMap: Record = { const screenReaderMessage = 'Some screen readers may require you to enter focus mode to interact with Loadbalancer rule list items. In focus mode, press spacebar to begin a drag or tab to access item actions.'; -export const RulesTable = ({ loadbalancerId, route, onEditRule }: Props) => { +export const RulesTable = ({ loadbalancerId, onEditRule, route }: Props) => { const { label, protocol, rules } = route; const theme = useTheme(); const { enqueueSnackbar } = useSnackbar(); @@ -250,7 +250,7 @@ export const RulesTable = ({ loadbalancerId, route, onEditRule }: Props) => { }} > {rule.match_condition - .session_stickiness_cookie && + .session_stickiness_cookie || rule.match_condition.session_stickiness_ttl ? 'Yes' : 'No'} diff --git a/packages/manager/src/mocks/serverHandlers.ts b/packages/manager/src/mocks/serverHandlers.ts index cd16329e708..827368a93d0 100644 --- a/packages/manager/src/mocks/serverHandlers.ts +++ b/packages/manager/src/mocks/serverHandlers.ts @@ -354,7 +354,7 @@ const aglb = [ }), // Routes rest.get('*/v4beta/aglb/:id/routes', (req, res, ctx) => { - return res(ctx.json(makeResourcePage(routeFactory.buildList(1)))); + return res(ctx.json(makeResourcePage(routeFactory.buildList(5)))); }), rest.post('*/v4beta/aglb/:id/routes', (req, res, ctx) => { return res(ctx.json(createRouteFactory.buildList(4))); @@ -363,7 +363,7 @@ const aglb = [ const id = Number(req.params.routeId); const body = req.body as any; return res( - ctx.delay(2000), + ctx.delay(1000), ctx.json(createRouteFactory.build({ id, ...body })) ); }), From e0d7b0f68ee4b8f4292c32e502819eb30e98fe98 Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Tue, 10 Oct 2023 14:07:11 -0400 Subject: [PATCH 4/5] Added changeset: Add AGLB Routes - Rule Edit Drawer --- .../.changeset/pr-9778-upcoming-features-1696961231870.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-9778-upcoming-features-1696961231870.md diff --git a/packages/manager/.changeset/pr-9778-upcoming-features-1696961231870.md b/packages/manager/.changeset/pr-9778-upcoming-features-1696961231870.md new file mode 100644 index 00000000000..e2fa1a175bd --- /dev/null +++ b/packages/manager/.changeset/pr-9778-upcoming-features-1696961231870.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Upcoming Features +--- + +Add AGLB Routes - Rule Edit Drawer ([#9778](https://github.com/linode/manager/pull/9778)) From b5ca24fc40f563ae794e8805f9dd3ea456a78ffd Mon Sep 17 00:00:00 2001 From: Banks Nussman Date: Wed, 11 Oct 2023 16:27:04 -0400 Subject: [PATCH 5/5] add empty state for no service targets --- .../LoadBalancerDetail/RulesTable.tsx | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx index 56d1bd19639..dfbd39a8fb4 100644 --- a/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx +++ b/packages/manager/src/features/LoadBalancers/LoadBalancerDetail/RulesTable.tsx @@ -216,22 +216,26 @@ export const RulesTable = ({ loadbalancerId, onEditRule, route }: Props) => { }} aria-label={`Service Targets: ${rule.service_targets.length}`} > - - {rule.service_targets.map( - ({ id, label }) => ( -
    - {label}:{id} -
    - ) - )} - - } - /> + {rule.service_targets.length > 0 ? ( + + {rule.service_targets.map( + ({ id, label }) => ( +
    + {label}:{id} +
    + ) + )} + + } + /> + ) : ( + 'None' + )}