feat!: upgrade bulk_beta_modify_access to drf ( 30 ) (#35604)

* feat!: upgrading api to DRF.

lms/djangoapps/instructor/tests/test_api.py

@@ -1990,6 +1990,15 @@ def test_add_notenrolled_username_autoenroll(self):
         self.add_notenrolled(response, self.notenrolled_student.username)
         assert CourseEnrollment.is_enrolled(self.notenrolled_student, self.course.id)
 
+    def test_add_notenrolled_username_autoenroll_with_multiple_users(self):
+        url = reverse('bulk_beta_modify_access', kwargs={'course_id': str(self.course.id)})
+        identifiers = (f"[email protected], "
+                       f"[email protected]\n[email protected]\r [email protected]\r, [email protected], "
+                       f"{self.notenrolled_student.username}"
+                       )
+        response = self.client.post(url, {'identifiers': identifiers, 'action': 'add', 'email_students': False, 'auto_enroll': True})  # lint-amnesty, pylint: disable=line-too-long
+        assert 6, len(json.loads(response.content.decode())['results'])
+
     @ddt.data('http', 'https')
     def test_add_notenrolled_with_email(self, protocol):
         url = reverse('bulk_beta_modify_access', kwargs={'course_id': str(self.course.id)})

lms/djangoapps/instructor/views/api.py

@@ -109,6 +109,7 @@
     CertificateSerializer,
     CertificateStatusesSerializer,
     ListInstructorTaskInputSerializer,
+    ModifyAccessSerializer,
     RoleNameSerializer,
     SendEmailSerializer,
     ShowStudentExtensionSerializer,
@@ -914,88 +915,91 @@ def students_update_enrollment(request, course_id):  # lint-amnesty, pylint: dis
     return JsonResponse(response_payload)
 
 
-@require_POST
-@ensure_csrf_cookie
-@cache_control(no_cache=True, no_store=True, must_revalidate=True)
-@require_course_permission(permissions.CAN_BETATEST)
-@common_exceptions_400
-@require_post_params(
-    identifiers="stringified list of emails and/or usernames",
-    action="add or remove",
-)
-def bulk_beta_modify_access(request, course_id):
+@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch')
+class BulkBetaModifyAccess(DeveloperErrorViewMixin, APIView):
     """
     Enroll or unenroll users in beta testing program.
-
-    Query parameters:
-    - identifiers is string containing a list of emails and/or usernames separated by
-      anything split_input_list can handle.
-    - action is one of ['add', 'remove']
     """
-    course_id = CourseKey.from_string(course_id)
-    action = request.POST.get('action')
-    identifiers_raw = request.POST.get('identifiers')
-    identifiers = _split_input_list(identifiers_raw)
-    email_students = _get_boolean_param(request, 'email_students')
-    auto_enroll = _get_boolean_param(request, 'auto_enroll')
-    results = []
-    rolename = 'beta'
-    course = get_course_by_id(course_id)
+    permission_classes = (IsAuthenticated, permissions.InstructorPermission)
+    permission_name = permissions.CAN_BETATEST
+    serializer_class = ModifyAccessSerializer
 
-    email_params = {}
-    if email_students:
-        secure = request.is_secure()
-        email_params = get_email_params(course, auto_enroll=auto_enroll, secure=secure)
+    @method_decorator(ensure_csrf_cookie)
+    def post(self, request, course_id):
+        """
+        Query parameters:
+        - identifiers is string containing a list of emails and/or usernames separated by
+          anything split_input_list can handle.
+        - action is one of ['add', 'remove']
+        """
+        course_id = CourseKey.from_string(course_id)
+        serializer = self.serializer_class(data=request.data)
+        if not serializer.is_valid():
+            return JsonResponse({'message': serializer.errors}, status=400)
 
-    for identifier in identifiers:
-        try:
-            error = False
-            user_does_not_exist = False
-            user = get_student_from_identifier(identifier)
-            user_active = user.is_active
+        action = serializer.validated_data['action']
+        identifiers = serializer.validated_data['identifiers']
+        email_students = serializer.validated_data['email_students']
+        auto_enroll = serializer.validated_data['auto_enroll']
 
-            if action == 'add':
-                allow_access(course, user, rolename)
-            elif action == 'remove':
-                revoke_access(course, user, rolename)
+        results = []
+        rolename = 'beta'
+        course = get_course_by_id(course_id)
+
+        email_params = {}
+        if email_students:
+            secure = request.is_secure()
+            email_params = get_email_params(course, auto_enroll=auto_enroll, secure=secure)
+
+        for identifier in identifiers:
+            try:
+                error = False
+                user_does_not_exist = False
+                user = get_student_from_identifier(identifier)
+                user_active = user.is_active
+
+                if action == 'add':
+                    allow_access(course, user, rolename)
+                elif action == 'remove':
+                    revoke_access(course, user, rolename)
+                else:
+                    return HttpResponseBadRequest(strip_tags(
+                        f"Unrecognized action '{action}'"
+                    ))
+            except User.DoesNotExist:
+                error = True
+                user_does_not_exist = True
+                user_active = None
+            # catch and log any unexpected exceptions
+            # so that one error doesn't cause a 500.
+            except Exception as exc:  # pylint: disable=broad-except
+                log.exception("Error while #{}ing student")
+                log.exception(exc)
+                error = True
             else:
-                return HttpResponseBadRequest(strip_tags(
-                    f"Unrecognized action '{action}'"
-                ))
-        except User.DoesNotExist:
-            error = True
-            user_does_not_exist = True
-            user_active = None
-        # catch and log any unexpected exceptions
-        # so that one error doesn't cause a 500.
-        except Exception as exc:  # pylint: disable=broad-except
-            log.exception("Error while #{}ing student")
-            log.exception(exc)
-            error = True
-        else:
-            # If no exception thrown, see if we should send an email
-            if email_students:
-                send_beta_role_email(action, user, email_params)
-            # See if we should autoenroll the student
-            if auto_enroll:
-                # Check if student is already enrolled
-                if not is_user_enrolled_in_course(user, course_id):
-                    CourseEnrollment.enroll(user, course_id)
+                # If no exception thrown, see if we should send an email
+                if email_students:
+                    send_beta_role_email(action, user, email_params)
+                # See if we should autoenroll the student
+                if auto_enroll:
+                    # Check if student is already enrolled
+                    if not is_user_enrolled_in_course(user, course_id):
+                        CourseEnrollment.enroll(user, course_id)
 
-        finally:
-            # Tabulate the action result of this email address
-            results.append({
-                'identifier': identifier,
-                'error': error,  # pylint: disable=used-before-assignment
-                'userDoesNotExist': user_does_not_exist,  # pylint: disable=used-before-assignment
-                'is_active': user_active  # pylint: disable=used-before-assignment
-            })
+            finally:
+                # Tabulate the action result of this email address
+                results.append({
+                    'identifier': identifier,
+                    'error': error,  # pylint: disable=used-before-assignment
+                    'userDoesNotExist': user_does_not_exist,  # pylint: disable=used-before-assignment
+                    'is_active': user_active  # pylint: disable=used-before-assignment
+                })
 
-    response_payload = {
-        'action': action,
-        'results': results,
-    }
-    return JsonResponse(response_payload)
+        response_payload = {
+            'action': action,
+            'results': results,
+        }
+        return JsonResponse(response_payload)
 
 
 @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch')
@@ -1025,7 +1029,6 @@ def post(self, request, course_id):
         course = get_course_with_access(
             request.user, 'instructor', course_id, depth=None
         )
-
         serializer_data = AccessSerializer(data=request.data)
         if not serializer_data.is_valid():
             return HttpResponseBadRequest(reason=serializer_data.errors)

lms/djangoapps/instructor/views/api_urls.py

@@ -25,7 +25,7 @@
     path('register_and_enroll_students', api.RegisterAndEnrollStudents.as_view(), name='register_and_enroll_students'),
     path('list_course_role_members', api.ListCourseRoleMembersView.as_view(), name='list_course_role_members'),
     path('modify_access', api.ModifyAccess.as_view(), name='modify_access'),
-    path('bulk_beta_modify_access', api.bulk_beta_modify_access, name='bulk_beta_modify_access'),
+    path('bulk_beta_modify_access', api.BulkBetaModifyAccess.as_view(), name='bulk_beta_modify_access'),
     path('get_problem_responses', api.get_problem_responses, name='get_problem_responses'),
     path('get_issued_certificates/', api.GetIssuedCertificates.as_view(), name='get_issued_certificates'),
     re_path(r'^get_students_features(?P<csv>/csv)?$', api.GetStudentsFeatures.as_view(), name='get_students_features'),

lms/djangoapps/instructor/views/serializer.py

@@ -1,4 +1,5 @@
 """ Instructor apis serializers. """
+import re
 
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
 from django.core.exceptions import ValidationError
@@ -232,6 +233,76 @@ def __init__(self, *args, **kwargs):
             self.fields['due_datetime'].required = False
 
 
+class ModifyAccessSerializer(serializers.Serializer):
+    """
+    serializers for enroll or un-enroll users in beta testing program.
+    """
+    identifiers = serializers.CharField(
+        help_text="A comma separated list of emails or usernames.",
+        required=True
+    )
+    action = serializers.ChoiceField(
+        choices=["add", "remove"],
+        help_text="Action to perform: add or remove.",
+        required=True
+    )
+
+    email_students = serializers.BooleanField(
+        default=False,
+        help_text="Boolean flag to indicate if students should be emailed."
+    )
+
+    auto_enroll = serializers.BooleanField(
+        default=False,
+        help_text="Boolean flag to indicate if the user should be auto-enrolled."
+    )
+
+    def validate_identifiers(self, value):
+        """
+        Validate the 'identifiers' field which is now a list of strings.
+        """
+        # Iterate over the list of identifiers and validate each one
+        validated_list = _split_input_list(value)
+        if not validated_list:
+            raise serializers.ValidationError("The identifiers list cannot be empty.")
+
+        return validated_list
+
+    def validate_email_students(self, value):
+        """
+        handle string values like 'true' or 'false'.
+        """
+        if isinstance(value, str):
+            return value.lower() == 'true'
+        return bool(value)
+
+    def validate_auto_enroll(self, value):
+        """
+        handle string values like 'true' or 'false'.
+        """
+        if isinstance(value, str):
+            return value.lower() == 'true'
+        return bool(value)
+
+
+def _split_input_list(str_list):
+    """
+    Separate out individual student email from the comma, or space separated string.
+
+    e.g.
+    in: "[email protected], [email protected]\n[email protected]\r [email protected]\r, [email protected]"
+    out: ['[email protected]', '[email protected]', '[email protected]', '[email protected]', '[email protected]']
+
+    `str_list` is a string coming from an input text area
+    returns a list of separated values
+    """
+    new_list = re.split(r'[,\s\n\r]+', str_list)
+    new_list = [s.strip() for s in new_list]
+    new_list = [s for s in new_list if s != '']
+
+    return new_list
+
+
 class CertificateStatusesSerializer(serializers.Serializer):
     """
     Serializer for validating and serializing certificate status inputs.