Skip to content

Commit

Permalink
B #5833: Fix service template form (#3128)
Browse files Browse the repository at this point in the history
* Fixes bug where min/max/cooldown fields would not be updated properly
* Disables all role definition inputs when no role is being selected

Signed-off-by: Victor Hansson <vhansson@opennebula.io>
  • Loading branch information
vichansson authored Jun 26, 2024
1 parent 2342005 commit 0b6e58a
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const NETWORK_TYPE = {
.trim()
.oneOf([...Object.keys(NETWORK_TYPES), ...Object.values(NETWORK_TYPES)])
.default(() => Object.values(NETWORK_TYPES)[0]),
grid: { sm: 1.5, md: 1.5 },
grid: { md: 2 },
}

// Network Name Field
Expand All @@ -45,7 +45,7 @@ const NAME = {
label: 'Name',
type: INPUT_TYPES.TEXT,
validation: string().trim().required(),
grid: { sm: 2.5, md: 2.5 },
grid: { md: 2.5 },
}

// Network Description Field
Expand All @@ -57,7 +57,7 @@ const DESCRIPTION = {
.trim()
.notRequired()
.default(() => undefined),
grid: { sm: 2.5, md: 2.5 },
grid: { md: 2.5 },
}

// Network Selection Field (for 'reserve' or 'existing')
Expand Down Expand Up @@ -93,7 +93,7 @@ const NETEXTRA = {
then: string().strip(),
otherwise: string().notRequired(),
}),
grid: { sm: 2.5, md: 2.5 },
grid: { md: 2.5 },
}

// List of Network Input Fields
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ import { Component, useMemo, useEffect } from 'react'
import { Box, FormControl } from '@mui/material'
import { createMinMaxVmsFields } from 'client/components/Forms/ServiceTemplate/CreateForm/Steps/RoleConfig/MinMaxVms/schema'
import { FormWithSchema } from 'client/components/Forms'
import { useFieldArray, useFormContext } from 'react-hook-form'
import { useFieldArray, useFormContext, useWatch } from 'react-hook-form'
import { STEP_ID as ROLE_DEFINITION_ID } from 'client/components/Forms/ServiceTemplate/CreateForm/Steps/Roles'
import { STEP_ID as ROLE_CONFIG_ID } from 'client/components/Forms/ServiceTemplate/CreateForm/Steps/RoleConfig'

