Skip to content
This repository has been archived by the owner on Apr 2, 2021. It is now read-only.

Add model changes for real classes #71

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Blu3spirits
Copy link
Collaborator

This is the 'staging' commit for all the work that needs to be finished
before we can implement the CourseShell model.

Notes:

  • Renamed Course model to TransferCourse
  • Changed access url to /api/transfer-courses
  • The new url endpoint will be /api/courses for the joined viewset
  • Added base model for real MTU classes
  • This WILL break deployments since the sync scripts cannot yet write to
    the correct model. The scrape script for MTU classes needs to be
    finished first.

This is the 'staging' commit for all the work that needs to be finished
before we can implement the CourseShell model.

Notes:
- Renamed Course model to TransferCourse
- Changed access url to `/api/transfer-courses`
- New url endpoint will be `/api/courses` for the joined viewset
- Added base model for real MTU classes
- This WILL break deployments since the sync scripts cannot yet write to
  the correct model. The scrape script for MTU classes needs to be
  finished first.
@Blu3spirits Blu3spirits self-assigned this Sep 6, 2020
course_gather/models.py Outdated Show resolved Hide resolved

There's only one cyber sec at MTU
MTM<================>MTM
'''
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's some additional notes about this, the main point though is that there will be data loss when we join these two data sets. Namely the strange $COURSE_CODE 1XXX courses that are pulled in from places. Along with "Unassigned transfers"

db_table = 'course_shells'


class TransferCourse(models.Model):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

None of these model fields are in any way finalized so if anyone has comments about what they could/should be lmk.

Copy link
Collaborator

@codetheweb codetheweb left a comment

Choose a reason for hiding this comment

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

I'm having some trouble understanding the Shell model. Why is it a many-to-many relation between Shell and MTUCourse?

course_gather/models.py Show resolved Hide resolved
course_gather/models.py Show resolved Hide resolved
course_gather/models.py Outdated Show resolved Hide resolved
- Fix up some documentation errors
@Blu3spirits Blu3spirits added the on hold Waiting for another issue to be finished before taking on label Sep 7, 2020
@Blu3spirits
Copy link
Collaborator Author

Usually USD values are stored in databases as cents to avoid rounding errors; but I guess it doesn't matter as much here since we're not calculating anything off them. Up to you.

I hadn't heard that before but I see the sense in it. However looking at the data MTU stores the data with only two decimals anyway so really I don't see any issues coming about from this. I do think that eventually with the cart system a "total fees" would be useful to have. If it does become an issue ever we can switch over. Or I can increase the floating-point limits.

@Blu3spirits
Copy link
Collaborator Author

That being said, would using an enum or something for this field make more sense? I guess it doesn't have to be here but we should have a static list of possible tags somewhere so we can map them to values if necessary. I.e. tags with value 'Online' we may want to display with a yellow background.

I don't really think an enum would make sense since it would require a code and database change for a new tag. With this current tag model, it's very arbitrary on purpose so that we can actually do things like labels based on location. In the sync script that feeds from the scraper we just do a update_or_create() model method that creates a new label and applies to the current object. Since that bit of if location == 'online' would have to be handled it would effectively end up being static list anyway.

I think farther down the line as well after we collect data from rate my professor we could also add a tag to show that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
on hold Waiting for another issue to be finished before taking on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants