diff --git a/lms/djangoapps/ccx/api/v0/tests/test_views.py b/lms/djangoapps/ccx/api/v0/tests/test_views.py index c2096d9a295e..ee1d30b9053d 100644 --- a/lms/djangoapps/ccx/api/v0/tests/test_views.py +++ b/lms/djangoapps/ccx/api/v0/tests/test_views.py @@ -8,6 +8,7 @@ import string import urllib import urlparse +from itertools import izip import ddt import mock @@ -30,6 +31,7 @@ from courseware import courses from ccx_keys.locator import CCXLocator from student.models import CourseEnrollment +from instructor.access import list_with_level from instructor.enrollment import ( enroll_email, get_email_params, @@ -38,6 +40,7 @@ from lms.djangoapps.ccx.models import CcxFieldOverride, CustomCourseForEdX from lms.djangoapps.ccx.overrides import override_field_for_ccx from lms.djangoapps.ccx.tests.utils import CcxTestCase +from lms.djangoapps.ccx.utils import ccx_course as ccx_course_cm from opaque_keys.edx.keys import CourseKey from student.roles import CourseCcxCoachRole from student.tests.factories import AdminFactory @@ -459,6 +462,38 @@ def test_post_list(self): self.assertEqual(len(outbox), 1) self.assertIn(self.coach.email, outbox[0].recipients()) # pylint: disable=no-member + def test_post_list_staff_master_course_in_ccx(self): + """ + Specific test to check that the staff and instructor of the master + course are assigned to the CCX. + """ + outbox = self.get_outbox() + data = { + 'master_course_id': self.master_course_key_str, + 'max_students_allowed': 111, + 'display_name': 'CCX Test Title', + 'coach_email': self.coach.email + } + resp = self.client.post(self.list_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.assertEqual(resp.status_code, status.HTTP_201_CREATED) + # check that only one email has been sent and it is to to the coach + self.assertEqual(len(outbox), 1) + self.assertIn(self.coach.email, outbox[0].recipients()) # pylint: disable=no-member + + list_staff_master_course = list_with_level(self.course, 'staff') + list_instructor_master_course = list_with_level(self.course, 'instructor') + course_key = CourseKey.from_string(resp.data.get('ccx_course_id')) # pylint: disable=no-member + with ccx_course_cm(course_key) as course_ccx: + list_staff_ccx_course = list_with_level(course_ccx, 'staff') + list_instructor_ccx_course = list_with_level(course_ccx, 'instructor') + + self.assertEqual(len(list_staff_master_course), len(list_staff_ccx_course)) + for course_user, ccx_user in izip(sorted(list_staff_master_course), sorted(list_staff_ccx_course)): + self.assertEqual(course_user, ccx_user) + self.assertEqual(len(list_instructor_master_course), len(list_instructor_ccx_course)) + for course_user, ccx_user in izip(sorted(list_instructor_master_course), sorted(list_instructor_ccx_course)): + self.assertEqual(course_user, ccx_user) + @attr('shard_1') @ddt.ddt diff --git a/lms/djangoapps/ccx/api/v0/views.py b/lms/djangoapps/ccx/api/v0/views.py index fac2525a6f96..973c3cb1770f 100644 --- a/lms/djangoapps/ccx/api/v0/views.py +++ b/lms/djangoapps/ccx/api/v0/views.py @@ -33,6 +33,7 @@ override_field_for_ccx, ) from lms.djangoapps.ccx.utils import ( + add_master_course_staff_to_ccx, assign_coach_role_to_ccx, is_email, ) @@ -433,6 +434,13 @@ def post(self, request): ) # assign coach role for the coach to the newly created ccx assign_coach_role_to_ccx(ccx_course_key, coach, master_course_object.id) + # assign staff role for all the staff and instructor of the master course to the newly created ccx + add_master_course_staff_to_ccx( + master_course_object, + ccx_course_key, + ccx_course_object.display_name, + send_email=False + ) serializer = self.get_serializer(ccx_course_object) return Response( diff --git a/lms/djangoapps/ccx/tests/test_utils.py b/lms/djangoapps/ccx/tests/test_utils.py index 0601a92e0ffb..ce6180427339 100644 --- a/lms/djangoapps/ccx/tests/test_utils.py +++ b/lms/djangoapps/ccx/tests/test_utils.py @@ -1,6 +1,7 @@ """ test utils """ +import uuid from nose.plugins.attrib import attr from ccx_keys.locator import CCXLocator @@ -21,10 +22,14 @@ from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.django import modulestore -from lms.djangoapps.instructor.access import list_with_level, allow_access - -from lms.djangoapps.ccx.utils import add_master_course_staff_to_ccx -from lms.djangoapps.ccx.views import ccx_course +from lms.djangoapps.instructor.access import ( + allow_access, + list_with_level, +) +from lms.djangoapps.ccx.utils import ( + add_master_course_staff_to_ccx, + ccx_course, +) from lms.djangoapps.ccx.tests.factories import CcxFactory from lms.djangoapps.ccx.tests.utils import CcxTestCase @@ -62,7 +67,7 @@ def test_ccx_locator(self): self.assertEqual(result, ccx) -class TestStaffOnCCX(CcxTestCase, SharedModuleStoreTestCase): +class TestStaffOnCCX(CcxTestCase): """ Tests for staff on ccx courses. """ @@ -87,20 +92,21 @@ def setUp(self): allow_access(self.course, instructor, 'instructor') self.assertTrue(CourseInstructorRole(self.course.id).has_user(instructor)) + self.make_coach() + self.ccx = self.make_ccx() + self.ccx_locator = CCXLocator.from_course_locator(self.course.id, self.ccx.id) + def test_add_master_course_staff_to_ccx(self): """ Test add staff of master course to ccx course """ - self.make_coach() - ccx = self.make_ccx() - ccx_locator = CCXLocator.from_course_locator(self.course.id, ccx.id) - add_master_course_staff_to_ccx(self.course, ccx_locator, ccx.display_name) + add_master_course_staff_to_ccx(self.course, self.ccx_locator, self.ccx.display_name) # assert that staff and instructors of master course has staff and instructor roles on ccx list_staff_master_course = list_with_level(self.course, 'staff') list_instructor_master_course = list_with_level(self.course, 'instructor') - with ccx_course(ccx_locator) as course_ccx: + with ccx_course(self.ccx_locator) as course_ccx: list_staff_ccx_course = list_with_level(course_ccx, 'staff') self.assertEqual(len(list_staff_master_course), len(list_staff_ccx_course)) self.assertEqual(list_staff_master_course[0].email, list_staff_ccx_course[0].email) @@ -108,3 +114,69 @@ def test_add_master_course_staff_to_ccx(self): list_instructor_ccx_course = list_with_level(course_ccx, 'instructor') self.assertEqual(len(list_instructor_ccx_course), len(list_instructor_master_course)) self.assertEqual(list_instructor_ccx_course[0].email, list_instructor_master_course[0].email) + + def test_add_master_course_staff_to_ccx_display_name(self): + """ + Test add staff of master course to ccx course. + Specific test to check that a passed display name is in the + subject of the email sent to the enrolled users. + """ + outbox = self.get_outbox() + # create a unique display name + display_name = 'custom_display_{}'.format(uuid.uuid4()) + list_staff_master_course = list_with_level(self.course, 'staff') + list_instructor_master_course = list_with_level(self.course, 'instructor') + self.assertEqual(len(outbox), 0) + # give access to the course staff/instructor + add_master_course_staff_to_ccx(self.course, self.ccx_locator, display_name) + self.assertEqual(len(outbox), len(list_staff_master_course) + len(list_instructor_master_course)) + for email in outbox: + self.assertIn(display_name, email.subject) + + def test_add_master_course_staff_to_ccx_idempotent(self): + """ + Test add staff of master course to ccx course multiple time will + not result in multiple enrollments. + """ + outbox = self.get_outbox() + list_staff_master_course = list_with_level(self.course, 'staff') + list_instructor_master_course = list_with_level(self.course, 'instructor') + self.assertEqual(len(outbox), 0) + + # run the assignment the first time + add_master_course_staff_to_ccx(self.course, self.ccx_locator, self.ccx.display_name) + self.assertEqual(len(outbox), len(list_staff_master_course) + len(list_instructor_master_course)) + with ccx_course(self.ccx_locator) as course_ccx: + list_staff_ccx_course = list_with_level(course_ccx, 'staff') + list_instructor_ccx_course = list_with_level(course_ccx, 'instructor') + self.assertEqual(len(list_staff_master_course), len(list_staff_ccx_course)) + for user in list_staff_master_course: + self.assertIn(user, list_staff_ccx_course) + self.assertEqual(len(list_instructor_master_course), len(list_instructor_ccx_course)) + for user in list_instructor_master_course: + self.assertIn(user, list_instructor_ccx_course) + + # run the assignment again + add_master_course_staff_to_ccx(self.course, self.ccx_locator, self.ccx.display_name) + # there are no new duplicated email + self.assertEqual(len(outbox), len(list_staff_master_course) + len(list_instructor_master_course)) + # there are no duplicated staffs + with ccx_course(self.ccx_locator) as course_ccx: + list_staff_ccx_course = list_with_level(course_ccx, 'staff') + list_instructor_ccx_course = list_with_level(course_ccx, 'instructor') + self.assertEqual(len(list_staff_master_course), len(list_staff_ccx_course)) + for user in list_staff_master_course: + self.assertIn(user, list_staff_ccx_course) + self.assertEqual(len(list_instructor_master_course), len(list_instructor_ccx_course)) + for user in list_instructor_master_course: + self.assertIn(user, list_instructor_ccx_course) + + def test_add_master_course_staff_to_ccx_no_email(self): + """ + Test add staff of master course to ccx course without + sending enrollment email. + """ + outbox = self.get_outbox() + self.assertEqual(len(outbox), 0) + add_master_course_staff_to_ccx(self.course, self.ccx_locator, self.ccx.display_name, send_email=False) + self.assertEqual(len(outbox), 0) diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index ebbab7540c38..08838852b5d9 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -15,7 +15,10 @@ from courseware.tests.factories import StudentModuleFactory from courseware.tests.helpers import LoginEnrollmentTestCase from courseware.tabs import get_course_tab_list -from instructor.access import list_with_level, allow_access +from instructor.access import ( + allow_access, + list_with_level, +) from django.conf import settings from django.core.urlresolvers import reverse, resolve diff --git a/lms/djangoapps/ccx/utils.py b/lms/djangoapps/ccx/utils.py index 765476c6d892..86b487102bc2 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -21,7 +21,11 @@ get_email_params, unenroll_email, ) -from instructor.access import allow_access, list_with_level, revoke_access +from instructor.access import ( + allow_access, + list_with_level, + revoke_access, +) from instructor.views.tools import get_student_from_identifier from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from student.models import CourseEnrollment @@ -263,7 +267,7 @@ def is_email(identifier): return True -def add_master_course_staff_to_ccx(master_course, ccx_key, display_name): +def add_master_course_staff_to_ccx(master_course, ccx_key, display_name, send_email=True): """ Added staff role on ccx to all the staff members of master course. @@ -277,31 +281,37 @@ def add_master_course_staff_to_ccx(master_course, ccx_key, display_name): with ccx_course(ccx_key) as course_ccx: email_params = get_email_params(course_ccx, auto_enroll=True, course_key=ccx_key, display_name=display_name) + list_staff_ccx = list_with_level(course_ccx, 'staff') + list_instructor_ccx = list_with_level(course_ccx, 'instructor') for staff in list_staff: - # allow 'staff' access on ccx to staff of master course - allow_access(course_ccx, staff, 'staff') - - # Enroll the staff in the ccx - enroll_email( - course_id=ccx_key, - student_email=staff.email, - auto_enroll=True, - email_students=True, - email_params=email_params, - ) + # this call should be idempotent + if staff not in list_staff_ccx: + # allow 'staff' access on ccx to staff of master course + allow_access(course_ccx, staff, 'staff') + + # Enroll the staff in the ccx + enroll_email( + course_id=ccx_key, + student_email=staff.email, + auto_enroll=True, + email_students=send_email, + email_params=email_params, + ) for instructor in list_instructor: - # allow 'instructor' access on ccx to instructor of master course - allow_access(course_ccx, instructor, 'instructor') - - # Enroll the instructor in the ccx - enroll_email( - course_id=ccx_key, - student_email=instructor.email, - auto_enroll=True, - email_students=True, - email_params=email_params, - ) + # this call should be idempotent + if instructor not in list_instructor_ccx: + # allow 'instructor' access on ccx to instructor of master course + allow_access(course_ccx, instructor, 'instructor') + + # Enroll the instructor in the ccx + enroll_email( + course_id=ccx_key, + student_email=instructor.email, + auto_enroll=True, + email_students=send_email, + email_params=email_params, + ) def reverse_add_master_course_staff_to_ccx(master_course, ccx_key, display_name):