Commit 0f637fdc by Zia Fazal

check MILESTONES_APP when checking prerequisite courses enabled

parent d26d03da
...@@ -87,7 +87,8 @@ from student.auth import has_course_author_access ...@@ -87,7 +87,8 @@ from student.auth import has_course_author_access
from util.milestones_helpers import ( from util.milestones_helpers import (
set_prerequisite_courses, set_prerequisite_courses,
is_valid_course_key is_valid_course_key,
is_prerequisite_courses_enabled
) )
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -894,7 +895,6 @@ def settings_handler(request, course_key_string): ...@@ -894,7 +895,6 @@ def settings_handler(request, course_key_string):
json: update the Course and About xblocks through the CourseDetails model json: update the Course and About xblocks through the CourseDetails model
""" """
course_key = CourseKey.from_string(course_key_string) course_key = CourseKey.from_string(course_key_string)
prerequisite_course_enabled = settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False)
credit_eligibility_enabled = settings.FEATURES.get('ENABLE_CREDIT_ELIGIBILITY', False) credit_eligibility_enabled = settings.FEATURES.get('ENABLE_CREDIT_ELIGIBILITY', False)
with modulestore().bulk_operations(course_key): with modulestore().bulk_operations(course_key):
course_module = get_course_and_check_access(course_key, request.user) course_module = get_course_and_check_access(course_key, request.user)
...@@ -911,7 +911,6 @@ def settings_handler(request, course_key_string): ...@@ -911,7 +911,6 @@ def settings_handler(request, course_key_string):
about_page_editable = not marketing_site_enabled about_page_editable = not marketing_site_enabled
enrollment_end_editable = GlobalStaff().has_user(request.user) or not marketing_site_enabled enrollment_end_editable = GlobalStaff().has_user(request.user) or not marketing_site_enabled
short_description_editable = settings.FEATURES.get('EDITABLE_SHORT_DESCRIPTION', True) short_description_editable = settings.FEATURES.get('EDITABLE_SHORT_DESCRIPTION', True)
settings_context = { settings_context = {
'context_course': course_module, 'context_course': course_module,
...@@ -928,8 +927,9 @@ def settings_handler(request, course_key_string): ...@@ -928,8 +927,9 @@ def settings_handler(request, course_key_string):
'is_credit_course': False, 'is_credit_course': False,
'show_min_grade_warning': False, 'show_min_grade_warning': False,
'enrollment_end_editable': enrollment_end_editable, 'enrollment_end_editable': enrollment_end_editable,
'is_prerequisite_courses_enabled': is_prerequisite_courses_enabled()
} }
if prerequisite_course_enabled: if is_prerequisite_courses_enabled():
courses, in_process_course_actions = get_courses_accessible_to_user(request) courses, in_process_course_actions = get_courses_accessible_to_user(request)
# exclude current course from the list of available courses # exclude current course from the list of available courses
courses = [course for course in courses if course.id != course_key] courses = [course for course in courses if course.id != course_key]
...@@ -970,7 +970,7 @@ def settings_handler(request, course_key_string): ...@@ -970,7 +970,7 @@ def settings_handler(request, course_key_string):
# For every other possible method type submitted by the caller... # For every other possible method type submitted by the caller...
else: else:
# if pre-requisite course feature is enabled set pre-requisite course # if pre-requisite course feature is enabled set pre-requisite course
if prerequisite_course_enabled: if is_prerequisite_courses_enabled():
prerequisite_course_keys = request.json.get('pre_requisite_courses', []) prerequisite_course_keys = request.json.get('pre_requisite_courses', [])
if prerequisite_course_keys: if prerequisite_course_keys:
if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys): if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys):
......
...@@ -376,7 +376,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}'; ...@@ -376,7 +376,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
<input type="text" class="short time" id="course-effort" placeholder="HH:MM" /> <input type="text" class="short time" id="course-effort" placeholder="HH:MM" />
<span class="tip tip-inline">${_("Time spent on all course work")}</span> <span class="tip tip-inline">${_("Time spent on all course work")}</span>
</li> </li>
% if settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES'): % if is_prerequisite_courses_enabled:
<li class="field field-select" id="field-pre-requisite-course"> <li class="field field-select" id="field-pre-requisite-course">
<label for="pre-requisite-course">${_("Prerequisite Course")}</label> <label for="pre-requisite-course">${_("Prerequisite Course")}</label>
<select class="input" id="pre-requisite-course"> <select class="input" id="pre-requisite-course">
......
...@@ -24,6 +24,14 @@ def get_namespace_choices(): ...@@ -24,6 +24,14 @@ def get_namespace_choices():
return NAMESPACE_CHOICES return NAMESPACE_CHOICES
def is_prerequisite_courses_enabled():
"""
Returns boolean indicating prerequisite courses enabled system wide or not.
"""
return settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False) \
and settings.FEATURES.get('MILESTONES_APP', False)
def add_prerequisite_course(course_key, prerequisite_course_key): def add_prerequisite_course(course_key, prerequisite_course_key):
""" """
It would create a milestone, then it would set newly created It would create a milestone, then it would set newly created
...@@ -31,7 +39,7 @@ def add_prerequisite_course(course_key, prerequisite_course_key): ...@@ -31,7 +39,7 @@ def add_prerequisite_course(course_key, prerequisite_course_key):
and it would set newly created milestone as fulfillment and it would set newly created milestone as fulfillment
milestone for course referred by `prerequisite_course_key`. milestone for course referred by `prerequisite_course_key`.
""" """
if not settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False): if not is_prerequisite_courses_enabled():
return None return None
from milestones import api as milestones_api from milestones import api as milestones_api
milestone_name = _('Course {course_id} requires {prerequisite_course_id}').format( milestone_name = _('Course {course_id} requires {prerequisite_course_id}').format(
...@@ -55,7 +63,7 @@ def remove_prerequisite_course(course_key, milestone): ...@@ -55,7 +63,7 @@ def remove_prerequisite_course(course_key, milestone):
It would remove pre-requisite course milestone for course It would remove pre-requisite course milestone for course
referred by `course_key`. referred by `course_key`.
""" """
if not settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False): if not is_prerequisite_courses_enabled():
return None return None
from milestones import api as milestones_api from milestones import api as milestones_api
milestones_api.remove_course_milestone( milestones_api.remove_course_milestone(
...@@ -71,7 +79,7 @@ def set_prerequisite_courses(course_key, prerequisite_course_keys): ...@@ -71,7 +79,7 @@ def set_prerequisite_courses(course_key, prerequisite_course_keys):
To only remove course milestones pass `course_key` and empty list or To only remove course milestones pass `course_key` and empty list or
None as `prerequisite_course_keys` . None as `prerequisite_course_keys` .
""" """
if not settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False): if not is_prerequisite_courses_enabled():
return None return None
from milestones import api as milestones_api from milestones import api as milestones_api
#remove any existing requirement milestones with this pre-requisite course as requirement #remove any existing requirement milestones with this pre-requisite course as requirement
...@@ -104,7 +112,7 @@ def get_pre_requisite_courses_not_completed(user, enrolled_courses): # pylint: ...@@ -104,7 +112,7 @@ def get_pre_requisite_courses_not_completed(user, enrolled_courses): # pylint:
If a course has no incomplete prerequisites, it will be excluded from the If a course has no incomplete prerequisites, it will be excluded from the
dictionary. dictionary.
""" """
if not settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False): if not is_prerequisite_courses_enabled():
return {} return {}
from milestones import api as milestones_api from milestones import api as milestones_api
...@@ -137,7 +145,7 @@ def get_prerequisite_courses_display(course_descriptor): ...@@ -137,7 +145,7 @@ def get_prerequisite_courses_display(course_descriptor):
and course display name as `display` field. and course display name as `display` field.
""" """
pre_requisite_courses = [] pre_requisite_courses = []
if settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False) and course_descriptor.pre_requisite_courses: if is_prerequisite_courses_enabled() and course_descriptor.pre_requisite_courses:
for course_id in course_descriptor.pre_requisite_courses: for course_id in course_descriptor.pre_requisite_courses:
course_key = CourseKey.from_string(course_id) course_key = CourseKey.from_string(course_id)
required_course_descriptor = modulestore().get_course(course_key) required_course_descriptor = modulestore().get_course(course_key)
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
Tests for the milestones helpers library, which is the integration point for the edx_milestones API Tests for the milestones helpers library, which is the integration point for the edx_milestones API
""" """
import ddt
from mock import patch from mock import patch
from milestones.exceptions import InvalidCourseKeyException, InvalidUserException from milestones.exceptions import InvalidCourseKeyException, InvalidUserException
...@@ -11,6 +12,7 @@ from xmodule.modulestore.tests.factories import CourseFactory ...@@ -11,6 +12,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
@patch.dict('django.conf.settings.FEATURES', {'MILESTONES_APP': False}) @patch.dict('django.conf.settings.FEATURES', {'MILESTONES_APP': False})
@ddt.ddt
class MilestonesHelpersTestCase(ModuleStoreTestCase): class MilestonesHelpersTestCase(ModuleStoreTestCase):
""" """
Main test suite for Milestones API client library Main test suite for Milestones API client library
...@@ -35,6 +37,24 @@ class MilestonesHelpersTestCase(ModuleStoreTestCase): ...@@ -35,6 +37,24 @@ class MilestonesHelpersTestCase(ModuleStoreTestCase):
'description': 'Testing Milestones Helpers Library', 'description': 'Testing Milestones Helpers Library',
} }
@ddt.data(
(False, False, False),
(True, False, False),
(False, True, False),
(True, True, True),
)
def test_pre_requisite_courses_enabled(self, feature_flags):
"""
Tests is_prerequisite_courses_enabled function with a set of possible values for
ENABLE_PREREQUISITE_COURSES and MILESTONES_APP feature flags.
"""
with patch.dict("django.conf.settings.FEATURES", {
'ENABLE_PREREQUISITE_COURSES': feature_flags[0],
'MILESTONES_APP': feature_flags[1]
}):
self.assertEqual(feature_flags[2], milestones_helpers.is_prerequisite_courses_enabled())
def test_add_milestone_returns_none_when_app_disabled(self): def test_add_milestone_returns_none_when_app_disabled(self):
response = milestones_helpers.add_milestone(milestone_data=self.milestone) response = milestones_helpers.add_milestone(milestone_data=self.milestone)
self.assertIsNone(response) self.assertIsNone(response)
......
...@@ -64,7 +64,7 @@ from model_utils.models import TimeStampedModel ...@@ -64,7 +64,7 @@ from model_utils.models import TimeStampedModel
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from config_models.models import ConfigurationModel from config_models.models import ConfigurationModel
from xmodule_django.models import CourseKeyField, NoneToEmptyManager from xmodule_django.models import CourseKeyField, NoneToEmptyManager
from util.milestones_helpers import fulfill_course_milestone from util.milestones_helpers import fulfill_course_milestone, is_prerequisite_courses_enabled
from course_modes.models import CourseMode from course_modes.models import CourseMode
LOGGER = logging.getLogger(__name__) LOGGER = logging.getLogger(__name__)
...@@ -161,7 +161,7 @@ def handle_post_cert_generated(sender, instance, **kwargs): # pylint: disable=n ...@@ -161,7 +161,7 @@ def handle_post_cert_generated(sender, instance, **kwargs): # pylint: disable=n
User is assumed to have passed the course if certificate status is either 'generating' or 'downloadable'. User is assumed to have passed the course if certificate status is either 'generating' or 'downloadable'.
""" """
allowed_cert_states = [CertificateStatuses.generating, CertificateStatuses.downloadable] allowed_cert_states = [CertificateStatuses.generating, CertificateStatuses.downloadable]
if settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES') and instance.status in allowed_cert_states: if is_prerequisite_courses_enabled() and instance.status in allowed_cert_states:
fulfill_course_milestone(instance.course_id, instance.user) fulfill_course_milestone(instance.course_id, instance.user)
......
...@@ -50,6 +50,7 @@ from student.roles import ( ...@@ -50,6 +50,7 @@ from student.roles import (
from util.milestones_helpers import ( from util.milestones_helpers import (
get_pre_requisite_courses_not_completed, get_pre_requisite_courses_not_completed,
any_unfulfilled_milestones, any_unfulfilled_milestones,
is_prerequisite_courses_enabled,
) )
from ccx_keys.locator import CCXLocator from ccx_keys.locator import CCXLocator
...@@ -211,7 +212,7 @@ def _can_view_courseware_with_prerequisites(user, course): # pylint: disable=in ...@@ -211,7 +212,7 @@ def _can_view_courseware_with_prerequisites(user, course): # pylint: disable=in
""" """
Checks if prerequisites are disabled in the settings. Checks if prerequisites are disabled in the settings.
""" """
return ACCESS_DENIED if settings.FEATURES['ENABLE_PREREQUISITE_COURSES'] else ACCESS_GRANTED return ACCESS_DENIED if is_prerequisite_courses_enabled() else ACCESS_GRANTED
return ( return (
_is_prerequisites_disabled() _is_prerequisites_disabled()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment