Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use config settings for olxcleaner (#27294)

Browse Source 
* Use config settings for olxcleaner

Use config settings instead of hardcoded values for olx validation. This would help in adding a great deal of control when you want to change these settings in the future. With this approch we would not need a redeploy.

* Use configs and deprecate waffleflag and also add / update tests
This commit is contained in:
Awais Jibran
2021-04-12 16:27:42 +05:00
committed by GitHub
parent e3a449d63e
commit 30c80082c3
8 changed files with 67 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
cms/djangoapps/contentstore/tasks.py
View File

@@ -64,7 +64,7 @@ from xmodule.modulestore.xml_exporter import export_course_to_xml, export_librar
from xmodule.modulestore.xml_importer import import_course_from_xml, import_library_from_xml
from .outlines import update_outline_from_modulestore
from .toggles import course_import_olx_validation_is_enabled
from .utils import course_import_olx_validation_is_enabled
User = get_user_model()
@@ -688,7 +688,12 @@ def validate_course_olx(courselike_key, course_dir, status):
if not course_import_olx_validation_is_enabled():
return olx_is_valid
try:
__, errorstore, __ = olxcleaner.validate(course_dir, steps=8, allowed_xblocks=ALL_ALLOWED_XBLOCKS)
__, errorstore, __ = olxcleaner.validate(
filename=course_dir,
steps=settings.COURSE_OLX_VALIDATION_STAGE,
ignore=settings.COURSE_OLX_VALIDATION_IGNORE_LIST,
allowed_xblocks=ALL_ALLOWED_XBLOCKS
)
except Exception: # pylint: disable=broad-except
LOGGER.exception(f'{log_prefix}: CourseOlx Could not be validated')
return olx_is_valid

36
cms/djangoapps/contentstore/tests/test_utils.py
View File

@@ -3,8 +3,10 @@ import collections
from datetime import datetime, timedelta
from uuid import uuid4
import ddt
from django.conf import settings
from django.test import TestCase
from edx_toggles.toggles.testutils import override_waffle_flag
from django.test.utils import override_settings
from mock import Mock, mock, patch
from opaque_keys.edx.locator import CourseLocator, LibraryLocator
from path import Path as path
@@ -12,9 +14,8 @@ from pytz import UTC
from user_tasks.models import UserTaskArtifact, UserTaskStatus
from cms.djangoapps.contentstore import utils
from cms.djangoapps.contentstore.tasks import validate_course_olx
from cms.djangoapps.contentstore.tasks import ALL_ALLOWED_XBLOCKS, 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
@@ -32,7 +33,7 @@ class LMSLinksTestCase(TestCase):
location = course_key.make_usage_key('vertical', 'contacting_us')
link = utils.get_lms_link_for_item(location, False)
self.assertEqual(link, "//localhost:8000/courses/course-v1:mitX+101+test/jump_to/block-v1:mitX+101+test+type"
"@vertical+block@contacting_us")
"@vertical+block@contacting_us")
# test preview
link = utils.get_lms_link_for_item(location, True)
@@ -46,7 +47,7 @@ class LMSLinksTestCase(TestCase):
location = course_key.make_usage_key('course', 'test')
link = utils.get_lms_link_for_item(location)
self.assertEqual(link, "//localhost:8000/courses/course-v1:mitX+101+test/jump_to/block-v1:mitX+101+test+type"
"@course+block@test")
"@course+block@test")
def lms_link_for_certificate_web_view_test(self):
""" Tests get_lms_link_for_certificate_web_view. """
@@ -623,8 +624,9 @@ class GetUserPartitionInfoTest(ModuleStoreTestCase):
return utils.get_user_partition_info(self.block, schemes=schemes)
@override_waffle_flag(COURSE_IMPORT_OLX_VALIDATION, active=True)
@patch.dict(settings.FEATURES, ENABLE_COURSE_OLX_VALIDATION=True)
@mock.patch('olxcleaner.validate')
@ddt.ddt
class ValidateCourseOlxTests(CourseTestCase):
"""Tests for olx validation"""
@@ -636,7 +638,6 @@ class ValidateCourseOlxTests(CourseTestCase):
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):
"""
@@ -650,7 +651,7 @@ class ValidateCourseOlxTests(CourseTestCase):
"""
Tests olx validation in case of waffle flag is off.
"""
with override_waffle_flag(self.waffle_flg, active=False):
with patch.dict(settings.FEATURES, ENABLE_COURSE_OLX_VALIDATION=False):
self.assertTrue(validate_course_olx(self.course.id, self.toy_course_path, self.status))
self.assertFalse(mock_olxcleaner_validate.called)
@@ -661,7 +662,6 @@ class ValidateCourseOlxTests(CourseTestCase):
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))
@@ -708,3 +708,21 @@ class ValidateCourseOlxTests(CourseTestCase):
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)
def test_validate_calls_with(self, mock_olxcleaner_validate):
"""
Tests that olx library is called with expected keyword arguments.
"""
allowed_xblocks = ALL_ALLOWED_XBLOCKS
steps = 2
ignore = ['edx-xblock']
mock_olxcleaner_validate.return_value = [Mock(), Mock(errors=[], return_error=Mock()), Mock()]
with override_settings(COURSE_OLX_VALIDATION_STAGE=steps, COURSE_OLX_VALIDATION_IGNORE_LIST=ignore):
validate_course_olx(self.course.id, self.toy_course_path, self.status)
mock_olxcleaner_validate.assert_called_with(
filename=self.toy_course_path,
steps=steps,
ignore=ignore,
allowed_xblocks=allowed_xblocks
)

21
cms/djangoapps/contentstore/toggles.py
View File

@@ -35,30 +35,9 @@ SPLIT_LIBRARY_ON_DASHBOARD = LegacyWaffleFlag(
module_name=__name__
)
# .. toggle_name: course_import_olx_validation
# .. toggle_implementation: WaffleFlag
# .. toggle_default: False
# .. toggle_description: Enables olx validation during course import.
# .. toggle_use_cases: open_edx
# .. toggle_creation_date: 2021-04-01
# .. toggle_target_removal_date: 2021-05-01
# .. toggle_tickets: TNL-8151
COURSE_IMPORT_OLX_VALIDATION = LegacyWaffleFlag(
waffle_namespace=LegacyWaffleFlagNamespace(name=WAFFLE_NAMESPACE),
flag_name='course_import_olx_validation',
module_name=__name__
)
def split_library_view_on_dashboard():
"""
check if data new view for library is enabled on studio dashboard.
"""
return SPLIT_LIBRARY_ON_DASHBOARD.is_enabled()
def course_import_olx_validation_is_enabled():
"""
Check if course olx validation is enabled on course import.
"""
return COURSE_IMPORT_OLX_VALIDATION.is_enabled()

7
cms/djangoapps/contentstore/utils.py
View File

@@ -171,6 +171,13 @@ def get_proctored_exam_settings_url(course_module):
return course_authoring_microfrontend_url
def course_import_olx_validation_is_enabled():
"""
Check if course olx validation is enabled on course import.
"""
return settings.FEATURES.get('ENABLE_COURSE_OLX_VALIDATION', False)
# pylint: disable=invalid-name
def is_currently_visible_to_students(xblock):
"""

