Merge pull request #10215 from edx/ziafazal/SOL-1317

SOL-1317: check MILESTONES_APP also when checking prerequisite courses enabled

cms/djangoapps/contentstore/views/course.py

@@ -87,7 +87,8 @@ from student.auth import has_course_author_access
 
 from util.milestones_helpers import (
     set_prerequisite_courses,
-    is_valid_course_key
+    is_valid_course_key,
+    is_prerequisite_courses_enabled
 )
 
 log = logging.getLogger(__name__)
@@ -894,7 +895,6 @@ def settings_handler(request, course_key_string):
         json: update the Course and About xblocks through the CourseDetails model
     """
     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)
     with modulestore().bulk_operations(course_key):
         course_module = get_course_and_check_access(course_key, request.user)
@@ -911,7 +911,6 @@ def settings_handler(request, course_key_string):
 
             about_page_editable = 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)
             settings_context = {
                 'context_course': course_module,
@@ -928,8 +927,9 @@ def settings_handler(request, course_key_string):
                 'is_credit_course': False,
                 'show_min_grade_warning': False,
                 '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)
                 # exclude current course from the list of available courses
                 courses = [course for course in courses if course.id != course_key]
@@ -970,7 +970,7 @@ def settings_handler(request, course_key_string):
             # For every other possible method type submitted by the caller...
             else:
                 # 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', [])
                     if prerequisite_course_keys:
                         if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys):

cms/templates/settings.html

@@ -376,7 +376,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
                   <input type="text" class="short time" id="course-effort" placeholder="HH:MM" />
                   <span class="tip tip-inline">${_("Time spent on all course work")}</span>
                 </li>
-                % if settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES'):
+                % if is_prerequisite_courses_enabled:
                 <li class="field field-select" id="field-pre-requisite-course">
                     <label for="pre-requisite-course">${_("Prerequisite Course")}</label>
                     <select class="input" id="pre-requisite-course">

common/djangoapps/util/milestones_helpers.py

@@ -24,6 +24,14 @@ def get_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):
     """
     It would create a milestone, then it would set newly created
@@ -31,7 +39,7 @@ def add_prerequisite_course(course_key, prerequisite_course_key):
     and it would set newly created milestone as fulfillment
     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
     from milestones import api as milestones_api
     milestone_name = _('Course {course_id} requires {prerequisite_course_id}').format(
@@ -55,7 +63,7 @@ def remove_prerequisite_course(course_key, milestone):
     It would remove pre-requisite course milestone for course
     referred by `course_key`.
     """
-    if not settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False):
+    if not is_prerequisite_courses_enabled():
         return None
     from milestones import api as milestones_api
     milestones_api.remove_course_milestone(
@@ -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
     None as `prerequisite_course_keys` .
     """
-    if not settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False):
+    if not is_prerequisite_courses_enabled():
         return None
     from milestones import api as milestones_api
     #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:
         If a course has no incomplete prerequisites, it will be excluded from the
         dictionary.
     """
-    if not settings.FEATURES.get('ENABLE_PREREQUISITE_COURSES', False):
+    if not is_prerequisite_courses_enabled():
         return {}
 
     from milestones import api as milestones_api
@@ -137,7 +145,7 @@ def get_prerequisite_courses_display(course_descriptor):
     and course display name as `display` field.
     """
     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:
             course_key = CourseKey.from_string(course_id)
             required_course_descriptor = modulestore().get_course(course_key)

common/djangoapps/util/tests/test_milestones_helpers.py

@@ -2,6 +2,7 @@
 Tests for the milestones helpers library, which is the integration point for the edx_milestones API
 """
 
+import ddt
 from mock import patch
 
 from milestones.exceptions import InvalidCourseKeyException, InvalidUserException
@@ -11,6 +12,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
 
 
 @patch.dict('django.conf.settings.FEATURES', {'MILESTONES_APP': False})
+@ddt.ddt
 class MilestonesHelpersTestCase(ModuleStoreTestCase):
     """
     Main test suite for Milestones API client library
@@ -35,6 +37,24 @@ class MilestonesHelpersTestCase(ModuleStoreTestCase):
             '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):
         response = milestones_helpers.add_milestone(milestone_data=self.milestone)
         self.assertIsNone(response)

lms/djangoapps/certificates/models.py

@@ -64,7 +64,7 @@ from model_utils.models import TimeStampedModel
 from xmodule.modulestore.django import modulestore
 from config_models.models import ConfigurationModel
 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
 
 LOGGER = logging.getLogger(__name__)
@@ -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'.
     """
     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)
 
 

lms/djangoapps/courseware/access.py

@@ -50,6 +50,7 @@ from student.roles import (
 from util.milestones_helpers import (
     get_pre_requisite_courses_not_completed,
     any_unfulfilled_milestones,
+    is_prerequisite_courses_enabled,
 )
 from ccx_keys.locator import CCXLocator
 
@@ -211,7 +212,7 @@ def _can_view_courseware_with_prerequisites(user, course):  # pylint: disable=in
         """
         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 (
         _is_prerequisites_disabled()