From 2ee86b563166744e71bd175c843ad2623aa6135f Mon Sep 17 00:00:00 2001 From: Sanford Student Date: Tue, 2 Aug 2016 10:25:43 -0400 Subject: [PATCH] return empty list on invalid milestone relationship type --- milestones/api.py | 55 ++++++++++++--------------- milestones/tests/test_api.py | 73 ++++++++++++++++++++++++++++++++++-- setup.py | 2 +- 3 files changed, 96 insertions(+), 34 deletions(-) diff --git a/milestones/api.py b/milestones/api.py index c633928b..10631d3c 100644 --- a/milestones/api.py +++ b/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): diff --git a/milestones/tests/test_api.py b/milestones/tests/test_api.py index 98be70fe..e0a117ce 100644 --- a/milestones/tests/test_api.py +++ b/milestones/tests/test_api.py @@ -309,7 +309,7 @@ def test_add_course_milestone_bogus_course_key(self): 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 @@ def test_get_course_milestones(self): ) 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 @@ def test_get_courses_milestones(self): ) 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 @@ def test_add_course_content_milestone_bogus_content_key(self): 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 @@ def test_add_course_content_milestone_bogus_milestone_relationship_type(self): '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 @@ def test_get_course_content_milestones_without_content_key(self): ) 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( diff --git a/setup.py b/setup.py index d0602bf7..2e91e83e 100755 --- a/setup.py +++ b/setup.py @@ -4,7 +4,7 @@ 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',