From 3a246cb239a66b0dc1110e79c52fb9c8ff3adeea Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Thu, 5 Sep 2024 21:55:49 +1000 Subject: [PATCH 01/17] fix: change eslint rules so frontend can compile with unused variables --- frontend/.eslintrc | 5 +++-- frontend/package.json | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/.eslintrc b/frontend/.eslintrc index 18f151e2c..fb31bf0dd 100644 --- a/frontend/.eslintrc +++ b/frontend/.eslintrc @@ -35,9 +35,10 @@ "jsx-a11y/click-events-have-key-events": ["off"], "jsx-a11y/interactive-supports-focus": ["off"], "no-plusplus": ["error", { "allowForLoopAfterthoughts": true }], - "no-unused-vars": ["error", { "argsIgnorePattern": "^_$" }], + "@typescript-eslint/no-unused-vars": ["warn", { "argsIgnorePattern": "^_$" }], + "no-unused-vars": ["warn", { "argsIgnorePattern": "^_$" }], "react/function-component-definition": ["error", { "namedComponents": "arrow-function", "unnamedComponents": "arrow-function" }], - "simple-import-sort/imports": ["error", { + "simple-import-sort/imports": ["warn", { "groups": [ [ // react based packages diff --git a/frontend/package.json b/frontend/package.json index 9635575ca..d0c3953f8 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -47,7 +47,7 @@ "preview:local": "VITE_BACKEND_API_BASE_URL=https://circlesapi.devsoc.app vite preview --port 3000 --host", "test": "vitest run", "coverage": "vitest run --coverage", - "lint": "eslint src/**/*.{ts,tsx}", + "lint": "eslint src/**/*.{ts,tsx} --max-warnings 0", "lint:fix": "eslint --fix src/**/*.{ts,tsx}", "format": "prettier --write 'src/**/*.{ts,tsx}'", "types": "tsc --project tsconfig.json" From 0a48c35988cec822c3827d9b0b74ad838e1f862c Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Thu, 5 Sep 2024 21:57:48 +1000 Subject: [PATCH 02/17] wip: created mirrored prepareUserPayload backend helper and converted --- backend/server/routers/courses.py | 7 +-- backend/server/routers/model.py | 2 +- backend/server/routers/utility/user.py | 47 ++++++++++++++++++- .../CourseDescriptionPanel.tsx | 8 ++-- frontend/src/utils/api/coursesApi.ts | 12 ++--- 5 files changed, 61 insertions(+), 15 deletions(-) diff --git a/backend/server/routers/courses.py b/backend/server/routers/courses.py index 8419d8b63..861bea9f2 100644 --- a/backend/server/routers/courses.py +++ b/backend/server/routers/courses.py @@ -13,7 +13,7 @@ from fastapi import APIRouter, HTTPException, Security from fuzzywuzzy import fuzz # type: ignore from server.routers.utility.sessions.middleware import HTTPBearerToUserID -from server.routers.utility.user import get_setup_user +from server.routers.utility.user import get_setup_user, prepare_user_payload from server.db.mongo.conn import archivesDB, coursesCOL from server.routers.model import (CACHED_HANDBOOK_NOTE, CONDITIONS, CourseCodes, CourseDetails, CoursesPath, CoursesPathDict, CoursesState, CoursesUnlockedWhenTaken, ProgramCourses, TermsList, @@ -415,7 +415,6 @@ def get_path_from(course: str) -> CoursesPathDict: "courses" : get_incoming_edges(course), } - @router.post("/coursesUnlockedWhenTaken/{courseToBeTaken}", response_model=CoursesUnlockedWhenTaken, responses={ 400: { "description": "Uh oh you broke me" }, @@ -431,8 +430,10 @@ def get_path_from(course: str) -> CoursesPathDict: } } }) -def courses_unlocked_when_taken(userData: UserData, courseToBeTaken: str) -> Dict[str, List[str]]: +def courses_unlocked_when_taken(uid: Annotated[str, Security(require_uid)], courseToBeTaken: str) -> Dict[str, List[str]]: """ Returns all courses which are unlocked when given course is taken """ + user = get_setup_user(uid) + userData = prepare_user_payload(user) ## initial state courses_initially_unlocked = unlocked_set(get_all_unlocked(userData)['courses_state']) ## add course to the user diff --git a/backend/server/routers/model.py b/backend/server/routers/model.py index 5f72d94a2..da8cd24cd 100644 --- a/backend/server/routers/model.py +++ b/backend/server/routers/model.py @@ -80,7 +80,7 @@ class UserData(BaseModel): program: str specialisations: list[str] - courses: dict + courses: dict # TODO-OLLI: type this correctly class CourseState(BaseModel): diff --git a/backend/server/routers/utility/user.py b/backend/server/routers/utility/user.py index eb034717b..dfb4e3bc3 100644 --- a/backend/server/routers/utility/user.py +++ b/backend/server/routers/utility/user.py @@ -1,7 +1,8 @@ +from typing import Optional from fastapi import HTTPException from starlette.status import HTTP_403_FORBIDDEN -from server.routers.model import CourseStorage, SettingsStorage, DegreeLocalStorage, PlannerLocalStorage, Storage +from server.routers.model import CourseStorage, Mark, SettingsStorage, DegreeLocalStorage, PlannerLocalStorage, Storage, UserData import server.db.helpers.users as udb from server.db.helpers.models import PartialUserStorage, UserStorage as NEWUserStorage, UserDegreeStorage as NEWUserDegreeStorage, UserPlannerStorage as NEWUserPlannerStorage, UserCoursesStorage as NEWUserCoursesStorage, UserCourseStorage as NEWUserCourseStorage, UserSettingsStorage as NEWUserSettingsStorage @@ -90,3 +91,47 @@ def set_user(uid: str, item: Storage, overwrite: bool = False): )) assert res + + +def parse_mark_to_int(mark: Mark) -> Optional[int]: + '''Converts the stored mark into a number grade for validation''' + # 'SY': null, 'FL': 25, 'PS': 60, 'CR': 70, 'DN': 80, 'HD': 90 + # TODO-OLLI: unify this with convert_to_planner_data + match mark: + case int() as n if 0 <= n <= 100: + return n + case 'SY': + return None + case 'FL': + return 25 + case 'PS': + return 60 + case 'CR': + return 70 + case 'DN': + return 80 + case 'HD': + return 90 + case _: + return None + +def prepare_user_payload(user: Storage) -> UserData: + ''' + Temporary helper, lifted directly from the frontend prepareUserPayload... + ''' + + courseMarks = { + code: parse_mark_to_int(courseData['mark']) + for code, courseData + in user['courses'].items() + } + + # TODO: was in the original prepareUserPayload, this should be an invariant though, thus unecessary + for code in user['planner']['unplanned']: + courseMarks[code] = None + + return UserData( + program=user['degree']['programCode'], + specialisations=user['degree']['specs'], + courses=courseMarks, + ) diff --git a/frontend/src/components/CourseDescriptionPanel/CourseDescriptionPanel.tsx b/frontend/src/components/CourseDescriptionPanel/CourseDescriptionPanel.tsx index ad5d48529..3202c5b66 100644 --- a/frontend/src/components/CourseDescriptionPanel/CourseDescriptionPanel.tsx +++ b/frontend/src/components/CourseDescriptionPanel/CourseDescriptionPanel.tsx @@ -12,6 +12,7 @@ import { LoadingCourseDescriptionPanelSidebar } from 'components/LoadingSkeleton'; import PlannerButton from 'components/PlannerButton'; +import useToken from 'hooks/useToken'; import CourseAttributes from './CourseAttributes'; import CourseInfoDrawers from './CourseInfoDrawers'; import S from './styles'; @@ -43,10 +44,11 @@ const CourseDescriptionPanel = ({ courses, degree }: CourseDescriptionPanelProps) => { + const token = useToken(); + const coursesUnlockedQuery = useQuery({ - queryKey: ['coursesUnlocked', courseCode, degree, planner, courses], - queryFn: () => getCoursesUnlockedWhenTaken(degree!, planner!, courses!, courseCode), - enabled: !!degree && !!planner && !!courses + queryKey: ['courses', 'coursesUnlockedWhenTaken', courseCode], + queryFn: () => getCoursesUnlockedWhenTaken(token, courseCode) }); const { pathname } = useLocation(); diff --git a/frontend/src/utils/api/coursesApi.ts b/frontend/src/utils/api/coursesApi.ts index 138f2e598..e2e88b1c1 100644 --- a/frontend/src/utils/api/coursesApi.ts +++ b/frontend/src/utils/api/coursesApi.ts @@ -37,15 +37,13 @@ export const getCourseChildren = async (courseCode: string) => { return res.data; }; -export const getCoursesUnlockedWhenTaken = async ( - degree: DegreeResponse, - planner: PlannerResponse, - courses: CoursesResponse, - courseCode: string -) => { +export const getCoursesUnlockedWhenTaken = async (token: string, courseCode: string) => { const res = await axios.post( `/courses/coursesUnlockedWhenTaken/${courseCode}`, - JSON.stringify(prepareUserPayload(degree, planner, courses)) + {}, + { + headers: withAuthorization(token) + } ); return res.data; }; From 1eba95716736139e05ca4c96426b3bd84b8f4382 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Fri, 6 Sep 2024 00:06:11 +1000 Subject: [PATCH 03/17] fix: tokenified the route --- backend/server/routers/courses.py | 48 +++++++++++-------- .../CourseSelector/CourseMenu/CourseMenu.tsx | 5 +- .../CourseGraph/CourseGraph.tsx | 5 +- frontend/src/utils/api/coursesApi.ts | 13 ++--- 4 files changed, 36 insertions(+), 35 deletions(-) diff --git a/backend/server/routers/courses.py b/backend/server/routers/courses.py index 861bea9f2..b736f3548 100644 --- a/backend/server/routers/courses.py +++ b/backend/server/routers/courses.py @@ -15,7 +15,7 @@ from server.routers.utility.sessions.middleware import HTTPBearerToUserID from server.routers.utility.user import get_setup_user, prepare_user_payload from server.db.mongo.conn import archivesDB, coursesCOL -from server.routers.model import (CACHED_HANDBOOK_NOTE, CONDITIONS, CourseCodes, CourseDetails, CoursesPath, +from server.routers.model import (CACHED_HANDBOOK_NOTE, CONDITIONS, CourseCodes, CourseDetails, CourseState, CoursesPath, CoursesPathDict, CoursesState, CoursesUnlockedWhenTaken, ProgramCourses, TermsList, TermsOffered, UserData) from server.routers.utility.common import get_core_courses, get_course_details, get_incoming_edges, get_legacy_course_details, get_program_structure, get_terms_offered_multiple_years @@ -72,6 +72,23 @@ def fix_user_data(userData: dict): userData["core_courses"] = get_core_courses(userData["program"], list(userData["specialisations"])) return userData +# TODO-OLLI: move this out +def get_all_course_states_for_user(userData: UserData) -> dict[str, CourseState]: + coursesState: dict[str, CourseState] = {} + user = User(fix_user_data(userData.model_dump())) + + for course, condition in CONDITIONS.items(): + result, warnings = condition.validate(user) if condition is not None else (True, []) + if result: + coursesState[course] = CourseState( + is_accurate=condition is not None, + unlocked=result, + handbook_note=CACHED_HANDBOOK_NOTE.get(course, ""), + warnings=warnings, + ) + + return coursesState + @router.get("/") def api_index() -> str: @@ -251,29 +268,18 @@ def search(search_string: str, uid: Annotated[str, Security(require_uid)]) -> Di }, }, }, - deprecated=True ) -def get_all_unlocked(userData: UserData) -> Dict[str, Dict]: +def get_all_unlocked(uid: Annotated[str, Security(require_uid)]) -> CoursesState: """ Given the userData and a list of locked courses, returns the state of all the courses. Note that locked courses always return as True with no warnings since it doesn't make sense for us to tell the user they can't take a course that they have already completed """ - - coursesState = {} - user = User(fix_user_data(userData.model_dump())) - for course, condition in CONDITIONS.items(): - result, warnings = condition.validate(user) if condition is not None else (True, []) - if result: - coursesState[course] = { - "is_accurate": condition is not None, - "unlocked": result, - "handbook_note": CACHED_HANDBOOK_NOTE.get(course, ""), - "warnings": warnings, - } - - return {"courses_state": coursesState} + user = get_setup_user(uid) + userData = prepare_user_payload(user) + coursesState = get_all_course_states_for_user(userData) + return CoursesState(courses_state=coursesState) @router.get( @@ -435,11 +441,11 @@ def courses_unlocked_when_taken(uid: Annotated[str, Security(require_uid)], cour user = get_setup_user(uid) userData = prepare_user_payload(user) ## initial state - courses_initially_unlocked = unlocked_set(get_all_unlocked(userData)['courses_state']) + courses_initially_unlocked = unlocked_set(get_all_course_states_for_user(userData)) ## add course to the user userData.courses[courseToBeTaken] = [get_course_details(courseToBeTaken)['UOC'], None] ## final state - courses_now_unlocked = unlocked_set(get_all_unlocked(userData)['courses_state']) + courses_now_unlocked = unlocked_set(get_all_course_states_for_user(userData)) new_courses = courses_now_unlocked - courses_initially_unlocked ## Differentiate direct and indirect unlocks @@ -514,9 +520,9 @@ def terms_offered(course: str, years:str) -> TermsOffered: ############################################################################### -def unlocked_set(courses_state) -> Set[str]: +def unlocked_set(courses_state: dict[str, CourseState]) -> Set[str]: """ Fetch the set of unlocked courses from the courses_state of a getAllUnlocked call """ - return set(course for course in courses_state if courses_state[course]['unlocked']) + return set(course for course in courses_state if courses_state[course].unlocked) def is_course_unlocked(course: str, user: User) -> Tuple[bool, List[str]]: """ diff --git a/frontend/src/pages/CourseSelector/CourseMenu/CourseMenu.tsx b/frontend/src/pages/CourseSelector/CourseMenu/CourseMenu.tsx index ff6a64927..0e82ee4c9 100644 --- a/frontend/src/pages/CourseSelector/CourseMenu/CourseMenu.tsx +++ b/frontend/src/pages/CourseSelector/CourseMenu/CourseMenu.tsx @@ -50,9 +50,8 @@ const CourseMenu = ({ planner, courses, degree }: CourseMenuProps) => { }); const coursesStateQuery = useQuery({ - queryKey: ['coursesState', degree, planner, courses], - queryFn: () => getAllUnlockedCourses(degree!, planner!, courses!), - enabled: !!degree && !!planner && !!courses + queryKey: ['courses', 'coursesState'], + queryFn: () => getAllUnlockedCourses(token) }); const queryClient = useQueryClient(); diff --git a/frontend/src/pages/GraphicalSelector/CourseGraph/CourseGraph.tsx b/frontend/src/pages/GraphicalSelector/CourseGraph/CourseGraph.tsx index eae3765c1..246259136 100644 --- a/frontend/src/pages/GraphicalSelector/CourseGraph/CourseGraph.tsx +++ b/frontend/src/pages/GraphicalSelector/CourseGraph/CourseGraph.tsx @@ -89,9 +89,8 @@ const CourseGraph = ({ }); const coursesStateQuery = useQuery({ - queryKey: ['coursesState', degreeQuery.data, plannerQuery.data, coursesQuery.data], - queryFn: () => getAllUnlockedCourses(degreeQuery.data!, plannerQuery.data!, coursesQuery.data!), - enabled: degreeQuery.isSuccess && plannerQuery.isSuccess && coursesQuery.isSuccess + queryKey: ['courses', 'coursesState'], + queryFn: () => getAllUnlockedCourses(token) }); const queriesSuccess = diff --git a/frontend/src/utils/api/coursesApi.ts b/frontend/src/utils/api/coursesApi.ts index e2e88b1c1..ad4d8fa3e 100644 --- a/frontend/src/utils/api/coursesApi.ts +++ b/frontend/src/utils/api/coursesApi.ts @@ -7,8 +7,6 @@ import { CoursesUnlockedWhenTaken, SearchCourse } from 'types/api'; -import { CoursesResponse, DegreeResponse, PlannerResponse } from 'types/userResponse'; -import prepareUserPayload from 'utils/prepareUserPayload'; import { LIVE_YEAR } from 'config/constants'; import { withAuthorization } from './authApi'; @@ -48,14 +46,13 @@ export const getCoursesUnlockedWhenTaken = async (token: string, courseCode: str return res.data; }; -export const getAllUnlockedCourses = async ( - degree: DegreeResponse, - planner: PlannerResponse, - courses: CoursesResponse -) => { +export const getAllUnlockedCourses = async (token: string) => { const res = await axios.post( '/courses/getAllUnlocked/', - JSON.stringify(prepareUserPayload(degree, planner, courses)) + {}, + { + headers: withAuthorization(token) + } ); return res.data; From 86e5b14ce0e7738bc126365248a5c4add023fb98 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Sat, 7 Sep 2024 18:12:09 +1000 Subject: [PATCH 04/17] fix: coursesUnlockedWhenTaken and getAllUnlocked tests now use tokens --- .../example_local_storage_data.json | 136 +++++++++++++++ .../test_courses_unlocked_when_taken.py | 162 +++++++++++------- .../tests/courses/test_get_all_unlocked.py | 29 +++- 3 files changed, 259 insertions(+), 68 deletions(-) diff --git a/backend/server/example_input/example_local_storage_data.json b/backend/server/example_input/example_local_storage_data.json index f20189d84..bd4f5fe72 100644 --- a/backend/server/example_input/example_local_storage_data.json +++ b/backend/server/example_input/example_local_storage_data.json @@ -185,5 +185,141 @@ "isSummerEnabled": false, "lockedTerms": {} } + }, + "sample_user_1": { + "degree": { + "programCode": "3778", + "specs": [ + "COMPA1", + "ACCTA2" + ] + }, + "planner": { + "years": [ { + "T0": [], + "T1": ["COMP1511"], + "T2": ["COMP1521"], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + } + ], + "unplanned": [], + "startYear": 2020, + "isSummerEnabled": false, + "lockedTerms": {} + } + }, + "sample_user_2": { + "degree": { + "programCode": "3789", + "specs": [ + "COMPJ1", + "FOODH1" + ] + }, + "planner": { + "years": [ { + "T0": [], + "T1": ["COMP1511"], + "T2": ["COMP1521", "COMP1531"], + "T3": ["COMP2521"] + }, + { + "T0": [], + "T1": ["COMP3231"], + "T2": ["COMP9242"], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + } + ], + "unplanned": [], + "startYear": 2020, + "isSummerEnabled": false, + "lockedTerms": {} + } + }, + "sample_user_one_math": { + "degree": { + "programCode": "3707", + "specs": [ + "COMPA1", + "ACCTA2" + ] + }, + "planner": { + "years": [ { + "T0": [], + "T1": ["MATH1131"], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + } + ], + "unplanned": [], + "startYear": 2020, + "isSummerEnabled": false, + "lockedTerms": {} + } + }, + "sample_user_no_courses": { + "degree": { + "programCode": "3707", + "specs": [ + "COMPA1", + "ACCTA2" + ] + }, + "planner": { + "years": [ { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + } + ], + "unplanned": [], + "startYear": 2020, + "isSummerEnabled": false, + "lockedTerms": {} + } } } \ No newline at end of file diff --git a/backend/server/tests/courses/test_courses_unlocked_when_taken.py b/backend/server/tests/courses/test_courses_unlocked_when_taken.py index fe7faf0cb..41941e5be 100644 --- a/backend/server/tests/courses/test_courses_unlocked_when_taken.py +++ b/backend/server/tests/courses/test_courses_unlocked_when_taken.py @@ -1,64 +1,98 @@ -import json - -import requests - -PATH = "./algorithms/tests/exampleUsers.json" - -with open(PATH, encoding="utf8") as f: - USERS = json.load(f) - -def test_no_courses_completed(): - x = requests.post( - 'http://127.0.0.1:8000/courses/coursesUnlockedWhenTaken/COMP1511', json=USERS["user_no_courses"]) - assert x.json() == { - "direct_unlock": [ - 'COMP1521', - 'COMP1531', - 'COMP2041', - 'COMP2121', - 'COMP2521', - 'COMP9334' - ], - # COMP1511 is equivalent to 'DPST1091' so it unlocks DPST1093 - # Maybe "equivalent" courses should be under `direct_unlock`? - "indirect_unlock": ["DPST1093"] - } - - -def test_malformed_request(): - x = requests.post( - 'http://127.0.0.1:8000/courses/coursesUnlockedWhenTaken/&&&&&', json=USERS["user_no_courses"]) - assert x.status_code == 400 - x = requests.post( - 'http://127.0.0.1:8000/courses/coursesUnlockedWhenTaken/COMPXXXX', json=USERS["user_no_courses"]) - assert x.status_code == 400 - - -def test_two_courses_completed(): - x = requests.post( - 'http://127.0.0.1:8000/courses/coursesUnlockedWhenTaken/COMP2521', json=USERS["user1"]) - # TABL2710 is unlocked because USER1 now meets the 18UOC requirement - assert x.json() == { - "direct_unlock": [ - "COMP3121", - "COMP3141", - "COMP3151", - "COMP3161", - "COMP3231", - "COMP3311", - "COMP3331", - "COMP3411", - "COMP3431", - "COMP3821", - "COMP3891", - "COMP6451", - "COMP6714", - "COMP6991", - "COMP9319", - "COMP9417", - "COMP9444", - "COMP9517", - "COMP9727", - ], - "indirect_unlock": ['BABS3301', "TABL2710"] - } +import json +import requests + +from server.tests.user.utility import clear, get_token, get_token_headers + +PATH = "server/example_input/example_local_storage_data.json" + +with open(PATH, encoding="utf8") as f: + DATA = json.load(f) + +def test_no_courses_completed(): + clear() + token = get_token() + headers = get_token_headers(token) + requests.post('http://127.0.0.1:8000/user/saveLocalStorage', json=DATA["sample_user_no_courses"], headers=headers) + + x = requests.post( + 'http://127.0.0.1:8000/courses/coursesUnlockedWhenTaken/COMP1511', json={}, headers=headers) + assert x.json() == { + "direct_unlock": [ + 'COMP1521', + 'COMP1531', + 'COMP2041', + 'COMP2121', + 'COMP2521', + 'COMP9334' + ], + # COMP1511 is equivalent to 'DPST1091' so it unlocks DPST1093 + # Maybe "equivalent" courses should be under `direct_unlock`? + "indirect_unlock": ["DPST1093"] + } + + +def test_malformed_request(): + clear() + token = get_token() + headers = get_token_headers(token) + requests.post('http://127.0.0.1:8000/user/saveLocalStorage', json=DATA["sample_user_no_courses"], headers=headers) + + x = requests.post( + 'http://127.0.0.1:8000/courses/coursesUnlockedWhenTaken/&&&&&', {}, headers=headers) + assert x.status_code == 400 + x = requests.post( + 'http://127.0.0.1:8000/courses/coursesUnlockedWhenTaken/COMPXXXX', {}, headers=headers) + assert x.status_code == 400 + + +def test_two_courses_completed(): + clear() + token = get_token() + headers = get_token_headers(token) + requests.post('http://127.0.0.1:8000/user/saveLocalStorage', json=DATA["sample_user_1"], headers=headers) + + requests.put( + 'http://127.0.0.1:8000/user/updateCourseMark', + json={ + 'course': 'COMP1511', + 'mark': 84 + }, + headers=headers + ) + + requests.put( + 'http://127.0.0.1:8000/user/updateCourseMark', + json={ + 'course': 'COMP1521', + 'mark': 57 + }, + headers=headers + ) + + x = requests.post( + 'http://127.0.0.1:8000/courses/coursesUnlockedWhenTaken/COMP2521', {}, headers=headers) + # TABL2710 is unlocked because USER1 now meets the 18UOC requirement + assert x.json() == { + "direct_unlock": [ + "COMP3121", + "COMP3141", + "COMP3151", + "COMP3161", + "COMP3231", + "COMP3311", + "COMP3331", + "COMP3411", + "COMP3431", + "COMP3821", + "COMP3891", + "COMP6451", + "COMP6714", + "COMP6991", + "COMP9319", + "COMP9417", + "COMP9444", + "COMP9517", + "COMP9727", + ], + "indirect_unlock": ['BABS3301', "TABL2710"] + } diff --git a/backend/server/tests/courses/test_get_all_unlocked.py b/backend/server/tests/courses/test_get_all_unlocked.py index 5f8ed5c93..71b4022a0 100644 --- a/backend/server/tests/courses/test_get_all_unlocked.py +++ b/backend/server/tests/courses/test_get_all_unlocked.py @@ -2,15 +2,22 @@ import requests -PATH = "./algorithms/tests/exampleUsers.json" +from server.tests.user.utility import clear, get_token, get_token_headers + +PATH = "server/example_input/example_local_storage_data.json" with open(PATH, encoding="utf8") as f: - USERS = json.load(f) + DATA = json.load(f) def test_fix_wam_only_unlock_given_course(): + clear() + token = get_token() + headers = get_token_headers(token) + requests.post('http://127.0.0.1:8000/user/saveLocalStorage', json=DATA["sample_user_2"], headers=headers) + x = requests.post( - "http://127.0.0.1:8000/courses/getAllUnlocked", json=USERS["user6"] + "http://127.0.0.1:8000/courses/getAllUnlocked", json={}, headers=headers ) assert x.status_code != 500 assert x.json()["courses_state"]["COMP1521"]["unlocked"] is True @@ -18,8 +25,22 @@ def test_fix_wam_only_unlock_given_course(): def test_unlock_dependent_course(): + clear() + token = get_token() + headers = get_token_headers(token) + requests.post('http://127.0.0.1:8000/user/saveLocalStorage', json=DATA["sample_user_one_math"], headers=headers) + + requests.put( + 'http://127.0.0.1:8000/user/updateCourseMark', + json={ + 'course': 'MATH1131', + 'mark': 100 + }, + headers=headers + ) + x = requests.post( - "http://127.0.0.1:8000/courses/getAllUnlocked", json=USERS["user_one_math"] + "http://127.0.0.1:8000/courses/getAllUnlocked", json={}, headers=headers ) assert x.status_code != 500 print(x.json()) From ddb0f5799145450d05e901556cd77a9bac86c648 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Sat, 7 Sep 2024 18:23:50 +1000 Subject: [PATCH 05/17] remove prepareUserPayload from frontend --- frontend/src/utils/prepareUserPayload.ts | 36 ------------------------ 1 file changed, 36 deletions(-) delete mode 100644 frontend/src/utils/prepareUserPayload.ts diff --git a/frontend/src/utils/prepareUserPayload.ts b/frontend/src/utils/prepareUserPayload.ts deleted file mode 100644 index 27c61162f..000000000 --- a/frontend/src/utils/prepareUserPayload.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { CoursesResponse, DegreeResponse, PlannerResponse } from 'types/userResponse'; -import { parseMarkToInt } from 'pages/TermPlanner/utils'; - -// key = course code, value = mark of course (number | null) -type UserPayloadCourse = Record; - -type UserPayload = { - program: string; - courses: UserPayloadCourse; - specialisations: string[]; -}; - -// TODO: Remove the slice types once fully migrated -const prepareUserPayload = ( - degree: DegreeResponse, - planner: PlannerResponse, - courses: CoursesResponse -): UserPayload => { - const { programCode, specs } = degree; - - const selectedCourses: UserPayloadCourse = {}; - Object.entries(courses).forEach(([courseCode, courseData]) => { - selectedCourses[courseCode] = parseMarkToInt(courseData.mark); - }); - planner.unplanned.forEach((courseCode) => { - selectedCourses[courseCode] = null; - }); - - return { - program: programCode, - specialisations: specs, - courses: selectedCourses - }; -}; - -export default prepareUserPayload; From 71dfc9f76b00ad1e517975df863bba0c521e9ab2 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Sat, 7 Sep 2024 22:50:54 +1000 Subject: [PATCH 06/17] feat: created function, and finally removed old UserData model --- backend/algorithms/objects/user.py | 21 ++++++++---- backend/server/routers/courses.py | 44 +++++++++----------------- backend/server/routers/model.py | 10 ------ backend/server/routers/utility/user.py | 35 ++++++++++---------- 4 files changed, 48 insertions(+), 62 deletions(-) diff --git a/backend/algorithms/objects/user.py b/backend/algorithms/objects/user.py index 4c701a47a..af567036f 100644 --- a/backend/algorithms/objects/user.py +++ b/backend/algorithms/objects/user.py @@ -9,7 +9,7 @@ import json import re from itertools import chain -from typing import List, Literal, Optional, Tuple +from typing import List, Literal, Optional, Tuple, TypedDict from algorithms.cache.cache_config import CACHED_EQUIVALENTS_FILE, CACHED_EXCLUSIONS_FILE from algorithms.objects.categories import AnyCategory, Category @@ -20,6 +20,13 @@ with open(CACHED_EXCLUSIONS_FILE, "r", encoding="utf8") as f: CACHED_EXCLUSIONS: dict[str, dict[str, Literal[1]]] = json.load(f) +class UserJSON(TypedDict, total=False): + courses: dict[str, tuple[int, Optional[int]]] + program: Optional[str] + specialisations: list[str] + year: int + core_courses: list[str] + class User: """ A user and their data which will be used to determine if they can take a course """ # pylint: disable=too-many-public-methods @@ -119,18 +126,18 @@ def in_specialisation(self, specialisation: str): for spec in self.specialisations ) - def load_json(self, data): + def load_json(self, data: UserJSON): """ Given the user data, correctly loads it into this user class """ - if "program" in data.keys(): + if "program" in data: self.program = copy.deepcopy(data["program"]) - if "specialisations" in data.keys(): + if "specialisations" in data: self.specialisations = copy.deepcopy(data["specialisations"]) - if "courses" in data.keys(): + if "courses" in data: self.courses = copy.deepcopy(data["courses"]) - if "year" in data.keys(): + if "year" in data: self.year = copy.deepcopy(data["year"]) - if "core_courses" in data.keys(): + if "core_courses" in data: self.core_courses = copy.deepcopy(data["core_courses"]) def get_grade(self, course: str): diff --git a/backend/server/routers/courses.py b/backend/server/routers/courses.py index b736f3548..30f21219a 100644 --- a/backend/server/routers/courses.py +++ b/backend/server/routers/courses.py @@ -13,12 +13,12 @@ from fastapi import APIRouter, HTTPException, Security from fuzzywuzzy import fuzz # type: ignore from server.routers.utility.sessions.middleware import HTTPBearerToUserID -from server.routers.utility.user import get_setup_user, prepare_user_payload +from server.routers.utility.user import get_setup_user, user_storage_to_algo_user from server.db.mongo.conn import archivesDB, coursesCOL from server.routers.model import (CACHED_HANDBOOK_NOTE, CONDITIONS, CourseCodes, CourseDetails, CourseState, CoursesPath, CoursesPathDict, CoursesState, CoursesUnlockedWhenTaken, ProgramCourses, TermsList, - TermsOffered, UserData) -from server.routers.utility.common import get_core_courses, get_course_details, get_incoming_edges, get_legacy_course_details, get_program_structure, get_terms_offered_multiple_years + TermsOffered) +from server.routers.utility.common import get_course_details, get_incoming_edges, get_legacy_course_details, get_program_structure, get_terms_offered_multiple_years router = APIRouter( prefix="/courses", @@ -57,25 +57,9 @@ def fetch_all_courses() -> Dict[str, str]: return ALL_COURSES -def fix_user_data(userData: dict): - """ Updates and returns the userData with the UOC of a course """ - coursesWithoutUoc = [ - course - for course in userData["courses"] - if not isinstance(userData["courses"][course], list) - ] - filledInCourses = { - course: [get_course_details(course)['UOC'], userData["courses"][course]] - for course in coursesWithoutUoc - } - userData["courses"].update(filledInCourses) - userData["core_courses"] = get_core_courses(userData["program"], list(userData["specialisations"])) - return userData - # TODO-OLLI: move this out -def get_all_course_states_for_user(userData: UserData) -> dict[str, CourseState]: +def get_all_course_states_for_user(user: User) -> dict[str, CourseState]: coursesState: dict[str, CourseState] = {} - user = User(fix_user_data(userData.model_dump())) for course, condition in CONDITIONS.items(): result, warnings = condition.validate(user) if condition is not None else (True, []) @@ -277,9 +261,9 @@ def get_all_unlocked(uid: Annotated[str, Security(require_uid)]) -> CoursesState that they have already completed """ user = get_setup_user(uid) - userData = prepare_user_payload(user) - coursesState = get_all_course_states_for_user(userData) - return CoursesState(courses_state=coursesState) + algo_user = user_storage_to_algo_user(user) + courses_states = get_all_course_states_for_user(algo_user) + return CoursesState(courses_state=courses_states) @router.get( @@ -354,12 +338,13 @@ def get_legacy_course(year: str, courseCode: str): } } }) -def unselect_course(userData: UserData, unselectedCourse: str) -> dict[str, list[str]]: +def unselect_course(uid: Annotated[str, Security(require_uid)], unselectedCourse: str) -> dict[str, list[str]]: """ Creates a new user class and returns all the courses affected from the course that was unselected in alphabetically sorted order """ - user = User(fix_user_data(userData.dict())) + user_data = get_setup_user(uid) + user = user_storage_to_algo_user(user_data) if not user.has_taken_course(unselectedCourse): return { 'courses' : [] } @@ -439,13 +424,14 @@ def get_path_from(course: str) -> CoursesPathDict: def courses_unlocked_when_taken(uid: Annotated[str, Security(require_uid)], courseToBeTaken: str) -> Dict[str, List[str]]: """ Returns all courses which are unlocked when given course is taken """ user = get_setup_user(uid) - userData = prepare_user_payload(user) + algo_user = user_storage_to_algo_user(user) + ## initial state - courses_initially_unlocked = unlocked_set(get_all_course_states_for_user(userData)) + courses_initially_unlocked = unlocked_set(get_all_course_states_for_user(algo_user)) ## add course to the user - userData.courses[courseToBeTaken] = [get_course_details(courseToBeTaken)['UOC'], None] + algo_user.add_courses({ courseToBeTaken: (get_course_details(courseToBeTaken)['UOC'], None) }) ## final state - courses_now_unlocked = unlocked_set(get_all_course_states_for_user(userData)) + courses_now_unlocked = unlocked_set(get_all_course_states_for_user(algo_user)) new_courses = courses_now_unlocked - courses_initially_unlocked ## Differentiate direct and indirect unlocks diff --git a/backend/server/routers/model.py b/backend/server/routers/model.py index da8cd24cd..6d00fa8b2 100644 --- a/backend/server/routers/model.py +++ b/backend/server/routers/model.py @@ -73,16 +73,6 @@ class Structure(BaseModel): structure: dict[str, StructureContainer] uoc: int - -# TODO: routes that use this should just take a token now -class UserData(BaseModel): - model_config = ConfigDict(extra='forbid') - - program: str - specialisations: list[str] - courses: dict # TODO-OLLI: type this correctly - - class CourseState(BaseModel): model_config = ConfigDict(extra='forbid') diff --git a/backend/server/routers/utility/user.py b/backend/server/routers/utility/user.py index dfb4e3bc3..811a125fd 100644 --- a/backend/server/routers/utility/user.py +++ b/backend/server/routers/utility/user.py @@ -2,7 +2,9 @@ from fastapi import HTTPException from starlette.status import HTTP_403_FORBIDDEN -from server.routers.model import CourseStorage, Mark, SettingsStorage, DegreeLocalStorage, PlannerLocalStorage, Storage, UserData +from algorithms.objects.user import UserJSON, User +from server.routers.utility.common import get_core_courses, get_course_details +from server.routers.model import CourseStorage, Mark, SettingsStorage, DegreeLocalStorage, PlannerLocalStorage, Storage import server.db.helpers.users as udb from server.db.helpers.models import PartialUserStorage, UserStorage as NEWUserStorage, UserDegreeStorage as NEWUserDegreeStorage, UserPlannerStorage as NEWUserPlannerStorage, UserCoursesStorage as NEWUserCoursesStorage, UserCourseStorage as NEWUserCourseStorage, UserSettingsStorage as NEWUserSettingsStorage @@ -115,23 +117,24 @@ def parse_mark_to_int(mark: Mark) -> Optional[int]: case _: return None -def prepare_user_payload(user: Storage) -> UserData: - ''' - Temporary helper, lifted directly from the frontend prepareUserPayload... - ''' - - courseMarks = { - code: parse_mark_to_int(courseData['mark']) +def user_storage_to_algo_user(user: Storage) -> User: + '''Convert the database user into the algorithm object user.''' + courses_with_uoc: dict[str, tuple[int, Optional[int]]] = { + code: ( + get_course_details(code)['UOC'], + parse_mark_to_int(courseData['mark']) if code not in user['planner']['unplanned'] else None + ) for code, courseData in user['courses'].items() } - # TODO: was in the original prepareUserPayload, this should be an invariant though, thus unecessary - for code in user['planner']['unplanned']: - courseMarks[code] = None + user_data: UserJSON = { + 'specialisations': user['degree']['specs'], + 'program': user['degree']['programCode'], + 'core_courses': get_core_courses(user['degree']['programCode'], list(user['degree']['specs'])), + 'courses': courses_with_uoc + } - return UserData( - program=user['degree']['programCode'], - specialisations=user['degree']['specs'], - courses=courseMarks, - ) + algo_user = User() + algo_user.load_json(user_data) + return algo_user From 70521bf9c06834246f67e9fd747c732739b33fc9 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Sat, 7 Sep 2024 22:52:28 +1000 Subject: [PATCH 07/17] fix: unselect tests now use token --- .../tests/courses/test_unselect_course.py | 69 ++++++++++++++++--- backend/server/tests/test_validation.py | 5 +- 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/backend/server/tests/courses/test_unselect_course.py b/backend/server/tests/courses/test_unselect_course.py index 1eb829754..ade635782 100644 --- a/backend/server/tests/courses/test_unselect_course.py +++ b/backend/server/tests/courses/test_unselect_course.py @@ -1,27 +1,74 @@ -import copy import json - import requests -with open("./algorithms/tests/exampleUsers.json", encoding="utf8") as f: - USERS = json.load(f) +from server.tests.user.utility import clear, get_token, get_token_headers + +PATH = "server/example_input/example_local_storage_data.json" +with open(PATH, encoding="utf8") as f: + DATA = json.load(f) -USER_NO_UOC = copy.deepcopy(USERS["user6"]) -for course in USER_NO_UOC["courses"]: - [uoc, mark] = USER_NO_UOC["courses"][course] - USER_NO_UOC["courses"][course] = [mark] +MARKS = { + "COMP9242": 81, + "COMP3231": 61, + "COMP2521": 57, + "COMP1531": 66, + "COMP1521": 57, + "COMP1511": 84 +} def test_no_dependencies(): - x = requests.post("http://127.0.0.1:8000/courses/unselectCourse/COMP1531", json=USERS["user6"]) + clear() + token = get_token() + headers = get_token_headers(token) + requests.post('http://127.0.0.1:8000/user/saveLocalStorage', json=DATA["sample_user_2"], headers=headers) + + for code, mark in MARKS.items(): + requests.put('http://127.0.0.1:8000/user/updateCourseMark', + json={ + 'course': code, + 'mark': mark + }, + headers=headers + ) + + x = requests.post("http://127.0.0.1:8000/courses/unselectCourse/COMP1531", json={}, headers=headers) assert x.status_code == 200 assert x.json()["courses"] == ["COMP1531", "COMP9242"] def test_multiple_dependencies(): - x = requests.post("http://127.0.0.1:8000/courses/unselectCourse/COMP1511", json=USERS["user6"]) + clear() + token = get_token() + headers = get_token_headers(token) + requests.post('http://127.0.0.1:8000/user/saveLocalStorage', json=DATA["sample_user_2"], headers=headers) + + for code, mark in MARKS.items(): + requests.put('http://127.0.0.1:8000/user/updateCourseMark', + json={ + 'course': code, + 'mark': mark + }, + headers=headers + ) + + x = requests.post("http://127.0.0.1:8000/courses/unselectCourse/COMP1511", json={}, headers=headers) assert x.status_code == 200 assert x.json()["courses"] == ["COMP1511", "COMP1521", "COMP1531", "COMP2521", "COMP3231", "COMP9242"] def test_invalid_course(): - x = requests.post('http://127.0.0.1:8000/courses/unselectCourse/BADC0000', json=USERS["user6"]) + clear() + token = get_token() + headers = get_token_headers(token) + requests.post('http://127.0.0.1:8000/user/saveLocalStorage', json=DATA["sample_user_2"], headers=headers) + + for code, mark in MARKS.items(): + requests.put('http://127.0.0.1:8000/user/updateCourseMark', + json={ + 'course': code, + 'mark': mark + }, + headers=headers + ) + + x = requests.post('http://127.0.0.1:8000/courses/unselectCourse/BADC0000', json={}, headers=headers) assert x.status_code == 200 diff --git a/backend/server/tests/test_validation.py b/backend/server/tests/test_validation.py index 397cdb72b..03be21be9 100644 --- a/backend/server/tests/test_validation.py +++ b/backend/server/tests/test_validation.py @@ -1,13 +1,16 @@ # we want to assure that courses that may be accessed from a container are always accurately computed. from contextlib import suppress from itertools import chain +import json from typing import Any import pytest import requests from server.routers.model import StructureContainer -from server.tests.courses.test_get_all_unlocked import USERS from server.tests.programs.test_get_structure import fake_specs +with open("./algorithms/tests/exampleUsers.json", encoding="utf8") as f: + USERS = json.load(f) + FAILS: list[Any] = [] # TODO: some of these should probs not be ignored From cb46ff401b9830741f0595e7260fc1c3e212fccd Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Sat, 7 Sep 2024 22:53:42 +1000 Subject: [PATCH 08/17] added note into unselectCourse route --- backend/server/routers/courses.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/server/routers/courses.py b/backend/server/routers/courses.py index 30f21219a..7121df9b4 100644 --- a/backend/server/routers/courses.py +++ b/backend/server/routers/courses.py @@ -319,7 +319,6 @@ def get_legacy_course(year: str, courseCode: str): return result -# TODO-OLLI(pm): this is not used anymore... (was a step in removing a course from planner) @router.post("/unselectCourse/{unselectedCourse}", response_model=CourseCodes, responses={ 400: {"description": "Uh oh you broke me"}, @@ -342,6 +341,8 @@ def unselect_course(uid: Annotated[str, Security(require_uid)], unselectedCourse """ Creates a new user class and returns all the courses affected from the course that was unselected in alphabetically sorted order + + NOTE: No longer in use... """ user_data = get_setup_user(uid) user = user_storage_to_algo_user(user_data) From f8d221bbcb93a05aacf5035355e2d11d972fdb59 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Sat, 7 Sep 2024 23:01:44 +1000 Subject: [PATCH 09/17] fix: aligned mark mappings to the WAM spec --- backend/server/routers/utility/user.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/server/routers/utility/user.py b/backend/server/routers/utility/user.py index 811a125fd..f221c0d28 100644 --- a/backend/server/routers/utility/user.py +++ b/backend/server/routers/utility/user.py @@ -97,7 +97,7 @@ def set_user(uid: str, item: Storage, overwrite: bool = False): def parse_mark_to_int(mark: Mark) -> Optional[int]: '''Converts the stored mark into a number grade for validation''' - # 'SY': null, 'FL': 25, 'PS': 60, 'CR': 70, 'DN': 80, 'HD': 90 + # https://www.student.unsw.edu.au/wam # TODO-OLLI: unify this with convert_to_planner_data match mark: case int() as n if 0 <= n <= 100: @@ -107,7 +107,7 @@ def parse_mark_to_int(mark: Mark) -> Optional[int]: case 'FL': return 25 case 'PS': - return 60 + return 55 case 'CR': return 70 case 'DN': From df2c667e4813ebe542ef5de42f1857535aca3a10 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Sat, 7 Sep 2024 23:39:00 +1000 Subject: [PATCH 10/17] wip: move out convert_to_planner_data --- backend/server/routers/ctf.py | 3 +-- backend/server/routers/model.py | 2 ++ backend/server/routers/planner.py | 29 ++++---------------------- backend/server/routers/utility/user.py | 22 ++++++++++++++++++- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/backend/server/routers/ctf.py b/backend/server/routers/ctf.py index e96509dc2..0b356ca61 100644 --- a/backend/server/routers/ctf.py +++ b/backend/server/routers/ctf.py @@ -46,7 +46,7 @@ from fastapi import APIRouter, Security from server.routers.utility.sessions.middleware import HTTPBearerToUserID -from server.routers.utility.user import get_setup_user +from server.routers.utility.user import convert_to_planner_data, get_setup_user from server.routers.model import ValidPlannerData router = APIRouter( @@ -254,7 +254,6 @@ def validate_ctf(uid: Annotated[str, Security(require_uid)]): """ Validates the CTF """ - from server.routers.planner import convert_to_planner_data # TODO: remove when converted data = convert_to_planner_data(get_setup_user(uid)) passed: list[str] = [] diff --git a/backend/server/routers/model.py b/backend/server/routers/model.py index 6d00fa8b2..49ed7f8aa 100644 --- a/backend/server/routers/model.py +++ b/backend/server/routers/model.py @@ -127,6 +127,7 @@ class CoursesTypeState(BaseModel): # TODO: this is only used in ctf.py and the validatePlanner route... # Shuffle those to use the user objects directly, then we can delete this and the `convert_to_planner_data` function +# TODO-OLLI: remove class ValidPlannerData(BaseModel): model_config = ConfigDict(extra='forbid') @@ -135,6 +136,7 @@ class ValidPlannerData(BaseModel): plan: list[list[dict[str, tuple[int, Optional[int]]]]] # TODO: the last surviving route that uses this is the autoplanning, convert it and delete this... +# TODO-OLLI: remove class PlannerData(BaseModel): programCode: str specialisations: list[str] diff --git a/backend/server/routers/planner.py b/backend/server/routers/planner.py index 452201860..6c6511edc 100644 --- a/backend/server/routers/planner.py +++ b/backend/server/routers/planner.py @@ -4,42 +4,21 @@ from math import lcm from operator import itemgetter -from typing import Annotated, Dict, List, Optional, Tuple +from typing import Annotated, Dict, List from algorithms.autoplanning import autoplan from algorithms.transcript import parse_transcript from algorithms.validate_term_planner import validate_terms from fastapi import APIRouter, HTTPException, Security, UploadFile from server.routers.utility.sessions.middleware import HTTPBearerToUserID -from server.routers.utility.user import get_setup_user, set_user -from server.routers.model import (CourseCode, PlannedToTerm, PlannerData, ProgramTime, Storage, UnPlannedToTerm, - ValidCoursesState, ValidPlannerData, markMap) +from server.routers.utility.user import convert_to_planner_data, get_setup_user, set_user +from server.routers.model import (CourseCode, PlannedToTerm, PlannerData, ProgramTime, UnPlannedToTerm, + ValidCoursesState) from server.routers.utility.common import get_course_details, get_course_object MIN_COMPLETED_COURSE_UOC = 6 -def convert_to_planner_data(user: Storage) -> ValidPlannerData: - """ fixes the planner data to add missing UOC info """ - plan: list[list[dict[str, Tuple[int, Optional[int]]]]] = [] - for year_index, year in enumerate(user['planner']['years']): - plan.append([]) - for term_index, term in enumerate(year.values()): - plan[year_index].append({}) - for courseName in term: - c = user['courses'][courseName] - mark = c['mark'] - if not isinstance(mark, int) and mark is not None: - mark = markMap.get(mark, None) - plan[year_index][term_index][courseName] = ( - int(c["uoc"]), mark) - return ValidPlannerData( - programCode=user['degree']['programCode'], - specialisations=user['degree']['specs'], - plan=plan, - ) - - router = APIRouter( prefix="/planner", tags=["planner"], responses={404: {"description": "Not found"}} ) diff --git a/backend/server/routers/utility/user.py b/backend/server/routers/utility/user.py index f221c0d28..7dd452000 100644 --- a/backend/server/routers/utility/user.py +++ b/backend/server/routers/utility/user.py @@ -4,7 +4,7 @@ from algorithms.objects.user import UserJSON, User from server.routers.utility.common import get_core_courses, get_course_details -from server.routers.model import CourseStorage, Mark, SettingsStorage, DegreeLocalStorage, PlannerLocalStorage, Storage +from server.routers.model import CourseStorage, Mark, SettingsStorage, DegreeLocalStorage, PlannerLocalStorage, Storage, ValidPlannerData, markMap import server.db.helpers.users as udb from server.db.helpers.models import PartialUserStorage, UserStorage as NEWUserStorage, UserDegreeStorage as NEWUserDegreeStorage, UserPlannerStorage as NEWUserPlannerStorage, UserCoursesStorage as NEWUserCoursesStorage, UserCourseStorage as NEWUserCourseStorage, UserSettingsStorage as NEWUserSettingsStorage @@ -138,3 +138,23 @@ def user_storage_to_algo_user(user: Storage) -> User: algo_user = User() algo_user.load_json(user_data) return algo_user + +def convert_to_planner_data(user: Storage) -> ValidPlannerData: + """ fixes the planner data to add missing UOC info """ + plan: list[list[dict[str, tuple[int, Optional[int]]]]] = [] + for year_index, year in enumerate(user['planner']['years']): + plan.append([]) + for term_index, term in enumerate(year.values()): + plan[year_index].append({}) + for courseName in term: + c = user['courses'][courseName] + mark = c['mark'] + if not isinstance(mark, int) and mark is not None: + mark = markMap.get(mark, None) + plan[year_index][term_index][courseName] = ( + int(c["uoc"]), mark) + return ValidPlannerData( + programCode=user['degree']['programCode'], + specialisations=user['degree']['specs'], + plan=plan, + ) From 47c54db1b64c509fdecfbf67fd4dfe80d2668c11 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Sun, 8 Sep 2024 01:26:20 +1000 Subject: [PATCH 11/17] feat: removed model and reworked route --- backend/server/routers/model.py | 62 -------------- backend/server/routers/planner.py | 20 ++--- .../server/tests/planner/test_autoplanning.py | 82 +++++++++++-------- 3 files changed, 60 insertions(+), 104 deletions(-) diff --git a/backend/server/routers/model.py b/backend/server/routers/model.py index 49ed7f8aa..f18231b71 100644 --- a/backend/server/routers/model.py +++ b/backend/server/routers/model.py @@ -4,7 +4,6 @@ from typing import Literal, Optional, Set, TypedDict, Union from algorithms.objects.conditions import CompositeCondition -from algorithms.objects.user import User from pydantic import BaseModel, ConfigDict, with_config @@ -135,67 +134,6 @@ class ValidPlannerData(BaseModel): specialisations: list[str] plan: list[list[dict[str, tuple[int, Optional[int]]]]] -# TODO: the last surviving route that uses this is the autoplanning, convert it and delete this... -# TODO-OLLI: remove -class PlannerData(BaseModel): - programCode: str - specialisations: list[str] - plan: list[list[dict[str, Optional[list[Optional[int]]]]]] - model_config = ConfigDict(json_schema_extra={ - "example": { - "program": "3707", - "specialisations": ["COMPA1"], - "plan": [ - [ - {}, - { - "COMP1511": [6, None], - "MATH1141": [6, None], - "MATH1081": [6, None], - }, - { - "COMP1521": [6, None], - "COMP9444": [6, None], - }, - { - "COMP2521": [6, None], - "MATH1241": [6, None], - "COMP3331": [6, None], - }, - ], - [ - {}, - { - "COMP1531": [6, None], - "COMP6080": [6, None], - "COMP3821": [6, None], - }, - ], - ], - } - }, extra='forbid') - - def to_user(self) -> User: - user = User() - user.program = self.programCode - user.specialisations = self.specialisations[:] - - # prevent circular import; TODO: There has to be a better way - from server.routers.utility.common import get_core_courses, get_course_details - - for year in self.plan: - for term in year: - cleaned_term = {} - for course_name, course_value in term.items(): - cleaned_term[course_name] = ( - (course_value[0], course_value[1]) if course_value - else (get_course_details(course_name)['UOC'], None) # type: ignore - ) - user.add_courses(cleaned_term) - # get the cores of the user - user.core_courses = get_core_courses(user.program, user.specialisations) - return user - # TODO-OLLI(pm): get rid of these user models in favour of the database models @with_config(ConfigDict(extra='forbid')) class DegreeLocalStorage(TypedDict): diff --git a/backend/server/routers/planner.py b/backend/server/routers/planner.py index 6c6511edc..ed2854637 100644 --- a/backend/server/routers/planner.py +++ b/backend/server/routers/planner.py @@ -11,9 +11,8 @@ from algorithms.validate_term_planner import validate_terms from fastapi import APIRouter, HTTPException, Security, UploadFile from server.routers.utility.sessions.middleware import HTTPBearerToUserID -from server.routers.utility.user import convert_to_planner_data, get_setup_user, set_user -from server.routers.model import (CourseCode, PlannedToTerm, PlannerData, ProgramTime, UnPlannedToTerm, - ValidCoursesState) +from server.routers.utility.user import convert_to_planner_data, get_setup_user, set_user, user_storage_to_algo_user +from server.routers.model import CourseCode, PlannedToTerm, ProgramTime, UnPlannedToTerm, ValidCoursesState from server.routers.utility.common import get_course_details, get_course_object MIN_COMPLETED_COURSE_UOC = 6 @@ -491,24 +490,25 @@ def out_of_bounds(num_years, dest_row, terms): # } # } # ) -def autoplanning(courseCodes: list[str], plannerData: PlannerData, programTime: ProgramTime) -> dict: +def autoplanning(uid: Annotated[str, Security(require_uid)], courseCodes: list[str], programTime: ProgramTime) -> dict: print("started to_user") - user = plannerData.to_user() + user_data = get_setup_user(uid) + user = user_storage_to_algo_user(user_data) print('finished the to_user') try: courses = [get_course_object(courseCode, programTime) for courseCode in courseCodes] print('in the try') - for year_index, year in enumerate(list(plannerData.plan)): - for term_index, term in enumerate(year): - for course in term: + for year_index, year in enumerate(user_data['planner']['years']): + for term_index in range(4): + for course_code in year[f'T{term_index}']: courses.append( get_course_object( - course, + course_code, programTime, (year_index, term_index), - user.get_grade(course) + user.get_grade(course_code) ) ) print("got to end") diff --git a/backend/server/tests/planner/test_autoplanning.py b/backend/server/tests/planner/test_autoplanning.py index d3fe94075..2c64e746b 100644 --- a/backend/server/tests/planner/test_autoplanning.py +++ b/backend/server/tests/planner/test_autoplanning.py @@ -1,51 +1,69 @@ import requests from pytest import mark +from server.tests.user.utility import clear, get_token, get_token_headers + @mark.skip(reason = "Autoplanning incompatiable with migration") def test_autoplanning_generic(): + clear() + token = get_token() + headers = get_token_headers(token) + requests.post('http://127.0.0.1:8000/user/saveLocalStorage', json={ + "degree": { + "programCode": "3778", + "specs": [ + "COMPA1" + ] + }, + "planner": { + "years": [ + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + } + ], + "unplanned": [], + "startYear": 2020, + "isSummerEnabled": False, + "lockedTerms": {} + } + }, headers=headers) + x = requests.post( 'http://127.0.0.1:8000/planner/autoplanning', json={ 'courseCodes': [ 'COMP1511', 'COMP2521', 'COMP1531', 'COMP1521', 'MATH1141', 'MATH1241', 'MATH1081', 'COMP2041', 'ENGG2600', 'ENGG2600', 'ENGG2600', 'COMP2511', 'MATH3411', 'COMP3411', 'COMP6841', 'COMP3231', 'COMP3141', 'COMP3121', 'COMP3131', 'COMP4141', 'COMP3901', 'ARTS1360' ], - 'plannerData': { - "program": "3778", - "specialisations": ["COMPA1"], - "plan": [ - [ - {}, - {}, - {}, - {}, - ], - [ - {}, - {}, - {}, - {}, - ], - [ - {}, - {}, - {}, - {}, - ], - [ - {}, - {}, - {}, - {}, - ], - ], - }, 'programTime': { 'startTime': [2020, 1], 'endTime': [2023, 3], 'uocMax': [ 12, 20, 20, 20, 12, 20, 20, 20, 10, 20, 20, 20, 10, 20, 20, 20 ] - } - }) + }, + }, + headers=headers + ) assert x.status_code == 200 # TODO: please write actual tests From 249fc7d30aa17d8cdbeebaff8fbf2fe9d14773af Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Sun, 8 Sep 2024 01:48:08 +1000 Subject: [PATCH 12/17] fix: convert ctf to fully use user Storage --- backend/server/routers/ctf.py | 84 +++++++++++++++++------------------ 1 file changed, 40 insertions(+), 44 deletions(-) diff --git a/backend/server/routers/ctf.py b/backend/server/routers/ctf.py index 0b356ca61..01924737c 100644 --- a/backend/server/routers/ctf.py +++ b/backend/server/routers/ctf.py @@ -46,8 +46,8 @@ from fastapi import APIRouter, Security from server.routers.utility.sessions.middleware import HTTPBearerToUserID -from server.routers.utility.user import convert_to_planner_data, get_setup_user -from server.routers.model import ValidPlannerData +from server.routers.utility.user import get_setup_user +from server.routers.model import Storage router = APIRouter( prefix="/ctf", tags=["ctf"], responses={404: {"description": "Not found"}} @@ -56,14 +56,14 @@ require_uid = HTTPBearerToUserID() -def all_courses(data: ValidPlannerData) -> set[str]: +def all_courses(data: Storage) -> set[str]: """ Returns all courses from a planner """ return { course - for year in data.plan - for term in year + for year in data["planner"]["years"] + for term in year.values() for course in term } @@ -95,19 +95,19 @@ def gen_eds(courses: set[str]) -> set[str]: ) -def hard_requirements(data: ValidPlannerData) -> bool: +def hard_requirements(data: Storage) -> bool: # NOTE: Can't check start year from this # Frontend should handle most of this anyways # including validity of the program return ( - data.programCode == "3778" - and "COMPA1" in data.specialisations - and "MATHC2" in data.specialisations - and len(data.plan) == 3 + data["degree"]["programCode"] == "3778" + and "COMPA1" in data["degree"]["specs"] + and "MATHC2" in data["degree"]["specs"] + and len(data["planner"]["years"]) == 3 ) -def extended_courses(data: ValidPlannerData) -> bool: +def extended_courses(data: Storage) -> bool: """ Must take atleast 3 courses with extended in the name """ @@ -121,61 +121,55 @@ def extended_courses(data: ValidPlannerData) -> bool: return len(extended_course_codes & all_courses(data)) >= 3 -def summer_course(data: ValidPlannerData) -> bool: +def summer_course(data: Storage) -> bool: """ Must take atleast one summer course """ return any( course.startswith("COMP") - for year in data.plan - for course in year[0] + for year in data["planner"]["years"] + for course in year["T0"] ) -def term_sums_even(data: ValidPlannerData) -> bool: +def term_sums_even(data: Storage) -> bool: """ Check that the sum of the course codes in even terms is even """ is_even: Callable[[int], bool] = lambda x: x % 2 == 0 return all( - is_even(sum(map(get_code, term.keys()))) - for year in data.plan - for term in year[2::2] + is_even(sum(map(get_code, term))) + for year in data["planner"]["years"] + for term in list(year.values())[2::2] ) -def term_sums_odd(data: ValidPlannerData) -> bool: +def term_sums_odd(data: Storage) -> bool: """ Check that the sum of the course codes in odd terms is odd """ is_odd: Callable[[int], bool] = lambda x: x % 2 == 1 return all( - is_odd(sum(map(get_code, term.keys()))) - for year in data.plan - for term in year[1::2] + is_odd(sum(map(get_code, term))) + for year in data["planner"]["years"] + for term in list(year.values())[1::2] ) -def comp1511_marks(data: ValidPlannerData) -> bool: +def comp1511_marks(data: Storage) -> bool: """ Ollie must achieve a mark of 100 in COMP1511 to keep his scholarship """ - return any( - course == "COMP1511" and mark == 100 - for year in data.plan - for term in year - for (course, val) in term.items() - if val is not None and len(val) >= 2 and (mark := val[1]) is not None - ) + return "COMP1511" in data["courses"] and data["courses"]["COMP1511"]["mark"] == 100 -def gen_ed_sum(data: ValidPlannerData) -> bool: +def gen_ed_sum(data: Storage) -> bool: """ The sum of GENED course codes must not exceed 2200 """ return sum(map(get_code, gen_eds(all_courses(data)))) <= 2200 -def gen_ed_faculty(data: ValidPlannerData) -> bool: +def gen_ed_faculty(data: Storage) -> bool: """ Gen-Eds must all be from different faculties """ @@ -183,7 +177,7 @@ def gen_ed_faculty(data: ValidPlannerData) -> bool: return len(gen_eds_facs) == len(set(gen_eds_facs)) -def same_code_diff_faculty(data: ValidPlannerData) -> bool: +def same_code_diff_faculty(data: Storage) -> bool: """ Must take two courses with the same code but, from different faculties """ @@ -192,15 +186,15 @@ def same_code_diff_faculty(data: ValidPlannerData) -> bool: return len(codes) != len(set(codes)) -def math_limit(data: ValidPlannerData) -> bool: +def math_limit(data: Storage) -> bool: """ In your N-th year, you can only take N + 1 math courses """ - for i, year in enumerate(data.plan, 1): + for i, year in enumerate(data["planner"]["years"], 1): # Use sum(1, ...) instead of len to avoid dual allocation num_math = sum( 1 - for term in year + for term in year.values() for course in term if course.startswith("MATH") ) @@ -209,7 +203,7 @@ def math_limit(data: ValidPlannerData) -> bool: return True -def six_threes_limit(data: ValidPlannerData) -> bool: +def six_threes_limit(data: Storage) -> bool: """ There can by at most 6 occurrences of the number 3 in the entire planner @@ -217,18 +211,20 @@ def six_threes_limit(data: ValidPlannerData) -> bool: all_codes = "".join(str(get_code(course)) for course in all_courses(data)) return all_codes.count("3") <= 6 -def comp1531_third_year(data: ValidPlannerData) -> bool: +def comp1531_third_year(data: Storage) -> bool: """ COMP1531 must be taken in the third year """ - third_year = data.plan[2] + if len(data["planner"]["years"]) < 3: + return False + + third_year = data["planner"]["years"][2] return any( - course == "COMP1531" - for term in third_year - for course in term + "COMP1531" in term + for term in third_year.values() ) -ValidatorFn = Callable[[ValidPlannerData], bool] +ValidatorFn = Callable[[Storage], bool] ObjectiveMessage = str Flag = str requirements: list[tuple[ValidatorFn, ObjectiveMessage, Optional[Flag]]] = [ @@ -255,7 +251,7 @@ def validate_ctf(uid: Annotated[str, Security(require_uid)]): Validates the CTF """ - data = convert_to_planner_data(get_setup_user(uid)) + data = get_setup_user(uid) passed: list[str] = [] flags: list[str] = [] for req_num, (fn, msg, flag) in enumerate(requirements): From 5cc88707b27a453f137a42b271308c72b82898bd Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Mon, 9 Sep 2024 01:36:27 +1000 Subject: [PATCH 13/17] feat: convert /validateTermPlanner to no longer use old models --- backend/algorithms/tests/test_autoplanning.py | 12 +++---- backend/algorithms/validate_term_planner.py | 29 ++++++++------- backend/server/routers/planner.py | 17 +++++---- backend/server/routers/utility/user.py | 36 +++++++++---------- 4 files changed, 49 insertions(+), 45 deletions(-) diff --git a/backend/algorithms/tests/test_autoplanning.py b/backend/algorithms/tests/test_autoplanning.py index 3eb509ed4..2c0bf3c08 100644 --- a/backend/algorithms/tests/test_autoplanning.py +++ b/backend/algorithms/tests/test_autoplanning.py @@ -3,9 +3,9 @@ from algorithms.autoplanning import autoplan, terms_between from algorithms.objects.course import Course from algorithms.objects.user import User -from algorithms.validate_term_planner import validate_terms +from algorithms.validate_term_planner import RawUserPlan, validate_terms from pytest import raises -from server.routers.model import CONDITIONS, ValidPlannerData +from server.routers.model import CONDITIONS def get_uoc(course_name: str, courses: list[Course]): @@ -132,11 +132,11 @@ def assert_autoplanning_guarantees(uoc_max: list[int], courses: list[Course], pr course_names = [course[0] for course in res if terms_between((2020, 0), course[1]) == index] assert number >= sum(get_uoc(course_name, courses) for course_name in course_names) # all courses valid - plan: list[list[dict[str, tuple[int, Optional[int]]]]] = [[{}, {}, {}, {}] for _ in range(2023-2020)] + plan: RawUserPlan = [[{}, {}, {}, {}] for _ in range(2023-2020)] for course_name, (course_year, course_term) in res: plan[course_year - 2020][course_term][course_name] = (get_uoc(course_name, courses), get_mark(course_name, courses)) - assert all(course_state['is_accurate'] for course_state in validate_terms(ValidPlannerData( + assert all(course_state.is_accurate for course_state in validate_terms( programCode="3778", - specialisations=["COMPA1"], + specs=["COMPA1"], plan=plan - )).values()) + ).values()) diff --git a/backend/algorithms/validate_term_planner.py b/backend/algorithms/validate_term_planner.py index 5054393a8..5c8705008 100644 --- a/backend/algorithms/validate_term_planner.py +++ b/backend/algorithms/validate_term_planner.py @@ -1,18 +1,22 @@ +from typing import Optional from algorithms.objects.user import User -from server.routers.model import CACHED_HANDBOOK_NOTE, CONDITIONS, ValidPlannerData +from server.routers.model import CACHED_HANDBOOK_NOTE, CONDITIONS, CourseState -def validate_terms(data: ValidPlannerData): +type RawUserPlan = list[list[dict[str, tuple[int, Optional[int]]]]] + +def validate_terms(programCode: str, specs: list[str], plan: RawUserPlan) -> dict[str, CourseState]: + """Validates the term planner, returning all warnings.""" emptyUserData = { - "program": data.programCode, - "specialisations": data.specialisations, + "program": programCode, + "specialisations": specs[:], "courses": {}, # Start off the user with an empty year } user = User(emptyUserData) # State of courses on the term planner - coursesState = {} + coursesState: dict[str, CourseState] = {} - for year in data.plan: + for year in plan: # Go through all the years for term in year: user.add_current_courses(term) @@ -24,12 +28,13 @@ def validate_terms(data: ValidPlannerData): if is_answer_accurate else (True, []) ) - coursesState[course] = { - "is_accurate": is_answer_accurate, - "handbook_note": CACHED_HANDBOOK_NOTE.get(course, ""), - "unlocked": unlocked, - "warnings": warnings - } + coursesState[course] = CourseState( + is_accurate=is_answer_accurate, + handbook_note=CACHED_HANDBOOK_NOTE.get(course, ""), + unlocked=unlocked, + warnings=warnings + ) + # Add all these courses to the user in preparation for the next term user.empty_current_courses() user.add_courses(term) diff --git a/backend/server/routers/planner.py b/backend/server/routers/planner.py index ed2854637..68516f29a 100644 --- a/backend/server/routers/planner.py +++ b/backend/server/routers/planner.py @@ -11,8 +11,8 @@ from algorithms.validate_term_planner import validate_terms from fastapi import APIRouter, HTTPException, Security, UploadFile from server.routers.utility.sessions.middleware import HTTPBearerToUserID -from server.routers.utility.user import convert_to_planner_data, get_setup_user, set_user, user_storage_to_algo_user -from server.routers.model import CourseCode, PlannedToTerm, ProgramTime, UnPlannedToTerm, ValidCoursesState +from server.routers.utility.user import get_setup_user, set_user, user_storage_to_algo_user, user_storage_to_raw_plan +from server.routers.model import CourseCode, CoursesState, PlannedToTerm, ProgramTime, UnPlannedToTerm from server.routers.utility.common import get_course_details, get_course_object MIN_COMPLETED_COURSE_UOC = 6 @@ -30,8 +30,8 @@ def planner_index() -> str: return "Index of planner" -@router.get("/validateTermPlanner", response_model=ValidCoursesState) -def validate_term_planner(uid: Annotated[str, Security(require_uid)]): +@router.get("/validateTermPlanner", response_model=CoursesState) +def validate_term_planner(uid: Annotated[str, Security(require_uid)]) -> CoursesState: """ Will iteratively go through the term planner data whilst iteratively filling the user with courses. @@ -39,9 +39,12 @@ def validate_term_planner(uid: Annotated[str, Security(require_uid)]): Returns the state of all the courses on the term planner """ user = get_setup_user(uid) - data = convert_to_planner_data(user) - coursesState = validate_terms(data) - return {"courses_state": coursesState} + courses_state = validate_terms( + programCode=user['degree']['programCode'], + specs=user['degree']['specs'], + plan=user_storage_to_raw_plan(user) + ) + return CoursesState(courses_state=courses_state) @router.post("/addToUnplanned") diff --git a/backend/server/routers/utility/user.py b/backend/server/routers/utility/user.py index 7dd452000..6172369d1 100644 --- a/backend/server/routers/utility/user.py +++ b/backend/server/routers/utility/user.py @@ -4,7 +4,7 @@ from algorithms.objects.user import UserJSON, User from server.routers.utility.common import get_core_courses, get_course_details -from server.routers.model import CourseStorage, Mark, SettingsStorage, DegreeLocalStorage, PlannerLocalStorage, Storage, ValidPlannerData, markMap +from server.routers.model import CourseStorage, Mark, SettingsStorage, DegreeLocalStorage, PlannerLocalStorage, Storage import server.db.helpers.users as udb from server.db.helpers.models import PartialUserStorage, UserStorage as NEWUserStorage, UserDegreeStorage as NEWUserDegreeStorage, UserPlannerStorage as NEWUserPlannerStorage, UserCoursesStorage as NEWUserCoursesStorage, UserCourseStorage as NEWUserCourseStorage, UserSettingsStorage as NEWUserSettingsStorage @@ -139,22 +139,18 @@ def user_storage_to_algo_user(user: Storage) -> User: algo_user.load_json(user_data) return algo_user -def convert_to_planner_data(user: Storage) -> ValidPlannerData: - """ fixes the planner data to add missing UOC info """ - plan: list[list[dict[str, tuple[int, Optional[int]]]]] = [] - for year_index, year in enumerate(user['planner']['years']): - plan.append([]) - for term_index, term in enumerate(year.values()): - plan[year_index].append({}) - for courseName in term: - c = user['courses'][courseName] - mark = c['mark'] - if not isinstance(mark, int) and mark is not None: - mark = markMap.get(mark, None) - plan[year_index][term_index][courseName] = ( - int(c["uoc"]), mark) - return ValidPlannerData( - programCode=user['degree']['programCode'], - specialisations=user['degree']['specs'], - plan=plan, - ) +def user_storage_to_raw_plan(user: Storage) -> list[list[dict[str, tuple[int, Optional[int]]]]]: + '''Attaches UOC and marks to the user's planner''' + return [ + [ + { + courseCode: ( + user['courses'][courseCode]['uoc'], + parse_mark_to_int(user['courses'][courseCode]['mark']) + ) + for courseCode in year[term] + } + for term in ['T0', 'T1', 'T2', 'T3'] + ] + for year in user['planner']['years'] + ] From 9014b28d16d00d30f37603d10c42ca515e98eba2 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Mon, 9 Sep 2024 01:48:26 +1000 Subject: [PATCH 14/17] fix: update frontend mark parsing to match backend --- backend/server/routers/courses.py | 1 - backend/server/routers/model.py | 33 ------------------------- backend/server/routers/utility/user.py | 1 - frontend/src/pages/TermPlanner/utils.ts | 2 +- 4 files changed, 1 insertion(+), 36 deletions(-) diff --git a/backend/server/routers/courses.py b/backend/server/routers/courses.py index 7121df9b4..74e4966ae 100644 --- a/backend/server/routers/courses.py +++ b/backend/server/routers/courses.py @@ -57,7 +57,6 @@ def fetch_all_courses() -> Dict[str, str]: return ALL_COURSES -# TODO-OLLI: move this out def get_all_course_states_for_user(user: User) -> dict[str, CourseState]: coursesState: dict[str, CourseState] = {} diff --git a/backend/server/routers/model.py b/backend/server/routers/model.py index f18231b71..730843dcd 100644 --- a/backend/server/routers/model.py +++ b/backend/server/routers/model.py @@ -81,27 +81,12 @@ class CourseState(BaseModel): warnings: list -class ValidCourseState(BaseModel): - model_config = ConfigDict(extra='forbid') - - is_accurate: bool - unlocked: bool - handbook_note: str - warnings: list - - class CoursesState(BaseModel): model_config = ConfigDict(extra='forbid') courses_state: dict[str, CourseState] = {} -class ValidCoursesState(BaseModel): - model_config = ConfigDict(extra='forbid') - - courses_state: dict[str, ValidCourseState] = {} - - class CoursesUnlockedWhenTaken (BaseModel): model_config = ConfigDict(extra='forbid') @@ -124,15 +109,6 @@ class CoursesTypeState(BaseModel): courses_state: dict[str, CourseTypeState] = {} -# TODO: this is only used in ctf.py and the validatePlanner route... -# Shuffle those to use the user objects directly, then we can delete this and the `convert_to_planner_data` function -# TODO-OLLI: remove -class ValidPlannerData(BaseModel): - model_config = ConfigDict(extra='forbid') - - programCode: str - specialisations: list[str] - plan: list[list[dict[str, tuple[int, Optional[int]]]]] # TODO-OLLI(pm): get rid of these user models in favour of the database models @with_config(ConfigDict(extra='forbid')) @@ -151,15 +127,6 @@ class PlannerLocalStorage(TypedDict): LetterGrade = Literal['SY', 'FL', 'PS', 'CR', 'DN', 'HD'] Mark = Optional[int | LetterGrade] -markMap = { - "SY": 50, - "FL": 25, - "PS": 50, - "CR": 65, - "DN": 75, - "HD": 85, -} - @with_config(ConfigDict(extra='forbid')) class CourseStorage(TypedDict): code: str diff --git a/backend/server/routers/utility/user.py b/backend/server/routers/utility/user.py index 6172369d1..504836e8e 100644 --- a/backend/server/routers/utility/user.py +++ b/backend/server/routers/utility/user.py @@ -98,7 +98,6 @@ def set_user(uid: str, item: Storage, overwrite: bool = False): def parse_mark_to_int(mark: Mark) -> Optional[int]: '''Converts the stored mark into a number grade for validation''' # https://www.student.unsw.edu.au/wam - # TODO-OLLI: unify this with convert_to_planner_data match mark: case int() as n if 0 <= n <= 100: return n diff --git a/frontend/src/pages/TermPlanner/utils.ts b/frontend/src/pages/TermPlanner/utils.ts index a1b1f377d..cc943eba5 100644 --- a/frontend/src/pages/TermPlanner/utils.ts +++ b/frontend/src/pages/TermPlanner/utils.ts @@ -8,7 +8,7 @@ const parseMarkToInt = (mark: Mark): number | null => { const letterGradeToIntMap: Record = { SY: null, FL: 25, - PS: 60, + PS: 55, CR: 70, DN: 80, HD: 90 From 0fa26a70694767ea30c03172815b8a9f6686f528 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Mon, 9 Sep 2024 01:54:26 +1000 Subject: [PATCH 15/17] fix: re-enable circular imports pylint and remove type keyword --- .pylintrc | 2 -- backend/algorithms/validate_term_planner.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.pylintrc b/.pylintrc index c22418279..7d8adad96 100644 --- a/.pylintrc +++ b/.pylintrc @@ -79,8 +79,6 @@ disable=import-error, # allow import from folder.file instea missing-module-docstring, # allow for missing module docstrings, democratially ignored missing-function-docstring, # TODO: be much more selective with these, and docstring most functions missing-class-docstring, # - import-outside-toplevel, # allow non-top level imports. used to prevent circular dependencies - cyclic-import, # TODO: remove these when utility refactor + old user storage dicts removal # parameter-unpacking, # unpacking-in-except, # old-raise-syntax, diff --git a/backend/algorithms/validate_term_planner.py b/backend/algorithms/validate_term_planner.py index 5c8705008..0831a8311 100644 --- a/backend/algorithms/validate_term_planner.py +++ b/backend/algorithms/validate_term_planner.py @@ -3,7 +3,7 @@ from server.routers.model import CACHED_HANDBOOK_NOTE, CONDITIONS, CourseState -type RawUserPlan = list[list[dict[str, tuple[int, Optional[int]]]]] +RawUserPlan = list[list[dict[str, tuple[int, Optional[int]]]]] def validate_terms(programCode: str, specs: list[str], plan: RawUserPlan) -> dict[str, CourseState]: """Validates the term planner, returning all warnings.""" From 75ead8ff41fecdd2ead632e8aa1b245b9613da36 Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Mon, 9 Sep 2024 02:08:28 +1000 Subject: [PATCH 16/17] fix: remove potentially bad load_dotenv --- backend/server/routers/utility/oidc/constants.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/backend/server/routers/utility/oidc/constants.py b/backend/server/routers/utility/oidc/constants.py index a7e2188a0..f7174ae30 100644 --- a/backend/server/routers/utility/oidc/constants.py +++ b/backend/server/routers/utility/oidc/constants.py @@ -1,8 +1,4 @@ import os -from dotenv import load_dotenv - - -load_dotenv("./env/backend.env") CLIENT_ID = os.getenv("AUTH_CSE_CLIENT_ID") CLIENT_SECRET = os.getenv("AUTH_CSE_CLIENT_SECRET") From cedc5b3f6f2e02f4c31d18965de5d80f2415342a Mon Sep 17 00:00:00 2001 From: olli <80164276+ollibowers@users.noreply.github.com> Date: Mon, 9 Sep 2024 02:19:51 +1000 Subject: [PATCH 17/17] fix: remove unused props for CourseDescriptionPanel and CourseMenu after tokenifying last routes --- .../CourseDescriptionPanel.tsx | 8 ++------ .../CourseSelector/CourseMenu/CourseMenu.tsx | 5 ++--- .../src/pages/CourseSelector/CourseSelector.tsx | 15 ++------------- 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/frontend/src/components/CourseDescriptionPanel/CourseDescriptionPanel.tsx b/frontend/src/components/CourseDescriptionPanel/CourseDescriptionPanel.tsx index 3202c5b66..2505a70d9 100644 --- a/frontend/src/components/CourseDescriptionPanel/CourseDescriptionPanel.tsx +++ b/frontend/src/components/CourseDescriptionPanel/CourseDescriptionPanel.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { useLocation } from 'react-router-dom'; import { useQuery } from '@tanstack/react-query'; import { Typography } from 'antd'; -import { CoursesResponse, DegreeResponse, PlannerResponse } from 'types/userResponse'; +import { CoursesResponse } from 'types/userResponse'; import { getCourseInfo, getCoursePrereqs, getCoursesUnlockedWhenTaken } from 'utils/api/coursesApi'; import { getCourseTimetable } from 'utils/api/timetableApi'; import getEnrolmentCapacity from 'utils/getEnrolmentCapacity'; @@ -31,18 +31,14 @@ type CourseDescriptionPanelProps = { className?: string; courseCode: string; onCourseClick?: (code: string) => void; - planner?: PlannerResponse; courses?: CoursesResponse; - degree?: DegreeResponse; }; const CourseDescriptionPanel = ({ className, courseCode, onCourseClick, - planner, - courses, - degree + courses }: CourseDescriptionPanelProps) => { const token = useToken(); diff --git a/frontend/src/pages/CourseSelector/CourseMenu/CourseMenu.tsx b/frontend/src/pages/CourseSelector/CourseMenu/CourseMenu.tsx index 0e82ee4c9..46311f187 100644 --- a/frontend/src/pages/CourseSelector/CourseMenu/CourseMenu.tsx +++ b/frontend/src/pages/CourseSelector/CourseMenu/CourseMenu.tsx @@ -5,7 +5,7 @@ import type { MenuProps } from 'antd'; import { CourseUnitsStructure, MenuDataStructure, MenuDataSubgroup } from 'types/courseMenu'; import { CourseValidation } from 'types/courses'; import { ProgramStructure } from 'types/structure'; -import { CoursesResponse, DegreeResponse, PlannerResponse } from 'types/userResponse'; +import { CoursesResponse, DegreeResponse } from 'types/userResponse'; import { getAllUnlockedCourses } from 'utils/api/coursesApi'; import { addToUnplanned, removeCourse } from 'utils/api/plannerApi'; import { getProgramStructure } from 'utils/api/programsApi'; @@ -24,7 +24,6 @@ type SubgroupTitleProps = { }; type CourseMenuProps = { - planner?: PlannerResponse; courses?: CoursesResponse; degree?: DegreeResponse; }; @@ -38,7 +37,7 @@ const SubgroupTitle = ({ title, currUOC, totalUOC }: SubgroupTitleProps) => ( ); -const CourseMenu = ({ planner, courses, degree }: CourseMenuProps) => { +const CourseMenu = ({ courses, degree }: CourseMenuProps) => { const token = useToken(); const inPlanner = (courseId: string) => courses && !!courses[courseId]; diff --git a/frontend/src/pages/CourseSelector/CourseSelector.tsx b/frontend/src/pages/CourseSelector/CourseSelector.tsx index 31c27d51b..0c7674e58 100644 --- a/frontend/src/pages/CourseSelector/CourseSelector.tsx +++ b/frontend/src/pages/CourseSelector/CourseSelector.tsx @@ -1,7 +1,7 @@ import React, { useCallback, useEffect, useRef, useState } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { useQuery } from '@tanstack/react-query'; -import { getUserCourses, getUserDegree, getUserPlanner } from 'utils/api/userApi'; +import { getUserCourses, getUserDegree } from 'utils/api/userApi'; import openNotification from 'utils/openNotification'; import infographic from 'assets/infographicFontIndependent.svg'; import CourseDescriptionPanel from 'components/CourseDescriptionPanel'; @@ -17,11 +17,6 @@ import S from './styles'; const CourseSelector = () => { const token = useToken(); - const plannerQuery = useQuery({ - queryKey: ['planner'], - queryFn: () => getUserPlanner(token) - }); - const coursesQuery = useQuery({ queryKey: ['courses'], queryFn: () => getUserCourses(token) @@ -88,19 +83,13 @@ const CourseSelector = () => { - + {courseCode ? (