mattdrayer/api-roles-opaque-keys: Fixed unhandled DoesNotExist exception

lms/djangoapps/api_manager/users/tests.py

@@ -1472,6 +1472,12 @@ class UsersApiTests(ModuleStoreTestCase):
             name=FORUM_ROLE_MODERATOR,
             course_id=course3.id)
 
+        course4 = CourseFactory.create(
+            display_name="COURSE4 NO MODERATOR",
+            start=datetime(2014, 6, 16, 14, 30),
+            end=datetime(2015, 1, 16, 14, 30)
+        )
+
         test_uri = '{}/{}/roles/'.format(self.users_base_uri, self.user.id)
         response = self.do_get(test_uri)
         self.assertEqual(response.status_code, 200)
@@ -1480,8 +1486,9 @@ class UsersApiTests(ModuleStoreTestCase):
         data = [
             {'course_id': unicode(self.course.id), 'role': 'instructor'},
             {'course_id': unicode(course2.id), 'role': 'instructor'},
-            {'course_id': unicode(course3.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)
@@ -1502,6 +1509,20 @@ class UsersApiTests(ModuleStoreTestCase):
         for role in response.data['results']:
             self.assertEqual(role['role'], 'staff')
 
+        # Add a role that does not have a corresponding moderator role configured
+        allow_access(course4, self.user, 'staff')
+        # Now modify the existing no-moderator role using the API, which tries to set the moderator role
+        # Also change one of the existing moderator roles, but call it using the deprecated string version
+        data = [
+            {'course_id': course4.id.to_deprecated_string(), 'role': 'instructor'},
+            {'course_id': course2.id.to_deprecated_string(), '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'], 2)
+
     def test_users_roles_list_put_invalid_user(self):
         test_uri = '{}/2131/roles/'.format(self.users_base_uri)
         data = [{'course_id': unicode(self.course.id), 'role': 'instructor'}]
@@ -1528,6 +1549,9 @@ class UsersApiTests(ModuleStoreTestCase):
         data = []
         response = self.do_put(test_uri, data)
         self.assertEqual(response.status_code, 400)
+        data = [{'course_id': unicode(self.course.id), 'role': 'invalid-role'}]
+        response = self.do_put(test_uri, data)
+        self.assertEqual(response.status_code, 400)
 
     def test_users_roles_courses_detail_delete(self):
         test_uri = '{}/{}/roles/'.format(self.users_base_uri, self.user.id)

lms/djangoapps/api_manager/users/views.py

@@ -18,12 +18,12 @@ from rest_framework.response import Response
 from courseware import grades, module_render
 from courseware.model_data import FieldDataCache
 from courseware.models import StudentModule
-from courseware.views import save_child_position, get_current_child
-from django_comment_common.models import FORUM_ROLE_MODERATOR
+from django_comment_common.models import Role, FORUM_ROLE_MODERATOR
 from instructor.access import revoke_access, update_forum_role
 from lang_pref import LANGUAGE_KEY
 from lms.lib.comment_client.user import User as CommentUser
 from lms.lib.comment_client.utils import CommentClientRequestError
+from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from student.models import CourseEnrollment, PasswordHistory, UserProfile
 from openedx.core.djangoapps.user_api.models import UserPreference
 from student.roles import CourseAccessRole, CourseInstructorRole, CourseObserverRole, CourseStaffRole, UserBasedRole
@@ -144,7 +144,16 @@ def _manage_role(course_descriptor, user, role, action):
             new_role = CourseAccessRole(user=user, role=role, course_id=course_descriptor.id, org=course_descriptor.org)
             new_role.save()
         if role in forum_moderator_roles:
-            update_forum_role(course_descriptor.id, user, FORUM_ROLE_MODERATOR, 'allow')
+            try:
+                dep_string = course_descriptor.id.to_deprecated_string()
+                ssck = SlashSeparatedCourseKey.from_deprecated_string(dep_string)
+                update_forum_role(ssck, user, FORUM_ROLE_MODERATOR, 'allow')
+            except Role.DoesNotExist:
+                try:
+                    update_forum_role(course_descriptor.id, user, FORUM_ROLE_MODERATOR, 'allow')
+                except Role.DoesNotExist:
+                    pass
+
     elif action is 'revoke':
         revoke_access(course_descriptor, user, role)
         if role in forum_moderator_roles:
@@ -157,7 +166,15 @@ def _manage_role(course_descriptor, user, role, action):
             queryset = user_instructor_courses | user_staff_courses
             queryset = queryset.filter(course_id=course_descriptor.id)
             if len(queryset) == 0:
-                update_forum_role(course_descriptor.id, user, FORUM_ROLE_MODERATOR, 'revoke')
+                try:
+                    dep_string = course_descriptor.id.to_deprecated_string()
+                    ssck = SlashSeparatedCourseKey.from_deprecated_string(dep_string)
+                    update_forum_role(ssck, user, FORUM_ROLE_MODERATOR, 'revoke')
+                except Role.DoesNotExist:
+                    try:
+                        update_forum_role(course_descriptor.id, user, FORUM_ROLE_MODERATOR, 'revoke')
+                    except Role.DoesNotExist:
+                        pass
 
 
 class UsersList(SecureListAPIView):