feat: timetable models by ngjunsiang · Pull Request #326 · nyjc-computing/campus

ngjunsiang · 2026-02-02T01:57:10Z

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.

sparsetable · 2026-02-02T03:04:56Z

I have no further comments, this pull request can be merged when review passes 👍

ngjunsiang · 2026-02-02T09:52:54Z

@sparsetable also remove timetable.dbml; We avoid specifying explicit DB schema because we are not coupling ourselves to a specific DB type. We may use SQLite for local testing, deploy on PostGres, and if the hosting provider changes, we may find ourselves switching to MariaDB or other SQL databases. It is useful for thinking through the schema and its implications on what we can model, but we otherwise don't add it to the codebase (treat it as working, not final product)

ngjunsiang · 2026-02-02T09:54:10Z

Since this PR is closed, re-open another PR for this branch.

ngjunsiang

See code review

ngjunsiang · 2026-02-02T09:15:10Z

campus/model/timetable.py

+    """
+    Describes a day in a repeating timetable.
+    """


[C#01]
We follow PEP257 on docstring formatting; one-line docstrings should have both triple-quotes on the same line.

See https://peps.python.org/pep-0257/

Suggested change

"""

Describes a day in a repeating timetable.

"""

"""Describes a day in a repeating timetable."""

ngjunsiang · 2026-02-02T09:21:53Z

campus/model/timetable.py

+    """
+    Describes a day in a repeating timetable.
+    """
+    id: schema.CampusID = unique_field


[C#02]
While this isn't specified in Campus API/schema, it should (and I'll probably update it sometime or have one of you take it on):

We reserve Campus IDs for entities that will be queried by the API: Users, Venues, Circles, Clients, ...

These are entities that will have their own URL in the Campus API (/api/v1/users/{user_uuid})

We give them a string UUID to avoid the confusion of integer IDs: getting an ID isn't enough to tell what you're looking at--e.g. "is this User ID 1, Client ID 1, or Venue ID 1"? Campus ID is designed to tell you at a glance what you "have"

This means non-queryable entities are not required/obliged to use Campus schema, and since we are not tracking their use in the DB/API, we should avoiding creating Campus IDs unless necessary

WeekDay and TimeSlot are not Campus entities, but technically enum values. This is a common feature in many programming languages, e.g. https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/enum, but we will not force their use because enum types often have their own constraints. Here we define the model for reference, i.e. to help other readers understand what we mean by a WeekDay or TimeSlot when they encounter it in the DB or in code.

So we can stick/revert to using int IDs

Suggested change

id: schema.CampusID = unique_field

id: int

Hi Mr Ng, i realise the reason i used CampusID initially is because setting id to schema.Integer triggers a type error. Model constrains id to be id: schema.CampusID | schema.UserID.

Hi Mr Ng, i realise the reason i used CampusID initially is because setting id to schema.Integer triggers a type error. Model constrains id to be id: schema.CampusID | schema.UserID.

@sparsetable I forgot to mention: for "internal representations" (won't be queried directly through Campus API), there is no need to inherit base.Model. We use that primarily to enforce that all public Campus models have an id and created_at property polymorphically.

Internal representations will never be returned by the resource/API, just used as intermediate representations between JSON and database rows, and do not need to be polymorphic in the same way.

I realise that means we need another way to create the database table; let me investigate this and get back to you

@sparsetable I just added an InternalModel class without id and created_at fields that you can use: #345

This has been merged into weekly branch, merge those changes in to see and use the class.

ngjunsiang · 2026-02-02T09:23:32Z

campus/model/timetable.py

+    id: schema.CampusID = field(default_factory=(
+        lambda: uid.generate_category_uid("timetable", length=8)
+    ))


See [C#02].

Suggested change

id: schema.CampusID = field(default_factory=(

lambda: uid.generate_category_uid("timetable", length=8)

))

id: int

ngjunsiang · 2026-02-02T09:25:25Z

campus/model/timetable.py

+from dataclasses import dataclass, field
+
+from campus.common import schema
+from campus.common.schema.openapi import String, Integer, Boolean, DateTime


Prefer referencing String, Integer, Boolean, DateTime from schema (they are re-exported under the schema namespace for ease of use)

See docs/STYLE-GUIDE.md for examples

ngjunsiang · 2026-02-02T09:26:23Z

campus/model/timetable.py

+    Describes a day in a repeating timetable.
+    """
+    id: schema.CampusID = unique_field
+    label: String


[C#03]
Reference String datatype from schema namespace for clarity.

Suggested change

label: String

label: schema.String

ngjunsiang · 2026-02-02T09:33:42Z

campus/model/timetable.py

+    id: schema.CampusID = field(default_factory=(
+        lambda: uid.generate_category_uid("timetable", length=8)
+    ))
+    weekday_id: schema.CampusID # FK -> weekday_id
+    timeslot_id: schema.CampusID
+    venue_id: schema.CampusID


See [C#02].

Suggested change

id: schema.CampusID = field(default_factory=(

lambda: uid.generate_category_uid("timetable", length=8)

))

weekday_id: schema.CampusID # FK -> weekday_id

timeslot_id: schema.CampusID

venue_id: schema.CampusID

id: int

weekday_id: int # FK -> weekday_id

timeslot_id: int

venue_id: int

ngjunsiang · 2026-02-02T09:35:15Z

campus/model/timetable.py

+    id: schema.CampusID = field(default_factory=(
+        lambda: uid.generate_category_uid("timetable", length=8)
+    ))


[C#05]
These are essentially what we understand as TTCodes. We can name the category more explicitly.

Suggested change

id: schema.CampusID = field(default_factory=(

lambda: uid.generate_category_uid("timetable", length=8)

))

id: schema.CampusID = field(default_factory=(

lambda: uid.generate_category_uid("timetable-group", length=8)

))

ngjunsiang · 2026-02-02T09:37:10Z

campus/model/timetable.py

+    id: schema.CampusID = field(default_factory=(
+        lambda: uid.generate_category_uid("timetable", length=8)
+    ))


[C#06]
We use 16-char UUIDs for IDs that we know are likely to be much more numerous (due to the logic of combinatorics)

Suggested change

id: schema.CampusID = field(default_factory=(

lambda: uid.generate_category_uid("timetable", length=8)

))

id: schema.CampusID = field(default_factory=(

lambda: uid.generate_category_uid("timetable-groupmember", length=16)

))

ngjunsiang · 2026-02-02T09:40:38Z

campus/model/timetable.py

+class Timetable(Model):
+    """
+    A timetable represents a lesson for a LessonGroup
+    at some VenueTimeSlot
+    """


[C#07]
On 2nd thought, let's make this more explicit.

Suggested change

class Timetable(Model):

"""

A timetable represents a lesson for a LessonGroup

at some VenueTimeSlot

"""

class TimetableEntry(Model):

"""

A timetable entry represents the intersection of:

- a LessonGroup (students + tutor(s))

- a TimetableVenue

- a WeekDay

- a TimeSlot

"""

ngjunsiang · 2026-02-02T09:41:12Z

campus/model/timetable.py

+    id: schema.CampusID = field(default_factory=(
+        lambda: uid.generate_category_uid("timetable", length=8)
+    ))


[C#08]

See [C#05], [C#06]

Suggested change

id: schema.CampusID = field(default_factory=(

lambda: uid.generate_category_uid("timetable", length=8)

))

id: schema.CampusID = field(default_factory=(

lambda: uid.generate_category_uid("timetable-entry", length=16)

))

ngjunsiang · 2026-02-04T08:18:50Z

@SavioNYJC to confirm, only the following database tables are needed in Campus:

LessonGroup
TimetableEntry
Timetable

These are the only Models/InternalModels that need _storage.init_from_model() (to ensure the table exists) in the resource module. The other models are used as intermediate representations (used temporarily within resource methods, but not stored in storage)

Preliminary schema

ee653c2

ngjunsiang changed the title ~~Timetable models~~ feat: timetable models Feb 2, 2026

ngjunsiang added this to the Campus Timetable API milestone Feb 2, 2026

sparsetable self-assigned this Feb 2, 2026

sparsetable marked this pull request as ready for review February 2, 2026 03:04

sparsetable merged commit cc4df82 into weekly Feb 2, 2026
8 checks passed

ngjunsiang mentioned this pull request Feb 2, 2026

feat: add timetable resource for campus.api #331

Open

ngjunsiang linked an issue Feb 3, 2026 that may be closed by this pull request

Timetable models #306

Closed

1 task

ngjunsiang mentioned this pull request Feb 3, 2026

Timetable models #306

Closed

1 task

sparsetable mentioned this pull request Feb 3, 2026

Timetable model: Remove dbml and migrate documentation to python #332

Merged

ngjunsiang commented Feb 4, 2026

View reviewed changes

sparsetable mentioned this pull request Feb 5, 2026

Refine timetable model #346

Merged

-class Timetable(Model):
-    """
-    A timetable represents a lesson for a LessonGroup
-    at some VenueTimeSlot
-    """
+class TimetableEntry(Model):
+    """
+    A timetable entry represents the intersection of:
+    - a LessonGroup (students + tutor(s))
+    - a TimetableVenue
+    - a WeekDay
+    - a TimeSlot
+    """

Conversation

ngjunsiang commented Feb 2, 2026

Uh oh!

sparsetable commented Feb 2, 2026

Uh oh!

Uh oh!

ngjunsiang commented Feb 2, 2026

Uh oh!

ngjunsiang commented Feb 2, 2026

Uh oh!

ngjunsiang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngjunsiang Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngjunsiang Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngjunsiang commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ngjunsiang Feb 4, 2026 •

edited

Loading

ngjunsiang Feb 4, 2026 •

edited

Loading