Timetable models

[ngjunsiang]

Task

• Create models required to represent timetable data in campus.model.timetable

[lomnom]

A commit has been made containing the preliminary schema definition and its corresponding Python schema outline.

First, a few small changes have been made to the schema:

• All IDs (including primary keys) are now CampusIDs, as enforced by Model. By convention, all foreign keys therefore also point to CampusID primary keys.
• To preserve the meaning previously conveyed by integer IDs, an index field has been added to both WeekDay and TimeSlot, where 0 denotes the earliest instance and larger indices represent later instances.
• Several additional [unique] constraints have been added.

For convenience, an image of the current schema, rendered from the committed DBML using the tool referenced in the docstring of timetable.py, is shown.

Secondly, it seems that the Python schema syntax currently in use does not support specifying foreign key constraints. Perhaps it is somewhere besides constraints.py and I do not see it. While this does not prevent JOINs, it does mean that PostgreSQL cannot enforce reference validity, and intent is not communicated fully.

Do let me know if any design changes should be made at this stage. I understand the importance of addressing such issues early.

[ngjunsiang]

@lomnom Will follow up with a more detailed review and comments.

For now I just want to note that we don't actually need foreign keys for timetable import and lookup. While semantically they declare intent, functionally they work like a constraint on allowed values (like CHECK, but against entries in another table's column). For our case, they don't serve a functional purpose: the timetable is imported monolithically from an XML file, and subsequently never modified (unless/except in some special edge case). So the foreign keys will never get a chance to validate INSERTs; in fact they will make the import process more tedious, by requiring entities are inserted before relations.

Nonetheless, you pointed out a real gap in the model design, and constraints will be required for future features, such as realised lesson events. This work is scheduled for after timetable launch.

[ngjunsiang]

@nyjc-computing/collaborators Pinging you to demonstrate a workflow you can use.

Here we have a conundrum:

1. @lomnom made a commit that's pretty important (model may need to be referenced in multiple places), doesn't cause tests to fail, and can be committed to weekly directly actually
2. He could have done this in a separate feature branch, but then the commit won't be useable by other collaborators on the same branch (Savio & Matthew) until PR is reviewed and merged.
3. At the same time, he can't create a PR from this branch (https://github.com/nyjc-computing/campus/tree/205-timetable-resource), because what if Savio and Matthew begin work on the branch and make pushes to it? Those changes will show up in the PR.

The resolution:

1. Create a feature branch from https://github.com/nyjc-computing/campus/tree/205-timetable-resource (here, I created https://github.com/nyjc-computing/campus/tree/feat/timetable-model)
2. PR from the feature branch to weekly. Further commits to https://github.com/nyjc-computing/campus/tree/feat/timetable-model do not affect https://github.com/nyjc-computing/campus/tree/205-timetable-resource
3. When PR is approved, https://github.com/nyjc-computing/campus/tree/205-timetable-resource can merge changes from weekly to get the updated schema (if any further commits were made)

@lomnom please note to continue any commits on the timetable model in https://github.com/nyjc-computing/campus/tree/feat/timetable-model and not here, until the PR is merged and that branch is deleted.

[sparsetable]

Thanks for this advice, this workflow does make more sense.

I agree that foreign key constraints are not needed and may actually end up as an inconvenience for this usecase. I have no more comments, and the pull request can be merged when review passes. Thanks for your time.