Merge pull request #23 from edx/sstudent/handle-unknown-rt

Handle unknown relationship types gracefully when retrieving milestones

milestones/api.py

@@ -59,16 +59,6 @@ def _validate_milestone_data(milestone):
         )
 
 
-def _validate_milestone_relationship_type(name):
-    """ Validation helper """
-    if not validators.milestone_relationship_type_is_valid(name):
-        exceptions.raise_exception(
-            "MilestoneRelationshipType",
-            name,
-            exceptions.InvalidMilestoneRelationshipTypeException
-        )
-
-
 def _validate_user(user):
     """ Validation helper """
     if not validators.user_is_valid(user):
@@ -145,7 +135,6 @@ def add_course_milestone(course_key, relationship, milestone):
     'relationship': string value (eg: 'requires')
     """
     _validate_course_key(course_key)
-    _validate_milestone_relationship_type(relationship)
     _validate_milestone_data(milestone)
     data.create_course_milestone(
         course_key=course_key,
@@ -162,9 +151,12 @@ def get_course_milestones(course_key, relationship=None):
     """
     _validate_course_key(course_key)
 
-    if relationship is not None:
-        _validate_milestone_relationship_type(relationship)
-    return data.fetch_courses_milestones(course_keys=[course_key], relationship=relationship)
+    try:
+        milestones = data.fetch_courses_milestones(course_keys=[course_key], relationship=relationship)
+    except exceptions.InvalidMilestoneRelationshipTypeException:
+        milestones = []
+
+    return milestones
 
 
 def get_course_required_milestones(course_key, user):
@@ -253,14 +245,16 @@ def get_courses_milestones(course_keys, relationship=None, user=None):
     Returns an array of dicts containing milestones
     """
     [_validate_course_key(course_key) for course_key in course_keys]  # pylint: disable=expression-not-assigned
+    try:
+        milestones = data.fetch_courses_milestones(
+            course_keys=course_keys,
+            relationship=relationship,
+            user=user
+        )
+    except exceptions.InvalidMilestoneRelationshipTypeException:
+        milestones = []
 
-    if relationship is not None:
-        _validate_milestone_relationship_type(relationship)
-
-    return data.fetch_courses_milestones(
-        course_keys=course_keys,
-        relationship=relationship,
-        user=user)
+    return milestones
 
 
 def remove_course_milestone(course_key, milestone):
@@ -288,7 +282,6 @@ def add_course_content_milestone(course_key, content_key, relationship, mileston
     """
     _validate_course_key(course_key)
     _validate_content_key(content_key)
