Fix dates fields can be blank

closes LF-1263 flag=differentiated_modules test plan: - In the assignment tray, check for an error when you fill in the time field but leave the date field blank Change-Id: I3d9010358f4cdca76364f736f36e54306e187341 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/344977 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Robin Kuss <rkuss@instructure.com> QA-Review: Robin Kuss <rkuss@instructure.com> Product-Review: Alvaro Talavera <alvaro.talavera@instructure.com>
AgileVentures · Apr 22, 2024 · cadcb7e · cadcb7e
1 parent fe6dffd
commit cadcb7e
Show file tree

Hide file tree

Showing 9 changed files with 87 additions and 16 deletions.
diff --git a/jest/jest-setup.js b/jest/jest-setup.js
@@ -53,6 +53,7 @@ const ignoredErrors = [
   /The above error occurred in the <.*> component/,
   /You seem to have overlapping act\(\) calls/,
   /Warning: `value` prop on `%s` should not be null. Consider using an empty string to clear the component or `undefined` for uncontrolled components.%s/,
+  /Warning: This synthetic event is reused for performance reasons/,
 ]
 const globalWarn = global.console.warn
 const ignoredWarnings = [/JQMIGRATE:/] // ignore warnings about jquery migrate; these are muted globally when not in a jest test

diff --git a/ui/shared/context-modules/differentiated-modules/react/Item/AvailableFromDateTimeInput.tsx b/ui/shared/context-modules/differentiated-modules/react/Item/AvailableFromDateTimeInput.tsx
@@ -38,6 +38,7 @@ export function AvailableFromDateTimeInput({
   unparsedFieldKeys,
   blueprintDateLocks,
   dateInputRefs,
+  timeInputRefs,
   handleBlur,
   ...otherProps
 }: AvailableFromDateTimeInputProps) {
@@ -48,6 +49,11 @@ export function AvailableFromDateTimeInput({
     // eslint-disable-next-line react-hooks/exhaustive-deps
     []
   )
+  const timeInputRef = useCallback(
+    el => (timeInputRefs[key] = el),
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    []
+  )
   const onBlur = useMemo(() => handleBlur(key), [handleBlur])
   const messages = useMemo(
     () =>
@@ -71,6 +77,7 @@ export function AvailableFromDateTimeInput({
     messages,
     onBlur,
     dateInputRef,
+    timeInputRef,
   }
 
   return <ClearableDateTimeInput {...availableFromDateProps} {...otherProps} />

diff --git a/ui/shared/context-modules/differentiated-modules/react/Item/AvailableToDateTimeInput.tsx b/ui/shared/context-modules/differentiated-modules/react/Item/AvailableToDateTimeInput.tsx
@@ -38,6 +38,7 @@ export function AvailableToDateTimeInput({
   unparsedFieldKeys,
   blueprintDateLocks,
   dateInputRefs,
+  timeInputRefs,
   handleBlur,
   ...otherProps
 }: AvailableToDateTimeInputProps) {
@@ -48,6 +49,11 @@ export function AvailableToDateTimeInput({
     // eslint-disable-next-line react-hooks/exhaustive-deps
     []
   )
+  const timeInputRef = useCallback(
+    el => (timeInputRefs[key] = el),
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    []
+  )
   const onBlur = useMemo(() => handleBlur(key), [handleBlur])
   const messages = useMemo(
     () =>
@@ -67,6 +73,7 @@ export function AvailableToDateTimeInput({
     messages,
     onBlur,
     dateInputRef,
+    timeInputRef,
   }
 
   return <ClearableDateTimeInput {...availableToDateProps} {...otherProps} />

diff --git a/ui/shared/context-modules/differentiated-modules/react/Item/ClearableDateTimeInput.tsx b/ui/shared/context-modules/differentiated-modules/react/Item/ClearableDateTimeInput.tsx
@@ -63,6 +63,7 @@ export interface ClearableDateTimeInputProps {
   locale?: string
   timezone?: string
   dateInputRef?: (el: HTMLInputElement | null) => void
+  timeInputRef?: (el: HTMLInputElement | null) => void
 }
 
 function ClearableDateTimeInput({
@@ -80,6 +81,7 @@ function ClearableDateTimeInput({
   locale,
   timezone,
   dateInputRef,
+  timeInputRef,
 }: ClearableDateTimeInputProps) {
   const [hasErrorBorder, setHasErrorBorder] = useState(false)
   const clearButtonContainer = useRef<HTMLElement | null>()
@@ -134,6 +136,7 @@ function ClearableDateTimeInput({
           onChange={onChange}
           onBlur={onBlur}
           dateInputRef={dateInputRef}
+          timeInputRef={timeInputRef}
           interaction={disabled ? 'disabled' : 'enabled'}
         />
       </Flex.Item>

diff --git a/ui/shared/context-modules/differentiated-modules/react/Item/DueDateTimeInput.tsx b/ui/shared/context-modules/differentiated-modules/react/Item/DueDateTimeInput.tsx
@@ -38,6 +38,7 @@ export function DueDateTimeInput({
   unparsedFieldKeys,
   blueprintDateLocks,
   dateInputRefs,
+  timeInputRefs,
   handleBlur,
   ...otherProps
 }: DueDateTimeInputProps) {
@@ -48,6 +49,11 @@ export function DueDateTimeInput({
     // eslint-disable-next-line react-hooks/exhaustive-deps
     []
   )
+  const timeInputRef = useCallback(
+    el => (timeInputRefs[key] = el),
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    []
+  )
   const onBlur = useMemo(() => handleBlur(key), [handleBlur])
   const messages = useMemo(
     () => generateMessages(dueDate, validationErrors[key] ?? null, unparsedFieldKeys.has(key)),
@@ -66,6 +72,7 @@ export function DueDateTimeInput({
     messages,
     onBlur,
     dateInputRef,
+    timeInputRef,
   }
 
   return <ClearableDateTimeInput {...dueDateProps} {...otherProps} />

diff --git a/ui/shared/context-modules/differentiated-modules/react/Item/ItemAssignToCard.tsx b/ui/shared/context-modules/differentiated-modules/react/Item/ItemAssignToCard.tsx
@@ -130,6 +130,7 @@ export default forwardRef(function ItemAssignToCard(
   const deleteCardButtonRef = useRef<Element | null>(null)
   const assigneeSelectorRef = useRef<HTMLInputElement | null>(null)
   const dateInputRefs = useRef<Record<string, HTMLInputElement | null>>({})
+  const timeInputRefs = useRef<Record<string, HTMLInputElement | null>>({})
   const dateValidator = useRef<DateValidator>(
     new DateValidator({
       date_range: {...ENV.VALID_DATE_RANGE},
@@ -214,16 +215,29 @@ export default forwardRef(function ItemAssignToCard(
   const handleBlur = useCallback(
     (unparsedFieldKey: string) => (e: SyntheticEvent) => {
       const target = e.target as HTMLInputElement
-      if (!target || target !== dateInputRefs.current[unparsedFieldKey]) return
+
+      const dateField = dateInputRefs.current[unparsedFieldKey]
+      const isEmpty = dateField.value.trim() === ''
+      const isValid = moment(dateField.value, 'll').isValid()
       const unparsedFieldExists = unparsedFieldKeys.has(unparsedFieldKey)
-      const isEmpty = target.value.trim() === ''
-      const isValid = moment(target.value, 'll').isValid()
       const newUnparsedFieldKeys = new Set(Array.from(unparsedFieldKeys))
-      if ((isEmpty || isValid) && unparsedFieldExists) {
-        newUnparsedFieldKeys.delete(unparsedFieldKey)
-      } else if (!isEmpty && !isValid && !unparsedFieldExists) {
-        newUnparsedFieldKeys.add(unparsedFieldKey)
+
+      // e.target is not working in the onBlur event from the DateTimeInput component.
+      // so if we get a null target, we asumme it's the time field
+      if (!target && timeInputRefs.current[unparsedFieldKey].value.length > 0) {
+        if (isEmpty && !unparsedFieldExists) {
+          newUnparsedFieldKeys.add(unparsedFieldKey)
+        }
       }
+
+      if (target && target === dateInputRefs.current[unparsedFieldKey]) {
+        if ((isEmpty || isValid) && unparsedFieldExists) {
+          newUnparsedFieldKeys.delete(unparsedFieldKey)
+        } else if (!isEmpty && !isValid && !unparsedFieldExists) {
+          newUnparsedFieldKeys.add(unparsedFieldKey)
+        }
+      }
+
       if (!setEquals(newUnparsedFieldKeys, unparsedFieldKeys))
         setUnparsedFieldKeys(newUnparsedFieldKeys)
     },
@@ -298,41 +312,48 @@ export default forwardRef(function ItemAssignToCard(
             {...{
               dueDate,
               setDueDate,
-              handleDueDateChange,
               validationErrors,
               unparsedFieldKeys,
               blueprintDateLocks,
               dateInputRefs: dateInputRefs.current,
+              timeInputRefs: timeInputRefs.current,
               handleBlur,
             }}
             {...commonDateTimeInputProps}
+            handleDueDateChange={handleDueDateChange(timeInputRefs.current.due_at?.value || '')}
           />
         )}
         <AvailableFromDateTimeInput
           {...{
             availableFromDate,
             setAvailableFromDate,
-            handleAvailableFromDateChange,
             validationErrors,
             unparsedFieldKeys,
             blueprintDateLocks,
             dateInputRefs: dateInputRefs.current,
+            timeInputRefs: timeInputRefs.current,
             handleBlur,
           }}
           {...commonDateTimeInputProps}
+          handleAvailableFromDateChange={handleAvailableFromDateChange(
+            timeInputRefs.current.unlock_at?.value || ''
+          )}
         />
         <AvailableToDateTimeInput
           {...{
             availableToDate,
             setAvailableToDate,
-            handleAvailableToDateChange,
             validationErrors,
             unparsedFieldKeys,
             blueprintDateLocks,
             dateInputRefs: dateInputRefs.current,
+            timeInputRefs: timeInputRefs.current,
             handleBlur,
           }}
           {...commonDateTimeInputProps}
+          handleAvailableToDateChange={handleAvailableToDateChange(
+            timeInputRefs.current.lock_at?.value || ''
+          )}
         />
         <ContextModuleLink
           courseId={courseId}

diff --git a/...red/context-modules/differentiated-modules/react/Item/__tests__/ItemAssignToCard.test.tsx b/...red/context-modules/differentiated-modules/react/Item/__tests__/ItemAssignToCard.test.tsx
@@ -19,6 +19,7 @@
 import React from 'react'
 import {render, fireEvent, screen, waitFor} from '@testing-library/react'
 import ItemAssignToCard, {type ItemAssignToCardProps} from '../ItemAssignToCard'
+import userEvent from '@testing-library/user-event'
 
 const props: ItemAssignToCardProps = {
   courseId: '1',
@@ -219,6 +220,19 @@ describe('ItemAssignToCard', () => {
     expect(link).toHaveAttribute('target', '_blank')
   })
 
+  it('show error when date field is blank and time field has value on blur', async () => {
+    const {getAllByLabelText, getAllByText} = renderComponent()
+    const timeInput = getAllByLabelText('Time')[0]
+
+    await userEvent.type(timeInput, '3:30 PM')
+    await userEvent.tab()
+
+    await waitFor(async () => {
+      expect(timeInput).toHaveValue('3:30 PM')
+      expect(await getAllByText('Invalid date')[0]).toBeInTheDocument()
+    })
+  })
+
   describe('when course and user timezones differ', () => {
     beforeAll(() => {
       window.ENV.TIMEZONE = 'America/Denver'

diff --git a/ui/shared/context-modules/differentiated-modules/react/Item/types.d.ts b/ui/shared/context-modules/differentiated-modules/react/Item/types.d.ts
@@ -99,6 +99,7 @@ export type CustomDateTimeInputProps = {
   unparsedFieldKeys: Set<string>
   blueprintDateLocks?: DateLockTypes[]
   dateInputRefs: Record<string, HTMLInputElement | null>
+  timeInputRefs: Record<string, HTMLInputElement | null>
   handleBlur: (unparsedFieldKey: string) => (e: SyntheticEvent) => void
   breakpoints: Breakpoints
   showMessages?: boolean

diff --git a/ui/shared/context-modules/differentiated-modules/react/Item/utils.ts b/ui/shared/context-modules/differentiated-modules/react/Item/utils.ts
@@ -126,9 +126,13 @@ export function useDates({
   }, [availableToDate])
 
   const handleDueDateChange = useCallback(
-    (_event: React.SyntheticEvent, value: string | undefined) => {
+    (timeValue: String) => (_event: React.SyntheticEvent, value: string | undefined) => {
       const defaultDueTime = ENV.DEFAULT_DUE_TIME ?? '23:59:00'
-      const newDueDate = dueDate ? value : setTimeToStringDate(defaultDueTime, value)
+      const newDueDate = dueDate
+        ? value
+        : timeValue === ''
+        ? setTimeToStringDate(defaultDueTime, value)
+        : value
       // When user uses calendar pop-up type is "click", but for KB is "blur"
       if (_event.type !== 'blur') {
         setDueDate(newDueDate || null)
@@ -140,10 +144,12 @@ export function useDates({
   )
 
   const handleAvailableFromDateChange = useCallback(
-    (_event: React.SyntheticEvent, value: string | undefined) => {
+    (timeValue: String) => (_event: React.SyntheticEvent, value: string | undefined) => {
       const newAvailableFromDate = availableFromDate
         ? value
-        : setTimeToStringDate('00:00:00', value)
+        : timeValue === ''
+        ? setTimeToStringDate('00:00:00', value)
+        : value
       // When user uses calendar pop-up type is "click", but for KB is "blur"
       if (_event.type !== 'blur') {
         setAvailableFromDate(newAvailableFromDate || null)
@@ -155,8 +161,12 @@ export function useDates({
   )
 
   const handleAvailableToDateChange = useCallback(
-    (_event: React.SyntheticEvent, value: string | undefined) => {
-      const newAvailableToDate = availableToDate ? value : setTimeToStringDate('23:59:00', value)
+    (timeValue: String) => (_event: React.SyntheticEvent, value: string | undefined) => {
+      const newAvailableToDate = availableToDate
+        ? value
+        : timeValue === ''
+        ? setTimeToStringDate('23:59:00', value)
+        : value
       // When user uses calendar pop-up type is "click", but for KB is "blur"
       if (_event.type !== 'blur') {
         setAvailableToDate(newAvailableToDate || null)