From 60ecb81402c8fda6e8aa50f1212f126d07ae157e Mon Sep 17 00:00:00 2001 From: IanLondon Date: Thu, 4 Jun 2020 13:46:03 -0400 Subject: [PATCH] feat(protocol-designer): finish form-level errors for dynamic fields * Show form-level errors for steps that are inside a cycle (not just top-level steps) * Disable save when form-level errors are present --- .../src/components/StepEditForm/FormAlerts.js | 13 ++-- .../src/components/StepEditForm/utils.js | 31 +++++---- .../src/step-forms/selectors/index.js | 21 +++++- .../src/step-forms/test/selectors.test.js | 7 +- .../src/steplist/formLevel/profileErrors.js | 64 ++++++++++++------- 5 files changed, 90 insertions(+), 46 deletions(-) diff --git a/protocol-designer/src/components/StepEditForm/FormAlerts.js b/protocol-designer/src/components/StepEditForm/FormAlerts.js index a435fb22708..03496810585 100644 --- a/protocol-designer/src/components/StepEditForm/FormAlerts.js +++ b/protocol-designer/src/components/StepEditForm/FormAlerts.js @@ -10,7 +10,7 @@ import { selectors as stepFormSelectors } from '../../step-forms' import { getVisibleFormErrors, getVisibleFormWarnings, - getVisibleProfileErrors, + getVisibleProfileFormLevelErrors, } from './utils' import type { Dispatch } from 'redux' import type { StepIdType } from '../../form-types' @@ -43,7 +43,7 @@ const mapStateToProps = (state: BaseState, ownProps: OP): SP => { const formLevelErrors = stepFormSelectors.getFormLevelErrorsForUnsavedForm( state ) - const filteredErrors = getVisibleFormErrors({ + const visibleErrors = getVisibleFormErrors({ focusedField, dirtyFields, errors: formLevelErrors, @@ -51,26 +51,27 @@ const mapStateToProps = (state: BaseState, ownProps: OP): SP => { // deal with special-case dynamic field form-level errors const { profileItemsById } = stepFormSelectors.getHydratedUnsavedForm(state) - let filteredDynamicFieldFormErrors = [] + let visibleDynamicFieldFormErrors = [] if (profileItemsById != null) { const dynamicFieldFormErrors = stepFormSelectors.getDynamicFieldFormErrorsForUnsavedForm( state ) - filteredDynamicFieldFormErrors = getVisibleProfileErrors({ + visibleDynamicFieldFormErrors = getVisibleProfileFormLevelErrors({ focusedField, dirtyFields, errors: dynamicFieldFormErrors, profileItemsById, }) + console.log({ dynamicFieldFormErrors, visibleDynamicFieldFormErrors }) } return { errors: [ - ...filteredErrors.map(error => ({ + ...visibleErrors.map(error => ({ title: error.title, description: error.body || null, })), - ...filteredDynamicFieldFormErrors.map(error => ({ + ...visibleDynamicFieldFormErrors.map(error => ({ title: error.title, description: error.body || null, })), diff --git a/protocol-designer/src/components/StepEditForm/utils.js b/protocol-designer/src/components/StepEditForm/utils.js index 8fd3c58a46a..979cf327843 100644 --- a/protocol-designer/src/components/StepEditForm/utils.js +++ b/protocol-designer/src/components/StepEditForm/utils.js @@ -3,6 +3,7 @@ import assert from 'assert' import * as React from 'react' import difference from 'lodash/difference' import { i18n } from '../../localization' +import { PROFILE_CYCLE } from '../../form-types' import { SOURCE_WELL_BLOWOUT_DESTINATION, DEST_WELL_BLOWOUT_DESTINATION, @@ -110,27 +111,35 @@ export const getDynamicFieldFocusHandlerId = ({ name: string, |}) => `${id}:${name}` -export const getVisibleProfileErrors = (args: {| +// NOTE: if any fields of a given name are pristine, treat all fields of that name as pristine. +// (Errors don't currently specify the id, so if we later want to only mask form-level errors +// for specific profile fields, the field's parent ProfileItem id needs to be included in the error) +export const getVisibleProfileFormLevelErrors = (args: {| focusedField: ?string, dirtyFields: Array, errors: Array, profileItemsById: { [itemId: string]: ProfileItem }, |}): Array => { const { dirtyFields, focusedField, errors, profileItemsById } = args - const profileIds = Object.keys(profileItemsById) + const profileItemIds = Object.keys(profileItemsById) return errors.filter(error => { - return profileIds.every(itemId => { - const fieldsForItem = error.dependentProfileFields.map( - fieldName => `${itemId}:${fieldName}` // TODO IMMEDIATELY import util for "hashing" the name + id - ) - - const dependentFieldsAreNotFocused = !fieldsForItem.includes(focusedField) + return profileItemIds.every(itemId => { + const item = profileItemsById[itemId] + const steps = item.type === PROFILE_CYCLE ? item.steps : [item] + return steps.every(step => { + const fieldsForStep = error.dependentProfileFields.map(fieldName => + getDynamicFieldFocusHandlerId({ id: step.id, name: fieldName }) + ) - const dependentProfileFieldsAreDirty = - difference(fieldsForItem, dirtyFields).length === 0 + const dependentFieldsAreNotFocused = !fieldsForStep.includes( + focusedField + ) - return dependentFieldsAreNotFocused && dependentProfileFieldsAreDirty + const dependentProfileFieldsAreDirty = + difference(fieldsForStep, dirtyFields).length === 0 + return dependentFieldsAreNotFocused && dependentProfileFieldsAreDirty + }) }) }) } diff --git a/protocol-designer/src/step-forms/selectors/index.js b/protocol-designer/src/step-forms/selectors/index.js index 6238b6a3722..50a818f9608 100644 --- a/protocol-designer/src/step-forms/selectors/index.js +++ b/protocol-designer/src/step-forms/selectors/index.js @@ -432,7 +432,10 @@ const _dynamicFieldFormErrors = ( export const _hasFieldLevelErrors = (hydratedForm: FormData): boolean => { for (const fieldName in hydratedForm) { const value = hydratedForm[fieldName] - if (fieldName === 'profileItemsById') { + if ( + hydratedForm.stepType === 'thermocycler' && + fieldName === 'profileItemsById' + ) { if (getProfileItemsHaveErrors(value)) { return true } @@ -447,10 +450,22 @@ export const _hasFieldLevelErrors = (hydratedForm: FormData): boolean => { return false } +// TODO type with hydrated form type +export const _hasFormLevelErrors = (hydratedForm: FormData): boolean => { + if (_formLevelErrors(hydratedForm).length > 0) return true + + if ( + hydratedForm.stepType === 'thermocycler' && + _dynamicFieldFormErrors(hydratedForm).length > 0 + ) { + return true + } + return false +} + // TODO type with hydrated form type export const _formHasErrors = (hydratedForm: FormData): boolean => { - const hasFormLevelErrors = _formLevelErrors(hydratedForm).length > 0 - return _hasFieldLevelErrors(hydratedForm) || hasFormLevelErrors + return _hasFieldLevelErrors(hydratedForm) || _hasFormLevelErrors(hydratedForm) } export const getInvariantContext: Selector = createSelector( diff --git a/protocol-designer/src/step-forms/test/selectors.test.js b/protocol-designer/src/step-forms/test/selectors.test.js index e29529a7387..a6d1a268669 100644 --- a/protocol-designer/src/step-forms/test/selectors.test.js +++ b/protocol-designer/src/step-forms/test/selectors.test.js @@ -20,8 +20,11 @@ beforeEach(() => { }) describe('_hasFieldLevelErrors', () => { - it('should return true if form has "profileItemsById" field and _getProfileItemsHaveErrors returns true', () => { - const formData = { profileItemsById: { foo: 'abc' } } + it('should return true if form is "thermocycler", has "profileItemsById" field, and _getProfileItemsHaveErrors returns true', () => { + const formData = { + stepType: 'thermocycler', + profileItemsById: { foo: 'abc' }, + } mockGetProfileItemsHaveErrors.mockImplementation(profileItems => { expect(profileItems).toEqual(formData.profileItemsById) return true diff --git a/protocol-designer/src/steplist/formLevel/profileErrors.js b/protocol-designer/src/steplist/formLevel/profileErrors.js index 9e782f645f5..d899beb14a5 100644 --- a/protocol-designer/src/steplist/formLevel/profileErrors.js +++ b/protocol-designer/src/steplist/formLevel/profileErrors.js @@ -1,5 +1,7 @@ // @flow +import uniqBy from 'lodash/uniqBy' import { THERMOCYCLER_PROFILE } from '../../constants' +import { PROFILE_STEP, type ProfileStepItem } from '../../form-types' import type { Node } from 'react' // TODO: real HydratedFormData type @@ -22,37 +24,51 @@ const PROFILE_FORM_ERRORS: { [ProfileFormErrorKey]: ProfileFormError } = { // TC Profile multi-field error fns -export const profileItemsValidDuration = ( - fields: HydratedFormData +export const profileStepValidDuration = ( + step: ProfileStepItem ): ProfileFormError | null => { - const { thermocyclerFormType, orderedProfileItems, profileItemsById } = fields - if (thermocyclerFormType === THERMOCYCLER_PROFILE) { - const someErrors = orderedProfileItems.some(itemId => { - const item = profileItemsById[itemId] - const minutes = parseFloat(item.durationMinutes) || 0 - const seconds = parseFloat(item.durationSeconds) || 0 - const isValid = minutes > 0 || seconds > 0 - return !isValid - }) - if (someErrors) { - return PROFILE_FORM_ERRORS.INVALID_PROFILE_DURATION - } - } - return null + const minutes = parseFloat(step.durationMinutes) || 0 + const seconds = parseFloat(step.durationSeconds) || 0 + const isValid = minutes > 0 || seconds > 0 + return isValid ? null : PROFILE_FORM_ERRORS.INVALID_PROFILE_DURATION } // ===== -const ALL_PROFILE_ERROR_GETTERS = [profileItemsValidDuration] +const PROFILE_STEP_ERROR_GETTERS = [profileStepValidDuration] export const getProfileFormErrors = ( hydratedForm: HydratedFormData ): Array => { - return ALL_PROFILE_ERROR_GETTERS.reduce>( - (acc, errorGetter) => { - const nextErrors = errorGetter(hydratedForm) - return nextErrors === null ? acc : [...acc, nextErrors] - }, - [] - ) + if ( + hydratedForm.stepType !== 'thermocycler' || + hydratedForm.thermocyclerFormType !== THERMOCYCLER_PROFILE + ) { + return [] + } + const { orderedProfileItems, profileItemsById } = hydratedForm + const errors: Array = [] + + const addStepErrors = (step: ProfileStepItem): void => { + PROFILE_STEP_ERROR_GETTERS.forEach(errorGetter => { + const nextErrors = errorGetter(step) + if (nextErrors !== null) { + errors.push(nextErrors) + } + }) + } + + orderedProfileItems.forEach((itemId: string) => { + const item = profileItemsById[itemId] + if (item.type === PROFILE_STEP) { + addStepErrors(item) + } else { + // Cycles themselves don't currently have any form-level errors, + // so we just validate each cycle's steps + item.steps.forEach(addStepErrors) + } + }) + + // NOTE: since errors stacking doesn't seem to serve a purpose, remove repeats + return uniqBy(errors, error => error.title) }