Skip to content

fix(FAN-41): catch the EnrollmentNotAllowed and InvalidEnrollmentAttribute - #6

Merged
igobranco merged 1 commit into
nau/lilac.masterfrom
nau/fix-fan41
Dec 16, 2022
Merged

fix(FAN-41): catch the EnrollmentNotAllowed and InvalidEnrollmentAttribute#6
igobranco merged 1 commit into
nau/lilac.masterfrom
nau/fix-fan41

Conversation

@MaferMazu

@MaferMazu MaferMazu commented Dec 14, 2022

Copy link
Copy Markdown
Contributor

Description

This PR catches EnrollmentNotAllowed and InvalidEnrollmentAttribute exceptions in favor of returning a status 400 bad request. This fixes the 500 error when you make a post request to the /api/enrollment/v1/enrollment.

How to test

  • Have an environment with this version of edx-platform.
  • Have the filter configured and a course with the proper "other course settings" as we explained in the FAN-41 PR.
  • With a user without an allowed domain, make a post request to /api/enrollment/v1/enrollment with {"course_details":{"course_id": "<your_course_id>"}}, and you are going to get an error 400 instead of 500.

Before

Screenshot from 2022-12-14 16-59-19
Response in /api/enrollment/v1/enrollment
Screenshot from 2022-12-14 16-58-50
Logs

After

Screenshot from 2022-12-14 17-08-31
Response in /api/enrollment/v1/enrollment
Screenshot from 2022-12-14 17-07-31
Logs

Note: If you want to change the message, you need to edit this and its translation here.

@MaferMazu
MaferMazu marked this pull request as ready for review December 14, 2022 22:34

@felipemontoya felipemontoya left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks exactly as we discussed in the call.
The code is tested in a development environment and the screenshots are pasted here. I think we are good to merge. Just waiting for Ivo's approval

@igobranco
igobranco merged commit c7340a0 into nau/lilac.master Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants