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

TA Mode doesn't work for some courses #3929

Open
HollaG opened this issue Jan 3, 2025 · 4 comments · May be fixed by #3930
Open

TA Mode doesn't work for some courses #3929

HollaG opened this issue Jan 3, 2025 · 4 comments · May be fixed by #3930

Comments

@HollaG
Copy link
Contributor

HollaG commented Jan 3, 2025

Describe the bug

Some courses cannot be set to "TA Mode".
Example courses:

  • CS2103T
  • CS3281

To Reproduce

Steps to reproduce the behavior:

  1. Add either one of these courses to the timetable
  2. Click on TA Mode Button
  3. Nothing happens (no errors too, in console)

Expected behavior

TA mode is enabled

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Chrome
  • Version 131.0.6778.205
@leslieyip02
Copy link
Contributor

This happens because TA mode filters out all lecture lessons, so the issue arises because both CS2103T and CS3281 only have lecture lessons.

One way to fix this would be to make lectures also togglable. @zwliew, I don't mind working on this.

@zwliew
Copy link
Member

zwliew commented Jan 3, 2025

Yes, let's make all lessons types TA-able.

@leslieyip02
Copy link
Contributor

I've run into a bit of a problem for CS2103T specifically. Because one class number is associated with multiple lesson slots, this means that adding multiple tutorial slots will add a bunch of the lecture slots as well.

For example, for a timetable with the following TA config:

[[ "Lecture", "G11" ], [ "Lecture", "G15" ], ​​​​[ "Lecture", "G05" ], ​​​​[ "Lecture", "G21" ]]

image

This is because our toggle is based only on class numbers at the moment, so one way to overcome this is to store start and end times in TaModulesConfig as well?

@zwliew
Copy link
Member

zwliew commented Jan 3, 2025

This is because our toggle is based only on class numbers at the moment, so one way to overcome this is to store start and end times in TaModulesConfig as well?

I believe that adding just the start time (and day) would be enough to ensure that each slot is unique. If that works, please also add a migration to convert the current [LessonType, ClassNo] format to the new [LessonType, ClassNo, StartTime] format.

@leslieyip02 leslieyip02 linked a pull request Jan 3, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants