diff --git a/Makefile b/Makefile index 055923d1afcd..1012e558a7d9 100644 --- a/Makefile +++ b/Makefile @@ -86,7 +86,8 @@ else atlas pull $(ATLAS_OPTIONS) translations/edx-platform/conf/locale:conf/locale i18n_tool generate endif - paver i18n_compilejs + python manage.py lms compilejsi18n + python manage.py cms compilejsi18n detect_changed_source_translations: ## check if translation files are up-to-date diff --git a/lms/djangoapps/support/tests/test_views.py b/lms/djangoapps/support/tests/test_views.py index 6338935470f4..4bf71a16d8c6 100644 --- a/lms/djangoapps/support/tests/test_views.py +++ b/lms/djangoapps/support/tests/test_views.py @@ -54,7 +54,7 @@ from common.djangoapps.third_party_auth.tests.factories import SAMLProviderConfigFactory from common.test.utils import disable_signal from lms.djangoapps.program_enrollments.tests.factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory -from lms.djangoapps.support.models import CourseResetAudit +from lms.djangoapps.support.models import CourseResetAudit, CourseResetCourseOptIn from lms.djangoapps.support.serializers import ProgramEnrollmentSerializer from lms.djangoapps.support.tests.factories import CourseResetCourseOptInFactory, CourseResetAuditFactory from lms.djangoapps.verify_student.models import VerificationDeadline @@ -2331,3 +2331,97 @@ def test_multiple_failed_audits(self): 'can_reset': True, 'status': most_recent_audit.status_message() }]) + + +class TestResetCourseViewPost(SupportViewTestCase): + """ + Tests for creating course request + """ + + def setUp(self): + super().setUp() + SupportStaffRole().add_users(self.user) + + self.course_id = 'course-v1:a+b+c' + + self.other_user = User.objects.create(username='otheruser', password='test') + + self.course = CourseFactory.create( + org='a', + course='b', + run='c', + enable_proctored_exams=True, + proctoring_provider=settings.PROCTORING_BACKENDS['DEFAULT'], + ) + self.enrollment = CourseEnrollmentFactory( + is_active=True, + mode='verified', + course_id=self.course.id, + user=self.user + ) + self.opt_in = CourseResetCourseOptInFactory.create(course_id=self.course.id) + + self.other_course = CourseFactory.create( + org='x', + course='y', + run='z', + ) + + def _url(self, username): + return reverse("support:course_reset", kwargs={'username_or_email': username}) + + def test_wrong_username(self): + """ + Test that a request with a username which does not exits returns 404 + """ + response = self.client.post(self._url(username='does_not_exist'), data={'course_id': 'course-v1:aa+bb+c'}) + self.assertEqual(response.status_code, 404) + + def test_learner_course_reset(self): + response = self.client.post(self._url(username=self.user.username), data={'course_id': self.course_id}) + self.assertEqual(response.status_code, 201) + self.assertEqual(response.data, { + 'course_id': self.course_id, + 'status': response.data['status'], + 'can_reset': False, + 'display_name': self.course.display_name + }) + + def test_course_not_opt_in(self): + response = self.client.post(self._url(username=self.user.username), data={'course_id': 'course-v1:aa+bb+c'}) + self.assertEqual(response.status_code, 404) + + def test_course_reset_failed(self): + course = CourseFactory.create( + org='xx', + course='yy', + run='zz', + ) + enrollment = CourseEnrollmentFactory( + is_active=True, + mode='verified', + course_id=course.id, + user=self.user + ) + + opt_in_course = CourseResetCourseOptIn.objects.create( + course_id=course.id, + active=True + ) + + CourseResetAudit.objects.create( + course=opt_in_course, + course_enrollment=enrollment, + reset_by=self.other_user, + status=CourseResetAudit.CourseResetStatus.FAILED + ) + response = self.client.post(self._url(username=self.user.username), data={'course_id': course.id}) + self.assertEqual(response.status_code, 200) + + def test_course_reset_dupe(self): + CourseResetAuditFactory.create( + course=self.opt_in, + course_enrollment=self.enrollment, + ) + response2 = self.client.post(self._url(username=self.user.username), data={'course_id': self.course_id}) + self.assertEqual(response2.status_code, 204) diff --git a/lms/djangoapps/support/views/course_reset.py b/lms/djangoapps/support/views/course_reset.py index b274d4a97fa1..c8d4abb2cec1 100644 --- a/lms/djangoapps/support/views/course_reset.py +++ b/lms/djangoapps/support/views/course_reset.py @@ -97,4 +97,69 @@ def get(self, request, username_or_email): @method_decorator(require_support_permission) def post(self, request, username_or_email): - """ Other Ticket """ + """ + Resets a course for the given learner + + returns a dicts with the format { + 'course_id': + 'display_name': + 'status': + 'can_reset': (boolean) + } + """ + course_id = request.data['course_id'] + try: + user = get_user_by_username_or_email(username_or_email) + except User.DoesNotExist: + return Response({'error': 'User does not exist'}, status=404) + try: + opt_in_course = CourseResetCourseOptIn.objects.get(course_id=course_id) + except CourseResetCourseOptIn.DoesNotExist: + return Response({'error': 'Course is not eligible'}, status=404) + enrollment = CourseEnrollment.objects.get( + course=course_id, + user=user, + is_active=True + ) + user_passed = user_has_passing_grade_in_course(enrollment=enrollment) + course_overview = enrollment.course_overview + course_reset_audit = CourseResetAudit.objects.filter(course_enrollment=enrollment).first() + + if course_reset_audit and ( + course_reset_audit.status == CourseResetAudit.CourseResetStatus.FAILED + and not user_passed + ): + course_reset_audit.status = CourseResetAudit.CourseResetStatus.ENQUEUED + course_reset_audit.save() + # Call celery task + resp = { + 'course_id': course_id, + 'status': course_reset_audit.status_message(), + 'can_reset': False, + 'display_name': course_overview.display_name + } + return Response(resp, status=200) + + elif course_reset_audit and course_reset_audit.status in ( + CourseResetAudit.CourseResetStatus.IN_PROGRESS, + CourseResetAudit.CourseResetStatus.ENQUEUED + ): + return Response(None, status=204) + + if enrollment and opt_in_course and not user_passed: + course_reset_audit = CourseResetAudit.objects.create( + course=opt_in_course, + course_enrollment=enrollment, + reset_by=request.user, + ) + resp = { + 'course_id': course_id, + 'status': course_reset_audit.status_message(), + 'can_reset': False, + 'display_name': course_overview.display_name + } + + # Call celery task + return Response(resp, status=201) + else: + return Response(None, status=400) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index b9b7f47a7039..c167eba3a629 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -7,7 +7,8 @@ import openedx_tagging.core.tagging.api as oel_tagging from django.db.models import Exists, OuterRef, Q, QuerySet -from opaque_keys.edx.keys import CourseKey, LearningContextKey +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy from organizations.models import Organization @@ -131,24 +132,28 @@ def get_unassigned_taxonomies(enabled=True) -> QuerySet: def get_all_object_tags( - content_key: LearningContextKey, + content_key: LibraryLocatorV2 | CourseKey, ) -> tuple[ObjectTagByObjectIdDict, TaxonomyDict]: """ + Get all the object tags applied to components in the given course/library. + + Includes any tags applied to the course/library as a whole. Returns a tuple with a dictionary of grouped object tags for all blocks and a dictionary of taxonomies. """ - # ToDo: Add support for other content types (like LibraryContent and LibraryBlock) + context_key_str = str(content_key) + # We use a block_id_prefix (i.e. the modified course id) to get the tags for the children of the Content + # (course/library) in a single db query. if isinstance(content_key, CourseKey): - course_key_str = str(content_key) - # We use a block_id_prefix (i.e. the modified course id) to get the tags for the children of the Content - # (course) in a single db query. - block_id_prefix = course_key_str.replace("course-v1:", "block-v1:", 1) + block_id_prefix = context_key_str.replace("course-v1:", "block-v1:", 1) + elif isinstance(content_key, LibraryLocatorV2): + block_id_prefix = context_key_str.replace("lib:", "lb:", 1) else: raise NotImplementedError(f"Invalid content_key: {type(content_key)} -> {content_key}") # There is no API method in oel_tagging.api that does this yet, # so for now we have to build the ORM query directly. all_object_tags = list(ObjectTag.objects.filter( - Q(object_id__startswith=block_id_prefix) | Q(object_id=course_key_str), + Q(object_id__startswith=block_id_prefix) | Q(object_id=content_key), Q(tag__isnull=False, tag__taxonomy__isnull=False), ).select_related("tag__taxonomy")) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/objecttag_export_helpers.py index 14103642d8ec..e99017f432e8 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/objecttag_export_helpers.py @@ -4,11 +4,15 @@ from __future__ import annotations -from typing import Iterator +from typing import Any, Callable, Iterator, Union from attrs import define -from opaque_keys.edx.keys import CourseKey, LearningContextKey +from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.locator import LibraryLocatorV2 +from xblock.core import XBlock +import openedx.core.djangoapps.content_libraries.api as library_api +from openedx.core.djangoapps.content_libraries.api import LibraryXBlockMetadata from xmodule.modulestore.django import modulestore from ...types import ObjectTagByObjectIdDict, ObjectTagByTaxonomyIdDict @@ -38,51 +42,137 @@ def iterate_with_level( yield from iterate_with_level(child, level + 1) -def build_object_tree_with_objecttags( - content_key: LearningContextKey, - object_tag_cache: ObjectTagByObjectIdDict, -) -> TaggedContent: +def _get_course_tagged_object_and_children( + course_key: CourseKey, object_tag_cache: ObjectTagByObjectIdDict +) -> tuple[TaggedContent, list[XBlock]]: """ - Returns the object with the tags associated with it. + Returns a TaggedContent with course metadata with its tags, and its children. """ store = modulestore() - if isinstance(content_key, CourseKey): - course = store.get_course(content_key) - if course is None: - raise ValueError(f"Course not found: {content_key}") - else: - raise NotImplementedError(f"Invalid content_key: {type(content_key)} -> {content_key}") + course = store.get_course(course_key) + if course is None: + raise ValueError(f"Course not found: {course_key}") - display_name = course.display_name_with_default - course_id = str(course.id) + course_id = str(course_key) tagged_course = TaggedContent( - display_name=display_name, + display_name=course.display_name_with_default, block_id=course_id, category=course.category, - object_tags=object_tag_cache.get(str(content_key), {}), + object_tags=object_tag_cache.get(course_id, {}), + children=None, + ) + + return tagged_course, course.children if course.has_children else [] + + +def _get_library_tagged_object_and_children( + library_key: LibraryLocatorV2, object_tag_cache: ObjectTagByObjectIdDict +) -> tuple[TaggedContent, list[LibraryXBlockMetadata]]: + """ + Returns a TaggedContent with library metadata with its tags, and its children. + """ + library = library_api.get_library(library_key) + if library is None: + raise ValueError(f"Library not found: {library_key}") + + library_id = str(library_key) + + tagged_library = TaggedContent( + display_name=library.title, + block_id=library_id, + category='library', + object_tags=object_tag_cache.get(library_id, {}), + children=None, + ) + + library_components = library_api.get_library_components(library_key) + children = [ + LibraryXBlockMetadata.from_component(library_key, component) + for component in library_components + ] + + return tagged_library, children + + +def _get_xblock_tagged_object_and_children( + usage_key: UsageKey, object_tag_cache: ObjectTagByObjectIdDict +) -> tuple[TaggedContent, list[XBlock]]: + """ + Returns a TaggedContent with xblock metadata with its tags, and its children. + """ + store = modulestore() + block = store.get_item(usage_key) + block_id = str(usage_key) + tagged_block = TaggedContent( + display_name=block.display_name_with_default, + block_id=block_id, + category=block.category, + object_tags=object_tag_cache.get(block_id, {}), children=None, ) - blocks = [(tagged_course, course)] + return tagged_block, block.children if block.has_children else [] + + +def _get_library_block_tagged_object( + library_block: LibraryXBlockMetadata, object_tag_cache: ObjectTagByObjectIdDict +) -> tuple[TaggedContent, None]: + """ + Returns a TaggedContent with library content block metadata and its tags, + and 'None' as children. + """ + block_id = str(library_block.usage_key) + tagged_library_block = TaggedContent( + display_name=library_block.display_name, + block_id=block_id, + category=library_block.usage_key.block_type, + object_tags=object_tag_cache.get(block_id, {}), + children=None, + ) + + return tagged_library_block, None + + +def build_object_tree_with_objecttags( + content_key: LibraryLocatorV2 | CourseKey, + object_tag_cache: ObjectTagByObjectIdDict, +) -> TaggedContent: + """ + Returns the object with the tags associated with it. + """ + get_tagged_children: Union[ + # _get_course_tagged_object_and_children type + Callable[[LibraryXBlockMetadata, dict[str, dict[int, list[Any]]]], tuple[TaggedContent, None]], + # _get_library_block_tagged_object type + Callable[[UsageKey, dict[str, dict[int, list[Any]]]], tuple[TaggedContent, list[Any]]] + ] + if isinstance(content_key, CourseKey): + tagged_content, children = _get_course_tagged_object_and_children( + content_key, object_tag_cache + ) + get_tagged_children = _get_xblock_tagged_object_and_children + elif isinstance(content_key, LibraryLocatorV2): + tagged_content, children = _get_library_tagged_object_and_children( + content_key, object_tag_cache + ) + get_tagged_children = _get_library_block_tagged_object + else: + raise ValueError(f"Invalid content_key: {type(content_key)} -> {content_key}") + + blocks: list[tuple[TaggedContent, list | None]] = [(tagged_content, children)] while blocks: - tagged_block, xblock = blocks.pop() + tagged_block, block_children = blocks.pop() tagged_block.children = [] - if xblock.has_children: - for child_id in xblock.children: - child_block = store.get_item(child_id) - tagged_child = TaggedContent( - display_name=child_block.display_name_with_default, - block_id=str(child_id), - category=child_block.category, - object_tags=object_tag_cache.get(str(child_id), {}), - children=None, - ) - tagged_block.children.append(tagged_child) - - blocks.append((tagged_child, child_block)) - - return tagged_course + if not block_children: + continue + + for child in block_children: + tagged_child, child_children = get_tagged_children(child, object_tag_cache) + tagged_block.children.append(tagged_child) + blocks.append((tagged_child, child_children)) + + return tagged_content diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_objecttag_export_helpers.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_objecttag_export_helpers.py index 28d75f0fdfb6..61132a69a227 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_objecttag_export_helpers.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_objecttag_export_helpers.py @@ -3,6 +3,7 @@ """ from unittest.mock import patch +from openedx.core.djangoapps.content_libraries import api as library_api from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory @@ -33,7 +34,8 @@ def setUp(self): run="test_run", display_name="Test Course", ) - self.expected_tagged_xblock = TaggedContent( + + self.expected_course_tagged_xblock = TaggedContent( display_name="Test Course", block_id="course-v1:orgA+test_course+test_run", category="course", @@ -61,8 +63,8 @@ def setUp(self): }, ) - assert self.expected_tagged_xblock.children is not None # type guard - self.expected_tagged_xblock.children.append(tagged_sequential) + assert self.expected_course_tagged_xblock.children is not None # type guard + self.expected_course_tagged_xblock.children.append(tagged_sequential) # Untagged blocks sequential2 = BlockFactory.create( @@ -77,8 +79,8 @@ def setUp(self): children=[], object_tags={}, ) - assert self.expected_tagged_xblock.children is not None # type guard - self.expected_tagged_xblock.children.append(untagged_sequential) + assert self.expected_course_tagged_xblock.children is not None # type guard + self.expected_course_tagged_xblock.children.append(untagged_sequential) BlockFactory.create( parent=sequential2, category="vertical", @@ -127,12 +129,12 @@ def setUp(self): assert tagged_sequential.children is not None # type guard tagged_sequential.children.append(untagged_vertical2) - html = BlockFactory.create( + BlockFactory.create( parent=vertical2, category="html", display_name="test html", ) - tagged_text = TaggedContent( + tagged_html = TaggedContent( display_name="test html", block_id="block-v1:orgA+test_course+test_run+type@html+block@test_html", category="html", @@ -142,36 +144,126 @@ def setUp(self): }, ) assert untagged_vertical2.children is not None # type guard - untagged_vertical2.children.append(tagged_text) + untagged_vertical2.children.append(tagged_html) - self.all_object_tags, _ = api.get_all_object_tags(self.course.id) - self.expected_tagged_content_list = [ - (self.expected_tagged_xblock, 0), + self.all_course_object_tags, _ = api.get_all_object_tags(self.course.id) + self.expected_course_tagged_content_list = [ + (self.expected_course_tagged_xblock, 0), (tagged_sequential, 1), (tagged_vertical, 2), (untagged_vertical2, 2), - (tagged_text, 3), + (tagged_html, 3), (untagged_sequential, 1), (untagged_vertical, 2), ] + # Create a library + self.library = library_api.create_library( + self.orgA, + f"lib_{self.block_suffix}", + "Test Library", + ) + self.expected_library_tagged_xblock = TaggedContent( + display_name="Test Library", + block_id=f"lib:orgA:lib_{self.block_suffix}", + category="library", + children=[], + object_tags={ + self.taxonomy_2.id: list(self.library_tags), + }, + ) + + library_api.create_library_block( + self.library.key, + "problem", + f"problem1_{self.block_suffix}", + ) + tagged_problem = TaggedContent( + display_name="Blank Problem", + block_id=f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}", + category="problem", + children=[], + object_tags={ + self.taxonomy_1.id: list(self.problem1_tags), + }, + ) + + library_api.create_library_block( + self.library.key, + "problem", + f"problem2_{self.block_suffix}", + ) + untagged_problem = TaggedContent( + display_name="Blank Problem", + block_id=f"lb:orgA:lib_{self.block_suffix}:problem:problem2_{self.block_suffix}", + category="problem", + children=[], + object_tags={}, + ) + + library_api.create_library_block( + self.library.key, + "html", + f"html_{self.block_suffix}", + ) + tagged_library_html = TaggedContent( + display_name="Text", + block_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + category="html", + children=[], + object_tags={ + self.taxonomy_1.id: list(self.library_html_tags1), + self.taxonomy_2.id: list(self.library_html_tags2), + }, + ) + + assert self.expected_library_tagged_xblock.children is not None # type guard + # The children are sorted by add order + self.expected_library_tagged_xblock.children.append(tagged_problem) + self.expected_library_tagged_xblock.children.append(untagged_problem) + self.expected_library_tagged_xblock.children.append(tagged_library_html) + + self.all_library_object_tags, _ = api.get_all_object_tags(self.library.key) + self.expected_library_tagged_content_list = [ + (self.expected_library_tagged_xblock, 0), + (tagged_problem, 1), + (untagged_problem, 1), + (tagged_library_html, 1), + ] + class TestContentTagChildrenExport(TaggedCourseMixin): # type: ignore[misc] """ Test helper functions for exporting tagged content """ - def test_build_object_tree(self) -> None: + def test_build_course_object_tree(self) -> None: """ Test if we can export a course """ with self.assertNumQueries(3): - tagged_xblock = build_object_tree_with_objecttags(self.course.id, self.all_object_tags) + tagged_course = build_object_tree_with_objecttags(self.course.id, self.all_course_object_tags) + + assert tagged_course == self.expected_course_tagged_xblock - assert tagged_xblock == self.expected_tagged_xblock + def test_build_library_object_tree(self) -> None: + """ + Test if we can export a library + """ + with self.assertNumQueries(8): + tagged_library = build_object_tree_with_objecttags(self.library.key, self.all_library_object_tags) + + assert tagged_library == self.expected_library_tagged_xblock + + def test_course_iterate_with_level(self) -> None: + """ + Test if we can iterate over the tagged course in the correct order + """ + tagged_content_list = list(iterate_with_level(self.expected_course_tagged_xblock)) + assert tagged_content_list == self.expected_course_tagged_content_list - def test_iterate_with_level(self) -> None: + def test_library_iterate_with_level(self) -> None: """ - Test if we can iterate over the tagged content in the correct order + Test if we can iterate over the tagged library in the correct order """ - tagged_content_list = list(iterate_with_level(self.expected_tagged_xblock)) - assert tagged_content_list == self.expected_tagged_content_list + tagged_content_list = list(iterate_with_level(self.expected_library_tagged_xblock)) + assert tagged_content_list == self.expected_library_tagged_content_list diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index cea834b38d04..d9bf18cd88e0 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -8,8 +8,7 @@ from django.db.models import Count from django.http import StreamingHttpResponse -from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.keys import UsageKey from openedx_tagging.core.tagging import rules as oel_tagging_rules from openedx_tagging.core.tagging.rest_api.v1.views import ObjectTagView, TaxonomyView from rest_framework import status @@ -29,6 +28,7 @@ set_taxonomy_orgs ) from ...rules import get_admin_orgs +from ...utils import get_content_key_from_string from .filters import ObjectTagTaxonomyOrgFilterBackend, UserOrgFilterBackend from .objecttag_export_helpers import build_object_tree_with_objecttags, iterate_with_level from .serializers import TaxonomyOrgListQueryParamsSerializer, TaxonomyOrgSerializer, TaxonomyUpdateOrgBodySerializer @@ -202,12 +202,12 @@ def _generate_csv_rows() -> Iterator[str]: object_id: str = kwargs.get('context_id', None) try: - content_key = CourseKey.from_string(object_id) - except InvalidKeyError as e: - raise ValidationError("context_id is not a valid course key.") from e + content_key = get_content_key_from_string(object_id) - # Check if the user has permission to view object tags for this object_id - try: + if isinstance(content_key, UsageKey): + raise ValidationError("The object_id must be a CourseKey or a LibraryLocatorV2.") + + # Check if the user has permission to view object tags for this object_id if not self.request.user.has_perm( "oel_tagging.view_objecttag", # The obj arg expects a model, but we are passing an object @@ -216,11 +216,13 @@ def _generate_csv_rows() -> Iterator[str]: raise PermissionDenied( "You do not have permission to view object tags for this object_id." ) + + all_object_tags, taxonomies = get_all_object_tags(content_key) + tagged_content = build_object_tree_with_objecttags(content_key, all_object_tags) + except ValueError as e: raise ValidationError from e - all_object_tags, taxonomies = get_all_object_tags(content_key) - tagged_content = build_object_tree_with_objecttags(content_key, all_object_tags) pseudo_buffer = Echo() return StreamingHttpResponse( diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 229b8d62bc6a..31e6802393c0 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -1,7 +1,11 @@ """Tests for the Tagging models""" +import time + import ddt from django.test.testcases import TestCase + from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import ObjectTag, Tag from organizations.models import Organization @@ -323,7 +327,7 @@ def setUp(self): _name="deleted taxonomy", ) - self.expected_objecttags = { + self.expected_course_objecttags = { "course-v1:orgA+test_course+test_run": { self.taxonomy_1.id: list(self.course_tags), }, @@ -339,22 +343,103 @@ def setUp(self): }, } + # Library tags and library contents need a unique block_id that is persisted along test runs + self.block_suffix = str(round(time.time() * 1000)) + + api.tag_object( + object_id=f"lib:orgA:lib_{self.block_suffix}", + taxonomy=self.taxonomy_2, + tags=['Tag 2.1'], + ) + self.library_tags = api.get_object_tags(f"lib:orgA:lib_{self.block_suffix}") + + api.tag_object( + object_id=f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}", + taxonomy=self.taxonomy_1, + tags=['Tag 1.1'], + ) + self.problem1_tags = api.get_object_tags( + f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}" + ) + + api.tag_object( + object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + taxonomy=self.taxonomy_1, + tags=['Tag 1.2'], + ) + self.library_html_tags1 = api.get_object_tags( + object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + taxonomy_id=self.taxonomy_1.id, + ) + + api.tag_object( + object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + taxonomy=self.taxonomy_2, + tags=['Tag 2.2'], + ) + self.library_html_tags2 = api.get_object_tags( + object_id=f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + taxonomy_id=self.taxonomy_2.id, + ) + + # Create "deleted" object tags, which will be omitted from the results. + for object_id in ( + f"lib:orgA:lib_{self.block_suffix}", + f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}", + f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}", + ): + ObjectTag.objects.create( + object_id=object_id, + taxonomy=None, + tag=None, + _value="deleted tag", + _name="deleted taxonomy", + ) + + self.expected_library_objecttags = { + f"lib:orgA:lib_{self.block_suffix}": { + self.taxonomy_2.id: list(self.library_tags), + }, + f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}": { + self.taxonomy_1.id: list(self.problem1_tags), + }, + f"lb:orgA:lib_{self.block_suffix}:html:html_{self.block_suffix}": { + self.taxonomy_1.id: list(self.library_html_tags1), + self.taxonomy_2.id: list(self.library_html_tags2), + }, + } + class TestGetAllObjectTags(TestGetAllObjectTagsMixin, TestCase): """ Test get_all_object_tags api function """ - def test_get_all_object_tags(self): + def test_get_course_object_tags(self): """ - Test the get_all_object_tags function + Test the get_all_object_tags function using a course """ with self.assertNumQueries(1): object_tags, taxonomies = api.get_all_object_tags( CourseKey.from_string("course-v1:orgA+test_course+test_run") ) - assert object_tags == self.expected_objecttags + assert object_tags == self.expected_course_objecttags + assert taxonomies == { + self.taxonomy_1.id: self.taxonomy_1, + self.taxonomy_2.id: self.taxonomy_2, + } + + def test_get_library_object_tags(self): + """ + Test the get_all_object_tags function using a library + """ + with self.assertNumQueries(1): + object_tags, taxonomies = api.get_all_object_tags( + LibraryLocatorV2.from_string(f"lib:orgA:lib_{self.block_suffix}") + ) + + assert object_tags == self.expected_library_objecttags assert taxonomies == { self.taxonomy_1.id: self.taxonomy_1, self.taxonomy_2.id: self.taxonomy_2, diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 405f0543b28f..6e7a21118852 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -12,6 +12,7 @@ from django.core.validators import ValidationError, validate_email from django.utils.translation import override as override_language from django.utils.translation import gettext as _ +from eventtracking import tracker from pytz import UTC from common.djangoapps.student import views as student_views from common.djangoapps.student.models import ( @@ -679,11 +680,14 @@ def _validate_password(password, username=None, email=None, reset_password_page= (settings.ENABLE_AUTHN_RESET_PASSWORD_HIBP_POLICY and reset_password_page) or (settings.ENABLE_AUTHN_REGISTER_HIBP_POLICY and not reset_password_page) ): - pwned_response = check_pwned_password(password) - if pwned_response.get('vulnerability', 'no') == 'yes': + pwned_properties = check_pwned_password(password) + if pwned_properties.get('vulnerability', 'no') == 'yes': + if reset_password_page is False: + pwned_properties['user_request_page'] = 'registration' + tracker.emit('edx.bi.user.pwned.password.status', pwned_properties) if ( reset_password_page or - pwned_response.get('frequency', 0) >= settings.HIBP_REGISTRATION_PASSWORD_FREQUENCY_THRESHOLD + pwned_properties.get('frequency', 0) >= settings.HIBP_REGISTRATION_PASSWORD_FREQUENCY_THRESHOLD ): raise errors.AccountPasswordInvalid(accounts.AUTHN_PASSWORD_COMPROMISED_MSG) diff --git a/openedx/core/djangoapps/user_authn/api/form_fields.py b/openedx/core/djangoapps/user_authn/api/form_fields.py index 8bba651f4709..ca001ba3f1b5 100644 --- a/openedx/core/djangoapps/user_authn/api/form_fields.py +++ b/openedx/core/djangoapps/user_authn/api/form_fields.py @@ -346,7 +346,8 @@ def add_country_field(is_field_required=False): to send the field name and whether or not we want to show error message if this field is empty """ - return {'name': 'country', 'error_message': is_field_required} + error_msg = accounts.REQUIRED_FIELD_COUNTRY_MSG + return {'name': 'country', 'error_message': error_msg if is_field_required else ''} def add_confirm_email_field(is_field_required=False): diff --git a/openedx/core/djangoapps/user_authn/tasks.py b/openedx/core/djangoapps/user_authn/tasks.py index 9c2c4ef1098c..c1c781d2f26f 100644 --- a/openedx/core/djangoapps/user_authn/tasks.py +++ b/openedx/core/djangoapps/user_authn/tasks.py @@ -24,7 +24,12 @@ @shared_task @set_code_owner_attribute -def check_pwned_password_and_send_track_event(user_id, password, internal_user=False, is_new_user=False): +def check_pwned_password_and_send_track_event( + user_id, password, + internal_user=False, + is_new_user=False, + request_page='' +): """ Check the Pwned Databases and send its event to Segment. """ @@ -33,6 +38,7 @@ def check_pwned_password_and_send_track_event(user_id, password, internal_user=F if pwned_properties: pwned_properties['internal_user'] = internal_user pwned_properties['new_user'] = is_new_user + pwned_properties['user_request_page'] = request_page segment.track(user_id, 'edx.bi.user.pwned.password.status', pwned_properties) return pwned_properties except Exception: # pylint: disable=W0703 diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index cbadf2e74182..7d049871266e 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -591,7 +591,10 @@ def login_user(request, api_version='v1'): # pylint: disable=too-many-statement _handle_failed_authentication(user, possibly_authenticated_user) pwned_properties = check_pwned_password_and_send_track_event( - user.id, request.POST.get('password'), user.is_staff + user_id=user.id, + password=request.POST.get('password'), + internal_user=user.is_staff, + request_page='login' ) if not is_user_third_party_authenticated else {} # Set default for third party login password_frequency = pwned_properties.get('frequency', -1) diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index a0e56fb43862..cbb0cb49f453 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -288,7 +288,13 @@ def create_account_with_params(request, params): # pylint: disable=too-many-sta def is_new_user(password, user): if user is not None: AUDIT_LOG.info(f"Login success on new account creation - {user.username}") - check_pwned_password_and_send_track_event.delay(user.id, password, user.is_staff, True) + check_pwned_password_and_send_track_event.delay( + user_id=user.id, + password=password, + internal_user=user.is_staff, + is_new_user=True, + request_page='registration' + ) def _link_user_to_third_party_provider( diff --git a/openedx/core/djangoapps/user_authn/views/registration_form.py b/openedx/core/djangoapps/user_authn/views/registration_form.py index fb0631f0c0c6..1d1089ce0214 100644 --- a/openedx/core/djangoapps/user_authn/views/registration_form.py +++ b/openedx/core/djangoapps/user_authn/views/registration_form.py @@ -4,6 +4,7 @@ import copy from importlib import import_module +from eventtracking import tracker import re from django import forms @@ -241,12 +242,14 @@ def clean_password(self): if settings.ENABLE_AUTHN_REGISTER_HIBP_POLICY: # Checks the Pwned Databases for password vulnerability. - pwned_response = check_pwned_password(password) + pwned_properties = check_pwned_password(password) if ( - pwned_response.get('vulnerability', 'no') == 'yes' and - pwned_response.get('frequency', 0) >= settings.HIBP_REGISTRATION_PASSWORD_FREQUENCY_THRESHOLD + pwned_properties.get('vulnerability', 'no') == 'yes' and + pwned_properties.get('frequency', 0) >= settings.HIBP_REGISTRATION_PASSWORD_FREQUENCY_THRESHOLD ): + pwned_properties['user_request_page'] = 'registration' + tracker.emit('edx.bi.user.pwned.password.status', pwned_properties) raise ValidationError(accounts.AUTHN_PASSWORD_COMPROMISED_MSG) return password diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_register.py b/openedx/core/djangoapps/user_authn/views/tests/test_register.py index 3de71534f2a6..37aef7643a88 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -25,6 +25,7 @@ from openedx.core.djangoapps.user_api.accounts import ( AUTHN_EMAIL_CONFLICT_MSG, AUTHN_EMAIL_INVALID_MSG, + AUTHN_PASSWORD_COMPROMISED_MSG, AUTHN_USERNAME_CONFLICT_MSG, EMAIL_BAD_LENGTH_MSG, EMAIL_MAX_LENGTH, @@ -2320,6 +2321,40 @@ def test_logs_for_error_when_setting_is_marketable_attribute(self, set_is_market assert response.status_code == 200 + @override_settings( + ENABLE_AUTHN_REGISTER_HIBP_POLICY=True + ) + @mock.patch('eventtracking.tracker.emit') + @mock.patch( + 'openedx.core.djangoapps.user_authn.views.registration_form.check_pwned_password', + mock.Mock(return_value={ + 'vulnerability': 'yes', + 'frequency': 3, + 'user_request_page': 'registration', + }) + ) + def test_register_error_with_pwned_password(self, emit): + post_params = { + "email": self.EMAIL, + "name": self.NAME, + "username": self.USERNAME, + "password": self.PASSWORD, + "honor_code": "true", + } + response = self.client.post( + self.url, + post_params, + HTTP_ACCEPT='*/*', + ) + emit.assert_called_with( + 'edx.bi.user.pwned.password.status', + { + 'frequency': 3, + 'vulnerability': 'yes', + 'user_request_page': 'registration', + }) + assert response.status_code == 400 + @httpretty.activate @ddt.ddt @@ -2812,3 +2847,28 @@ def test_single_field_validation(self): {'username': 'user', 'email': 'user@email.com', 'is_authn_mfe': True, 'form_field_key': 'email'}, {'email': AUTHN_EMAIL_CONFLICT_MSG} ) + + @override_settings( + ENABLE_AUTHN_REGISTER_HIBP_POLICY=True + ) + @mock.patch('eventtracking.tracker.emit') + @mock.patch( + 'openedx.core.djangoapps.user_api.accounts.api.check_pwned_password', + mock.Mock(return_value={ + 'vulnerability': 'yes', + 'frequency': 3, + 'user_request_page': 'registration', + }) + ) + def test_pwned_password_and_emit_track_event(self, emit): + self.assertValidationDecision( + {'password': 'testtest12'}, + {'password': AUTHN_PASSWORD_COMPROMISED_MSG} + ) + emit.assert_called_with( + 'edx.bi.user.pwned.password.status', + { + 'frequency': 3, + 'vulnerability': 'yes', + 'user_request_page': 'registration', + }) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 2f590d061c92..35699ba6318c 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -23,7 +23,7 @@ click>=8.0,<9.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.13.0 +edx-enterprise==4.13.2 # 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 6f07c9862b35..ac88fc83e24a 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -477,7 +477,7 @@ edx-drf-extensions==10.2.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.13.0 +edx-enterprise==4.13.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index fd114e017f59..187aa5919e59 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -751,7 +751,7 @@ edx-drf-extensions==10.2.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.13.0 +edx-enterprise==4.13.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 6b304bf66eb2..2b0096697caf 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -551,7 +551,7 @@ edx-drf-extensions==10.2.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.13.0 +edx-enterprise==4.13.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index c9c7790d00e4..c37e638f047e 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -577,7 +577,7 @@ edx-drf-extensions==10.2.0 # edx-when # edxval # openedx-learning -edx-enterprise==4.13.0 +edx-enterprise==4.13.2 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/xmodule/modulestore/split_mongo/mongo_connection.py b/xmodule/modulestore/split_mongo/mongo_connection.py index 964bb238c663..bfb20fe0f5d5 100644 --- a/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/xmodule/modulestore/split_mongo/mongo_connection.py @@ -701,7 +701,9 @@ def insert_course_index(self, course_index, course_context=None): # pylint: dis course_index['last_update'] = datetime.datetime.now(pytz.utc) new_index = SplitModulestoreCourseIndex(**SplitModulestoreCourseIndex.fields_from_v1_schema(course_index)) new_index.save() - # TEMP: Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well: + # Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well. + # NOTE: This is REQUIRED for pruning (structures.py) to run safely. Don't remove this write until + # pruning is modified to read from SplitModulestoreCourseIndex to get active versions. super().insert_course_index(course_index, course_context) def update_course_index(self, course_index, from_index=None, course_context=None): # pylint: disable=arguments-differ @@ -755,7 +757,10 @@ def update_course_index(self, course_index, from_index=None, course_context=None # Save the course index entry and create a historical record: index_obj.save() - # TEMP: Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well: + + # Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well. + # NOTE: This is REQUIRED for pruning (structures.py) to run safely. Don't remove this write until + # pruning is modified to read from SplitModulestoreCourseIndex to get active versions. super().update_course_index(course_index, from_index, course_context) def delete_course_index(self, course_key): @@ -764,7 +769,9 @@ def delete_course_index(self, course_key): """ RequestCache(namespace="course_index_cache").clear() SplitModulestoreCourseIndex.objects.filter(course_id=course_key).delete() - # TEMP: Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well: + # Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well. + # NOTE: This is REQUIRED for pruning (structures.py) to run safely. Don't remove this write until + # pruning is modified to read from SplitModulestoreCourseIndex to get active versions. super().delete_course_index(course_key) def _drop_database(self, database=True, collections=True, connections=True): @@ -782,5 +789,7 @@ def _drop_database(self, database=True, collections=True, connections=True): "post-test cleanup failed with TransactionManagementError. " "Use 'with self.allow_transaction_exception():' from ModuleStoreTestCase/...IsolationMixin to fix it." ) from err - # TEMP: Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well: + # Also write to MongoDB, so we can switch back to using it if this new MySQL version doesn't work well. + # NOTE: This is REQUIRED for pruning (structures.py) to run safely. Don't remove this write until + # pruning is modified to read from SplitModulestoreCourseIndex to get active versions. super()._drop_database(database, collections, connections)