diff --git a/lms/djangoapps/ccx/api/v0/tests/test_views.py b/lms/djangoapps/ccx/api/v0/tests/test_views.py index 7279b94263..a8c9070df0 100644 --- a/lms/djangoapps/ccx/api/v0/tests/test_views.py +++ b/lms/djangoapps/ccx/api/v0/tests/test_views.py @@ -730,8 +730,8 @@ class CcxDetailTest(CcxRestApiTest): course_id=ccx_course_key, student_email=self.coach.email, auto_enroll=True, - email_students=False, - email_params=email_params, + message_students=False, + message_params=email_params, ) return ccx diff --git a/lms/djangoapps/ccx/api/v0/views.py b/lms/djangoapps/ccx/api/v0/views.py index b3e345a770..8ca15e0650 100644 --- a/lms/djangoapps/ccx/api/v0/views.py +++ b/lms/djangoapps/ccx/api/v0/views.py @@ -505,8 +505,8 @@ class CCXListView(GenericAPIView): course_id=ccx_course_key, student_email=coach.email, auto_enroll=True, - email_students=True, - email_params=email_params, + message_students=True, + message_params=email_params, ) # assign staff role for the coach to the newly created ccx assign_staff_role_to_ccx(ccx_course_key, coach, master_course_object.id) @@ -768,8 +768,8 @@ class CCXDetailView(GenericAPIView): course_id=ccx_course_key, student_email=coach.email, auto_enroll=True, - email_students=True, - email_params=email_params, + message_students=True, + message_params=email_params, ) # make the new coach staff on the CCX assign_staff_role_to_ccx(ccx_course_key, coach, master_course_object.id) diff --git a/lms/djangoapps/ccx/utils.py b/lms/djangoapps/ccx/utils.py index 9f7c0eff39..2afed619dd 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -269,7 +269,7 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke log.info("%s", error) errors.append(error) break - enroll_email(course_key, email, auto_enroll=True, email_students=email_students, email_params=email_params) + enroll_email(course_key, email, auto_enroll=True, message_students=email_students, message_params=email_params) elif action == 'Unenroll' or action == 'revoke': # lint-amnesty, pylint: disable=consider-using-in for identifier in identifiers: try: @@ -278,7 +278,7 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke log.info("%s", exp) errors.append(f"{exp}") continue - unenroll_email(course_key, email, email_students=email_students, email_params=email_params) + unenroll_email(course_key, email, message_students=email_students, message_params=email_params) return errors @@ -348,8 +348,8 @@ def add_master_course_staff_to_ccx(master_course, ccx_key, display_name, send_em course_id=ccx_key, student_email=staff.email, auto_enroll=True, - email_students=send_email, - email_params=email_params, + message_students=send_email, + message_params=email_params, ) # allow 'staff' access on ccx to staff of master course @@ -373,8 +373,8 @@ def add_master_course_staff_to_ccx(master_course, ccx_key, display_name, send_em course_id=ccx_key, student_email=instructor.email, auto_enroll=True, - email_students=send_email, - email_params=email_params, + message_students=send_email, + message_params=email_params, ) # allow 'instructor' access on ccx to instructor of master course @@ -417,8 +417,8 @@ def remove_master_course_staff_from_ccx(master_course, ccx_key, display_name, se unenroll_email( course_id=ccx_key, student_email=staff.email, - email_students=send_email, - email_params=email_params, + message_students=send_email, + message_params=email_params, ) for instructor in list_instructor: @@ -430,6 +430,6 @@ def remove_master_course_staff_from_ccx(master_course, ccx_key, display_name, se unenroll_email( course_id=ccx_key, student_email=instructor.email, - email_students=send_email, - email_params=email_params, + message_students=send_email, + message_params=email_params, ) diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index 3c5f3130a1..7c6a75aaf6 100644 --- a/lms/djangoapps/ccx/views.py +++ b/lms/djangoapps/ccx/views.py @@ -223,8 +223,8 @@ def create_ccx(request, course, ccx=None): course_id=ccx_id, student_email=request.user.email, auto_enroll=True, - email_students=True, - email_params=email_params, + message_students=True, + message_params=email_params, ) assign_staff_role_to_ccx(ccx_id, request.user, course.id) diff --git a/lms/djangoapps/instructor/access.py b/lms/djangoapps/instructor/access.py index 9255d113f0..a5d25769ca 100644 --- a/lms/djangoapps/instructor/access.py +++ b/lms/djangoapps/instructor/access.py @@ -86,8 +86,8 @@ def _change_access(course, user, level, action, send_email=True): course_id=course.id, student_email=user.email, auto_enroll=True, - email_students=send_email, - email_params=email_params, + message_students=send_email, + message_params=email_params, ) role.add_users(user) elif action == 'revoke': diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index 4abf68dec0..55b4b8cf19 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -125,7 +125,7 @@ def get_user_email_language(user): return UserPreference.get_value(user, LANGUAGE_KEY) -def enroll_email(course_id, student_email, auto_enroll=False, email_students=False, message_params=None, language=None): +def enroll_email(course_id, student_email, auto_enroll=False, message_students=False, message_params=None, language=None): """ Enroll a student by email. @@ -133,7 +133,7 @@ def enroll_email(course_id, student_email, auto_enroll=False, email_students=Fal `auto_enroll` determines what is put in CourseEnrollmentAllowed.auto_enroll if auto_enroll is set, then when the email registers, they will be enrolled in the course automatically. - `email_students` determines if student should be notified of action by email. + `message_students` determines if student should be notified of action by email or push message. `message_params` parameters used while parsing message templates (a `dict`). `language` is the language used to render the email. @@ -150,6 +150,8 @@ def enroll_email(course_id, student_email, auto_enroll=False, email_students=Fal 'course_id': str(course_id), }, }) + else: + message_params = {} if previous_state.user and previous_state.user.is_active: # if the student is currently unenrolled, don't enroll them in their # previous mode @@ -167,7 +169,7 @@ def enroll_email(course_id, student_email, auto_enroll=False, email_students=Fal course_mode = previous_state.mode enrollment_obj = CourseEnrollment.enroll_by_email(student_email, course_id, course_mode) - if email_students: + if message_students: message_params['message_type'] = 'enrolled_enroll' message_params['email_address'] = student_email message_params['user_id'] = previous_state.user.id @@ -178,7 +180,7 @@ def enroll_email(course_id, student_email, auto_enroll=False, email_students=Fal cea, _ = CourseEnrollmentAllowed.objects.get_or_create(course_id=course_id, email=student_email) cea.auto_enroll = auto_enroll cea.save() - if email_students: + if message_students: message_params['message_type'] = 'allowed_enroll' message_params['email_address'] = student_email if previous_state.user: @@ -190,74 +192,76 @@ def enroll_email(course_id, student_email, auto_enroll=False, email_students=Fal return previous_state, after_state, enrollment_obj -def unenroll_email(course_id, student_email, email_students=False, email_params=None, language=None): +def unenroll_email(course_id, student_email, message_students=False, message_params=None, language=None): """ Unenroll a student by email. `student_email` is student's emails e.g. "foo@bar.com" - `email_students` determines if student should be notified of action by email. - `email_params` parameters used while parsing email templates (a `dict`). + `message_students` determines if student should be notified of action by email or push message. + `message_params` parameters used while parsing email templates (a `dict`). `language` is the language used to render the email. returns two EmailEnrollmentState's representing state before and after the action. """ previous_state = EmailEnrollmentState(course_id, student_email) - if email_params: - email_params.update({ + if message_params: + message_params.update({ 'app_label': 'instructor', 'push_notification_extra_context': { 'notification_type': 'unenroll', }, }) + else: + message_params = {} if previous_state.enrollment: CourseEnrollment.unenroll_by_email(student_email, course_id) - if email_students: - email_params['message_type'] = 'enrolled_unenroll' - email_params['email_address'] = student_email + if message_students: + message_params['message_type'] = 'enrolled_unenroll' + message_params['email_address'] = student_email if previous_state.user: - email_params['user_id'] = previous_state.user.id - email_params['full_name'] = previous_state.full_name - send_mail_to_student(student_email, email_params, language=language) + message_params['user_id'] = previous_state.user.id + message_params['full_name'] = previous_state.full_name + send_mail_to_student(student_email, message_params, language=language) if previous_state.allowed: CourseEnrollmentAllowed.objects.get(course_id=course_id, email=student_email).delete() - if email_students: - email_params['message_type'] = 'allowed_unenroll' - email_params['email_address'] = student_email + if message_students: + message_params['message_type'] = 'allowed_unenroll' + message_params['email_address'] = student_email if previous_state.user: - email_params['user_id'] = previous_state.user.id + message_params['user_id'] = previous_state.user.id # Since no User object exists for this student there is no "full_name" available. - send_mail_to_student(student_email, email_params, language=language) + send_mail_to_student(student_email, message_params, language=language) after_state = EmailEnrollmentState(course_id, student_email) return previous_state, after_state -def send_beta_role_email(action, user, email_params): +def send_beta_role_email(action, user, message_params): """ Send an email to a user added or removed as a beta tester. `action` is one of 'add' or 'remove' `user` is the User affected - `email_params` parameters used while parsing email templates (a `dict`). + `message_params` parameters used while parsing email templates (a `dict`). """ if action in ('add', 'remove'): - email_params['message_type'] = '%s_beta_tester' % action - email_params['email_address'] = user.email - email_params['user_id'] = user.id - email_params['full_name'] = user.profile.name - email_params['app_label'] = 'instructor' - email_params['push_notification_extra_context'] = { - 'notification_type': email_params['message_type'], - 'course_id': str(getattr(email_params.get('course'), 'id', '')), + message_params['message_type'] = '%s_beta_tester' % action + message_params['email_address'] = user.email + message_params['user_id'] = user.id + message_params['full_name'] = user.profile.name + message_params['app_label'] = 'instructor' + message_params['push_notification_extra_context'] = { + 'notification_type': message_params['message_type'], + 'course_id': str(getattr(message_params.get('course'), 'id', '')), } else: raise ValueError(f"Unexpected action received '{action}' - expected 'add' or 'remove'") trying_to_add_inactive_user = not user.is_active and action == 'add' if not trying_to_add_inactive_user: - send_mail_to_student(user.email, email_params, language=get_user_email_language(user)) + send_mail_to_student(user.email, message_params, language=get_user_email_language(user)) @contextmanager