Skip to content

Commit

Permalink
Refactor new exam form to handle errors better
Browse files Browse the repository at this point in the history
  • Loading branch information
@nygrenh
nygrenh committed Jan 22, 2025
1 parent 8825ed6 commit 037509a
Show file tree
Hide file tree
Showing 7 changed files with 235 additions and 45 deletions.
2 changes: 2 additions & 0 deletions .eslintrc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ module.exports = {
"JSON.parse",
"addFilter",
"removeFilter",
"setError",
"clearErrors",
],
},
"object-properties": {
Expand Down
203 changes: 165 additions & 38 deletions services/main-frontend/src/components/forms/NewExamForm.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { css } from "@emotion/css"
import React, { useState } from "react"
import { parseISO } from "date-fns"
import React, { useEffect, useState } from "react"
import { useForm } from "react-hook-form"
import { useTranslation } from "react-i18next"

Expand All @@ -22,12 +23,12 @@ interface NewExamFormProps {

interface NewExamFields {
name: string
startsAt: Date
endsAt: Date
startsAt: string
endsAt: string
timeMinutes: number
parentId: string | null
automaticCompletionEnabled: boolean
minimumPointsTreshold: number
minimumPointsThreshold: number
manualGradingEnabled: boolean
}

Expand All @@ -37,68 +38,181 @@ const NewExamForm: React.FC<React.PropsWithChildren<NewExamFormProps>> = ({
exams,
onCreateNewExam,
onDuplicateExam,
onCancel,
}) => {
const { t } = useTranslation()
const [startTimeWarning, setStartTimeWarning] = useState<string | null>(null)

const {
register,
handleSubmit,
formState: { errors },
clearErrors,
setValue,
getValues,
watch,
setError,
} = useForm<NewExamFields>()

const [exam, setExam] = useState(initialData)
const [parentId, setParentId] = useState<string | null>(null)
const [duplicateExam, setDuplicateExam] = useState(false)

const startsAt = watch("startsAt")

useEffect(() => {
try {
if (startsAt) {
const start = parseISO(startsAt)
// Check if it's a valid date
if (!isNaN(start.getTime())) {
const now = new Date()
if (start < now) {
setStartTimeWarning(t("start-time-in-past-warning"))
} else {
setStartTimeWarning(null)
}
}
} else {
setStartTimeWarning(null)
}
} catch (e) {
// Invalid date format, clear warning
setStartTimeWarning(null)
}
}, [startsAt, t])

const validateDates = (data: NewExamFields): boolean => {
const start = parseISO(data.startsAt)
const end = parseISO(data.endsAt)

if (end <= start) {
setError("endsAt", { message: t("end-date-must-be-after-start") })
return false
}

return true
}

const validateForm = (data: NewExamFields): boolean => {
let isValid = true
clearErrors(["startsAt", "endsAt", "timeMinutes", "minimumPointsThreshold"])

// Validate numbers
if (isNaN(Number(data.timeMinutes))) {
setError("timeMinutes", { message: t("invalid-number-format") })
isValid = false
} else if (!Number.isInteger(Number(data.timeMinutes)) || Number(data.timeMinutes) <= 0) {
setError("timeMinutes", { message: t("time-must-be-a-positive-integer") })
isValid = false
}

if (data.automaticCompletionEnabled) {
if (isNaN(Number(data.minimumPointsThreshold))) {
setError("minimumPointsThreshold", { message: t("invalid-number-format") })
isValid = false
} else if (
!Number.isInteger(Number(data.minimumPointsThreshold)) ||
Number(data.minimumPointsThreshold) < 0
) {
setError("minimumPointsThreshold", { message: t("points-must-be-a-non-negative-integer") })
isValid = false
}
}

// Validate dates are parseable
try {
parseISO(data.startsAt).toISOString()
parseISO(data.endsAt).toISOString()
} catch (e) {
setError("startsAt", { message: t("invalid-date-format") })
setError("endsAt", { message: t("invalid-date-format") })
isValid = false
}

// Validate date logic
if (isValid && !validateDates(data)) {
isValid = false
}

return isValid
}

const onCreateNewExamWrapper = handleSubmit((data) => {
if (!validateForm(data)) {
return
}

onCreateNewExam({
organization_id: organizationId,
name: data.name,
starts_at: new Date(data.startsAt).toISOString(),
ends_at: new Date(data.endsAt).toISOString(),
starts_at: parseISO(data.startsAt).toISOString(),
ends_at: parseISO(data.endsAt).toISOString(),
time_minutes: Number(data.timeMinutes),
minimum_points_treshold: data.automaticCompletionEnabled
? Number(data.minimumPointsTreshold)
? Number(data.minimumPointsThreshold)
: 0,
grade_manually: data.manualGradingEnabled,
})
})

const onDuplicateExamWrapper = handleSubmit((data) => {
if (exam) {
const newExam: NewExam = {
organization_id: organizationId,
name: data.name,
starts_at: new Date(data.startsAt).toISOString(),
ends_at: new Date(data.endsAt).toISOString(),
time_minutes: Number(data.timeMinutes),
minimum_points_treshold: data.automaticCompletionEnabled
? Number(data.minimumPointsTreshold)
: 0,
grade_manually: data.manualGradingEnabled,
}
const examId = String(parentId)
onDuplicateExam(examId, newExam)
if (!validateForm(data)) {
return
}

if (duplicateExam && !parentId) {
setError("parentId", { type: "manual", message: t("required-field") })
return
}

if (!exam) {
setError("parentId", { message: t("exam-not-found") })
return
}

const newExam: NewExam = {
organization_id: organizationId,
name: data.name,
starts_at: parseISO(data.startsAt).toISOString(),
ends_at: parseISO(data.endsAt).toISOString(),
time_minutes: Number(data.timeMinutes),
minimum_points_treshold: data.automaticCompletionEnabled
? Number(data.minimumPointsThreshold)
: 0,
grade_manually: data.manualGradingEnabled,
}
const examId = String(parentId)
onDuplicateExam(examId, newExam)
})

const handleSetExamToDuplicate = (examId: string) => {
clearErrors()
setParentId(examId)
const exam = exams.filter((e) => e.id === examId)[0]
setExam(exam)
if (getValues("timeMinutes").toString() === "") {
setValue("timeMinutes", exam.time_minutes)
const selectedExam = exams.find((e) => e.id === examId)
if (!selectedExam) {
setError("parentId", { message: t("exam-not-found") })
return
}

setParentId(examId)
setExam(selectedExam)
}

const automaticEnabled = watch("automaticCompletionEnabled")

const handleDuplicateToggle = () => {
if (!duplicateExam) {
// Switching to duplicate mode
setDuplicateExam(true)
if (exams.length > 0) {
handleSetExamToDuplicate(exams[0].id)
}
} else {
// Switching back to create mode
setDuplicateExam(false)
setParentId(null)
setExam(null)
clearErrors()
}
}

return (
<div>
<form onSubmit={duplicateExam ? onDuplicateExamWrapper : onCreateNewExamWrapper}>
Expand All @@ -115,6 +229,7 @@ const NewExamForm: React.FC<React.PropsWithChildren<NewExamFormProps>> = ({
/>
<DateTimeLocal
error={errors.startsAt?.message}
warning={startTimeWarning ?? undefined}
defaultValue={
initialData?.starts_at ? dateToDateTimeLocalString(initialData?.starts_at) : undefined
}
Expand All @@ -133,7 +248,12 @@ const NewExamForm: React.FC<React.PropsWithChildren<NewExamFormProps>> = ({
id={"timeMinutes"}
error={errors.timeMinutes?.message}
label={t("label-time-minutes")}
{...register("timeMinutes", { required: t("required-field") })}
type="number"
{...register("timeMinutes", {
required: t("required-field"),
min: { value: 1, message: t("time-must-be-positive") },
valueAsNumber: true,
})}
/>
<CheckBox label={t("label-grade-exam-manually")} {...register("manualGradingEnabled")} />
<CheckBox
Expand All @@ -143,25 +263,32 @@ const NewExamForm: React.FC<React.PropsWithChildren<NewExamFormProps>> = ({

{automaticEnabled && (
<TextField
id={"minimumPointsTreshold"}
error={errors.timeMinutes?.message}
id={"minimumPointsThreshold"}
error={errors.minimumPointsThreshold?.message}
label={t("label-exam-minimum-points")}
{...register("minimumPointsTreshold", { required: t("required-field") })}
type="number"
{...register("minimumPointsThreshold", {
required: t("required-field"),
min: { value: 0, message: t("points-must-be-non-negative") },
valueAsNumber: true,
})}
/>
)}

<CheckBox
checked={duplicateExam}
label={t("duplicate")}
onChange={() => setDuplicateExam(!duplicateExam)}
onChange={handleDuplicateToggle}
/>
{duplicateExam && (
{duplicateExam && exams.length > 0 && (
<SelectField
id={"parentId"}
error={errors.parentId?.message}
onChangeByValue={(value) => handleSetExamToDuplicate(value)}
options={exams.map((e) => {
return { label: e.name, value: e.id }
})}
options={exams.map((e) => ({
label: e.name,
value: e.id,
}))}
defaultValue={exams[0].id}
/>
)}
Expand Down
37 changes: 31 additions & 6 deletions shared-module/packages/common/src/components/InputFields/DateTimeLocal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,32 @@ import React, { forwardRef, InputHTMLAttributes } from "react"
import { baseTheme } from "../../styles"
import { dateToString } from "../../utils/time"

const error = css`
const errorStyle = css`
color: #f76d82;
font-size: 14px;
display: inline-block;
margin-top: -15px;
`

const warningStyle = css`
color: #b3440d;
font-size: 14px;
display: inline-block;
margin-top: -15px;
`

export interface TimePickerProps extends InputHTMLAttributes<HTMLInputElement> {
label: string
onChangeByValue?: (value: string, name?: string) => void
error?: string
warning?: string
className?: string
}

const DateTimeLocal = forwardRef<HTMLInputElement, TimePickerProps>(
(props: TimePickerProps, ref) => {
const { onChangeByValue, onChange, className, defaultValue, value, ...rest } = props
const { onChangeByValue, onChange, className, defaultValue, value, warning, error, ...rest } =
props

const handleOnChange = (event: React.ChangeEvent<HTMLInputElement>) => {
if (onChangeByValue) {
Expand Down Expand Up @@ -83,7 +92,17 @@ const DateTimeLocal = forwardRef<HTMLInputElement, TimePickerProps>(
>
<label>
<span>{rest.label}</span>
<input ref={ref} type="datetime-local" step="1" onChange={handleOnChange} {...rest} />
<input
ref={ref}
type="datetime-local"
step="1"
onChange={handleOnChange}
value={value}
defaultValue={defaultValue}
aria-label={rest.label}
aria-invalid={!!error}
{...rest}
/>
</label>

{effectiveValue && typeof effectiveValue === "string" && (
Expand All @@ -97,9 +116,15 @@ const DateTimeLocal = forwardRef<HTMLInputElement, TimePickerProps>(
</small>
)}

{rest.error && (
<span className={cx(error)} id={`${rest.label}_error`} role="alert">
{rest.error}
{error && (
<span className={cx(errorStyle)} id={`${rest.label}_error`} role="alert">
{error}
</span>
)}

{warning && !error && (
<span className={cx(warningStyle)} id={`${rest.label}_warning`} role="alert">
{warning}
</span>
)}
</div>
Expand Down
Loading

0 comments on commit 037509a

Please sign in to comment.