Skip to content

Commit

Permalink
Merge branch 'master' into jci/issue#35836
Browse files Browse the repository at this point in the history
  • Loading branch information
jciasenza authored Dec 17, 2024
2 parents f1e94fc + 882475d commit 8201e87
Show file tree
Hide file tree
Showing 12 changed files with 247 additions and 98 deletions.
10 changes: 0 additions & 10 deletions common/djangoapps/util/memcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

from django.conf import settings
from django.utils.encoding import smart_str
from edx_django_utils.monitoring.utils import increment


def fasthash(string):
Expand Down Expand Up @@ -49,18 +48,9 @@ def safe_key(key, key_prefix, version):
# Attempt to combine the prefix, version, and key
combined = ":".join([key_prefix, version, key])

# Temporary: Add observability to large-key hashing to help us
# understand the safety of a cutover from md4 to blake2b hashing.
# See https://github.com/edx/edx-arch-experiments/issues/872
increment('memcache.safe_key.called')

# If the total length is too long for memcache, hash it
if len(combined) > 250:
combined = fasthash(combined)
# Temporary: See
# https://github.com/edx/edx-arch-experiments/issues/872 and
# previous comment.
increment('memcache.safe_key.hash_large')

# Return the result
return combined
9 changes: 9 additions & 0 deletions lms/djangoapps/instructor/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)})
Expand Down
151 changes: 77 additions & 74 deletions lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
CertificateSerializer,
CertificateStatusesSerializer,
ListInstructorTaskInputSerializer,
ModifyAccessSerializer,
RoleNameSerializer,
SendEmailSerializer,
ShowStudentExtensionSerializer,
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/instructor/views/api_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
71 changes: 71 additions & 0 deletions lms/djangoapps/instructor/views/serializer.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions openedx/core/djangoapps/schedules/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from edx_ace.recipient import Recipient
from edx_ace.recipient_resolver import RecipientResolver
from edx_django_utils.monitoring import function_trace, set_custom_attribute
from openedx_filters.learning.filters import ScheduleQuerySetRequested

from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link, can_show_verified_upgrade
from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher
Expand Down Expand Up @@ -154,6 +155,10 @@ def get_schedules_with_target_date_by_bin_and_orgs(

schedules = self.filter_by_org(schedules)

# .. filter_implemented_name: ScheduleQuerySetRequested
# .. filter_type: org.openedx.learning.schedule.queryset.requested.v1
schedules = ScheduleQuerySetRequested.run_filter(schedules)

if "read_replica" in settings.DATABASES:
schedules = schedules.using("read_replica")

Expand Down
Loading

0 comments on commit 8201e87

Please sign in to comment.