Skip to content

Conversation

yadobler
Copy link

@yadobler yadobler commented Sep 13, 2025

Calculate all conflicts so that prereq and duplicate checks for future sems do not get shadowed.

Context

Fixes #4231

image

If a module in the future fails semesterCheck, it will not ignore prereqCheck or duplicateCheck warnings. This allows for the intended case of only showing Prerequisites / Duplicates warnings regardless of whether Module is currently offered in that semester.

Correct Future Sem? Prereqs fail? Duplicates? result
No No No ✅ Green Light
No Yes No ⚠️ Prereq Error
No Yes Yes ⚠️ Prereq Error
No No Yes ⚠️ Duplicate Error
Yes No No ✅ Green Light
Yes Yes No ⚠️ Prereq Error
Yes Yes Yes ⚠️ Prereq Error
Yes No Yes ⚠️ Duplicate Error

Implementation

  • ModuleInfo now takes an array of Conflict instead of just a single one
  • All conflict checks are done, instead of terminating at the first non-null conflict
  • The displayedConflict will either be the first conflict in the Conflicts array if current year, or else either Prerequisite or Duplicate conflict for non-current years

Assumptions:

  • conflict-checking functions are added into the conflictChecks array in sequence of priority
  • prerequisites takes precedence over duplicates
  • future semesters do not include Semester conflicts due to possible changes in future timetable

Other Information

  • Not sure if I need to edit any test files add more tests to check this behaviour
  • please ignore banana cursor in screenshot

Copy link

vercel bot commented Sep 13, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
nusmods-export Ready Ready Preview Comment Sep 13, 2025 5:29am

Copy link

vercel bot commented Sep 13, 2025

@yadobler is attempting to deploy a commit to the modsbot's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

codecov bot commented Sep 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.96%. Comparing base (988c6fd) to head (4505a45).
⚠️ Report is 142 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4232      +/-   ##
==========================================
- Coverage   54.52%   52.96%   -1.56%     
==========================================
  Files         274      289      +15     
  Lines        6076     6687     +611     
  Branches     1455     1634     +179     
==========================================
+ Hits         3313     3542     +229     
- Misses       2763     3145     +382     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

[Planner] Prerequisite check false-positive for alternate-semester mods if placed in the wrong semester of a future year
1 participant