fix: hide masquerade bar for anonymous users and on failed loads#266
fix: hide masquerade bar for anonymous users and on failed loads#266arbrandes wants to merge 1 commit into
Conversation
Anonymous users no longer mount MasqueradeBar: the slot op is gated on authenticated: true so the component never runs for them and the masquerade query is never fired. For authenticated users, the bar now hides on any failed load instead of rendering a partial shell with a "failed to load" alert. The LMS's 302-to-login response for denied masquerade requests is transparently followed by browser XHR, so it is detected via the final response URL and surfaced as a 403-shaped error; the existing denial branch then hides the bar. Mutation errors after the bar has rendered still surface as before. Co-Authored-By: Claude <noreply@anthropic.com>
arbrandes
force-pushed
the
arbrandes/fix-masquerade-behavior
branch
from
967c6af to
7ed1f25
Compare
| * Anything other than a successful load with `success: true` hides the bar: | ||
| * 403, 200 with `success: false`, network errors, 5xx, redirected requests. | ||
| */ | ||
| function isQueryDenied(query: { isError: boolean, error: unknown, data: MasqueradeStatus | undefined }): boolean { | ||
| if (query.isError) { | ||
| return getHttpStatus(query.error) === 403; | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The PR description says
The LMS's 302-to-login redirect for denied masquerade requests is detected via the final response URL and surfaced as a 403-shaped error
That doesn't seem to match up with the change to deny on all errors here. Am I looking in the wrong spot?
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah! Yeah, the changes in api.ts match the PR description.
I guess that changes my question to: "why is isQueryDenied changing?"
The previous logic matched the previous comment
Other failures (network, 5xx) are treated as "couldn't load" and surface as an alert.
Is that not desired behavior?
There was a problem hiding this comment.
No, it's not desired. This is because, as I just realized when fixing this, one of the error modes - "couldn't load" - also happens when simply being logged out. This is because the backend returns a 302 instead of a 403 (why it would do that for an XHR request, I don't even dare guess), and, not knowing what to do with it, React Query just treats it as a network failure, renders the bar, and then the alert.
This made me think that any error except a mutation error should result in the bar not showing, because otherwise, a regular learner would suddenly be able to see the bar because of an actual network failure.
4 participants
Description
Fixes #265.
Anonymous users no longer mount
MasqueradeBar: the slot op is gated onauthenticated: true, so the component never runs for them and the masquerade query is never fired.For authenticated users, the bar now hides on any failed load instead of rendering a partial shell with an "Unable to load masquerade options." alert. The LMS's 302-to-login redirect for denied masquerade requests is detected via the final response URL and surfaced as a 403-shaped error, so the existing denial branch hides the bar. Mutation errors after the bar has rendered still surface as before.
LLM usage notice
Built with assistance from Claude.