Skip to content

[STASHED] WIP: Implement Course Registration tests and refactor its code

Important

Urgent This MR depends on !156 (merged) after which the commit tagged [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 (???)

Testing To-Do: #189 (closed)

  • 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

Refactoring To-Do: #111 (closed)

  • Change name to alternateName 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:

Core changes

  • Use sessionStorage as much as possible:
    • Lowers API calls to just one for most cases
    • Underlined in red or green are objects only fetched once. It used to be that the one in green was first fetched fully (10kb) and then every other time partially (1.5kb) but this is no longer the case
      Screenshot_20190809_151435
  • Registration process overhaul:
    • Changed route to default to checkout instead of overview Use 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)
      Peek_2019-08-21_11-31
    • Hide button when registration has been submitted successfully or when registration contains the same items as one already submitted (see gif below)
      no_edit
  • Minor translation fixes by using alternativeName #183 (closed) Screenshot_20190809_183603
    Screenshot_20190809_183536
  • Allows empty registration and shows relevant message #144 (closed) Screenshot_20190815_102300 Screenshot_20190815_102317
  • Allows user to skip specialty selection #176 (closed) Screenshot_20190821_171204
  • 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)
Edited by George Katsikas

Merge request reports