From 0259ba2f141502bf139620375ce8e5ef4407a313 Mon Sep 17 00:00:00 2001 From: Nick De Villiers Date: Wed, 15 May 2024 11:51:15 +0200 Subject: [PATCH 1/4] feat(subnets): Edit reserved IPs MAASENG-2976 & MAASENG-2984 --- src/app/store/reservedip/reducers.test.ts | 50 +++++++++ src/app/store/reservedip/slice.ts | 14 +++ .../ReserveDHCPLease.test.tsx | 105 +++++++++++++++++- .../ReserveDHCPLease/ReserveDHCPLease.tsx | 86 ++++++++++---- .../StaticDHCPTable/StaticDHCPTable.test.tsx | 42 ++++++- .../StaticDHCPTable/StaticDHCPTable.tsx | 7 +- .../SubnetActionForms/SubnetActionForms.tsx | 2 +- 7 files changed, 277 insertions(+), 29 deletions(-) diff --git a/src/app/store/reservedip/reducers.test.ts b/src/app/store/reservedip/reducers.test.ts index 79b4eddd66..7e869cf2d6 100644 --- a/src/app/store/reservedip/reducers.test.ts +++ b/src/app/store/reservedip/reducers.test.ts @@ -128,6 +128,56 @@ describe("create reducers", () => { }); }); +describe("update reducers", () => { + it("should correctly reduce updateStart", () => { + const initialState = factory.reservedIpState({ saving: false }); + expect(reducers(initialState, actions.updateStart())).toEqual( + factory.reservedIpState({ + saving: true, + }) + ); + }); + + it("should correctly reduce updateSuccess", () => { + const reservedIp = factory.reservedIp(); + const initialState = factory.reservedIpState({ + items: [reservedIp], + saving: true, + saved: false, + }); + expect(reducers(initialState, actions.updateSuccess(reservedIp))).toEqual( + factory.reservedIpState({ + saving: false, + saved: true, + items: [reservedIp], + }) + ); + }); + + it("should correctly reduce updateError", () => { + const initialState = factory.reservedIpState({ + saving: true, + saved: false, + }); + expect(reducers(initialState, actions.updateError("Error"))).toEqual( + factory.reservedIpState({ + saving: false, + errors: "Error", + }) + ); + }); + + it("should correctly reduce updateNotify", () => { + const items = [factory.reservedIp(), factory.reservedIp()]; + const initialState = factory.reservedIpState({ + items, + }); + expect( + reducers(initialState, actions.updateNotify(items[1])).items + ).toEqual(items); + }); +}); + describe("delete reducers", () => { it("should correctly reduce deleteStart", () => { const initialState = factory.reservedIpState({ saving: false }); diff --git a/src/app/store/reservedip/slice.ts b/src/app/store/reservedip/slice.ts index 1dfc2f0fb6..47a69d91e3 100644 --- a/src/app/store/reservedip/slice.ts +++ b/src/app/store/reservedip/slice.ts @@ -71,6 +71,20 @@ const reservedIpSlice = createSlice({ } }, }, + updateSuccess: ( + state: ReservedIpState, + action: PayloadAction + ) => { + commonReducers.updateSuccess(state); + const item = action.payload; + const index = (state.items as ReservedIp[]).findIndex( + (draftItem: ReservedIp) => + draftItem[ReservedIpMeta.PK] === item[ReservedIpMeta.PK] + ); + if (index !== -1) { + state.items[index] = item; + } + }, }, }); diff --git a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.test.tsx b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.test.tsx index 08d6bf3c49..3ecdcb7f07 100644 --- a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.test.tsx +++ b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.test.tsx @@ -37,7 +37,10 @@ afterAll(() => { it("displays an error if an invalid IP address is entered", async () => { renderWithBrowserRouter( - , + , { state } ); @@ -55,7 +58,10 @@ it("displays an error if an invalid IP address is entered", async () => { it("displays an error if an out-of-range IP address is entered", async () => { state.subnet.items = [factory.subnet({ id: 1, cidr: "10.0.0.0/25" })]; renderWithBrowserRouter( - , + , { state } ); @@ -73,7 +79,10 @@ it("displays an error if an out-of-range IP address is entered", async () => { it("closes the side panel when the cancel button is clicked", async () => { const setSidePanelContent = vi.fn(); renderWithBrowserRouter( - , + , { state } ); @@ -85,7 +94,10 @@ it("closes the side panel when the cancel button is clicked", async () => { it("dispatches an action to create a reserved IP", async () => { const store = mockStore(state); renderWithBrowserRouter( - , + , { store } ); @@ -126,3 +138,88 @@ it("dispatches an action to create a reserved IP", async () => { type: "reservedip/create", }); }); + +it("pre-fills the form if a reserved IP's ID is present", async () => { + const reservedIp = factory.reservedIp({ + id: 1, + ip: "10.0.0.2", + mac_address: "FF:FF:FF:FF:FF:FF", + comment: "bla bla bla", + }); + state.reservedip = factory.reservedIpState({ + loading: false, + loaded: true, + items: [reservedIp], + }); + + renderWithBrowserRouter( + , + { state } + ); + + expect(screen.getByRole("textbox", { name: "IP address" })).toHaveValue("2"); + expect(screen.getByRole("textbox", { name: "MAC address" })).toHaveValue( + reservedIp.mac_address + ); + expect(screen.getByRole("textbox", { name: "Comment" })).toHaveValue( + reservedIp.comment + ); +}); + +it("dispatches an action to update a reserved IP", async () => { + const reservedIp = factory.reservedIp({ + id: 1, + ip: "10.0.0.69", + mac_address: "FF:FF:FF:FF:FF:FF", + comment: "bla bla bla", + }); + state.reservedip = factory.reservedIpState({ + loading: false, + loaded: true, + items: [reservedIp], + }); + + const store = mockStore(state); + renderWithBrowserRouter( + , + { store } + ); + + await userEvent.clear(screen.getByRole("textbox", { name: "Comment" })); + + await userEvent.type( + screen.getByRole("textbox", { name: "Comment" }), + "something imaginative and funny" + ); + + await userEvent.click( + screen.getByRole("button", { name: "Update static DHCP lease" }) + ); + + expect( + store.getActions().find((action) => action.type === "reservedip/update") + ).toEqual({ + meta: { + method: "update", + model: "reservedip", + }, + payload: { + params: { + subnet: 1, + id: reservedIp.id, + ip: "10.0.0.69", + mac_address: "FF:FF:FF:FF:FF:FF", + comment: "something imaginative and funny", + }, + }, + type: "reservedip/update", + }); +}); diff --git a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx index 392804cce2..68b16e1405 100644 --- a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx +++ b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx @@ -1,4 +1,4 @@ -import { useCallback } from "react"; +import { useCallback, useMemo } from "react"; import { Spinner } from "@canonical/react-components"; import { useDispatch, useSelector } from "react-redux"; @@ -21,7 +21,10 @@ import { isIpInSubnet, } from "@/app/utils/subnetIpRange"; -type Props = Pick; +type Props = Pick< + SubnetActionProps, + "subnetId" | "setSidePanelContent" | "reservedIpId" +>; type FormValues = { ip_address: string; @@ -29,21 +32,49 @@ type FormValues = { comment: string; }; -const ReserveDHCPLease = ({ subnetId, setSidePanelContent }: Props) => { +const ReserveDHCPLease = ({ + subnetId, + setSidePanelContent, + reservedIpId, +}: Props) => { const subnet = useSelector((state: RootState) => subnetSelectors.getById(state, subnetId) ); + const reservedIp = useSelector((state: RootState) => + reservedIpSelectors.getById(state, reservedIpId) + ); const subnetLoading = useSelector(subnetSelectors.loading); const reservedIpLoading = useSelector(reservedIpSelectors.loading); const errors = useSelector(reservedIpSelectors.errors); const saving = useSelector(reservedIpSelectors.saving); const saved = useSelector(reservedIpSelectors.saved); - - const loading = subnetLoading || reservedIpLoading; - const dispatch = useDispatch(); const cleanup = useCallback(() => reservedIpActions.cleanup(), []); + const loading = subnetLoading || reservedIpLoading; + const isEditing = !!reservedIpId; + + const initialValues = useMemo(() => { + if (reservedIp && subnet) { + const [startIp, endIp] = getIpRangeFromCidr(subnet.cidr); + const [immutableOctets, _] = getImmutableAndEditableOctets( + startIp, + endIp + ); + return { + ip_address: reservedIp.ip.replace(`${immutableOctets}.`, ""), + mac_address: reservedIp.mac_address || "", + comment: reservedIp.comment || "", + }; + } else { + return { + ip_address: "", + mac_address: "", + comment: "", + }; + } + }, [reservedIp, subnet]); + const onClose = () => setSidePanelContent(null); if (loading) { @@ -96,34 +127,45 @@ const ReserveDHCPLease = ({ subnetId, setSidePanelContent }: Props) => { const handleSubmit = (values: FormValues) => { dispatch(cleanup()); - - dispatch( - reservedIpActions.create({ - comment: values.comment, - ip: `${immutableOctets}.${values.ip_address}`, - mac_address: values.mac_address, - subnet: subnetId, - }) - ); + if (isEditing) { + dispatch( + reservedIpActions.update({ + comment: values.comment, + ip: `${immutableOctets}.${values.ip_address}`, + mac_address: values.mac_address, + subnet: subnetId, + id: reservedIpId, + }) + ); + } else { + dispatch( + reservedIpActions.create({ + comment: values.comment, + ip: `${immutableOctets}.${values.ip_address}`, + mac_address: values.mac_address, + subnet: subnetId, + }) + ); + } }; return ( - aria-label="Reserve static DHCP lease" + aria-label={ + isEditing ? "Edit static DHCP lease" : "Reserve static DHCP lease" + } cleanup={cleanup} errors={errors} - initialValues={{ - ip_address: "", - mac_address: "", - comment: "", - }} + initialValues={initialValues} onCancel={onClose} onSubmit={handleSubmit} onSuccess={onClose} resetOnSave saved={saved} saving={saving} - submitLabel="Reserve static DHCP lease" + submitLabel={ + isEditing ? "Update static DHCP lease" : "Reserve static DHCP lease" + } validationSchema={ReserveDHCPLeaseSchema} > { + vi.spyOn(sidePanelHooks, "useSidePanel").mockReturnValue({ + setSidePanelContent, + sidePanelContent: null, + setSidePanelSize: vi.fn(), + sidePanelSize: "regular", + }); +}); it("renders a static DHCP table with no data", () => { renderWithBrowserRouter(); @@ -27,3 +39,31 @@ it("renders a static DHCP table when data is provided", () => { screen.getByRole("cell", { name: reservedIps[0].ip }) ).toBeInTheDocument(); }); + +it("opens the side panel with the correct view when the edit button is clicked", async () => { + const reservedIps = [reservedIp()]; + renderWithBrowserRouter( + + ); + + await userEvent.click(screen.getByRole("button", { name: "Edit" })); + + expect(setSidePanelContent).toHaveBeenCalledWith({ + extras: { reservedIpId: reservedIps[0].id }, + view: ["", SubnetActionTypes.ReserveDHCPLease], + }); +}); + +it("opens the side panel with the correct view when the delete button is clicked", async () => { + const reservedIps = [reservedIp()]; + renderWithBrowserRouter( + + ); + + await userEvent.click(screen.getByRole("button", { name: "Delete" })); + + expect(setSidePanelContent).toHaveBeenCalledWith({ + extras: { reservedIpId: reservedIps[0].id }, + view: ["", SubnetActionTypes.DeleteDHCPLease], + }); +}); diff --git a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/StaticDHCPTable/StaticDHCPTable.tsx b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/StaticDHCPTable/StaticDHCPTable.tsx index 329e80c258..a63289f48f 100644 --- a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/StaticDHCPTable/StaticDHCPTable.tsx +++ b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/StaticDHCPTable/StaticDHCPTable.tsx @@ -62,7 +62,12 @@ const generateRows = ( extras: { reservedIpId: reservedIp.id }, }) } - onEdit={() => {}} + onEdit={() => + setSidePanelContent({ + view: SubnetDetailsSidePanelViews.ReserveDHCPLease, + extras: { reservedIpId: reservedIp.id }, + }) + } /> diff --git a/src/app/subnets/views/SubnetDetails/SubnetActionForms/SubnetActionForms.tsx b/src/app/subnets/views/SubnetDetails/SubnetActionForms/SubnetActionForms.tsx index 12b016d606..6e96d27deb 100644 --- a/src/app/subnets/views/SubnetDetails/SubnetActionForms/SubnetActionForms.tsx +++ b/src/app/subnets/views/SubnetDetails/SubnetActionForms/SubnetActionForms.tsx @@ -28,7 +28,7 @@ const FormComponents: Record< [SubnetActionTypes.ReserveRange]: ReservedRangeForm, [SubnetActionTypes.DeleteReservedRange]: ReservedRangeDeleteForm, [SubnetActionTypes.ReserveDHCPLease]: ReserveDHCPLease, - [SubnetActionTypes.EditDHCPLease]: () => null, + [SubnetActionTypes.EditDHCPLease]: ReserveDHCPLease, [SubnetActionTypes.DeleteDHCPLease]: DeleteDHCPLease, }; From 042ea8a3fd06398b4ceb1a40d8a95fbb27011d7f Mon Sep 17 00:00:00 2001 From: Nick De Villiers Date: Wed, 15 May 2024 12:26:12 +0200 Subject: [PATCH 2/4] add the wee little character counter thingy --- .../ReserveDHCPLease/ReserveDHCPLease.tsx | 39 ++++++++++++------- src/scss/_utilities.scss | 4 ++ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx index 68b16e1405..be27c397c4 100644 --- a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx +++ b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx @@ -21,6 +21,8 @@ import { isIpInSubnet, } from "@/app/utils/subnetIpRange"; +const MAX_COMMENT_LENGTH = 255; + type Props = Pick< SubnetActionProps, "subnetId" | "setSidePanelContent" | "reservedIpId" @@ -168,20 +170,29 @@ const ReserveDHCPLease = ({ } validationSchema={ReserveDHCPLeaseSchema} > - - - + {({ values }: { values: FormValues }) => ( + <> + + + + + {values.comment.length}/{MAX_COMMENT_LENGTH} + + + )} ); }; diff --git a/src/scss/_utilities.scss b/src/scss/_utilities.scss index 3ff5083f18..9d04255dd5 100644 --- a/src/scss/_utilities.scss +++ b/src/scss/_utilities.scss @@ -112,6 +112,10 @@ flex-wrap: wrap !important; } + .u-margin-bottom--x-small { + margin-bottom: $sp-x-small !important; + } + .u-no-border { border: 0 !important; } From f623cfbb8b54e7239cebc0f55a8c1032e02d4f38 Mon Sep 17 00:00:00 2001 From: Nick De Villiers Date: Wed, 15 May 2024 13:39:24 +0200 Subject: [PATCH 3/4] de-memoize inital values --- .../StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx index be27c397c4..30b683fa7d 100644 --- a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx +++ b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx @@ -1,4 +1,4 @@ -import { useCallback, useMemo } from "react"; +import { useCallback } from "react"; import { Spinner } from "@canonical/react-components"; import { useDispatch, useSelector } from "react-redux"; @@ -56,7 +56,7 @@ const ReserveDHCPLease = ({ const loading = subnetLoading || reservedIpLoading; const isEditing = !!reservedIpId; - const initialValues = useMemo(() => { + const getInitialValues = () => { if (reservedIp && subnet) { const [startIp, endIp] = getIpRangeFromCidr(subnet.cidr); const [immutableOctets, _] = getImmutableAndEditableOctets( @@ -75,7 +75,9 @@ const ReserveDHCPLease = ({ comment: "", }; } - }, [reservedIp, subnet]); + }; + + const initialValues = getInitialValues(); const onClose = () => setSidePanelContent(null); From c1d08b6a97988e3402c7b8a03569c48767db5baf Mon Sep 17 00:00:00 2001 From: Nick De Villiers Date: Wed, 15 May 2024 14:50:11 +0200 Subject: [PATCH 4/4] enable form reinit --- .../StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx index 30b683fa7d..f23f0730c0 100644 --- a/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx +++ b/src/app/subnets/views/SubnetDetails/StaticDHCPLease/ReserveDHCPLease/ReserveDHCPLease.tsx @@ -159,6 +159,7 @@ const ReserveDHCPLease = ({ isEditing ? "Edit static DHCP lease" : "Reserve static DHCP lease" } cleanup={cleanup} + enableReinitialize errors={errors} initialValues={initialValues} onCancel={onClose}