Skip to content

fix: respect teacher filters in random fill schedule generation - #724

Open
anitalves08 wants to merge 1 commit into
developfrom
fix/random-fill-teacher-filters
Open

fix: respect teacher filters in random fill schedule generation#724
anitalves08 wants to merge 1 commit into
developfrom
fix/random-fill-teacher-filters

Conversation

@anitalves08

Copy link
Copy Markdown
Contributor

Closes #308

What does this PR do?

The Random Fill algorithm was ignoring the teacher filters set by the user in each course's class selector.

  • Filter available classes by filteredTeachers in getSchedulesGenerator()
  • Reset the generator when filteredTeachers change, in addition to when locked courses change

@netlify

netlify Bot commented Apr 29, 2026

Copy link
Copy Markdown

Deploy Preview for tts-fe-preview ready!

Name Link
🔨 Latest commit 218fece
🔍 Latest deploy log https://app.netlify.com/projects/tts-fe-preview/deploys/69f2290e3d54630008d2d994
😎 Deploy Preview https://deploy-preview-724--tts-fe-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions

Copy link
Copy Markdown

🌐 Netlify Preview disponível:
https://69f2290e3d54630008d2d994--tts-fe-preview.netlify.app

const newLockedCourses = selected?.filter((course) => course.locked)
.map((course) => course.course_id)

const newFilteredTeachers = selected?.map((course) =>
const newFilteredTeachers = selected?.map((course) =>
(course.filteredTeachers ?? []).join(',')).join('|')

const prevFilteredTeachers = courseOptions?.map((course) =>

@mariana2103 mariana2103 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The filter logic in getSchedulesGenerator looks correct. The generator reset on teacher filter change is missing though — one-line fix needed, plus worth confirming courseOptions is stale state for the comparison to be meaningful.

(course.filteredTeachers ?? []).join(',')).join('|')

// Only update if locked courses changed
if (newLockedCourses?.join() !== lockedCourses?.join()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

newFilteredTeachers and prevFilteredTeachers are computed here but never used — the condition only checks locked courses, so the generator is never reset when teacher filters change. This contradicts the PR description which lists it as one of the two goals.

Suggested change
if (newLockedCourses?.join() !== lockedCourses?.join()) {
if (newLockedCourses?.join() !== lockedCourses?.join() || newFilteredTeachers !== prevFilteredTeachers) {

One thing worth double-checking before merging: this comparison assumes courseOptions holds the previous filter state (one render behind) while selected holds the new one. If courseOptions is always current, the comparison will always be equal and the reset will never fire even after this fix. Can you confirm courseOptions is stale state here?

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.

fix: random Fill is not respecting the teacher filters

3 participants