diff --git a/client/web/src/settings/ApplicationComponent.js b/client/web/src/settings/ApplicationComponent.js index ab9bcef8..746bbc82 100644 --- a/client/web/src/settings/ApplicationComponent.js +++ b/client/web/src/settings/ApplicationComponent.js @@ -171,7 +171,6 @@ export function ApplicationComponent(props) { provisionObjectStorage: !!thisService?.s3, windowsVersion: windowsVersion, tiers: initialTierValues, - tombstone: false, } } @@ -199,15 +198,12 @@ export function ApplicationComponent(props) { } // min, max, computeSize, cpu/memory/instanceType (not in form), filesystem, database - const singleTierValidationSpec = (tombstone) => { + const singleTierValidationSpec = () => { return Yup.object({ - min: requiredIfNotTombstoned( - tombstone, - Yup.number() - .integer('Minimum count must be an integer value') - .min(1, 'Minimum count must be at least ${min}'), - 'Minimum count is a required field.' - ), + min: Yup.number() + .integer('Minimum count must be an integer value') + .min(1, 'Minimum count must be at least ${min}') + .required('Minimum count is a required field.'), max: Yup.number() .required('Maximum count is a required field.') .integer('Maximum count must be an integer value') @@ -215,11 +211,7 @@ export function ApplicationComponent(props) { .test('match', 'Max cannot be smaller than min', function (max) { return max >= this.parent.min }), - computeSize: requiredIfNotTombstoned( - tombstone, - Yup.string(), - 'Compute size is a required field.' - ), + computeSize: Yup.string().required('Compute size is a required field.'), filesystem: Yup.object().when(['provisionFS', 'filesystemType'], (provisionFS, filesystemType) => { if (provisionFS) { return FILESYSTEM_TYPES[filesystemType]?.validationSchema || Yup.object() @@ -229,34 +221,26 @@ export function ApplicationComponent(props) { }) } - const allTiersValidationSpec = (tombstone) => { + const allTiersValidationSpec = () => { let allTiers = {} for (var i = 0; i < tiers.length; i++) { var tierName = tiers[i].name - allTiers[tierName] = singleTierValidationSpec(tombstone) + allTiers[tierName] = singleTierValidationSpec() } return Yup.object(allTiers) } - const allTiersDatabaseValidationSpec = (tombstone) => { + const allTiersDatabaseValidationSpec = () => { let allTiers = {} for (var i = 0; i < tiers.length; i++) { var tierName = tiers[i].name allTiers[tierName] = Yup.object({ - instance: requiredIfNotTombstoned( - tombstone, - Yup.string(), - 'Database instance is required.' - ) + instance: Yup.string().required('Database instance is required.') }) } return Yup.object(allTiers) } - const requiredIfNotTombstoned = (tombstone, schema, requiredMessage) => { - return tombstone ? schema : schema.required(requiredMessage) - } - // TODO public service paths cannot match const validationSpecs = Yup.object({ name: Yup.string().required('Name is a required field.'), @@ -268,26 +252,16 @@ export function ApplicationComponent(props) { Yup.object({ public: Yup.boolean().required(), name: Yup.string() - .when('tombstone', (tombstone, schema) => { - return requiredIfNotTombstoned( - tombstone, - schema, - 'Service Name is a required field.' - ) - }) + .required('Service Name is a required field.') .matches( /^[\.\-_a-zA-Z0-9]+$/, 'Name must only contain alphanumeric characters or .-_' ), description: Yup.string(), path: Yup.string() - .when(['public', 'tombstone'], (isPublic, tombstone, schema) => { + .when(['public'], (isPublic, schema) => { if (isPublic) { - return requiredIfNotTombstoned( - tombstone, - schema, - 'Path is required for public services' - ) + return schema.required('Path is required for public services') } return schema }) @@ -304,13 +278,7 @@ export function ApplicationComponent(props) { healthCheckUrl: Yup.string() .required('Health Check URL is a required field') .matches(/^\//, 'Health Check must start with forward slash (/)'), - operatingSystem: Yup.string().when('tombstone', (tombstone, schema) => { - return requiredIfNotTombstoned( - tombstone, - schema, - 'Container OS is a required field.' - ) - }), + operatingSystem: Yup.string().required('Container OS is a required field.'), windowsVersion: Yup.string().when('operatingSystem', { is: (containerOs) => containerOs && containerOs === WINDOWS, then: Yup.string().required('Windows version is a required field'), @@ -322,9 +290,7 @@ export function ApplicationComponent(props) { then: Yup.object({ engine: Yup.string().required('Engine is required'), version: Yup.string().required('Version is required'), - tiers: Yup.object().when('tombstone', (tombstone, schema) => { - return allTiersDatabaseValidationSpec(tombstone) - }), + tiers: allTiersDatabaseValidationSpec(), username: Yup.string() .matches('^[a-zA-Z]+[a-zA-Z0-9_$]*$', 'Username is not valid') .required('Username is required'), @@ -342,10 +308,7 @@ export function ApplicationComponent(props) { otherwise: Yup.object(), }), provisionObjectStorage: Yup.boolean(), - tiers: Yup.object().when(['tombstone'], (tombstone) => { - return allTiersValidationSpec(tombstone) - }), - tombstone: Yup.boolean(), + tiers: allTiersValidationSpec(), }) ).min(1, 'Application must have at least ${min} service(s).'), provisionBilling: Yup.boolean(), @@ -404,7 +367,6 @@ export function ApplicationComponent(props) { )} - {/* {loading !== "idle" &&
Loading...
} */} {!!error && ( {error} @@ -419,8 +381,8 @@ export function ApplicationComponent(props) { initialValues={initialValues} validationSchema={validationSpecs} validateOnChange={true} + validateOnBlur={false} onSubmit={updateConfig} - enableReinitialize={true} > {(formik) => { return ( @@ -438,7 +400,6 @@ export function ApplicationComponent(props) { > { - const { formikErrors, serviceValues, osOptions, dbOptions, serviceName, onFileSelected, isLocked, serviceIndex } = props + const { serviceValues, osOptions, dbOptions, serviceName, onFileSelected, isLocked, serviceIndex } = props const getWinServerOptions = (serviceIndex) => { if (!osOptions) { return null @@ -54,8 +54,6 @@ const ServiceSettingsSubform = (props) => { ) : null } - const operatingSystemFeedback = !!formikErrors.services ? formikErrors.services[serviceIndex]?.operatingSystem : undefined - const getLaunchTypeOptions = (serviceIndex) => { return serviceValues?.operatingSystem === 'LINUX' && ( @@ -86,15 +84,6 @@ const ServiceSettingsSubform = (props) => { Fargate - - {formikErrors.ecsLaunchType} - ) } @@ -115,6 +104,7 @@ const ServiceSettingsSubform = (props) => { label="Service Name" name={"services[" + serviceIndex + "].name"} type="text" + autoFocus /> @@ -169,11 +159,6 @@ const ServiceSettingsSubform = (props) => { Windows - - {operatingSystemFeedback} - {getLaunchTypeOptions(serviceIndex)} {getWinServerOptions(serviceIndex)} @@ -242,7 +227,6 @@ ServiceSettingsSubform.propTypes = { dbOptions: PropTypes.array, onFileSelected: PropTypes.func, serviceValues: PropTypes.object, - formikErrors: PropTypes.object, serviceIndex: PropTypes.number, } diff --git a/client/web/src/settings/ServicesComponent.js b/client/web/src/settings/ServicesComponent.js index 82c31b81..fbf0eeed 100644 --- a/client/web/src/settings/ServicesComponent.js +++ b/client/web/src/settings/ServicesComponent.js @@ -24,7 +24,6 @@ import { PropTypes } from 'prop-types' const ServicesComponent = (props) => { const { formik, - formikErrors, hasTenants, osOptions, dbOptions, @@ -33,8 +32,8 @@ const ServicesComponent = (props) => { initService } = props - const [services, setServices] = useState(formik.values.services) const [showModal, setShowModal] = useState(false) + const [openAccordionItem, setOpenAccordionItem] = useState() const toggleModal = () => { setShowModal((state) => !state) @@ -42,16 +41,18 @@ const ServicesComponent = (props) => { const addService = (serviceName) => { let newService = initService(serviceName) + setOpenAccordionItem(formik.values.services.length) formik.values.services.push(newService) - setServices([...formik.values.services]) - formik.validateForm() } const deleteService = (index) => { - // we can't just remove this service from the list because it'll mess with our indices - formik.values.services[index].tombstone = true - setServices(formik.values.services) - // kick off validation so the schema recognizes the tombstone and clears any pending errors + // remove one entry starting from `index` from services + formik.values.services.splice(index, 1) + // manually remove this entry from errors if necessary, since Formik apparently doesn't do it itself + formik.errors.services?.splice(index, 1) + if (openAccordionItem === index) { + setOpenAccordionItem(null) + } formik.validateForm() } @@ -76,10 +77,9 @@ const ServicesComponent = (props) => { - - {services.map( - (service, index) => - !service.tombstone && ( + setOpenAccordionItem(index)}> + {formik.values.services?.map( + (service, index) => ( {service.name} @@ -87,7 +87,6 @@ const ServicesComponent = (props) => { formik={formik} isLocked={hasTenants} serviceValues={formik.values.services[index]} - formikErrors={formikErrors} osOptions={osOptions} dbOptions={dbOptions} onFileSelected={onFileSelected} diff --git a/client/web/src/settings/TierServiceSettingsSubform.js b/client/web/src/settings/TierServiceSettingsSubform.js index b7b6aa99..ff16ede2 100644 --- a/client/web/src/settings/TierServiceSettingsSubform.js +++ b/client/web/src/settings/TierServiceSettingsSubform.js @@ -32,9 +32,7 @@ const TierServiceSettingsSubform = (props) => { setFieldValue } = props - const [selectedTier, setSelectedTier] = useState( - !!tiers && !!tiers[0] ? tiers[0].name : '' - ) + const [selectedTier, setSelectedTier] = useState(defaultTier) const formikTierPrefix = formikServicePrefix + '.tiers[' + selectedTier + ']'