7
cms/envs/common.py
View File

@@ -355,7 +355,8 @@ FEATURES = {
'ENABLE_READING_FROM_MULTIPLE_HISTORY_TABLES': True,
'SHOW_FOOTER_LANGUAGE_SELECTOR': False,
'ENABLE_ENROLLMENT_RESET': False,
# Settings for course import olx validation
'ENABLE_COURSE_OLX_VALIDATION': True,
# .. toggle_name: FEATURES['DISABLE_MOBILE_COURSE_AVAILABLE']
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
@@ -2416,3 +2417,7 @@ LOGO_URL_PNG = None
LOGO_TRADEMARK_URL = None
FAVICON_URL = None
DEFAULT_EMAIL_LOGO_URL = 'https://edx-cdn.org/v3/default/logo.png'
############## Settings for course import olx validation ############################
COURSE_OLX_VALIDATION_STAGE = 1
COURSE_OLX_VALIDATION_IGNORE_LIST = None

7
cms/envs/production.py
View File

@@ -601,3 +601,10 @@ EXPLICIT_QUEUES = {
}
LOGO_IMAGE_EXTRA_TEXT = ENV_TOKENS.get('LOGO_IMAGE_EXTRA_TEXT', '')
############## Settings for course import olx validation ############################
COURSE_OLX_VALIDATION_STAGE = ENV_TOKENS.get('COURSE_OLX_VALIDATION_STAGE', COURSE_OLX_VALIDATION_STAGE)
COURSE_OLX_VALIDATION_IGNORE_LIST = ENV_TOKENS.get(
'COURSE_OLX_VALIDATION_IGNORE_LIST',
COURSE_OLX_VALIDATION_IGNORE_LIST
)

7
lms/envs/common.py
View File

@@ -65,7 +65,6 @@ CC_MERCHANT_NAME = PLATFORM_NAME
PLATFORM_FACEBOOK_ACCOUNT = "http://www.facebook.com/YourPlatformFacebookAccount"
PLATFORM_TWITTER_ACCOUNT = "@YourPlatformTwitterAccount"
ENABLE_JASMINE = False
LMS_ROOT_URL = 'https://localhost:18000'
@@ -311,6 +310,8 @@ FEATURES = {
# Toggle to enable certificates of courses on dashboard
'ENABLE_VERIFIED_CERTIFICATES': False,
# Settings for course import olx validation
'ENABLE_COURSE_OLX_VALIDATION': True,
# .. toggle_name: FEATURES['DISABLE_HONOR_CERTIFICATES']
# .. toggle_implementation: DjangoSetting
@@ -4672,3 +4673,7 @@ LOGO_URL_PNG = None
LOGO_TRADEMARK_URL = None
FAVICON_URL = None
DEFAULT_EMAIL_LOGO_URL = 'https://edx-cdn.org/v3/default/logo.png'
################# Settings for olx validation. #################
COURSE_OLX_VALIDATION_STAGE = 1
COURSE_OLX_VALIDATION_IGNORE_LIST = None

7
lms/envs/production.py
View File

@@ -1051,3 +1051,10 @@ LOGO_IMAGE_EXTRA_TEXT = ENV_TOKENS.get('LOGO_IMAGE_EXTRA_TEXT', '')
############## XBlock extra mixins ############################
XBLOCK_MIXINS += tuple(XBLOCK_EXTRA_MIXINS)
############## Settings for course import olx validation ############################
COURSE_OLX_VALIDATION_STAGE = ENV_TOKENS.get('COURSE_OLX_VALIDATION_STAGE', COURSE_OLX_VALIDATION_STAGE)
COURSE_OLX_VALIDATION_IGNORE_LIST = ENV_TOKENS.get(
'COURSE_OLX_VALIDATION_IGNORE_LIST',
COURSE_OLX_VALIDATION_IGNORE_LIST
)