mattdrayer/api-course-observer-role: Added new 'observer' role, plus batch PUT operation

common/djangoapps/student/roles.py

@@ -247,6 +247,24 @@ class CourseSalesAdminRole(CourseRole):
 
 
 @register_access_role
+class CourseObserverRole(CourseRole):
+    """A course Observer"""
+    ROLE = 'observer'
+
+    def __init__(self, *args, **kwargs):
+        super(CourseObserverRole, self).__init__(self.ROLE, *args, **kwargs)
+
+
+@register_access_role
+class CourseObserverRole(CourseRole):
+    """A course Observer"""
+    ROLE = 'observer'
+
+    def __init__(self, *args, **kwargs):
+        super(CourseObserverRole, self).__init__(self.ROLE, *args, **kwargs)
+
+
+@register_access_role
 class CourseBetaTesterRole(CourseRole):
     """A course Beta Tester"""
     ROLE = 'beta_testers'

lms/djangoapps/api_manager/courses/tests.py

@@ -1788,7 +1788,7 @@ class CoursesApiTests(TestCase):
     def test_courses_roles_list_get(self):
         allow_access(self.course, self.users[0], 'staff')
         allow_access(self.course, self.users[1], 'instructor')
-        allow_access(self.course, self.users[2], 'staff')
+        allow_access(self.course, self.users[2], 'observer')
         test_uri = '/api/courses/{}/roles/'.format(unicode(self.course.id))
         response = self.do_get(test_uri)
         self.assertEqual(response.status_code, 200)
@@ -1829,7 +1829,7 @@ class CoursesApiTests(TestCase):
         test_uri = '/api/courses/{}/roles/'.format(unicode(self.course.id))
         data = {'user_id': 23423, 'role': 'instructor'}
         response = self.do_post(test_uri, data)
-        self.assertEqual(response.status_code, 404)
+        self.assertEqual(response.status_code, 400)
 
     def test_courses_roles_list_post_invalid_role(self):
         test_uri = '/api/courses/{}/roles/'.format(unicode(self.course.id))
@@ -1844,9 +1844,11 @@ class CoursesApiTests(TestCase):
         self.assertEqual(response.status_code, 201)
 
         response = self.do_get(test_uri)
+        print response.data
         self.assertEqual(len(response.data), 1)
 
         delete_uri = '{}instructor/users/{}'.format(test_uri, self.users[0].id)
+        print delete_uri
         response = self.do_delete(delete_uri)
         self.assertEqual(response.status_code, 204)
 

lms/djangoapps/api_manager/courses/views.py

@@ -24,7 +24,7 @@ from courseware.views import get_static_tab_contents
 from django_comment_common.models import FORUM_ROLE_MODERATOR
 from instructor.access import allow_access, revoke_access, update_forum_role
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
-from student.roles import CourseInstructorRole, CourseStaffRole
+from student.roles import CourseInstructorRole, CourseStaffRole, CourseObserverRole, UserBasedRole
 
 from xmodule.modulestore.django import modulestore
 
@@ -275,6 +275,30 @@ def _parse_updates_html(html):
     return result
 
 
+def _manage_role(course_descriptor, user, role, action):
+    """
+    Helper method for managing course/forum roles
+    """
+    forum_moderator_roles = ('instructor', 'staff')
+    if action is 'allow':
+        allow_access(course_descriptor, user, role)
+        if role in forum_moderator_roles:
+            update_forum_role(course_descriptor.id, user, FORUM_ROLE_MODERATOR, 'allow')
+    elif action is 'revoke':
+        if role in forum_moderator_roles:
+            # There's a possibilty that the user may play more than one role in a course
+            # And that more than one of these roles allow for forum moderation
+            # So we need to confirm the current role is the only one for this user for this course
+            # Before we can safely remove the corresponding forum moderator role
+            user_instructor_courses = UserBasedRole(user, CourseInstructorRole.ROLE).courses_with_role()
+            user_staff_courses = UserBasedRole(user, CourseStaffRole.ROLE).courses_with_role()
+            queryset = user_instructor_courses | user_staff_courses
+            queryset = queryset.filter(course_id=course_descriptor.id)
+            if len(queryset) <= 1:
+                update_forum_role(course_descriptor.id, user, FORUM_ROLE_MODERATOR, 'allow')
+        revoke_access(course_descriptor, user, role)
+
+
 class CourseContentList(SecureAPIView):
     """
     **Use Case**
@@ -1666,7 +1690,7 @@ class CoursesRolesList(SecureAPIView):
         GET /api/courses/{course_id}/roles/
         """
         course_id = self.kwargs['course_id']
-        course_descriptor, course_key, course_content = get_course(self.request, self.request.user, course_id, depth=None)  # pylint: disable=W0612
+        course_descriptor, course_key, course_content = get_course(self.request, self.request.user, course_id)  # pylint: disable=W0612
         if not course_descriptor:
             raise Http404
 
@@ -1680,6 +1704,10 @@ class CoursesRolesList(SecureAPIView):
         for admin in staff:
             response_data.append({'id': admin.id, 'role': 'staff'})
 
+        observers = CourseObserverRole(course_key).users_with_role()
+        for observer in observers:
+            response_data.append({'id': observer.id, 'role': 'observer'})
+
         user_id = self.request.QUERY_PARAMS.get('user_id', None)
         if user_id:
             response_data = list([item for item in response_data if int(item['id']) == int(user_id)])
@@ -1691,7 +1719,7 @@ class CoursesRolesList(SecureAPIView):
         POST /api/courses/{course_id}/roles/
         """
         course_id = self.kwargs['course_id']
-        course_descriptor, course_key, course_content = get_course(self.request, self.request.user, course_id, depth=None)  # pylint: disable=W0612
+        course_descriptor, course_key, course_content = get_course(self.request, self.request.user, course_id)  # pylint: disable=W0612
         if not course_descriptor:
             raise Http404
 
@@ -1699,12 +1727,11 @@ class CoursesRolesList(SecureAPIView):
         try:
             user = User.objects.get(id=user_id)
         except ObjectDoesNotExist:
-            raise Http404
+            return Response({}, status=status.HTTP_400_BAD_REQUEST)
 
         role = request.DATA.get('role', None)
         try:
-            allow_access(course_descriptor, user, role)
-            update_forum_role(course_key, user, FORUM_ROLE_MODERATOR, 'allow')
+            _manage_role(course_descriptor, user, role, 'allow')
         except ValueError:
             return Response({}, status=status.HTTP_400_BAD_REQUEST)
         return Response(request.DATA, status=status.HTTP_201_CREATED)
@@ -1722,21 +1749,17 @@ class CoursesRolesUsersDetail(SecureAPIView):
         """
         DELETE /api/courses/{course_id}/roles/{role}/users/{user_id}
         """
-        course_id = self.kwargs['course_id']
-        course_descriptor, course_key, course_content = get_course(self.request, self.request.user, course_id, depth=None)  # pylint: disable=W0612
+        course_descriptor, course_key, course_content = get_course(self.request, self.request.user, course_id)  # pylint: disable=W0612
         if not course_descriptor:
             return Response({}, status=status.HTTP_404_NOT_FOUND)
 
-        user_id = self.kwargs['user_id']
         try:
             user = User.objects.get(id=user_id)
         except ObjectDoesNotExist:
             return Response({}, status=status.HTTP_404_NOT_FOUND)
 
-        role = self.kwargs['role']
         try:
-            revoke_access(course_descriptor, user, role)
-            update_forum_role(course_key, user, FORUM_ROLE_MODERATOR, 'revoke')
+            _manage_role(course_descriptor, user, role, 'revoke')
         except ValueError:
             return Response({}, status=status.HTTP_404_NOT_FOUND)
 

lms/djangoapps/api_manager/users/tests.py

@@ -110,6 +110,17 @@ class UsersApiTests(ModuleStoreTestCase):
             uri, headers=headers, content_type='application/json', data=json_data)
         return response
 
+    def do_put(self, uri, data):
+        """Submit an HTTP PUT request"""
+        headers = {
+            'X-Edx-Api-Key': str(TEST_API_KEY),
+        }
+        json_data = json.dumps(data)
+
+        response = self.client.put(
+            uri, headers=headers, content_type='application/json', data=json_data)
+        return response
+
     def do_get(self, uri):
         """Submit an HTTP GET request"""
         headers = {
@@ -1414,7 +1425,7 @@ class UsersApiTests(ModuleStoreTestCase):
         test_uri = '/api/users/{}/roles/'.format(self.user.id)
         data = {'course_id': self.test_bogus_course_id, 'role': 'instructor'}
         response = self.do_post(test_uri, data)
-        self.assertEqual(response.status_code, 404)
+        self.assertEqual(response.status_code, 400)
 
     def test_users_roles_list_post_invalid_role(self):
         test_uri = '/api/users/{}/roles/'.format(self.user.id)
@@ -1422,6 +1433,82 @@ class UsersApiTests(ModuleStoreTestCase):
         response = self.do_post(test_uri, data)
         self.assertEqual(response.status_code, 400)
 
+    def test_users_roles_list_put(self):
+        course2 = CourseFactory.create(
+            display_name="TEST COURSE2",
+            start=datetime(2014, 6, 16, 14, 30),
+            end=datetime(2015, 1, 16, 14, 30)
+        )
+        Role.objects.get_or_create(
+            name=FORUM_ROLE_MODERATOR,
+            course_id=course2.id)
+
+        course3 = CourseFactory.create(
+            display_name="TEST COURSE3",
+            start=datetime(2014, 6, 16, 14, 30),
+            end=datetime(2015, 1, 16, 14, 30)
+        )
+        Role.objects.get_or_create(
+            name=FORUM_ROLE_MODERATOR,
+            course_id=course3.id)
+
+        test_uri = '/api/users/{}/roles/'.format(self.user.id)
+        response = self.do_get(test_uri)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data['count'], 0)
+
+        data = [
+            {'course_id': unicode(self.course.id), 'role': 'instructor'},
+            {'course_id': unicode(course2.id), 'role': 'instructor'},
+            {'course_id': unicode(course3.id), 'role': 'instructor'}
+        ]
+        response = self.do_put(test_uri, data)
+        self.assertEqual(response.status_code, 200)
+        response = self.do_get(test_uri)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data['count'], 3)
+        for role in response.data['results']:
+            self.assertEqual(role['role'], 'instructor')
+
+        data = [
+            {'course_id': unicode(self.course.id), 'role': 'staff'},
+            {'course_id': unicode(course2.id), 'role': 'staff'},
+        ]
+        response = self.do_put(test_uri, data)
+        self.assertEqual(response.status_code, 200)
+        response = self.do_get(test_uri)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data['count'], 2)
+        for role in response.data['results']:
+            self.assertEqual(role['role'], 'staff')
+
+    def test_users_roles_list_put_invalid_user(self):
+        test_uri = '/api/users/2131/roles/'
+        data = [{'course_id': unicode(self.course.id), 'role': 'instructor'}]
+        response = self.do_put(test_uri, data)
+        self.assertEqual(response.status_code, 404)
+
+    def test_users_roles_list_put_invalid_course(self):
+        test_uri = '/api/users/{}/roles/'.format(self.user.id)
+        data = {'course_id': unicode(self.course.id), 'role': 'instructor'}
+        response = self.do_post(test_uri, data)
+        self.assertEqual(response.status_code, 201)
+
+        data = [{'course_id': self.test_bogus_course_id, 'role': 'instructor'}]
+        response = self.do_put(test_uri, data)
+        self.assertEqual(response.status_code, 400)
+
+        response = self.do_get(test_uri)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.data['count'], 1)
+        self.assertEqual(response.data['results'][0]['course_id'], unicode(self.course.id))
+
+    def test_users_roles_list_put_invalid_roles(self):
+        test_uri = '/api/users/{}/roles/'.format(self.user.id)
+        data = []
+        response = self.do_put(test_uri, data)
+        self.assertEqual(response.status_code, 400)
+
     def test_users_roles_courses_detail_delete(self):
         test_uri = '/api/users/{}/roles/'.format(self.user.id)
         data = {'course_id': unicode(self.course.id), 'role': 'instructor'}

lms/djangoapps/api_manager/users/views.py

@@ -27,6 +27,7 @@ from lms.lib.comment_client.utils import CommentClientRequestError
 from student.models import CourseEnrollment, PasswordHistory, UserProfile
 from openedx.core.djangoapps.user_api.models import UserPreference
 from student.roles import CourseInstructorRole, CourseStaffRole, UserBasedRole
+from student.roles import CourseInstructorRole, CourseObserverRole, CourseStaffRole, UserBasedRole
 from util.bad_request_rate_limiter import BadRequestRateLimiter
 from util.password_policy_validators import (
     validate_password_length, validate_password_complexity,
@@ -94,6 +95,30 @@ def _save_content_position(request, user, course_key, position):
     return unicode(saved_content.scope_ids.usage_id)
 
 
+def _manage_role(course_descriptor, user, role, action):
+    """
+    Helper method for managing course/forum roles
+    """
+    forum_moderator_roles = ('instructor', 'staff')
+    if action is 'allow':
+        allow_access(course_descriptor, user, role)
+        if role in forum_moderator_roles:
+            update_forum_role(course_descriptor.id, user, FORUM_ROLE_MODERATOR, 'allow')
+    elif action is 'revoke':
+        if role in forum_moderator_roles:
+            # There's a possibilty that the user may play more than one role in a course
+            # And that more than one of these roles allow for forum moderation
+            # So we need to confirm the current role is the only one for this user for this course
+            # Before we can safely remove the corresponding forum moderator role
+            user_instructor_courses = UserBasedRole(user, CourseInstructorRole.ROLE).courses_with_role()
+            user_staff_courses = UserBasedRole(user, CourseStaffRole.ROLE).courses_with_role()
+            queryset = user_instructor_courses | user_staff_courses
+            queryset = queryset.filter(course_id=course_descriptor.id)
+            if len(queryset) == 1:
+                update_forum_role(course_descriptor.id, user, FORUM_ROLE_MODERATOR, 'allow')
+        revoke_access(course_descriptor, user, role)
+
+
 class UsersList(SecureListAPIView):
     """
     ### The UsersList view allows clients to retrieve/append a list of User entities
