Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Course Management System Major Updates #45

Merged
merged 27 commits into from
Apr 10, 2021

Conversation

will-r-wang
Copy link
Owner

Closes: #23 #15 #43 #16

What this PR accomplishes

Not easy to describe everything in this PR - you'll have to tophat this yourself commit by commit

Before you deploy

  • I have tophatted or tested this change.
  • I have requested my team members for review and received one approval.

This fixes a previously broken association since the StudentCourse table
was dropped and removes two of the helper methods in
application_controller in exchange for adding two instance methods on
the Course model.

Note: StudentCourse was replaced with CourseRegistration in a previous
commit.
This fixes up the course_registration association on the Student model
to instead be user_id since we're using single table inheritance. Also
adds the student course view whereby students can see a list of all
their currently registered courses and click to view course.
Having helper_method in application_controller is a bit of a code smell
following Rails conventions that we want to avoid in the long run. This
moves all of the helper methods into the ApplicationHelper and includes
it in the controller instead.

Also replaces some of the unnecessary helpers with the direct reference
and deletes the empty helpers.
This restyles the css property of link to stay blue after click so that
on page reload, the link stays blue instead of greyed out. Also restyles
the h1 header in the home page to be of nil class so that the text font
will be folder. Finally, this touches up the card header wordings to a
different possessive determiner so that, as a user, the system makes
more sense to me.
This adds the where -> status: "approved" for both the course and user
model on the course_registrations relationship so that denied and
pending requests are filtered out (only one approved per User<>Course).

Also adds a mocked final_grade helper for the CourseRegistration model
and an active_course_registration helper on the Student model so that
finding the active CourseRegistration is easier.
This adds logic that allows a teacher to edit the course upon login.
As our application_helper grew bloated and the assets started to
increase, we noticed a sizeable delay on page load for the home and
courses show pages. This PR addresses some of the performance issues in
the following manners:

- Preloading through ActiveRecord using :includes to solve N+1 queries
- Setting assets.debug to false for faster view rendering
- Removing all helper functions that were only used in select pages and
replacing them with simple conditional logic
- Rendering partials with locals to prevent repeated function calls
- Deleting unused css files and precompiling application.scss

The combined changes brought page loading times from 3s to <0.1s 🎉
Since we are using single table inheritance on the User model, the
inherited tables aren't being used (we simply store a type column in the
User's table to get the correct subclass type). We drop all of the three
tables in this commit.
This makes the relevant schema changes, updates the abstract_factory
diagram, and unsets the abstract_class=true in the models for
Deliverable so that we're ready to implement a working Abstract Factory
Pattern. Using a combination of multiple and single table inheritance,
we set Activity to be an abstract class (MTI) then have our subclasses
Deliverable and Resource be concrete and store the subtypes as a column.
I used boxy.svg to combine the three emojis 📖, 🍁, and 🛡 with colo
alterations and minor warping so that the final design matches somewhat
that of cuLearn. © CMSLearn > cuLearn!
This adds the instructions and description models to the deliverables
and resources tables respectively. These two fields will be optional but
helpful to give our Course Management System a bit more functionality.
This adds the grade and comments columns to our submissions table and
renames the final_grade column in the course_registration table to be
just grade instead. These fields will be optional and useful later on
w.r.t our Visitor Pattern.
This moves the factories into the root app/services subfolder and
adds the newly created factories to be autoloaded in zeitwork mode
so that we can access their functionality directly in the controllers.
This updates the course and deliverable model to include or rename the
has_many associations based on our latest updates and adds the missing
recently created three Resource subclasses (Video, Url, and Document).
This renames Assignment / Tutorial which are not Resources
(Deliverables) to be Video / Document respectively instead. Made a small
typo when we first implemented this pattern.
Since a deliverable belongs_to a given course (meaning it's mandatory)
and we don't have a way of passing in the param when we make the GET to
the deliverables controller, this sets the current_course id in the
session[:course_id] param so that we have a way of references where
deliverables were called from.

context ^
Note:* This definitely runs into some race conditions that we didn't
have time to address in this project but we considered the following
errored paths:
  - Race condition when a user navigates from two different courses at
  the same time, with the second session variable overriding the first
  - When navigating to deliverables_controller without having the
  request originate from courses_controller, this will throw a nil
  exception
  - When a course is deleted, if the reference to session is left
  unchanged, we have a RecordNotFound error

Note:* Deliverables and resources are mutually exchangeable in this
This adds the deliverable and resource controllers along with their
respective CRUD actions to allow for the creation, updating, reading and
deleting of deliverables/resources by authorized users. By Rails' STI
conventions the subclass controllers are delegated to the superclass on
the basis that there exists a type column on the parent class table
model.

Additionally includes the relevant views corresponding to the new, edit,
and show actions.
One caveat of using Single Table Inheritance in Rails is that the
specific subtype of object will be lost on edit/update (the default
parent class will always be used) meaning that the param is missing
exception will raise every time. This adds a before_action callback that
catches this behaviour by verifying if a key with value exists already
in the params and replaces the default parent class hash values with
that value. Essentially fixes the update functionality for all users,
resources, and deliverables.
Originally, the nav user profile dropdown was rendering blank links.
This creates the standard link_to edit_profile_path that users would
typically expect when clicking on the section in the nav.
This is one of two last major parts of this issue - allowing teachers
to edit their courses (linking out to the correct controllers) and allowing
students view resources/deliverables. This hooks up all the FED/BD work.
@will-r-wang will-r-wang linked an issue Apr 10, 2021 that may be closed by this pull request
5 tasks
Copy link
Collaborator

@alexatshopify alexatshopify left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work! 🚀 🚢

@will-r-wang will-r-wang merged commit 7e2cdd9 into main Apr 10, 2021
@will-r-wang will-r-wang deleted the add-deliverable-submission-functionality branch April 10, 2021 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants