Refactor hybrid text parsing to account for arbitrarily-nested ANDs and ORs

CHANGELOG.md

@@ -19,6 +19,7 @@
 - Fixed course node dimensions and centering in Generate graph to match main graph
 - Fixed the positions of the Theory of Computing and Artificial Intelligence labels
 - Fixed MAT237 prerequisite parsing by enabling `lParen` and `rParen` in `WebParsing.ReqParser` to handle square brackets
+- Fix hybrid node text parsing not properly accounting for all types of logical prerequisite strings
 
 ### 🔧 Internal changes
 

js/components/graph/Graph.js

@@ -945,17 +945,29 @@ export class Graph extends React.Component {
    */
   updateNode = (nodeId, recursive) => {
     let newState
-    if (this.arePrereqsSatisfiedNode(nodeId)) {
-      if (this.isSelected(nodeId) || this.state.hybridsJSON[nodeId]) {
+    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 = "takeable"
+        newState = "inactive"
       }
     } else {
-      if (this.isSelected(nodeId) && !this.state.hybridsJSON[nodeId]) {
-        newState = "overridden"
+      // 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 {
-        newState = "inactive"
+        if (this.isSelected(nodeId)) {
+          newState = "overridden"
+        } else {
+          newState = "inactive"
+        }
       }
     }
 
@@ -1278,6 +1290,45 @@ export class Graph extends React.Component {
     return parents.every(isAllTrue)
   }
 
+  /**
+   * Checks whether a hybrid node's prereq string is satisfied
+   * @return {boolean}
+   */
+  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
@@ -1761,42 +1812,32 @@ export { ZOOM_INCREMENT, KEYBOARD_PANNING_INCREMENT }
  */
 export function populateHybridRelatives(hybridNode, nodesJSON, parents, childrenObj) {
   // parse prereqs based on text
-  let hybridText = ""
-  hybridNode.text.forEach(textTag => (hybridText += textTag.text))
+  let hybridText = hybridNode.text.map(textTag => textTag.text).join("")
   const nodeParents = []
-  // First search for entire string (see Stats graph)
+  // First search for a node that matches the entire string (see Stats graph)
   let prereqNode = findRelationship(hybridText, nodesJSON)
   if (prereqNode !== undefined) {
     nodeParents.push(prereqNode.id_)
     childrenObj[prereqNode.id_].push(hybridNode.id_)
-  } else {
-    // Parse text first
-    const prereqs = parseAnd(hybridText)[0]
-    prereqs.forEach(course => {
-      if (typeof course === "string") {
+  } 
+  // Otherwise, parse the hybrid node's text as a prerequisite string of multiple courses,
+  // and add a parent-child connection for each involved course
+  else {
+    let prereqs = parseAnd(hybridText)
+    if (typeof prereqs === "object") {
+      prereqs = prereqs.flat(Infinity)
+      prereqs.forEach(course => {
         prereqNode = findRelationship(course, nodesJSON)
         if (prereqNode !== undefined) {
           nodeParents.push(prereqNode.id_)
           childrenObj[prereqNode.id_].push(hybridNode.id_)
         } else {
           console.error("Could not find prereq for ", hybridText)
         }
-      } else if (typeof course === "object") {
-        const orPrereq = []
-        course.forEach(c => {
-          const prereqNode = findRelationship(c, nodesJSON)
-          if (prereqNode !== undefined) {
-            orPrereq.push(prereqNode.id_)
-            childrenObj[prereqNode.id_].push(hybridNode.id_)
-          } else {
-            console.error("Could not find prereq for ", hybridText)
-          }
-        })
-        if (orPrereq.length > 0) {
-          nodeParents.push(orPrereq)
-        }
-      }
-    })
+      })
+    } else {
+      console.error("Could not find prereq for ", hybridText)
+    }
   }
   parents[hybridNode.id_] = nodeParents
 }

js/components/graph/__tests__/populateHybridRelatives.test.js

@@ -147,7 +147,7 @@ describe("populateHybridRelatives", () => {
           id_: "mat135136137157calc1",
           text: [
             {
-              text: "MAT(135,136)/137/157",
+              text: "(MAT135,136)/MAT137/157",
             },
             {
               text: "Calc1",
@@ -175,7 +175,7 @@ describe("populateHybridRelatives", () => {
         csc111: [],
         csc165: [],
         mat135136137157calc1: [],
-        h62: [["csc111", "csc165", "mat135136137157calc1"]],
+        h62: ["csc111", "csc165", "mat135136137157calc1"],
       }
       const expectedChildren = {
         csc111: ["h62"],
@@ -193,7 +193,7 @@ describe("populateHybridRelatives", () => {
         csc111: [],
         csc165: [],
         mat135136137157calc1: [],
-        h62: [["csc111", "csc165"]],
+        h62: ["csc111", "csc165"],
       }
       const expectedChildren = {
         csc111: ["h62"],

js/util/parse.test.js

@@ -1,125 +1,159 @@
 // Tests for parsing algorithms from js/util/util.js
-import { parseAnd, parseOr, parseCourse } from "./util"
+import { parseAnd, parseOr, splitPrereqString, removeOuterParens } from "./util"
 
 describe("parseAnd", () => {
   test("parseAnd correctly parses courses when a comma separates two couress", () => {
     const input = "CSC111, STA247"
     const actual = parseAnd(input)
-    const expected = [["CSC111", "STA247"], ""]
+    const expected = ["CSC111", "STA247"]
+    expect(actual).toEqual(expected)
+  })
+  test("parseAnd correctly accounts for shorthand course code expansion", () => {
+    const input = "CSC110,111"
+    const actual = parseAnd(input)
+    const expected = ["CSC110", "CSC111"]
+    expect(actual).toEqual(expected)
+  })
+  test("parseAnd correctly filters out grade requirements", () => {
+    const input = "CSC110 (70%),111 (77%)"
+    const actual = parseAnd(input)
+    const expected = ["CSC110", "CSC111"]
+    expect(actual).toEqual(expected)
+  })
+  test("parseAnd correctly returns parsed course when a string of a single course is wrapped in parentheses", () => {
+    const input = "(CSC110)"
+    const actual = parseAnd(input)
+    const expected = "CSC110"
+    expect(actual).toEqual(expected)
+  })
+  test("parseAnd correctly returns parsed course when the string only contains one course", () => {
+    const input = "CSC110"
+    const actual = parseAnd(input)
+    const expected = "CSC110"
     expect(actual).toEqual(expected)
   })
   test("parseAnd correctly parses courses separated by both comma and slash", () => {
     const input = "CSC111, MAT135/136/137"
     const actual = parseAnd(input)
-    const expected = [["CSC111", ["MAT135", "MAT136", "MAT137"]], ""]
+    const expected = ["CSC111", ["MAT135", "MAT136", "MAT137"]]
     expect(actual).toEqual(expected)
   })
-
-  test("parseAnd correctly parses courses separated by both ; and slash", () => {
+  test("parseAnd correctly parses courses separated by ; and slash together", () => {
     const input = "CSC111/; MAT135/136/137"
     const actual = parseAnd(input)
-    const expected = [["CSC111", ["MAT135", "MAT136", "MAT137"]], ""]
+    const expected = [["CSC111"], ["MAT135", "MAT136", "MAT137"]]
     expect(actual).toEqual(expected)
   })
   test("parseAnd correctly parses courses separated by multiple commas and slash and ;", () => {
     const input = "CSC111, STA247, Calc1/; MAT135/136, CSC145/CSC165/; CSC108/199"
     const actual = parseAnd(input)
     const expected = [
-      [
         "CSC111",
         "STA247",
-        "CALC1",
+        ["Calc1"],
         ["MAT135", "MAT136"],
         ["CSC145", "CSC165"],
         ["CSC108", "CSC199"],
-      ],
-      "",
     ]
     expect(actual).toEqual(expected)
   })
 })
 
 describe("parseOr", () => {
-  test("parseOr correctly calls returns all parsed courses separated by /", () => {
+  test("parseOr correctly parses courses separated by /", () => {
+    const input = "CSC111/CSC165/MAT149"
+    const actual = parseOr(input)
+    const expected = ["CSC111", "CSC165", "MAT149"]
+    expect(actual).toEqual(expected)
+  })
+  test("parseOr correctly accounts for shorthand course code expansion", () => {
     const input = "CSC111/207/209/258"
     const actual = parseOr(input)
-    const expected = [["CSC111", "CSC207", "CSC209", "CSC258"], ""]
+    const expected = ["CSC111", "CSC207", "CSC209", "CSC258"]
+    expect(actual).toEqual(expected)
+  })
+  test("parseOr correctly filters out grade requirements", () => {
+    const input = "MAT137(73%) / MAT157(67%)"
+    const actual = parseOr(input)
+    const expected = ["MAT137", "MAT157"]
     expect(actual).toEqual(expected)
   })
   test("parseOr correctly returns parsed course when a string of a single course is wrapped in parentheses", () => {
     const input = "(CSC207)"
     const actual = parseOr(input)
-    const expected = ["CSC207)", ""]
+    const expected = "CSC207"
     expect(actual).toEqual(expected)
   })
   test("parseOr correctly returns parsed course when the string only contains one course", () => {
     const input = "CSC207"
     const actual = parseOr(input)
-    const expected = ["CSC207", ""]
+    const expected = "CSC207"
     expect(actual).toEqual(expected)
   })
-  test("parseOr correctly returns parsed course when a comma separates two courses and breaks after parsing the first course", () => {
+  test("parseOr correctly returns a nested conjunction when only a comma split is to be made", () => {
     const input = "CSC207,209"
     const actual = parseOr(input)
-    const expected = ["CSC207", ",209"]
+    const expected = [["CSC207", "CSC209"]]
     expect(actual).toEqual(expected)
   })
   test("parseOr correctly returns parsed course when there is empty space between some of the courses", () => {
-    const input = "csc311/ Calc1/ 301"
+    const input = "CSC311/ 301/ Calc1"
     const actual = parseOr(input)
-    const expected = [["CSC311", "CALC1", "CSC301"], ""]
+    const expected = ["CSC311", "CSC301", "Calc1"]
     expect(actual).toEqual(expected)
   })
-  test("parseOr correctly returns parsed course when the last two courses are separated by , or ;", () => {
-    const input1 = "csc301/317/,Calc1"
-    const actual1 = parseOr(input1)
-    const expected1 = [["CSC301", "CSC317"], ",Calc1"]
-    expect(actual1).toEqual(expected1)
-
-    const input2 = "csc301/317/;Calc1"
-    const actual2 = parseOr(input2)
-    const expected2 = [["CSC301", "CSC317"], ";Calc1"]
-    expect(actual2).toEqual(expected2)
-  })
 })
 
-describe("parseCourse", () => {
-  test("parseCourse correctly returns an array with a string starting with a prefix", () => {
-    const input = "CSC111/207/209/258"
-    const actual = parseCourse(input, "CSC")
-    const expected = ["CSC111", "/207/209/258"]
+describe("removeOuterParens", () => {
+  test("removeOuterParens correctly strips a set of enclosing parentheses around a string", () => {
+    const input = "(CSC111/207/209/258)"
+    const actual = removeOuterParens(input)
+    const expected = "CSC111/207/209/258"
     expect(actual).toEqual(expected)
   })
-  test("parseCourse correctly returns an array with a string starting without a prefix", () => {
-    const input = "207/209/258"
-    const actual = parseCourse(input, "CSC")
-    const expected = ["CSC207", "/209/258"]
+  test("removeOuterParens does not strip extra nested parentheses inside a string", () => {
+    const input = "(CSC111/207/209/258, (MAT149/159), CSC300)"
+    const actual = removeOuterParens(input)
+    const expected = "CSC111/207/209/258, (MAT149/159), CSC300"
     expect(actual).toEqual(expected)
   })
-  test("parseCourse correctly returns an array with a string containing one course number", () => {
-    const input = "207"
-    const actual = parseCourse(input, "CSC")
-    const expected = ["CSC207", ""]
+  test("removeOuterParens does not strip parentheses enclosing only part of a string", () => {
+    const input = "(MAT235, MAT236)/MAT237/MAT257"
+    const actual = removeOuterParens(input)
+    const expected = "(MAT235, MAT236)/MAT237/MAT257"
     expect(actual).toEqual(expected)
   })
-
-  test("parseCourse correctly returns an array with a string containing one course", () => {
-    const input = "CSC209"
-    const actual = parseCourse(input, "CSC")
-    const expected = ["CSC209", ""]
+  test("removeOuterParens does not strip disjoint sets of parentheses enclosing a string", () => {
+    const input = "(MAT235, MAT236)/(MAT237/MAT257)"
+    const actual = removeOuterParens(input)
+    const expected = "(MAT235, MAT236)/(MAT237/MAT257)"
     expect(actual).toEqual(expected)
   })
+})
 
-  test("parseCourse correctly returns an array with a string with comma as separator", () => {
-    const input = "CSC207,209,236"
-    const actual = parseCourse(input, "CSC")
-    const expected = ["CSC207", ",209,236"]
+describe("splitPrereqString", () => {
+  test("splitPrereqString correctly splits courses by a separator", () => {
+    const input = "CSC110,CSC111"
+    const actual = splitPrereqString(input, ",")
+    const expected = ["CSC110", "CSC111"]
+    expect(actual).toEqual(expected)
+  })
+  test("splitPrereqString correctly filters out spaces when performing a split", () => {
+    const input = "CSC110/ CSC111/ CSC207"
+    const actual = splitPrereqString(input, "/")
+    const expected = ["CSC110", "CSC111", "CSC207"]
+    expect(actual).toEqual(expected)
+  })
+  test("splitPrereqString correctly filters out enclosing parentheses when performing a split", () => {
+    const input = "CSC111/207/209/258, (MAT149/159), CSC300"
+    const actual = splitPrereqString(input, ",")
+    const expected = ["CSC111/207/209/258", "MAT149/159", "CSC300"]
     expect(actual).toEqual(expected)
   })
-  test("parseCourse returns an array containing two empty strings when the input s is empty", () => {
-    const input = ""
-    const actual = parseCourse(input, "CSC")
-    const expected = ["", ""]
+  test("splitPrereqString does not perform a split inside a parenthesis layer", () => {
+    const input = "(MAT235, MAT236)/MAT237/MAT257, CSC111"
+    const actual = splitPrereqString(input, ",")
+    const expected = ["(MAT235,MAT236)/MAT237/MAT257", "CSC111"]
     expect(actual).toEqual(expected)
   })
 })