Skip to content

MST-181: Add the ability to sync Program Enrollment Linkage with SSO'd edX account - #23873

Merged
michaelroytman merged 1 commit into
masterfrom
mroytman/MST-181-program-enrollment-sync-support-tool
May 7, 2020
Merged

MST-181: Add the ability to sync Program Enrollment Linkage with SSO'd edX account#23873
michaelroytman merged 1 commit into
masterfrom
mroytman/MST-181-program-enrollment-sync-support-tool

Conversation

@michaelroytman

@michaelroytman michaelroytman commented May 1, 2020

Copy link
Copy Markdown
Contributor

MST-181

Enhance the Link Program Enrollments Support Tool to handle the case of linking a user to a ProgramEnrollment that is already linked to a different edX account.

In this case,

  • unenroll the currently linked user from the courses associated with related ProgramCourseEnrollments
  • move the currently linked user into the audit track in the courses associated with related ProgramCourseEnrollments
  • link the new user to the ProgramEnrollment
  • enroll the new user in the Master's track in the courses associated with related ProgramCourseEnrollments

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TODO: I will create a ticket to include logic around handling the transference of course_staff roles.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See MST-238.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm just fixing a typo I noticed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't believe this is true anymore, since this is now a ForeignKey and not a OneToOneField. Please correct me if I am wrong.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You are correct here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I'll change this to unlink_program_enrollment, since it operates on more than just the course enrollments now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, changing the function name to unlink_program_enrollment make sense.

@michaelroytman

Copy link
Copy Markdown
Contributor Author

jenkins run python

@michaelroytman
michaelroytman force-pushed the mroytman/MST-181-program-enrollment-sync-support-tool branch from 2175dd6 to d1ac943 Compare May 4, 2020 19:18

@schenedx schenedx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My comments are NIT. You can choose to ignore them.

The changes looks good to me.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, changing the function name to unlink_program_enrollment make sense.

# assert that all CourseAccessRoleAssignment objects are deleted
assert not active_enrollment_1.courseaccessroleassignment_set.all().exists()

def test_enrollment_already_linked_to_different_user(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The function name can be clearer with test_update_linking_enrollment_to_another_user

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You are correct here.

@michaelroytman
michaelroytman force-pushed the mroytman/MST-181-program-enrollment-sync-support-tool branch 2 times, most recently from 17347d9 to 6e99892 Compare May 5, 2020 00:52
@schenedx schenedx changed the title [WIP] MST-181: Add the ability to sync Program Enrollment Linkage with SSO'd edX account MST-181: Add the ability to sync Program Enrollment Linkage with SSO'd edX account May 5, 2020
Unlinks CourseEnrollments from the ProgramEnrollment by doing the following for
each ProgramCourseEnrollment associated with the Program Enrollment.
1. unenrolling the corresponding user from the course
2. moving the user into the audit track

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is laudable learner empathy; I definitely did not think about this & it's a great idea to do.

@michaelroytman
michaelroytman requested a review from a team May 5, 2020 21:12
@michaelroytman
michaelroytman force-pushed the mroytman/MST-181-program-enrollment-sync-support-tool branch from 2e86388 to 4a99a2c Compare May 5, 2020 21:40

@schenedx schenedx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed again. LGTM

…of linking a learner to a ProgramEnrollment that is already linked to a different edX account.

In this case,
* unenroll the currently linked learner from the courses associated with related ProgramCourseEnrollments
* for each course the is enrolled in as part of a ProgramCourseEnrollment, if the coures has an audit track, then move the enrollment into the audit track. Otherwise, keep the learner's enrollment in the existing track.
* link the new user to the ProgramEnrollment
* enroll the new user in the Master's track in the courses associated with related ProgramCourseEnrollments
@michaelroytman
michaelroytman force-pushed the mroytman/MST-181-program-enrollment-sync-support-tool branch from 4a99a2c to 38c255d Compare May 6, 2020 14:15
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. There were no failures.

@michaelroytman
michaelroytman merged commit 681c3eb into master May 7, 2020
@michaelroytman
michaelroytman deleted the mroytman/MST-181-program-enrollment-sync-support-tool branch May 7, 2020 13:25
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

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.

5 participants