From 9cfea08978acede50d6929dfd753de06664c0115 Mon Sep 17 00:00:00 2001 From: David Martin Date: Mon, 16 Sep 2024 15:23:20 +0100 Subject: [PATCH] Fix showing of server side validation error response in RLP & DNS create views Signed-off-by: David Martin --- .../en/plugin__console-plugin-template.json | 3 +- package.json | 1 - .../KuadrantDNSPolicyCreatePage.tsx | 89 +++++---------- .../KuadrantRateLimitPolicyCreatePage.tsx | 108 ++++++++---------- src/components/namespace/NamespaceSelect.tsx | 2 +- yarn.lock | 31 +---- 6 files changed, 81 insertions(+), 153 deletions(-) diff --git a/locales/en/plugin__console-plugin-template.json b/locales/en/plugin__console-plugin-template.json index b5fd6c7..30a2a4a 100644 --- a/locales/en/plugin__console-plugin-template.json +++ b/locales/en/plugin__console-plugin-template.json @@ -68,5 +68,6 @@ "Configured Limits": "Configured Limits", "Target reference type": "Target reference type", "Unique name of the RateLimitPolicy": "Unique name of the RateLimitPolicy", - "RateLimitPolicy enables rate limiting for service workloads in a Gateway API network": "RateLimitPolicy enables rate limiting for service workloads in a Gateway API network" + "RateLimitPolicy enables rate limiting for service workloads in a Gateway API network": "RateLimitPolicy enables rate limiting for service workloads in a Gateway API network", + "To set defaults, overrides, and more complex limits, use the YAML view.": "To set defaults, overrides, and more complex limits, use the YAML view." } diff --git a/package.json b/package.json index e5d476b..e0b8a95 100644 --- a/package.json +++ b/package.json @@ -93,7 +93,6 @@ "babel-loader": "^8.2.0", "graphlib": "^2.1.8", "graphlib-dot": "^0.6.4", - "js-yaml": "^4.1.0", "react-policy-topology": "^0.1.10" } } diff --git a/src/components/KuadrantDNSPolicyCreatePage.tsx b/src/components/KuadrantDNSPolicyCreatePage.tsx index 92531bd..3a1ab3a 100644 --- a/src/components/KuadrantDNSPolicyCreatePage.tsx +++ b/src/components/KuadrantDNSPolicyCreatePage.tsx @@ -1,6 +1,5 @@ import * as React from 'react'; import Helmet from 'react-helmet'; -import yaml from 'js-yaml'; import { PageSection, Title, @@ -14,8 +13,10 @@ import { Button, ExpandableSection, ButtonVariant, - Modal, ActionGroup, + AlertVariant, + Alert, + AlertGroup, } from '@patternfly/react-core'; import { useTranslation } from 'react-i18next'; import './kuadrant.css'; @@ -27,9 +28,8 @@ import HealthCheckField from './dnspolicy/HealthCheckField'; import getModelFromResource from '../utils/getModelFromResource'; import { Gateway } from './gateway/types'; import GatewaySelect from './gateway/GatewaySelect'; -import { ModalHeader, ModalBody, ModalFooter } from '@patternfly/react-core/next'; import NamespaceSelect from './namespace/NamespaceSelect'; -import { removeUndefinedFields, convertMatchLabelsArrayToObject, convertMatchLabelsObjectToArray } from '../utils/modelUtils'; +import { removeUndefinedFields, convertMatchLabelsArrayToObject } from '../utils/modelUtils'; const KuadrantDNSPolicyCreatePage: React.FC = () => { const { t } = useTranslation('plugin__console-plugin-template'); @@ -48,6 +48,7 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => { port: null, protocol: 'HTTP', }); + const [isCreateButtonDisabled, setIsCreateButtonDisabled] = React.useState(true); // Initialize the YAML resource object based on form state const [yamlResource, setYamlResource] = React.useState(() => { @@ -107,12 +108,15 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => { healthCheck: healthCheck.endpoint ? healthCheck : undefined, }, }; - + setYamlResource(removeUndefinedFields(updatedYamlResource)); // Clean undefined values - }, [policy, selectedNamespace, selectedGateway, routingStrategy, loadBalancing, healthCheck]); - const [isErrorModalOpen, setIsErrorModalOpen] = React.useState(false); - const [errorModalMsg, setErrorModalMsg] = React.useState('') + // Check if the Create button should be enabled + const isFormValid = policy && selectedNamespace && selectedGateway.name; + setIsCreateButtonDisabled(!isFormValid); // Update the button state + }, [policy, selectedNamespace, selectedGateway, routingStrategy, loadBalancing, healthCheck]); + + const [errorAlertMsg, setErrorAlertMsg] = React.useState('') const handleCreateViewChange = (value: 'form' | 'yaml') => { setCreateView(value); @@ -122,39 +126,10 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => { setPolicy(policy); }; - const handleYamlSave = (content: string) => { - try { - const parsedYaml = yaml.load(content) as DNSPolicy; - if (parsedYaml) { - setPolicy(parsedYaml.metadata.name || ''); - setSelectedNamespace(parsedYaml.metadata.namespace || ''); - setRoutingStrategy(parsedYaml.spec.routingStrategy); - setSelectedGateway(parsedYaml.spec.targetRef); - setLoadBalancing({ - ...parsedYaml.spec.loadBalancing, - weighted: { - ...parsedYaml.spec.loadBalancing?.weighted, - custom: parsedYaml.spec.loadBalancing?.weighted?.custom?.map((customWeight) => ({ - ...customWeight, - selector: { - ...customWeight.selector, - // Convert map back to array for form rendering - matchLabels: convertMatchLabelsObjectToArray( - (typeof customWeight.selector.matchLabels === 'object' ? customWeight.selector.matchLabels : {}) as { [key: string]: string } - ), - }, - })), - }, - }); - setHealthCheck(parsedYaml.spec.healthCheck || healthCheck); - } - } catch (error) { - console.error('Error parsing YAML:', error); - } - handleSubmit(); - }; - const handleSubmit = async () => { + if (isCreateButtonDisabled) return; // Early return if form is not valid + setErrorAlertMsg('') + const isHealthCheckValid = healthCheck.endpoint && healthCheck.failureThreshold > 0 && @@ -216,9 +191,8 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => { }); history.push('/kuadrant/all-namespaces/policies/dns'); // Navigate after successful creation } catch (error) { - console.error(t('Error creating DNSPolicy'), error); - setErrorModalMsg(error) - setIsErrorModalOpen(true) + console.error(t('Error creating DNSPolicy'), { error }); + setErrorAlertMsg(error.message) } }; @@ -290,8 +264,16 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => { + {errorAlertMsg != '' && ( + + + {errorAlertMsg} + + + )} + - - - ); }; diff --git a/src/components/KuadrantRateLimitPolicyCreatePage.tsx b/src/components/KuadrantRateLimitPolicyCreatePage.tsx index 0f0f0e3..d0beb93 100644 --- a/src/components/KuadrantRateLimitPolicyCreatePage.tsx +++ b/src/components/KuadrantRateLimitPolicyCreatePage.tsx @@ -19,6 +19,8 @@ import { Flex, FlexItem, Alert, + AlertGroup, + AlertVariant, } from '@patternfly/react-core'; import { useTranslation } from 'react-i18next'; import './kuadrant.css'; @@ -28,7 +30,6 @@ import { LimitConfig, RateLimitPolicy } from './ratelimitpolicy/types' import getModelFromResource from '../utils/getModelFromResource'; import { Gateway } from './gateway/types'; import GatewaySelect from './gateway/GatewaySelect'; -import { ModalHeader, ModalBody, ModalFooter } from '@patternfly/react-core/next'; import NamespaceSelect from './namespace/NamespaceSelect'; import HTTPRouteSelect from './httproute/HTTPRouteSelect'; import { HTTPRoute } from './httproute/types'; @@ -54,6 +55,7 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => { rates: [{ duration: 60, limit: 100, unit: 'minute' }] }); const [rateName, setRateName] = React.useState(''); + const [isCreateButtonDisabled, setIsCreateButtonDisabled] = React.useState(true); const handleOpenModal = () => { // Reset temporary state when opening modal for new entry @@ -110,30 +112,6 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => { }; }); - React.useEffect(() => { - const newTargetRef = targetType === 'gateway' - ? { - group: 'gateway.networking.k8s.io', - kind: 'Gateway', - name: selectedGateway.name, - namespace: selectedGateway.namespace, - } - : { - group: 'gateway.networking.k8s.io', - kind: 'HTTPRoute', - name: selectedHTTPRoute.name, - namespace: selectedHTTPRoute.namespace, - }; - setYamlResource((prevResource) => ({ - ...prevResource, - spec: { - ...prevResource.spec, - targetRef: newTargetRef, - limits, - }, - })); - }, [targetType, selectedGateway, selectedHTTPRoute]); - const history = useHistory(); React.useEffect(() => { @@ -145,21 +123,31 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => { namespace: selectedNamespace, }, spec: { - targetRef: { + targetRef: targetType === 'gateway' + ? { group: 'gateway.networking.k8s.io', kind: 'Gateway', name: selectedGateway.name, namespace: selectedGateway.namespace, + } + : { + group: 'gateway.networking.k8s.io', + kind: 'HTTPRoute', + name: selectedHTTPRoute.name, + namespace: selectedHTTPRoute.namespace, }, limits: { ...limits }, }, }; setYamlResource(updatedYamlResource); - }, [policy, selectedNamespace, selectedGateway, limits]); - const [isErrorModalOpen, setIsErrorModalOpen] = React.useState(false); - const [errorModalMsg, setErrorModalMsg] = React.useState('') + // Check if the Create button should be enabled + const isFormValid = policy && selectedNamespace && (selectedGateway.name || selectedHTTPRoute.name); + setIsCreateButtonDisabled(!isFormValid); // Update the button state + }, [policy, selectedNamespace, targetType, selectedGateway, selectedHTTPRoute, limits]); + + const [errorAlertMsg, setErrorAlertMsg] = React.useState('') const handleCreateViewChange = (value: 'form' | 'yaml') => { setCreateView(value); @@ -170,6 +158,8 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => { }; const handleSubmit = async () => { + if (isCreateButtonDisabled) return; // Early return if form is not valid + setErrorAlertMsg('') const ratelimitPolicy: RateLimitPolicy = { apiVersion: 'kuadrant.io/v1beta2', kind: 'RateLimitPolicy', @@ -178,12 +168,19 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => { namespace: selectedNamespace, }, spec: { - targetRef: { - group: 'gateway.networking.k8s.io', - kind: 'Gateway', - name: selectedGateway.name, - namespace: selectedGateway.namespace - }, + targetRef: targetType === 'gateway' + ? { + group: 'gateway.networking.k8s.io', + kind: 'Gateway', + name: selectedGateway.name, + namespace: selectedGateway.namespace, + } + : { + group: 'gateway.networking.k8s.io', + kind: 'HTTPRoute', + name: selectedHTTPRoute.name, + namespace: selectedHTTPRoute.namespace, + }, }, }; @@ -196,8 +193,7 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => { history.push('/kuadrant/all-namespaces/policies/ratelimit'); // Navigate after successful creation } catch (error) { console.error(t('Error creating RateLimitPolicy'), error); - setErrorModalMsg(error) - setIsErrorModalOpen(true) + setErrorAlertMsg(error.message) } }; @@ -316,13 +312,22 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => { - + + + + {errorAlertMsg != '' && ( + + {errorAlertMsg} + + )} + + - - - ); }; diff --git a/src/components/namespace/NamespaceSelect.tsx b/src/components/namespace/NamespaceSelect.tsx index d5b826e..c1cf009 100644 --- a/src/components/namespace/NamespaceSelect.tsx +++ b/src/components/namespace/NamespaceSelect.tsx @@ -30,7 +30,7 @@ const NamespaceSelect: React.FC = ({ selectedNamespace, on onChange(event.currentTarget.value); }; return ( - +