[BB-1504] Fix the automatic enrollment issue for inactive user - #21594
Conversation
|
Thanks for the pull request, @lgp171188! I've created OSPR-3832 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
openedx-webhooks
added
needs triage
open-source-contribution
|
@lgp171188, could you please add a point info about disabling 👍
|
|
@lgp171188 Thank you for your contribution. Please let me know once it is ready for our review. |
openedx-webhooks
added
waiting on author
|
@natabene, this is ready for your review :) |
|
@ormsbee This is ready for your review when you have a chance. |
openedx-webhooks
added
awaiting prioritization
and removed
waiting on author
|
@ormsbee, did you get a chance to check this? |
|
@ormsbee, did you get a chance to check this out? |
1 similar comment
|
@ormsbee, did you get a chance to check this out? |
|
@lgp171188 Sorry, we don't have bandwidth right now. Please coordinate with @pomegranited |
|
jenkins run all |
353087a to
c6bf24d
Compare
openedx-webhooks
added
open edx community review
and removed
awaiting prioritization
labels
|
@bradenmacdonald, since you already left a review comment, I wanted to know if you will be reviewing this PR as a part of your core committer role. |
I don't have time for this one right now but was considering it. I may be able to review it in the future. |
c6bf24d to
92793fd
Compare
|
@bradenmacdonald, I have updated the PR and the CI checks are now passing. The sandbox mentioned in the PR description has the latest changes from this PR and can be used to verify the fix. |
|
Thanks @lgp171188 ! Hoping to get to this tomorrow. |
bradenmacdonald
left a comment
bradenmacdonald
left a comment
There was a problem hiding this comment.
👍 Thanks for this fix, and for the sandbox. Approving but had a couple minor questions.
- I tested this:
https://pr25142.sandbox.opencraft.hosting/ (without fix)
⚫ Created account user+exists@opencraft.com (did not verify email)
⚫ As staff, invited user+exists@opencraft.com and user+future@opencraft.com to an invite-only course
⚫ Noted that email to user+exists says "You have been enrolled in Private Course"
✅ Confirmed user+exists@opencraft.com is now enrolled, can see the course
⚫ Created account user+future@opencraft.com (did not verify email)
✅ Confirmed user+future@opencraft.com is not enrolled, cannot see the course
⚫ Activated user+future@opencraft.com's account - ✅ confirmed that the course now shows up
https://pr21594.sandbox.opencraft.hosting/ (with this fix)
⚫ Created account user+exists@opencraft.com (did not verify email)
⚫ As staff, invited user+exists@opencraft.com and user+future@opencraft.com to an invite-only course
✅ Confirmed email to both now says "Please finish your registration... Once you have registered and activated your account, you will see Super Secret Private Course listed on your dashboard."
✅ Confirmed user+exists@opencraft.com is not yet enrolled, cannot even log in again until verifying email.
⚫ Activated user+exists@opencraft.com's account - ✅ confirmed that the course now shows up
⚫ Created account user+future@opencraft.com (did not verify email)
✅ Confirmed user+future@opencraft.com is not enrolled, cannot see the course
⚫ Activated user+future@opencraft.com's account - ✅ confirmed that the course now shows up
Note that cases where "a user registers an account and then is enrolled in a course by the instructor before completing the email validation" should be relatively rare, while cases where "the user is enrolled before the account is created" should be more common, so this is bringing the edge case in line with the more common (and secure) behavior.
- I read through the code
- I checked for accessibility issues: n/a
- Includes documentation: n/a
When a user registers an account and is enrolled in a course by the instructor before completing the email validation, the course shows up in the dashboard of the logged-in user. This change adds a check for the email validation before enrolling the user.
92793fd to
ba5c5e0
Compare
|
Your PR has finished running tests. There were no failures. |
|
@lgp171188 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
9 participants
When a user registers an account and then is enrolled in a course by the instructor before completing the email validation, the course shows up in the dashboard of the unverified, logged-in user. This contrasts with the behavior observed when the user is enrolled before the account is created - the user is unable to see the enrolled course before verifying the email address.
This PR adds a check for the email validation before enrolling the user.
JIRA tickets: OSPR-3832
Dependencies: None
Sandbox URL: https://pr21594.sandbox.opencraft.hosting/
Merge deadline: None
Testing instructions:
While the unverified user can still navigate to the public courses and enroll, this issue breaks the expected behaviour with invitation-only (and optionally, private) courses.
Reviewers