-    _validate_milestone_relationship_type(relationship)
     _validate_milestone_data(milestone)
     _validate_course_content_milestone_requirements(requirements)
     data.create_course_content_milestone(
@@ -317,15 +310,17 @@ def get_course_content_milestones(course_key=None, content_key=None, relationshi
         _validate_course_key(course_key)
     if content_key is not None:
         _validate_content_key(content_key)
-    if relationship is not None:
-        _validate_milestone_relationship_type(relationship)
+    try:
+        milestones = data.fetch_course_content_milestones(
+            course_key=course_key,
+            content_key=content_key,
+            relationship=relationship,
+            user=user
+        )
+    except exceptions.InvalidMilestoneRelationshipTypeException:
+        milestones = []
 
-    return data.fetch_course_content_milestones(
-        course_key=course_key,
-        content_key=content_key,
-        relationship=relationship,
-        user=user
-    )
+    return milestones
 
 
 def remove_course_content_milestone(course_key, content_key, milestone):

milestones/tests/test_api.py

@@ -309,7 +309,7 @@ class MilestonesApiTestCase(utils.MilestonesTestCaseMixin, utils.MilestonesTestC
 
     def test_add_course_milestone_bogus_milestone_relationship_type(self):
         """ Unit Test: test_add_course_milestone_bogus_milestone_relationship_type """
-        with self.assertNumQueries(0):
+        with self.assertNumQueries(1):
             with self.assertRaises(exceptions.InvalidMilestoneRelationshipTypeException):
                 api.add_course_milestone(self.test_course_key, 'whatever', self.test_milestone)
 
@@ -327,6 +327,20 @@ class MilestonesApiTestCase(utils.MilestonesTestCaseMixin, utils.MilestonesTestC
             )
         self.assertEqual(len(requirer_milestones), 1)
 
+    def test_get_course_milestones_with_invalid_relationship_type(self):
+        """ Unit Test: test_get_course_milestones_with_invalid_relationship_type """
+        api.add_course_milestone(
+            self.test_course_key,
+            self.relationship_types['REQUIRES'],
+            self.test_milestone
+        )
+        with self.assertNumQueries(1):
+            requirer_milestones = api.get_course_milestones(
+                self.test_course_key,
+                'INVALID RELATIONSHIP TYPE'
+            )
+        self.assertEqual(len(requirer_milestones), 0)
+
     def test_get_course_unfulfilled_milestones(self):
         """ Unit Test: test_get_course_unfulfilled_milestones """
         namespace = 'test_get_milestones'
@@ -427,6 +441,37 @@ class MilestonesApiTestCase(utils.MilestonesTestCaseMixin, utils.MilestonesTestC
             )
         self.assertEqual(len(requirer_milestones), 2)
 
+    def test_get_courses_milestones_with_invalid_relationship_type(self):
+        """ Unit Test: test_get_courses_milestones_with_invalid_relationship_type """
+        api.add_course_milestone(
+            self.test_course_key,
+            self.relationship_types['REQUIRES'],
+            self.test_milestone
+        )
+        api.add_course_milestone(
+            self.test_prerequisite_course_key,
+            self.relationship_types['REQUIRES'],
+            self.test_milestone
+        )
+        local_milestone = api.add_milestone({
+            'display_name': 'Local Milestone',
+            'name': 'local_milestone',
+            'namespace': unicode(self.test_course_key),
+            'description': 'Local Milestone Description'
+        })
+        api.add_course_milestone(
+            self.test_course_key,
+            self.relationship_types['FULFILLS'],
+            local_milestone
+        )
+
+        with self.assertNumQueries(1):
+            requirer_milestones = api.get_courses_milestones(
+                [self.test_course_key],
+                'INVALID RELATIONSHIP TYPE'
+            )
+        self.assertEqual(len(requirer_milestones), 0)
+
     def test_remove_course_milestone(self):
         """ Unit Test: test_remove_course_milestone """
         api.add_course_milestone(
@@ -605,7 +650,7 @@ class MilestonesApiTestCase(utils.MilestonesTestCaseMixin, utils.MilestonesTestC
 
     def test_add_course_content_milestone_bogus_milestone_relationship_type(self):
         """ Unit Test: test_add_course_content_milestone_bogus_milestone_relationship_type """
-        with self.assertNumQueries(0):
+        with self.assertNumQueries(1):
             with self.assertRaises(exceptions.InvalidMilestoneRelationshipTypeException):
                 api.add_course_content_milestone(
                     self.test_course_key,
@@ -613,7 +658,7 @@ class MilestonesApiTestCase(utils.MilestonesTestCaseMixin, utils.MilestonesTestC
                     'whatever',
                     self.test_milestone
                 )
-        with self.assertNumQueries(0):
+        with self.assertNumQueries(1):
             with self.assertRaises(exceptions.InvalidMilestoneRelationshipTypeException):
                 api.add_course_content_milestone(
                     self.test_course_key,
@@ -718,6 +763,28 @@ class MilestonesApiTestCase(utils.MilestonesTestCaseMixin, utils.MilestonesTestC
             )
         self.assertEqual(len(requirer_milestones), 2)
 
+    def test_get_course_content_milestones_with_invalid_relationship(self):
+        """ Unit Test: test_get_course_content_milestones_with_invalid_relationship """
+        api.add_course_content_milestone(
+            self.test_course_key,
+            self.test_content_key,
+            self.relationship_types['REQUIRES'],
+            self.test_milestone
+        )
+        api.add_course_content_milestone(
+            self.test_course_key,
+            self.test_alternate_content_key,
+            self.relationship_types['REQUIRES'],
+            self.test_milestone
+        )
+        with self.assertNumQueries(1):
+            requirer_milestones = api.get_course_content_milestones(
+                self.test_course_key,
+                self.test_content_key,
+                'INVALID RELATIONSHIP TYPE'
+            )
+        self.assertEqual(len(requirer_milestones), 0)
+
     def test_remove_course_content_milestone(self):
         """ Unit Test: test_remove_course_content_milestone """
         api.add_course_content_milestone(

setup.py

@@ -4,7 +4,7 @@ from setuptools import setup, find_packages
 
 setup(
     name='edx-milestones',
-    version='0.1.9',
+    version='0.1.10',
     description='Significant events module for Open edX',
     long_description=open('README.md').read(),
     author='edX',