From aa540ab6106cde61d899d2f08a9a7088f78efa4d Mon Sep 17 00:00:00 2001 From: Mason Hu Date: Wed, 19 Feb 2025 13:41:37 +0200 Subject: [PATCH] review: review changes Signed-off-by: Mason Hu --- src/api/networks.tsx | 67 +++------------ src/components/forms/YamlForm.tsx | 9 +- src/context/useNetworks.tsx | 59 ++++++++++--- src/pages/instances/EditInstance.tsx | 2 +- src/pages/networks/EditNetwork.tsx | 40 +++------ src/pages/networks/NetworkForwards.tsx | 82 ++++++++++--------- src/pages/networks/NetworkList.tsx | 18 ++-- .../actions/DeleteNetworkForwardBtn.tsx | 2 +- src/pages/networks/forms/NetworkForm.tsx | 2 +- .../networks/forms/NetworkParentSelector.tsx | 18 ++-- tests/helpers/network.ts | 2 +- 11 files changed, 130 insertions(+), 171 deletions(-) diff --git a/src/api/networks.tsx b/src/api/networks.tsx index e39808f50b..97fa58b04a 100644 --- a/src/api/networks.tsx +++ b/src/api/networks.tsx @@ -16,18 +16,19 @@ import { withEntitlementsQuery } from "util/entitlements/api"; const networkEntitlements = ["can_edit", "can_delete"]; -export const fetchNetworksOnMember = ( +export const fetchNetworks = ( project: string, - target: string, isFineGrained: boolean | null, + target?: string, ): Promise => { + const targetParam = target ? `&target=${target}` : ""; const entitlements = `&${withEntitlementsQuery( isFineGrained, networkEntitlements, )}`; return new Promise((resolve, reject) => { fetch( - `/1.0/networks?project=${project}&recursion=1&target=${target}${entitlements}`, + `/1.0/networks?project=${project}&recursion=1${targetParam}${entitlements}`, ) .then(handleResponse) .then((data: LxdApiResponse) => { @@ -42,29 +43,6 @@ export const fetchNetworksOnMember = ( }); }; -export const fetchNetworks = ( - project: string, - isFineGrained: boolean | null, -): Promise => { - const entitlements = `&${withEntitlementsQuery( - isFineGrained, - networkEntitlements, - )}`; - return new Promise((resolve, reject) => { - fetch(`/1.0/networks?project=${project}&recursion=1${entitlements}`) - .then(handleResponse) - .then((data: LxdApiResponse) => { - const filteredNetworks = data.metadata.filter( - // Filter out loopback and unknown networks, both are not useful for the user. - // this is in line with the filtering done in the LXD CLI - (network) => !["loopback", "unknown"].includes(network.type), - ); - resolve(filteredNetworks); - }) - .catch(reject); - }); -}; - export const fetchNetworksFromClusterMembers = ( project: string, clusterMembers: LxdClusterMember[], @@ -73,11 +51,7 @@ export const fetchNetworksFromClusterMembers = ( return new Promise((resolve, reject) => { Promise.allSettled( clusterMembers.map((member) => { - return fetchNetworksOnMember( - project, - member.server_name, - isFineGrained, - ); + return fetchNetworks(project, isFineGrained, member.server_name); }), ) .then((results) => { @@ -100,19 +74,20 @@ export const fetchNetworksFromClusterMembers = ( }); }; -export const fetchNetworkOnMember = ( +export const fetchNetwork = ( name: string, project: string, - target: string, isFineGrained: boolean | null, + target?: string, ): Promise => { + const targetParam = target ? `&target=${target}` : ""; const entitlements = `&${withEntitlementsQuery( isFineGrained, networkEntitlements, )}`; return new Promise((resolve, reject) => { fetch( - `/1.0/networks/${name}?project=${project}&target=${target}${entitlements}`, + `/1.0/networks/${name}?project=${project}${targetParam}${entitlements}`, ) .then(handleEtagResponse) .then((data) => resolve(data as LxdNetwork)) @@ -120,23 +95,6 @@ export const fetchNetworkOnMember = ( }); }; -export const fetchNetwork = ( - name: string, - project: string, - isFineGrained: boolean | null, -): Promise => { - const entitlements = `&${withEntitlementsQuery( - isFineGrained, - networkEntitlements, - )}`; - return new Promise((resolve, reject) => { - fetch(`/1.0/networks/${name}?project=${project}${entitlements}`) - .then(handleEtagResponse) - .then((data) => resolve(data as LxdNetwork)) - .catch(reject); - }); -}; - export const fetchNetworkFromClusterMembers = ( name: string, project: string, @@ -146,12 +104,7 @@ export const fetchNetworkFromClusterMembers = ( return new Promise((resolve, reject) => { Promise.allSettled( clusterMembers.map((member) => { - return fetchNetworkOnMember( - name, - project, - member.server_name, - isFineGrained, - ); + return fetchNetwork(name, project, isFineGrained, member.server_name); }), ) .then((results) => { diff --git a/src/components/forms/YamlForm.tsx b/src/components/forms/YamlForm.tsx index edff933dfd..a7f8512eab 100644 --- a/src/components/forms/YamlForm.tsx +++ b/src/components/forms/YamlForm.tsx @@ -2,10 +2,7 @@ import { FC, ReactNode, useEffect, useRef, useState } from "react"; import { Editor, loader } from "@monaco-editor/react"; import { updateMaxHeight } from "util/updateMaxHeight"; import useEventListener from "util/useEventListener"; -import { - editor, - IMarkdownString, -} from "monaco-editor/esm/vs/editor/editor.api"; +import { editor } from "monaco-editor/esm/vs/editor/editor.api"; import IStandaloneCodeEditor = editor.IStandaloneCodeEditor; import classnames from "classnames"; @@ -19,7 +16,7 @@ interface Props { children?: ReactNode; autoResize?: boolean; readOnly?: boolean; - readOnlyMessage?: IMarkdownString; + readOnlyMessage?: string; } const YamlForm: FC = ({ @@ -75,7 +72,7 @@ const YamlForm: FC = ({ }, overviewRulerLanes: 0, readOnly: readOnly, - readOnlyMessage: readOnlyMessage, + readOnlyMessage: { value: readOnlyMessage ?? "" }, }} onMount={(editor: IStandaloneCodeEditor) => { setEditor(editor); diff --git a/src/context/useNetworks.tsx b/src/context/useNetworks.tsx index 6ef4769e5b..451f8ba1b3 100644 --- a/src/context/useNetworks.tsx +++ b/src/context/useNetworks.tsx @@ -4,11 +4,12 @@ import { UseQueryResult } from "@tanstack/react-query"; import { useAuth } from "./auth"; import { fetchNetwork, - fetchNetworkOnMember, + fetchNetworkFromClusterMembers, fetchNetworks, - fetchNetworksOnMember, + fetchNetworksFromClusterMembers, } from "api/networks"; -import { LxdNetwork } from "types/network"; +import { LxdNetwork, LXDNetworkOnClusterMember } from "types/network"; +import { LxdClusterMember } from "types/cluster"; export const useNetworks = ( project: string, @@ -16,9 +17,6 @@ export const useNetworks = ( enabled?: boolean, ): UseQueryResult => { const { isFineGrained } = useAuth(); - const queryFn = target - ? () => fetchNetworksOnMember(project, target, isFineGrained) - : () => fetchNetworks(project, isFineGrained); const queryKey = [queryKeys.projects, project, queryKeys.networks]; if (target) { @@ -28,7 +26,7 @@ export const useNetworks = ( return useQuery({ queryKey, - queryFn: queryFn, + queryFn: () => fetchNetworks(project, isFineGrained, target), enabled: (enabled ?? true) && isFineGrained !== null, }); }; @@ -40,9 +38,6 @@ export const useNetwork = ( enabled?: boolean, ): UseQueryResult => { const { isFineGrained } = useAuth(); - const queryFn = target - ? () => fetchNetworkOnMember(network, project, target, isFineGrained) - : () => fetchNetwork(network, project, isFineGrained); const queryKey = [queryKeys.projects, project, queryKeys.networks, network]; if (target) { @@ -52,7 +47,49 @@ export const useNetwork = ( return useQuery({ queryKey, - queryFn, + queryFn: () => fetchNetwork(network, project, isFineGrained, target), + enabled: (enabled ?? true) && isFineGrained !== null, + }); +}; + +export const useNetworksFromClusterMembers = ( + project: string, + clusterMembers: LxdClusterMember[], + enabled?: boolean, +): UseQueryResult => { + const { isFineGrained } = useAuth(); + + return useQuery({ + queryKey: [queryKeys.networks, project, queryKeys.cluster], + queryFn: () => + fetchNetworksFromClusterMembers(project, clusterMembers, isFineGrained), + enabled: (enabled ?? true) && isFineGrained !== null, + }); +}; + +export const useNetworkFromClusterMembers = ( + network: string, + project: string, + clusterMembers: LxdClusterMember[], + enabled?: boolean, +): UseQueryResult => { + const { isFineGrained } = useAuth(); + + return useQuery({ + queryKey: [ + queryKeys.projects, + project, + queryKeys.networks, + network, + queryKeys.cluster, + ], + queryFn: () => + fetchNetworkFromClusterMembers( + network, + project, + clusterMembers, + isFineGrained, + ), enabled: (enabled ?? true) && isFineGrained !== null, }); }; diff --git a/src/pages/instances/EditInstance.tsx b/src/pages/instances/EditInstance.tsx index 60d90f8010..c7940b087f 100644 --- a/src/pages/instances/EditInstance.tsx +++ b/src/pages/instances/EditInstance.tsx @@ -276,7 +276,7 @@ const EditInstance: FC = ({ instance }) => { void formik.setFieldValue("yaml", yaml); }} readOnly={!!formik.values.editRestriction} - readOnlyMessage={{ value: formik.values.editRestriction ?? "" }} + readOnlyMessage={formik.values.editRestriction} > = ({ network, project }) => { const { hash } = useLocation(); const initialSection = hash ? hash.substring(1) : slugify(CONNECTIONS); const [section, updateSection] = useState(initialSection); - const { isFineGrained } = useAuth(); const queryClient = useQueryClient(); const controllerState = useState(null); @@ -57,27 +52,14 @@ const EditNetwork: FC = ({ network, project }) => { const isClustered = clusterMembers.length > 0; const { canEditNetwork } = useNetworkEntitlements(); - const { data: networkOnMembers = [], error } = useQuery({ - queryKey: [ - queryKeys.projects, - project, - queryKeys.networks, - network.name, - queryKeys.cluster, - ], - queryFn: () => - fetchNetworkFromClusterMembers( - network.name, - project, - clusterMembers, - isFineGrained, - ), - enabled: - isClustered && - network.managed && - network.type === "physical" && - isFineGrained !== null, - }); + const queryEnabled = + isClustered && network.managed && network.type === "physical"; + const { data: networkOnMembers = [], error } = useNetworkFromClusterMembers( + network.name, + project, + clusterMembers, + queryEnabled, + ); useEffect(() => { if (error) { diff --git a/src/pages/networks/NetworkForwards.tsx b/src/pages/networks/NetworkForwards.tsx index 034acb4e3a..6d72c95640 100644 --- a/src/pages/networks/NetworkForwards.tsx +++ b/src/pages/networks/NetworkForwards.tsx @@ -14,7 +14,7 @@ import Loader from "components/Loader"; import { fetchNetworkForwards } from "api/network-forwards"; import { useDocs } from "context/useDocs"; import DeleteNetworkForwardBtn from "pages/networks/actions/DeleteNetworkForwardBtn"; -import { useNavigate } from "react-router-dom"; +import { Link } from "react-router-dom"; import ExpandableList from "components/ExpandableList"; import NetworkForwardPort from "pages/networks/NetworkForwardPort"; import ScrollableTable from "components/ScrollableTable"; @@ -29,7 +29,6 @@ const NetworkForwards: FC = ({ network, project }) => { const docBaseLink = useDocs(); const notify = useNotify(); const { canEditNetwork } = useNetworkEntitlements(); - const navigate = useNavigate(); const { data: forwards = [], @@ -93,27 +92,29 @@ const NetworkForwards: FC = ({ network, project }) => { { content: ( <> - + {canEditNetwork(network) && ( + + + + )} + {!canEditNetwork(network) && ( + + )} = ({ network, project }) => { return ( <> - + {canEditNetwork(network) && ( + + Create forward + + )} + {!canEditNetwork(network) && ( + + )} {hasNetworkForwards && ( { const { data: clusterMembers = [] } = useClusterMembers(); const isClustered = clusterMembers.length > 0; const [searchParams] = useSearchParams(); - const { isFineGrained } = useAuth(); const { canCreateNetworks } = useProjectEntitlements(); const { project: currentProject } = useCurrentProject(); @@ -74,16 +72,12 @@ const NetworkList: FC = () => { } }, [error]); + const queryEnabled = clusterMembers.length > 0; const { data: networksOnClusterMembers = [], error: clusterNetworkError, isLoading: isClusterNetworksLoading, - } = useQuery({ - queryKey: [queryKeys.networks, project, queryKeys.cluster], - queryFn: () => - fetchNetworksFromClusterMembers(project, clusterMembers, isFineGrained), - enabled: clusterMembers.length > 0 && isFineGrained !== null, - }); + } = useNetworksFromClusterMembers(project, clusterMembers, queryEnabled); useEffect(() => { if (clusterNetworkError) { diff --git a/src/pages/networks/actions/DeleteNetworkForwardBtn.tsx b/src/pages/networks/actions/DeleteNetworkForwardBtn.tsx index 9c29147334..7f654e6b3d 100644 --- a/src/pages/networks/actions/DeleteNetworkForwardBtn.tsx +++ b/src/pages/networks/actions/DeleteNetworkForwardBtn.tsx @@ -51,7 +51,7 @@ const DeleteNetworkForwardBtn: FC = ({ network, forward, project }) => { onHoverText={ canDeleteNetwork(network) ? "Delete network forward" - : "You do not have permission to delete this forwards for this network" + : "You do not have permission to delete this network forward" } confirmationModalProps={{ title: "Confirm delete", diff --git a/src/pages/networks/forms/NetworkForm.tsx b/src/pages/networks/forms/NetworkForm.tsx index 8b26af5532..d471dc419c 100644 --- a/src/pages/networks/forms/NetworkForm.tsx +++ b/src/pages/networks/forms/NetworkForm.tsx @@ -322,7 +322,7 @@ const NetworkForm: FC = ({ void formik.setFieldValue("yaml", yaml); }} readOnly={!!formik.values.editRestriction} - readOnlyMessage={{ value: formik.values.editRestriction ?? "" }} + readOnlyMessage={formik.values.editRestriction} > ; @@ -32,7 +31,6 @@ const NetworkParentSelector: FC = ({ props, formik, isClustered }) => { const { project } = useParams<{ project: string }>(); const { data: clusterMembers = [] } = useClusterMembers(); const notify = useNotify(); - const { isFineGrained } = useAuth(); if (!project) { return <>Missing project; @@ -51,16 +49,12 @@ const NetworkParentSelector: FC = ({ props, formik, isClustered }) => { } }, [networkError]); + const queryEnabled = clusterMembers.length > 0; const { data: networksOnClusterMembers = [], error: clusterNetworkError, isLoading: isClusterNetworksLoading, - } = useQuery({ - queryKey: [queryKeys.networks, "default", queryKeys.cluster], - queryFn: () => - fetchNetworksFromClusterMembers("default", clusterMembers, isFineGrained), - enabled: clusterMembers.length > 0 && isFineGrained !== null, - }); + } = useNetworksFromClusterMembers("default", clusterMembers, queryEnabled); useEffect(() => { if (clusterNetworkError) { diff --git a/tests/helpers/network.ts b/tests/helpers/network.ts index 0f31079837..2eb6d5d047 100644 --- a/tests/helpers/network.ts +++ b/tests/helpers/network.ts @@ -74,7 +74,7 @@ export const createNetworkForward = async (page: Page, network: string) => { const targetAddress = networkSubnet.replace("1/24", "3"); await page.getByTestId("tab-link-Forwards").click(); - await page.getByRole("button", { name: "Create forward" }).click(); + await page.getByRole("link", { name: "Create forward" }).click(); await page.getByLabel("Listen address").fill(listenAddress); await page.getByRole("button", { name: "Add port" }).click();