diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 0d1dbbff4a..8dc2352edd 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -662,14 +662,20 @@ def update_outline_from_modulestore_task(course_key_str): def validate_course_olx(courselike_key, course_dir, status): """ - Validates course olx and records the errors as artifact. + Validates course olx and records the errors as an artifact. + Arguments: + courselike_key: A locator identifies a course resource. course_dir: complete path to the course olx status: UserTaskStatus object. """ + is_library = isinstance(courselike_key, LibraryLocator) olx_is_valid = True log_prefix = f'Course import {courselike_key}' + if is_library: + return olx_is_valid + if not course_import_olx_validation_is_enabled(): return olx_is_valid try: diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 97e6fd492d..41ac9c6b8e 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -1,15 +1,20 @@ """ Tests for utils. """ - - import collections from datetime import datetime, timedelta +from uuid import uuid4 from django.test import TestCase -from opaque_keys.edx.locator import CourseLocator +from edx_toggles.toggles.testutils import override_waffle_flag +from mock import Mock, mock, patch +from opaque_keys.edx.locator import CourseLocator, LibraryLocator +from path import Path as path from pytz import UTC +from user_tasks.models import UserTaskArtifact, UserTaskStatus from cms.djangoapps.contentstore import utils -from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from cms.djangoapps.contentstore.tasks import validate_course_olx +from cms.djangoapps.contentstore.tests.utils import TEST_DATA_DIR, CourseTestCase +from cms.djangoapps.contentstore.toggles import COURSE_IMPORT_OLX_VALIDATION from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration_context from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore @@ -616,3 +621,90 @@ class GetUserPartitionInfoTest(ModuleStoreTestCase): def _get_partition_info(self, schemes=None): """Retrieve partition info and selected groups. """ return utils.get_user_partition_info(self.block, schemes=schemes) + + +@override_waffle_flag(COURSE_IMPORT_OLX_VALIDATION, active=True) +@mock.patch('olxcleaner.validate') +class ValidateCourseOlxTests(CourseTestCase): + """Tests for olx validation""" + + def setUp(self): + super().setUp() + self.LOGGER = 'cms.djangoapps.contentstore.tasks.LOGGER' + self.data_dir = path(TEST_DATA_DIR) + self.toy_course_path = self.data_dir / 'course_ignore' + self.status = UserTaskStatus.objects.create( + user=self.user, task_id=str(uuid4()), task_class='sample_task', name='CourseImport', total_steps=4 + ) + self.waffle_flg = COURSE_IMPORT_OLX_VALIDATION + + def test_with_library_locator(self, mock_olxcleaner_validate): + """ + Tests that olx is validation is skipped with library locator. + """ + library_key = LibraryLocator(org='TestOrg', library='TestProbs') + self.assertTrue(validate_course_olx(library_key, self.toy_course_path, self.status)) + self.assertFalse(mock_olxcleaner_validate.called) + + def test_waffle_flag_settings(self, mock_olxcleaner_validate): + """ + Tests olx validation in case of waffle flag is off. + """ + with override_waffle_flag(self.waffle_flg, active=False): + self.assertTrue(validate_course_olx(self.course.id, self.toy_course_path, self.status)) + self.assertFalse(mock_olxcleaner_validate.called) + + def test_exception_during_validation(self, mock_olxcleaner_validate): + """ + Tests olx validation in case of unexpected error. + + In case of any unexpected exception during the olx validation, + the course import continues and information is logged on the server. + """ + + mock_olxcleaner_validate.side_effect = Exception + with mock.patch(self.LOGGER) as patched_log: + self.assertTrue(validate_course_olx(self.course.id, self.toy_course_path, self.status)) + self.assertTrue(mock_olxcleaner_validate.called) + patched_log.exception.assert_called_once_with( + f'Course import {self.course.id}: CourseOlx Could not be validated') + + def test_no_errors(self, mock_olxcleaner_validate): + """ + Tests olx validation with no errors. + Verify that in case for no validation errors, no artifact object is created. + """ + mock_olxcleaner_validate.return_value = [ + Mock(), + Mock(errors=[], return_error=Mock(return_value=False)), + Mock() + ] + self.assertTrue(validate_course_olx(self.course.id, self.toy_course_path, self.status)) + task_artifact = UserTaskArtifact.objects.filter(status=self.status, name='OLX_VALIDATION_ERROR').first() + self.assertIsNone(task_artifact) + self.assertTrue(mock_olxcleaner_validate.called) + + @mock.patch('cms.djangoapps.contentstore.tasks.report_error_summary') + @mock.patch('cms.djangoapps.contentstore.tasks.report_errors') + def test_creates_artifact(self, mock_report_errors, mock_report_error_summary, mock_olxcleaner_validate): + """ + Tests olx validation in case of errors. + Verify that in case of olx validation errors, course import does not fail & errors + are logged in task artifact. + """ + errors = [Mock(description='DuplicateURLNameError', level_val=3)] + + mock_olxcleaner_validate.return_value = [ + Mock(), + Mock(errors=errors, return_error=Mock(return_value=True)), + Mock() + ] + mock_report_errors.return_value = [f'ERROR {error.description} found in content' for error in errors] + mock_report_error_summary.return_value = [f'Errors: {len(errors)}'] + + with patch(self.LOGGER) as patched_log: + self.assertTrue(validate_course_olx(self.course.id, self.toy_course_path, self.status)) + patched_log.error.assert_called_once_with( + f'Course import {self.course.id}: CourseOlx validation failed') + task_artifact = UserTaskArtifact.objects.filter(status=self.status, name='OLX_VALIDATION_ERROR').first() + self.assertIsNotNone(task_artifact)