Skip to content

Commit 26fcc02

Browse files
matt-dahlgrenmatt-dahlgren
authored
Refactored prerequisite filtering in dynamic graph generation (#1628)
1 parent 810664b commit 26fcc02

File tree

6 files changed

+202
-23
lines changed

6 files changed

+202
-23
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
- Added a `database-migrate` CLI option that runs SQL migrations
1717
- Updated the course info modal timetable styling and layout
1818
- Updated drag and drop functionality on the graph page to support mobile devices
19+
- Added a recursive function to remove Requirements from the Requirement Tree
1920

2021
### 🐛 Bug fixes
2122

‎app/DynamicGraphs/CourseFinder.hs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,7 @@ lookupCourse options code = do
3333

3434
lookupReqs :: GraphOptions -> Req -> StateT (Map.Map T.Text Req) IO ()
3535
lookupReqs options (J name _) = do
36-
if Set.member name (Set.fromList $ map T.unpack (taken options)) ||
37-
not (Set.member (take 3 name) $ Set.fromList $ map T.unpack (departments options))
38-
-- This course has been taken or is not a department we want to include; we don't need its prerequisites
39-
then return ()
40-
else lookupCourse options $ T.pack name
36+
lookupCourse options $ T.pack name
4137
lookupReqs options (ReqAnd parents) = mapM_ (lookupReqs options) parents
4238
lookupReqs options (ReqOr parents) =
4339
if any hasTaken parents

‎app/DynamicGraphs/GraphGenerator.hs‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ module DynamicGraphs.GraphGenerator
55
, coursesToPrereqGraph
66
, coursesToPrereqGraphExcluding
77
, graphProfileHash
8+
, filterReq
89
)
910
where
1011

@@ -60,14 +61,43 @@ sampleGraph = fst $ State.runState (reqsToGraph
6061
-- ** Main algorithm for converting requirements into a DotGraph
6162

6263
-- | Convert a list of coursenames and requirements to a DotGraph object for
63-
-- drawing using Dot. Also prunes any repeated edges that arise from
64-
-- multiple Reqs using the same Grade requirement
64+
-- drawing using Dot. Also prunes any repeated edges that arise from
65+
-- multiple Reqs using the same Grade requirement
6566
reqsToGraph :: GraphOptions -> [(Text, Req)] -> State GeneratorState (DotGraph Text)
6667
reqsToGraph options reqs = do
67-
allStmts <- concatUnique <$> mapM (reqToStmts options) reqs
68+
allStmts <- concatUnique <$> mapM (reqToStmts options) reqs'
6869
return $ buildGraph allStmts
6970
where
7071
concatUnique = nubOrd . Prelude.concat
72+
filteredReqs = [(n, filterReq options req) | (n, req) <- reqs]
73+
reqs' = Prelude.filter includeName filteredReqs
74+
75+
includeName (n, _) =
76+
pickCourse options n &&
77+
n `notElem`taken options
78+
79+
-- | Recurse through the Req Tree to remove any nodes specified in GraphOptions
80+
filterReq :: GraphOptions -> Req -> Req
81+
filterReq _ None = None
82+
filterReq options (J course info)
83+
| not (pickCourse options (pack course)) = None
84+
| pack course `elem` taken options = None
85+
| otherwise = J course info
86+
filterReq options (ReqAnd reqs) =
87+
case Prelude.filter (/= None) (map (filterReq options) reqs) of
88+
[] -> None
89+
[r] -> r
90+
reqs' -> ReqAnd reqs'
91+
filterReq options (ReqOr reqs) =
92+
case Prelude.filter (/= None) (map (filterReq options) reqs) of
93+
[] -> None
94+
[r] -> r
95+
reqs' -> ReqOr reqs'
96+
filterReq _ (Fces fl modifier) = Fces fl modifier
97+
filterReq options (Grade str req) = Grade str (filterReq options req)
98+
filterReq _ (Gpa fl str) = Gpa fl str
99+
filterReq _ (Program pro) = Program pro
100+
filterReq _ (Raw s) = Raw s
71101

72102
data GeneratorState = GeneratorState Integer (Map.Map Text (DotNode Text))
73103

@@ -107,12 +137,9 @@ nodeColor options name = colors !! depIndex
107137
-- corresponding DotGraph objects.
108138
reqToStmts :: GraphOptions -> (Text, Req) -> State GeneratorState [DotStatement Text]
109139
reqToStmts options (name, req) = do
110-
if pickCourse options name
111-
then do
112-
node <- makeNode name $ Just (nodeColor options name)
113-
stmts <- reqToStmtsTree options (nodeID node) req
114-
return $ DN node:Prelude.concat (toList stmts)
115-
else return []
140+
node <- makeNode name $ Just (nodeColor options name)
141+
stmts <- reqToStmtsTree options (nodeID node) req
142+
return $ DN node:Prelude.concat (toList stmts)
116143

117144
reqToStmtsTree :: GraphOptions -- ^ Options to toggle dynamic graph
118145
-> Text -- ^ Name of parent course
@@ -121,12 +148,9 @@ reqToStmtsTree :: GraphOptions -- ^ Options to toggle dynamic graph
121148
reqToStmtsTree _ _ None = return (Node [] [])
122149
reqToStmtsTree options parentID (J name2 _) = do
123150
let name = pack name2
124-
if pickCourse options name then do
125-
prereq <- makeNode name $ Just (nodeColor options name)
126-
edge <- makeEdge (nodeID prereq) parentID Nothing
127-
return (Node [DN prereq, DE edge] [])
128-
else
129-
return (Node [] [])
151+
prereq <- makeNode name $ Just (nodeColor options name)
152+
edge <- makeEdge (nodeID prereq) parentID Nothing
153+
return (Node [DN prereq, DE edge] [])
130154
-- Two or more required prerequisites.
131155
reqToStmtsTree options parentID (ReqAnd reqs) = do
132156
(andNode, _) <- makeBool "and" reqs

‎backend-test/Controllers/GenerateControllerTests.hs‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import Database.Tables (Courses (..))
2222
import Happstack.Server (rsBody)
2323
import Test.Tasty (TestTree)
2424
import Test.Tasty.HUnit (assertEqual, testCase)
25-
import TestHelpers (mockPutRequest, clearDatabase, runServerPartWith, withDatabase)
25+
import TestHelpers (clearDatabase, mockPutRequest, runServerPartWith, withDatabase)
2626

2727
-- | Helper function to insert courses into the database
2828
insertCoursesWithPrerequisites :: [(T.Text, Maybe T.Text)] -> SqlPersistM ()
@@ -36,13 +36,43 @@ findAndSavePrereqsResponseTestCases :: [(String, [(T.Text, Maybe T.Text)], BSL.B
3636
findAndSavePrereqsResponseTestCases =
3737
[("CSC148H1",
3838
[("CSC108H1", Nothing), ("CSC148H1", Just "CSC108H1")],
39-
"{\"courses\":[\"CSC148H1\"],\"programs\":[],\"graphOptions\":{\"taken\":[],\"departments\":[\"CSC\",\"MAT\",\"STA\"]}}",
39+
"{\"courses\":[\"CSC148H1\"],\"programs\":[],\"taken\":[],\"departments\":[\"CSC\",\"MAT\",\"STA\"]}",
4040
2,
4141
0
4242
),
4343
("CSC368H1",
4444
[("CSC209H1", Nothing), ("CSC258H1", Nothing), ("CSC368H1", Just "CSC209H1, CSC258H1"), ("CSC369H1", Just "CSC209H1, CSC258H1")],
45-
"{\"courses\":[\"CSC368H1\", \"CSC369H1\"],\"programs\":[],\"graphOptions\":{\"taken\":[],\"departments\":[\"CSC\",\"MAT\",\"STA\"]}}",
45+
"{\"courses\":[\"CSC368H1\", \"CSC369H1\"],\"programs\":[],\"taken\":[],\"departments\":[]}",
46+
4,
47+
1
48+
),
49+
("CSC149H1",
50+
[("CSC108H1", Nothing), ("CSC149H1", Just "CSC108H1")],
51+
"{\"courses\":[\"CSC149H1\"],\"programs\":[],\"taken\":[\"CSC108H1\"],\"departments\":[\"CSC\",\"MAT\",\"STA\"]}",
52+
1,
53+
0
54+
),
55+
("CSC150H1",
56+
[("CSC108H1", Nothing), ("MAT135H1", Nothing), ("CSC150H1", Just "CSC108H1, MAT135H1")],
57+
"{\"courses\":[\"CSC150H1\"],\"programs\":[],\"taken\":[\"CSC108H1\"],\"departments\":[\"\"]}",
58+
2,
59+
0
60+
),
61+
("CSC373H1",
62+
[("CSC236H1", Nothing), ("CSC165H1", Nothing), ("MAT237Y1", Nothing), ("CSC373H1", Just "CSC236H1, CSC165H1, MAT237Y1")],
63+
"{\"courses\":[\"CSC373H1\"],\"programs\":[],\"taken\":[\"\"],\"departments\":[\"CSC\"]}",
64+
3,
65+
1
66+
),
67+
("MAT257Y1 with an empty department field",
68+
[("MAT240H1", Nothing), ("MAT157Y1", Nothing), ("MAT247H1", Just "MAT240H1"), ("MAT257Y1", Just "MAT157Y1, MAT247H1")],
69+
"{\"courses\":[\"MAT257Y1\"],\"programs\":[],\"taken\":[\"\"],\"departments\":[\"\"]}",
70+
4,
71+
1
72+
),
73+
("MAT257Y1 with a populated department field",
74+
[("MAT240H1", Nothing), ("MAT157Y1", Nothing), ("MAT247H1", Just "MAT240H1"), ("MAT257Y1", Just "MAT157Y1, MAT247H1")],
75+
"{\"courses\":[\"MAT257Y1\"],\"programs\":[],\"taken\":[\"\"],\"departments\":[\"MAT\"]}",
4676
4,
4777
1
4878
)]

‎backend-test/RequirementTests/FilterReqTests.hs‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
{-|
2+
Description: Filter Req Data module tests.
3+
4+
Module that contains the tests for the filtering of Req within the DynamicGraphs module.
5+
6+
-}
7+
8+
module RequirementTests.FilterReqTests
9+
( test_filterReqs ) where
10+
11+
import Database.Requirement (Req (..))
12+
import DynamicGraphs.GraphGenerator (filterReq)
13+
import DynamicGraphs.GraphOptions (GraphOptions (..))
14+
import Test.Tasty (TestTree, testGroup)
15+
import Test.Tasty.HUnit (assertEqual, testCase)
16+
17+
-- Test GraphOptions
18+
graphOptionsSimple :: GraphOptions
19+
graphOptionsSimple =
20+
GraphOptions { taken = [],
21+
departments = [],
22+
excludedDepth = 0,
23+
maxDepth = -1,
24+
courseNumPrefix = [],
25+
distribution = [],
26+
location = [],
27+
includeRaws = True,
28+
includeGrades = True
29+
}
30+
31+
graphOptionsWithDepartmentsAndTaken :: GraphOptions
32+
graphOptionsWithDepartmentsAndTaken =
33+
GraphOptions { taken = ["CSC108H1"],
34+
departments = ["CSC", "STA"],
35+
excludedDepth = 0,
36+
maxDepth = -1,
37+
courseNumPrefix = [],
38+
distribution = [],
39+
location = [],
40+
includeRaws = True,
41+
includeGrades = True
42+
}
43+
44+
graphOptionsWithTaken :: GraphOptions
45+
graphOptionsWithTaken =
46+
GraphOptions { taken = ["CSC108H1"],
47+
departments = [],
48+
excludedDepth = 0,
49+
maxDepth = -1,
50+
courseNumPrefix = [],
51+
distribution = [],
52+
location = [],
53+
includeRaws = True,
54+
includeGrades = True
55+
}
56+
57+
-- | List of (description, input requirements, input GraphOptions, expected filtered result)
58+
filterReqTestCases :: [(String, Req, GraphOptions, Req)]
59+
filterReqTestCases =
60+
[("Removes already taken courses",
61+
ReqAnd [J "CSC207H1" "", J "CSC148H1" "", J "CSC108H1" ""],
62+
graphOptionsWithDepartmentsAndTaken,
63+
ReqAnd [J "CSC207H1" "", J "CSC148H1" ""]
64+
),
65+
("Does not filer out valid departments",
66+
ReqAnd [J "CSC148H1" "", J "STA257H1" ""],
67+
graphOptionsWithDepartmentsAndTaken,
68+
ReqAnd [J "CSC148H1" "", J "STA257H1" ""]
69+
),
70+
("Filters out courses not in a valid department",
71+
ReqAnd [J "MAT137Y1" "", J "CSC207H1" ""],
72+
graphOptionsWithDepartmentsAndTaken,
73+
J "CSC207H1" ""
74+
),
75+
("Handles ReqAnd",
76+
ReqAnd [J "CSC207H1" "", J "CSC148H1" "", J "CSC108H1" ""],
77+
graphOptionsWithDepartmentsAndTaken,
78+
ReqAnd [J "CSC207H1" "", J "CSC148H1" ""]
79+
),
80+
("Handles None input",
81+
None,
82+
graphOptionsWithDepartmentsAndTaken,
83+
None
84+
),
85+
("Returns None when both in an or should be removed",
86+
ReqOr [J "MAT137Y1" "", J "MAT237Y1" ""],
87+
graphOptionsWithDepartmentsAndTaken,
88+
None
89+
),
90+
("Removes OR when only one req remains",
91+
ReqOr [J "MAT137Y1" "", ReqAnd [J "CSC207H1" "", J "CSC148H1" "", J "CSC108H1" ""]],
92+
graphOptionsWithDepartmentsAndTaken,
93+
ReqAnd [J "CSC207H1" "", J "CSC148H1" ""]
94+
),
95+
("Removes Nothing from dept filter if departments is empty in GraphOptions",
96+
ReqOr [J "MAT137Y1" "", J "MAT237Y1" ""],
97+
graphOptionsWithTaken,
98+
ReqOr [J "MAT137Y1" "", J "MAT237Y1" ""]
99+
),
100+
("Removes nothing from a height of > 1 tree with no departments",
101+
ReqAnd [J "CSC369H1" "", ReqAnd [J "CSC209H1" "", ReqAnd [J "CSC207H1" "", J "CSC148H1" "", J "CSC108H1" ""], J "CSC236H1" ""]],
102+
graphOptionsSimple,
103+
ReqAnd [J "CSC369H1" "", ReqAnd [J "CSC209H1" "", ReqAnd [J "CSC207H1" "", J "CSC148H1" "", J "CSC108H1" ""], J "CSC236H1" ""]]
104+
),
105+
("Removes only CSC108H1 at the bottom of a height > 1 tree and empty department",
106+
ReqAnd [J "CSC369H1" "", ReqAnd [J "CSC209H1" "", ReqAnd [J "CSC207H1" "", J "CSC148H1" "", J "CSC108H1" ""], J "CSC236H1" ""]],
107+
graphOptionsWithTaken,
108+
ReqAnd [J "CSC369H1" "", ReqAnd [J "CSC209H1" "", ReqAnd [J "CSC207H1" "", J "CSC148H1" ""], J "CSC236H1" ""]]
109+
)
110+
]
111+
112+
-- | Helper to run a single test case
113+
runFilterReqTest :: String -> Req -> GraphOptions -> Req -> TestTree
114+
runFilterReqTest description input options expected =
115+
testCase description $
116+
assertEqual ("Failed test: " ++ description)
117+
expected
118+
(filterReq options input)
119+
120+
-- | Generate all tests from the list above
121+
runFilterReqTests :: [TestTree]
122+
runFilterReqTests =
123+
[runFilterReqTest name input options expected | (name, input, options, expected) <- filterReqTestCases]
124+
125+
-- | Test suite for FilterReq
126+
test_filterReqs :: TestTree
127+
test_filterReqs = testGroup "FilterReq Tests" runFilterReqTests

‎courseography.cabal‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ test-suite Tests
121121
RequirementTests.PostParserTests,
122122
RequirementTests.PreProcessingTests,
123123
RequirementTests.ReqParserTests,
124+
RequirementTests.FilterReqTests,
124125
SvgTests.IntersectionTests,
125126
TestHelpers
126127
Type: exitcode-stdio-1.0

0 commit comments

Comments
 (0)