export const SECTION_ID = 'MINMAXVMS'
/**
Expand All @@ -29,17 +30,32 @@ export const SECTION_ID = 'MINMAXVMS'
* @returns {Component} - component
*/
const MinMaxVms = ({ stepId, selectedRoleIndex }) => {
const { control, setValue, getValues } = useFormContext()
const cardinality = useMemo(
() =>
getValues(ROLE_DEFINITION_ID)?.[selectedRoleIndex]?.CARDINALITY ??
undefined,
[selectedRoleIndex]
)
const { control, setValue, getValues, setError, clearErrors } =
useFormContext()

const watchedActiveField = useWatch({
control,
name: `${stepId}.${SECTION_ID}.${selectedRoleIndex}`,
})

const cardinality = useMemo(() => {
const baseCardinality =
getValues(ROLE_DEFINITION_ID)?.[selectedRoleIndex]?.CARDINALITY
const minVms =
getValues(ROLE_CONFIG_ID)?.[SECTION_ID]?.[selectedRoleIndex]?.min_vms

const maxVms =
getValues(ROLE_CONFIG_ID)?.[SECTION_ID]?.[selectedRoleIndex]?.max_vms

return {
min_vms: minVms,
max_vms: maxVms,
default: baseCardinality,
}
}, [selectedRoleIndex])

const fields = createMinMaxVmsFields(
`${stepId}.${SECTION_ID}.${selectedRoleIndex}`,
cardinality
`${stepId}.${SECTION_ID}.${selectedRoleIndex}`
)

useFieldArray({
Expand All @@ -51,13 +67,37 @@ const MinMaxVms = ({ stepId, selectedRoleIndex }) => {
return null
}

useEffect(() => {
const validateFields = (activeFields) => {
const { min_vms: minVms, max_vms: maxVms } = activeFields
if (maxVms < minVms) {
;['min_vms', 'max_vms'].forEach((field) => {
setError(`${stepId}.${SECTION_ID}.${selectedRoleIndex}.${field}`, {
type: 'manual',
message: `Min/Max validation error`,
})
})
} else if (maxVms >= minVms) {
;['min_vms', 'max_vms'].forEach((field) => {
clearErrors(`${stepId}.${SECTION_ID}.${selectedRoleIndex}.${field}`)
})
}
}

watchedActiveField && validateFields(watchedActiveField)
}, [watchedActiveField])

// Set default values
useEffect(() => {
fields.forEach((field) => {
const defaultValue = field.validation.default()
setValue(field.name, defaultValue || 0)
setValue(
field.name,
cardinality?.[field?.name?.split('.')?.at(-1)] ??
cardinality?.default ??
0
)
})
}, [fields])
}, [cardinality])

return (
<Box>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ const MAX_VALUE = 999999
* Creates fields for minmax vms schema based on a path prefix.
*
* @param {string} pathPrefix - Path prefix for field names.
* @param { number } cardinality - Number of VMs defined in Role Def. step.
* @returns {object[]} - Array of field definitions for minmax vms.
*/
export const createMinMaxVmsFields = (pathPrefix, cardinality) => {
export const createMinMaxVmsFields = (pathPrefix) => {
const getPath = (fieldName) =>
pathPrefix ? `${pathPrefix}.${fieldName}` : fieldName

Expand All @@ -39,11 +38,7 @@ export const createMinMaxVmsFields = (pathPrefix, cardinality) => {
cy: 'elasticity',
validation: number()
.integer('Min VMs must be an integer')
.min(
cardinality,
`Min VMs cannot be less than defined cardinality: ${cardinality}`
)
.default(() => cardinality),
.default(() => undefined),
fieldProps: {
type: 'number',
},
Expand All @@ -56,9 +51,8 @@ export const createMinMaxVmsFields = (pathPrefix, cardinality) => {
cy: 'elasticity',
validation: number()
.integer('Max VMs must be an integer')
.min(cardinality, `Max VMs cannot be less than ${cardinality}`)
.max(MAX_VALUE, `Max VMs cannot exceed ${MAX_VALUE}`)
.default(() => cardinality),
.default(() => undefined),
fieldProps: {
type: 'number',
},
Expand All @@ -73,7 +67,7 @@ export const createMinMaxVmsFields = (pathPrefix, cardinality) => {
.integer('Cooldown must be an integer')
.min(0, 'Cooldown cannot be less than 0')
.max(MAX_VALUE, `Cooldown exceed ${MAX_VALUE}`)
.default(() => 0),
.default(() => undefined),
fieldProps: {
type: 'number',
},
Expand All @@ -86,11 +80,10 @@ export const createMinMaxVmsFields = (pathPrefix, cardinality) => {
* Creates a Yup schema for minmax vms based on a given path prefix.
*
* @param {string} pathPrefix - Path prefix for field names in the schema.
* @param { number } cardinality - Number of VMs defined in Role Def. step.
* @returns {object} - Yup schema object for minmax vms.
*/
export const createMinMaxVmsSchema = (pathPrefix, cardinality = 0) => {
const fields = createMinMaxVmsFields(pathPrefix, cardinality)
export const createMinMaxVmsSchema = (pathPrefix) => {
const fields = createMinMaxVmsFields(pathPrefix)

return object(getValidationFromFields(fields))
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,7 @@
* See the License for the specific language governing permissions and *
* limitations under the License. *
* ------------------------------------------------------------------------- */
import {
Card,
CardContent,
CardActions,
Typography,
Divider,
} from '@mui/material'
import { Card, CardContent, Typography, Divider } from '@mui/material'
import PropTypes from 'prop-types'
import { T } from 'client/constants'
import { Tr } from 'client/components/HOC'
Expand Down Expand Up @@ -166,20 +160,6 @@ const RoleSummary = ({ role, selectedRoleIndex }) => (
' ' + Tr(T.RoleAdjustmentTypePositiveNegative)}
</Typography>
</CardContent>
<CardActions sx={{ p: 2, pt: 0 }}>
<Typography
variant="subtitle2"
color="textSecondary"
sx={{ opacity: 0.7 }}
>
<strong>{Tr(T.VMGroupConfiguration)}:</strong>
<ul>
<li>{Tr(T.RoleDefineRoles)}</li>
<li>{Tr(T.RoleOptimize)}</li>
<li>{Tr(T.RoleManageApps)}</li>
</ul>
</Typography>
</CardActions>
</Card>
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ const RoleVmVmPanel = ({ roles, onChange, selectedRoleIndex }) => {
onChange({ ...roles[selectedRoleIndex], [name]: value }) // updated role
}

const isDisabled = !roles?.[selectedRoleIndex] || roles?.length <= 0

return (
<Box p={2}>
<Box sx={{ flex: 1 }}>
Expand All @@ -53,7 +55,7 @@ const RoleVmVmPanel = ({ roles, onChange, selectedRoleIndex }) => {
name="NAME"
value={roles?.[selectedRoleIndex]?.NAME ?? ''}
onChange={handleInputChange}
disabled={!roles?.[selectedRoleIndex]}
disabled={isDisabled}
inputProps={{ 'data-cy': `role-name-${selectedRoleIndex}` }}
fullWidth
/>
Expand All @@ -63,8 +65,9 @@ const RoleVmVmPanel = ({ roles, onChange, selectedRoleIndex }) => {
<TextField
type="number"
label={Tr(T.NumberOfVms)}
value={roles?.[selectedRoleIndex]?.CARDINALITY ?? 1}
value={roles?.[selectedRoleIndex]?.CARDINALITY ?? 0}
onChange={handleInputChange}
disabled={isDisabled}
name="CARDINALITY"
InputProps={{
inputProps: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const CARDINALITY_FIELD = {

validation: number()
.positive('Number of VMs must be positive')
.default(() => 1),
.default(() => 0),
}

const SELECTED_VM_TEMPLATE_ID_FIELD = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,17 @@ const VmTemplatesPanel = ({ roles, selectedRoleIndex, onChange }) => {
setPage(0)
}

const isDisabled = !roles?.[selectedRoleIndex] || roles?.length <= 0

return (
<Box
sx={{
my: 2,
p: 2,
borderRadius: 2,
maxHeight: '40%',
pointerEvents: isDisabled ? 'none' : 'auto',
opacity: isDisabled ? '50%' : '100%',
}}
>
<Typography variant="h6" gutterBottom>
Expand Down

0 comments on commit 0b6e58a

Please sign in to comment.