diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index c8b8796420..15d60afa84 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -153,7 +153,7 @@ class RoleBase(AccessRole): # legit get updated. from student.models import CourseAccessRole for user in users: - if user.is_authenticated and user.is_active and not self.has_user(user): + if user.is_authenticated() and user.is_active and not self.has_user(user): entry = CourseAccessRole(user=user, role=self._role_name, course_id=self.course_key, org=self.org) entry.save() if hasattr(user, '_roles'): @@ -349,7 +349,7 @@ class UserBasedRole(object): """ Grant this object's user the object's role for the supplied courses """ - if self.user.is_authenticated and self.user.is_active: + if self.user.is_authenticated() and self.user.is_active: for course_key in course_keys: entry = CourseAccessRole(user=self.user, role=self.role, course_id=course_key, org=course_key.org) entry.save() diff --git a/lms/djangoapps/ccx/api/v0/views.py b/lms/djangoapps/ccx/api/v0/views.py index 7c9106d0dd..8525519154 100644 --- a/lms/djangoapps/ccx/api/v0/views.py +++ b/lms/djangoapps/ccx/api/v0/views.py @@ -37,7 +37,7 @@ from lms.djangoapps.ccx.overrides import ( ) from lms.djangoapps.ccx.utils import ( add_master_course_staff_to_ccx, - assign_coach_role_to_ccx, + assign_staff_role_to_ccx, is_email, get_course_chapters, ) @@ -507,8 +507,8 @@ class CCXListView(GenericAPIView): email_students=True, email_params=email_params, ) - # 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 the coach to the newly created ccx + assign_staff_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, diff --git a/lms/djangoapps/ccx/migrations/0004_change_ccx_coach_to_staff.py b/lms/djangoapps/ccx/migrations/0004_change_ccx_coach_to_staff.py new file mode 100644 index 0000000000..43754c88ba --- /dev/null +++ b/lms/djangoapps/ccx/migrations/0004_change_ccx_coach_to_staff.py @@ -0,0 +1,67 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + +# We're doing something awful here, but it's necessary for the greater good: +from django.contrib.auth.models import User + +from instructor.access import allow_access, revoke_access + +from ccx_keys.locator import CCXLocator +from lms.djangoapps.ccx.utils import ccx_course + +def change_existing_ccx_coaches_to_staff(apps, schema_editor): + """ + Modify all coaches of CCX courses so that they have the staff role on the + CCX course they coach, but retain the CCX Coach role on the parent course. + + Arguments: + apps (Applications): Apps in edX platform. + schema_editor (SchemaEditor): For editing database schema (unused) + + """ + CustomCourseForEdX = apps.get_model('ccx', 'CustomCourseForEdX') + db_alias = schema_editor.connection.alias + list_ccx = CustomCourseForEdX.objects.using(db_alias).all() + for ccx in list_ccx: + ccx_locator = CCXLocator.from_course_locator(ccx.course_id, unicode(ccx.id)) + with ccx_course(ccx_locator) as course: + coach = User.objects.get(id=ccx.coach.id) + allow_access(course, coach, 'staff', send_email=False) + revoke_access(course, coach, 'ccx_coach', send_email=False) + +def revert_ccx_staff_to_coaches(apps, schema_editor): + """ + Modify all staff on CCX courses so that they no longer have the staff role + on the course that they coach. + + Arguments: + apps (Applications): Apps in edX platform. + schema_editor (SchemaEditor): For editing database schema (unused) + + """ + CustomCourseForEdX = apps.get_model('ccx', 'CustomCourseForEdX') + db_alias = schema_editor.connection.alias + list_ccx = CustomCourseForEdX.objects.using(db_alias).all() + for ccx in list_ccx: + ccx_locator = CCXLocator.from_course_locator(ccx.course_id, unicode(ccx.id)) + with ccx_course(ccx_locator) as course: + coach = User.objects.get(id=ccx.coach.id) + allow_access(course, coach, 'ccx_coach', send_email=False) + revoke_access(course, coach, 'staff', send_email=False) + +class Migration(migrations.Migration): + + dependencies = [ + ('ccx', '0001_initial'), + ('ccx', '0002_customcourseforedx_structure_json'), + ('ccx', '0003_add_master_course_staff_in_ccx'), + ] + + operations = [ + migrations.RunPython( + code=change_existing_ccx_coaches_to_staff, + reverse_code=revert_ccx_staff_to_coaches + ) + ] diff --git a/lms/djangoapps/ccx/utils.py b/lms/djangoapps/ccx/utils.py index 66170ddd6c..abb3b6a318 100644 --- a/lms/djangoapps/ccx/utils.py +++ b/lms/djangoapps/ccx/utils.py @@ -292,9 +292,9 @@ def ccx_course(ccx_locator): yield course -def assign_coach_role_to_ccx(ccx_locator, user, master_course_id): +def assign_staff_role_to_ccx(ccx_locator, user, master_course_id): """ - Check if user has ccx_coach role on master course then assign him coach role on ccx only + Check if user has ccx_coach role on master course then assign him staff role on ccx only if role is not already assigned. Because of this coach can open dashboard from master course as well as ccx. :param ccx_locator: CCX key @@ -304,12 +304,12 @@ def assign_coach_role_to_ccx(ccx_locator, user, master_course_id): coach_role_on_master_course = CourseCcxCoachRole(master_course_id) # check if user has coach role on master course if coach_role_on_master_course.has_user(user): - # Check if user has coach role on ccx. - role = CourseCcxCoachRole(ccx_locator) + # Check if user has staff role on ccx. + role = CourseStaffRole(ccx_locator) if not role.has_user(user): - # assign user role coach on ccx + # assign user the staff role on ccx with ccx_course(ccx_locator) as course: - allow_access(course, user, "ccx_coach", send_email=False) + allow_access(course, user, "staff", send_email=False) def is_email(identifier): diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index 76f30fdaf5..9e1136c0f7 100644 --- a/lms/djangoapps/ccx/views.py +++ b/lms/djangoapps/ccx/views.py @@ -56,7 +56,7 @@ from lms.djangoapps.ccx.overrides import ( ) from lms.djangoapps.ccx.utils import ( add_master_course_staff_to_ccx, - assign_coach_role_to_ccx, + assign_staff_role_to_ccx, ccx_course, ccx_students_enrolling_center, get_ccx_for_coach, @@ -147,7 +147,8 @@ def dashboard(request, course, ccx=None): if ccx: ccx_locator = CCXLocator.from_course_locator(course.id, unicode(ccx.id)) - + # At this point we are done with verification that current user is ccx coach. + assign_staff_role_to_ccx(ccx_locator, request.user, course.id) schedule = get_ccx_schedule(course, ccx) grading_policy = get_override_for_ccx( ccx, course, 'grading_policy', course.grading_policy) @@ -239,7 +240,7 @@ def create_ccx(request, course, ccx=None): email_params=email_params, ) - assign_coach_role_to_ccx(ccx_id, request.user, course.id) + assign_staff_role_to_ccx(ccx_id, request.user, course.id) add_master_course_staff_to_ccx(course, ccx_id, ccx.display_name) # using CCX object as sender here.