Merge pull request #2966 from edx/waheed/lms2346-fix-assigning-forum-roles-beta-membership-tab

Fixed assigning forum roles from beta dashboard membership.

lms/djangoapps/instructor/tests/test_api.py

@@ -14,7 +14,8 @@ from mock import Mock, patch
 from django.test.utils import override_settings
 from django.core.urlresolvers import reverse
 from django.http import HttpRequest, HttpResponse
-from django_comment_common.models import FORUM_ROLE_COMMUNITY_TA
+from django_comment_common.models import FORUM_ROLE_COMMUNITY_TA, Role
+from django_comment_common.utils import seed_permissions_roles
 from django.core import mail
 from django.utils.timezone import utc
 
@@ -126,7 +127,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase):
             ('get_distribution', {}),
             ('get_student_progress_url', {'unique_student_identifier': self.user.username}),
             ('reset_student_attempts', {'problem_to_reset': self.problem_urlname, 'unique_student_identifier': self.user.email}),
-            ('update_forum_role_membership', {'email': self.user.email, 'rolename': 'Moderator', 'action': 'allow'}),
+            ('update_forum_role_membership', {'unique_student_identifier': self.user.email, 'rolename': 'Moderator', 'action': 'allow'}),
             ('list_forum_members', {'rolename': FORUM_ROLE_COMMUNITY_TA}),
             ('proxy_legacy_analytics', {'aname': 'ProblemGradeDistribution'}),
             ('send_email', {'send_to': 'staff', 'subject': 'test', 'message': 'asdf'}),
@@ -957,6 +958,58 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
         res_json = json.loads(response.content)
         self.assertEqual(res_json, expected)
 
+    def test_update_forum_role_membership(self):
+        """
+        Test update forum role membership with user's email and username.
+        """
+
+        # Seed forum roles for course.
+        seed_permissions_roles(self.course.id)
+
+        # Test add discussion admin with email.
+        self.assert_update_forum_role_membership(self.other_user.email, "Administrator", "allow")
+
+        # Test revoke discussion admin with email.
+        self.assert_update_forum_role_membership(self.other_user.email, "Administrator", "revoke")
+
+        # Test add discussion moderator with username.
+        self.assert_update_forum_role_membership(self.other_user.username, "Moderator", "allow")
+
+        # Test revoke discussion moderator with username.
+        self.assert_update_forum_role_membership(self.other_user.username, "Moderator", "revoke")
+
+        # Test add discussion community TA with email.
+        self.assert_update_forum_role_membership(self.other_user.email, "Community TA", "allow")
+
+        # Test revoke discussion community TA with username.
+        self.assert_update_forum_role_membership(self.other_user.username, "Community TA", "revoke")
+
+    def assert_update_forum_role_membership(self, unique_student_identifier, rolename, action):
+        """
+        Test update forum role membership.
+        Get unique_student_identifier, rolename and action and update forum role.
+        """
+
+        url = reverse('update_forum_role_membership', kwargs={'course_id': self.course.id})
+        response = self.client.get(
+            url,
+            {
+                'unique_student_identifier': unique_student_identifier,
+                'rolename': rolename,
+                'action': action,
+            }
+        )
+
+        # Status code should be 200.
+        self.assertEqual(response.status_code, 200)
+
+        user_roles = self.other_user.roles.filter(course_id=self.course.id).values_list("name", flat=True)
+        if action == 'allow':
+            self.assertIn(rolename, user_roles)
+        elif action == 'revoke':
+            self.assertNotIn(rolename, user_roles)
+
+
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCase):

lms/djangoapps/instructor/views/api.py

@@ -973,7 +973,7 @@ def send_email(request, course_id):
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
 @require_level('staff')
 @require_query_params(
-    email="the target users email",
+    unique_student_identifier="email or username of user to change access",
     rolename="the forum role",
     action="'allow' or 'revoke'",
 )
@@ -998,7 +998,7 @@ def update_forum_role_membership(request, course_id):
         request.user, course_id, FORUM_ROLE_ADMINISTRATOR
     )
 
-    email = strip_if_string(request.GET.get('email'))
+    unique_student_identifier = request.GET.get('unique_student_identifier')
     rolename = request.GET.get('rolename')
     action = request.GET.get('action')
 
@@ -1017,7 +1017,7 @@ def update_forum_role_membership(request, course_id):
             "Unrecognized rolename '{}'.".format(rolename)
         ))
 
-    user = User.objects.get(email=email)
+    user = get_student_from_identifier(unique_student_identifier)
     target_is_instructor = has_access(user, course, 'instructor')
     # cannot revoke instructor
     if target_is_instructor and action == 'revoke' and rolename == FORUM_ROLE_ADMINISTRATOR: