Merge pull request #15623 from edx/george/enrollments-prereq

Fixed milestone issues

AUTHORS

@@ -259,6 +259,7 @@ Muhammad Rehan <muhammadrehan69@gmail.com>
 Shawn Milochik <shawn@milochik.com>
 Afeef Janjua <janjua.afeef@gmail.com>
 Jacek Bzdak <jbzdak@gmail.com>
+Ahmed Jazzar <ajazzar@qrf.org>
 Jillian Vogel <pomegranited@gmail.com>
 Dan Powell <dan@abakas.com>
 Mariana Araújo <simbelm.ne@gmail.com>

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -15,6 +15,7 @@ from milestones.tests.utils import MilestonesTestCaseMixin
 from mock import Mock, patch
 
 from contentstore.utils import reverse_course_url, reverse_usage_url
+from milestones.models import MilestoneRelationshipType
 from models.settings.course_grading import CourseGradingModel, GRADING_POLICY_CHANGED_EVENT_TYPE, hash_grading_policy
 from models.settings.course_metadata import CourseMetadata
 from models.settings.encoder import CourseSettingsEncoder
@@ -22,6 +23,7 @@ from openedx.core.djangoapps.models.course_details import CourseDetails
 from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration
 from student.roles import CourseInstructorRole, CourseStaffRole
 from student.tests.factories import UserFactory
+from util import milestones_helpers
 from xblock_django.models import XBlockStudioConfigurationFlag
 from xmodule.fields import Date
 from xmodule.modulestore import ModuleStoreEnum
@@ -100,6 +102,9 @@ class CourseDetailsViewTest(CourseTestCase, MilestonesTestCaseMixin):
         resp = self.client.ajax_post(url, payload)
         self.compare_details_with_encoding(json.loads(resp.content), details.__dict__, field + str(val))
 
+        MilestoneRelationshipType.objects.get_or_create(name='requires')
+        MilestoneRelationshipType.objects.get_or_create(name='fulfills')
+
     @staticmethod
     def convert_datetime_to_iso(datetime_obj):
         """
@@ -172,6 +177,9 @@ class CourseDetailsViewTest(CourseTestCase, MilestonesTestCaseMixin):
 
     @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True})
     def test_pre_requisite_course_update_and_fetch(self):
+        self.assertFalse(milestones_helpers.any_unfulfilled_milestones(self.course.id, self.user.id),
+                         msg='The initial empty state should be: no prerequisite courses')
+
         url = get_url(self.course.id)
         resp = self.client.get_json(url)
         course_detail_json = json.loads(resp.content)
@@ -190,6 +198,9 @@ class CourseDetailsViewTest(CourseTestCase, MilestonesTestCaseMixin):
         course_detail_json = json.loads(resp.content)
         self.assertEqual(pre_requisite_course_keys, course_detail_json['pre_requisite_courses'])
 
+        self.assertTrue(milestones_helpers.any_unfulfilled_milestones(self.course.id, self.user.id),
+                        msg='Should have prerequisite courses')
+
         # remove pre requisite course
         course_detail_json['pre_requisite_courses'] = []
         self.client.ajax_post(url, course_detail_json)
@@ -197,6 +208,9 @@ class CourseDetailsViewTest(CourseTestCase, MilestonesTestCaseMixin):
         course_detail_json = json.loads(resp.content)
         self.assertEqual([], course_detail_json['pre_requisite_courses'])
 
+        self.assertFalse(milestones_helpers.any_unfulfilled_milestones(self.course.id, self.user.id),
+                         msg='Should not have prerequisite courses anymore')
+
     @mock.patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True})
     def test_invalid_pre_requisite_course(self):
         url = get_url(self.course.id)
@@ -268,6 +282,14 @@ class CourseDetailsViewTest(CourseTestCase, MilestonesTestCaseMixin):
 
     @unittest.skipUnless(settings.FEATURES.get('ENTRANCE_EXAMS', False), True)
     def test_entrance_exam_created_updated_and_deleted_successfully(self):
+        """
+        This tests both of the entrance exam settings and the `any_unfulfilled_milestones` helper.
+
+        Splitting the test requires significant refactoring `settings_handler()` view.
+        """
+        self.assertFalse(milestones_helpers.any_unfulfilled_milestones(self.course.id, self.user.id),
+                         msg='The initial empty state should be: no entrance exam')
+
         settings_details_url = get_url(self.course.id)
         data = {
             'entrance_exam_enabled': 'true',
@@ -299,6 +321,9 @@ class CourseDetailsViewTest(CourseTestCase, MilestonesTestCaseMixin):
         self.assertTrue(course.entrance_exam_enabled)
         self.assertEquals(course.entrance_exam_minimum_score_pct, .80)
 
+        self.assertTrue(milestones_helpers.any_unfulfilled_milestones(self.course.id, self.user.id),
+                        msg='The entrance exam should be required.')
+
         # Delete the entrance exam
         data['entrance_exam_enabled'] = "false"
         response = self.client.post(
@@ -312,6 +337,9 @@ class CourseDetailsViewTest(CourseTestCase, MilestonesTestCaseMixin):
         self.assertFalse(course.entrance_exam_enabled)
         self.assertEquals(course.entrance_exam_minimum_score_pct, None)
 
+        self.assertFalse(milestones_helpers.any_unfulfilled_milestones(self.course.id, self.user.id),
+                         msg='The entrance exam should not be required anymore')
+
     @unittest.skipUnless(settings.FEATURES.get('ENTRANCE_EXAMS', False), True)
     def test_entrance_exam_store_default_min_score(self):
         """

cms/djangoapps/contentstore/views/course.py

@@ -51,6 +51,7 @@ from course_action_state.managers import CourseActionStateItemNotFoundError
 from course_action_state.models import CourseRerunState, CourseRerunUIStateManager
 from course_creators.views import add_user_with_status_unrequested, get_course_creator_status
 from edxmako.shortcuts import render_to_response
+from milestones import api as milestones_api
 from models.settings.course_grading import CourseGradingModel
 from models.settings.course_metadata import CourseMetadata
 from models.settings.encoder import CourseSettingsEncoder
@@ -73,6 +74,7 @@ from util.milestones_helpers import (
     is_entrance_exams_enabled,
     is_prerequisite_courses_enabled,
     is_valid_course_key,
+    remove_prerequisite_course,
     set_prerequisite_courses
 )
 from util.organizations_helpers import add_organization_course, get_organization_by_short_name, organizations_enabled
@@ -1075,6 +1077,11 @@ def settings_handler(request, course_key_string):
                         if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys):
                             return JsonResponseBadRequest({"error": _("Invalid prerequisite course key")})
                         set_prerequisite_courses(course_key, prerequisite_course_keys)
+                    else:
+                        # None is chosen, so remove the course prerequisites
+                        course_milestones = milestones_api.get_course_milestones(course_key=course_key, relationship="requires")
+                        for milestone in course_milestones:
+                            remove_prerequisite_course(course_key, milestone)
 
                 # If the entrance exams feature has been enabled, we'll need to check for some
                 # feature-specific settings and handle them accordingly

common/djangoapps/util/milestones_helpers.py

@@ -384,9 +384,12 @@ def any_unfulfilled_milestones(course_id, user_id):
     """ Returns a boolean if user has any unfulfilled milestones """
     if not settings.FEATURES.get('MILESTONES_APP'):
         return False
-    return bool(
-        get_course_milestones_fulfillment_paths(course_id, {"id": user_id})
-    )
+
+    fulfillment_paths = milestones_api.get_course_milestones_fulfillment_paths(course_id, {'id': user_id})
+
+    # Returns True if any of the milestones is unfulfilled. False if
+    # values is empty or all values are.
+    return any(fulfillment_paths.values())
 
 
 def get_course_milestones_fulfillment_paths(course_id, user_id):

common/djangoapps/util/tests/test_milestones_helpers.py

@@ -3,15 +3,18 @@ Tests for the milestones helpers library, which is the integration point for the
 """
 
 import ddt
+from django.conf import settings
 from milestones.exceptions import InvalidCourseKeyException, InvalidUserException
 from mock import patch
 
-from util import milestones_helpers
+from milestones import api as milestones_api
+from milestones.models import MilestoneRelationshipType
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
+from util import milestones_helpers
 
 
-@patch.dict('django.conf.settings.FEATURES', {'MILESTONES_APP': False})
+@patch.dict(settings.FEATURES, {'MILESTONES_APP': False})
 @ddt.ddt
 class MilestonesHelpersTestCase(ModuleStoreTestCase):
     """
@@ -33,11 +36,14 @@ class MilestonesHelpersTestCase(ModuleStoreTestCase):
 
         self.user = {'id': '123'}
 
-        self.milestone = {
+        self.milestone = milestones_api.add_milestone({
             'name': 'Test Milestone',
             'namespace': 'doesnt.matter',
             'description': 'Testing Milestones Helpers Library',
-        }
+        })
+
+        MilestoneRelationshipType.objects.get_or_create(name='requires')
+        MilestoneRelationshipType.objects.get_or_create(name='fulfills')
 
     @ddt.data(
         (False, False, False),
@@ -115,13 +121,16 @@ class MilestonesHelpersTestCase(ModuleStoreTestCase):
         response = milestones_helpers.get_service()
         self.assertIsNone(response)
 
-    @patch.dict('django.conf.settings.FEATURES', {'MILESTONES_APP': True})
+    @patch.dict(settings.FEATURES, {'MILESTONES_APP': True})
     def test_any_unfulfilled_milestones(self):
         """
-        Tests any_unfulfilled_milestones for invalid arguments with
-        the app enabled
-         """
+        Tests any_unfulfilled_milestones for invalid arguments with the app enabled.
+        """
+
+        # Should not raise any exceptions
+        milestones_helpers.any_unfulfilled_milestones(self.course.id, self.user['id'])
+
         with self.assertRaises(InvalidCourseKeyException):
-            milestones_helpers.any_unfulfilled_milestones(None, self.user)
+            milestones_helpers.any_unfulfilled_milestones(None, self.user['id'])
         with self.assertRaises(InvalidUserException):
             milestones_helpers.any_unfulfilled_milestones(self.course.id, None)