Merge pull request #7320 from edx/mattdrayer/add-milestones-app-check

Cleaned up milestones API references

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -6,6 +6,7 @@ import json
 import copy
 import mock
 from mock import patch
+import unittest
 
 from django.utils.timezone import UTC
 from django.test.utils import override_settings
@@ -139,19 +140,9 @@ class CourseDetailsTestCase(CourseTestCase):
             self.assertNotContains(response, "Course Introduction Video")
             self.assertNotContains(response, "Requirements")
 
-    def _seed_milestone_relationship_types(self):
-        """
-        Helper method to prepopulate MRTs so the tests can run
-        Note the settings check -- exams feature must be enabled for the tests to run correctly
-        """
-        if settings.FEATURES.get('ENTRANCE_EXAMS', False):
-            from milestones.models import MilestoneRelationshipType
-            MilestoneRelationshipType.objects.create(name='requires')
-            MilestoneRelationshipType.objects.create(name='fulfills')
-
-    @patch.dict(settings.FEATURES, {'ENTRANCE_EXAMS': True})
+    @unittest.skipUnless(settings.FEATURES.get('ENTRANCE_EXAMS', False), True)
     def test_entrance_exam_created_updated_and_deleted_successfully(self):
-        self._seed_milestone_relationship_types()
+        seed_milestone_relationship_types()
         settings_details_url = get_url(self.course.id)
         data = {
             'entrance_exam_enabled': 'true',
@@ -196,13 +187,13 @@ class CourseDetailsTestCase(CourseTestCase):
         self.assertFalse(course.entrance_exam_enabled)
         self.assertEquals(course.entrance_exam_minimum_score_pct, None)
 
-    @patch.dict(settings.FEATURES, {'ENTRANCE_EXAMS': True})
+    @unittest.skipUnless(settings.FEATURES.get('ENTRANCE_EXAMS', False), True)
     def test_entrance_exam_store_default_min_score(self):
         """
         test that creating an entrance exam should store the default value, if key missing in json request
         or entrance_exam_minimum_score_pct is an empty string
         """
-        self._seed_milestone_relationship_types()
+        seed_milestone_relationship_types()
         settings_details_url = get_url(self.course.id)
         test_data_1 = {
             'entrance_exam_enabled': 'true',

cms/djangoapps/contentstore/views/entrance_exam.py

@@ -2,22 +2,22 @@
 Entrance Exams view module -- handles all requests related to entrance exam management via Studio
 Intended to be utilized as an AJAX callback handler, versus a proper view/screen
 """
+from functools import wraps
 import json
 import logging
 
 from django.contrib.auth.decorators import login_required
 from django_future.csrf import ensure_csrf_cookie
-from django.http import HttpResponse
+from django.http import HttpResponse, HttpResponseBadRequest
 from django.test import RequestFactory
 
 from contentstore.views.helpers import create_xblock
 from contentstore.views.item import delete_item
-from milestones import api as milestones_api
 from models.settings.course_metadata import CourseMetadata
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys import InvalidKeyError
 from student.auth import has_course_author_access
-from util.milestones_helpers import generate_milestone_namespace, NAMESPACE_CHOICES
+from util import milestones_helpers
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from django.conf import settings
@@ -40,8 +40,24 @@ def _get_default_entrance_exam_minimum_pct():
     return entrance_exam_minimum_score_pct
 
 
+# pylint: disable=missing-docstring
+def check_feature_enabled(feature_name):
+    """
+    Ensure the specified feature is turned on.  Return an HTTP 400 code if not.
+    """
+    def _check_feature_enabled(view_func):
+        def _decorator(request, *args, **kwargs):
+            # Deny access if the entrance exam feature is disabled
+            if not settings.FEATURES.get(feature_name, False):
+                return HttpResponseBadRequest()
+            return view_func(request, *args, **kwargs)
+        return wraps(view_func)(_decorator)
+    return _check_feature_enabled
+
+
 @login_required
 @ensure_csrf_cookie
+@check_feature_enabled(feature_name='ENTRANCE_EXAMS')
 def entrance_exam(request, course_key_string):
     """
     The restful handler for entrance exams.
@@ -88,6 +104,7 @@ def entrance_exam(request, course_key_string):
         return HttpResponse(status=405)
 
 
+@check_feature_enabled(feature_name='ENTRANCE_EXAMS')
 def create_entrance_exam(request, course_key, entrance_exam_minimum_score_pct):
     """
     api method to create an entrance exam.
@@ -150,27 +167,28 @@ def _create_entrance_exam(request, course_key, entrance_exam_minimum_score_pct=N
     )
 
     # Add an entrance exam milestone if one does not already exist
-    milestone_namespace = generate_milestone_namespace(
-        NAMESPACE_CHOICES['ENTRANCE_EXAM'],
+    namespace_choices = milestones_helpers.get_namespace_choices()
+    milestone_namespace = milestones_helpers.generate_milestone_namespace(
+        namespace_choices.get('ENTRANCE_EXAM'),
         course_key
     )
-    milestones = milestones_api.get_milestones(milestone_namespace)
+    milestones = milestones_helpers.get_milestones(milestone_namespace)
     if len(milestones):
         milestone = milestones[0]
     else:
         description = 'Autogenerated during {} entrance exam creation.'.format(unicode(course.id))
-        milestone = milestones_api.add_milestone({
+        milestone = milestones_helpers.add_milestone({
             'name': 'Completed Course Entrance Exam',
             'namespace': milestone_namespace,
             'description': description
         })
-    relationship_types = milestones_api.get_milestone_relationship_types()
-    milestones_api.add_course_milestone(
+    relationship_types = milestones_helpers.get_milestone_relationship_types()
+    milestones_helpers.add_course_milestone(
         unicode(course.id),
         relationship_types['REQUIRES'],
         milestone
     )
-    milestones_api.add_course_content_milestone(
+    milestones_helpers.add_course_content_milestone(
         unicode(course.id),
         unicode(created_block.location),
         relationship_types['FULFILLS'],
@@ -202,6 +220,7 @@ def _get_entrance_exam(request, course_key):  # pylint: disable=W0613
         return HttpResponse(status=404)
 
 
+@check_feature_enabled(feature_name='ENTRANCE_EXAMS')
 def update_entrance_exam(request, course_key, exam_data):
     """
     Operation to update course fields pertaining to entrance exams
@@ -215,6 +234,7 @@ def update_entrance_exam(request, course_key, exam_data):
         CourseMetadata.update_from_dict(metadata, course, request.user)
 
 
+@check_feature_enabled(feature_name='ENTRANCE_EXAMS')
 def delete_entrance_exam(request, course_key):
     """
     api method to delete an entrance exam
@@ -238,7 +258,7 @@ def _delete_entrance_exam(request, course_key):
     for course_child in course_children:
         if course_child.is_entrance_exam:
             delete_item(request, course_child.scope_ids.usage_id)
-            milestones_api.remove_content_references(unicode(course_child.scope_ids.usage_id))
+            milestones_helpers.remove_content_references(unicode(course_child.scope_ids.usage_id))
 
     # Reset the entrance exam flags on the course
     # Reload the course so we have the latest state

cms/djangoapps/contentstore/views/tests/test_entrance_exam.py

@@ -2,6 +2,7 @@
 Test module for Entrance Exams AJAX callback handler workflows
 """
 import json
+from mock import patch
 
 from django.conf import settings
 from django.contrib.auth.models import User
@@ -9,18 +10,14 @@ from django.test.client import RequestFactory
 
 from contentstore.tests.utils import AjaxEnabledTestClient, CourseTestCase
 from contentstore.utils import reverse_url
-from contentstore.views.entrance_exam import create_entrance_exam
+from contentstore.views.entrance_exam import create_entrance_exam, update_entrance_exam, delete_entrance_exam
 from models.settings.course_grading import CourseGradingModel
 from models.settings.course_metadata import CourseMetadata
 from opaque_keys.edx.keys import UsageKey
 from student.tests.factories import UserFactory
+from util import milestones_helpers
 from xmodule.modulestore.django import modulestore
 
-if settings.FEATURES.get('MILESTONES_APP', False):
-    from milestones import api as milestones_api
-    from milestones.models import MilestoneRelationshipType
-    from util.milestones_helpers import serialize_user
-
 
 class EntranceExamHandlerTests(CourseTestCase):
     """
@@ -36,9 +33,8 @@ class EntranceExamHandlerTests(CourseTestCase):
             self.usage_key = self.course.location
             self.course_url = '/course/{}'.format(unicode(self.course.id))
             self.exam_url = '/course/{}/entrance_exam/'.format(unicode(self.course.id))
-            MilestoneRelationshipType.objects.create(name='requires', active=True)
-            MilestoneRelationshipType.objects.create(name='fulfills', active=True)
-            self.milestone_relationship_types = milestones_api.get_milestone_relationship_types()
+            milestones_helpers.seed_milestone_relationship_types()
+            self.milestone_relationship_types = milestones_helpers.get_milestone_relationship_types()
 
         def test_contentstore_views_entrance_exam_post(self):
             """
@@ -55,8 +51,8 @@ class EntranceExamHandlerTests(CourseTestCase):
             self.assertTrue(metadata['entrance_exam_enabled'])
             self.assertIsNotNone(metadata['entrance_exam_minimum_score_pct'])
             self.assertIsNotNone(metadata['entrance_exam_id']['value'])
-            self.assertTrue(len(milestones_api.get_course_milestones(unicode(self.course.id))))
-            content_milestones = milestones_api.get_course_content_milestones(
+            self.assertTrue(len(milestones_helpers.get_course_milestones(unicode(self.course.id))))
+            content_milestones = milestones_helpers.get_course_content_milestones(
                 unicode(self.course.id),
                 metadata['entrance_exam_id']['value'],
                 self.milestone_relationship_types['FULFILLS']
@@ -123,12 +119,12 @@ class EntranceExamHandlerTests(CourseTestCase):
             )
             user.set_password('test')
             user.save()
-            milestones = milestones_api.get_course_milestones(unicode(self.course_key))
+            milestones = milestones_helpers.get_course_milestones(unicode(self.course_key))
             self.assertEqual(len(milestones), 1)
             milestone_key = '{}.{}'.format(milestones[0]['namespace'], milestones[0]['name'])
-            paths = milestones_api.get_course_milestones_fulfillment_paths(
+            paths = milestones_helpers.get_course_milestones_fulfillment_paths(
                 unicode(self.course_key),
-                serialize_user(user)
+                milestones_helpers.serialize_user(user)
             )
 
             # What we have now is a course milestone requirement and no valid fulfillment
@@ -250,3 +246,22 @@ class EntranceExamHandlerTests(CourseTestCase):
 
             resp = create_entrance_exam(request, self.course.id, None)
             self.assertEqual(resp.status_code, 201)
+
+        @patch.dict('django.conf.settings.FEATURES', {'ENTRANCE_EXAMS': False})
+        def test_entrance_exam_feature_flag_gating(self):
+            user = UserFactory()
+            user.is_staff = True
+            request = RequestFactory()
+            request.user = user
+
+            resp = self.client.get(self.exam_url)
+            self.assertEqual(resp.status_code, 400)
+
+            resp = create_entrance_exam(request, self.course.id, None)
+            self.assertEqual(resp.status_code, 400)
+
+            resp = delete_entrance_exam(request, self.course.id)
+            self.assertEqual(resp.status_code, 400)
+
+            # No return, so we'll just ensure no exception is thrown
+            update_entrance_exam(request, self.course.id, {})

common/djangoapps/util/tests/test_milestones_helpers.py

+"""
+Tests for the milestones helpers library, which is the integration point for the edx_milestones API
+"""
+
+from mock import patch
+from util import milestones_helpers
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+
+
+@patch.dict('django.conf.settings.FEATURES', {'MILESTONES_APP': False})
+class MilestonesHelpersTestCase(ModuleStoreTestCase):
+    """
+    Main test suite for Milestones API client library
+    """
+
+    def setUp(self):
+        """
+        Test case scaffolding
+        """
+        super(MilestonesHelpersTestCase, self).setUp(create_user=False)
+        self.course = CourseFactory.create(
+            metadata={
+                'entrance_exam_enabled': True,
+            }
+        )
+
+        self.user = {'id': '123'}
+
+        self.milestone = {
+            'name': 'Test Milestone',
+            'namespace': 'doesnt.matter',
+            'description': 'Testing Milestones Helpers Library',
+        }
+
+    def test_add_milestone_returns_none_when_app_disabled(self):
+        response = milestones_helpers.add_milestone(milestone_data=self.milestone)
+        self.assertIsNone(response)
+
+    def test_get_milestones_returns_none_when_app_disabled(self):
+        response = milestones_helpers.get_milestones(namespace="whatever")
+        self.assertEqual(len(response), 0)
+
+    def test_get_milestone_relationship_types_returns_none_when_app_disabled(self):
+        response = milestones_helpers.get_milestone_relationship_types()
+        self.assertEqual(len(response), 0)
+
+    def test_add_course_milestone_returns_none_when_app_disabled(self):
+        response = milestones_helpers.add_course_milestone(unicode(self.course.id), 'requires', self.milestone)
+        self.assertIsNone(response)
+
+    def test_get_course_milestones_returns_none_when_app_disabled(self):
+        response = milestones_helpers.get_course_milestones(unicode(self.course.id))
+        self.assertEqual(len(response), 0)
+
+    def test_add_course_content_milestone_returns_none_when_app_disabled(self):
+        response = milestones_helpers.add_course_content_milestone(
+            unicode(self.course.id),
+            'i4x://any/content/id',
+            'requires',
+            self.milestone
+        )
+        self.assertIsNone(response)
+
+    def test_get_course_content_milestones_returns_none_when_app_disabled(self):
+        response = milestones_helpers.get_course_content_milestones(
+            unicode(self.course.id),
+            'i4x://doesnt/matter/for/this/test',
+            'requires'
+        )
+        self.assertEqual(len(response), 0)
+
+    def test_remove_content_references_returns_none_when_app_disabled(self):
+        response = milestones_helpers.remove_content_references("i4x://any/content/id/will/do")
+        self.assertIsNone(response)
+
+    def test_get_namespace_choices_returns_values_when_app_disabled(self):
+        response = milestones_helpers.get_namespace_choices()
+        self.assertIn('ENTRANCE_EXAM', response)
+
+    def test_get_course_milestones_fulfillment_paths_returns_none_when_app_disabled(self):
+        response = milestones_helpers.get_course_milestones_fulfillment_paths(unicode(self.course.id), self.user)
+        self.assertIsNone(response)
+
+    def test_add_user_milestone_returns_none_when_app_disabled(self):
+        response = milestones_helpers.add_user_milestone(self.user, self.milestone)
+        self.assertIsNone(response)

lms/djangoapps/courseware/module_render.py

@@ -63,10 +63,8 @@ from xmodule.x_module import XModuleDescriptor
 from xblock_django.user_service import DjangoXBlockUserService
 from util.json_request import JsonResponse
 from util.sandboxing import can_execute_unsafe_code, get_python_lib_zip
-if settings.FEATURES.get('MILESTONES_APP', False):
-    from milestones import api as milestones_api
-    from util.milestones_helpers import calculate_entrance_exam_score, get_required_content
-    from util.module_utils import yield_dynamic_descriptor_descendents
+from util import milestones_helpers
+from util.module_utils import yield_dynamic_descriptor_descendents
 
 log = logging.getLogger(__name__)
 
@@ -136,14 +134,14 @@ def toc_for_course(request, course, active_chapter, active_section, field_data_c
             return None
 
         # Check to see if the course is gated on milestone-required content (such as an Entrance Exam)
-        required_content = get_required_content(course, request.user)
+        required_content = milestones_helpers.get_required_content(course, request.user)
 
         chapters = list()
         for chapter in course_module.get_display_items():
             # Only show required content, if there is required content
             # chapter.hide_from_toc is read-only (boo)
             local_hide_from_toc = False
-            if len(required_content):
+            if required_content:
                 if unicode(chapter.location) not in required_content:
                     local_hide_from_toc = True
 
@@ -375,7 +373,7 @@ def get_module_system_for_user(user, field_data_cache,
             inner_get_module
         )
         exam_modules = [module for module in exam_module_generators]
-        exam_score = calculate_entrance_exam_score(user, course_descriptor, exam_modules)
+        exam_score = milestones_helpers.calculate_entrance_exam_score(user, course_descriptor, exam_modules)
         return exam_score
 
     def _fulfill_content_milestones(user, course_key, content_key):
@@ -394,8 +392,8 @@ def get_module_system_for_user(user, field_data_cache,
                 exam_pct = _calculate_entrance_exam_score(user, course)
                 if exam_pct >= course.entrance_exam_minimum_score_pct:
                     exam_key = UsageKey.from_string(course.entrance_exam_id)
-                    relationship_types = milestones_api.get_milestone_relationship_types()
-                    content_milestones = milestones_api.get_course_content_milestones(
+                    relationship_types = milestones_helpers.get_milestone_relationship_types()
+                    content_milestones = milestones_helpers.get_course_content_milestones(
                         course_key,
                         exam_key,
                         relationship=relationship_types['FULFILLS']
@@ -403,7 +401,7 @@ def get_module_system_for_user(user, field_data_cache,
                     # Add each milestone to the user's set...
                     user = {'id': user.id}
                     for milestone in content_milestones:
-                        milestones_api.add_user_milestone(user, milestone)
+                        milestones_helpers.add_user_milestone(user, milestone)
 
     def handle_grade_event(block, event_type, event):  # pylint: disable=unused-argument
         """

lms/djangoapps/courseware/tabs.py

@@ -9,9 +9,7 @@ from courseware.access import has_access
 from student.models import CourseEnrollment, EntranceExamConfiguration
 from xmodule.tabs import CourseTabList
 
-if settings.FEATURES.get('MILESTONES_APP', False):
-    from milestones.api import get_course_milestones_fulfillment_paths
-    from util.milestones_helpers import serialize_user
+from util import milestones_helpers
 
 
 def get_course_tab_list(course, user):
@@ -33,9 +31,9 @@ def get_course_tab_list(course, user):
     entrance_exam_mode = False
     if settings.FEATURES.get('ENTRANCE_EXAMS', False):
         if getattr(course, 'entrance_exam_enabled', False):
-            course_milestones_paths = get_course_milestones_fulfillment_paths(
+            course_milestones_paths = milestones_helpers.get_course_milestones_fulfillment_paths(
                 unicode(course.id),
-                serialize_user(user)
+                milestones_helpers.serialize_user(user)
             )
             for __, value in course_milestones_paths.iteritems():
                 if len(value.get('content', [])):

lms/djangoapps/courseware/tests/test_tabs.py

@@ -15,16 +15,14 @@ from xmodule import tabs
 from xmodule.modulestore.tests.django_utils import (
     TEST_DATA_MIXED_TOY_MODULESTORE, TEST_DATA_MIXED_CLOSED_MODULESTORE
 )
+
+from courseware.tabs import get_course_tab_list
 from courseware.views import get_static_tab_contents, static_tab
 from student.tests.factories import UserFactory
+from util import milestones_helpers
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 
-if settings.FEATURES.get('MILESTONES_APP', False):
-    from courseware.tabs import get_course_tab_list
-    from milestones import api as milestones_api
-    from milestones.models import MilestoneRelationshipType
-
 
 class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
     """Test cases for Static Tab Dates."""
@@ -140,9 +138,8 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
             self.setup_user()
             self.enroll(self.course)
             self.user.is_staff = True
-            self.relationship_types = milestones_api.get_milestone_relationship_types()
-            MilestoneRelationshipType.objects.create(name='requires')
-            MilestoneRelationshipType.objects.create(name='fulfills')
+            self.relationship_types = milestones_helpers.get_milestone_relationship_types()
+            milestones_helpers.seed_milestone_relationship_types()
 
         def test_get_course_tabs_list_entrance_exam_enabled(self):
             """
@@ -160,13 +157,13 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
             }
             self.course.entrance_exam_enabled = True
             self.course.entrance_exam_id = unicode(entrance_exam.location)
-            milestone = milestones_api.add_milestone(milestone)
-            milestones_api.add_course_milestone(
+            milestone = milestones_helpers.add_milestone(milestone)
+            milestones_helpers.add_course_milestone(
                 unicode(self.course.id),
                 self.relationship_types['REQUIRES'],
                 milestone
             )
-            milestones_api.add_course_content_milestone(
+            milestones_helpers.add_course_content_milestone(
                 unicode(self.course.id),
                 unicode(entrance_exam.location),
                 self.relationship_types['FULFILLS'],