Today page not handling boundary between semesters properly

[Wxy2003-xy]

Refer to #3757 (comment) for analysis of this issue.

---

Today dashboard display classes only according to days in week, not checking if the class is conducted on particular weeks

1. Go to 'Today'

Expected behavior

Tutorial slot not showing up on 1 Aug, which only starts from week 1

Actual behavior

Slot showed up

Screenshots

If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: MacOS
• Browser Chrome
• Version [20240726-3e1ce70]

Additional context

Add any other context about the problem here.

[ravern]

I'm able to reproduce this for CS2104. Behaviour seems quite finnicky, perhaps can investigate fully before making the fixes.

[xinyuwang-nus]

I'm able to reproduce this for CS2104. Behaviour seems quite finnicky, perhaps can investigate fully before making the fixes.

I made a new commit based on a292172.
Looks like being fixed

[Wxy2003-xy]

I'm able to reproduce this for CS2104. Behaviour seems quite finnicky, perhaps can investigate fully before making the fixes.

Apology, this issue has been fixed by #3760, i forgot to close the issue.

[ravern]

Re-opening this issue becuase #3760 wasn't merged, the author closed the PR without merging.

Can I check if it was merged in some other PR that isn't linked by this issue? I think that this issue went away by itself because the semester started, and not due to the fix.

[xinyuwang-nus]

Re-opening this issue becuase #3760 wasn't merged, the author closed the PR without merging.

Can I check if it was merged in some other PR that isn't linked by this issue? I think that this issue went away by itself because the semester started, and not due to the fix.

I also closed my pr for now cause I found getWeek(date) wasn’t returning the expected week number. Will look into this next week

[ravern]

This has taken a while, but big thanks to @xinyuwang-nus on #3814 whose fix pointed out some deeper issues.

1. Special term modules do not work at all in the current Today page. Functionality already exists for them to show up properly, so this is just a bug to fix. I've filed this in Modules in Special Term do not show up in Today page #3830.

2. It seems like the way boundaries between two semesters are rendered is bugged. We only ever render modules from one semester, so this causes issues during the first/last week of each semester. This GitHub issue will be rescoped to solve this problem.

Explanation

We only ever render modules from one semester. So on the last Thursday of Semester 1, do we render the Thursday class of Semester 1, or the Monday class of Semester 2. The correct answer is both, but it seems like there's no code to currently support that.

The current code in TodayContainer.tsx picks a single semester and passes that as an argument to the data fetching functions.

nusmods/website/src/views/today/TodayContainer/TodayContainer.tsx

Lines 348 to 361 in 6588b48

	export const mapStateToProps = (state: StoreState, ownProps: OwnProps) => {
	const { modules } = state.moduleBank;
	const lastDay = addDays(ownProps.currentTime, DAYS);
	const weekInfo = NUSModerator.academicCalendar.getAcadWeekInfo(lastDay);
	const semester = semesterNameMap[weekInfo.sem];
	const timetable = getSemesterTimetableLessons(state)(semester);
	const colors = getSemesterTimetableColors(state)(semester);
	const timetableWithLessons = hydrateSemTimetableWithLessons(timetable, modules, semester);
	return {
	colors,
	timetableWithLessons,
	};
	};

In the past, a special fix was added in #1391 to handle the boundary between Semester 1 and 2. However, the fix doesn't work because it relies on there not being any classes in Semester 1, since it just switches to rendering Semester 2 modules one week early.

This causes problems when the opposite case is true, i.e. for the boundary between Special Term II and Semester 1, which surfaces as modules being wrongly rendered as in this issue and in #2789.

Solution

A full fix to this problem would be fetch the modules from both semesters and render them accordingly, in the mapStateToProps function linked above^

In the interim, I'm okay to merge #3814 (pending fixes to linting) since Special Term rendering of modules is broken anyway. The solution there reverses the fix in #1391 and then handles the boundary between Semester 1 and Semester 2 as a special case.