diff --git a/csm_web/frontend/src/components/course/CreateSectionModal.tsx b/csm_web/frontend/src/components/course/CreateSectionModal.tsx index e058f326..6e7632f3 100644 --- a/csm_web/frontend/src/components/course/CreateSectionModal.tsx +++ b/csm_web/frontend/src/components/course/CreateSectionModal.tsx @@ -1,4 +1,4 @@ -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; import { DAYS_OF_WEEK } from "../../utils/datetime"; import { useUserEmails } from "../../utils/queries/base"; @@ -7,6 +7,8 @@ import { Spacetime } from "../../utils/types"; import Modal from "../Modal"; import TimeInput from "../TimeInput"; +import ExclamationCircle from "../../../static/frontend/img/exclamation-circle.svg"; + const makeSpacetime = (): Spacetime => { return { id: -1, duration: 0, dayOfWeek: 1, startTime: "", location: "" }; }; @@ -45,7 +47,21 @@ export const CreateSectionModal = ({ courseId, closeModal, reloadSections }: Cre /** * Capacity for the new section. */ - const [capacity, setCapacity] = useState(""); + const [capacity, setCapacity] = useState(0); + + /** + * Validation text; if empty string, no validation text is displayed. + */ + const [validationText, setValidationText] = useState(""); + + /** + * Automatically re-validate form if there was a previous validation error. + */ + useEffect(() => { + if (validationText !== "") { + validateSectionForm(); + } + }, [mentorEmail, spacetimes, description, capacity]); /** * Create a new empty spacetime for the new section. @@ -55,6 +71,44 @@ export const CreateSectionModal = ({ courseId, closeModal, reloadSections }: Cre setSpacetimes(oldSpacetimes => [...oldSpacetimes, makeSpacetime()]); }; + /** + * Validate current spacetime values. + */ + const validateSectionForm = (): boolean => { + // all fields must be filled out + if (mentorEmail === null || mentorEmail.trim() === "") { + setValidationText("Mentor email must not be blank"); + return false; + } else if (spacetimes.length === 0) { + setValidationText("Must have at least one section time"); + return false; + } else if (isNaN(capacity) || capacity < 0) { + setValidationText("Capacity must be non-negative"); + return false; + } + + // validate spacetime fields + for (const spacetime of spacetimes) { + if (spacetime.location === null || spacetime.location === undefined || spacetime.location.trim() === "") { + setValidationText("All section locations must be specified"); + return false; + } else if (spacetime.dayOfWeek <= 0) { + setValidationText("All section occurrences must have a specified day of week"); + return false; + } else if (spacetime.startTime.trim() === "") { + setValidationText("All section occurrences must have a specified start time"); + return false; + } else if (isNaN(spacetime.duration) || spacetime.duration <= 0) { + setValidationText("All section occurrences must have duration greater than 0"); + return false; + } + } + + // all valid + setValidationText(""); + return true; + }; + /** * Handle the change of a form field. */ @@ -79,7 +133,7 @@ export const CreateSectionModal = ({ courseId, closeModal, reloadSections }: Cre setDescription(value); break; case "capacity": - setCapacity(value); + setCapacity(parseInt(value)); break; default: console.error("Unknown input name: " + name); @@ -93,6 +147,12 @@ export const CreateSectionModal = ({ courseId, closeModal, reloadSections }: Cre */ const handleSubmit = (event: React.MouseEvent): void => { event.preventDefault(); + + if (!validateSectionForm()) { + // don't do anything if invalid + return; + } + const data = { mentorEmail, spacetimes, @@ -105,6 +165,9 @@ export const CreateSectionModal = ({ courseId, closeModal, reloadSections }: Cre onSuccess: () => { closeModal(); reloadSections(); + }, + onError: () => { + setValidationText("Error occurred on create"); } }); }; @@ -141,8 +204,8 @@ export const CreateSectionModal = ({ courseId, closeModal, reloadSections }: Cre type="number" min="0" inputMode="numeric" - pattern="[0-9]*" - value={capacity} + pattern="[0-9]+" + value={isNaN(capacity) ? "" : capacity.toString()} onChange={e => handleChange(-1, "capacity", e.target.value)} /> @@ -180,11 +243,11 @@ export const CreateSectionModal = ({ courseId, closeModal, reloadSections }: Cre className="form-select" onChange={e => handleChange(index, "dayOfWeek", e.target.value)} name={`dayOfWeek|${index}`} - value={dayOfWeek} + value={dayOfWeek.toString()} required > - {[["---", ""], ...Array.from(DAYS_OF_WEEK)].map(([label, value]) => ( - ))} @@ -206,7 +269,7 @@ export const CreateSectionModal = ({ courseId, closeModal, reloadSections }: Cre className="form-input" type="number" name={`duration|${index}`} - value={duration} + value={isNaN(duration) ? "" : duration.toString()} min={0} onChange={e => handleChange(index, "duration", e.target.value)} /> @@ -217,6 +280,12 @@ export const CreateSectionModal = ({ courseId, closeModal, reloadSections }: Cre
+ {validationText !== "" && ( +
+ + {validationText} +
+ )} diff --git a/csm_web/frontend/src/components/section/MetaEditModal.tsx b/csm_web/frontend/src/components/section/MetaEditModal.tsx index df9774a5..90b8dd39 100644 --- a/csm_web/frontend/src/components/section/MetaEditModal.tsx +++ b/csm_web/frontend/src/components/section/MetaEditModal.tsx @@ -1,8 +1,10 @@ -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; import { useSectionUpdateMutation } from "../../utils/queries/sections"; import Modal from "../Modal"; +import ExclamationCircle from "../../../static/frontend/img/exclamation-circle.svg"; + interface MetaEditModalProps { sectionId: number; closeModal: () => void; @@ -18,19 +20,49 @@ export default function MetaEditModal({ }: MetaEditModalProps): React.ReactElement { // use existing capacity and description as initial values const [formState, setFormState] = useState({ capacity: capacity, description: description }); + const [validationText, setValidationText] = useState(""); const sectionUpdateMutation = useSectionUpdateMutation(sectionId); - function handleChange({ target: { name, value } }: React.ChangeEvent) { - setFormState(prevFormState => ({ ...prevFormState, [name]: value })); - } + useEffect(() => { + if (validationText !== "") { + validateForm(); + } + }); + + const handleChange = ({ target: { name, value } }: React.ChangeEvent) => { + if (name === "capacity") { + setFormState(prevFormState => ({ ...prevFormState, [name]: parseInt(value) })); + } else { + setFormState(prevFormState => ({ ...prevFormState, [name]: value })); + } + }; + + const validateForm = () => { + if (isNaN(formState.capacity) || formState.capacity < 0) { + setValidationText("Capacity must be non-negative"); + return false; + } - function handleSubmit(event: React.MouseEvent) { + setValidationText(""); + return true; + }; + + const handleSubmit = (event: React.MouseEvent) => { event.preventDefault(); - //TODO: Handle API Failure - sectionUpdateMutation.mutate(formState); - closeModal(); - } + + if (!validateForm()) { + // don't do anything if invalid + return; + } + + sectionUpdateMutation.mutate(formState, { + onSuccess: closeModal, + onError: () => { + setValidationText("Error occurred on save"); + } + }); + }; return ( @@ -46,7 +78,7 @@ export default function MetaEditModal({ min="0" inputMode="numeric" pattern="[0-9]*" - value={formState.capacity} + value={isNaN(formState.capacity) ? "" : formState.capacity.toString()} onChange={handleChange} autoFocus /> @@ -62,6 +94,12 @@ export default function MetaEditModal({ />
+ {validationText !== "" && ( +
+ + {validationText} +
+ )} diff --git a/csm_web/frontend/src/components/section/SpacetimeEditModal.tsx b/csm_web/frontend/src/components/section/SpacetimeEditModal.tsx index 3db100f1..34e95aa8 100644 --- a/csm_web/frontend/src/components/section/SpacetimeEditModal.tsx +++ b/csm_web/frontend/src/components/section/SpacetimeEditModal.tsx @@ -1,5 +1,5 @@ import { DateTime } from "luxon"; -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; import { DAYS_OF_WEEK } from "../../utils/datetime"; import { useSpacetimeModifyMutation, useSpacetimeOverrideMutation } from "../../utils/queries/spacetime"; @@ -8,6 +8,8 @@ import LoadingSpinner from "../LoadingSpinner"; import Modal from "../Modal"; import TimeInput from "../TimeInput"; +import ExclamationCircle from "../../../static/frontend/img/exclamation-circle.svg"; + import "../../css/spacetime-edit.scss"; interface SpacetimeEditModalProps { @@ -30,26 +32,84 @@ const SpaceTimeEditModal = ({ const [date, setDate] = useState(""); const [mode, setMode] = useState(prevLocation && prevLocation.startsWith("http") ? "virtual" : "inperson"); const [showSaveSpinner, setShowSaveSpinner] = useState(false); + const [validationText, setValidationText] = useState(""); const spacetimeModifyMutation = useSpacetimeModifyMutation(sectionId, spacetimeId); const spacetimeOverrideMutation = useSpacetimeOverrideMutation(sectionId, spacetimeId); + useEffect(() => { + if (validationText !== "") { + validateSpacetime(); + } + }, [location, day, time, date, isPermanent]); + + /** + * Validate current spacetime values. + */ + const validateSpacetime = (): boolean => { + // validate spacetime fields + if (location === null || location === undefined || location.trim() === "") { + setValidationText("All section locations must be specified"); + return false; + } else if (isPermanent && day <= 0) { + // only check this if it's for permanent changes + setValidationText("All section occurrences must have a specified day of week"); + return false; + } else if (time === "") { + setValidationText("All section occurrences must have a specified start time"); + return false; + } + + if (!isPermanent && (date === null || date.trim() === "")) { + setValidationText("Section date to override must be specified"); + return false; + } + + // all valid + setValidationText(""); + return true; + }; + const handleSubmit = (e: React.MouseEvent) => { e.preventDefault(); - //TODO: Handle API failure + + if (!validateSpacetime()) { + // don't do anythinng if invalid + return; + } + setShowSaveSpinner(true); - isPermanent - ? spacetimeModifyMutation.mutate({ + if (isPermanent) { + spacetimeModifyMutation.mutate( + { dayOfWeek: day, location: location, startTime: time - }) - : spacetimeOverrideMutation.mutate({ + }, + { + onSuccess: closeModal, + onError: () => { + setValidationText("Error occurred on save"); + setShowSaveSpinner(false); + } + } + ); + } else { + spacetimeOverrideMutation.mutate( + { location: location, startTime: time, date: date - }); - closeModal(); + }, + { + onSuccess: closeModal, + onError: () => { + setValidationText("Error occurred on save"); + setShowSaveSpinner(false); + } + } + ); + } }; const today = DateTime.now().toISODate()!; @@ -113,7 +173,7 @@ const SpaceTimeEditModal = ({ disabled={!isPermanent} value={isPermanent ? day : "---"} > - {[["---", ""], ...Array.from(DAYS_OF_WEEK)].map(([label, value]) => ( + {[["---", -1], ...Array.from(DAYS_OF_WEEK)].map(([label, value]) => ( @@ -186,6 +246,12 @@ const SpaceTimeEditModal = ({ )}
+ {validationText !== "" && ( +
+ + {validationText} +
+ )} {showSaveSpinner ? ( ) : ( diff --git a/csm_web/frontend/src/css/course.scss b/csm_web/frontend/src/css/course.scss index 1d824b33..f1f72c96 100644 --- a/csm_web/frontend/src/css/course.scss +++ b/csm_web/frontend/src/css/course.scss @@ -120,8 +120,7 @@ grid-template-rows: repeat(auto-fit, 30px); grid-template-columns: repeat(3, 1fr); gap: 1%; - align-items: center; - justify-items: center; + place-items: center center; width: 100%; height: 50%; } @@ -276,3 +275,12 @@ $create-section-modal-height: 65vh; #create-section-form .create-section-submit-container { grid-area: submit; } + +.create-section-validation-text-container { + margin-bottom: 8px; + color: red; +} + +.create-section-validation-text { + margin-left: 4px; +} diff --git a/csm_web/frontend/src/css/spacetime-edit.scss b/csm_web/frontend/src/css/spacetime-edit.scss index 59be989e..22a3b3da 100644 --- a/csm_web/frontend/src/css/spacetime-edit.scss +++ b/csm_web/frontend/src/css/spacetime-edit.scss @@ -9,3 +9,13 @@ align-items: flex-start; justify-content: space-between; } + +.spacetime-edit-form-validation-container { + margin-bottom: 8px; + color: red; +} + +.spacetime-edit-form-validation-text { + margin-left: 4px; + color: red; +} diff --git a/csm_web/frontend/src/utils/queries/sections.tsx b/csm_web/frontend/src/utils/queries/sections.tsx index a4bbcf8c..075af89f 100644 --- a/csm_web/frontend/src/utils/queries/sections.tsx +++ b/csm_web/frontend/src/utils/queries/sections.tsx @@ -442,7 +442,7 @@ export interface SectionCreateMutationBody { mentorEmail: string; spacetimes: Spacetime[]; description: string; - capacity: string; + capacity: number; courseId: number; } diff --git a/csm_web/scheduler/views/section.py b/csm_web/scheduler/views/section.py index 1a063140..47c4ca06 100644 --- a/csm_web/scheduler/views/section.py +++ b/csm_web/scheduler/views/section.py @@ -1,6 +1,7 @@ import datetime import re +from django.core.exceptions import ValidationError as ModelValidationError from django.db import transaction from django.db.models import Prefetch, Q from django.utils import timezone @@ -83,37 +84,83 @@ def create(self, request): # any objects created within the atomic block before the point of failure are rolled back. with transaction.atomic(): mentor_user, _ = User.objects.get_or_create( - email=self.request.data["mentor_email"], - username=self.request.data["mentor_email"].split("@")[0], + email=request.data["mentor_email"], + username=request.data["mentor_email"].split("@")[0], ) - if "spacetimes" not in self.request.data: - return ValidationError("Spacetimes must be provided") + if "spacetimes" not in request.data: + raise ValidationError("Spacetimes must be provided") + + request_spacetimes = request.data["spacetimes"] + request_description = request.data.get("description", "") + request_capacity = request.data.get("capacity", 0) spacetime_objects = [] - for spacetime in self.request.data["spacetimes"]: + for spacetime in request_spacetimes: + spacetime_duration = spacetime.get("duration", None) + if spacetime_duration is None: + raise ValidationError("Spacetime durations must all be specified") + + spacetime_day_of_week = spacetime.get("day_of_week", None) + try: + spacetime_day_of_week = int(spacetime_day_of_week) + except ValueError as err: + raise ValidationError( + "Spacetime day of week must be an integer" + ) from err + if spacetime_day_of_week is None: + raise ValidationError("Spacetime day of week must be specified") + if spacetime_day_of_week < 1 or spacetime_day_of_week > 7: + raise ValidationError( + "Spacetime day of week must be between 1 and 7 inclusive" + ) + + spacetime_location = spacetime.get("location", None) + if spacetime_location is None: + raise ValidationError("Spacetime location must be specified") + spacetime_start_time = spacetime.get("start_time", None) + if spacetime_start_time is None: + raise ValidationError("Spacetime start time must be specified") + # create and validate all spacetimes prior to saving them - converted_duration = datetime.timedelta(minutes=spacetime["duration"]) - converted_day_of_week = weekday_iso_to_string(spacetime["day_of_week"]) + converted_duration = datetime.timedelta(minutes=spacetime_duration) + converted_day_of_week = weekday_iso_to_string(spacetime_day_of_week) new_spacetime = Spacetime( location=spacetime.get("location"), start_time=spacetime.get("start_time"), duration=converted_duration, day_of_week=converted_day_of_week, ) - new_spacetime.full_clean() + + try: + new_spacetime.full_clean() + except ModelValidationError as err: + raise ValidationError(err.error_dict) from err + spacetime_objects.append(new_spacetime) for spacetime in spacetime_objects: + try: + spacetime.full_clean() + except ModelValidationError as err: + raise ValidationError(err.error_dict) from err + spacetime.save() - mentor = Mentor.objects.create(user=mentor_user, course=course) - section = Section.objects.create( - mentor=mentor, - description=self.request.data["description"], - capacity=self.request.data["capacity"], - ) - section.spacetimes.set(spacetime_objects) + try: + mentor = Mentor.objects.create(user=mentor_user, course=course) + section = Section.objects.create( + mentor=mentor, + description=request_description, + capacity=request_capacity, + ) + section.spacetimes.set(spacetime_objects) + + section.full_clean() + except ModelValidationError as err: + # re-raise any validation errors + raise ValidationError(err.error_dict) from err + section.save() serializer = self.serializer_class(section) @@ -356,10 +403,8 @@ class RestrictedAction: if student_queryset.count() > 1: # something bad happened, return immediately with error logger.error( - ( - " Multiple student objects exist in the" - " database (Students %s)!" - ), + " Multiple student objects exist in the" + " database (Students %s)!", student_queryset.all(), ) return Response( @@ -536,10 +581,8 @@ class RestrictedAction: ) student.save() logger.info( - ( - " User %s swapped into Section %s from" - " Section %s" - ), + " User %s swapped into Section %s from" + " Section %s", log_str(student.user), log_str(section), log_str(old_section), @@ -564,26 +607,20 @@ def _student_add(self, request, section): """ if not request.user.can_enroll_in_course(section.mentor.course): logger.warning( - ( - " User %s was unable to enroll in Section %s" - " because they are already involved in this course" - ), + " User %s was unable to enroll in Section %s" + " because they are already involved in this course", log_str(request.user), log_str(section), ) raise PermissionDenied( - ( - "You are already either mentoring for this course or enrolled in a" - " section, or the course is closed for enrollment" - ), + "You are already either mentoring for this course or enrolled in a" + " section, or the course is closed for enrollment", status.HTTP_422_UNPROCESSABLE_ENTITY, ) if section.current_student_count >= section.capacity: logger.warning( - ( - " User %s was unable to enroll in Section %s" - " because it was full" - ), + " User %s was unable to enroll in Section %s" + " because it was full", log_str(request.user), log_str(section), ) @@ -596,18 +633,14 @@ def _student_add(self, request, section): ) if student_queryset.count() > 1: logger.error( - ( - " Multiple student objects exist in the" - " database (Students %s)!" - ), + " Multiple student objects exist in the" + " database (Students %s)!", student_queryset.all(), ) return PermissionDenied( - ( - "An internal error occurred; email mentors@berkeley.edu" - " immediately. (Duplicate students exist in the database (Students" - f" {student_queryset.all()}))" - ), + "An internal error occurred; email mentors@berkeley.edu" + " immediately. (Duplicate students exist in the database (Students" + f" {student_queryset.all()}))", code=status.HTTP_500_INTERNAL_SERVER_ERROR, ) if student_queryset.count() == 1: