FIX BUG: By added continue in approval check#5332
Open
hemannt003 wants to merge 1 commit intolearning-unlimited:mainfrom
Open
FIX BUG: By added continue in approval check#5332hemannt003 wants to merge 1 commit intolearning-unlimited:mainfrom
hemannt003 wants to merge 1 commit intolearning-unlimited:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding continue after the warning, then implementing tests: locating a class/section for the timeslot and ensuring student profiles exist.
Description
The bug
In save_priorities (two-phase lottery: student picks classes per priority slot per timeslot), there’s a check: if the section or parent class isn’t approved (status > 0), the code logs a warning and should not register the student for that class.
The problem was that after logger.warning(...) there was no continue. So the loop kept going: it ran the grade-range check and then created or updated a StudentRegistration anyway. So “unapproved” classes could still get a priority registration.
The fix
Right after the warning, a continue was added so that iteration of the loop stops there for that priority/class. The next items in the same POST (other priority slots) still run normally.
So: warn → skip this class → go to the next rel_index, cls_id in the loop.
The tests (in studentregtwophase tests)
Approved path – With a normal scheduled class (section + class both “approved”), posting save_priorities should create a valid Priority/1 registration.
Unapproved path – Force ClassSubject and ClassSection to an unapproved status (UNREVIEWED), post the same kind of request, then assert:
Helpers were added to give the student a profile (so grade checks behave) and to find a class/section that actually uses the chosen timeslot after schedule_randomly().
Closes #5331
Type of Change
Testing
AI Disclosure
Used AI to get better understanding of bug and got approaches to fix it.
Checklist