diff --git a/CHANGELOG.md b/CHANGELOG.md index ad073a450..c3ce9beb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ ### 🔧 Internal changes +- Extracted read-only selection, prerequisite, and status computations (`isSelected`, `arePrereqsSatisfied*`, `compute*Status`, `totalSelectedCredits`) from `Graph.js` into a typed `graphState.ts` module - Fixed lint-staged pre-commit hook running Prettier and ESLint over the whole repository instead of only staged files, and added `pnpm-lock.yaml` to `.prettierignore` - Extracted the root `` setup (viewBox calculation, shared filters/markers) of `Graph.js` into a typed `GraphCanvas.tsx` component - Refactored `react_modal.js.jsx` into modular functional TypeScript components (`CampusMap.tsx`, `CourseModal.tsx`, `DayBox.tsx`, `Description.tsx`, `FocusModal.tsx`, `MapModal.tsx`, `MapSidebar.tsx`), added shared `types.ts`, and removed unused dead code diff --git a/js/components/graph/Graph.js b/js/components/graph/Graph.js index 7c56b794d..93dabf5bd 100644 --- a/js/components/graph/Graph.js +++ b/js/components/graph/Graph.js @@ -10,9 +10,15 @@ import Button from "./Button" import InfoBox from "./InfoBox" import GraphCanvas, { NODE_DROPSHADOW_FILTER_ID } from "./GraphCanvas" import Sidebar from "./Sidebar" -import { parseAnd } from "./requirementParser" -import { findRelationship, numCredits, transformPoint, formatTransform } from "./util" +import { numCredits, transformPoint, formatTransform } from "./util" import { loadGraphData } from "./loadGraphData" +import { + isSelected, + computeNodeStatus, + computeBoolStatus, + computeEdgeStatus, + totalSelectedCredits, +} from "./graphState" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" import { faMagnifyingGlassMinus, @@ -136,17 +142,8 @@ export class Graph extends React.Component { componentDidUpdate(prevProps, prevState) { if (prevState.nodesJSON !== this.state.nodesJSON) { - let totalFCEs = 0 - Object.values(this.state.nodesJSON).forEach(nodeJSON => { - if ( - !this.state.hybridsJSON[nodeJSON.id_] && - this.state.nodesStatus[nodeJSON.id_].selected - ) { - totalFCEs += numCredits(nodeJSON.id_) - } - }) if (this.props.setFCECount) { - this.props.setFCECount(totalFCEs) + this.props.setFCECount(totalSelectedCredits(this.state)) } } @@ -178,22 +175,8 @@ export class Graph extends React.Component { * Update the status of the Edge, based on the status of the Node/Bool it points from/to. */ updateEdgeStatus = (status, edgeID, source, target) => { - const isSourceSelected = this.isSelected(source) - const isTargetSelected = this.isSelected(target) - const targetStatus = this.state.nodesStatus[target] - ? this.state.nodesStatus[target].status - : this.state.boolsStatus[target] - if (!status) { - if (!isSourceSelected && targetStatus === "missing") { - status = "missing" - } else if (!isSourceSelected) { - status = "inactive" - } else if (!isTargetSelected) { - status = "takeable" - } else { - status = "active" - } + status = computeEdgeStatus(this.state, source, target) } this.setState(state => { const edgesStatus = { ...state.edgesStatus } @@ -691,7 +674,7 @@ export class Graph extends React.Component { * Update the Bool's state at any moment given the prereqs and current state. */ updateNodeBool = boolId => { - const newState = this.arePrereqsSatisfiedBool(boolId) ? "active" : "inactive" + const newState = computeBoolStatus(this.state, boolId) const childs = this.state.connections.children[boolId] const inEdges = this.state.connections.inEdges[boolId] const outEdges = this.state.connections.outEdges[boolId] @@ -728,32 +711,7 @@ export class Graph extends React.Component { * @param {boolean} recursive whether we should recurse on its children */ updateNode = (nodeId, recursive) => { - let newState - if (this.state.hybridsJSON[nodeId]) { - // For a hybrid node, set the state to 'active' or 'inactive' depending on if its text is satisfied - // as a prerequisite string - if (this.arePrereqsSatisfiedHybrid(nodeId)) { - newState = "active" - } else { - newState = "inactive" - } - } else { - // For a regular course node, set the state to 'active', 'takeable', 'inactive', or 'overridden' - // depending on whether the user has selected the course and whether the course's prereqs are met - if (this.arePrereqsSatisfiedNode(nodeId)) { - if (this.isSelected(nodeId)) { - newState = "active" - } else { - newState = "takeable" - } - } else { - if (this.isSelected(nodeId)) { - newState = "overridden" - } else { - newState = "inactive" - } - } - } + const newState = computeNodeStatus(this.state, nodeId) const childs = this.state.connections.children[nodeId] const status = @@ -890,7 +848,7 @@ export class Graph extends React.Component { () => { inEdges?.forEach(edge => { const currentEdge = this.state.edgesJSON[edge] - if (!this.isSelected(currentEdge.source)) { + if (!isSelected(this.state, currentEdge.source)) { this.updateEdgeStatus( "missing", currentEdge.id_, @@ -940,7 +898,7 @@ export class Graph extends React.Component { () => { inEdges?.forEach(edge => { const currentEdge = this.state.edgesJSON[edge] - if (!this.isSelected(currentEdge.source)) { + if (!isSelected(this.state, currentEdge.source)) { this.updateEdgeStatus( "missing", currentEdge.id_, @@ -1002,125 +960,6 @@ export class Graph extends React.Component { }) } - /** - * Checks if a node or bool node is currently selected or active. - * @param {string} nodeId - * @returns {boolean} - */ - isSelected = nodeId => { - if (this.state.nodesStatus[nodeId]) { - return this.isSelectedNode(nodeId) - } else { - return this.isSelectedBool(nodeId) - } - } - - /** - * Checks whether this Node is selected - * @return {boolean} - */ - isSelectedNode = nodeId => { - if (this.state.nodesJSON[nodeId]) { - return this.state.nodesStatus[nodeId].selected - } else { - return this.state.nodesStatus[nodeId].status === "active" - } - } - - /** - * Check whether the Bool is selected. - * @returns {boolean} Whether status is active or not. - */ - isSelectedBool = boolId => { - return this.state.boolsStatus[boolId] === "active" - } - - /** - * Check if the prerequisite courses have been satisfied based on bool type. - * @param {string} boolId the ID of the bool node. - * @returns {boolean} Whether the bool is satisfied based on its prerequisite nodes. - */ - arePrereqsSatisfiedBool = boolId => { - const isAllTrue = element => { - return this.state.nodesStatus[element] - ? this.isSelected(element) - : this.isSelected(element) - } - - if (this.state.boolsJSON[boolId].text[0].text === "and") { - return this.state.connections.parents[boolId].every(isAllTrue) - } else if (this.state.boolsJSON[boolId].text[0].text === "or") { - return this.state.connections.parents[boolId].some(isAllTrue) - } - } - - /** - * Recursively checks whether all prerequisite/preceding nodes for a node are satisfied. - * @param {string} nodeId the ID of the node. - * @return {boolean} whether all of the node's prerequisite nodes are satisfied by the selected courses. - */ - arePrereqsSatisfiedNode = nodeId => { - const parents = this.state.connections.parents[nodeId] - /** - * Recursively checks that preceding nodes are selected - * @param {string|Array} element Node(s)/other on the graph - * @return {boolean} - */ - const isAllTrue = element => { - if (typeof element === "string") { - if (this.state.nodesStatus[element] || this.state.boolsJSON[element]) { - return this.isSelected(element) - } else { - return false - } - } else { - return element.some(isAllTrue) - } - } - - return parents.every(isAllTrue) - } - - /** - * Checks whether a hybrid node's prereq string is satisfied. - * @param {string} nodeId the ID of the hybrid node. - * @return {boolean} whether the node's text prerequisite string is satisfied by the selected courses. - */ - arePrereqsSatisfiedHybrid = nodeId => { - // Concatenate prereq string - let hybridNode = this.state.hybridsJSON[nodeId] - let hybridText = hybridNode.text.map(textTag => textTag.text).join("") - - // Parse prereq string into a nested list alternating between AND and OR conditions - let prereqList = parseAnd(hybridText) - - // Recursively check if each prerequisite condition is satisfied within the selected courses - let nodesList = Object.values(this.state.nodesJSON) - const isSelectedCourse = course => { - let prereqNode = findRelationship(course, nodesList) - if (prereqNode !== undefined) { - return this.isSelectedNode(prereqNode.id_) - } else { - return false - } - } - const andSatisfied = andList => { - if (typeof andList === "string") { - return isSelectedCourse(andList) - } else { - return andList.every(orSatisfied) - } - } - const orSatisfied = orList => { - if (typeof orList === "string") { - return isSelectedCourse(orList) - } else { - return orList.some(andSatisfied) - } - } - return andSatisfied(prereqList) - } - /** * Renders a group of Bools * @param {JSON} boolsJSON diff --git a/js/components/graph/__tests__/graphState.test.js b/js/components/graph/__tests__/graphState.test.js new file mode 100644 index 000000000..b32b4f4ec --- /dev/null +++ b/js/components/graph/__tests__/graphState.test.js @@ -0,0 +1,239 @@ +import { + isSelected, + isSelectedNode, + isSelectedBool, + arePrereqsSatisfiedBool, + arePrereqsSatisfiedNode, + arePrereqsSatisfiedHybrid, + computeNodeStatus, + computeBoolStatus, + computeEdgeStatus, + totalSelectedCredits, +} from "../graphState" + +const node = (id, text, type_ = "Node") => ({ + id_: id, + type_, + pos: [0, 0], + width: 40, + height: 32, + fill: "", + text: [{ pos: [0, 0], text }], +}) + +/** + * A small graph: CSC108 (selected) and CSC148/CSC165 (unselected) course nodes, + * "and"/"or" bools over them, and hybrid nodes with AND, OR, and unknown prereq strings. + */ +function buildState() { + return { + nodesJSON: { + csc108: node("csc108", "CSC108"), + csc148: node("csc148", "CSC148"), + csc165: node("csc165", "CSC165"), + csc236: node("csc236", "CSC236"), + }, + nodesStatus: { + csc108: { status: "active", selected: true }, + csc148: { status: "takeable", selected: false }, + csc165: { status: "takeable", selected: false }, + csc236: { status: "inactive", selected: false }, + hAnd: { status: "inactive", selected: false }, + hOr: { status: "active", selected: false }, + hMissing: { status: "inactive", selected: false }, + }, + hybridsJSON: { + hAnd: node("hAnd", "CSC108, CSC148", "Hybrid"), + hOr: node("hOr", "CSC108/CSC165", "Hybrid"), + hMissing: node("hMissing", "MAT999", "Hybrid"), + }, + boolsJSON: { + andBool: node("andBool", "and", "BoolNode"), + orBool: node("orBool", "or", "BoolNode"), + otherBool: node("otherBool", "xor", "BoolNode"), + }, + boolsStatus: { + andBool: "inactive", + orBool: "active", + otherBool: "inactive", + }, + connections: { + parents: { + csc148: ["csc108"], + csc165: ["csc108", "unknown"], + csc236: ["orBool"], + andBool: ["csc108", "csc148"], + orBool: ["csc108", "csc165"], + otherBool: ["csc108"], + }, + inEdges: {}, + children: {}, + outEdges: {}, + }, + } +} + +describe("isSelected", () => { + it("Should use the selected flag for course nodes", () => { + const state = buildState() + expect(isSelected(state, "csc108")).toBe(true) + expect(isSelected(state, "csc148")).toBe(false) + }) + + it("Should use the bool status for bools", () => { + const state = buildState() + expect(isSelected(state, "orBool")).toBe(true) + expect(isSelected(state, "andBool")).toBe(false) + }) +}) + +describe("isSelectedNode", () => { + it("Should treat active hybrid nodes as selected", () => { + const state = buildState() + expect(isSelectedNode(state, "hOr")).toBe(true) + expect(isSelectedNode(state, "hAnd")).toBe(false) + }) +}) + +describe("isSelectedBool", () => { + it("Should be true only for active bools", () => { + const state = buildState() + expect(isSelectedBool(state, "orBool")).toBe(true) + expect(isSelectedBool(state, "andBool")).toBe(false) + }) +}) + +describe("arePrereqsSatisfiedBool", () => { + it("Should require every parent to be selected for an 'and' bool", () => { + const state = buildState() + expect(arePrereqsSatisfiedBool(state, "andBool")).toBe(false) + state.nodesStatus.csc148.selected = true + expect(arePrereqsSatisfiedBool(state, "andBool")).toBe(true) + }) + + it("Should require some parent to be selected for an 'or' bool", () => { + const state = buildState() + expect(arePrereqsSatisfiedBool(state, "orBool")).toBe(true) + state.nodesStatus.csc108.selected = false + expect(arePrereqsSatisfiedBool(state, "orBool")).toBe(false) + }) + + it("Should be false for an unrecognized logical type", () => { + expect(arePrereqsSatisfiedBool(buildState(), "otherBool")).toBe(false) + }) +}) + +describe("arePrereqsSatisfiedNode", () => { + it("Should be true when all parent nodes are selected", () => { + expect(arePrereqsSatisfiedNode(buildState(), "csc148")).toBe(true) + }) + + it("Should accept selected bool parents", () => { + expect(arePrereqsSatisfiedNode(buildState(), "csc236")).toBe(true) + }) + + it("Should be false when a parent is not in the graph", () => { + expect(arePrereqsSatisfiedNode(buildState(), "csc165")).toBe(false) + }) +}) + +describe("arePrereqsSatisfiedHybrid", () => { + it("Should require all courses in an AND prereq string", () => { + const state = buildState() + expect(arePrereqsSatisfiedHybrid(state, "hAnd")).toBe(false) + state.nodesStatus.csc148.selected = true + expect(arePrereqsSatisfiedHybrid(state, "hAnd")).toBe(true) + }) + + it("Should require one course in an OR prereq string", () => { + const state = buildState() + expect(arePrereqsSatisfiedHybrid(state, "hOr")).toBe(true) + state.nodesStatus.csc108.selected = false + expect(arePrereqsSatisfiedHybrid(state, "hOr")).toBe(false) + }) + + it("Should be false when a prereq course is not in the graph", () => { + expect(arePrereqsSatisfiedHybrid(buildState(), "hMissing")).toBe(false) + }) +}) + +describe("computeNodeStatus", () => { + it("Should be 'active' for a selected course with satisfied prereqs", () => { + const state = buildState() + state.nodesStatus.csc148.selected = true + expect(computeNodeStatus(state, "csc148")).toBe("active") + }) + + it("Should be 'takeable' for an unselected course with satisfied prereqs", () => { + expect(computeNodeStatus(buildState(), "csc148")).toBe("takeable") + }) + + it("Should be 'overridden' for a selected course with unsatisfied prereqs", () => { + const state = buildState() + state.nodesStatus.csc165.selected = true + expect(computeNodeStatus(state, "csc165")).toBe("overridden") + }) + + it("Should be 'inactive' for an unselected course with unsatisfied prereqs", () => { + expect(computeNodeStatus(buildState(), "csc165")).toBe("inactive") + }) + + it("Should base a hybrid node's status on its prereq string", () => { + const state = buildState() + expect(computeNodeStatus(state, "hOr")).toBe("active") + expect(computeNodeStatus(state, "hAnd")).toBe("inactive") + }) +}) + +describe("computeBoolStatus", () => { + it("Should be 'active' exactly when the bool's prereqs are satisfied", () => { + const state = buildState() + expect(computeBoolStatus(state, "orBool")).toBe("active") + expect(computeBoolStatus(state, "andBool")).toBe("inactive") + }) +}) + +describe("computeEdgeStatus", () => { + it("Should be 'active' when both endpoints are selected", () => { + const state = buildState() + state.nodesStatus.csc148.selected = true + expect(computeEdgeStatus(state, "csc108", "csc148")).toBe("active") + }) + + it("Should be 'takeable' when only the source is selected", () => { + expect(computeEdgeStatus(buildState(), "csc108", "csc148")).toBe("takeable") + }) + + it("Should be 'inactive' when the source is not selected", () => { + expect(computeEdgeStatus(buildState(), "csc148", "csc236")).toBe("inactive") + }) + + it("Should be 'missing' when the source is not selected and the target is missing", () => { + const state = buildState() + state.nodesStatus.csc236.status = "missing" + expect(computeEdgeStatus(state, "csc148", "csc236")).toBe("missing") + }) + + it("Should use the bool status when the target is a bool", () => { + const state = buildState() + state.boolsStatus.andBool = "missing" + expect(computeEdgeStatus(state, "csc148", "andBool")).toBe("missing") + }) +}) + +describe("totalSelectedCredits", () => { + it("Should sum the credits of selected courses, excluding hybrids", () => { + const state = buildState() + state.nodesJSON.mat235y1 = node("mat235y1", "MAT235") + state.nodesStatus.mat235y1 = { status: "active", selected: true } + state.nodesJSON.hOr = state.hybridsJSON.hOr + state.nodesStatus.hOr.selected = true + expect(totalSelectedCredits(state)).toBe(1.5) + }) + + it("Should be 0 when nothing is selected", () => { + const state = buildState() + state.nodesStatus.csc108.selected = false + expect(totalSelectedCredits(state)).toBe(0) + }) +}) diff --git a/js/components/graph/graphState.ts b/js/components/graph/graphState.ts new file mode 100644 index 000000000..43c6bf4da --- /dev/null +++ b/js/components/graph/graphState.ts @@ -0,0 +1,205 @@ +import { parseAnd, PrereqTree } from "./requirementParser" +import { findRelationship, numCredits } from "./util" +import { LoadedGraphData } from "./loadGraphData" + +/** The parts of Graph's state that describe the graph's nodes, bools, and their statuses. */ +export type GraphState = Pick< + LoadedGraphData, + | "nodesJSON" + | "nodesStatus" + | "hybridsJSON" + | "boolsJSON" + | "boolsStatus" + | "connections" +> + +/** + * Checks if a node or bool node is currently selected or active. + * @param state + * @param nodeId + * @return whether the node or bool is selected + */ +export function isSelected(state: GraphState, nodeId: string): boolean { + if (state.nodesStatus[nodeId]) { + return isSelectedNode(state, nodeId) + } else { + return isSelectedBool(state, nodeId) + } +} + +/** + * Checks whether a node is selected. Course nodes are selected by the user, while + * hybrid nodes count as selected when they are active. + * @param state + * @param nodeId + * @return whether the node is selected + */ +export function isSelectedNode(state: GraphState, nodeId: string): boolean { + if (state.nodesJSON[nodeId]) { + return state.nodesStatus[nodeId].selected + } else { + return state.nodesStatus[nodeId].status === "active" + } +} + +/** + * Checks whether a bool is selected. + * @param state + * @param boolId + * @return whether the bool's status is active + */ +export function isSelectedBool(state: GraphState, boolId: string): boolean { + return state.boolsStatus[boolId] === "active" +} + +/** + * Checks if the prerequisite courses have been satisfied based on bool type. + * @param state + * @param boolId the ID of the bool node. + * @return whether the bool is satisfied based on its prerequisite nodes. + */ +export function arePrereqsSatisfiedBool(state: GraphState, boolId: string): boolean { + const parents = state.connections.parents[boolId] + const logicalType = state.boolsJSON[boolId].text[0].text + if (logicalType === "and") { + return parents.every(parent => isSelected(state, parent)) + } else if (logicalType === "or") { + return parents.some(parent => isSelected(state, parent)) + } + return false +} + +/** + * Checks whether all prerequisite/preceding nodes for a node are satisfied. + * @param state + * @param nodeId the ID of the node. + * @return whether all of the node's prerequisite nodes are satisfied by the selected courses. + */ +export function arePrereqsSatisfiedNode(state: GraphState, nodeId: string): boolean { + return state.connections.parents[nodeId].every(parent => { + if (state.nodesStatus[parent] || state.boolsJSON[parent]) { + return isSelected(state, parent) + } else { + return false + } + }) +} + +/** + * Checks whether a hybrid node's prereq string is satisfied. + * @param state + * @param nodeId the ID of the hybrid node. + * @return whether the node's text prerequisite string is satisfied by the selected courses. + */ +export function arePrereqsSatisfiedHybrid(state: GraphState, nodeId: string): boolean { + // Concatenate prereq string + const hybridNode = state.hybridsJSON[nodeId] + const hybridText = hybridNode.text.map(textTag => textTag.text).join("") + + // Parse prereq string into a nested list alternating between AND and OR conditions + const prereqList = parseAnd(hybridText) + + // Recursively check if each prerequisite condition is satisfied within the selected courses + const nodesList = Object.values(state.nodesJSON) + const isSelectedCourse = (course: string): boolean => { + const prereqNode = findRelationship(course, nodesList) + if (prereqNode !== undefined) { + return isSelectedNode(state, prereqNode.id_) + } else { + return false + } + } + const andSatisfied = (andList: PrereqTree): boolean => { + if (typeof andList === "string") { + return isSelectedCourse(andList) + } else { + return andList.every(orSatisfied) + } + } + const orSatisfied = (orList: PrereqTree): boolean => { + if (typeof orList === "string") { + return isSelectedCourse(orList) + } else { + return orList.some(andSatisfied) + } + } + return andSatisfied(prereqList) +} + +/** + * Computes the status of a course or hybrid node from its prerequisites and selection. + * Hybrid nodes are 'active' or 'inactive' depending on whether their prereq string is + * satisfied. Course nodes are 'active', 'takeable', 'overridden', or 'inactive' depending + * on whether the user has selected the course and whether its prereqs are met. + * @param state + * @param nodeId + * @return the node's new status + */ +export function computeNodeStatus( + state: GraphState, + nodeId: string +): "active" | "takeable" | "overridden" | "inactive" { + if (state.hybridsJSON[nodeId]) { + return arePrereqsSatisfiedHybrid(state, nodeId) ? "active" : "inactive" + } + + const selected = isSelected(state, nodeId) + if (arePrereqsSatisfiedNode(state, nodeId)) { + return selected ? "active" : "takeable" + } else { + return selected ? "overridden" : "inactive" + } +} + +/** + * Computes the status of a bool from whether its prerequisites are satisfied. + * @param state + * @param boolId + * @return the bool's new status + */ +export function computeBoolStatus( + state: GraphState, + boolId: string +): "active" | "inactive" { + return arePrereqsSatisfiedBool(state, boolId) ? "active" : "inactive" +} + +/** + * Computes the status of an edge from the statuses of the node/bool it points from/to. + * @param state + * @param source the ID of the edge's source node or bool + * @param target the ID of the edge's target node or bool + * @return the edge's new status + */ +export function computeEdgeStatus( + state: GraphState, + source: string, + target: string +): "missing" | "inactive" | "takeable" | "active" { + const targetStatus = state.nodesStatus[target] + ? state.nodesStatus[target].status + : state.boolsStatus[target] + + if (!isSelected(state, source)) { + return targetStatus === "missing" ? "missing" : "inactive" + } else if (!isSelected(state, target)) { + return "takeable" + } else { + return "active" + } +} + +/** + * Computes the total number of credits of the selected course nodes (excluding hybrids). + * @param state + * @return the total credits of the selected courses + */ +export function totalSelectedCredits(state: GraphState): number { + let total = 0 + Object.values(state.nodesJSON).forEach(nodeJSON => { + if (!state.hybridsJSON[nodeJSON.id_] && state.nodesStatus[nodeJSON.id_].selected) { + total += numCredits(nodeJSON.id_) + } + }) + return total +}