diff --git a/CHANGELOG.md b/CHANGELOG.md index 834403d3e..68f4a400e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ ### ✨ 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/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/__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 b6e21d17e..180b961fd 100644 --- a/js/components/common/react_modal.js.jsx +++ b/js/components/common/react_modal.js.jsx @@ -100,27 +100,36 @@ class CourseModal extends React.Component { currVisitedIndex: 0, }) } else if (prevState.courseId !== this.state.courseId) { - 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), - } + 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"), - } + 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}`, + 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", + }) }) - }) } } diff --git a/js/components/common/utils.js b/js/components/common/utils.js index 3e3f6dd19..b54cd686c 100644 --- a/js/components/common/utils.js +++ b/js/components/common/utils.js @@ -1,16 +1,19 @@ /** * 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()) - .catch(error => { - throw error - }) + return fetch("course?name=" + courseName).then(response => { + if (!response.ok) { + throw new Error(`Failed to fetch course with name ${courseName}`) + } + + 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 9616057e5..84035a6ee 100644 --- a/js/components/grid/course_panel.js.jsx +++ b/js/components/grid/course_panel.js.jsx @@ -150,22 +150,33 @@ function Course(props) { }, [props.selectedLectures]) useEffect(() => { - getCourse(props.courseCode).then(data => { - const course = { - courseCode: "", - F: [], - S: [], - Y: [], - } - course.courseCode = data.name + getCourse(props.courseCode) + .then(data => { + const course = { + courseCode: "", + F: [], + S: [], + Y: [], + } - 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) - }) + 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`) + setCourseInfo({ + courseCode: "", + F: [], + S: [], + Y: [], + }) + }) }, []) return (