feat(protocol-designer): finish form-level errors for dynamic fields

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,34 +43,35 @@ const mapStateToProps = (state: BaseState, ownProps: OP): SP => {
   const formLevelErrors = stepFormSelectors.getFormLevelErrorsForUnsavedForm(
     state
   )
-  const filteredErrors = getVisibleFormErrors({
+  const visibleErrors = getVisibleFormErrors({
     focusedField,
     dirtyFields,
     errors: formLevelErrors,
   })
 
   // 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,
       })),

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<string>,
   errors: Array<ProfileFormError>,
   profileItemsById: { [itemId: string]: ProfileItem },
 |}): Array<ProfileFormError> => {
   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
+      })
     })
   })
 }

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<InvariantContext> = createSelector(

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

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<ProfileFormError> => {
-  return ALL_PROFILE_ERROR_GETTERS.reduce<Array<ProfileFormError>>(
-    (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<ProfileFormError> = []
+
+  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)
 }