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

feat: force all enrollments #1761

Merged
merged 9 commits into from
Aug 10, 2023
Merged

feat: force all enrollments #1761

merged 9 commits into from
Aug 10, 2023

Conversation

asadali145
Copy link
Contributor

@asadali145 asadali145 commented Jul 20, 2023

What are the relevant tickets?

Fixes https://github.com/mitodl/hq/issues/1773

Description (What does it do?)

We are going to mark courses as invite-only in edX to stop direct enrollments in edX. It will require us to force the course enrollments to bypass invitation checks. This PR forces all the course enrollments.

Screenshots (if appropriate):

  • Desktop screenshots
  • Mobile width screenshots

How can this be tested?

  • Make sure that the MITxOnline / edX integration is working
  • Enable COURSES_INVITE_ONLY in edX.
  • Check out this branch in MITxOnline.
  • Verify Audit / Verified Enrollment/Unenrollment, Also, test 100% discount enrollments.
  • Verify the following management commands:
    • defer_enrollment
    • transfer_enrollment
    • unenroll_enrollment
    • refund_fulfilled_order
    • retry_edx_enrollment
    • create_enrollment
    • create_verified_enrollment

Additional Context

@asadali145 asadali145 changed the title [WIP] feat: force all enrollments feat: force all enrollments Jul 21, 2023
@asadali145 asadali145 marked this pull request as ready for review July 21, 2023 11:46
@asadali145 asadali145 requested a review from arslanashraf7 July 21, 2023 11:46
@arslanashraf7 arslanashraf7 self-assigned this Aug 1, 2023
@arslanashraf7
Copy link
Contributor

On the testing part:

  1. I could only test enrollments
  2. I couldn't test unenrollment because It's not working. And the reason for that is that we are using user based client instead of service api client while deactivating the enrollments. Take a look at https://github.com/mitodl/mitxonline/blob/main/openedx/api.py#L747 line. Now that we are moving to invitation only courses we would a change in this as well because it eventually calls the enrollment API of edX with is_active=False and the enrollment API doesn't allow this after being Invitation only.
  3. Because of unenrollment, I couldn't test defer_enrollment , unenroll_enrollment, unenroll_enrollment etc.

I've created some related issues #1797 and #1798

@asadali145
Copy link
Contributor Author

As per our discussion, unenrollment looks fine now. Maybe it was some issue with the API keys. Could you please test this PR again?

Copy link
Contributor

@arslanashraf7 arslanashraf7 left a comment

Choose a reason for hiding this comment

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

I was facing issues with unenrollment but after adding the right value for EDX_API_KEY I was able to test all the cases mentioned in the PR description. So this looks good. 👍

@asadali145 asadali145 force-pushed the asad/force-all-enrollments branch from adfe800 to 59b045c Compare August 9, 2023 06:21
@pdpinch
Copy link
Member

pdpinch commented Aug 10, 2023

This seems ok to merge to me.

When we change the setting in open edX to invite only, we'll need to test carefully & also monitor the logs for any errors.

@asadali145 asadali145 merged commit 65ce553 into main Aug 10, 2023
@odlbot odlbot mentioned this pull request Aug 10, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants