Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🪲 Students joining a class via link succeeds with an error message #6160

Closed
confiks opened this issue Feb 5, 2025 · 1 comment · Fixed by #6161
Closed

🪲 Students joining a class via link succeeds with an error message #6160

confiks opened this issue Feb 5, 2025 · 1 comment · Fixed by #6161
Labels
bug Something isn't working teacher-happiness Features that make teachers happier

Comments

@confiks
Copy link
Contributor

confiks commented Feb 5, 2025

The following bug is present on production and main.

When a student uses a teacher link to join a class, and clicks the button to join the class, the request to do so succeeds, but the student gets an error message about a network error. Subsequent presses on the button result in an error message explaining that the class has already been joined. Refreshing the page reveals that the student has successfully joined the class.

The error occurs because the response of join_class in classes.py return an empty string text/html response, while join_class in teachers.ts expects a JSON response. Because the request actually returns a 200, it shows a general network error, and the end-to-end tests only verify the 200.

This bug seems to have been introduced in #5558 in May 2024, which switched from jsonify to make_response. Or perhaps it was fixed shortly after that and recently reintroduced, because that's almost a year ago and this bug is so visible I'd think it would've been noticed sooner.

It's likely this bug affects other actions in a similar way.

Image
@confiks
Copy link
Contributor Author

confiks commented Feb 5, 2025

Fixed in #6161

@AnneliesVlaar AnneliesVlaar added the teacher-happiness Features that make teachers happier label Feb 5, 2025
mergify bot pushed a commit that referenced this issue Feb 6, 2025
…#6161)

Fixes #6160, and potentially similar bugs as well. 

⚠️ I changes all occurrences of `'', 200` in `make_response`, but I only tested that `join_class` works as intended and #6160 is fixed. I don't know enough about the application to test the other occurrences effectively. This should be tested for the other functionality as well by a maintainer.
@mergify mergify bot closed this as completed in #6161 Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working teacher-happiness Features that make teachers happier
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants