Repository navigation
Fall back to the basic role when somebody holds no office - #2001
Merged
vikrantwiz02 merged 1 commit intoSep 30, 2026
Merged
vikrantwiz02 merged 1 commit into
vikrantwiz02 merged 1 commit into
Conversation
active_designation returned the first office held, and a basic role is not an office — so anyone holding only student, faculty or staff resolved to no role at all and was refused by every one of the 175 role_required endpoints. An account that also holds an office looked fine, which is why it reached production before it was caught. A role that is no longer held now falls back the same way rather than to nothing, and the two cases that were missing are tested: somebody holding only the basic role, and somebody holding nothing at all.
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
1 participant
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.
Follow-up to #2000, which moved authorisation from "every role held" to "the role being acted in". The fallback it used when nobody has chosen a role was wrong.
What broke
active_designationreturned the first office held. A basic role —student,faculty,staff— is not an office, so anyone holding only one of those resolved to no role at all and was refused by every one of the 175@role_requiredendpoints.An account that also holds an office resolved fine, which is why it got past testing and showed up as a student seeing
Error: Request failed with status code 403on their own Registered Courses page.The fix
The fallback now tries offices first and then the basic role, so a plain student acts as
student. A role that is no longer held falls back the same way instead of to nothing.Tests
The two cases that were missing:
One existing assertion corrected: it expected
Noneafter an office was revoked, where falling back to thestudentthe user still holds is the right answer.52 tests pass in the affected apps.