Skip to content

Commit

Permalink
🪲 Fix make_response incorrectly sending text instead of JSON (#5599)
Browse files Browse the repository at this point in the history
#5558 Introduced an error where some calls where expecting a JSON, and got text instead. I checked the places where this happened and fix the calls. 

If I'm missing some, please add them 😄 

**How to test**
Perform the following: 

* Update an adventure
* Update  a class customization
* Create students account
* Reset password
* Reset a student's password
* Update your public profile

And all of them should show the success modal! 

Also:
Fixes #5550
  • Loading branch information
jpelay authored Jun 10, 2024
1 parent b02aefb commit f24810f
Show file tree
Hide file tree
Showing 11 changed files with 37 additions and 32 deletions.
14 changes: 9 additions & 5 deletions app.py
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,12 @@ def prepare_files():
threader = textwrap.dedent("""
import time
from turtlethread import Turtle
def int_with_error(value, error_message):
try:
return int(value)
except ValueError:
raise ValueError(error_message.format(value))
t = Turtle()
t.left(90)
with t.running_stitch(stitch_length=20):
Expand All @@ -751,7 +757,6 @@ def prepare_files():
for file in [x for x in files if x[:len(filename)] == filename and x[-3:] != 'zip']:
zip_file.write('machine_files/' + file)
zip_file.close()

return make_response({'filename': filename}, 200)


Expand Down Expand Up @@ -788,7 +793,6 @@ def generate_microbit_file():
save_transpiled_code_for_microbit(transpile_result)
return make_response({'filename': 'Micro-bit.py', 'microbit': True}, 200)
else:
# TODO
return make_response({'message': 'Microbit feature is disabled'}, 403)


Expand Down Expand Up @@ -827,7 +831,6 @@ def remove_file(response):

return send_file(os.path.join(micro_bit_directory, "micropython.hex"), as_attachment=True)
else:
# TODO
return make_response({'message': 'Microbit feature is disabled'}, 403)


Expand Down Expand Up @@ -2604,9 +2607,10 @@ def update_public_profile(user):
body['tags'].append('admin')

DATABASE.update_public_profile(user['username'], body)
response = {"message": gettext("public_profile_updated")}
if achievement:
utils.add_pending_achievement({"achievement": achievement})
return make_response(gettext("public_profile_updated"), 200)
response["achievement"] = achievement
return response


@app.route('/translating')
Expand Down
2 changes: 1 addition & 1 deletion templates/incl/menubar.html
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
</a>
<div class="dropdown-menu dropdown-blue" id="language_dropdown" style="display: none;"
_="on mutation of @style
set arrow to #language-arrow
set arrow to #language_arrow
if *display == 'none'
remove .rotate-180 from arrow
else if not arrow.classList.contains('rotate-180')
Expand Down
2 changes: 1 addition & 1 deletion tests/cypress/e2e/feedback/feedback_modal.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@ describe("Test the feedback feature", () => {
cy.getDataCy('modal_feedback_input')
.should("not.be.visible")

cy.wait("@postFeedback").should('have.nested.property', 'response.statusCode', 204)
cy.wait("@postFeedback").should('have.nested.property', 'response.statusCode', 200)
});
});
2 changes: 1 addition & 1 deletion website/achievements.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,4 +280,4 @@ def push_new_achievement(self, user):
)
}
)
return make_response('', 204)
return make_response('', 200)
3 changes: 1 addition & 2 deletions website/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,7 @@ def change_user_email(self, user):
)
except BaseException:
return make_response(gettext("mail_error_change_processed"), 400)

return make_response('', 204)
return make_response('', 200)

@route("/getUserTags", methods=["POST"])
@requires_admin
Expand Down
4 changes: 2 additions & 2 deletions website/auth_pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ def change_student_password(self, user):
hashed = password_hash(body["password"], make_salt())
self.db.update_user(body["username"], {"password": hashed})

return make_response(gettext("password_change_success"), 200)
return make_response({"success": gettext("password_change_success")}, 200)

@ route("/change_password", methods=["POST"])
@ requires_login
Expand Down Expand Up @@ -467,5 +467,5 @@ def store_new_account(self, account, email):
)
except BaseException:
return user, make_response({gettext("mail_error_change_processed")}, 400)
resp = make_response('', 204)
resp = make_response('', 200)
return user, resp
29 changes: 15 additions & 14 deletions website/classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ def create_class(self, user):

self.db.store_class(Class)
achievement = self.achievements.add_single_achievement(user["username"], "ready_set_education")
if achievement:
utils.add_pending_achievement({"achievement": achievement})
return make_response({"id": Class["id"]}, 200)
response = {"id": Class["id"]}
response["achievement"] = achievement
return make_response(response, 200)

@route("/<class_id>", methods=["PUT"])
@requires_teacher
Expand Down Expand Up @@ -87,8 +87,8 @@ def update_class(self, user, class_id):
self.db.update_class(class_id, body["name"])
achievement = self.achievements.add_single_achievement(user["username"], "on_second_thoughts")
if achievement:
utils.add_pending_achievement({"achievement": achievement})
return make_response('', 204)
return make_response({"achievement": achievement}, 200)
return make_response('', 200)

@route("/<class_id>", methods=["DELETE"])
@requires_login
Expand Down Expand Up @@ -197,7 +197,7 @@ def join_class(self):
session["messages"] = session["messages"] - 1 if session["messages"] else 0

if achievement:
utils.add_pending_achievement({"achievement": achievement})
return make_response({"achievement": achievement}, 200)
return make_response('', 200)

@route("/<class_id>/student/<student_id>", methods=["DELETE"])
Expand All @@ -213,8 +213,8 @@ def leave_class(self, user, class_id, student_id):
if Class["teacher"] == user["username"]:
achievement = self.achievements.add_single_achievement(user["username"], "detention")
if achievement:
utils.add_pending_achievement({"achievement": achievement})
return make_response('', 204)
return make_response({"achievement": achievement}, 200)
return make_response('', 200)

@route("/<class_id>/second-teacher/<second_teacher>", methods=["DELETE"])
@requires_login
Expand All @@ -230,8 +230,8 @@ def remove_second_teacher(self, user, class_id, second_teacher):
if Class["teacher"] == user["username"]:
achievement = self.achievements.add_single_achievement(user["username"], "detention")
if achievement:
utils.add_pending_achievement({"achievement": achievement, "reload": True})
return make_response('', 205)
return make_response({"achievement": achievement, "reload": True}, 200)
return make_response('', 200)


class MiscClassPages(WebsiteModule):
Expand Down Expand Up @@ -305,11 +305,12 @@ def duplicate_class(self, user):
new_second_teachers = Class.get("second_teachers")

achievement = self.achievements.add_single_achievement(current_user()["username"], "one_for_money")
response = {"id": new_class["id"]}
if achievement:
utils.add_pending_achievement({"achievement": achievement})
response['achievement'] = achievement
if new_second_teachers:
return make_response({"id": new_class["id"], "second_teachers": new_second_teachers}, 200)
return make_response({"id": new_class["id"]}, 200)
response["second_teachers"] = new_second_teachers
return make_response(response, 200)

@route("/invite-student", methods=["POST"])
@requires_teacher
Expand Down Expand Up @@ -358,7 +359,7 @@ def invite_student(self, user):

@route("/invite-second-teacher", methods=["POST"])
@requires_teacher
def invite_second_teaceher(self, user):
def invite_second_teacher(self, user):
teacher = user
body = request.json
# Validations
Expand Down
2 changes: 1 addition & 1 deletion website/feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def teacher_feedback(self, user):
print(e)
return make_response(gettext('feedback_message_error'), 500)

response = make_response('', 204)
response = make_response('', 200)
response.headers["HX-Push-URL"] = 'false'
response.headers["HX-Trigger"] = json.dumps({"hideFeedbackModal": True})

Expand Down
7 changes: 4 additions & 3 deletions website/for_teachers.py
Original file line number Diff line number Diff line change
Expand Up @@ -914,9 +914,10 @@ def update_customizations(self, user, class_id):
self.db.update_class_customizations(customizations)

achievement = self.achievements.add_single_achievement(user["username"], "my_class_my_rules")
response = {"success": gettext("class_customize_success")}
if achievement:
utils.add_pending_achievement({"achievement": achievement})
return make_response(gettext("class_customize_success"), 200)
response["achievement"] = achievement
return make_response(response, 200)

@route("/create-accounts/<class_id>", methods=["GET"])
@requires_teacher
Expand Down Expand Up @@ -1186,7 +1187,7 @@ def update_adventure(self, user):
elif class_id not in current_classes:
self.add_adventure_to_class_level(user, class_id, body["id"], level, False)

return make_response(gettext("adventure_updated"), 200)
return make_response({"success": gettext("adventure_updated")}, 200)

@route("/customize-adventure/<adventure_id>", methods=["DELETE"])
@route("/customize-adventure/<adventure_id>/<owner>", methods=["DELETE"])
Expand Down
2 changes: 1 addition & 1 deletion website/programs.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ def report_program(self, user):
'<a href="' + link + '">Program link</a>',
)

return make_response(gettext("report_success"), 200)
return make_response({"message": gettext("report_success")}, 200)


class NotYourProgramError(RuntimeError):
Expand Down
2 changes: 1 addition & 1 deletion website/user_activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,6 @@ def index(self, user):

try:
logger.log(data)
return make_response('', 204)
return make_response({}, 200)
except IOError:
return make_response(gettext("request_invalid"), 400)

0 comments on commit f24810f

Please sign in to comment.