[STASHED] WIP: Implement Course Registration tests and refactor its code
Important
Urgent [DROP]
will be redundant.
Needs Feedback This MR introduces many changes. Should commits relevant to fixing #183 (closed) and #144 (closed) be separate MRs? @differentreality
What does this MR do?
This MR implements tests for the procedure of course registration and as such finds problems with the current code. The code is then refactored to fix these issues.
Status:
-
Testing: COMPLETED - Testing is done with Jasmine spies and NOT with ApiTestingModule
- Code readability may suffer, however we cannot test function calls without spies.
- I will see about using both of them "soon"
-
Refactoring: COMPLETED (???)
#189 (closed)
Testing To-Do:-
Add responses folder to src/app/test/
-
Add parser to converted dates loaded from JSON to a Date object -
Always use async blocks to make sure fixture is stable and all data is loaded. Was this the pitfall with mockApi? -
Fix/Rethink tests after refactoring
#111 (closed)
Refactoring To-Do:-
Change name
toalternateName
found at lines 121-122 in registration-courses.component.ts #183 (closed) -
Streamline groupByType/Semester: -
Streamline course registration/removal: -
Make code look identical
-
-
Better routing without using sessionStorage: -
Remove 'edit' variable from sessionStorage -
Stop page from reloading view or data unnecessarily -
Add a tooltip/message to warn the user if they are trying to resubmit a registration with errors -
Add failsafe to prevent user from editing a registration when status is closed -
Disable registration submission if registration is unchanged
-
-
API related: -
Do not make an API call to fetch registration if period is closed -
Do not fetch groupData for type of course multiple times -
Reset available classes after specialty selection -
Remove duplicate API calls -
6 x effectiveStatus -
3 x availableClasses -
2 x availableClasses/groupby=semester -
2 x currentRegistration (???)
-
-
-
Workflow Changes: -
Allow course registration before/without specialty selection #176 (closed) -
Allow empty registration #144 (closed)
-
Core changes
- Use sessionStorage as much as possible:
-
Registration process overhaul:
-
Changed route to default to checkout instead of overviewUse overview as default but check for registration change presence to show correct view- Fewer redirects when registration period is open (which is when students will usually launch this application menu)
- Allows the user to navigate straight to editing their registration if they so desire (with a correct link)
- Gets rid of the silly routing system where we ping-pong the user between checkout and overview
- Deny access to /checkout/ by link when status is closed or when the user has to select a specialty first
- Store registration on failure instead of resetting it, but show the user what they must do (see gif below)
- Hide button when registration has been submitted successfully or when registration contains the same items as one already submitted (see gif below)
-
- Minor translation fixes by using alternativeName #183 (closed)
- Allows empty registration and shows relevant message #144 (closed)
- Allows user to skip specialty selection #176 (closed)
- Remove deprecated code such as that for specialty selection
Related issues and links
Fix #189 (closed) #111 (closed) #183 (closed) #144 (closed) #176 (closed)
Developer Checklist
-
I have successfully run the code of this merge request locally -
I have verified locally that my changes work for all necessary screen sizes -
I have tried out the changes of this MR with different users to identify bugs -
Coding is in progress, and I have marked the MR as WIP -
Coding is completed and the MR is ready for review -
My branch is up-to-date with the upstream master
branch -
My MR follows the contribution guidelines -
I have added a comment with screenshots of the code running locally
Tech review Checklist
Have you verified that what is supposed to happen, actually does, and what is not supposed to happen, indeed does not?
-
The MR accurately describes the changes and has a relevant title/description -
The MR does what it is supposed to according to its title, description and related issues/links -
I have successfully run the changes locally, and tried the new code
Overall review Checklist
-
The MR references related issues/MRs -
The MR provides links to screens and screenshots -
The commits of the MR describe the changes, have proper wording, and follow the guidelines -
I have successfully run the changes locally, and tried the new code -
The MR is ready for merge (rebased, commit squashed if needed, etc)