From 6b172926ac1506c656061ea4b88dd2712a3c4dcb Mon Sep 17 00:00:00 2001 From: Jones Ogolo <47540149+Jay-Topher@users.noreply.github.com> Date: Mon, 15 Apr 2024 14:33:25 +0100 Subject: [PATCH] feat: move VLAN inline forms to side panel (#5404) --- .../ConfigureDHCP/ConfigureDHCP.test.tsx | 6 +- .../ConfigureDHCP/ConfigureDHCP.tsx | 154 +++++++++--------- .../ConfigureDHCPFields.tsx | 2 +- .../DHCPStatus/DHCPStatus.test.tsx | 16 +- .../VLANDetails/DHCPStatus/DHCPStatus.tsx | 13 +- .../views/VLANDetails/EditVLAN/EditVLAN.tsx | 4 +- .../VLANActionForms/VLANActionForms.tsx | 38 +++-- .../subnets/views/VLANDetails/VLANDetails.tsx | 18 +- .../VLANSummary/VLANSummary.test.tsx | 44 +++-- .../VLANDetails/VLANSummary/VLANSummary.tsx | 61 +++---- .../subnets/views/VLANDetails/constants.ts | 6 + 11 files changed, 197 insertions(+), 165 deletions(-) diff --git a/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCP.test.tsx b/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCP.test.tsx index 67f0c62d4d..47ff50c4f2 100644 --- a/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCP.test.tsx +++ b/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCP.test.tsx @@ -50,7 +50,7 @@ it("correctly initialises data if the VLAN has DHCP from rack controllers", asyn // Wait for Formik validateOnMount to run. await waitFor(() => { expect( - screen.getByRole("region", { name: "Configure DHCP" }) + screen.getByRole("form", { name: "Configure DHCP" }) ).toBeInTheDocument(); }); @@ -90,7 +90,7 @@ it("correctly initialises data if the VLAN has relayed DHCP", async () => { // Wait for Formik validateOnMount to run. await waitFor(() => { expect( - screen.getByRole("region", { name: "Configure DHCP" }) + screen.getByRole("form", { name: "Configure DHCP" }) ).toBeInTheDocument(); }); @@ -127,7 +127,7 @@ it("shows an error if no rack controllers are connected to the VLAN", async () = }); expect( - screen.getByRole("region", { name: "Configure DHCP" }) + screen.getByRole("form", { name: "Configure DHCP" }) ).toBeInTheDocument(); expect( diff --git a/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCP.tsx b/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCP.tsx index f4b150cdbd..88196d0aa7 100644 --- a/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCP.tsx +++ b/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCP.tsx @@ -1,7 +1,7 @@ import { useCallback } from "react"; import { ExternalLink } from "@canonical/maas-react-components"; -import { Card, Spinner } from "@canonical/react-components"; +import { Spinner } from "@canonical/react-components"; import { useDispatch, useSelector } from "react-redux"; import * as Yup from "yup"; @@ -9,7 +9,6 @@ import ConfigureDHCPFields from "./ConfigureDHCPFields"; import DHCPReservedRanges from "./DHCPReservedRanges"; import FormikForm from "@/app/base/components/FormikForm"; -import TitledSection from "@/app/base/components/TitledSection"; import docsUrls from "@/app/base/docsUrls"; import { useFetchActions, useCycled } from "@/app/base/hooks"; import { controllerActions } from "@/app/store/controller"; @@ -158,83 +157,82 @@ const ConfigureDHCP = ({ closeForm, id }: Props): JSX.Element | null => { }); return ( - - - {loading ? ( - - - - ) : ( - - allowUnchanged - buttonsHelp={ - About DHCP - } - cleanup={cleanup} - errors={configureDHCPError} - initialValues={{ - dhcpType: isId(vlan.relay_vlan) - ? DHCPType.RELAY - : DHCPType.CONTROLLERS, - enableDHCP: true, - endIP: "", - gatewayIP: "", - primaryRack: vlan.primary_rack || "", - relayVLAN: vlan.relay_vlan || "", - secondaryRack: vlan.secondary_rack || "", - startIP: "", - subnet: "", - }} - onCancel={closeForm} - onSaveAnalytics={{ - action: "Configure DHCP", - category: "VLAN details", - label: "Configure DHCP form", - }} - onSubmit={(values) => { - resetConfiguredDHCP(); - dispatch(cleanup()); - const { enableDHCP, primaryRack, relayVLAN, secondaryRack } = - values; - const params: ConfigureDHCPParams = { - controllers: [], - id: vlan.id, - relay_vlan: null, - }; - if (enableDHCP) { - if (primaryRack) { - params.controllers.push(primaryRack); - } - if (secondaryRack) { - params.controllers.push(secondaryRack); - } - if (isId(relayVLAN)) { - params.relay_vlan = Number(relayVLAN); - } - if (isId(values.subnet)) { - params.extra = { - end: values.endIP, - gateway: values.gatewayIP, - start: values.startIP, - subnet: Number(values.subnet), - }; - } + <> + {loading ? ( + + + + ) : ( + + allowUnchanged + aria-label="Configure DHCP" + buttonsHelp={ + About DHCP + } + cleanup={cleanup} + errors={configureDHCPError} + initialValues={{ + dhcpType: isId(vlan.relay_vlan) + ? DHCPType.RELAY + : DHCPType.CONTROLLERS, + enableDHCP: true, + endIP: "", + gatewayIP: "", + primaryRack: vlan.primary_rack || "", + relayVLAN: vlan.relay_vlan || "", + secondaryRack: vlan.secondary_rack || "", + startIP: "", + subnet: "", + }} + onCancel={closeForm} + onSaveAnalytics={{ + action: "Configure DHCP", + category: "VLAN details", + label: "Configure DHCP form", + }} + onSubmit={(values) => { + resetConfiguredDHCP(); + dispatch(cleanup()); + const { enableDHCP, primaryRack, relayVLAN, secondaryRack } = + values; + const params: ConfigureDHCPParams = { + controllers: [], + id: vlan.id, + relay_vlan: null, + }; + if (enableDHCP) { + if (primaryRack) { + params.controllers.push(primaryRack); + } + if (secondaryRack) { + params.controllers.push(secondaryRack); } - dispatch(vlanActions.configureDHCP(params)); - }} - onSuccess={() => closeForm()} - saved={saved} - saving={configuringDHCP} - submitLabel="Configure DHCP" - validateOnMount - validationSchema={Schema} - > - - - - )} - - + if (isId(relayVLAN)) { + params.relay_vlan = Number(relayVLAN); + } + if (isId(values.subnet)) { + params.extra = { + end: values.endIP, + gateway: values.gatewayIP, + start: values.startIP, + subnet: Number(values.subnet), + }; + } + } + dispatch(vlanActions.configureDHCP(params)); + }} + onSuccess={() => closeForm()} + saved={saved} + saving={configuringDHCP} + submitLabel="Configure DHCP" + validateOnMount + validationSchema={Schema} + > + + + + )} + ); }; diff --git a/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCPFields/ConfigureDHCPFields.tsx b/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCPFields/ConfigureDHCPFields.tsx index c502d017bd..cf9980ce91 100644 --- a/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCPFields/ConfigureDHCPFields.tsx +++ b/src/app/subnets/views/VLANDetails/ConfigureDHCP/ConfigureDHCPFields/ConfigureDHCPFields.tsx @@ -44,7 +44,7 @@ const ConfigureDHCPFields = ({ vlan }: Props): JSX.Element => { return ( - + { render( - + ); @@ -38,7 +38,7 @@ it(`shows a warning and disables Configure DHCP button if there are no subnets render( - + ); @@ -66,7 +66,7 @@ it("does not show a warning if there are subnets attached to the VLAN", () => { render( - + ); @@ -92,7 +92,7 @@ it("renders correctly when a VLAN does not have DHCP enabled", () => { render( - + ); @@ -116,7 +116,7 @@ it("renders correctly when a VLAN has external DHCP", () => { render( - + ); @@ -148,7 +148,7 @@ it("renders correctly when a VLAN has relayed DHCP", () => { render( - + ); @@ -178,7 +178,7 @@ it("renders correctly when a VLAN has MAAS-configured DHCP without high availabi render( - + ); @@ -225,7 +225,7 @@ it("renders correctly when a VLAN has MAAS-configured DHCP with high availabilit render( - + ); diff --git a/src/app/subnets/views/VLANDetails/DHCPStatus/DHCPStatus.tsx b/src/app/subnets/views/VLANDetails/DHCPStatus/DHCPStatus.tsx index a751c05109..22424200a7 100644 --- a/src/app/subnets/views/VLANDetails/DHCPStatus/DHCPStatus.tsx +++ b/src/app/subnets/views/VLANDetails/DHCPStatus/DHCPStatus.tsx @@ -14,6 +14,7 @@ import Definition from "@/app/base/components/Definition"; import TitledSection from "@/app/base/components/TitledSection"; import docsUrls from "@/app/base/docsUrls"; import { useFetchActions } from "@/app/base/hooks"; +import { SidePanelViews, useSidePanel } from "@/app/base/side-panel-context"; import urls from "@/app/base/urls"; import { fabricActions } from "@/app/store/fabric"; import fabricSelectors from "@/app/store/fabric/selectors"; @@ -29,7 +30,6 @@ import { isId } from "@/app/utils"; type Props = { id: VLAN[VLANMeta.PK] | null; - openForm: () => void; }; // Note this is not the same as the getDHCPStatus VLAN util as it uses slightly @@ -54,7 +54,8 @@ const getDHCPStatus = (vlan: VLAN, vlans: VLAN[], fabrics: Fabric[]) => { return "Disabled"; }; -const DHCPStatus = ({ id, openForm }: Props): JSX.Element | null => { +const DHCPStatus = ({ id }: Props): JSX.Element | null => { + const { setSidePanelContent, setSidePanelSize } = useSidePanel(); const fabrics = useSelector(fabricSelectors.all); const fabricsLoading = useSelector(fabricSelectors.loading); const vlans = useSelector(vlanSelectors.all); @@ -91,7 +92,13 @@ const DHCPStatus = ({ id, openForm }: Props): JSX.Element | null => { return ( + } diff --git a/src/app/subnets/views/VLANDetails/EditVLAN/EditVLAN.tsx b/src/app/subnets/views/VLANDetails/EditVLAN/EditVLAN.tsx index 03f58e4f65..3a5d6dbce0 100644 --- a/src/app/subnets/views/VLANDetails/EditVLAN/EditVLAN.tsx +++ b/src/app/subnets/views/VLANDetails/EditVLAN/EditVLAN.tsx @@ -107,7 +107,7 @@ const EditVLAN = ({ close, id, ...props }: Props): JSX.Element | null => { {...props} > - + @@ -117,7 +117,7 @@ const EditVLAN = ({ close, id, ...props }: Props): JSX.Element | null => { name="description" /> - + { - const ActionForm = actionForms[activeForm]; + const clearSidePanelContent = () => setSidePanelContent(null); - if (!ActionForm) { - return null; - } + switch (activeForm) { + case VLANActionTypes.ConfigureDHCP: { + return ; + } + case VLANActionTypes.EditVLAN: + return ; - return ( - - ); + default: { + const ActionForm = actionForms[activeForm]; + + if (!ActionForm) { + return null; + } + return ( + + ); + } + } }; export default VLANActionForms; diff --git a/src/app/subnets/views/VLANDetails/VLANDetails.tsx b/src/app/subnets/views/VLANDetails/VLANDetails.tsx index 967acc2fdb..9edb0fcf0c 100644 --- a/src/app/subnets/views/VLANDetails/VLANDetails.tsx +++ b/src/app/subnets/views/VLANDetails/VLANDetails.tsx @@ -1,8 +1,7 @@ -import { useEffect, useState } from "react"; +import { useEffect } from "react"; import { useDispatch, useSelector } from "react-redux"; -import ConfigureDHCP from "./ConfigureDHCP"; import DHCPStatus from "./DHCPStatus"; import VLANActionForms from "./VLANActionForms"; import VLANDetailsHeader from "./VLANDetailsHeader"; @@ -40,7 +39,6 @@ const VLANDetails = (): JSX.Element => { const subnets = useSelector((state: RootState) => subnetSelectors.getByIds(state, vlan?.subnet_ids || []) ); - const [showDHCPForm, setShowDHCPForm] = useState(false); useWindowTitle(`${vlan?.name || "VLAN"} details`); useEffect(() => { @@ -97,16 +95,10 @@ const VLANDetails = (): JSX.Element => { } sidePanelTitle={activeForm ? vlanActionLabels[activeForm] : ""} > - {showDHCPForm ? ( - setShowDHCPForm(false)} id={id} /> - ) : ( - <> - - setShowDHCPForm(true)} /> - 0} vlanId={id} /> - - - )} + + + 0} vlanId={id} /> + ); diff --git a/src/app/subnets/views/VLANDetails/VLANSummary/VLANSummary.test.tsx b/src/app/subnets/views/VLANDetails/VLANSummary/VLANSummary.test.tsx index cc7b7dede2..79bf1e84df 100644 --- a/src/app/subnets/views/VLANDetails/VLANSummary/VLANSummary.test.tsx +++ b/src/app/subnets/views/VLANDetails/VLANSummary/VLANSummary.test.tsx @@ -2,8 +2,11 @@ import { Provider } from "react-redux"; import { MemoryRouter } from "react-router-dom"; import configureStore from "redux-mock-store"; +import { VLANDetailsSidePanelViews } from "../constants"; + import VLANSummary from "./VLANSummary"; +import * as sidePanelHooks from "@/app/base/side-panel-context"; import urls from "@/app/base/urls"; import type { Controller } from "@/app/store/controller/types"; import type { Fabric } from "@/app/store/fabric/types"; @@ -11,7 +14,13 @@ import type { RootState } from "@/app/store/root/types"; import type { Space } from "@/app/store/space/types"; import type { VLAN } from "@/app/store/vlan/types"; import * as factory from "@/testing/factories"; -import { userEvent, render, screen, within } from "@/testing/utils"; +import { + userEvent, + render, + screen, + within, + renderWithBrowserRouter, +} from "@/testing/utils"; const mockStore = configureStore(); @@ -20,8 +29,16 @@ let fabric: Fabric; let space: Space; let state: RootState; let vlan: VLAN; +const setSidePanelContent = vi.fn(); beforeEach(() => { + vi.spyOn(sidePanelHooks, "useSidePanel").mockReturnValue({ + setSidePanelContent, + sidePanelContent: null, + setSidePanelSize: vi.fn(), + sidePanelSize: "regular", + }); + fabric = factory.fabric({ id: 1, name: "fabric-1" }); space = factory.space({ id: 22, name: "outer" }); controller = factory.controller({ @@ -49,6 +66,10 @@ beforeEach(() => { }); }); +afterEach(() => { + vi.restoreAllMocks(); +}); + it("renders correct details", () => { const store = mockStore(state); render( @@ -73,24 +94,13 @@ it("renders correct details", () => { ); }); -it("can display the edit form", async () => { +it("can trigger the edit form side panel", async () => { const store = mockStore(state); - render( - - - - - - ); - const formName = "Edit VLAN"; + renderWithBrowserRouter(, { store }); const button = screen.getByRole("button", { name: "Edit" }); expect(button).toBeInTheDocument(); - expect( - screen.queryByRole("form", { name: formName }) - ).not.toBeInTheDocument(); await userEvent.click(button); - expect( - screen.queryByRole("button", { name: "Edit" }) - ).not.toBeInTheDocument(); - expect(screen.getByRole("form", { name: formName })).toBeInTheDocument(); + expect(setSidePanelContent).toHaveBeenCalledWith({ + view: VLANDetailsSidePanelViews.EditVLAN, + }); }); diff --git a/src/app/subnets/views/VLANDetails/VLANSummary/VLANSummary.tsx b/src/app/subnets/views/VLANDetails/VLANSummary/VLANSummary.tsx index 22360ac168..f845320771 100644 --- a/src/app/subnets/views/VLANDetails/VLANSummary/VLANSummary.tsx +++ b/src/app/subnets/views/VLANDetails/VLANSummary/VLANSummary.tsx @@ -1,14 +1,13 @@ -import { Col, Row } from "@canonical/react-components"; +import { Button, Col, Row } from "@canonical/react-components"; import { useSelector } from "react-redux"; -import EditVLAN from "../EditVLAN"; - import VLANControllers from "./VLANControllers"; import Definition from "@/app/base/components/Definition"; -import EditableSection from "@/app/base/components/EditableSection"; import FabricLink from "@/app/base/components/FabricLink"; import SpaceLink from "@/app/base/components/SpaceLink"; +import TitledSection from "@/app/base/components/TitledSection"; +import { SidePanelViews, useSidePanel } from "@/app/base/side-panel-context"; import authSelectors from "@/app/store/auth/selectors"; import type { RootState } from "@/app/store/root/types"; import vlanSelectors from "@/app/store/vlan/selectors"; @@ -20,6 +19,7 @@ type Props = { const VLANSummary = ({ id }: Props): JSX.Element | null => { const isAdmin = useSelector(authSelectors.isAdmin); + const { setSidePanelContent } = useSidePanel(); const vlan = useSelector((state: RootState) => vlanSelectors.getById(state, id) ); @@ -29,33 +29,38 @@ const VLANSummary = ({ id }: Props): JSX.Element | null => { } return ( - - editing ? ( - setEditing(false)} id={id} /> - ) : ( - - - - - - - - - - - - - - - - - + + setSidePanelContent({ view: SidePanelViews.EditVLAN }) + } + > + Edit + ) } title="VLAN summary" - /> + > + + + + + + + + + + + + + + + + + + ); }; diff --git a/src/app/subnets/views/VLANDetails/constants.ts b/src/app/subnets/views/VLANDetails/constants.ts index 5f318e4bd8..0235ddc2f9 100644 --- a/src/app/subnets/views/VLANDetails/constants.ts +++ b/src/app/subnets/views/VLANDetails/constants.ts @@ -3,25 +3,31 @@ import type { ValueOf } from "@canonical/react-components"; import type { SidePanelContent } from "@/app/base/types"; export const VLANActionTypes = { + ConfigureDHCP: "ConfigureDHCP", DeleteVLAN: "DeleteVLAN", ReserveRange: "ReserveRange", DeleteReservedRange: "DeleteReservedRange", + EditVLAN: "EditVLAN", } as const; export type VLANActionType = ValueOf; export const vlanActionLabels = { + [VLANActionTypes.ConfigureDHCP]: "Configure DHCP", [VLANActionTypes.DeleteVLAN]: "Delete VLAN", [VLANActionTypes.ReserveRange]: "Reserve range", [VLANActionTypes.DeleteReservedRange]: "Delete reserved range", + [VLANActionTypes.EditVLAN]: "Edit VLAN", } as const; export const VLANDetailsSidePanelViews = { + [VLANActionTypes.ConfigureDHCP]: ["", VLANActionTypes.ConfigureDHCP], [VLANActionTypes.DeleteVLAN]: ["", VLANActionTypes.DeleteVLAN], [VLANActionTypes.ReserveRange]: ["", VLANActionTypes.ReserveRange], [VLANActionTypes.DeleteReservedRange]: [ "", VLANActionTypes.DeleteReservedRange, ], + [VLANActionTypes.EditVLAN]: ["", VLANActionTypes.EditVLAN], } as const; export type VLANDetailsSidePanelContent = SidePanelContent<