From 71f9f3019af5140aae6ab9d5fc1a9b2f2f8eb9e5 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Mon, 22 Jul 2024 16:03:21 +0500 Subject: [PATCH 1/8] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 4 +++- lms/djangoapps/instructor/views/api_urls.py | 2 +- lms/djangoapps/instructor/views/serializer.py | 21 +++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 2813daa8a318..d1c63766f9ca 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -105,7 +105,9 @@ from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError, QueueConnectionError from lms.djangoapps.instructor_task.data import InstructorTaskTypes from lms.djangoapps.instructor_task.models import ReportStore -from lms.djangoapps.instructor.views.serializer import RoleNameSerializer, UserSerializer, AccessSerializer +from lms.djangoapps.instructor.views.serializer import ( + RoleNameSerializer, UserSerializer, ShowStudentExtensionSerializer, AccessSerializer +) from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, is_course_cohorted from openedx.core.djangoapps.course_groups.models import CourseUserGroup diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 599aead57303..00f27e5d1f44 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -53,7 +53,7 @@ path('change_due_date', api.change_due_date, name='change_due_date'), path('reset_due_date', api.reset_due_date, name='reset_due_date'), path('show_unit_extensions', api.show_unit_extensions, name='show_unit_extensions'), - path('show_student_extensions', api.show_student_extensions, name='show_student_extensions'), + path('show_student_extensions', api.ShowStudentExtensions.as_view(), name='show_student_extensions'), # proctored exam downloads... path('get_proctored_exam_results', api.get_proctored_exam_results, name='get_proctored_exam_results'), diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 463f05ad45b8..f6096b4345df 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -59,3 +59,24 @@ def validate_unique_student_identifier(self, value): return None return user + + +class ShowStudentExtensionSerializer(serializers.Serializer): + """ + Serializer for validating and processing the student identifier. + """ + student = serializers.CharField(write_only=True, required=True) + def validate_student(self, value): + """ + Validate that the student corresponds to an existing user. + """ + try: + user = get_student_from_identifier(value) + except User.DoesNotExist: + response_payload = { + 'student': value, + 'userDoesNotExist': True, + } + return response_payload + + return user From a89adf4d495b56af4bc299cb95ff6dbabb4a872d Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 20 Aug 2024 12:18:31 +0500 Subject: [PATCH 2/8] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 678 +++++++++--------- lms/djangoapps/instructor/views/serializer.py | 7 +- 2 files changed, 342 insertions(+), 343 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index d1c63766f9ca..2b78e84b6654 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -285,305 +285,299 @@ def wrapped(request, course_id): return wrapped -@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') -class RegisterAndEnrollStudents(APIView): +@require_POST +@ensure_csrf_cookie +@cache_control(no_cache=True, no_store=True, must_revalidate=True) +@require_course_permission(permissions.CAN_ENROLL) +def register_and_enroll_students(request, course_id): # pylint: disable=too-many-statements """ Create new account and Enroll students in this course. - """ - permission_classes = (IsAuthenticated, permissions.InstructorPermission) - permission_name = permissions.CAN_ENROLL + Passing a csv file that contains a list of students. + Order in csv should be the following email = 0; username = 1; name = 2; country = 3. + If there are more than 4 columns in the csv: cohort = 4, course mode = 5. + Requires staff access. - @method_decorator(ensure_csrf_cookie) - def post(self, request, course_id): # pylint: disable=too-many-statements - """ - Create new account and Enroll students in this course. - Passing a csv file that contains a list of students. - Order in csv should be the following email = 0; username = 1; name = 2; country = 3. - If there are more than 4 columns in the csv: cohort = 4, course mode = 5. - Requires staff access. - - -If the email address and username already exists and the user is enrolled in the course, - do nothing (including no email gets sent out) - - -If the email address already exists, but the username is different, - match on the email address only and continue to enroll the user in the course using the email address - as the matching criteria. Note the change of username as a warning message (but not a failure). - Send a standard enrollment email which is the same as the existing manual enrollment - - -If the username already exists (but not the email), assume it is a different user and fail - to create the new account. - The failure will be messaged in a response in the browser. - """ - if not configuration_helpers.get_value( - 'ALLOW_AUTOMATED_SIGNUPS', - settings.FEATURES.get('ALLOW_AUTOMATED_SIGNUPS', False), - ): - return HttpResponseForbidden() + -If the email address and username already exists and the user is enrolled in the course, + do nothing (including no email gets sent out) - course_id = CourseKey.from_string(course_id) - warnings = [] - row_errors = [] - general_errors = [] + -If the email address already exists, but the username is different, + match on the email address only and continue to enroll the user in the course using the email address + as the matching criteria. Note the change of username as a warning message (but not a failure). + Send a standard enrollment email which is the same as the existing manual enrollment - # email-students is a checkbox input type; will be present in POST if checked, absent otherwise - notify_by_email = 'email-students' in request.POST + -If the username already exists (but not the email), assume it is a different user and fail + to create the new account. + The failure will be messaged in a response in the browser. + """ - # for white labels we use 'shopping cart' which uses CourseMode.HONOR as - # course mode for creating course enrollments. - if CourseMode.is_white_label(course_id): - default_course_mode = CourseMode.HONOR - else: - default_course_mode = None + if not configuration_helpers.get_value( + 'ALLOW_AUTOMATED_SIGNUPS', + settings.FEATURES.get('ALLOW_AUTOMATED_SIGNUPS', False), + ): + return HttpResponseForbidden() - # Allow bulk enrollments in all non-expired course modes including "credit" (which is non-selectable) - valid_course_modes = set(map(lambda x: x.slug, CourseMode.modes_for_course( - course_id=course_id, - only_selectable=False, - include_expired=False, - ))) + course_id = CourseKey.from_string(course_id) + warnings = [] + row_errors = [] + general_errors = [] - if 'students_list' in request.FILES: # lint-amnesty, pylint: disable=too-many-nested-blocks - students = [] + # email-students is a checkbox input type; will be present in POST if checked, absent otherwise + notify_by_email = 'email-students' in request.POST - try: - upload_file = request.FILES.get('students_list') - if upload_file.name.endswith('.csv'): - students = list(csv.reader(upload_file.read().decode('utf-8-sig').splitlines())) - course = get_course_by_id(course_id) - else: - general_errors.append({ - 'username': '', 'email': '', - 'response': _( - 'Make sure that the file you upload is in CSV format with no ' - 'extraneous characters or rows.') - }) + # for white labels we use 'shopping cart' which uses CourseMode.HONOR as + # course mode for creating course enrollments. + if CourseMode.is_white_label(course_id): + default_course_mode = CourseMode.HONOR + else: + default_course_mode = None - except Exception: # pylint: disable=broad-except + # Allow bulk enrollments in all non-expired course modes including "credit" (which is non-selectable) + valid_course_modes = set(map(lambda x: x.slug, CourseMode.modes_for_course( + course_id=course_id, + only_selectable=False, + include_expired=False, + ))) + + if 'students_list' in request.FILES: # lint-amnesty, pylint: disable=too-many-nested-blocks + students = [] + + try: + upload_file = request.FILES.get('students_list') + if upload_file.name.endswith('.csv'): + students = list(csv.reader(upload_file.read().decode('utf-8-sig').splitlines())) + course = get_course_by_id(course_id) + else: general_errors.append({ - 'username': '', 'email': '', 'response': _('Could not read uploaded file.') + 'username': '', 'email': '', + 'response': _( + 'Make sure that the file you upload is in CSV format with no extraneous characters or rows.') }) - finally: - upload_file.close() - - generated_passwords = [] - # To skip fetching cohorts from the DB while iterating on students, - # {: CourseUserGroup} - cohorts_cache = {} - already_warned_not_cohorted = False - extra_fields_is_enabled = configuration_helpers.get_value( - 'ENABLE_AUTOMATED_SIGNUPS_EXTRA_FIELDS', - settings.FEATURES.get('ENABLE_AUTOMATED_SIGNUPS_EXTRA_FIELDS', False), - ) - # Iterate each student in the uploaded csv file. - for row_num, student in enumerate(students, 1): + except Exception: # pylint: disable=broad-except + general_errors.append({ + 'username': '', 'email': '', 'response': _('Could not read uploaded file.') + }) + finally: + upload_file.close() - # Verify that we have the expected number of columns in every row - # but allow for blank lines. - if not student: - continue + generated_passwords = [] + # To skip fetching cohorts from the DB while iterating on students, + # {: CourseUserGroup} + cohorts_cache = {} + already_warned_not_cohorted = False + extra_fields_is_enabled = configuration_helpers.get_value( + 'ENABLE_AUTOMATED_SIGNUPS_EXTRA_FIELDS', + settings.FEATURES.get('ENABLE_AUTOMATED_SIGNUPS_EXTRA_FIELDS', False), + ) - if extra_fields_is_enabled: - is_valid_csv = 4 <= len(student) <= 6 - error = _('Data in row #{row_num} must have between four and six columns: ' - 'email, username, full name, country, cohort, and course mode. ' - 'The last two columns are optional.').format(row_num=row_num) - else: - is_valid_csv = len(student) == 4 - error = _('Data in row #{row_num} must have exactly four columns: ' - 'email, username, full name, and country.').format(row_num=row_num) - - if not is_valid_csv: - general_errors.append({ - 'username': '', - 'email': '', - 'response': error - }) - continue + # Iterate each student in the uploaded csv file. + for row_num, student in enumerate(students, 1): - # Extract each column, handle optional columns if they exist. - email, username, name, country, *optional_cols = student - if optional_cols: - optional_cols.append(default_course_mode) - cohort_name, course_mode, *_tail = optional_cols - else: - cohort_name = None - course_mode = None + # Verify that we have the expected number of columns in every row + # but allow for blank lines. + if not student: + continue - # Validate cohort name, and get the cohort object. Skip if course - # is not cohorted. - cohort = None + if extra_fields_is_enabled: + is_valid_csv = 4 <= len(student) <= 6 + error = _('Data in row #{row_num} must have between four and six columns: ' + 'email, username, full name, country, cohort, and course mode. ' + 'The last two columns are optional.').format(row_num=row_num) + else: + is_valid_csv = len(student) == 4 + error = _('Data in row #{row_num} must have exactly four columns: ' + 'email, username, full name, and country.').format(row_num=row_num) - if cohort_name and not already_warned_not_cohorted: - if not is_course_cohorted(course_id): - row_errors.append({ - 'username': username, - 'email': email, - 'response': _('Course is not cohorted but cohort provided. ' - 'Ignoring cohort assignment for all users.') - }) - already_warned_not_cohorted = True - elif cohort_name in cohorts_cache: - cohort = cohorts_cache[cohort_name] - else: - # Don't attempt to create cohort or assign student if cohort - # does not exist. - try: - cohort = get_cohort_by_name(course_id, cohort_name) - except CourseUserGroup.DoesNotExist: - row_errors.append({ - 'username': username, - 'email': email, - 'response': _('Cohort name not found: {cohort}. ' - 'Ignoring cohort assignment for ' - 'all users.').format(cohort=cohort_name) - }) - cohorts_cache[cohort_name] = cohort - - # Validate course mode. - if not course_mode: - course_mode = default_course_mode - - if (course_mode is not None - and course_mode not in valid_course_modes): - # If `default is None` and the user is already enrolled, - # `CourseEnrollment.change_mode()` will not update the mode, - # hence two error messages. - if default_course_mode is None: - err_msg = _( - 'Invalid course mode: {mode}. Falling back to the ' - 'default mode, or keeping the current mode in case the ' - 'user is already enrolled.' - ).format(mode=course_mode) - else: - err_msg = _( - 'Invalid course mode: {mode}. Failling back to ' - '{default_mode}, or resetting to {default_mode} in case ' - 'the user is already enrolled.' - ).format(mode=course_mode, default_mode=default_course_mode) - row_errors.append({ - 'username': username, - 'email': email, - 'response': err_msg, - }) - course_mode = default_course_mode + if not is_valid_csv: + general_errors.append({ + 'username': '', + 'email': '', + 'response': error + }) + continue + + # Extract each column, handle optional columns if they exist. + email, username, name, country, *optional_cols = student + if optional_cols: + optional_cols.append(default_course_mode) + cohort_name, course_mode, *_tail = optional_cols + else: + cohort_name = None + course_mode = None - email_params = get_email_params(course, True, secure=request.is_secure()) - try: - validate_email(email) # Raises ValidationError if invalid - except ValidationError: + # Validate cohort name, and get the cohort object. Skip if course + # is not cohorted. + cohort = None + + if cohort_name and not already_warned_not_cohorted: + if not is_course_cohorted(course_id): row_errors.append({ 'username': username, 'email': email, - 'response': _('Invalid email {email_address}.').format(email_address=email) + 'response': _('Course is not cohorted but cohort provided. ' + 'Ignoring cohort assignment for all users.') }) + already_warned_not_cohorted = True + elif cohort_name in cohorts_cache: + cohort = cohorts_cache[cohort_name] else: - if User.objects.filter(email=email).exists(): - # Email address already exists. assume it is the correct user - # and just register the user in the course and send an enrollment email. - user = User.objects.get(email=email) - - # see if it is an exact match with email and username - # if it's not an exact match then just display a warning message, but continue onwards - if not User.objects.filter(email=email, username=username).exists(): - warning_message = _( - 'An account with email {email} exists but the provided username {username} ' - 'is different. Enrolling anyway with {email}.' - ).format(email=email, username=username) - - warnings.append({ - 'username': username, 'email': email, 'response': warning_message - }) - log.warning('email %s already exist', email) - else: - log.info( - "user already exists with username '%s' and email '%s'", - username, - email - ) - - # enroll a user if it is not already enrolled. - if not is_user_enrolled_in_course(user, course_id): - # Enroll user to the course and add manual enrollment audit trail - create_manual_course_enrollment( - user=user, - course_id=course_id, - mode=course_mode, - enrolled_by=request.user, - reason='Enrolling via csv upload', - state_transition=UNENROLLED_TO_ENROLLED, - ) - enroll_email(course_id=course_id, - student_email=email, - auto_enroll=True, - email_students=notify_by_email, - email_params=email_params) - else: - # update the course mode if already enrolled - existing_enrollment = CourseEnrollment.get_enrollment(user, course_id) - if existing_enrollment.mode != course_mode: - existing_enrollment.change_mode(mode=course_mode) - if cohort: - try: - add_user_to_cohort(cohort, user) - except ValueError: - # user already in this cohort; ignore - pass - elif is_email_retired(email): - # We are either attempting to enroll a retired user or create a new user with an email which is - # already associated with a retired account. Simply block these attempts. + # Don't attempt to create cohort or assign student if cohort + # does not exist. + try: + cohort = get_cohort_by_name(course_id, cohort_name) + except CourseUserGroup.DoesNotExist: row_errors.append({ 'username': username, 'email': email, - 'response': _('Invalid email {email_address}.').format(email_address=email), + 'response': _('Cohort name not found: {cohort}. ' + 'Ignoring cohort assignment for ' + 'all users.').format(cohort=cohort_name) }) - log.warning('Email address %s is associated with a retired user, so course enrollment was ' + # lint-amnesty, pylint: disable=logging-not-lazy - 'blocked.', email) + cohorts_cache[cohort_name] = cohort + + # Validate course mode. + if not course_mode: + course_mode = default_course_mode + + if (course_mode is not None + and course_mode not in valid_course_modes): + # If `default is None` and the user is already enrolled, + # `CourseEnrollment.change_mode()` will not update the mode, + # hence two error messages. + if default_course_mode is None: + err_msg = _( + 'Invalid course mode: {mode}. Falling back to the ' + 'default mode, or keeping the current mode in case the ' + 'user is already enrolled.' + ).format(mode=course_mode) + else: + err_msg = _( + 'Invalid course mode: {mode}. Failling back to ' + '{default_mode}, or resetting to {default_mode} in case ' + 'the user is already enrolled.' + ).format(mode=course_mode, default_mode=default_course_mode) + row_errors.append({ + 'username': username, + 'email': email, + 'response': err_msg, + }) + course_mode = default_course_mode + + email_params = get_email_params(course, True, secure=request.is_secure()) + try: + validate_email(email) # Raises ValidationError if invalid + except ValidationError: + row_errors.append({ + 'username': username, + 'email': email, + 'response': _('Invalid email {email_address}.').format(email_address=email) + }) + else: + if User.objects.filter(email=email).exists(): + # Email address already exists. assume it is the correct user + # and just register the user in the course and send an enrollment email. + user = User.objects.get(email=email) + + # see if it is an exact match with email and username + # if it's not an exact match then just display a warning message, but continue onwards + if not User.objects.filter(email=email, username=username).exists(): + warning_message = _( + 'An account with email {email} exists but the provided username {username} ' + 'is different. Enrolling anyway with {email}.' + ).format(email=email, username=username) + + warnings.append({ + 'username': username, 'email': email, 'response': warning_message + }) + log.warning('email %s already exist', email) else: - # This email does not yet exist, so we need to create a new account - # If username already exists in the database, then create_and_enroll_user - # will raise an IntegrityError exception. - password = generate_unique_password(generated_passwords) - errors = create_and_enroll_user( - email, + log.info( + "user already exists with username '%s' and email '%s'", username, - name, - country, - password, - course_id, - course_mode, - request.user, - email_params, - email_user=notify_by_email, + email ) - row_errors.extend(errors) - if cohort: - try: - add_user_to_cohort(cohort, email) - except ValueError: - # user already in this cohort; ignore - # NOTE: Checking this here may be unnecessary if we can prove that a - # new user will never be - # automatically assigned to a cohort from the above. - pass - except ValidationError: - row_errors.append({ - 'username': username, - 'email': email, - 'response': _('Invalid email {email_address}.').format(email_address=email), - }) - else: - general_errors.append({ - 'username': '', 'email': '', 'response': _('File is not attached.') - }) + # enroll a user if it is not already enrolled. + if not is_user_enrolled_in_course(user, course_id): + # Enroll user to the course and add manual enrollment audit trail + create_manual_course_enrollment( + user=user, + course_id=course_id, + mode=course_mode, + enrolled_by=request.user, + reason='Enrolling via csv upload', + state_transition=UNENROLLED_TO_ENROLLED, + ) + enroll_email(course_id=course_id, + student_email=email, + auto_enroll=True, + email_students=notify_by_email, + email_params=email_params) + else: + # update the course mode if already enrolled + existing_enrollment = CourseEnrollment.get_enrollment(user, course_id) + if existing_enrollment.mode != course_mode: + existing_enrollment.change_mode(mode=course_mode) + if cohort: + try: + add_user_to_cohort(cohort, user) + except ValueError: + # user already in this cohort; ignore + pass + elif is_email_retired(email): + # We are either attempting to enroll a retired user or create a new user with an email which is + # already associated with a retired account. Simply block these attempts. + row_errors.append({ + 'username': username, + 'email': email, + 'response': _('Invalid email {email_address}.').format(email_address=email), + }) + log.warning('Email address %s is associated with a retired user, so course enrollment was ' + # lint-amnesty, pylint: disable=logging-not-lazy + 'blocked.', email) + else: + # This email does not yet exist, so we need to create a new account + # If username already exists in the database, then create_and_enroll_user + # will raise an IntegrityError exception. + password = generate_unique_password(generated_passwords) + errors = create_and_enroll_user( + email, + username, + name, + country, + password, + course_id, + course_mode, + request.user, + email_params, + email_user=notify_by_email, + ) + row_errors.extend(errors) + if cohort: + try: + add_user_to_cohort(cohort, email) + except ValueError: + # user already in this cohort; ignore + # NOTE: Checking this here may be unnecessary if we can prove that a new user will never be + # automatically assigned to a cohort from the above. + pass + except ValidationError: + row_errors.append({ + 'username': username, + 'email': email, + 'response': _('Invalid email {email_address}.').format(email_address=email), + }) - results = { - 'row_errors': row_errors, - 'general_errors': general_errors, - 'warnings': warnings - } - return JsonResponse(results) + else: + general_errors.append({ + 'username': '', 'email': '', 'response': _('File is not attached.') + }) + + results = { + 'row_errors': row_errors, + 'general_errors': general_errors, + 'warnings': warnings + } + return JsonResponse(results) def generate_random_string(length): @@ -989,8 +983,17 @@ def bulk_beta_modify_access(request, course_id): return JsonResponse(response_payload) -@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') -class ModifyAccess(APIView): +@require_POST +@ensure_csrf_cookie +@cache_control(no_cache=True, no_store=True, must_revalidate=True) +@require_course_permission(permissions.EDIT_COURSE_ACCESS) +@require_post_params( + unique_student_identifier="email or username of user to change access", + rolename="'instructor', 'staff', 'beta', or 'ccx_coach'", + action="'allow' or 'revoke'" +) +@common_exceptions_400 +def modify_access(request, course_id): """ Modify staff/instructor access of other user. Requires instructor access. @@ -1002,77 +1005,67 @@ class ModifyAccess(APIView): rolename is one of ['instructor', 'staff', 'beta', 'ccx_coach'] action is one of ['allow', 'revoke'] """ - permission_classes = (IsAuthenticated, permissions.InstructorPermission) - permission_name = permissions.EDIT_COURSE_ACCESS - serializer_class = AccessSerializer - - @method_decorator(ensure_csrf_cookie) - def post(self, request, course_id): - """ - Modify staff/instructor access of other user. - Requires instructor access. - """ - course_id = CourseKey.from_string(course_id) - course = get_course_with_access( - request.user, 'instructor', course_id, depth=None - ) + course_id = CourseKey.from_string(course_id) + course = get_course_with_access( + request.user, 'instructor', course_id, depth=None + ) + unique_student_identifier = request.POST.get('unique_student_identifier') + try: + user = get_student_from_identifier(unique_student_identifier) + except User.DoesNotExist: + response_payload = { + 'unique_student_identifier': unique_student_identifier, + 'userDoesNotExist': True, + } + return JsonResponse(response_payload) - serializer_data = AccessSerializer(data=request.data) - if not serializer_data.is_valid(): - return HttpResponseBadRequest(reason=serializer_data.errors) + # Check that user is active, because add_users + # in common/djangoapps/student/roles.py fails + # silently when we try to add an inactive user. + if not user.is_active: + response_payload = { + 'unique_student_identifier': user.username, + 'inactiveUser': True, + } + return JsonResponse(response_payload) - user = serializer_data.validated_data.get('unique_student_identifier') - if not user: - response_payload = { - 'unique_student_identifier': request.data.get('unique_student_identifier'), - 'userDoesNotExist': True, - } - return JsonResponse(response_payload) + rolename = request.POST.get('rolename') + action = request.POST.get('action') - if not user.is_active: - response_payload = { - 'unique_student_identifier': user.username, - 'inactiveUser': True, - } - return JsonResponse(response_payload) - - rolename = serializer_data.data['rolename'] - action = serializer_data.data['action'] - - if rolename not in ROLES: - error = strip_tags(f"unknown rolename '{rolename}'") - log.error(error) - return HttpResponseBadRequest(error) - - # disallow instructors from removing their own instructor access. - if rolename == 'instructor' and user == request.user and action != 'allow': - response_payload = { - 'unique_student_identifier': user.username, - 'rolename': rolename, - 'action': action, - 'removingSelfAsInstructor': True, - } - return JsonResponse(response_payload) - - if action == 'allow': - allow_access(course, user, rolename) - if not is_user_enrolled_in_course(user, course_id): - CourseEnrollment.enroll(user, course_id) - elif action == 'revoke': - revoke_access(course, user, rolename) - else: - return HttpResponseBadRequest(strip_tags( - f"unrecognized action u'{action}'" - )) + if rolename not in ROLES: + error = strip_tags(f"unknown rolename '{rolename}'") + log.error(error) + return HttpResponseBadRequest(error) + # disallow instructors from removing their own instructor access. + if rolename == 'instructor' and user == request.user and action != 'allow': response_payload = { 'unique_student_identifier': user.username, 'rolename': rolename, 'action': action, - 'success': 'yes', + 'removingSelfAsInstructor': True, } return JsonResponse(response_payload) + if action == 'allow': + allow_access(course, user, rolename) + if not is_user_enrolled_in_course(user, course_id): + CourseEnrollment.enroll(user, course_id) + elif action == 'revoke': + revoke_access(course, user, rolename) + else: + return HttpResponseBadRequest(strip_tags( + f"unrecognized action u'{action}'" + )) + + response_payload = { + 'unique_student_identifier': user.username, + 'rolename': rolename, + 'action': action, + 'success': 'yes', + } + return JsonResponse(response_payload) + @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') class ListCourseRoleMembersView(APIView): @@ -2969,20 +2962,29 @@ def show_unit_extensions(request, course_id): return JsonResponse(dump_block_extensions(course, unit)) -@handle_dashboard_error -@require_POST -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_course_permission(permissions.GIVE_STUDENT_EXTENSION) -@require_post_params('student') -def show_student_extensions(request, course_id): +@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') +# @method_decorator(handle_dashboard_error) +class ShowStudentExtensions(APIView): """ Shows all of the due date extensions granted to a particular student in a particular course. """ - student = require_student_from_identifier(request.POST.get('student')) - course = get_course_by_id(CourseKey.from_string(course_id)) - return JsonResponse(dump_student_extensions(course, student)) + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + serializer_class = ShowStudentExtensionSerializer + permission_name = permissions.GIVE_STUDENT_EXTENSION + + @method_decorator(ensure_csrf_cookie) + @method_decorator(handle_dashboard_error) + def post(self, request, course_id): + data = { + 'student': request.data.get('student') + } + serializer = self.serializer_class(data=data) + + serializer.is_valid(raise_exception=True) + student = serializer.validated_data['student'] + course = get_course_by_id(CourseKey.from_string(course_id)) + return Response(dump_student_extensions(course, student)) def _split_input_list(str_list): diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index f6096b4345df..0697bed6832d 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -66,6 +66,7 @@ class ShowStudentExtensionSerializer(serializers.Serializer): Serializer for validating and processing the student identifier. """ student = serializers.CharField(write_only=True, required=True) + def validate_student(self, value): """ Validate that the student corresponds to an existing user. @@ -73,10 +74,6 @@ def validate_student(self, value): try: user = get_student_from_identifier(value) except User.DoesNotExist: - response_payload = { - 'student': value, - 'userDoesNotExist': True, - } - return response_payload + return None return user From 6c823c3c0d44a2af2e1d46f9bf31c55056cd30e0 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 20 Aug 2024 12:36:56 +0500 Subject: [PATCH 3/8] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 34 +++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 2b78e84b6654..362597272f27 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -106,7 +106,7 @@ from lms.djangoapps.instructor_task.data import InstructorTaskTypes from lms.djangoapps.instructor_task.models import ReportStore from lms.djangoapps.instructor.views.serializer import ( - RoleNameSerializer, UserSerializer, ShowStudentExtensionSerializer, AccessSerializer + RoleNameSerializer, UserSerializer, ShowStudentExtensionSerializer ) from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, is_course_cohorted @@ -2976,13 +2976,39 @@ class ShowStudentExtensions(APIView): @method_decorator(ensure_csrf_cookie) @method_decorator(handle_dashboard_error) def post(self, request, course_id): + """ + Handles POST requests to retrieve due date extensions for a specific student + within a specified course. + + Parameters: + - `request`: The HTTP request object containing user-submitted data. + - `course_id`: The ID of the course for which the extensions are being queried. + + Data expected in the request: + - `student`: A required field containing the identifier of the student for whom + the due date extensions are being retrieved. This data is extracted from the + request body. + + Returns: + - A JSON response containing the details of the due date extensions granted to + the specified student in the specified course. + """ data = { 'student': request.data.get('student') } - serializer = self.serializer_class(data=data) + serializer_data = self.serializer_class(data=data) + + if not serializer_data.is_valid(): + return HttpResponseBadRequest(reason=serializer_data.errors) + + student = serializer_data.validated_data.get('student') + if not student: + response_payload = { + 'student': request.data.get('student'), + 'userDoesNotExist': True, + } + return JsonResponse(response_payload) - serializer.is_valid(raise_exception=True) - student = serializer.validated_data['student'] course = get_course_by_id(CourseKey.from_string(course_id)) return Response(dump_student_extensions(course, student)) From 44e208ae12e486cc22bdf143222748bc6aa635aa Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 20 Aug 2024 19:01:09 +0500 Subject: [PATCH 4/8] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 651 +++++++++++++------------ 1 file changed, 329 insertions(+), 322 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 362597272f27..ea0ee4266174 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -106,7 +106,7 @@ from lms.djangoapps.instructor_task.data import InstructorTaskTypes from lms.djangoapps.instructor_task.models import ReportStore from lms.djangoapps.instructor.views.serializer import ( - RoleNameSerializer, UserSerializer, ShowStudentExtensionSerializer + AccessSerializer, RoleNameSerializer, ShowStudentExtensionSerializer, UserSerializer ) from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, is_course_cohorted @@ -285,299 +285,305 @@ def wrapped(request, course_id): return wrapped -@require_POST -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_course_permission(permissions.CAN_ENROLL) -def register_and_enroll_students(request, course_id): # pylint: disable=too-many-statements +@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') +class RegisterAndEnrollStudents(APIView): """ Create new account and Enroll students in this course. - Passing a csv file that contains a list of students. - Order in csv should be the following email = 0; username = 1; name = 2; country = 3. - If there are more than 4 columns in the csv: cohort = 4, course mode = 5. - Requires staff access. - - -If the email address and username already exists and the user is enrolled in the course, - do nothing (including no email gets sent out) - - -If the email address already exists, but the username is different, - match on the email address only and continue to enroll the user in the course using the email address - as the matching criteria. Note the change of username as a warning message (but not a failure). - Send a standard enrollment email which is the same as the existing manual enrollment - - -If the username already exists (but not the email), assume it is a different user and fail - to create the new account. - The failure will be messaged in a response in the browser. """ + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.CAN_ENROLL - if not configuration_helpers.get_value( - 'ALLOW_AUTOMATED_SIGNUPS', - settings.FEATURES.get('ALLOW_AUTOMATED_SIGNUPS', False), - ): - return HttpResponseForbidden() + @method_decorator(ensure_csrf_cookie) + def post(self, request, course_id): # pylint: disable=too-many-statements + """ + Create new account and Enroll students in this course. + Passing a csv file that contains a list of students. + Order in csv should be the following email = 0; username = 1; name = 2; country = 3. + If there are more than 4 columns in the csv: cohort = 4, course mode = 5. + Requires staff access. + + -If the email address and username already exists and the user is enrolled in the course, + do nothing (including no email gets sent out) + + -If the email address already exists, but the username is different, + match on the email address only and continue to enroll the user in the course using the email address + as the matching criteria. Note the change of username as a warning message (but not a failure). + Send a standard enrollment email which is the same as the existing manual enrollment + + -If the username already exists (but not the email), assume it is a different user and fail + to create the new account. + The failure will be messaged in a response in the browser. + """ + if not configuration_helpers.get_value( + 'ALLOW_AUTOMATED_SIGNUPS', + settings.FEATURES.get('ALLOW_AUTOMATED_SIGNUPS', False), + ): + return HttpResponseForbidden() - course_id = CourseKey.from_string(course_id) - warnings = [] - row_errors = [] - general_errors = [] + course_id = CourseKey.from_string(course_id) + warnings = [] + row_errors = [] + general_errors = [] - # email-students is a checkbox input type; will be present in POST if checked, absent otherwise - notify_by_email = 'email-students' in request.POST + # email-students is a checkbox input type; will be present in POST if checked, absent otherwise + notify_by_email = 'email-students' in request.POST - # for white labels we use 'shopping cart' which uses CourseMode.HONOR as - # course mode for creating course enrollments. - if CourseMode.is_white_label(course_id): - default_course_mode = CourseMode.HONOR - else: - default_course_mode = None + # for white labels we use 'shopping cart' which uses CourseMode.HONOR as + # course mode for creating course enrollments. + if CourseMode.is_white_label(course_id): + default_course_mode = CourseMode.HONOR + else: + default_course_mode = None - # Allow bulk enrollments in all non-expired course modes including "credit" (which is non-selectable) - valid_course_modes = set(map(lambda x: x.slug, CourseMode.modes_for_course( - course_id=course_id, - only_selectable=False, - include_expired=False, - ))) + # Allow bulk enrollments in all non-expired course modes including "credit" (which is non-selectable) + valid_course_modes = set(map(lambda x: x.slug, CourseMode.modes_for_course( + course_id=course_id, + only_selectable=False, + include_expired=False, + ))) - if 'students_list' in request.FILES: # lint-amnesty, pylint: disable=too-many-nested-blocks - students = [] + if 'students_list' in request.FILES: # lint-amnesty, pylint: disable=too-many-nested-blocks + students = [] - try: - upload_file = request.FILES.get('students_list') - if upload_file.name.endswith('.csv'): - students = list(csv.reader(upload_file.read().decode('utf-8-sig').splitlines())) - course = get_course_by_id(course_id) - else: + try: + upload_file = request.FILES.get('students_list') + if upload_file.name.endswith('.csv'): + students = list(csv.reader(upload_file.read().decode('utf-8-sig').splitlines())) + course = get_course_by_id(course_id) + else: + general_errors.append({ + 'username': '', 'email': '', + 'response': _( + 'Make sure that the file you upload is in CSV format with no ' + 'extraneous characters or rows.') + }) + + except Exception: # pylint: disable=broad-except general_errors.append({ - 'username': '', 'email': '', - 'response': _( - 'Make sure that the file you upload is in CSV format with no extraneous characters or rows.') + 'username': '', 'email': '', 'response': _('Could not read uploaded file.') }) + finally: + upload_file.close() + + generated_passwords = [] + # To skip fetching cohorts from the DB while iterating on students, + # {: CourseUserGroup} + cohorts_cache = {} + already_warned_not_cohorted = False + extra_fields_is_enabled = configuration_helpers.get_value( + 'ENABLE_AUTOMATED_SIGNUPS_EXTRA_FIELDS', + settings.FEATURES.get('ENABLE_AUTOMATED_SIGNUPS_EXTRA_FIELDS', False), + ) - except Exception: # pylint: disable=broad-except - general_errors.append({ - 'username': '', 'email': '', 'response': _('Could not read uploaded file.') - }) - finally: - upload_file.close() - - generated_passwords = [] - # To skip fetching cohorts from the DB while iterating on students, - # {: CourseUserGroup} - cohorts_cache = {} - already_warned_not_cohorted = False - extra_fields_is_enabled = configuration_helpers.get_value( - 'ENABLE_AUTOMATED_SIGNUPS_EXTRA_FIELDS', - settings.FEATURES.get('ENABLE_AUTOMATED_SIGNUPS_EXTRA_FIELDS', False), - ) - - # Iterate each student in the uploaded csv file. - for row_num, student in enumerate(students, 1): + # Iterate each student in the uploaded csv file. + for row_num, student in enumerate(students, 1): - # Verify that we have the expected number of columns in every row - # but allow for blank lines. - if not student: - continue + # Verify that we have the expected number of columns in every row + # but allow for blank lines. + if not student: + continue - if extra_fields_is_enabled: - is_valid_csv = 4 <= len(student) <= 6 - error = _('Data in row #{row_num} must have between four and six columns: ' - 'email, username, full name, country, cohort, and course mode. ' - 'The last two columns are optional.').format(row_num=row_num) - else: - is_valid_csv = len(student) == 4 - error = _('Data in row #{row_num} must have exactly four columns: ' - 'email, username, full name, and country.').format(row_num=row_num) + if extra_fields_is_enabled: + is_valid_csv = 4 <= len(student) <= 6 + error = _('Data in row #{row_num} must have between four and six columns: ' + 'email, username, full name, country, cohort, and course mode. ' + 'The last two columns are optional.').format(row_num=row_num) + else: + is_valid_csv = len(student) == 4 + error = _('Data in row #{row_num} must have exactly four columns: ' + 'email, username, full name, and country.').format(row_num=row_num) + + if not is_valid_csv: + general_errors.append({ + 'username': '', + 'email': '', + 'response': error + }) + continue - if not is_valid_csv: - general_errors.append({ - 'username': '', - 'email': '', - 'response': error - }) - continue + # Extract each column, handle optional columns if they exist. + email, username, name, country, *optional_cols = student + if optional_cols: + optional_cols.append(default_course_mode) + cohort_name, course_mode, *_tail = optional_cols + else: + cohort_name = None + course_mode = None - # Extract each column, handle optional columns if they exist. - email, username, name, country, *optional_cols = student - if optional_cols: - optional_cols.append(default_course_mode) - cohort_name, course_mode, *_tail = optional_cols - else: - cohort_name = None - course_mode = None + # Validate cohort name, and get the cohort object. Skip if course + # is not cohorted. + cohort = None - # Validate cohort name, and get the cohort object. Skip if course - # is not cohorted. - cohort = None + if cohort_name and not already_warned_not_cohorted: + if not is_course_cohorted(course_id): + row_errors.append({ + 'username': username, + 'email': email, + 'response': _('Course is not cohorted but cohort provided. ' + 'Ignoring cohort assignment for all users.') + }) + already_warned_not_cohorted = True + elif cohort_name in cohorts_cache: + cohort = cohorts_cache[cohort_name] + else: + # Don't attempt to create cohort or assign student if cohort + # does not exist. + try: + cohort = get_cohort_by_name(course_id, cohort_name) + except CourseUserGroup.DoesNotExist: + row_errors.append({ + 'username': username, + 'email': email, + 'response': _('Cohort name not found: {cohort}. ' + 'Ignoring cohort assignment for ' + 'all users.').format(cohort=cohort_name) + }) + cohorts_cache[cohort_name] = cohort + + # Validate course mode. + if not course_mode: + course_mode = default_course_mode + + if (course_mode is not None + and course_mode not in valid_course_modes): + # If `default is None` and the user is already enrolled, + # `CourseEnrollment.change_mode()` will not update the mode, + # hence two error messages. + if default_course_mode is None: + err_msg = _( + 'Invalid course mode: {mode}. Falling back to the ' + 'default mode, or keeping the current mode in case the ' + 'user is already enrolled.' + ).format(mode=course_mode) + else: + err_msg = _( + 'Invalid course mode: {mode}. Failling back to ' + '{default_mode}, or resetting to {default_mode} in case ' + 'the user is already enrolled.' + ).format(mode=course_mode, default_mode=default_course_mode) + row_errors.append({ + 'username': username, + 'email': email, + 'response': err_msg, + }) + course_mode = default_course_mode - if cohort_name and not already_warned_not_cohorted: - if not is_course_cohorted(course_id): + email_params = get_email_params(course, True, secure=request.is_secure()) + try: + validate_email(email) # Raises ValidationError if invalid + except ValidationError: row_errors.append({ 'username': username, 'email': email, - 'response': _('Course is not cohorted but cohort provided. ' - 'Ignoring cohort assignment for all users.') + 'response': _('Invalid email {email_address}.').format(email_address=email) }) - already_warned_not_cohorted = True - elif cohort_name in cohorts_cache: - cohort = cohorts_cache[cohort_name] else: - # Don't attempt to create cohort or assign student if cohort - # does not exist. - try: - cohort = get_cohort_by_name(course_id, cohort_name) - except CourseUserGroup.DoesNotExist: + if User.objects.filter(email=email).exists(): + # Email address already exists. assume it is the correct user + # and just register the user in the course and send an enrollment email. + user = User.objects.get(email=email) + + # see if it is an exact match with email and username + # if it's not an exact match then just display a warning message, but continue onwards + if not User.objects.filter(email=email, username=username).exists(): + warning_message = _( + 'An account with email {email} exists but the provided username {username} ' + 'is different. Enrolling anyway with {email}.' + ).format(email=email, username=username) + + warnings.append({ + 'username': username, 'email': email, 'response': warning_message + }) + log.warning('email %s already exist', email) + else: + log.info( + "user already exists with username '%s' and email '%s'", + username, + email + ) + + # enroll a user if it is not already enrolled. + if not is_user_enrolled_in_course(user, course_id): + # Enroll user to the course and add manual enrollment audit trail + create_manual_course_enrollment( + user=user, + course_id=course_id, + mode=course_mode, + enrolled_by=request.user, + reason='Enrolling via csv upload', + state_transition=UNENROLLED_TO_ENROLLED, + ) + enroll_email(course_id=course_id, + student_email=email, + auto_enroll=True, + email_students=notify_by_email, + email_params=email_params) + else: + # update the course mode if already enrolled + existing_enrollment = CourseEnrollment.get_enrollment(user, course_id) + if existing_enrollment.mode != course_mode: + existing_enrollment.change_mode(mode=course_mode) + if cohort: + try: + add_user_to_cohort(cohort, user) + except ValueError: + # user already in this cohort; ignore + pass + elif is_email_retired(email): + # We are either attempting to enroll a retired user or create a new user with an email which is + # already associated with a retired account. Simply block these attempts. row_errors.append({ 'username': username, 'email': email, - 'response': _('Cohort name not found: {cohort}. ' - 'Ignoring cohort assignment for ' - 'all users.').format(cohort=cohort_name) - }) - cohorts_cache[cohort_name] = cohort - - # Validate course mode. - if not course_mode: - course_mode = default_course_mode - - if (course_mode is not None - and course_mode not in valid_course_modes): - # If `default is None` and the user is already enrolled, - # `CourseEnrollment.change_mode()` will not update the mode, - # hence two error messages. - if default_course_mode is None: - err_msg = _( - 'Invalid course mode: {mode}. Falling back to the ' - 'default mode, or keeping the current mode in case the ' - 'user is already enrolled.' - ).format(mode=course_mode) - else: - err_msg = _( - 'Invalid course mode: {mode}. Failling back to ' - '{default_mode}, or resetting to {default_mode} in case ' - 'the user is already enrolled.' - ).format(mode=course_mode, default_mode=default_course_mode) - row_errors.append({ - 'username': username, - 'email': email, - 'response': err_msg, - }) - course_mode = default_course_mode - - email_params = get_email_params(course, True, secure=request.is_secure()) - try: - validate_email(email) # Raises ValidationError if invalid - except ValidationError: - row_errors.append({ - 'username': username, - 'email': email, - 'response': _('Invalid email {email_address}.').format(email_address=email) - }) - else: - if User.objects.filter(email=email).exists(): - # Email address already exists. assume it is the correct user - # and just register the user in the course and send an enrollment email. - user = User.objects.get(email=email) - - # see if it is an exact match with email and username - # if it's not an exact match then just display a warning message, but continue onwards - if not User.objects.filter(email=email, username=username).exists(): - warning_message = _( - 'An account with email {email} exists but the provided username {username} ' - 'is different. Enrolling anyway with {email}.' - ).format(email=email, username=username) - - warnings.append({ - 'username': username, 'email': email, 'response': warning_message + 'response': _('Invalid email {email_address}.').format(email_address=email), }) - log.warning('email %s already exist', email) + log.warning('Email address %s is associated with a retired user, so course enrollment was ' + # lint-amnesty, pylint: disable=logging-not-lazy + 'blocked.', email) else: - log.info( - "user already exists with username '%s' and email '%s'", + # This email does not yet exist, so we need to create a new account + # If username already exists in the database, then create_and_enroll_user + # will raise an IntegrityError exception. + password = generate_unique_password(generated_passwords) + errors = create_and_enroll_user( + email, username, - email + name, + country, + password, + course_id, + course_mode, + request.user, + email_params, + email_user=notify_by_email, ) + row_errors.extend(errors) + if cohort: + try: + add_user_to_cohort(cohort, email) + except ValueError: + # user already in this cohort; ignore + # NOTE: Checking this here may be unnecessary if we can prove that a + # new user will never be + # automatically assigned to a cohort from the above. + pass + except ValidationError: + row_errors.append({ + 'username': username, + 'email': email, + 'response': _('Invalid email {email_address}.').format(email_address=email), + }) - # enroll a user if it is not already enrolled. - if not is_user_enrolled_in_course(user, course_id): - # Enroll user to the course and add manual enrollment audit trail - create_manual_course_enrollment( - user=user, - course_id=course_id, - mode=course_mode, - enrolled_by=request.user, - reason='Enrolling via csv upload', - state_transition=UNENROLLED_TO_ENROLLED, - ) - enroll_email(course_id=course_id, - student_email=email, - auto_enroll=True, - email_students=notify_by_email, - email_params=email_params) - else: - # update the course mode if already enrolled - existing_enrollment = CourseEnrollment.get_enrollment(user, course_id) - if existing_enrollment.mode != course_mode: - existing_enrollment.change_mode(mode=course_mode) - if cohort: - try: - add_user_to_cohort(cohort, user) - except ValueError: - # user already in this cohort; ignore - pass - elif is_email_retired(email): - # We are either attempting to enroll a retired user or create a new user with an email which is - # already associated with a retired account. Simply block these attempts. - row_errors.append({ - 'username': username, - 'email': email, - 'response': _('Invalid email {email_address}.').format(email_address=email), - }) - log.warning('Email address %s is associated with a retired user, so course enrollment was ' + # lint-amnesty, pylint: disable=logging-not-lazy - 'blocked.', email) - else: - # This email does not yet exist, so we need to create a new account - # If username already exists in the database, then create_and_enroll_user - # will raise an IntegrityError exception. - password = generate_unique_password(generated_passwords) - errors = create_and_enroll_user( - email, - username, - name, - country, - password, - course_id, - course_mode, - request.user, - email_params, - email_user=notify_by_email, - ) - row_errors.extend(errors) - if cohort: - try: - add_user_to_cohort(cohort, email) - except ValueError: - # user already in this cohort; ignore - # NOTE: Checking this here may be unnecessary if we can prove that a new user will never be - # automatically assigned to a cohort from the above. - pass - except ValidationError: - row_errors.append({ - 'username': username, - 'email': email, - 'response': _('Invalid email {email_address}.').format(email_address=email), - }) - - else: - general_errors.append({ - 'username': '', 'email': '', 'response': _('File is not attached.') - }) + else: + general_errors.append({ + 'username': '', 'email': '', 'response': _('File is not attached.') + }) - results = { - 'row_errors': row_errors, - 'general_errors': general_errors, - 'warnings': warnings - } - return JsonResponse(results) + results = { + 'row_errors': row_errors, + 'general_errors': general_errors, + 'warnings': warnings + } + return JsonResponse(results) def generate_random_string(length): @@ -983,17 +989,8 @@ def bulk_beta_modify_access(request, course_id): return JsonResponse(response_payload) -@require_POST -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_course_permission(permissions.EDIT_COURSE_ACCESS) -@require_post_params( - unique_student_identifier="email or username of user to change access", - rolename="'instructor', 'staff', 'beta', or 'ccx_coach'", - action="'allow' or 'revoke'" -) -@common_exceptions_400 -def modify_access(request, course_id): +@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') +class ModifyAccess(APIView): """ Modify staff/instructor access of other user. Requires instructor access. @@ -1005,67 +1002,77 @@ def modify_access(request, course_id): rolename is one of ['instructor', 'staff', 'beta', 'ccx_coach'] action is one of ['allow', 'revoke'] """ - course_id = CourseKey.from_string(course_id) - course = get_course_with_access( - request.user, 'instructor', course_id, depth=None - ) - unique_student_identifier = request.POST.get('unique_student_identifier') - try: - user = get_student_from_identifier(unique_student_identifier) - except User.DoesNotExist: - response_payload = { - 'unique_student_identifier': unique_student_identifier, - 'userDoesNotExist': True, - } - return JsonResponse(response_payload) + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.EDIT_COURSE_ACCESS + serializer_class = AccessSerializer - # Check that user is active, because add_users - # in common/djangoapps/student/roles.py fails - # silently when we try to add an inactive user. - if not user.is_active: - response_payload = { - 'unique_student_identifier': user.username, - 'inactiveUser': True, - } - return JsonResponse(response_payload) + @method_decorator(ensure_csrf_cookie) + def post(self, request, course_id): + """ + Modify staff/instructor access of other user. + Requires instructor access. + """ + course_id = CourseKey.from_string(course_id) + course = get_course_with_access( + request.user, 'instructor', course_id, depth=None + ) - rolename = request.POST.get('rolename') - action = request.POST.get('action') + serializer_data = AccessSerializer(data=request.data) + if not serializer_data.is_valid(): + return HttpResponseBadRequest(reason=serializer_data.errors) + + user = serializer_data.validated_data.get('unique_student_identifier') + if not user: + response_payload = { + 'unique_student_identifier': request.data.get('unique_student_identifier'), + 'userDoesNotExist': True, + } + return JsonResponse(response_payload) - if rolename not in ROLES: - error = strip_tags(f"unknown rolename '{rolename}'") - log.error(error) - return HttpResponseBadRequest(error) + if not user.is_active: + response_payload = { + 'unique_student_identifier': user.username, + 'inactiveUser': True, + } + return JsonResponse(response_payload) + + rolename = serializer_data.data['rolename'] + action = serializer_data.data['action'] + + if rolename not in ROLES: + error = strip_tags(f"unknown rolename '{rolename}'") + log.error(error) + return HttpResponseBadRequest(error) + + # disallow instructors from removing their own instructor access. + if rolename == 'instructor' and user == request.user and action != 'allow': + response_payload = { + 'unique_student_identifier': user.username, + 'rolename': rolename, + 'action': action, + 'removingSelfAsInstructor': True, + } + return JsonResponse(response_payload) + + if action == 'allow': + allow_access(course, user, rolename) + if not is_user_enrolled_in_course(user, course_id): + CourseEnrollment.enroll(user, course_id) + elif action == 'revoke': + revoke_access(course, user, rolename) + else: + return HttpResponseBadRequest(strip_tags( + f"unrecognized action u'{action}'" + )) - # disallow instructors from removing their own instructor access. - if rolename == 'instructor' and user == request.user and action != 'allow': response_payload = { 'unique_student_identifier': user.username, 'rolename': rolename, 'action': action, - 'removingSelfAsInstructor': True, + 'success': 'yes', } return JsonResponse(response_payload) - if action == 'allow': - allow_access(course, user, rolename) - if not is_user_enrolled_in_course(user, course_id): - CourseEnrollment.enroll(user, course_id) - elif action == 'revoke': - revoke_access(course, user, rolename) - else: - return HttpResponseBadRequest(strip_tags( - f"unrecognized action u'{action}'" - )) - - response_payload = { - 'unique_student_identifier': user.username, - 'rolename': rolename, - 'action': action, - 'success': 'yes', - } - return JsonResponse(response_payload) - @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') class ListCourseRoleMembersView(APIView): From 7add5fb0e467811cdd9ec3507f35a7510d9b033b Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 20 Aug 2024 19:07:01 +0500 Subject: [PATCH 5/8] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index ea0ee4266174..fd869c4165af 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -3010,11 +3010,8 @@ def post(self, request, course_id): student = serializer_data.validated_data.get('student') if not student: - response_payload = { - 'student': request.data.get('student'), - 'userDoesNotExist': True, - } - return JsonResponse(response_payload) + response_payload = f'Could not find student matching identifier: {request.data.get("student")}' + return HttpResponseBadRequest(response_payload) course = get_course_by_id(CourseKey.from_string(course_id)) return Response(dump_student_extensions(course, student)) From f442a515cab955bfa199167ef48454d91f1954fd Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 20 Aug 2024 19:11:44 +0500 Subject: [PATCH 6/8] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index fd869c4165af..06392f4c55dd 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -3011,7 +3011,7 @@ def post(self, request, course_id): student = serializer_data.validated_data.get('student') if not student: response_payload = f'Could not find student matching identifier: {request.data.get("student")}' - return HttpResponseBadRequest(response_payload) + return JsonResponse({'error': response_payload}, status=400) course = get_course_by_id(CourseKey.from_string(course_id)) return Response(dump_student_extensions(course, student)) From dd1a957125679d7a7097d8e4667771301d2bdead Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 20 Aug 2024 19:13:23 +0500 Subject: [PATCH 7/8] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 06392f4c55dd..ab078ba166ac 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -2970,7 +2970,6 @@ def show_unit_extensions(request, course_id): @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') -# @method_decorator(handle_dashboard_error) class ShowStudentExtensions(APIView): """ Shows all of the due date extensions granted to a particular student in a From e84db6daf031272f1e532952146f91c46faf0729 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 20 Aug 2024 19:17:05 +0500 Subject: [PATCH 8/8] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index ab078ba166ac..d5b99ed7ea1b 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -2980,7 +2980,6 @@ class ShowStudentExtensions(APIView): permission_name = permissions.GIVE_STUDENT_EXTENSION @method_decorator(ensure_csrf_cookie) - @method_decorator(handle_dashboard_error) def post(self, request, course_id): """ Handles POST requests to retrieve due date extensions for a specific student