From 5fe4f9bbc6b7dc1a11a607994acf88b8f0e650f8 Mon Sep 17 00:00:00 2001 From: Alan Su Date: Fri, 30 Jan 2026 02:30:04 -0500 Subject: [PATCH 1/2] Add not found status code to retrieveCourse and remove unused variable --- CHANGELOG.md | 3 ++ app/Controllers/Course.hs | 6 ++-- .../Controllers/CourseControllerTests.hs | 30 +++++++++++-------- js/components/common/react_modal.js.jsx | 10 +++++++ js/components/common/utils.js | 15 ++++++++-- js/components/graph/Graph.js | 3 -- js/components/grid/course_panel.js.jsx | 7 +++++ 7 files changed, 55 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a652e3fb2..95c94201d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### 🚨 Breaking changes +- Added 404 status code return to `retrieveCourse` in `Controllers/Course` + ### ✨ New features/enhancements ### 🐛 Bug fixes @@ -14,6 +16,7 @@ - Refactor GraphDropdown component from being a child of Graph to being a child of NavBar - Added test cases for the saveGraphJSON function in `Controllers/Graph` +- Remove unused variable from `Graph.js` ## [0.7.2] - 2025-12-10 diff --git a/app/Controllers/Course.hs b/app/Controllers/Course.hs index 5c118bb54..6921be3b1 100644 --- a/app/Controllers/Course.hs +++ b/app/Controllers/Course.hs @@ -7,7 +7,7 @@ import qualified Data.Text as T (Text, unlines) import Database.Persist (Entity) import Database.Persist.Sqlite (SqlPersistM, entityVal, selectList) import Database.Tables as Tables (Courses, coursesCode) -import Happstack.Server (Response, ServerPart, lookText', toResponse) +import Happstack.Server (Response, ServerPart, lookText', notFound, ok, toResponse) import Models.Course (getDeptCourses, returnCourse) import Util.Happstack (createJSONResponse) @@ -17,7 +17,9 @@ retrieveCourse :: ServerPart Response retrieveCourse = do name <- lookText' "name" courses <- liftIO $ returnCourse name - return $ createJSONResponse courses + case courses of + Just x -> ok $ createJSONResponse x + Nothing -> notFound $ toResponse ("Course not found" :: String) -- | Builds a list of all course codes in the database. index :: ServerPart Response diff --git a/backend-test/Controllers/CourseControllerTests.hs b/backend-test/Controllers/CourseControllerTests.hs index 1a49657ad..0aa2610fb 100644 --- a/backend-test/Controllers/CourseControllerTests.hs +++ b/backend-test/Controllers/CourseControllerTests.hs @@ -18,13 +18,13 @@ import Data.Maybe (fromMaybe) import qualified Data.Text as T import Database.Persist.Sqlite (SqlPersistM, insert_) import Database.Tables (Courses (..)) -import Happstack.Server (rsBody) +import Happstack.Server (rsBody, rsCode) import Test.Tasty (TestTree) import Test.Tasty.HUnit (assertEqual, testCase) -import TestHelpers (mockGetRequest, clearDatabase, runServerPart, runServerPartWith, withDatabase) +import TestHelpers (clearDatabase, mockGetRequest, runServerPart, runServerPartWith, withDatabase) --- | List of test cases as (input course name, course data, expected JSON output) -retrieveCourseTestCases :: [(String, T.Text, Map.Map T.Text T.Text, String)] +-- | List of test cases as (input course name, course data, status code, expected JSON output) +retrieveCourseTestCases :: [(String, T.Text, Map.Map T.Text T.Text, Int, String)] retrieveCourseTestCases = [ ("Course exists", "STA238", @@ -40,25 +40,28 @@ retrieveCourseTestCases = ("coreqs", "CSC108H1/ CSC110Y1/ CSC148H1 *Note: the corequisite may be completed either concurrently or in advance."), ("videoUrls", "https://example.com/video1, https://example.com/video2") ], + 200, "{\"allMeetingTimes\":[],\"breadth\":null,\"coreqs\":\"CSC108H1/ CSC110Y1/ CSC148H1 *Note: the corequisite may be completed either concurrently or in advance.\",\"description\":\"An introduction to statistical inference and practice. Statistical models and parameters, estimators of parameters and their statistical properties, methods of estimation, confidence intervals, hypothesis testing, likelihood function, the linear model. Use of statistical computation for data analysis and simulation.\",\"distribution\":null,\"exclusions\":\"ECO220Y1/ ECO227Y1/ GGR270H1/ PSY201H1/ SOC300H1/ SOC202H1/ SOC252H1/ STA220H1/ STA221H1/ STA255H1/ STA248H1/ STA261H1/ STA288H1/ EEB225H1/ STAB22H3/ STAB27H3/ STAB57H3/ STA220H5/ STA221H5/ STA258H5/ STA260H5/ ECO220Y5/ ECO227Y5\",\"name\":\"STA238H1\",\"prereqString\":\"STA237H1/ STA247H1/ STA257H1/ STAB52H3/ STA256H5\",\"title\":\"Probability, Statistics and Data Analysis II\",\"videoUrls\":[\"https://example.com/video1\",\"https://example.com/video2\"]}" ), ("Course does not exist", "STA238", Map.empty, - "null" + 404, + "Course not found" ), ("No course provided", "", Map.empty, - "null" + 404, + "Course not found" ) ] --- | Run a test case (case, input, expected output) on the retrieveCourse function. -runRetrieveCourseTest :: String -> T.Text -> Map.Map T.Text T.Text -> String -> TestTree -runRetrieveCourseTest label courseName courseData expected = +-- | Run a test case (case, input, expected status code, expected output) on the retrieveCourse function. +runRetrieveCourseTest :: String -> T.Text -> Map.Map T.Text T.Text -> Int -> String -> TestTree +runRetrieveCourseTest label courseName courseData expectedCode expectedBody = testCase label $ do let currCourseName = fromMaybe "" $ Map.lookup "name" courseData @@ -86,12 +89,15 @@ runRetrieveCourseTest label courseName courseData expected = insert_ courseToInsert response <- runServerPartWith Controllers.Course.retrieveCourse $ mockGetRequest "/course" [("name", T.unpack courseName)] "" - let actual = BL.unpack $ rsBody response - assertEqual ("Unexpected response body for " ++ label) expected actual + let statusCode = rsCode response + assertEqual ("Unexpected status code for " ++ label) expectedCode statusCode + + let actualBody = BL.unpack $ rsBody response + assertEqual ("Unexpected response body for " ++ label) expectedBody actualBody -- | Run all the retrieveCourse test cases runRetrieveCourseTests :: [TestTree] -runRetrieveCourseTests = map (\(label, courseName, courseData, expected) -> runRetrieveCourseTest label courseName courseData expected) retrieveCourseTestCases +runRetrieveCourseTests = map (\(label, courseName, courseData, expectedCode, expectedBody) -> runRetrieveCourseTest label courseName courseData expectedCode expectedBody) retrieveCourseTestCases -- | Helper function to insert courses into the database insertCourses :: [T.Text] -> SqlPersistM () diff --git a/js/components/common/react_modal.js.jsx b/js/components/common/react_modal.js.jsx index b6e21d17e..750689d40 100644 --- a/js/components/common/react_modal.js.jsx +++ b/js/components/common/react_modal.js.jsx @@ -101,6 +101,16 @@ class CourseModal extends React.Component { }) } else if (prevState.courseId !== this.state.courseId) { getCourse(this.state.courseId).then(course => { + if (!course) { + this.setState({ + course: {}, + sessions: {}, + courseTitle: "Course Not Found", + }) + console.error(`Course with code ${this.state.courseId} not found`) + return + } + const newCourse = { ...course, description: this.convertToLink(course.description), diff --git a/js/components/common/utils.js b/js/components/common/utils.js index 3e3f6dd19..01b641545 100644 --- a/js/components/common/utils.js +++ b/js/components/common/utils.js @@ -1,13 +1,24 @@ /** * Retrieves a course from file. * @param {string} courseName The course code. This + '.txt' is the name of the file. - * @returns {Promise} Promise object representing the JSON object containing course information. + * @returns {Promise} Promise object representing the JSON object containing course information + * or null if not found. */ export function getCourse(courseName) { "use strict" return fetch("course?name=" + courseName) - .then(response => response.json()) + .then(response => { + if (response.status === 404) { + return null + } + + if (!response.ok) { + throw new Error(`Failed to fetch course with name ${courseName}`) + } + + return response.json() + }) .catch(error => { throw error }) diff --git a/js/components/graph/Graph.js b/js/components/graph/Graph.js index b3a76b8ec..b85d56e0f 100644 --- a/js/components/graph/Graph.js +++ b/js/components/graph/Graph.js @@ -22,9 +22,6 @@ const ZOOM_ENUM = { ZOOM_OUT: -1, ZOOM_IN: 1, } -const TIMEOUT_NAMES_ENUM = { - INFOBOX: 0 -} export class Graph extends React.Component { constructor(props) { diff --git a/js/components/grid/course_panel.js.jsx b/js/components/grid/course_panel.js.jsx index 9616057e5..1b3b1f9b6 100644 --- a/js/components/grid/course_panel.js.jsx +++ b/js/components/grid/course_panel.js.jsx @@ -157,6 +157,13 @@ function Course(props) { S: [], Y: [], } + + if (!data) { + setCourseInfo(course) + console.error(`Course with code ${props.courseCode} not found`) + return + } + course.courseCode = data.name const parsedLectures = parseLectures(data.allMeetingTimes) From 27cb7e43137c8024bbb2a11e34fc1aa6b22de150 Mon Sep 17 00:00:00 2001 From: Alan Su Date: Wed, 11 Feb 2026 19:35:28 -0500 Subject: [PATCH 2/2] Refactor and add tests for 404 on front-end --- CHANGELOG.md | 4 +- .../__tests__/CourseModalUpdate.test.js | 64 ++++++++++++++ js/components/common/__tests__/utils.test.js | 36 ++++++++ js/components/common/react_modal.js.jsx | 49 +++++------ js/components/common/utils.js | 20 ++--- .../grid/__tests__/CoursePanel.test.js | 88 +++++++++++++++++++ js/components/grid/course_panel.js.jsx | 44 +++++----- 7 files changed, 244 insertions(+), 61 deletions(-) create mode 100644 js/components/common/__tests__/CourseModalUpdate.test.js create mode 100644 js/components/common/__tests__/utils.test.js create mode 100644 js/components/grid/__tests__/CoursePanel.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f8c8a2bd..68f4a400e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,10 @@ ### 🚨 Breaking changes -- Added 404 status code return to `retrieveCourse` in `Controllers/Course` - ### ✨ New features/enhancements +- Added 404 status code return to `retrieveCourse` in `Controllers/Course` and added front-end tests for affected components + ### 🐛 Bug fixes - Fixed a bug where duplicate graph components were being added diff --git a/js/components/common/__tests__/CourseModalUpdate.test.js b/js/components/common/__tests__/CourseModalUpdate.test.js new file mode 100644 index 000000000..428528619 --- /dev/null +++ b/js/components/common/__tests__/CourseModalUpdate.test.js @@ -0,0 +1,64 @@ +import { render, screen, cleanup } from "@testing-library/react" +import { CourseModal } from "../react_modal.js.jsx" +import fetchMock from "fetch-mock" + +describe("componentDidUpdate", () => { + beforeEach(() => { + cleanup() + fetchMock.restore() + }) + + afterEach(() => { + fetchMock.restore() + }) + + it("fetches course data and updates title when a new course is selected", async () => { + const mockCourseData = { + name: "CSC110Y1", + title: "Foundations of Computer Science I", + description: null, + prereqs: null, + exclusions: null, + breadth: null, + distribution: null, + prereqString: null, + coreqs: null, + allMeetingTimes: [], + videoUrls: [], + } + + fetchMock.get("/course?name=CSC110Y1", { + status: 200, + body: mockCourseData, + }) + + const { rerender } = render( + + ) + + rerender( + + ) + + await screen.findByText("CSC110Y1 Foundations of Computer Science I") + + expect(fetchMock.called("/course?name=CSC110Y1")).toBe(true) + }) + + it("handles the case when course is not found", async () => { + fetchMock.get("/course?name=MISSING110Y1", { + status: 404, + body: {}, + }) + + const { rerender } = render( + + ) + + rerender( + + ) + + await screen.findByText("Course Not Found") + }) +}) diff --git a/js/components/common/__tests__/utils.test.js b/js/components/common/__tests__/utils.test.js new file mode 100644 index 000000000..53abe25e0 --- /dev/null +++ b/js/components/common/__tests__/utils.test.js @@ -0,0 +1,36 @@ +import fetchMock from "fetch-mock" +import { getCourse } from "../utils" + +describe("getCourse (using fetch-mock)", () => { + afterEach(() => { + fetchMock.restore() + }) + + it("successfully returns course data when found", async () => { + const mockCourseData = { + name: "CSC110Y1", + title: "Foundations of Computer Science I", + } + + fetchMock.get("/course?name=CSC110Y1", { + status: 200, + body: mockCourseData, + }) + + const result = await getCourse("CSC110Y1") + + expect(result).toEqual(mockCourseData) + expect(fetchMock.called("/course?name=CSC110Y1")).toBe(true) + }) + + it("throws an error when the course is not found", async () => { + fetchMock.get("/course?name=MISSING110Y1", { + status: 404, + body: {}, + }) + + await expect(getCourse("MISSING110Y1")).rejects.toThrow( + "Failed to fetch course with name MISSING110Y1" + ) + }) +}) diff --git a/js/components/common/react_modal.js.jsx b/js/components/common/react_modal.js.jsx index 750689d40..180b961fd 100644 --- a/js/components/common/react_modal.js.jsx +++ b/js/components/common/react_modal.js.jsx @@ -100,37 +100,36 @@ class CourseModal extends React.Component { currVisitedIndex: 0, }) } else if (prevState.courseId !== this.state.courseId) { - getCourse(this.state.courseId).then(course => { - if (!course) { + getCourse(this.state.courseId) + .then(course => { + const newCourse = { + ...course, + description: this.convertToLink(course.description), + prereqString: this.convertToLink(course.prereqString), + coreqs: this.convertToLink(course.coreq), + exclusions: this.convertToLink(course.exclusions), + } + + const sessions = { + F: this.getTable(course.allMeetingTimes, "F"), + S: this.getTable(course.allMeetingTimes, "S"), + Y: this.getTable(course.allMeetingTimes, "Y"), + } + + this.setState({ + course: newCourse, + sessions: sessions, + courseTitle: `${course.name} ${course.title}`, + }) + }) + .catch(error => { + console.error(`Course with code ${this.state.courseId} not found`) this.setState({ course: {}, sessions: {}, courseTitle: "Course Not Found", }) - console.error(`Course with code ${this.state.courseId} not found`) - return - } - - const newCourse = { - ...course, - description: this.convertToLink(course.description), - prereqString: this.convertToLink(course.prereqString), - coreqs: this.convertToLink(course.coreq), - exclusions: this.convertToLink(course.exclusions), - } - - const sessions = { - F: this.getTable(course.allMeetingTimes, "F"), - S: this.getTable(course.allMeetingTimes, "S"), - Y: this.getTable(course.allMeetingTimes, "Y"), - } - - this.setState({ - course: newCourse, - sessions: sessions, - courseTitle: `${course.name} ${course.title}`, }) - }) } } diff --git a/js/components/common/utils.js b/js/components/common/utils.js index 01b641545..b54cd686c 100644 --- a/js/components/common/utils.js +++ b/js/components/common/utils.js @@ -7,21 +7,13 @@ export function getCourse(courseName) { "use strict" - return fetch("course?name=" + courseName) - .then(response => { - if (response.status === 404) { - return null - } - - if (!response.ok) { - throw new Error(`Failed to fetch course with name ${courseName}`) - } + return fetch("course?name=" + courseName).then(response => { + if (!response.ok) { + throw new Error(`Failed to fetch course with name ${courseName}`) + } - return response.json() - }) - .catch(error => { - throw error - }) + return response.json() + }) } /** diff --git a/js/components/grid/__tests__/CoursePanel.test.js b/js/components/grid/__tests__/CoursePanel.test.js new file mode 100644 index 000000000..b23e5befd --- /dev/null +++ b/js/components/grid/__tests__/CoursePanel.test.js @@ -0,0 +1,88 @@ +import { render, screen, cleanup, waitFor } from "@testing-library/react" +import userEvent from "@testing-library/user-event" +import { CoursePanel } from "../course_panel.js.jsx" +import fetchMock from "fetch-mock" + +describe("CoursePanel", () => { + const coursePanelProps = { + selectedCourses: [], + selectedLectures: [], + removeCourse: jest.fn(), + clearCourses: jest.fn(), + hoverLecture: jest.fn(), + unhoverLecture: jest.fn(), + selectLecture: jest.fn(), + } + + beforeEach(() => { + cleanup() + fetchMock.restore() + }) + + afterEach(() => { + fetchMock.restore() + }) + + it("fetches course sections and updates course info", async () => { + const user = userEvent.setup() + fetchMock.get("/courses", "CSC110Y1\nCSC111H1\nMAT137Y1") + + const mockCourseData = { + name: "CSC110Y1", + title: "Foundations of Computer Science I", + description: null, + prereqs: null, + exclusions: null, + breadth: null, + distribution: null, + prereqString: null, + coreqs: null, + allMeetingTimes: [ + { + meetData: { session: "F", section: "LEC0101", code: "CSC110Y1" }, + timeData: [], + }, + ], + videoUrls: [], + } + + fetchMock.get("/course?name=CSC110Y1", { + status: 200, + body: mockCourseData, + }) + + render() + + const courseHeader = await screen.findByText("CSC110Y1") + await user.click(courseHeader) + + await screen.findByText("L0101") + }) + + it("handles the case when course is not found", async () => { + const user = userEvent.setup() + const consoleErrorMock = jest.spyOn(console, "error").mockImplementation() + + fetchMock.get("/courses", "CSC110Y1\nCSC111H1\nMAT137Y1") + + fetchMock.get("/course?name=MISSING110Y1", { + status: 404, + body: {}, + }) + + render() + + const courseHeader = await screen.findByText("MISSING110Y1") + await user.click(courseHeader) + + expect(screen.queryByText("L0101")).toBeNull() + + await waitFor(() => { + expect(consoleErrorMock).toHaveBeenCalledWith( + expect.stringContaining("Course with code MISSING110Y1 not found") + ) + }) + + consoleErrorMock.mockRestore() + }) +}) diff --git a/js/components/grid/course_panel.js.jsx b/js/components/grid/course_panel.js.jsx index 1b3b1f9b6..84035a6ee 100644 --- a/js/components/grid/course_panel.js.jsx +++ b/js/components/grid/course_panel.js.jsx @@ -150,29 +150,33 @@ function Course(props) { }, [props.selectedLectures]) useEffect(() => { - getCourse(props.courseCode).then(data => { - const course = { - courseCode: "", - F: [], - S: [], - Y: [], - } + getCourse(props.courseCode) + .then(data => { + const course = { + courseCode: "", + F: [], + S: [], + Y: [], + } - if (!data) { + course.courseCode = data.name + + const parsedLectures = parseLectures(data.allMeetingTimes) + // Split the lecture sections into Fall, Spring and Years + course.F = filterLectureList(parsedLectures, "F") + course.S = filterLectureList(parsedLectures, "S") + course.Y = filterLectureList(parsedLectures, "Y") setCourseInfo(course) + }) + .catch(error => { console.error(`Course with code ${props.courseCode} not found`) - return - } - - course.courseCode = data.name - - const parsedLectures = parseLectures(data.allMeetingTimes) - // Split the lecture sections into Fall, Spring and Years - course.F = filterLectureList(parsedLectures, "F") - course.S = filterLectureList(parsedLectures, "S") - course.Y = filterLectureList(parsedLectures, "Y") - setCourseInfo(course) - }) + setCourseInfo({ + courseCode: "", + F: [], + S: [], + Y: [], + }) + }) }, []) return (