diff --git a/lms/djangoapps/discussion/rest_api/discussions_notifications.py b/lms/djangoapps/discussion/rest_api/discussions_notifications.py
index 21b1e27fcdf8..96e392c35d2a 100644
--- a/lms/djangoapps/discussion/rest_api/discussions_notifications.py
+++ b/lms/djangoapps/discussion/rest_api/discussions_notifications.py
@@ -3,7 +3,10 @@
"""
import re
+from bs4 import BeautifulSoup
from django.conf import settings
+from django.utils.text import Truncator
+
from lms.djangoapps.discussion.django_comment_client.permissions import get_team
from openedx_events.learning.data import UserNotificationData, CourseNotificationData
from openedx_events.learning.signals import USER_NOTIFICATION_REQUESTED, COURSE_NOTIFICATION_REQUESTED
@@ -27,13 +30,24 @@ class DiscussionNotificationSender:
Class to send notifications to users who are subscribed to the thread.
"""
- def __init__(self, thread, course, creator, parent_id=None):
+ def __init__(self, thread, course, creator, parent_id=None, comment_id=None):
self.thread = thread
self.course = course
self.creator = creator
self.parent_id = parent_id
+ self.comment_id = comment_id
self.parent_response = None
+ self.comment = None
self._get_parent_response()
+ self._get_comment()
+
+ def _get_comment(self):
+ """
+ Get comment object
+ """
+ if not self.comment_id:
+ return
+ self.comment = Comment(id=self.comment_id).retrieve()
def _send_notification(self, user_ids, notification_type, extra_context=None):
"""
@@ -99,7 +113,10 @@ def send_new_response_notification(self):
there is a response to the main thread.
"""
if not self.parent_id and self.creator.id != int(self.thread.user_id):
- self._send_notification([self.thread.user_id], "new_response")
+ context = {
+ 'email_content': clean_thread_html_body(self.comment.body),
+ }
+ self._send_notification([self.thread.user_id], "new_response", extra_context=context)
def _response_and_thread_has_same_creator(self) -> bool:
"""
@@ -136,6 +153,7 @@ def send_new_comment_notification(self):
context = {
"author_name": str(author_name),
"author_pronoun": str(author_pronoun),
+ "email_content": clean_thread_html_body(self.comment.body),
}
self._send_notification([self.thread.user_id], "new_comment", extra_context=context)
@@ -149,7 +167,14 @@ def send_new_comment_on_response_notification(self):
self.creator.id != int(self.parent_response.user_id) and not
self._response_and_thread_has_same_creator()
):
- self._send_notification([self.parent_response.user_id], "new_comment_on_response")
+ context = {
+ "email_content": clean_thread_html_body(self.comment.body),
+ }
+ self._send_notification(
+ [self.parent_response.user_id],
+ "new_comment_on_response",
+ extra_context=context
+ )
def _check_if_subscriber_is_not_thread_or_content_creator(self, subscriber_id) -> bool:
"""
@@ -190,7 +215,12 @@ def send_response_on_followed_post_notification(self):
# Remove duplicate users from the list of users to send notification
users = list(set(users))
if not self.parent_id:
- self._send_notification(users, "response_on_followed_post")
+ self._send_notification(
+ users,
+ "response_on_followed_post",
+ extra_context={
+ "email_content": clean_thread_html_body(self.comment.body),
+ })
else:
author_name = f"{self.parent_response.username}'s"
# use 'their' if comment author is also response author.
@@ -206,6 +236,7 @@ def send_response_on_followed_post_notification(self):
extra_context={
"author_name": str(author_name),
"author_pronoun": str(author_pronoun),
+ "email_content": clean_thread_html_body(self.comment.body),
}
)
@@ -289,7 +320,8 @@ def send_new_thread_created_notification(self):
]
context = {
'username': self.creator.username,
- 'post_title': self.thread.title
+ 'post_title': self.thread.title,
+ "email_content": clean_thread_html_body(self.thread.body),
}
self._send_course_wide_notification(notification_type, audience_filters, context)
@@ -339,3 +371,26 @@ def is_discussion_cohorted(course_key_str):
def remove_html_tags(text):
clean = re.compile('<.*?>')
return re.sub(clean, '', text)
+
+
+def clean_thread_html_body(html_body):
+ """
+ Get post body with tags removed and limited to 500 characters
+ """
+ html_body = BeautifulSoup(Truncator(html_body).chars(500, html=True), 'html.parser')
+
+ tags_to_remove = [
+ "a", "link", # Link Tags
+ "img", "picture", "source", # Image Tags
+ "video", "track", # Video Tags
+ "audio", # Audio Tags
+ "embed", "object", "iframe", # Embedded Content
+ "script"
+ ]
+
+ # Remove the specified tags while keeping their content
+ for tag in tags_to_remove:
+ for match in html_body.find_all(tag):
+ match.unwrap()
+
+ return str(html_body)
diff --git a/lms/djangoapps/discussion/rest_api/tasks.py b/lms/djangoapps/discussion/rest_api/tasks.py
index 45bf41fe905f..a87fafd4ca54 100644
--- a/lms/djangoapps/discussion/rest_api/tasks.py
+++ b/lms/djangoapps/discussion/rest_api/tasks.py
@@ -33,7 +33,7 @@ def send_thread_created_notification(thread_id, course_key_str, user_id):
@shared_task
@set_code_owner_attribute
-def send_response_notifications(thread_id, course_key_str, user_id, parent_id=None):
+def send_response_notifications(thread_id, course_key_str, user_id, comment_id, parent_id=None):
"""
Send notifications to users who are subscribed to the thread.
"""
@@ -43,7 +43,7 @@ def send_response_notifications(thread_id, course_key_str, user_id, parent_id=No
thread = Thread(id=thread_id).retrieve()
user = User.objects.get(id=user_id)
course = get_course_with_access(user, 'load', course_key, check_if_enrolled=True)
- notification_sender = DiscussionNotificationSender(thread, course, user, parent_id)
+ notification_sender = DiscussionNotificationSender(thread, course, user, parent_id, comment_id)
notification_sender.send_new_comment_notification()
notification_sender.send_new_response_notification()
notification_sender.send_new_comment_on_response_notification()
diff --git a/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py b/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py
index b4d3f3d18a0d..f1a71fd1239e 100644
--- a/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py
+++ b/lms/djangoapps/discussion/rest_api/tests/test_discussions_notifications.py
@@ -1,13 +1,14 @@
"""
Unit tests for the DiscussionNotificationSender class
"""
-
+import re
import unittest
from unittest.mock import MagicMock, patch
import pytest
-from lms.djangoapps.discussion.rest_api.discussions_notifications import DiscussionNotificationSender
+from lms.djangoapps.discussion.rest_api.discussions_notifications import DiscussionNotificationSender, \
+ clean_thread_html_body
@patch('lms.djangoapps.discussion.rest_api.discussions_notifications.DiscussionNotificationSender'
@@ -88,3 +89,82 @@ def test_send_reported_content_notification_for_thread(self, mock_send_notificat
self.notification_sender.send_reported_content_notification()
self._assert_send_notification_called_with(mock_send_notification, 'thread')
+
+
+class TestCleanThreadHtmlBody(unittest.TestCase):
+ """
+ Tests for the clean_thread_html_body function
+ """
+
+ def test_html_tags_removal(self):
+ """
+ Test that the clean_thread_html_body function removes unwanted HTML tags
+ """
+ html_body = """
+ This is a link to a page.
+ Here is an image: 
+ Embedded video:
+ Script test:
+ Some other content that should remain.
+ """
+ expected_output = ("This is a link to a page.
"
+ "Here is an image:
"
+ "Embedded video:
"
+ "Script test: alert('hello');
"
+ "Some other content that should remain.
")
+
+ result = clean_thread_html_body(html_body)
+
+ def normalize_html(text):
+ """
+ Normalize the output by removing extra whitespace, newlines, and spaces between tags
+ """
+ text = re.sub(r'\s+', ' ', text).strip() # Replace any sequence of whitespace with a single space
+ text = re.sub(r'>\s+<', '><', text) # Remove spaces between HTML tags
+ return text
+
+ normalized_result = normalize_html(result)
+ normalized_expected_output = normalize_html(expected_output)
+
+ self.assertEqual(normalized_result, normalized_expected_output)
+
+ def test_truncate_html_body(self):
+ """
+ Test that the clean_thread_html_body function truncates the HTML body to 500 characters
+ """
+ html_body = """
+ This is a long text that should be truncated to 500 characters.
+ """ * 20 # Repeat to exceed 500 characters
+
+ result = clean_thread_html_body(html_body)
+ self.assertGreaterEqual(500, len(result))
+
+ def test_no_tags_to_remove(self):
+ """
+ Test that the clean_thread_html_body function does not remove any tags if there are no unwanted tags
+ """
+ html_body = "This paragraph has no tags to remove.
"
+ expected_output = "This paragraph has no tags to remove.
"
+
+ result = clean_thread_html_body(html_body)
+ self.assertEqual(result, expected_output)
+
+ def test_empty_html_body(self):
+ """
+ Test that the clean_thread_html_body function returns an empty string if the input is an empty string
+ """
+ html_body = ""
+ expected_output = ""
+
+ result = clean_thread_html_body(html_body)
+ self.assertEqual(result, expected_output)
+
+ def test_only_script_tag(self):
+ """
+ Test that the clean_thread_html_body function removes the script tag and its content
+ """
+ html_body = ""
+ expected_output = "alert('Hello');"
+
+ result = clean_thread_html_body(html_body)
+ self.assertEqual(result.strip(), expected_output)
diff --git a/lms/djangoapps/discussion/rest_api/tests/test_tasks.py b/lms/djangoapps/discussion/rest_api/tests/test_tasks.py
index 28cfe3395cb6..8efd5cd49cbd 100644
--- a/lms/djangoapps/discussion/rest_api/tests/test_tasks.py
+++ b/lms/djangoapps/discussion/rest_api/tests/test_tasks.py
@@ -273,6 +273,17 @@ def setUp(self):
})
self._register_subscriptions_endpoint()
+ self.comment = ThreadMock(thread_id=4, creator=self.user_2, title='test comment', body='comment body')
+ self.register_get_comment_response(
+ {
+ 'id': self.comment.id,
+ 'thread_id': self.thread.id,
+ 'parent_id': None,
+ 'user_id': self.comment.user_id,
+ 'body': self.comment.body,
+ }
+ )
+
def test_basic(self):
"""
Left empty intentionally. This test case is inherited from DiscussionAPIViewTestMixin
@@ -292,7 +303,13 @@ def test_send_notification_to_thread_creator(self):
# Post the form or do what it takes to send the signal
- send_response_notifications(self.thread.id, str(self.course.id), self.user_2.id, parent_id=None)
+ send_response_notifications(
+ self.thread.id,
+ str(self.course.id),
+ self.user_2.id,
+ self.comment.id,
+ parent_id=None
+ )
self.assertEqual(handler.call_count, 2)
args = handler.call_args_list[0][1]['notification_data']
self.assertEqual([int(user_id) for user_id in args.user_ids], [self.user_1.id])
@@ -300,6 +317,7 @@ def test_send_notification_to_thread_creator(self):
expected_context = {
'replier_name': self.user_2.username,
'post_title': 'test thread',
+ 'email_content': self.comment.body,
'course_name': self.course.display_name,
'sender_id': self.user_2.id
}
@@ -325,7 +343,13 @@ def test_send_notification_to_parent_threads(self):
'user_id': self.thread_2.user_id
})
- send_response_notifications(self.thread.id, str(self.course.id), self.user_3.id, parent_id=self.thread_2.id)
+ send_response_notifications(
+ self.thread.id,
+ str(self.course.id),
+ self.user_3.id,
+ self.comment.id,
+ parent_id=self.thread_2.id
+ )
# check if 2 call are made to the handler i.e. one for the response creator and one for the thread creator
self.assertEqual(handler.call_count, 2)
@@ -337,6 +361,7 @@ def test_send_notification_to_parent_threads(self):
expected_context = {
'replier_name': self.user_3.username,
'post_title': self.thread.title,
+ 'email_content': self.comment.body,
'author_name': 'dummy\'s',
'author_pronoun': 'dummy\'s',
'course_name': self.course.display_name,
@@ -355,6 +380,7 @@ def test_send_notification_to_parent_threads(self):
expected_context = {
'replier_name': self.user_3.username,
'post_title': self.thread.title,
+ 'email_content': self.comment.body,
'course_name': self.course.display_name,
'sender_id': self.user_3.id
}
@@ -372,7 +398,13 @@ def test_no_signal_on_creators_own_thread(self):
"""
handler = mock.Mock()
USER_NOTIFICATION_REQUESTED.connect(handler)
- send_response_notifications(self.thread.id, str(self.course.id), self.user_1.id, parent_id=None)
+
+ send_response_notifications(
+ self.thread.id,
+ str(self.course.id),
+ self.user_1.id,
+ self.comment.id, parent_id=None
+ )
self.assertEqual(handler.call_count, 1)
def test_comment_creators_own_response(self):
@@ -389,7 +421,13 @@ def test_comment_creators_own_response(self):
'user_id': self.thread_3.user_id
})
- send_response_notifications(self.thread.id, str(self.course.id), self.user_3.id, parent_id=self.thread_2.id)
+ send_response_notifications(
+ self.thread.id,
+ str(self.course.id),
+ self.user_3.id,
+ parent_id=self.thread_2.id,
+ comment_id=self.comment.id
+ )
# check if 1 call is made to the handler i.e. for the thread creator
self.assertEqual(handler.call_count, 2)
@@ -404,6 +442,7 @@ def test_comment_creators_own_response(self):
'author_pronoun': 'your',
'course_name': self.course.display_name,
'sender_id': self.user_3.id,
+ 'email_content': self.comment.body
}
self.assertDictEqual(args_comment.context, expected_context)
self.assertEqual(
@@ -429,7 +468,13 @@ def test_send_notification_to_followers(self, parent_id, notification_type):
USER_NOTIFICATION_REQUESTED.connect(handler)
# Post the form or do what it takes to send the signal
- notification_sender = DiscussionNotificationSender(self.thread, self.course, self.user_2, parent_id=parent_id)
+ notification_sender = DiscussionNotificationSender(
+ self.thread,
+ self.course,
+ self.user_2,
+ parent_id=parent_id,
+ comment_id=self.comment.id
+ )
notification_sender.send_response_on_followed_post_notification()
self.assertEqual(handler.call_count, 1)
args = handler.call_args[1]['notification_data']
@@ -439,6 +484,7 @@ def test_send_notification_to_followers(self, parent_id, notification_type):
expected_context = {
'replier_name': self.user_2.username,
'post_title': 'test thread',
+ 'email_content': self.comment.body,
'course_name': self.course.display_name,
'sender_id': self.user_2.id,
}
@@ -516,6 +562,7 @@ def test_new_comment_notification(self):
thread = ThreadMock(thread_id=1, creator=self.user_1, title='test thread')
response = ThreadMock(thread_id=2, creator=self.user_2, title='test response')
+ comment = ThreadMock(thread_id=3, creator=self.user_2, title='test comment', body='comment body')
self.register_get_thread_response({
'id': thread.id,
'course_id': str(self.course.id),
@@ -530,12 +577,20 @@ def test_new_comment_notification(self):
'thread_id': thread.id,
'user_id': response.user_id
})
+ self.register_get_comment_response({
+ 'id': comment.id,
+ 'parent_id': response.id,
+ 'user_id': comment.user_id,
+ 'body': comment.body
+ })
self.register_get_subscriptions(1, {})
- send_response_notifications(thread.id, str(self.course.id), self.user_2.id, parent_id=response.id)
+ send_response_notifications(thread.id, str(self.course.id), self.user_2.id, parent_id=response.id,
+ comment_id=comment.id)
handler.assert_called_once()
context = handler.call_args[1]['notification_data'].context
self.assertEqual(context['author_name'], 'dummy\'s')
self.assertEqual(context['author_pronoun'], 'their')
+ self.assertEqual(context['email_content'], comment.body)
@override_waffle_flag(ENABLE_NOTIFICATIONS, active=True)
diff --git a/lms/djangoapps/discussion/rest_api/tests/utils.py b/lms/djangoapps/discussion/rest_api/tests/utils.py
index 989fd63855d5..27e34705f5df 100644
--- a/lms/djangoapps/discussion/rest_api/tests/utils.py
+++ b/lms/djangoapps/discussion/rest_api/tests/utils.py
@@ -675,12 +675,13 @@ class ThreadMock(object):
A mock thread object
"""
- def __init__(self, thread_id, creator, title, parent_id=None):
+ def __init__(self, thread_id, creator, title, parent_id=None, body=''):
self.id = thread_id
self.user_id = str(creator.id)
self.username = creator.username
self.title = title
self.parent_id = parent_id
+ self.body = body
def url_with_id(self, params):
return f"http://example.com/{params['id']}"
diff --git a/lms/djangoapps/discussion/signals/handlers.py b/lms/djangoapps/discussion/signals/handlers.py
index 35288cdbd9be..2aa7d36456c4 100644
--- a/lms/djangoapps/discussion/signals/handlers.py
+++ b/lms/djangoapps/discussion/signals/handlers.py
@@ -176,8 +176,9 @@ def create_comment_created_notification(*args, **kwargs):
comment = kwargs['post']
thread_id = comment.attributes['thread_id']
parent_id = comment.attributes['parent_id']
+ comment_id = comment.attributes['id']
course_key_str = comment.attributes['course_id']
- send_response_notifications.apply_async(args=[thread_id, course_key_str, user.id, parent_id])
+ send_response_notifications.apply_async(args=[thread_id, course_key_str, user.id, comment_id, parent_id])
@receiver(signals.comment_endorsed)
diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py
index 300543def6c2..896d0deadcd9 100644
--- a/lms/djangoapps/instructor/enrollment.py
+++ b/lms/djangoapps/instructor/enrollment.py
@@ -34,6 +34,7 @@
get_event_transaction_id,
set_event_transaction_type
)
+from lms.djangoapps.branding.api import get_logo_url_for_email
from lms.djangoapps.courseware.models import StudentModule
from lms.djangoapps.grades.api import constants as grades_constants
from lms.djangoapps.grades.api import disconnect_submissions_signal_receiver
@@ -489,6 +490,7 @@ def get_email_params(course, auto_enroll, secure=True, course_key=None, display_
'contact_mailing_address': contact_mailing_address,
'platform_name': platform_name,
'site_configuration_values': configuration_helpers.get_current_site_configuration_values(),
+ 'logo_url': get_logo_url_for_email(),
}
return email_params
diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py
index 59ccfac6caa1..741f57ef6d2b 100644
--- a/lms/djangoapps/instructor/tests/test_enrollment.py
+++ b/lms/djangoapps/instructor/tests/test_enrollment.py
@@ -23,6 +23,7 @@
from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed, anonymous_id_for_user
from common.djangoapps.student.roles import CourseCcxCoachRole
from common.djangoapps.student.tests.factories import AdminFactory, UserFactory
+from lms.djangoapps.branding.api import get_logo_url_for_email
from lms.djangoapps.ccx.tests.factories import CcxFactory
from lms.djangoapps.course_blocks.api import get_course_blocks
from lms.djangoapps.courseware.models import StudentModule
@@ -940,6 +941,7 @@ def setUpClass(cls):
)
cls.course_about_url = cls.course_url + 'about'
cls.registration_url = f'https://{site}/register'
+ cls.logo_url = get_logo_url_for_email()
def test_normal_params(self):
# For a normal site, what do we expect to get for the URLs?
@@ -950,6 +952,7 @@ def test_normal_params(self):
assert result['course_about_url'] == self.course_about_url
assert result['registration_url'] == self.registration_url
assert result['course_url'] == self.course_url
+ assert result['logo_url'] == self.logo_url
def test_marketing_params(self):
# For a site with a marketing front end, what do we expect to get for the URLs?
@@ -962,6 +965,19 @@ def test_marketing_params(self):
assert result['course_about_url'] is None
assert result['registration_url'] == self.registration_url
assert result['course_url'] == self.course_url
+ assert result['logo_url'] == self.logo_url
+
+ @patch('lms.djangoapps.instructor.enrollment.get_logo_url_for_email', return_value='https://www.logo.png')
+ def test_logo_url_params(self, mock_get_logo_url_for_email):
+ # Verify that the logo_url is correctly set in the email params
+ result = get_email_params(self.course, False)
+
+ assert result['auto_enroll'] is False
+ assert result['course_about_url'] == self.course_about_url
+ assert result['registration_url'] == self.registration_url
+ assert result['course_url'] == self.course_url
+ mock_get_logo_url_for_email.assert_called_once()
+ assert result['logo_url'] == 'https://www.logo.png'
@ddt.ddt
diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py
index 7ca1e70467b0..1aa40b5e3376 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 (
+ 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
from openedx.core.djangoapps.course_groups.models import CourseUserGroup
@@ -2979,20 +2981,50 @@ 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')
+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)
+ 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_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 = f'Could not find student matching identifier: {request.data.get("student")}'
+ return JsonResponse({'error': response_payload}, status=400)
+
+ 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/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py
index c0e12e46756d..18da0b63b218 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..0697bed6832d 100644
--- a/lms/djangoapps/instructor/views/serializer.py
+++ b/lms/djangoapps/instructor/views/serializer.py
@@ -59,3 +59,21 @@ 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:
+ return None
+
+ return user
diff --git a/lms/djangoapps/verify_student/docs/decisions/0001_extending_identity_verification.rst b/lms/djangoapps/verify_student/docs/decisions/0001_extending_identity_verification.rst
new file mode 100644
index 000000000000..08735188fcdc
--- /dev/null
+++ b/lms/djangoapps/verify_student/docs/decisions/0001_extending_identity_verification.rst
@@ -0,0 +1,65 @@
+0001. Extending Identity Verification
+#####################################
+
+Status
+******
+
+**Accepted** *2024-08-26*
+
+Context
+*******
+
+The backend implementation of identity verification (IDV) is in the `verify_student Django application`_. The
+`verify_student Django application`_ also contains a frontend user experience for performing photo IDV via an
+integration with Software Secure. There is also a `React-based implementation of this flow`_ in the
+`frontend-app-account MFE`_, so the frontend user experience stored in the `verify_student Django application`_ is often
+called the "legacy flow".
+
+The current architecture of the `verify_student Django application`_ requires that any additional implementations of IDV
+are stored in the application. For example, the Software Secure integration is stored in this application even though
+it is a custom integration that the Open edX community does not use.
+
+Different Open edX operators have different IDV needs. There is currently no way to add additional IDV implementations
+to the platform without committing them to the core. The `verify_student Django application`_ needs enhanced
+extensibility mechanisms to enable per-deployment integration of IDV implementations without modifying the core.
+
+Decision
+********
+
+* We will support the integration of additional implementations of IDV through the use of Python plugins into the
+ platform.
+* We will add a ``VerificationAttempt`` model, which will store generic, implementation-agnostic information about an
+ IDV attempt.
+* We will expose a simple Python API to write and update instances of the ``VerificationAttempt`` model. This will
+ enable plugins to publish information about their IDV attempts to the platform.
+* The ``VerificationAttempt`` model will be integrated into the `verify_student Django application`_, particularly into
+ the `IDVerificationService`_.
+* We will emit Open edX events for each status change of a ``VerificationAttempt``.
+* We will add an Open edX filter hook to change the URL of the photo IDV frontend.
+
+Consequences
+************
+
+* It will become possible for Open edX operators to implement and integrate any additional forms of IDV necessary for
+ their deployment.
+* The `verify_student Django application`_ will contain both concrete implementations of forms of IDV (i.e. manual, SSO,
+ Software Secure, etc.) and a generic, extensible implementation. The work to deprecate and remove the Software Secure
+ integration and to transition the other existing forms of IDV (i.e. manual and SSO) to Django plugins will occur
+ independently of the improvements to extensibility described in this decision.
+
+Rejected Alternatives
+*********************
+
+We considered introducing a ``fetch_verification_attempts`` filter hook to allow plugins to expose additional
+``VerificationAttempts`` to the platform in lieu of an additional model. However, doing database queries via filter
+hooks can cause unpredictable performance problems, and this has been a pain point for Open edX.
+
+References
+**********
+`[Proposal] Add Extensibility Mechanisms to IDV to Enable Integration of New IDV Vendor Persona `_
+`Add Extensibility Mechanisms to IDV to Enable Integration of New IDV Vendor Persona `_
+
+.. _frontend-app-account MFE: https://github.com/openedx/frontend-app-account
+.. _IDVerificationService: https://github.com/openedx/edx-platform/blob/master/lms/djangoapps/verify_student/services.py#L55
+.. _React-based implementation of this flow: https://github.com/openedx/frontend-app-account/tree/master/src/id-verification
+.. _verify_student Django application: https://github.com/openedx/edx-platform/tree/master/lms/djangoapps/verify_student
diff --git a/lms/djangoapps/verify_student/migrations/0015_verificationattempt.py b/lms/djangoapps/verify_student/migrations/0015_verificationattempt.py
new file mode 100644
index 000000000000..3f01047f9f51
--- /dev/null
+++ b/lms/djangoapps/verify_student/migrations/0015_verificationattempt.py
@@ -0,0 +1,33 @@
+# Generated by Django 4.2.15 on 2024-08-26 14:05
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+import django.utils.timezone
+import model_utils.fields
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+ ('verify_student', '0014_remove_softwaresecurephotoverification_expiry_date'),
+ ]
+
+ operations = [
+ migrations.CreateModel(
+ name='VerificationAttempt',
+ fields=[
+ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+ ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')),
+ ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')),
+ ('name', models.CharField(blank=True, max_length=255)),
+ ('status', models.CharField(choices=[('created', 'created'), ('pending', 'pending'), ('approved', 'approved'), ('denied', 'denied')], max_length=64)),
+ ('expiration_datetime', models.DateTimeField(blank=True, null=True)),
+ ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
+ ],
+ options={
+ 'abstract': False,
+ },
+ ),
+ ]
diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py
index f7750a4cd662..903d80bf9245 100644
--- a/lms/djangoapps/verify_student/models.py
+++ b/lms/djangoapps/verify_student/models.py
@@ -31,6 +31,7 @@
from django.utils.translation import gettext_lazy
from model_utils import Choices
from model_utils.models import StatusModel, TimeStampedModel
+from lms.djangoapps.verify_student.statuses import VerificationAttemptStatus
from opaque_keys.edx.django.models import CourseKeyField
from lms.djangoapps.verify_student.ssencrypt import (
@@ -1189,3 +1190,27 @@ class Meta:
def __str__(self):
return str(self.arguments)
+
+
+class VerificationAttempt(TimeStampedModel):
+ """
+ The model represents impelementation-agnostic information about identity verification (IDV) attempts.
+
+ Plugins that implement forms of IDV can store information about IDV attempts in this model for use across
+ the platform.
+ """
+ user = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE)
+ name = models.CharField(blank=True, max_length=255)
+
+ STATUS_CHOICES = [
+ VerificationAttemptStatus.created,
+ VerificationAttemptStatus.pending,
+ VerificationAttemptStatus.approved,
+ VerificationAttemptStatus.denied,
+ ]
+ status = models.CharField(max_length=64, choices=[(status, status) for status in STATUS_CHOICES])
+
+ expiration_datetime = models.DateTimeField(
+ null=True,
+ blank=True,
+ )
diff --git a/lms/djangoapps/verify_student/statuses.py b/lms/djangoapps/verify_student/statuses.py
new file mode 100644
index 000000000000..b55a9042e0f6
--- /dev/null
+++ b/lms/djangoapps/verify_student/statuses.py
@@ -0,0 +1,21 @@
+"""
+Status enums for verify_student.
+"""
+
+
+class VerificationAttemptStatus:
+ """This class describes valid statuses for a verification attempt to be in."""
+
+ # This is the initial state of a verification attempt, before a learner has started IDV.
+ created = "created"
+
+ # A verification attempt is pending when it has been started but has not yet been completed.
+ pending = "pending"
+
+ # A verification attempt is approved when it has been approved by some mechanism (e.g. automatic review, manual
+ # review, etc).
+ approved = "approved"
+
+ # A verification attempt is denied when it has been denied by some mechanism (e.g. automatic review, manual review,
+ # etc).
+ denied = "denied"
diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py
index 4262b8a7b13f..9fb49b24b6d1 100644
--- a/openedx/core/djangoapps/content/search/api.py
+++ b/openedx/core/djangoapps/content/search/api.py
@@ -19,6 +19,7 @@
from meilisearch.models.task import TaskInfo
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import LibraryLocatorV2
+from openedx_learning.api import authoring as authoring_api
from common.djangoapps.student.roles import GlobalStaff
from rest_framework.request import Request
from common.djangoapps.student.role_helpers import get_course_roles
@@ -31,8 +32,9 @@
Fields,
meili_id_from_opaque_key,
searchable_doc_for_course_block,
+ searchable_doc_for_collection,
searchable_doc_for_library_block,
- searchable_doc_tags
+ searchable_doc_tags,
)
log = logging.getLogger(__name__)
@@ -294,12 +296,16 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
status_cb("Counting courses...")
num_courses = CourseOverview.objects.count()
+ # Get the list of collections
+ status_cb("Counting collections...")
+ num_collections = authoring_api.get_collections().count()
+
# Some counters so we can track our progress as indexing progresses:
- num_contexts = num_courses + num_libraries
+ num_contexts = num_courses + num_libraries + num_collections
num_contexts_done = 0 # How many courses/libraries we've indexed
num_blocks_done = 0 # How many individual components/XBlocks we've indexed
- status_cb(f"Found {num_courses} courses and {num_libraries} libraries.")
+ status_cb(f"Found {num_courses} courses, {num_libraries} libraries and {num_collections} collections.")
with _using_temp_index(status_cb) as temp_index_name:
############## Configure the index ##############
@@ -323,6 +329,7 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
Fields.type,
Fields.access_id,
Fields.last_published,
+ Fields.content + "." + Fields.problem_types,
])
# Mark which attributes are used for keyword search, in order of importance:
client.index(temp_index_name).update_searchable_attributes([
@@ -331,6 +338,7 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
Fields.block_id,
Fields.content,
Fields.tags,
+ Fields.description,
# If we don't list the following sub-fields _explicitly_, they're only sometimes searchable - that is, they
# are searchable only if at least one document in the index has a value. If we didn't list them here and,
# say, there were no tags.level3 tags in the index, the client would get an error if trying to search for
@@ -362,8 +370,8 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
############## Libraries ##############
status_cb("Indexing libraries...")
- for lib_key in lib_keys:
- status_cb(f"{num_contexts_done + 1}/{num_contexts}. Now indexing library {lib_key}")
+
+ def index_library(lib_key: str) -> list:
docs = []
for component in lib_api.get_library_components(lib_key):
try:
@@ -374,48 +382,88 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
docs.append(doc)
except Exception as err: # pylint: disable=broad-except
status_cb(f"Error indexing library component {component}: {err}")
- finally:
- num_blocks_done += 1
if docs:
try:
# Add all the docs in this library at once (usually faster than adding one at a time):
_wait_for_meili_task(client.index(temp_index_name).add_documents(docs))
except (TypeError, KeyError, MeilisearchError) as err:
status_cb(f"Error indexing library {lib_key}: {err}")
+ return docs
+ for lib_key in lib_keys:
+ status_cb(f"{num_contexts_done + 1}/{num_contexts}. Now indexing library {lib_key}")
+ lib_docs = index_library(lib_key)
+ num_blocks_done += len(lib_docs)
num_contexts_done += 1
############## Courses ##############
status_cb("Indexing courses...")
# To reduce memory usage on large instances, split up the CourseOverviews into pages of 1,000 courses:
+
+ def index_course(course: CourseOverview) -> list:
+ docs = []
+ # Pre-fetch the course with all of its children:
+ course = store.get_course(course.id, depth=None)
+
+ def add_with_children(block):
+ """ Recursively index the given XBlock/component """
+ doc = searchable_doc_for_course_block(block)
+ doc.update(searchable_doc_tags(block.usage_key))
+ docs.append(doc) # pylint: disable=cell-var-from-loop
+ _recurse_children(block, add_with_children) # pylint: disable=cell-var-from-loop
+
+ # Index course children
+ _recurse_children(course, add_with_children)
+
+ if docs:
+ # Add all the docs in this course at once (usually faster than adding one at a time):
+ _wait_for_meili_task(client.index(temp_index_name).add_documents(docs))
+ return docs
+
paginator = Paginator(CourseOverview.objects.only('id', 'display_name'), 1000)
for p in paginator.page_range:
for course in paginator.page(p).object_list:
status_cb(
f"{num_contexts_done + 1}/{num_contexts}. Now indexing course {course.display_name} ({course.id})"
)
- docs = []
-
- # Pre-fetch the course with all of its children:
- course = store.get_course(course.id, depth=None)
+ course_docs = index_course(course)
+ num_contexts_done += 1
+ num_blocks_done += len(course_docs)
- def add_with_children(block):
- """ Recursively index the given XBlock/component """
- doc = searchable_doc_for_course_block(block)
- doc.update(searchable_doc_tags(block.usage_key))
- docs.append(doc) # pylint: disable=cell-var-from-loop
- _recurse_children(block, add_with_children) # pylint: disable=cell-var-from-loop
+ ############## Collections ##############
+ status_cb("Indexing collections...")
- # Index course children
- _recurse_children(course, add_with_children)
+ def index_collection_batch(batch, num_contexts_done) -> int:
+ docs = []
+ for collection in batch:
+ status_cb(
+ f"{num_contexts_done + 1}/{num_contexts}. "
+ f"Now indexing collection {collection.title} ({collection.id})"
+ )
+ try:
+ doc = searchable_doc_for_collection(collection)
+ # Uncomment below line once collections are tagged.
+ # doc.update(searchable_doc_tags(collection.id))
+ docs.append(doc)
+ except Exception as err: # pylint: disable=broad-except
+ status_cb(f"Error indexing collection {collection}: {err}")
+ finally:
+ num_contexts_done += 1
- if docs:
- # Add all the docs in this course at once (usually faster than adding one at a time):
+ if docs:
+ try:
+ # Add docs in batch of 100 at once (usually faster than adding one at a time):
_wait_for_meili_task(client.index(temp_index_name).add_documents(docs))
- num_contexts_done += 1
- num_blocks_done += len(docs)
+ except (TypeError, KeyError, MeilisearchError) as err:
+ status_cb(f"Error indexing collection batch {p}: {err}")
+ return num_contexts_done
+
+ # To reduce memory usage on large instances, split up the Collections into pages of 100 collections:
+ paginator = Paginator(authoring_api.get_collections(enabled=True), 100)
+ for p in paginator.page_range:
+ num_contexts_done = index_collection_batch(paginator.page(p).object_list, num_contexts_done)
- status_cb(f"Done! {num_blocks_done} blocks indexed across {num_contexts_done} courses and libraries.")
+ status_cb(f"Done! {num_blocks_done} blocks indexed across {num_contexts_done} courses, collections and libraries.")
def upsert_xblock_index_doc(usage_key: UsageKey, recursive: bool = True) -> None:
diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py
index 032023f97c60..a45b37ab2ad2 100644
--- a/openedx/core/djangoapps/content/search/documents.py
+++ b/openedx/core/djangoapps/content/search/documents.py
@@ -13,6 +13,7 @@
from openedx.core.djangoapps.content_libraries import api as lib_api
from openedx.core.djangoapps.content_tagging import api as tagging_api
from openedx.core.djangoapps.xblock import api as xblock_api
+from openedx_learning.api.authoring_models import LearningPackage
log = logging.getLogger(__name__)
@@ -27,10 +28,12 @@ class Fields:
type = "type" # DocType.course_block or DocType.library_block (see below)
block_id = "block_id" # The block_id part of the usage key. Sometimes human-readable, sometimes a random hex ID
display_name = "display_name"
+ description = "description"
modified = "modified"
created = "created"
last_published = "last_published"
block_type = "block_type"
+ problem_types = "problem_types"
context_key = "context_key"
org = "org"
access_id = "access_id" # .models.SearchAccess.id
@@ -65,6 +68,7 @@ class DocType:
"""
course_block = "course_block"
library_block = "library_block"
+ collection = "collection"
def meili_id_from_opaque_key(usage_key: UsageKey) -> str:
@@ -275,3 +279,38 @@ def searchable_doc_for_course_block(block) -> dict:
doc.update(_fields_from_block(block))
return doc
+
+
+def searchable_doc_for_collection(collection) -> dict:
+ """
+ Generate a dictionary document suitable for ingestion into a search engine
+ like Meilisearch or Elasticsearch, so that the given collection can be
+ found using faceted search.
+ """
+ doc = {
+ Fields.id: collection.id,
+ Fields.type: DocType.collection,
+ Fields.display_name: collection.title,
+ Fields.description: collection.description,
+ Fields.created: collection.created.timestamp(),
+ Fields.modified: collection.modified.timestamp(),
+ # Add related learning_package.key as context_key by default.
+ # If related contentlibrary is found, it will override this value below.
+ # Mostly contentlibrary.library_key == learning_package.key
+ Fields.context_key: collection.learning_package.key,
+ }
+ # Just in case learning_package is not related to a library
+ try:
+ context_key = collection.learning_package.contentlibrary.library_key
+ org = str(context_key.org)
+ doc.update({
+ Fields.context_key: str(context_key),
+ Fields.org: org,
+ })
+ except LearningPackage.contentlibrary.RelatedObjectDoesNotExist:
+ log.warning(f"Related library not found for {collection}")
+ doc[Fields.access_id] = _meili_access_id_from_context_key(doc[Fields.context_key])
+ # Add the breadcrumbs.
+ doc[Fields.breadcrumbs] = [{"display_name": collection.learning_package.title}]
+
+ return doc
diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py
index e8616cee60a8..549817700370 100644
--- a/openedx/core/djangoapps/content/search/tests/test_api.py
+++ b/openedx/core/djangoapps/content/search/tests/test_api.py
@@ -6,12 +6,13 @@
import copy
from datetime import datetime, timezone
-from unittest.mock import MagicMock, call, patch
+from unittest.mock import MagicMock, Mock, call, patch
from opaque_keys.edx.keys import UsageKey
import ddt
from django.test import override_settings
from freezegun import freeze_time
+from openedx_learning.api import authoring as authoring_api
from organizations.tests.factories import OrganizationFactory
from common.djangoapps.student.tests.factories import UserFactory
@@ -174,6 +175,28 @@ def setUp(self):
tagging_api.add_tag_to_taxonomy(self.taxonomyB, "three")
tagging_api.add_tag_to_taxonomy(self.taxonomyB, "four")
+ # Create a collection:
+ self.learning_package = authoring_api.get_learning_package_by_key(self.library.key)
+ self.collection_dict = {
+ 'id': 1,
+ 'type': 'collection',
+ 'display_name': 'my_collection',
+ 'description': 'my collection description',
+ 'context_key': 'lib:org1:lib',
+ 'org': 'org1',
+ 'created': created_date.timestamp(),
+ 'modified': created_date.timestamp(),
+ "access_id": lib_access.id,
+ 'breadcrumbs': [{'display_name': 'Library'}]
+ }
+ with freeze_time(created_date):
+ self.collection = authoring_api.create_collection(
+ learning_package_id=self.learning_package.id,
+ title="my_collection",
+ created_by=None,
+ description="my collection description"
+ )
+
@override_settings(MEILISEARCH_ENABLED=False)
def test_reindex_meilisearch_disabled(self, mock_meilisearch):
with self.assertRaises(RuntimeError):
@@ -199,10 +222,27 @@ def test_reindex_meilisearch(self, mock_meilisearch):
[
call([doc_sequential, doc_vertical]),
call([doc_problem1, doc_problem2]),
+ call([self.collection_dict]),
],
any_order=True,
)
+ @override_settings(MEILISEARCH_ENABLED=True)
+ @patch(
+ "openedx.core.djangoapps.content.search.api.searchable_doc_for_collection",
+ Mock(side_effect=Exception("Failed to generate document")),
+ )
+ def test_reindex_meilisearch_collection_error(self, mock_meilisearch):
+
+ mock_logger = Mock()
+ api.rebuild_index(mock_logger)
+ assert call(
+ [self.collection_dict]
+ ) not in mock_meilisearch.return_value.index.return_value.add_documents.mock_calls
+ mock_logger.assert_any_call(
+ f"Error indexing collection {self.collection}: Failed to generate document"
+ )
+
@override_settings(MEILISEARCH_ENABLED=True)
def test_reindex_meilisearch_library_block_error(self, mock_meilisearch):
diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py
index e853fd425273..6140411705bb 100644
--- a/openedx/core/djangoapps/content/search/tests/test_documents.py
+++ b/openedx/core/djangoapps/content/search/tests/test_documents.py
@@ -1,8 +1,12 @@
"""
Tests for the Studio content search documents (what gets stored in the index)
"""
+from datetime import datetime, timezone
from organizations.models import Organization
+from freezegun import freeze_time
+from openedx_learning.api import authoring as authoring_api
+
from openedx.core.djangoapps.content_tagging import api as tagging_api
from openedx.core.djangolib.testing.utils import skip_unless_cms
from xmodule.modulestore.django import modulestore
@@ -11,10 +15,12 @@
try:
# This import errors in the lms because content.search is not an installed app there.
- from ..documents import searchable_doc_for_course_block, searchable_doc_tags
+ from ..documents import searchable_doc_for_course_block, searchable_doc_tags, searchable_doc_for_collection
from ..models import SearchAccess
except RuntimeError:
searchable_doc_for_course_block = lambda x: x
+ searchable_doc_tags = lambda x: x
+ searchable_doc_for_collection = lambda x: x
SearchAccess = {}
@@ -198,3 +204,30 @@ def test_video_block_untagged(self):
"content": {},
# This video has no tags.
}
+
+ def test_collection_with_no_library(self):
+ created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc)
+ with freeze_time(created_date):
+ learning_package = authoring_api.create_learning_package(
+ key="course-v1:edX+toy+2012_Fall",
+ title="some learning_package",
+ description="some description",
+ )
+ collection = authoring_api.create_collection(
+ learning_package_id=learning_package.id,
+ title="my_collection",
+ created_by=None,
+ description="my collection description"
+ )
+ doc = searchable_doc_for_collection(collection)
+ assert doc == {
+ "id": collection.id,
+ "type": "collection",
+ "display_name": collection.title,
+ "description": collection.description,
+ "context_key": learning_package.key,
+ "access_id": self.toy_course_access_id,
+ "breadcrumbs": [{"display_name": learning_package.title}],
+ "created": created_date.timestamp(),
+ "modified": created_date.timestamp(),
+ }
diff --git a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/test_views.py
index 4f6a9b442daf..b9321c136383 100644
--- a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/test_views.py
+++ b/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/test_views.py
@@ -6,6 +6,7 @@
import ddt
from openedx_learning.api.authoring_models import Collection
+from openedx_learning.api.authoring import create_collection
from opaque_keys.edx.locator import LibraryLocatorV2
from openedx.core.djangolib.testing.utils import skip_unless_cms
@@ -36,23 +37,23 @@ def setUp(self):
self.lib2 = ContentLibrary.objects.get(slug="test-lib-col-2")
# Create Content Library Collections
- self.col1 = Collection.objects.create(
+ self.col1 = create_collection(
learning_package_id=self.lib1.learning_package.id,
title="Collection 1",
+ created_by=self.user.id,
description="Description for Collection 1",
- created_by=self.user,
)
- self.col2 = Collection.objects.create(
+ self.col2 = create_collection(
learning_package_id=self.lib1.learning_package.id,
title="Collection 2",
+ created_by=self.user.id,
description="Description for Collection 2",
- created_by=self.user,
)
- self.col3 = Collection.objects.create(
+ self.col3 = create_collection(
learning_package_id=self.lib2.learning_package.id,
title="Collection 3",
+ created_by=self.user.id,
description="Description for Collection 3",
- created_by=self.user,
)
# Create some library blocks
@@ -111,6 +112,12 @@ def test_get_invalid_library_collection(self):
assert resp.status_code == 404
+ # Fetch collection with invalid library_key provided, it should fail
+ resp = self.client.get(
+ URL_LIB_COLLECTION.format(lib_key=123, collection_id=123)
+ )
+ assert resp.status_code == 404
+
def test_list_library_collections(self):
"""
Test listing Content Library Collections
@@ -142,6 +149,15 @@ def test_list_invalid_library_collections(self):
assert resp.status_code == 404
+ non_existing_key = LibraryLocatorV2.from_string("lib:DoesNotExist:NE1")
+ resp = self.client.get(URL_LIB_COLLECTIONS.format(lib_key=non_existing_key))
+
+ assert resp.status_code == 404
+
+ # List collections with invalid library_key provided, it should fail
+ resp = resp = self.client.get(URL_LIB_COLLECTIONS.format(lib_key=123))
+ assert resp.status_code == 404
+
def test_create_library_collection(self):
"""
Test creating a Content Library Collection
@@ -198,6 +214,14 @@ def test_create_invalid_library_collection(self):
assert resp.status_code == 400
+ # Create collection with invalid library_key provided, it should fail
+ resp = self.client.post(
+ URL_LIB_COLLECTIONS.format(lib_key=123),
+ {**post_data_missing_title, **post_data_missing_desc},
+ format="json"
+ )
+ assert resp.status_code == 404
+
def test_update_library_collection(self):
"""
Test updating a Content Library Collection
@@ -260,6 +284,14 @@ def test_update_invalid_library_collection(self):
assert resp.status_code == 404
+ # Update collection with invalid library_key provided, it should fail
+ resp = self.client.patch(
+ URL_LIB_COLLECTION.format(lib_key=123, collection_id=self.col3.id),
+ patch_data,
+ format="json"
+ )
+ assert resp.status_code == 404
+
def test_delete_library_collection(self):
"""
Test deleting a Content Library Collection
diff --git a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py b/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py
index cd4de80593f7..df2e2340155f 100644
--- a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py
+++ b/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py
@@ -14,7 +14,6 @@
from openedx_learning.api.authoring_models import Collection
from openedx_learning.api import authoring as authoring_api
from opaque_keys.edx.locator import LibraryLocatorV2
-
from openedx_events.content_authoring.data import LibraryCollectionData
from openedx_events.content_authoring.signals import (
LIBRARY_COLLECTION_CREATED,
@@ -27,7 +26,7 @@
ContentLibraryCollectionComponentsUpdateSerializer,
ContentLibraryCollectionCreateOrUpdateSerializer,
)
-from openedx.core.djangoapps.content_libraries.views import convert_exceptions
+from openedx.core.djangoapps.content_libraries.utils import convert_exceptions
class LibraryCollectionsView(ModelViewSet):
@@ -44,11 +43,7 @@ def _verify_and_fetch_library_collection(self, library_key, collection_id, user,
"""
Verify that the collection belongs to the library and the user has the correct permissions
"""
- try:
- library_obj = api.require_permission_for_library_key(library_key, user, permission)
- except api.ContentLibraryNotFound as exc:
- raise Http404 from exc
-
+ library_obj = api.require_permission_for_library_key(library_key, user, permission)
collection = None
if library_obj.learning_package_id:
collection = authoring_api.get_collections(
@@ -56,6 +51,7 @@ def _verify_and_fetch_library_collection(self, library_key, collection_id, user,
).filter(id=collection_id).first()
return collection
+ @convert_exceptions
def retrieve(self, request, *args, **kwargs):
"""
Retrieve the Content Library Collection
@@ -79,6 +75,7 @@ def retrieve(self, request, *args, **kwargs):
serializer = self.get_serializer(collection)
return Response(serializer.data)
+ @convert_exceptions
def list(self, request, *args, **kwargs):
"""
List Collections that belong to Content Library
@@ -90,17 +87,15 @@ def list(self, request, *args, **kwargs):
# Check if user has permissions to view collections by checking if user
# has permission to view the Content Library they belong to
library_key = LibraryLocatorV2.from_string(lib_key_str)
- try:
- content_library = api.require_permission_for_library_key(
- library_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY
- )
- except api.ContentLibraryNotFound as exc:
- raise Http404 from exc
+ content_library = api.require_permission_for_library_key(
+ library_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY
+ )
collections = authoring_api.get_collections(content_library.learning_package.id)
serializer = self.get_serializer(collections, many=True)
return Response(serializer.data)
+ @convert_exceptions
def create(self, request, *args, **kwargs):
"""
Create a Collection that belongs to a Content Library
@@ -112,12 +107,9 @@ def create(self, request, *args, **kwargs):
# Check if user has permissions to create a collection in the Content Library
# by checking if user has permission to edit the Content Library
library_key = LibraryLocatorV2.from_string(lib_key_str)
- try:
- content_library = api.require_permission_for_library_key(
- library_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY
- )
- except api.ContentLibraryNotFound as exc:
- raise Http404 from exc
+ content_library = api.require_permission_for_library_key(
+ library_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY
+ )
create_serializer = ContentLibraryCollectionCreateOrUpdateSerializer(data=request.data)
create_serializer.is_valid(raise_exception=True)
@@ -139,6 +131,7 @@ def create(self, request, *args, **kwargs):
return Response(serializer.data)
+ @convert_exceptions
def partial_update(self, request, *args, **kwargs):
"""
Update a Collection that belongs to a Content Library
@@ -175,6 +168,7 @@ def partial_update(self, request, *args, **kwargs):
return Response(serializer.data)
+ @convert_exceptions
def destroy(self, request, *args, **kwargs):
"""
Deletes a Collection that belongs to a Content Library
diff --git a/openedx/core/djangoapps/content_libraries/utils.py b/openedx/core/djangoapps/content_libraries/utils.py
new file mode 100644
index 000000000000..da21d9057c92
--- /dev/null
+++ b/openedx/core/djangoapps/content_libraries/utils.py
@@ -0,0 +1,48 @@
+""" Utils used for the content libraries. """
+
+from functools import wraps
+import logging
+
+from rest_framework.exceptions import NotFound, ValidationError
+
+from opaque_keys import InvalidKeyError
+
+from openedx.core.djangoapps.content_libraries import api
+
+
+log = logging.getLogger(__name__)
+
+
+def convert_exceptions(fn):
+ """
+ Catch any Content Library API exceptions that occur and convert them to
+ DRF exceptions so DRF will return an appropriate HTTP response
+ """
+
+ @wraps(fn)
+ def wrapped_fn(*args, **kwargs):
+ try:
+ return fn(*args, **kwargs)
+ except InvalidKeyError as exc:
+ log.exception(str(exc))
+ raise NotFound # lint-amnesty, pylint: disable=raise-missing-from
+ except api.ContentLibraryNotFound:
+ log.exception("Content library not found")
+ raise NotFound # lint-amnesty, pylint: disable=raise-missing-from
+ except api.ContentLibraryBlockNotFound:
+ log.exception("XBlock not found in content library")
+ raise NotFound # lint-amnesty, pylint: disable=raise-missing-from
+ except api.ContentLibraryCollectionNotFound:
+ log.exception("Collection not found in content library")
+ raise NotFound # lint-amnesty, pylint: disable=raise-missing-from
+
+ except api.LibraryBlockAlreadyExists as exc:
+ log.exception(str(exc))
+ raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from
+ except api.InvalidNameError as exc:
+ log.exception(str(exc))
+ raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from
+ except api.BlockLimitReachedError as exc:
+ log.exception(str(exc))
+ raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from
+ return wrapped_fn
diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py
index 820c30661c8b..11e5c9d23b2e 100644
--- a/openedx/core/djangoapps/content_libraries/views.py
+++ b/openedx/core/djangoapps/content_libraries/views.py
@@ -63,7 +63,6 @@
https://github.com/openedx/edx-platform/pull/30456
"""
-from functools import wraps
import itertools
import json
import logging
@@ -120,44 +119,13 @@
from openedx.core.djangoapps.xblock import api as xblock_api
from .models import ContentLibrary, LtiGradedResource, LtiProfile
+from .utils import convert_exceptions
User = get_user_model()
log = logging.getLogger(__name__)
-def convert_exceptions(fn):
- """
- Catch any Content Library API exceptions that occur and convert them to
- DRF exceptions so DRF will return an appropriate HTTP response
- """
-
- @wraps(fn)
- def wrapped_fn(*args, **kwargs):
- try:
- return fn(*args, **kwargs)
- except api.ContentLibraryNotFound:
- log.exception("Content library not found")
- raise NotFound # lint-amnesty, pylint: disable=raise-missing-from
- except api.ContentLibraryBlockNotFound:
- log.exception("XBlock not found in content library")
- raise NotFound # lint-amnesty, pylint: disable=raise-missing-from
- except api.ContentLibraryCollectionNotFound:
- log.exception("Collection not found in content library")
- raise NotFound # lint-amnesty, pylint: disable=raise-missing-from
-
- except api.LibraryBlockAlreadyExists as exc:
- log.exception(str(exc))
- raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from
- except api.InvalidNameError as exc:
- log.exception(str(exc))
- raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from
- except api.BlockLimitReachedError as exc:
- log.exception(str(exc))
- raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from
- return wrapped_fn
-
-
class LibraryApiPaginationDocs:
"""
API docs for query params related to paginating ContentLibraryMetadata objects.
diff --git a/openedx/core/djangoapps/notifications/email/utils.py b/openedx/core/djangoapps/notifications/email/utils.py
index da288750bb7d..e36c435e8ac0 100644
--- a/openedx/core/djangoapps/notifications/email/utils.py
+++ b/openedx/core/djangoapps/notifications/email/utils.py
@@ -19,6 +19,7 @@
)
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_EMAIL_NOTIFICATIONS
from openedx.core.djangoapps.notifications.email_notifications import EmailCadence
+from openedx.core.djangoapps.notifications.events import notification_preference_unsubscribe_event
from openedx.core.djangoapps.notifications.models import (
CourseNotificationPreference,
get_course_notification_preference_config_version
@@ -395,3 +396,4 @@ def get_updated_preference(pref):
if pref_value else EmailCadence.NEVER
type_prefs['email_cadence'] = cadence_value
preference.save()
+ notification_preference_unsubscribe_event(user)
diff --git a/openedx/core/djangoapps/notifications/events.py b/openedx/core/djangoapps/notifications/events.py
index fb50e134941b..91b12075a8a1 100644
--- a/openedx/core/djangoapps/notifications/events.py
+++ b/openedx/core/djangoapps/notifications/events.py
@@ -10,6 +10,7 @@
NOTIFICATION_APP_ALL_READ = 'edx.notifications.app_all_read'
NOTIFICATION_PREFERENCES_UPDATED = 'edx.notifications.preferences.updated'
NOTIFICATION_TRAY_OPENED = 'edx.notifications.tray_opened'
+NOTIFICATION_PREFERENCE_UNSUBSCRIBE = 'edx.notifications.preferences.one_click_unsubscribe'
def get_user_forums_roles(user, course_id):
@@ -155,3 +156,17 @@ def notification_tray_opened_event(user, unseen_notifications_count):
'unseen_notifications_count': unseen_notifications_count,
}
)
+
+
+def notification_preference_unsubscribe_event(user):
+ """
+ Emits an event when user clicks on one-click-unsubscribe url
+ """
+ event_data = {
+ 'user_id': user.id,
+ 'username': user.username,
+ 'event_type': 'email_digest_unsubscribe'
+ }
+ with tracker.get_tracker().context(NOTIFICATION_PREFERENCE_UNSUBSCRIBE, event_data):
+ tracker.emit(NOTIFICATION_PREFERENCE_UNSUBSCRIBE, event_data)
+ segment.track(user.id, NOTIFICATION_PREFERENCE_UNSUBSCRIBE, event_data)
diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py
index 720b3ba96af7..103c5bf24f6c 100644
--- a/openedx/core/djangoapps/user_api/accounts/views.py
+++ b/openedx/core/djangoapps/user_api/accounts/views.py
@@ -21,7 +21,6 @@
from edx_ace import ace
from edx_ace.recipient import Recipient
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
-from openedx.core.lib.api.authentication import BearerAuthentication
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
from enterprise.models import EnterpriseCourseEnrollment, EnterpriseCustomerUser, PendingEnterpriseCustomerUser
from integrated_channels.degreed.models import DegreedLearnerDataTransmissionAudit
@@ -50,9 +49,10 @@
get_retired_email_by_email,
get_retired_username_by_username,
is_email_retired,
- is_username_retired
+ is_username_retired,
)
from common.djangoapps.student.models_api import confirm_name_change, do_name_change_request, get_pending_name_change
+from lms.djangoapps.certificates.api import clear_pii_from_certificate_records_for_user
from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
from openedx.core.djangoapps.api_admin.models import ApiAccessRequest
from openedx.core.djangoapps.course_groups.models import UnregisteredLearnerCohortAssignments
@@ -64,9 +64,8 @@
from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_names, set_has_profile_image
from openedx.core.djangoapps.user_api.accounts.utils import handle_retirement_cancellation
from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError
-from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
+from openedx.core.lib.api.authentication import BearerAuthentication, BearerAuthenticationAllowInactiveUser
from openedx.core.lib.api.parsers import MergePatchParser
-from lms.djangoapps.certificates.api import clear_pii_from_certificate_records_for_user
from ..errors import AccountUpdateError, AccountValidationError, UserNotAuthorized, UserNotFound
from ..message_types import DeletionNotificationMessage
@@ -75,7 +74,7 @@
RetirementStateError,
UserOrgTag,
UserRetirementPartnerReportingStatus,
- UserRetirementStatus
+ UserRetirementStatus,
)
from .api import get_account_settings, update_account_settings
from .permissions import (
@@ -83,13 +82,13 @@
CanDeactivateUser,
CanGetAccountInfo,
CanReplaceUsername,
- CanRetireUser
+ CanRetireUser,
)
from .serializers import (
PendingNameChangeSerializer,
UserRetirementPartnerReportSerializer,
UserRetirementStatusSerializer,
- UserSearchEmailSerializer
+ UserSearchEmailSerializer,
)
from .signals import USER_RETIRE_LMS_CRITICAL, USER_RETIRE_LMS_MISC, USER_RETIRE_MAILINGS
from .utils import create_retirement_request_and_deactivate_account, username_suffix_generator
@@ -97,16 +96,16 @@
log = logging.getLogger(__name__)
USER_PROFILE_PII = {
- 'name': '',
- 'meta': '',
- 'location': '',
- 'year_of_birth': None,
- 'gender': None,
- 'mailing_address': None,
- 'city': None,
- 'country': None,
- 'bio': None,
- 'phone_number': None,
+ "name": "",
+ "meta": "",
+ "location": "",
+ "year_of_birth": None,
+ "gender": None,
+ "mailing_address": None,
+ "city": None,
+ "country": None,
+ "bio": None,
+ "phone_number": None,
}
@@ -118,12 +117,9 @@ def request_requires_username(function):
@wraps(function)
def wrapper(self, request): # pylint: disable=missing-docstring
- username = request.data.get('username', None)
+ username = request.data.get("username", None)
if not username:
- return Response(
- status=status.HTTP_404_NOT_FOUND,
- data={'message': 'The user was not specified.'}
- )
+ return Response(status=status.HTTP_404_NOT_FOUND, data={"message": "The user was not specified."})
return function(self, request)
return wrapper
@@ -131,177 +127,183 @@ def wrapper(self, request): # pylint: disable=missing-docstring
class AccountViewSet(ViewSet):
"""
- **Use Cases**
-
- Get or update a user's account information. Updates are supported
- only through merge patch.
-
- **Example Requests**
-
- GET /api/user/v1/me[?view=shared]
- GET /api/user/v1/accounts?usernames={username1,username2}[?view=shared]
- GET /api/user/v1/accounts?email={user_email}
- GET /api/user/v1/accounts/{username}/[?view=shared]
-
- PATCH /api/user/v1/accounts/{username}/{"key":"value"} "application/merge-patch+json"
-
- POST /api/user/v1/accounts/search_emails "application/json"
-
- **Notes for PATCH requests to /accounts endpoints**
- * Requested updates to social_links are automatically merged with
- previously set links. That is, any newly introduced platforms are
- add to the previous list. Updated links to pre-existing platforms
- replace their values in the previous list. Pre-existing platforms
- can be removed by setting the value of the social_link to an
- empty string ("").
-
- **Response Values for GET requests to the /me endpoint**
- If the user is not logged in, an HTTP 401 "Not Authorized" response
- is returned.
-
- Otherwise, an HTTP 200 "OK" response is returned. The response
- contains the following value:
-
- * username: The username associated with the account.
-
- **Response Values for GET requests to /accounts endpoints**
-
- If no user exists with the specified username, or email, an HTTP 404 "Not
- Found" response is returned.
-
- If the user makes the request for her own account, or makes a
- request for another account and has "is_staff" access, an HTTP 200
- "OK" response is returned. The response contains the following
- values.
-
- * id: numerical lms user id in db
- * activation_key: auto-genrated activation key when signed up via email
- * bio: null or textual representation of user biographical
- information ("about me").
- * country: An ISO 3166 country code or null.
- * date_joined: The date the account was created, in the string
- format provided by datetime. For example, "2014-08-26T17:52:11Z".
- * last_login: The latest date the user logged in, in the string datetime format.
- * email: Email address for the user. New email addresses must be confirmed
- via a confirmation email, so GET does not reflect the change until
- the address has been confirmed.
- * secondary_email: A secondary email address for the user. Unlike
- the email field, GET will reflect the latest update to this field
- even if changes have yet to be confirmed.
- * verified_name: Approved verified name of the learner present in name affirmation plugin
- * gender: One of the following values:
-
- * null
- * "f"
- * "m"
- * "o"
-
- * goals: The textual representation of the user's goals, or null.
- * is_active: Boolean representation of whether a user is active.
- * language: The user's preferred language, or null.
- * language_proficiencies: Array of language preferences. Each
- preference is a JSON object with the following keys:
-
- * "code": string ISO 639-1 language code e.g. "en".
-
- * level_of_education: One of the following values:
-
- * "p": PhD or Doctorate
- * "m": Master's or professional degree
- * "b": Bachelor's degree
- * "a": Associate's degree
- * "hs": Secondary/high school
- * "jhs": Junior secondary/junior high/middle school
- * "el": Elementary/primary school
- * "none": None
- * "o": Other
- * null: The user did not enter a value
-
- * mailing_address: The textual representation of the user's mailing
- address, or null.
- * name: The full name of the user.
- * profile_image: A JSON representation of a user's profile image
- information. This representation has the following keys.
-
- * "has_image": Boolean indicating whether the user has a profile
- image.
- * "image_url_*": Absolute URL to various sizes of a user's
- profile image, where '*' matches a representation of the
- corresponding image size, such as 'small', 'medium', 'large',
- and 'full'. These are configurable via PROFILE_IMAGE_SIZES_MAP.
-
- * requires_parental_consent: True if the user is a minor
- requiring parental consent.
- * social_links: Array of social links, sorted alphabetically by
- "platform". Each preference is a JSON object with the following keys:
-
- * "platform": A particular social platform, ex: 'facebook'
- * "social_link": The link to the user's profile on the particular platform
-
- * username: The username associated with the account.
- * year_of_birth: The year the user was born, as an integer, or null.
-
- * account_privacy: The user's setting for sharing her personal
- profile. Possible values are "all_users", "private", or "custom".
- If "custom", the user has selectively chosen a subset of shareable
- fields to make visible to others via the User Preferences API.
-
- * phone_number: The phone number for the user. String of numbers with
- an optional `+` sign at the start.
-
- * pending_name_change: If the user has an active name change request, returns the
- requested name.
-
- For all text fields, plain text instead of HTML is supported. The
- data is stored exactly as specified. Clients must HTML escape
- rendered values to avoid script injections.
-
- If a user who does not have "is_staff" access requests account
- information for a different user, only a subset of these fields is
- returned. The returned fields depend on the
- ACCOUNT_VISIBILITY_CONFIGURATION configuration setting and the
- visibility preference of the user for whom data is requested.
-
- Note that a user can view which account fields they have shared
- with other users by requesting their own username and providing
- the "view=shared" URL parameter.
-
- **Response Values for PATCH**
-
- Users can only modify their own account information. If the
- requesting user does not have the specified username and has staff
- access, the request returns an HTTP 403 "Forbidden" response. If
- the requesting user does not have staff access, the request
- returns an HTTP 404 "Not Found" response to avoid revealing the
- existence of the account.
-
- If no user exists with the specified username, an HTTP 404 "Not
- Found" response is returned.
-
- If "application/merge-patch+json" is not the specified content
- type, a 415 "Unsupported Media Type" response is returned.
-
- If validation errors prevent the update, this method returns a 400
- "Bad Request" response that includes a "field_errors" field that
- lists all error messages.
-
- If a failure at the time of the update prevents the update, a 400
- "Bad Request" error is returned. The JSON collection contains
- specific errors.
-
- If the update is successful, updated user account data is returned.
+ **Use Cases**
+
+ Get or update a user's account information. Updates are supported
+ only through merge patch.
+
+ **Example Requests**
+
+ GET /api/user/v1/me[?view=shared]
+ GET /api/user/v1/accounts?usernames={username1,username2}[?view=shared]
+ GET /api/user/v1/accounts?email={user_email}
+ GET /api/user/v1/accounts/{username}/[?view=shared]
+
+ PATCH /api/user/v1/accounts/{username}/{"key":"value"} "application/merge-patch+json"
+
+ POST /api/user/v1/accounts/search_emails "application/json"
+
+ **Notes for PATCH requests to /accounts endpoints**
+ * Requested updates to social_links are automatically merged with
+ previously set links. That is, any newly introduced platforms are
+ add to the previous list. Updated links to pre-existing platforms
+ replace their values in the previous list. Pre-existing platforms
+ can be removed by setting the value of the social_link to an
+ empty string ("").
+
+ **Response Values for GET requests to the /me endpoint**
+ If the user is not logged in, an HTTP 401 "Not Authorized" response
+ is returned.
+
+ Otherwise, an HTTP 200 "OK" response is returned. The response
+ contains the following value:
+
+ * username: The username associated with the account.
+
+ **Response Values for GET requests to /accounts endpoints**
+
+ If no user exists with the specified username, or email, an HTTP 404 "Not
+ Found" response is returned.
+
+ If the user makes the request for her own account, or makes a
+ request for another account and has "is_staff" access, an HTTP 200
+ "OK" response is returned. The response contains the following
+ values.
+
+ * id: numerical lms user id in db
+ * activation_key: auto-genrated activation key when signed up via email
+ * bio: null or textual representation of user biographical
+ information ("about me").
+ * country: An ISO 3166 country code or null.
+ * date_joined: The date the account was created, in the string
+ format provided by datetime. For example, "2014-08-26T17:52:11Z".
+ * last_login: The latest date the user logged in, in the string datetime format.
+ * email: Email address for the user. New email addresses must be confirmed
+ via a confirmation email, so GET does not reflect the change until
+ the address has been confirmed.
+ * secondary_email: A secondary email address for the user. Unlike
+ the email field, GET will reflect the latest update to this field
+ even if changes have yet to be confirmed.
+ * verified_name: Approved verified name of the learner present in name affirmation plugin
+ * gender: One of the following values:
+
+ * null
+ * "f"
+ * "m"
+ * "o"
+
+ * goals: The textual representation of the user's goals, or null.
+ * is_active: Boolean representation of whether a user is active.
+ * language: The user's preferred language, or null.
+ * language_proficiencies: Array of language preferences. Each
+ preference is a JSON object with the following keys:
+
+ * "code": string ISO 639-1 language code e.g. "en".
+
+ * level_of_education: One of the following values:
+
+ * "p": PhD or Doctorate
+ * "m": Master's or professional degree
+ * "b": Bachelor's degree
+ * "a": Associate's degree
+ * "hs": Secondary/high school
+ * "jhs": Junior secondary/junior high/middle school
+ * "el": Elementary/primary school
+ * "none": None
+ * "o": Other
+ * null: The user did not enter a value
+
+ * mailing_address: The textual representation of the user's mailing
+ address, or null.
+ * name: The full name of the user.
+ * profile_image: A JSON representation of a user's profile image
+ information. This representation has the following keys.
+
+ * "has_image": Boolean indicating whether the user has a profile
+ image.
+ * "image_url_*": Absolute URL to various sizes of a user's
+ profile image, where '*' matches a representation of the
+ corresponding image size, such as 'small', 'medium', 'large',
+ and 'full'. These are configurable via PROFILE_IMAGE_SIZES_MAP.
+
+ * requires_parental_consent: True if the user is a minor
+ requiring parental consent.
+ * social_links: Array of social links, sorted alphabetically by
+ "platform". Each preference is a JSON object with the following keys:
+
+ * "platform": A particular social platform, ex: 'facebook'
+ * "social_link": The link to the user's profile on the particular platform
+
+ * username: The username associated with the account.
+ * year_of_birth: The year the user was born, as an integer, or null.
+
+ * account_privacy: The user's setting for sharing her personal
+ profile. Possible values are "all_users", "private", or "custom".
+ If "custom", the user has selectively chosen a subset of shareable
+ fields to make visible to others via the User Preferences API.
+
+ * phone_number: The phone number for the user. String of numbers with
+ an optional `+` sign at the start.
+
+ * pending_name_change: If the user has an active name change request, returns the
+ requested name.
+
+ For all text fields, plain text instead of HTML is supported. The
+ data is stored exactly as specified. Clients must HTML escape
+ rendered values to avoid script injections.
+
+ If a user who does not have "is_staff" access requests account
+ information for a different user, only a subset of these fields is
+ returned. The returned fields depend on the
+ ACCOUNT_VISIBILITY_CONFIGURATION configuration setting and the
+ visibility preference of the user for whom data is requested.
+
+ Note that a user can view which account fields they have shared
+ with other users by requesting their own username and providing
+ the "view=shared" URL parameter.
+
+ **Response Values for PATCH**
+
+ Users can only modify their own account information. If the
+ requesting user does not have the specified username and has staff
+ access, the request returns an HTTP 403 "Forbidden" response. If
+ the requesting user does not have staff access, the request
+ returns an HTTP 404 "Not Found" response to avoid revealing the
+ existence of the account.
+
+ If no user exists with the specified username, an HTTP 404 "Not
+ Found" response is returned.
+
+ If "application/merge-patch+json" is not the specified content
+ type, a 415 "Unsupported Media Type" response is returned.
+
+ If validation errors prevent the update, this method returns a 400
+ "Bad Request" response that includes a "field_errors" field that
+ lists all error messages.
+
+ If a failure at the time of the update prevents the update, a 400
+ "Bad Request" error is returned. The JSON collection contains
+ specific errors.
+
+ If the update is successful, updated user account data is returned.
"""
+
authentication_classes = (
- JwtAuthentication, BearerAuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser
+ JwtAuthentication,
+ BearerAuthenticationAllowInactiveUser,
+ SessionAuthenticationAllowInactiveUser,
)
permission_classes = (permissions.IsAuthenticated, CanGetAccountInfo)
- parser_classes = (JSONParser, MergePatchParser,)
+ parser_classes = (
+ JSONParser,
+ MergePatchParser,
+ )
def get(self, request):
"""
GET /api/user/v1/me
"""
- return Response({'username': request.user.username})
+ return Response({"username": request.user.username})
def list(self, request):
"""
@@ -309,13 +311,13 @@ def list(self, request):
GET /api/user/v1/accounts?email={user_email} (Staff Only)
GET /api/user/v1/accounts?lms_user_id={lms_user_id} (Staff Only)
"""
- usernames = request.GET.get('username')
- user_email = request.GET.get('email')
- lms_user_id = request.GET.get('lms_user_id')
+ usernames = request.GET.get("username")
+ user_email = request.GET.get("email")
+ lms_user_id = request.GET.get("lms_user_id")
search_usernames = []
if usernames:
- search_usernames = usernames.strip(',').split(',')
+ search_usernames = usernames.strip(",").split(",")
elif user_email:
if is_email_retired(user_email):
can_cancel_retirement = True
@@ -325,22 +327,20 @@ def list(self, request):
retirement_status = UserRetirementStatus.objects.get(
created__gt=earliest_datetime,
created__lt=datetime.datetime.now(pytz.UTC),
- original_email=user_email
+ original_email=user_email,
)
retirement_id = retirement_status.id
except UserRetirementStatus.DoesNotExist:
can_cancel_retirement = False
context = {
- 'error_msg': accounts.RETIRED_EMAIL_MSG,
- 'can_cancel_retirement': can_cancel_retirement,
- 'retirement_id': retirement_id
+ "error_msg": accounts.RETIRED_EMAIL_MSG,
+ "can_cancel_retirement": can_cancel_retirement,
+ "retirement_id": retirement_id,
}
- return Response(
- context, status=status.HTTP_404_NOT_FOUND
- )
- user_email = user_email.strip('')
+ return Response(context, status=status.HTTP_404_NOT_FOUND)
+ user_email = user_email.strip("")
try:
user = User.objects.get(email=user_email)
except (UserNotFound, User.DoesNotExist):
@@ -355,9 +355,7 @@ def list(self, request):
return Response(status=status.HTTP_400_BAD_REQUEST)
search_usernames = [user.username]
try:
- account_settings = get_account_settings(
- request, search_usernames, view=request.query_params.get('view')
- )
+ account_settings = get_account_settings(request, search_usernames, view=request.query_params.get("view"))
except UserNotFound:
return Response(status=status.HTTP_404_NOT_FOUND)
@@ -386,23 +384,15 @@ def search_emails(self, request):
"""
if not request.user.is_staff:
return Response(
- {
- 'developer_message': 'not_found',
- 'user_message': 'Not Found'
- },
- status=status.HTTP_404_NOT_FOUND
+ {"developer_message": "not_found", "user_message": "Not Found"}, status=status.HTTP_404_NOT_FOUND
)
try:
- user_emails = request.data['emails']
+ user_emails = request.data["emails"]
except KeyError as error:
- error_message = f'{error} field is required'
+ error_message = f"{error} field is required"
return Response(
- {
- 'developer_message': error_message,
- 'user_message': error_message
- },
- status=status.HTTP_400_BAD_REQUEST
+ {"developer_message": error_message, "user_message": error_message}, status=status.HTTP_400_BAD_REQUEST
)
users = User.objects.filter(email__in=user_emails)
data = UserSearchEmailSerializer(users, many=True).data
@@ -413,8 +403,7 @@ def retrieve(self, request, username):
GET /api/user/v1/accounts/{username}/
"""
try:
- account_settings = get_account_settings(
- request, [username], view=request.query_params.get('view'))
+ account_settings = get_account_settings(request, [username], view=request.query_params.get("view"))
except UserNotFound:
return Response(status=status.HTTP_404_NOT_FOUND)
@@ -443,11 +432,8 @@ def partial_update(self, request, username):
return Response({"field_errors": err.field_errors}, status=status.HTTP_400_BAD_REQUEST)
except AccountUpdateError as err:
return Response(
- {
- "developer_message": err.developer_message,
- "user_message": err.user_message
- },
- status=status.HTTP_400_BAD_REQUEST
+ {"developer_message": err.developer_message, "user_message": err.user_message},
+ status=status.HTTP_400_BAD_REQUEST,
)
return Response(account_settings)
@@ -457,6 +443,7 @@ class NameChangeView(ViewSet):
"""
Viewset to manage profile name change requests.
"""
+
permission_classes = (permissions.IsAuthenticated,)
def create(self, request):
@@ -472,10 +459,10 @@ def create(self, request):
}
"""
user = request.user
- new_name = request.data.get('name', None)
- rationale = f'Name change requested through account API by {user.username}'
+ new_name = request.data.get("name", None)
+ rationale = f"Name change requested through account API by {user.username}"
- serializer = PendingNameChangeSerializer(data={'new_name': new_name})
+ serializer = PendingNameChangeSerializer(data={"new_name": new_name})
if serializer.is_valid():
pending_name_change = do_name_change_request(user, new_name, rationale)[0]
@@ -483,8 +470,8 @@ def create(self, request):
return Response(status=status.HTTP_201_CREATED)
else:
return Response(
- {'new_name': 'The profile name given was identical to the current name.'},
- status=status.HTTP_400_BAD_REQUEST
+ {"new_name": "The profile name given was identical to the current name."},
+ status=status.HTTP_400_BAD_REQUEST,
)
return Response(status=status.HTTP_400_BAD_REQUEST, data=serializer.errors)
@@ -514,6 +501,7 @@ class AccountDeactivationView(APIView):
Account deactivation viewset. Currently only supports POST requests.
Only admins can deactivate accounts.
"""
+
permission_classes = (permissions.IsAuthenticated, CanDeactivateUser)
def post(self, request, username):
@@ -559,6 +547,7 @@ class DeactivateLogoutView(APIView):
- Log the user out
- Create a row in the retirement table for that user
"""
+
# BearerAuthentication is added here to support account deletion
# from the mobile app until it moves to JWT Auth.
# See mobile roadmap issue https://github.com/openedx/edx-platform/issues/33307.
@@ -575,7 +564,7 @@ def post(self, request):
# Ensure the account deletion is not disable
enable_account_deletion = configuration_helpers.get_value(
- 'ENABLE_ACCOUNT_DELETION', settings.FEATURES.get('ENABLE_ACCOUNT_DELETION', False)
+ "ENABLE_ACCOUNT_DELETION", settings.FEATURES.get("ENABLE_ACCOUNT_DELETION", False)
)
if not enable_account_deletion:
@@ -595,11 +584,9 @@ def post(self, request):
# Send notification email to user
site = Site.objects.get_current()
notification_context = get_base_template_context(site)
- notification_context.update({'full_name': request.user.profile.name})
+ notification_context.update({"full_name": request.user.profile.name})
language_code = request.user.preferences.model.get_value(
- request.user,
- LANGUAGE_KEY,
- default=settings.LANGUAGE_CODE
+ request.user, LANGUAGE_KEY, default=settings.LANGUAGE_CODE
)
notification = DeletionNotificationMessage().personalize(
recipient=Recipient(lms_user_id=0, email_address=user_email),
@@ -608,22 +595,20 @@ def post(self, request):
)
ace.send(notification)
except Exception as exc:
- log.exception('Error sending out deletion notification email')
+ log.exception("Error sending out deletion notification email")
raise exc
# Log the user out.
logout(request)
return Response(status=status.HTTP_204_NO_CONTENT)
except KeyError:
- log.exception(f'Username not specified {request.user}')
- return Response('Username not specified.', status=status.HTTP_404_NOT_FOUND)
+ log.exception(f"Username not specified {request.user}")
+ return Response("Username not specified.", status=status.HTTP_404_NOT_FOUND)
except user_model.DoesNotExist:
log.exception(f'The user "{request.user.username}" does not exist.')
- return Response(
- f'The user "{request.user.username}" does not exist.', status=status.HTTP_404_NOT_FOUND
- )
+ return Response(f'The user "{request.user.username}" does not exist.', status=status.HTTP_404_NOT_FOUND)
except Exception as exc: # pylint: disable=broad-except
- log.exception(f'500 error deactivating account {exc}')
+ log.exception(f"500 error deactivating account {exc}")
return Response(str(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR)
def _verify_user_password(self, request):
@@ -636,7 +621,7 @@ def _verify_user_password(self, request):
"""
try:
self._check_excessive_login_attempts(request.user)
- user = authenticate(username=request.user.username, password=request.POST['password'], request=request)
+ user = authenticate(username=request.user.username, password=request.POST["password"], request=request)
if user:
if LoginFailures.is_feature_enabled():
LoginFailures.clear_lockout_counter(user)
@@ -644,9 +629,7 @@ def _verify_user_password(self, request):
else:
self._handle_failed_authentication(request.user)
except AuthFailedError as err:
- log.exception(
- f"The user password to deactivate was incorrect. {request.user.username}"
- )
+ log.exception(f"The user password to deactivate was incorrect. {request.user.username}")
return Response(str(err), status=status.HTTP_403_FORBIDDEN)
except Exception as err: # pylint: disable=broad-except
return Response(f"Could not verify user password: {err}", status=status.HTTP_400_BAD_REQUEST)
@@ -657,8 +640,9 @@ def _check_excessive_login_attempts(self, user):
"""
if user and LoginFailures.is_feature_enabled():
if LoginFailures.is_user_locked_out(user):
- raise AuthFailedError(_('This account has been temporarily locked due '
- 'to excessive login failures. Try again later.'))
+ raise AuthFailedError(
+ _("This account has been temporarily locked due to excessive login failures. Try again later.")
+ )
def _handle_failed_authentication(self, user):
"""
@@ -667,7 +651,7 @@ def _handle_failed_authentication(self, user):
if user and LoginFailures.is_feature_enabled():
LoginFailures.increment_lockout_counter(user)
- raise AuthFailedError(_('Email or password is incorrect.'))
+ raise AuthFailedError(_("Email or password is incorrect."))
def _set_unusable_password(user):
@@ -684,15 +668,19 @@ class AccountRetirementPartnerReportView(ViewSet):
Provides API endpoints for managing partner reporting of retired
users.
"""
- DELETION_COMPLETED_KEY = 'deletion_completed'
- ORGS_CONFIG_KEY = 'orgs_config'
- ORGS_CONFIG_ORG_KEY = 'org'
- ORGS_CONFIG_FIELD_HEADINGS_KEY = 'field_headings'
- ORIGINAL_EMAIL_KEY = 'original_email'
- ORIGINAL_NAME_KEY = 'original_name'
- STUDENT_ID_KEY = 'student_id'
-
- permission_classes = (permissions.IsAuthenticated, CanRetireUser,)
+
+ DELETION_COMPLETED_KEY = "deletion_completed"
+ ORGS_CONFIG_KEY = "orgs_config"
+ ORGS_CONFIG_ORG_KEY = "org"
+ ORGS_CONFIG_FIELD_HEADINGS_KEY = "field_headings"
+ ORIGINAL_EMAIL_KEY = "original_email"
+ ORIGINAL_NAME_KEY = "original_name"
+ STUDENT_ID_KEY = "student_id"
+
+ permission_classes = (
+ permissions.IsAuthenticated,
+ CanRetireUser,
+ )
parser_classes = (JSONParser,)
serializer_class = UserRetirementStatusSerializer
@@ -706,7 +694,7 @@ def _get_orgs_for_user(user):
org = enrollment.course_id.org
# Org can conceivably be blank or this bogus default value
- if org and org != 'outdated_entry':
+ if org and org != "outdated_entry":
orgs.add(org)
return orgs
@@ -718,9 +706,9 @@ def retirement_partner_report(self, request): # pylint: disable=unused-argument
that are not already being processed and updates their status
to indicate they are currently being processed.
"""
- retirement_statuses = UserRetirementPartnerReportingStatus.objects.filter(
- is_being_processed=False
- ).order_by('id')
+ retirement_statuses = UserRetirementPartnerReportingStatus.objects.filter(is_being_processed=False).order_by(
+ "id"
+ )
retirements = []
for retirement_status in retirement_statuses:
@@ -737,12 +725,12 @@ def _get_retirement_for_partner_report(self, retirement_status):
Get the retirement for this retirement_status. The retirement info will be included in the partner report.
"""
retirement = {
- 'user_id': retirement_status.user.pk,
- 'original_username': retirement_status.original_username,
+ "user_id": retirement_status.user.pk,
+ "original_username": retirement_status.original_username,
AccountRetirementPartnerReportView.ORIGINAL_EMAIL_KEY: retirement_status.original_email,
AccountRetirementPartnerReportView.ORIGINAL_NAME_KEY: retirement_status.original_name,
- 'orgs': self._get_orgs_for_user(retirement_status.user),
- 'created': retirement_status.created,
+ "orgs": self._get_orgs_for_user(retirement_status.user),
+ "created": retirement_status.created,
}
return retirement
@@ -761,7 +749,7 @@ def retirement_partner_status_create(self, request):
Creates a UserRetirementPartnerReportingStatus object for the given user
as part of the retirement pipeline.
"""
- username = request.data['username']
+ username = request.data["username"]
try:
retirement = UserRetirementStatus.get_retirement_for_retirement_action(username)
@@ -771,10 +759,10 @@ def retirement_partner_status_create(self, request):
UserRetirementPartnerReportingStatus.objects.get_or_create(
user=retirement.user,
defaults={
- 'original_username': retirement.original_username,
- 'original_email': retirement.original_email,
- 'original_name': retirement.original_name
- }
+ "original_username": retirement.original_username,
+ "original_email": retirement.original_email,
+ "original_name": retirement.original_name,
+ },
)
return Response(status=status.HTTP_204_NO_CONTENT)
@@ -790,14 +778,13 @@ def retirement_partner_cleanup(self, request):
Deletes UserRetirementPartnerReportingStatus objects for a list of users
that have been reported on.
"""
- usernames = [u['original_username'] for u in request.data]
+ usernames = [u["original_username"] for u in request.data]
if not usernames:
- return Response('No original_usernames given.', status=status.HTTP_400_BAD_REQUEST)
+ return Response("No original_usernames given.", status=status.HTTP_400_BAD_REQUEST)
retirement_statuses = UserRetirementPartnerReportingStatus.objects.filter(
- is_being_processed=True,
- original_username__in=usernames
+ is_being_processed=True, original_username__in=usernames
)
# Need to de-dupe usernames that differ only by case to find the exact right match
@@ -809,15 +796,15 @@ def retirement_partner_cleanup(self, request):
# to disambiguate them in Python, which will respect case in the comparison.
if len(usernames) != len(retirement_statuses_clean):
return Response(
- '{} original_usernames given, {} found!\n'
- 'Given usernames:\n{}\n'
- 'Found UserRetirementReportingStatuses:\n{}'.format(
+ "{} original_usernames given, {} found!\n"
+ "Given usernames:\n{}\n"
+ "Found UserRetirementReportingStatuses:\n{}".format(
len(usernames),
len(retirement_statuses_clean),
usernames,
- ', '.join([rs.original_username for rs in retirement_statuses_clean])
+ ", ".join([rs.original_username for rs in retirement_statuses_clean]),
),
- status=status.HTTP_400_BAD_REQUEST
+ status=status.HTTP_400_BAD_REQUEST,
)
retirement_statuses.delete()
@@ -829,7 +816,11 @@ class CancelAccountRetirementStatusView(ViewSet):
"""
Provides API endpoints for canceling retirement process for a user's account.
"""
- permission_classes = (permissions.IsAuthenticated, CanCancelUserRetirement,)
+
+ permission_classes = (
+ permissions.IsAuthenticated,
+ CanCancelUserRetirement,
+ )
def cancel_retirement(self, request):
"""
@@ -839,26 +830,23 @@ def cancel_retirement(self, request):
This also handles the top level error handling, and permissions.
"""
try:
- retirement_id = request.data['retirement_id']
+ retirement_id = request.data["retirement_id"]
except KeyError:
- return Response(
- status=status.HTTP_400_BAD_REQUEST,
- data={'message': 'retirement_id must be specified.'}
- )
+ return Response(status=status.HTTP_400_BAD_REQUEST, data={"message": "retirement_id must be specified."})
try:
retirement = UserRetirementStatus.objects.get(id=retirement_id)
except UserRetirementStatus.DoesNotExist:
- return Response(data={"message": 'Retirement does not exist!'}, status=status.HTTP_400_BAD_REQUEST)
+ return Response(data={"message": "Retirement does not exist!"}, status=status.HTTP_400_BAD_REQUEST)
- if retirement.current_state.state_name != 'PENDING':
+ if retirement.current_state.state_name != "PENDING":
return Response(
status=status.HTTP_400_BAD_REQUEST,
data={
"message": f"Retirement requests can only be cancelled for users in the PENDING state. Current "
- f"request state for '{retirement.original_username}': "
- f"{retirement.current_state.state_name}"
- }
+ f"request state for '{retirement.original_username}': "
+ f"{retirement.current_state.state_name}"
+ },
)
handle_retirement_cancellation(retirement)
@@ -870,7 +858,11 @@ class AccountRetirementStatusView(ViewSet):
"""
Provides API endpoints for managing the user retirement process.
"""
- permission_classes = (permissions.IsAuthenticated, CanRetireUser,)
+
+ permission_classes = (
+ permissions.IsAuthenticated,
+ CanRetireUser,
+ )
parser_classes = (JSONParser,)
serializer_class = UserRetirementStatusSerializer
@@ -883,37 +875,35 @@ def retirement_queue(self, request):
created in the retirement queue at least `cool_off_days` ago.
"""
try:
- cool_off_days = int(request.GET['cool_off_days'])
+ cool_off_days = int(request.GET["cool_off_days"])
if cool_off_days < 0:
- raise RetirementStateError('Invalid argument for cool_off_days, must be greater than 0.')
+ raise RetirementStateError("Invalid argument for cool_off_days, must be greater than 0.")
- states = request.GET.getlist('states')
+ states = request.GET.getlist("states")
if not states:
raise RetirementStateError('Param "states" required with at least one state.')
state_objs = RetirementState.objects.filter(state_name__in=states)
if state_objs.count() != len(states):
found = [s.state_name for s in state_objs]
- raise RetirementStateError(f'Unknown state. Requested: {states} Found: {found}')
+ raise RetirementStateError(f"Unknown state. Requested: {states} Found: {found}")
- limit = request.GET.get('limit')
+ limit = request.GET.get("limit")
if limit:
try:
limit_count = int(limit)
except ValueError:
return Response(
- f'Limit could not be parsed: {limit}, please ensure this is an integer',
- status=status.HTTP_400_BAD_REQUEST
+ f"Limit could not be parsed: {limit}, please ensure this is an integer",
+ status=status.HTTP_400_BAD_REQUEST,
)
earliest_datetime = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=cool_off_days)
- retirements = UserRetirementStatus.objects.select_related(
- 'user', 'current_state', 'last_state'
- ).filter(
- current_state__in=state_objs, created__lt=earliest_datetime
- ).order_by(
- 'id'
+ retirements = (
+ UserRetirementStatus.objects.select_related("user", "current_state", "last_state")
+ .filter(current_state__in=state_objs, created__lt=earliest_datetime)
+ .order_by("id")
)
if limit:
retirements = retirements[:limit_count]
@@ -921,10 +911,9 @@ def retirement_queue(self, request):
return Response(serializer.data)
# This should only occur on the int() conversion of cool_off_days at this point
except ValueError:
- return Response('Invalid cool_off_days, should be integer.', status=status.HTTP_400_BAD_REQUEST)
+ return Response("Invalid cool_off_days, should be integer.", status=status.HTTP_400_BAD_REQUEST)
except KeyError as exc:
- return Response(f'Missing required parameter: {str(exc)}',
- status=status.HTTP_400_BAD_REQUEST)
+ return Response(f"Missing required parameter: {str(exc)}", status=status.HTTP_400_BAD_REQUEST)
except RetirementStateError as exc:
return Response(str(exc), status=status.HTTP_400_BAD_REQUEST)
@@ -939,36 +928,33 @@ def retirements_by_status_and_date(self, request):
so to get one day you would set both dates to that day.
"""
try:
- start_date = datetime.datetime.strptime(request.GET['start_date'], '%Y-%m-%d').replace(tzinfo=pytz.UTC)
- end_date = datetime.datetime.strptime(request.GET['end_date'], '%Y-%m-%d').replace(tzinfo=pytz.UTC)
+ start_date = datetime.datetime.strptime(request.GET["start_date"], "%Y-%m-%d").replace(tzinfo=pytz.UTC)
+ end_date = datetime.datetime.strptime(request.GET["end_date"], "%Y-%m-%d").replace(tzinfo=pytz.UTC)
now = datetime.datetime.now(pytz.UTC)
if start_date > now or end_date > now or start_date > end_date:
- raise RetirementStateError('Dates must be today or earlier, and start must be earlier than end.')
+ raise RetirementStateError("Dates must be today or earlier, and start must be earlier than end.")
# Add a day to make sure we get all the way to 23:59:59.999, this is compared "lt" in the query
# not "lte".
end_date += datetime.timedelta(days=1)
- state = request.GET['state']
+ state = request.GET["state"]
state_obj = RetirementState.objects.get(state_name=state)
- retirements = UserRetirementStatus.objects.select_related(
- 'user', 'current_state', 'last_state', 'user__profile'
- ).filter(
- current_state=state_obj, created__lt=end_date, created__gte=start_date
- ).order_by(
- 'id'
+ retirements = (
+ UserRetirementStatus.objects.select_related("user", "current_state", "last_state", "user__profile")
+ .filter(current_state=state_obj, created__lt=end_date, created__gte=start_date)
+ .order_by("id")
)
serializer = UserRetirementStatusSerializer(retirements, many=True)
return Response(serializer.data)
# This should only occur on the datetime conversion of the start / end dates.
except ValueError as exc:
- return Response(f'Invalid start or end date: {str(exc)}', status=status.HTTP_400_BAD_REQUEST)
+ return Response(f"Invalid start or end date: {str(exc)}", status=status.HTTP_400_BAD_REQUEST)
except KeyError as exc:
- return Response(f'Missing required parameter: {str(exc)}',
- status=status.HTTP_400_BAD_REQUEST)
+ return Response(f"Missing required parameter: {str(exc)}", status=status.HTTP_400_BAD_REQUEST)
except RetirementState.DoesNotExist:
- return Response('Unknown retirement state.', status=status.HTTP_400_BAD_REQUEST)
+ return Response("Unknown retirement state.", status=status.HTTP_400_BAD_REQUEST)
except RetirementStateError as exc:
return Response(str(exc), status=status.HTTP_400_BAD_REQUEST)
@@ -980,9 +966,9 @@ def retrieve(self, request, username): # pylint: disable=unused-argument
"""
try:
user = get_potentially_retired_user_by_username(username)
- retirement = UserRetirementStatus.objects.select_related(
- 'user', 'current_state', 'last_state'
- ).get(user=user)
+ retirement = UserRetirementStatus.objects.select_related("user", "current_state", "last_state").get(
+ user=user
+ )
serializer = UserRetirementStatusSerializer(instance=retirement)
return Response(serializer.data)
except (UserRetirementStatus.DoesNotExist, User.DoesNotExist):
@@ -1008,7 +994,7 @@ def partial_update(self, request):
The content type for this request is 'application/json'.
"""
try:
- username = request.data['username']
+ username = request.data["username"]
retirements = UserRetirementStatus.objects.filter(original_username=username)
# During a narrow window learners were able to re-use a username that had been retired if
@@ -1049,20 +1035,19 @@ def cleanup(self, request):
Deletes a batch of retirement requests by username.
"""
try:
- usernames = request.data['usernames']
+ usernames = request.data["usernames"]
if not isinstance(usernames, list):
- raise TypeError('Usernames should be an array.')
+ raise TypeError("Usernames should be an array.")
- complete_state = RetirementState.objects.get(state_name='COMPLETE')
+ complete_state = RetirementState.objects.get(state_name="COMPLETE")
retirements = UserRetirementStatus.objects.filter(
- original_username__in=usernames,
- current_state=complete_state
+ original_username__in=usernames, current_state=complete_state
)
# Sanity check that they're all valid usernames in the right state
if len(usernames) != len(retirements):
- raise UserRetirementStatus.DoesNotExist('Not all usernames exist in the COMPLETE state.')
+ raise UserRetirementStatus.DoesNotExist("Not all usernames exist in the COMPLETE state.")
retirements.delete()
return Response(status=status.HTTP_204_NO_CONTENT)
@@ -1076,7 +1061,11 @@ class LMSAccountRetirementView(ViewSet):
"""
Provides an API endpoint for retiring a user in the LMS.
"""
- permission_classes = (permissions.IsAuthenticated, CanRetireUser,)
+
+ permission_classes = (
+ permissions.IsAuthenticated,
+ CanRetireUser,
+ )
parser_classes = (JSONParser,)
@request_requires_username
@@ -1093,13 +1082,13 @@ def post(self, request):
Retires the user with the given username in the LMS.
"""
- username = request.data['username']
+ username = request.data["username"]
try:
retirement = UserRetirementStatus.get_retirement_for_retirement_action(username)
RevisionPluginRevision.retire_user(retirement.user)
ArticleRevision.retire_user(retirement.user)
- PendingNameChange.delete_by_user_value(retirement.user, field='user')
+ PendingNameChange.delete_by_user_value(retirement.user, field="user")
ManualEnrollmentAudit.retire_manual_enrollments(retirement.user, retirement.retired_email)
CreditRequest.retire_user(retirement)
@@ -1115,7 +1104,7 @@ def post(self, request):
sender=self.__class__,
email=retirement.original_email,
new_email=retirement.retired_email,
- user=retirement.user
+ user=retirement.user,
)
except UserRetirementStatus.DoesNotExist:
return Response(status=status.HTTP_404_NOT_FOUND)
@@ -1131,7 +1120,11 @@ class AccountRetirementView(ViewSet):
"""
Provides API endpoint for retiring a user.
"""
- permission_classes = (permissions.IsAuthenticated, CanRetireUser,)
+
+ permission_classes = (
+ permissions.IsAuthenticated,
+ CanRetireUser,
+ )
parser_classes = (JSONParser,)
@request_requires_username
@@ -1148,7 +1141,7 @@ def post(self, request):
Retires the user with the given username. This includes retiring this username, the associated email address,
and any other PII associated with this user.
"""
- username = request.data['username']
+ username = request.data["username"]
try:
retirement_status = UserRetirementStatus.get_retirement_for_retirement_action(username)
@@ -1173,18 +1166,18 @@ def post(self, request):
self.retire_entitlement_support_detail(user)
# Retire misc. models that may contain PII of this user
- PendingEmailChange.delete_by_user_value(user, field='user')
- UserOrgTag.delete_by_user_value(user, field='user')
+ PendingEmailChange.delete_by_user_value(user, field="user")
+ UserOrgTag.delete_by_user_value(user, field="user")
# Retire any objects linked to the user via their original email
- CourseEnrollmentAllowed.delete_by_user_value(original_email, field='email')
- UnregisteredLearnerCohortAssignments.delete_by_user_value(original_email, field='email')
+ CourseEnrollmentAllowed.delete_by_user_value(original_email, field="email")
+ UnregisteredLearnerCohortAssignments.delete_by_user_value(original_email, field="email")
# This signal allows code in higher points of LMS to retire the user as necessary
USER_RETIRE_LMS_CRITICAL.send(sender=self.__class__, user=user)
- user.first_name = ''
- user.last_name = ''
+ user.first_name = ""
+ user.last_name = ""
user.is_active = False
user.username = retired_username
user.save()
@@ -1227,24 +1220,20 @@ def retire_users_data_sharing_consent(username, retired_username):
@staticmethod
def retire_sapsf_data_transmission(user): # lint-amnesty, pylint: disable=missing-function-docstring
for ent_user in EnterpriseCustomerUser.objects.filter(user_id=user.id):
- for enrollment in EnterpriseCourseEnrollment.objects.filter(
- enterprise_customer_user=ent_user
- ):
+ for enrollment in EnterpriseCourseEnrollment.objects.filter(enterprise_customer_user=ent_user):
audits = SapSuccessFactorsLearnerDataTransmissionAudit.objects.filter(
enterprise_course_enrollment_id=enrollment.id
)
- audits.update(sapsf_user_id='')
+ audits.update(sapsf_user_id="")
@staticmethod
def retire_degreed_data_transmission(user): # lint-amnesty, pylint: disable=missing-function-docstring
for ent_user in EnterpriseCustomerUser.objects.filter(user_id=user.id):
- for enrollment in EnterpriseCourseEnrollment.objects.filter(
- enterprise_customer_user=ent_user
- ):
+ for enrollment in EnterpriseCourseEnrollment.objects.filter(enterprise_customer_user=ent_user):
audits = DegreedLearnerDataTransmissionAudit.objects.filter(
enterprise_course_enrollment_id=enrollment.id
)
- audits.update(degreed_user_email='')
+ audits.update(degreed_user_email="")
@staticmethod
def retire_user_from_pending_enterprise_customer_user(user, retired_email):
@@ -1256,7 +1245,7 @@ def retire_entitlement_support_detail(user):
Updates all CourseEntitleSupportDetail records for the given user to have an empty ``comments`` field.
"""
for entitlement in CourseEntitlement.objects.filter(user_id=user.id):
- entitlement.courseentitlementsupportdetail_set.all().update(comments='')
+ entitlement.courseentitlementsupportdetail_set.all().update(comments="")
@staticmethod
def clear_pii_from_certificate_records(user):
@@ -1279,6 +1268,7 @@ class UsernameReplacementView(APIView):
This API will be called first, before calling the APIs in other services as this
one handles the checks on the usernames provided.
"""
+
permission_classes = (permissions.IsAuthenticated, CanReplaceUsername)
def post(self, request):
@@ -1320,16 +1310,16 @@ def post(self, request):
# (model_name, column_name)
MODELS_WITH_USERNAME = (
- ('auth.user', 'username'),
- ('consent.DataSharingConsent', 'username'),
- ('consent.HistoricalDataSharingConsent', 'username'),
- ('credit.CreditEligibility', 'username'),
- ('credit.CreditRequest', 'username'),
- ('credit.CreditRequirementStatus', 'username'),
- ('user_api.UserRetirementPartnerReportingStatus', 'original_username'),
- ('user_api.UserRetirementStatus', 'original_username')
+ ("auth.user", "username"),
+ ("consent.DataSharingConsent", "username"),
+ ("consent.HistoricalDataSharingConsent", "username"),
+ ("credit.CreditEligibility", "username"),
+ ("credit.CreditRequest", "username"),
+ ("credit.CreditRequirementStatus", "username"),
+ ("user_api.UserRetirementPartnerReportingStatus", "original_username"),
+ ("user_api.UserRetirementStatus", "original_username"),
)
- UNIQUE_SUFFIX_LENGTH = getattr(settings, 'SOCIAL_AUTH_UUID_LENGTH', 4)
+ UNIQUE_SUFFIX_LENGTH = getattr(settings, "SOCIAL_AUTH_UUID_LENGTH", 4)
username_mappings = request.data.get("username_mappings")
replacement_locations = self._load_models(MODELS_WITH_USERNAME)
@@ -1344,9 +1334,7 @@ def post(self, request):
desired_username = list(username_pair.values())[0]
new_username = self._generate_unique_username(desired_username, suffix_length=UNIQUE_SUFFIX_LENGTH)
successfully_replaced = self._replace_username_for_all_models(
- current_username,
- new_username,
- replacement_locations
+ current_username, new_username, replacement_locations
)
if successfully_replaced:
successful_replacements.append({current_username: new_username})
@@ -1354,14 +1342,11 @@ def post(self, request):
failed_replacements.append({current_username: new_username})
return Response(
status=status.HTTP_200_OK,
- data={
- "successful_replacements": successful_replacements,
- "failed_replacements": failed_replacements
- }
+ data={"successful_replacements": successful_replacements, "failed_replacements": failed_replacements},
)
def _load_models(self, models_with_fields):
- """ Takes tuples that contain a model path and returns the list with a loaded version of the model """
+ """Takes tuples that contain a model path and returns the list with a loaded version of the model"""
try:
replacement_locations = [(apps.get_model(model), column) for (model, column) in models_with_fields]
except LookupError:
@@ -1370,7 +1355,7 @@ def _load_models(self, models_with_fields):
return replacement_locations
def _has_valid_schema(self, post_data):
- """ Verifies the data is a list of objects with a single key:value pair """
+ """Verifies the data is a list of objects with a single key:value pair"""
if not isinstance(post_data, list):
return False
for obj in post_data:
@@ -1389,7 +1374,7 @@ def _generate_unique_username(self, desired_username, suffix_length=4):
while True:
if User.objects.filter(username=new_username).exists():
# adding a dash between user-supplied and system-generated values to avoid weird combinations
- new_username = desired_username + '-' + username_suffix_generator(suffix_length)
+ new_username = desired_username + "-" + username_suffix_generator(suffix_length)
else:
break
return new_username
@@ -1404,10 +1389,8 @@ def _replace_username_for_all_models(self, current_username, new_username, repla
try:
with transaction.atomic():
num_rows_changed = 0
- for (model, column) in replacement_locations:
- num_rows_changed += model.objects.filter(
- **{column: current_username}
- ).update(
+ for model, column in replacement_locations:
+ num_rows_changed += model.objects.filter(**{column: current_username}).update(
**{column: new_username}
)
except Exception as exc: # pylint: disable=broad-except
@@ -1416,7 +1399,7 @@ def _replace_username_for_all_models(self, current_username, new_username, repla
current_username,
new_username,
model.__class__.__name__, # Retrieves the model name that it failed on
- exc
+ exc,
)
return False
if num_rows_changed == 0:
diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py
index 17ca1d4f2a91..ab57687d2cd4 100644
--- a/openedx/core/djangoapps/user_authn/views/register.py
+++ b/openedx/core/djangoapps/user_authn/views/register.py
@@ -390,7 +390,9 @@ def _track_user_registration(user, profile, params, third_party_provider, regist
'is_year_of_birth_selected': bool(profile.year_of_birth),
'is_education_selected': bool(profile.level_of_education_display),
'is_goal_set': bool(profile.goals),
- 'total_registration_time': round(float(params.get('totalRegistrationTime', '0'))),
+ 'total_registration_time': round(
+ float(params.get('total_registration_time') or params.get('totalRegistrationTime') or 0)
+ ),
'activation_key': registration.activation_key if registration else None,
'host': params.get('host', ''),
'app_name': params.get('app_name', ''),
diff --git a/openedx/core/djangoapps/xblock/rest_api/views.py b/openedx/core/djangoapps/xblock/rest_api/views.py
index 8c2d16839a67..9972e7463b23 100644
--- a/openedx/core/djangoapps/xblock/rest_api/views.py
+++ b/openedx/core/djangoapps/xblock/rest_api/views.py
@@ -226,7 +226,9 @@ def post(self, request, usage_key_str):
old_metadata = block.get_explicitly_set_fields_by_scope(Scope.settings)
old_content = block.get_explicitly_set_fields_by_scope(Scope.content)
- block.data = data
+ # only update data if it was passed
+ if data is not None:
+ block.data = data
# update existing metadata with submitted metadata (which can be partial)
# IMPORTANT NOTE: if the client passed 'null' (None) for a piece of metadata that means 'remove it'.
diff --git a/requirements/constraints.txt b/requirements/constraints.txt
index 0c088275fedb..c39f6730d0dd 100644
--- a/requirements/constraints.txt
+++ b/requirements/constraints.txt
@@ -26,7 +26,7 @@ celery>=5.2.2,<6.0.0
# The team that owns this package will manually bump this package rather than having it pulled in automatically.
# This is to allow them to better control its deployment and to do it in a process that works better
# for them.
-edx-enterprise==4.23.9
+edx-enterprise==4.23.13
# Stay on LTS version, remove once this is added to common constraint
Django<5.0
diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt
index ac8d7e421701..3231c22f0473 100644
--- a/requirements/edx/base.txt
+++ b/requirements/edx/base.txt
@@ -467,7 +467,7 @@ edx-drf-extensions==10.3.0
# edx-when
# edxval
# openedx-learning
-edx-enterprise==4.23.9
+edx-enterprise==4.23.13
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/kernel.in
diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt
index 97d51c13facc..9608a1be892f 100644
--- a/requirements/edx/development.txt
+++ b/requirements/edx/development.txt
@@ -741,7 +741,7 @@ edx-drf-extensions==10.3.0
# edx-when
# edxval
# openedx-learning
-edx-enterprise==4.23.9
+edx-enterprise==4.23.13
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/doc.txt
diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt
index 2527d80833a6..059c0f81785e 100644
--- a/requirements/edx/doc.txt
+++ b/requirements/edx/doc.txt
@@ -547,7 +547,7 @@ edx-drf-extensions==10.3.0
# edx-when
# edxval
# openedx-learning
-edx-enterprise==4.23.9
+edx-enterprise==4.23.13
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/base.txt
diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt
index 340909a950b4..dc3e37a12d6c 100644
--- a/requirements/edx/testing.txt
+++ b/requirements/edx/testing.txt
@@ -571,7 +571,7 @@ edx-drf-extensions==10.3.0
# edx-when
# edxval
# openedx-learning
-edx-enterprise==4.23.9
+edx-enterprise==4.23.13
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/base.txt
diff --git a/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html b/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html
index 8d51b16498d7..9319217aa4cf 100644
--- a/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html
+++ b/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html
@@ -63,7 +63,7 @@
|
diff --git a/xmodule/js/src/tabs/tabs-aggregator.js b/xmodule/js/src/tabs/tabs-aggregator.js
index 83baca4cf6e9..da982b6afc9d 100644
--- a/xmodule/js/src/tabs/tabs-aggregator.js
+++ b/xmodule/js/src/tabs/tabs-aggregator.js
@@ -63,8 +63,8 @@
if ($.isFunction(onSwitchFunction)) {
onSwitchFunction();
}
- this.$tabs.removeClass('current');
- $currentTarget.addClass('current');
+ this.$tabs.attr('aria-current', 'false').removeClass('current');
+ $currentTarget.attr('aria-current', 'true').addClass('current');
/*
Tabs are implemeted like anchors. Therefore we can use hash to find
diff --git a/xmodule/js/src/video/09_video_caption.js b/xmodule/js/src/video/09_video_caption.js
index 37e3923067d5..f5db26514b64 100644
--- a/xmodule/js/src/video/09_video_caption.js
+++ b/xmodule/js/src/video/09_video_caption.js
@@ -1096,12 +1096,13 @@
if (typeof this.currentIndex !== 'undefined') {
this.subtitlesEl
.find('li.current')
+ .attr('aria-current', 'false')
.removeClass('current');
- }
-
+ }
this.subtitlesEl
.find("span[data-index='" + newIndex + "']")
.parent()
+ .attr('aria-current', 'true')
.addClass('current');
this.currentIndex = newIndex;
|