From 7d1cea15c1b893b91f3e3894b881ba97677ca550 Mon Sep 17 00:00:00 2001 From: Lucas Date: Sun, 18 Aug 2024 14:10:26 +1000 Subject: [PATCH] Separate backend and frontend settings --- backend/server/db/helpers/models.py | 5 ++ backend/server/db/helpers/users.py | 21 ++++++-- backend/server/db/mongo/col_types.py | 4 ++ backend/server/db/mongo/setup.py | 13 ++++- backend/server/routers/model.py | 6 +++ backend/server/routers/user.py | 28 +++++++++-- frontend/src/App.tsx | 4 +- .../FeedbackButton/FeedbackButton.tsx | 1 + frontend/src/hooks/useSettings.ts | 50 +++++++++++++++---- .../CourseTabs/CourseTabs.test.tsx | 1 - frontend/src/reducers/settingsSlice.ts | 10 +--- frontend/src/test/testUtil.tsx | 2 - frontend/src/types/userResponse.ts | 4 ++ frontend/src/utils/api/userApi.ts | 19 ++++++- 14 files changed, 136 insertions(+), 32 deletions(-) diff --git a/backend/server/db/helpers/models.py b/backend/server/db/helpers/models.py index 83a5a2ab6..061fd600e 100644 --- a/backend/server/db/helpers/models.py +++ b/backend/server/db/helpers/models.py @@ -39,6 +39,9 @@ class UserPlannerStorage(BaseModel): isSummerEnabled: bool years: List[PlannerYear] lockedTerms: Dict[str, bool] + +class UserSettingsStorage(BaseModel): + showMarks: bool class _BaseUserStorage(BaseModel): # NOTE: could also put uid here if we want @@ -49,6 +52,7 @@ class UserStorage(_BaseUserStorage): degree: UserDegreeStorage courses: UserCoursesStorage planner: UserPlannerStorage + settings: UserSettingsStorage class NotSetupUserStorage(_BaseUserStorage): setup: Literal[False] = False @@ -60,6 +64,7 @@ class PartialUserStorage(BaseModel): degree: Optional[UserDegreeStorage] = None courses: Optional[UserCoursesStorage] = None planner: Optional[UserPlannerStorage] = None + settings: Optional[UserSettingsStorage] = None # # Session Token Models (redis) diff --git a/backend/server/db/helpers/users.py b/backend/server/db/helpers/users.py index af87d8a1f..a41da8cda 100644 --- a/backend/server/db/helpers/users.py +++ b/backend/server/db/helpers/users.py @@ -6,7 +6,7 @@ from server.db.mongo.constants import UID_INDEX_NAME from server.db.mongo.conn import usersCOL -from .models import NotSetupUserStorage, PartialUserStorage, UserCoursesStorage, UserDegreeStorage, UserPlannerStorage, UserStorage +from .models import NotSetupUserStorage, PartialUserStorage, UserCoursesStorage, UserDegreeStorage, UserPlannerStorage, UserSettingsStorage, UserStorage # TODO-OLLI(pm): decide if we want to remove type ignores by constructing dictionaries manually @@ -121,14 +121,29 @@ def update_user_planner(uid: str, data: UserPlannerStorage) -> bool: return res.matched_count == 1 +def update_user_settings(uid: str, data: UserSettingsStorage) -> bool: + res = usersCOL.update_one( + { "uid": uid, "setup": True }, + { + "$set": { + "settings": data.model_dump(), + }, + }, + upsert=False, + hint=UID_INDEX_NAME, + ) + + return res.matched_count == 1 + def update_user(uid: str, data: PartialUserStorage) -> bool: # updates certain properties of the user # if enough are given, declares it as setup + fields = { "courses", "degree", "planner", "settings" } payload = { k: v for k, v in data.model_dump( - include={ "courses", "degree", "planner" }, + include=fields, exclude_unset=True, ).items() if v is not None # cannot exclude_none since subclasses use None @@ -138,7 +153,7 @@ def update_user(uid: str, data: PartialUserStorage) -> bool: # most semantically correct return user_is_setup(uid) - if "courses" in payload and "degree" in payload and "planner" in payload: + if fields.issubset(payload.keys()): # enough to declare user as setup payload["setup"] = True diff --git a/backend/server/db/mongo/col_types.py b/backend/server/db/mongo/col_types.py index 08dd8414f..f0a30b1e2 100644 --- a/backend/server/db/mongo/col_types.py +++ b/backend/server/db/mongo/col_types.py @@ -62,6 +62,9 @@ class UserPlannerInfoDict(TypedDict): years: List[PlannerYearDict] lockedTerms: Dict[str, bool] +class UserSettingsInfoDict(TypedDict): + showMarks: bool + class UserInfoDict(TypedDict): uid: str setup: Literal[True] @@ -69,3 +72,4 @@ class UserInfoDict(TypedDict): degree: UserDegreeInfoDict courses: Dict[str, UserCourseInfoDict] planner: UserPlannerInfoDict + settings: UserSettingsInfoDict diff --git a/backend/server/db/mongo/setup.py b/backend/server/db/mongo/setup.py index b8225a533..70910ac61 100644 --- a/backend/server/db/mongo/setup.py +++ b/backend/server/db/mongo/setup.py @@ -42,7 +42,7 @@ def _create_users_collection(): }, { 'bsonType': 'object', - 'required': ['uid', 'setup', 'guest', 'degree', 'planner', 'courses'], + 'required': ['uid', 'setup', 'guest', 'degree', 'planner', 'courses', 'settings'], 'additionalProperties': False, 'properties': { '_id': { 'bsonType': 'objectId' }, @@ -159,6 +159,17 @@ def _create_users_collection(): }, } } + }, + 'settings': { + 'bsonType': 'object', + 'required': ['showMarks'], + 'additionalProperties': False, + 'properties': { + 'showMarks': { + 'bsonType': 'bool', + 'description': 'Whether to show marks in the Term Planner' + } + } } } } diff --git a/backend/server/routers/model.py b/backend/server/routers/model.py index 30f283767..2a220000f 100644 --- a/backend/server/routers/model.py +++ b/backend/server/routers/model.py @@ -248,11 +248,17 @@ class CourseStorageWithExtra(TypedDict): isMultiterm: bool ignoreFromProgression: bool +class SettingsStorage(BaseModel): + model_config = ConfigDict(extra='forbid') + + showMarks: bool + @with_config(ConfigDict(extra='forbid')) class Storage(TypedDict): degree: DegreeLocalStorage planner: PlannerLocalStorage courses: dict[str, CourseStorage] + settings: SettingsStorage class LocalStorage(BaseModel): model_config = ConfigDict(extra='forbid') diff --git a/backend/server/routers/user.py b/backend/server/routers/user.py index b624fdd59..97a3acacb 100644 --- a/backend/server/routers/user.py +++ b/backend/server/routers/user.py @@ -5,12 +5,12 @@ from server.routers.auth_utility.middleware import HTTPBearerToUserID from server.routers.courses import get_course -from server.routers.model import CourseMark, CourseStorage, DegreeLength, DegreeWizardInfo, StartYear, CourseStorageWithExtra, DegreeLocalStorage, LocalStorage, PlannerLocalStorage, Storage, SpecType +from server.routers.model import CourseMark, CourseStorage, DegreeLength, DegreeWizardInfo, SettingsStorage, StartYear, CourseStorageWithExtra, DegreeLocalStorage, LocalStorage, PlannerLocalStorage, Storage, SpecType from server.routers.programs import get_programs from server.routers.specialisations import get_specialisation_types, get_specialisations 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 +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 router = APIRouter( @@ -31,6 +31,9 @@ def _otn_degree(s: DegreeLocalStorage) -> NEWUserDegreeStorage: def _otn_courses(s: dict[str, CourseStorage]) -> NEWUserCoursesStorage: return { code: NEWUserCourseStorage.model_validate(info) for code, info in s.items() } +def _otn_settings(s: SettingsStorage) -> NEWUserSettingsStorage: + return NEWUserSettingsStorage.model_validate(s.model_dump()) + def _nto_courses(s: NEWUserCoursesStorage) -> dict[str, CourseStorage]: return { code: { @@ -60,12 +63,16 @@ def _nto_degree(s: NEWUserDegreeStorage) -> DegreeLocalStorage: 'programCode': s.programCode, 'specs': s.specs, } + +def _nto_settings(s: NEWUserSettingsStorage) -> SettingsStorage: + return SettingsStorage(showMarks=s.showMarks) def _nto_storage(s: NEWUserStorage) -> Storage: return { 'courses': _nto_courses(s.courses), 'degree': _nto_degree(s.degree), 'planner': _nto_planner(s.planner), + 'settings': _nto_settings(s.settings), } @@ -92,6 +99,7 @@ def set_user(uid: str, item: Storage, overwrite: bool = False): courses=_otn_courses(item['courses']), degree=_otn_degree(item['degree']), planner=_otn_planner(item['planner']), + settings=_otn_settings(item['settings']), )) assert res @@ -117,7 +125,8 @@ def save_local_storage(localStorage: LocalStorage, uid: Annotated[str, Security( item: Storage = { 'degree': localStorage.degree, 'planner': real_planner, - 'courses': courses + 'courses': courses, + 'settings': SettingsStorage(showMarks=False) } set_user(uid, item) @@ -174,6 +183,16 @@ def get_user_p(uid: Annotated[str, Security(require_uid)]) -> Dict[str, CourseSt return res +@router.get("/data/settings") +def get_user_settings(uid: Annotated[str, Security(require_uid)]) -> SettingsStorage: + return get_setup_user(uid)['settings'] + +@router.post("/settings/toggleShowMarks") +def toggle_show_marks(uid: Annotated[str, Security(require_uid)]): + user = get_setup_user(uid) + user['settings'].showMarks = not user['settings'].showMarks + set_user(uid, user, True) + @router.post("/toggleSummerTerm") def toggle_summer_term(uid: Annotated[str, Security(require_uid)]): user = get_setup_user(uid) @@ -339,7 +358,8 @@ def setup_degree_wizard(wizard: DegreeWizardInfo, uid: Annotated[str, Security(r 'specs': wizard.specs, }, 'planner': planner, - 'courses': {} + 'courses': {}, + 'settings': SettingsStorage(showMarks=False), } set_user(uid, user, True) return user diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index b059850ef..a3919f212 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -30,12 +30,12 @@ const ProgressionChecker = React.lazy(() => import('./pages/ProgressionChecker') const TermPlanner = React.lazy(() => import('./pages/TermPlanner')); const App = () => { - const { theme } = useSettings(); - const [queryClient] = React.useState( () => new QueryClient({ defaultOptions: { queries: { refetchOnWindowFocus: false } } }) ); + const { theme } = useSettings(queryClient); + useEffect(() => { // using local storage since I don't want to risk invalidating the redux state right now const cooldownMs = 1000 * 60 * 60 * 24 * 7; // every 7 days diff --git a/frontend/src/components/FeedbackButton/FeedbackButton.tsx b/frontend/src/components/FeedbackButton/FeedbackButton.tsx index 07029c139..49a0febdd 100644 --- a/frontend/src/components/FeedbackButton/FeedbackButton.tsx +++ b/frontend/src/components/FeedbackButton/FeedbackButton.tsx @@ -12,6 +12,7 @@ const FeedbackButton = () => { const openFeedbackLink = () => { window.open(FEEDBACK_LINK, '_blank'); }; + const { theme } = useSettings(); // Move this to the drawer if the screen is too small diff --git a/frontend/src/hooks/useSettings.ts b/frontend/src/hooks/useSettings.ts index ef74ef244..edfe968cd 100644 --- a/frontend/src/hooks/useSettings.ts +++ b/frontend/src/hooks/useSettings.ts @@ -1,38 +1,68 @@ import { useCallback } from 'react'; +import { QueryClient, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; +import { getUserSettings, toggleShowMarks as toggleShowMarksApi } from 'utils/api/userApi'; import { useAppDispatch, useAppSelector } from 'hooks'; import { toggleLockedCourses as toggleLocked, - toggleShowMarks as toggleMarks, toggleShowPastWarnings as togglePastWarnings, toggleTheme } from 'reducers/settingsSlice'; +import useToken from './useToken'; + +type Theme = 'light' | 'dark'; interface Settings { - theme: string; + theme: Theme; showMarks: boolean; showLockedCourses: boolean; showPastWarnings: boolean; - mutateTheme: (theme: 'light' | 'dark') => void; + mutateTheme: (theme: Theme) => void; toggleShowMarks: () => void; toggleLockedCourses: () => void; toggleShowPastWarnings: () => void; } -function useSettings(): Settings { - const settings = useAppSelector((state) => state.settings); +function useSettings(queryClient?: QueryClient): Settings { + const localSettings = useAppSelector((state) => state.settings); const dispatch = useAppDispatch(); - const mutateTheme = useCallback( - (theme: 'light' | 'dark') => dispatch(toggleTheme(theme)), - [dispatch] + const token = useToken({ allowUnset: true }); + const realQueryClient = useQueryClient(queryClient); + const settingsQuery = useQuery( + { + queryKey: ['settings'], + queryFn: () => getUserSettings(token!), + placeholderData: { showMarks: false }, + enabled: !!token + }, + queryClient + ); + const userSettings = settingsQuery.data ?? { + showMarks: false + }; + + const showMarksMutation = useMutation( + { + mutationFn: () => (token ? toggleShowMarksApi(token) : Promise.reject(new Error('No token'))), + onSuccess: () => { + realQueryClient.invalidateQueries({ queryKey: ['settings'] }); + }, + onError: (error) => { + // eslint-disable-next-line no-console + console.error('Error toggling show marks: ', error); + } + }, + queryClient ); - const toggleShowMarks = useCallback(() => dispatch(toggleMarks()), [dispatch]); + const mutateTheme = useCallback((theme: Theme) => dispatch(toggleTheme(theme)), [dispatch]); const toggleLockedCourses = useCallback(() => dispatch(toggleLocked()), [dispatch]); const toggleShowPastWarnings = useCallback(() => dispatch(togglePastWarnings()), [dispatch]); + const toggleShowMarks = showMarksMutation.mutate; return { - ...settings, + ...userSettings, + ...localSettings, mutateTheme, toggleShowMarks, toggleLockedCourses, diff --git a/frontend/src/pages/CourseSelector/CourseTabs/CourseTabs.test.tsx b/frontend/src/pages/CourseSelector/CourseTabs/CourseTabs.test.tsx index 7f1fba567..67a459114 100644 --- a/frontend/src/pages/CourseSelector/CourseTabs/CourseTabs.test.tsx +++ b/frontend/src/pages/CourseSelector/CourseTabs/CourseTabs.test.tsx @@ -13,7 +13,6 @@ const preloadedState: RootState = { }, settings: { theme: 'dark', - showMarks: false, showLockedCourses: false, showPastWarnings: false }, diff --git a/frontend/src/reducers/settingsSlice.ts b/frontend/src/reducers/settingsSlice.ts index 545d2524c..29b625aa8 100644 --- a/frontend/src/reducers/settingsSlice.ts +++ b/frontend/src/reducers/settingsSlice.ts @@ -4,15 +4,13 @@ import { createSlice } from '@reduxjs/toolkit'; type Theme = 'light' | 'dark'; type SettingsSliceState = { - theme: string; - showMarks: boolean; + theme: Theme; showLockedCourses: boolean; showPastWarnings: boolean; }; export const initialSettingsState: SettingsSliceState = { theme: 'light', - showMarks: false, showLockedCourses: false, showPastWarnings: true }; @@ -24,9 +22,6 @@ const settingsSlice = createSlice({ toggleTheme: (state, action: PayloadAction) => { state.theme = action.payload; }, - toggleShowMarks: (state) => { - state.showMarks = !state.showMarks; - }, toggleLockedCourses: (state) => { state.showLockedCourses = !state.showLockedCourses; }, @@ -36,7 +31,6 @@ const settingsSlice = createSlice({ } }); -export const { toggleTheme, toggleShowMarks, toggleLockedCourses, toggleShowPastWarnings } = - settingsSlice.actions; +export const { toggleTheme, toggleLockedCourses, toggleShowPastWarnings } = settingsSlice.actions; export default settingsSlice.reducer; diff --git a/frontend/src/test/testUtil.tsx b/frontend/src/test/testUtil.tsx index 60a80a3e0..e9362cbe5 100644 --- a/frontend/src/test/testUtil.tsx +++ b/frontend/src/test/testUtil.tsx @@ -26,7 +26,6 @@ export const renderWithProviders = async ( }, settings: { theme: 'dark', - showMarks: false, showLockedCourses: false, showPastWarnings: false }, @@ -46,7 +45,6 @@ export const renderWithProviders = async ( settings: { theme: 'dark', showLockedCourses: true, - showMarks: true, showPastWarnings: true, token: 'token' // force token to be dummy } diff --git a/frontend/src/types/userResponse.ts b/frontend/src/types/userResponse.ts index 06e4d1e36..deb27490c 100644 --- a/frontend/src/types/userResponse.ts +++ b/frontend/src/types/userResponse.ts @@ -24,6 +24,10 @@ export type CourseResponse = { }; export type CoursesResponse = Record; +export type SettingsResponse = { + showMarks: boolean; +}; + export type ValidateResponse = { is_accurate: boolean; handbook_note: string; diff --git a/frontend/src/utils/api/userApi.ts b/frontend/src/utils/api/userApi.ts index 6e44237a4..170118494 100644 --- a/frontend/src/utils/api/userApi.ts +++ b/frontend/src/utils/api/userApi.ts @@ -1,5 +1,11 @@ import axios from 'axios'; -import { CoursesResponse, DegreeResponse, PlannerResponse, UserResponse } from 'types/userResponse'; +import { + CoursesResponse, + DegreeResponse, + PlannerResponse, + SettingsResponse, + UserResponse +} from 'types/userResponse'; import { withAuthorization } from './authApi'; export const getUser = async (token: string): Promise => { @@ -34,6 +40,17 @@ export const getUserCourses = async (token: string): Promise => return courses.data as CoursesResponse; }; +export const getUserSettings = async (token: string): Promise => { + const settings = await axios.get(`user/data/settings`, { + headers: withAuthorization(token) + }); + return settings.data as SettingsResponse; +}; + +export const toggleShowMarks = async (token: string): Promise => { + await axios.post(`user/settings/toggleShowMarks`, {}, { headers: withAuthorization(token) }); +}; + export const resetUserDegree = async (token: string): Promise => { await axios.post(`user/reset`, {}, { headers: withAuthorization(token) }); };