@@ -1083,16 +1108,20 @@ class UsersRolesList(SecureListAPIView):
     ### The UsersRolesList view allows clients to interact with the User's roleset
     - URI: ```/api/users/{user_id}/courses/{course_id}/roles```
     - GET: Returns a JSON representation of the specified Course roleset
+    - POST: Adds a new role to the User's roleset
+    - PUT: Replace the existing roleset with the provided roleset
 
     ### Use Cases/Notes:
     * Use the UsersRolesList view to manage a User's TA status
     * Use GET to retrieve the set of roles a User plays for a particular course
+    * Use POST to grant a role to a particular User
+    * Use PUT to perform a batch replacement of all roles assigned to a User
     """
 
     serializer_class = UserRolesSerializer
 
     def get_queryset(self):
-        user_id = self.kwargs['user_id']
+        user_id = self.kwargs.get('user_id')
         try:
             user = User.objects.get(id=user_id)
         except ObjectDoesNotExist:
@@ -1100,11 +1129,12 @@ class UsersRolesList(SecureListAPIView):
 
         instructor_courses = UserBasedRole(user, CourseInstructorRole.ROLE).courses_with_role()
         staff_courses = UserBasedRole(user, CourseStaffRole.ROLE).courses_with_role()
-        queryset = instructor_courses | staff_courses
+        observer_courses = UserBasedRole(user, CourseObserverRole.ROLE).courses_with_role()
+        queryset = instructor_courses | staff_courses | observer_courses
 
         course_id = self.request.QUERY_PARAMS.get('course_id', None)
         if course_id:
-            course_descriptor, course_key, course_content = get_course(self.request, user, course_id, depth=None)  # pylint: disable=W0612
+            course_descriptor, course_key, course_content = get_course(self.request, user, course_id)  # pylint: disable=W0612
             if not course_descriptor:
                 raise Http404
             queryset = queryset.filter(course_id=course_key)
@@ -1115,25 +1145,54 @@ class UsersRolesList(SecureListAPIView):
         """
         POST /api/users/{user_id}/roles/
         """
-        user_id = self.kwargs['user_id']
         try:
             user = User.objects.get(id=user_id)
         except ObjectDoesNotExist:
             raise Http404
 
         course_id = request.DATA.get('course_id', None)
-        course_descriptor, course_key, course_content = get_course(self.request, self.request.user, course_id, depth=None)  # pylint: disable=W0612
+        course_descriptor, course_key, course_content = get_course(self.request, self.request.user, course_id)  # pylint: disable=W0612
         if not course_descriptor:
-            raise Http404
+            return Response({}, status=status.HTTP_400_BAD_REQUEST)
 
         role = request.DATA.get('role', None)
         try:
-            allow_access(course_descriptor, user, role)
-            update_forum_role(course_key, user, FORUM_ROLE_MODERATOR, 'allow')
+            _manage_role(course_descriptor, user, role, 'allow')
         except ValueError:
             return Response({}, status=status.HTTP_400_BAD_REQUEST)
         return Response(request.DATA, status=status.HTTP_201_CREATED)
 
+    def put(self, request, user_id):
+        """
+        PUT /api/users/{user_id}/roles/
+        """
+        try:
+            user = User.objects.get(id=user_id)
+        except ObjectDoesNotExist:
+            raise Http404
+
+        if not len(request.DATA):
+            return Response({}, status=status.HTTP_400_BAD_REQUEST)
+        current_roles = self.get_queryset()
+        for current_role in current_roles:
+            course_descriptor, course_key, course_content = get_course(request, user, unicode(current_role.course_id))  # pylint: disable=W0612
+            _manage_role(course_descriptor, user, current_role.role, 'revoke')
+        for role in request.DATA:
+            try:
+                course_id = role['course_id']
+                course_descriptor, course_key, course_content = get_course(request, user, course_id)  # pylint: disable=W0612
+                if not course_descriptor:
+                    raise ValueError  # ValueError is also thrown by the following role setters
+                _manage_role(course_descriptor, user, role['role'], 'allow')
+            except ValueError:
+                # Restore the current roleset to the User
+                for current_role in current_roles:
+                    course_descriptor, course_key, course_content = get_course(
+                        request, user, unicode(current_role.course_id))  # pylint: disable=W0612
+                    _manage_role(course_descriptor, user, current_role.role, 'allow')
+                return Response({}, status=status.HTTP_400_BAD_REQUEST)
+        return Response(request.DATA, status=status.HTTP_200_OK)
+
 
 class UsersRolesCoursesDetail(SecureAPIView):
     """
