Skip to content

fix: canTermEnrollmentChange#1926

Open
jenniyan wants to merge 6 commits into
icssc:mainfrom
jenniyan:main
Open

fix: canTermEnrollmentChange#1926
jenniyan wants to merge 6 commits into
icssc:mainfrom
jenniyan:main

Conversation

@jenniyan

@jenniyan jenniyan commented Jun 9, 2026

Copy link
Copy Markdown

Summary

Test Plan

  • pnpm test should succeed

Issues

Closes #1818

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Re-trigger cubic

@KevinWu098 KevinWu098 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks solid. Two questions on correctness that would be good for you to confirm

  1. see below
  2. is drop deadline also 5 PM on friday for all summer sessions?

Comment thread apps/antalmanac/src/lib/termHelpers.ts Outdated
@jenniyan

jenniyan commented Jun 10, 2026

Copy link
Copy Markdown
Author

Looks solid. Two questions on correctness that would be good for you to confirm

  1. see below
  2. is drop deadline also 5 PM on friday for all summer sessions?

After looking at the summer sessions FAQ, there is no specific indicator that the drop deadline is 5PM, so for now I adjusted it so that for all summer closing enrollment times are EOD. I also adjusted it so that for the shorter terms (summer sessions 1 and 2), the drop deadline is Week 1 Friday.

Additionally, the app isn’t globally set to PST. Some features do explicitly use America/Los_Angeles(for example the mailer that sends notifications for changes in course enrollment), but the main date logic still follows the runtime’s local timezone. So for any user in EST, a regular quarter's cutoff would look like Friday 5 PM EST.

@KevinWu098 KevinWu098 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for looking into it. For summer, EOD seems like a reasonable choice. For timezones, however, what I meant was that it would be... incorrect for someone on, say, the east coast to no longer be able to subscribe to notifications at 5 PM local time, which is only 2 PM PST.

Hence, it probably makes sense to figure out how to pin it to 5 PM PST! (and, as a good completeness thing, scan if there are any related areas which would benefit from also being pinned to PST without regressing behavior)

Comment thread apps/antalmanac/src/lib/termHelpers.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

canTermEnrollmentChange closes enrollment at Friday 00:00 instead of end of day

2 participants