@@ -1147,23 +1206,19 @@ class UsersRolesCoursesDetail(SecureAPIView):
         """
         DELETE /api/users/{user_id}/roles/{role}/courses/{course_id}
         """
-        course_id = self.kwargs['course_id']
-        print course_id
-        course_descriptor, course_key, course_content = get_course(self.request, self.request.user, course_id, depth=None)  # pylint: disable=W0612
+        course_descriptor, course_key, course_content = get_course(self.request, self.request.user, course_id)  # pylint: disable=W0612
         if not course_descriptor:
             return Response({}, status=status.HTTP_404_NOT_FOUND)
 
-        user_id = self.kwargs['user_id']
-        print user_id
         try:
             user = User.objects.get(id=user_id)
         except ObjectDoesNotExist:
             return Response({}, status=status.HTTP_404_NOT_FOUND)
 
-        role = self.kwargs['role']
         try:
             revoke_access(course_descriptor, user, role)
-            update_forum_role(course_key, user, FORUM_ROLE_MODERATOR, 'revoke')
+            if role in ('instructor', 'staff'):
+                update_forum_role(course_key, user, FORUM_ROLE_MODERATOR, 'revoke')
         except ValueError:
             return Response({}, status=status.HTTP_404_NOT_FOUND)
 

lms/djangoapps/instructor/access.py

@@ -17,14 +17,15 @@ from student.roles import (
     CourseInstructorRole,
     CourseCcxCoachRole,
     CourseStaffRole,
+    CourseObserverRole,
 )
 
-
 log = logging.getLogger(__name__)
 
 ROLES = {
     'beta': CourseBetaTesterRole,
     'instructor': CourseInstructorRole,
+    'observer': CourseObserverRole,
     'staff': CourseStaffRole,
     'ccx_coach': CourseCcxCoachRole,
 }