diff --git a/milestones/data.py b/milestones/data.py index 22910d93..40cef843 100644 --- a/milestones/data.py +++ b/milestones/data.py @@ -29,7 +29,7 @@ from . import serializers -# PRIVATE/INTERNAL METHODS +# PRIVATE/INTERNAL METHODS (public methods located further down) def _get_milestone_relationship_type(relationship): """ Retrieves milestone relationship type object from backend datastore @@ -47,17 +47,132 @@ def _get_milestone_relationship_type(relationship): ) +def _activate_record(record): + """ + Enables database records by setting the 'active' attribute to True + The queries in this module filter out inactive records as part of their criteria + This effectively allows us to soft-delete records so they are not lost forever + """ + record.active = True + record.save() + + def _inactivate_record(record): """ Disables database records by setting the 'active' attribute to False The queries in this module filter out inactive records as part of their criteria This effectively allows us to soft-delete records so they are not lost forever """ - record.active = False record.save() +def _activate_milestone(milestone): + """ + Activates an inactivated (soft-deleted) milestone as well as any inactive relationships + """ + [_activate_course_milestone_relationship(record) for record + in internal.CourseMilestone.objects.filter(milestone_id=milestone.id, active=False)] + + [_activate_course_content_milestone_relationship(record) for record + in internal.CourseContentMilestone.objects.filter(milestone_id=milestone.id, active=False)] + + [_activate_user_milestone_relationship(record) for record + in internal.UserMilestone.objects.filter(milestone_id=milestone.id, active=False)] + + [_activate_record(record) for record + in internal.Milestone.objects.filter(id=milestone.id, active=False)] + + +def _inactivate_milestone(milestone): + """ + Inactivates an activated milestone as well as any active relationships + """ + [_inactivate_course_milestone_relationship(record) for record + in internal.CourseMilestone.objects.filter(milestone_id=milestone.id, active=True)] + + [_inactivate_course_content_milestone_relationship(record) for record + in internal.CourseContentMilestone.objects.filter(milestone_id=milestone.id, active=True)] + + [_inactivate_user_milestone_relationship(record) for record + in internal.UserMilestone.objects.filter(milestone_id=milestone.id, active=True)] + + [_inactivate_record(record) for record + in internal.Milestone.objects.filter(id=milestone.id, active=True)] + + +def _activate_course_milestone_relationship(relationship): + """ + Activates an inactive milestone relationship + """ + # If the relationship doesn't exist or the milestone isn't active we'll want to raise an error + relationship = internal.CourseMilestone.objects.get( + id=relationship.id, + active=False, + milestone__active=True + ) + _activate_record(relationship) + + +def _inactivate_course_milestone_relationship(relationship): + """ + Inactivates an active milestone relationship + """ + relationship = internal.CourseMilestone.objects.get( + id=relationship.id, + active=True + ) + _inactivate_record(relationship) + + +def _activate_course_content_milestone_relationship(relationship): + """ + Activates an inactive milestone relationship + """ + # If the relationship doesn't exist or the milestone isn't active we'll want to raise an error + relationship = internal.CourseContentMilestone.objects.get( + id=relationship.id, + active=False, + milestone__active=True + ) + _activate_record(relationship) + + +def _inactivate_course_content_milestone_relationship(relationship): + """ + Inactivates an active milestone relationship + """ + relationship = internal.CourseContentMilestone.objects.get( + id=relationship.id, + active=True + ) + _inactivate_record(relationship) + + +def _activate_user_milestone_relationship(relationship): + """ + Activates an inactive milestone relationship + """ + # If the relationship doesn't exist or the milestone isn't active we'll want to raise an error + relationship = internal.UserMilestone.objects.get( + id=relationship.id, + active=False, + milestone__active=True + ) + _activate_record(relationship) + + +def _inactivate_user_milestone_relationship(relationship): + """ + Inactivates an active milestone relationship + """ + relationship = internal.UserMilestone.objects.get( + id=relationship.id, + active=True + ) + _inactivate_record(relationship) + + # PUBLIC METHODS def create_milestone(milestone): """ @@ -76,15 +191,22 @@ def create_milestone(milestone): if not milestone.get('namespace'): exceptions.raise_exception("Milestone", milestone, exceptions.InvalidMilestoneException) milestone_obj = serializers.deserialize_milestone(milestone) - milestone, __ = internal.Milestone.objects.get_or_create( # pylint: disable=invalid-name - namespace=milestone_obj.namespace, - name=milestone_obj.name, - active=True, - defaults={ - 'description': milestone_obj.description, - 'display_name': milestone_obj.display_name, - } - ) + try: + milestone = internal.Milestone.objects.get( + namespace=milestone_obj.namespace, + name=milestone_obj.name, + ) + # If the milestone exists, but was inactivated, we can simply turn it back on + if not milestone.active: + _activate_milestone(milestone_obj) + except internal.Milestone.DoesNotExist: + milestone = internal.Milestone.objects.create( + namespace=milestone_obj.namespace, + name=milestone_obj.name, + description=milestone_obj.description, + display_name=milestone_obj.display_name, + active=True + ) return serializers.serialize_milestone(milestone) @@ -111,26 +233,7 @@ def delete_milestone(milestone): No return currently defined for this operation """ milestone_obj = serializers.deserialize_milestone(milestone) - - [_inactivate_record(record) for record in internal.CourseMilestone.objects.filter( - milestone_id=milestone_obj.id, - active=True - )] - - [_inactivate_record(record) for record in internal.CourseContentMilestone.objects.filter( - milestone_id=milestone_obj.id, - active=True - )] - - [_inactivate_record(record) for record in internal.UserMilestone.objects.filter( - milestone_id=milestone_obj.id, - active=True - )] - - [_inactivate_record(record) for record in internal.Milestone.objects.filter( - id=milestone_obj.id, - active=True - )] + _inactivate_milestone(milestone_obj) def fetch_milestone(milestone_id): @@ -184,12 +287,22 @@ def create_course_milestone(course_key, relationship, milestone): """ relationship_type = _get_milestone_relationship_type(relationship) milestone_obj = serializers.deserialize_milestone(milestone) - internal.CourseMilestone.objects.get_or_create( - course_id=unicode(course_key), - milestone=milestone_obj, - milestone_relationship_type=relationship_type, - active=True, - ) + try: + relationship = internal.CourseMilestone.objects.get( + course_id=unicode(course_key), + milestone=milestone_obj, + milestone_relationship_type=relationship_type + ) + # If the relationship exists, but was inactivated, we can simply turn it back on + if not relationship.active: + _activate_course_milestone_relationship(relationship) + except internal.CourseMilestone.DoesNotExist: + relationship = internal.CourseMilestone.objects.create( + course_id=unicode(course_key), + milestone=milestone_obj, + milestone_relationship_type=relationship_type, + active=True + ) def delete_course_milestone(course_key, milestone): @@ -198,12 +311,12 @@ def delete_course_milestone(course_key, milestone): No response currently defined for this operation """ try: - record = internal.CourseMilestone.objects.get( + relationship = internal.CourseMilestone.objects.get( course_id=unicode(course_key), milestone=milestone['id'], active=True, ) - _inactivate_record(record) + _inactivate_course_milestone_relationship(relationship) except internal.CourseMilestone.DoesNotExist: # If we're being asked to delete a course-milestone link # that does not exist in the database then our work is done @@ -245,13 +358,24 @@ def create_course_content_milestone(course_key, content_key, relationship, miles """ relationship_type = _get_milestone_relationship_type(relationship) milestone_obj = serializers.deserialize_milestone(milestone) - internal.CourseContentMilestone.objects.get_or_create( - course_id=unicode(course_key), - content_id=unicode(content_key), - milestone=milestone_obj, - milestone_relationship_type=relationship_type, - active=True, - ) + try: + relationship = internal.CourseContentMilestone.objects.get( + course_id=unicode(course_key), + content_id=unicode(content_key), + milestone=milestone_obj, + milestone_relationship_type=relationship_type + ) + # If the relationship exists, but was inactivated, we can simply turn it back on + if not relationship.active: + _activate_course_content_milestone_relationship(relationship) + except internal.CourseContentMilestone.DoesNotExist: + relationship = internal.CourseContentMilestone.objects.create( + course_id=unicode(course_key), + content_id=unicode(content_key), + milestone=milestone_obj, + milestone_relationship_type=relationship_type, + active=True + ) def delete_course_content_milestone(course_key, content_key, milestone): @@ -260,13 +384,13 @@ def delete_course_content_milestone(course_key, content_key, milestone): No response currently defined for this operation """ try: - record = internal.CourseContentMilestone.objects.get( + relationship = internal.CourseContentMilestone.objects.get( course_id=unicode(course_key), content_id=unicode(content_key), milestone=milestone['id'], active=True, ) - _inactivate_record(record) + _inactivate_course_content_milestone_relationship(relationship) except internal.CourseContentMilestone.DoesNotExist: # If we're being asked to delete a course-content-milestone link # that does not exist in the database then our work is done @@ -342,11 +466,20 @@ def create_user_milestone(user, milestone): No response currently defined for this operation """ milestone_obj = serializers.deserialize_milestone(milestone) - internal.UserMilestone.objects.get_or_create( - user_id=user['id'], - milestone=milestone_obj, - active=True, - ) + try: + relationship = internal.UserMilestone.objects.get( + user_id=user['id'], + milestone=milestone_obj.id, + ) + # If the relationship exists, but was inactivated, we can simply turn it back on + if not relationship.active: + _activate_user_milestone_relationship(relationship) + except internal.UserMilestone.DoesNotExist: + relationship = internal.UserMilestone.objects.create( + user_id=user['id'], + milestone=milestone_obj, + active=True + ) def delete_user_milestone(user, milestone): diff --git a/milestones/tests/test_api.py b/milestones/tests/test_api.py index 68d51985..9e3a5fb0 100644 --- a/milestones/tests/test_api.py +++ b/milestones/tests/test_api.py @@ -38,6 +38,61 @@ def test_add_milestone(self): }) self.assertGreater(milestone['id'], 0) + def test_add_milestone_active_exists(self): + """ Unit Test: test_add_milestone_active_exists""" + milestone_data = { + 'name': 'local_milestone', + 'display_name': 'Local Milestone', + 'namespace': unicode(self.test_course_key), + 'description': 'Local Milestone Description' + } + milestone = api.add_milestone(milestone_data) + self.assertGreater(milestone['id'], 0) + with self.assertNumQueries(1): + milestone = api.add_milestone(milestone_data) + + def test_add_milestone_inactive_to_active(self): + """ Unit Test: test_add_milestone_inactive_milestone_present""" + milestone_data = { + 'name': 'local_milestone', + 'display_name': 'Local Milestone', + 'namespace': unicode(self.test_course_key), + 'description': 'Local Milestone Description' + } + milestone = api.add_milestone(milestone_data) + self.assertGreater(milestone['id'], 0) + api.remove_milestone(milestone['id']) + + with self.assertNumQueries(5): + milestone = api.add_milestone(milestone_data) + + def test_add_milestone_inactive_milestone_with_relationships(self): + """ Unit Test: test_add_milestone_inactive_milestone_with_relationships""" + milestone_data = { + 'name': 'local_milestone', + 'display_name': 'Local Milestone', + 'namespace': unicode(self.test_course_key), + 'description': 'Local Milestone Description' + } + milestone = api.add_milestone(milestone_data) + api.add_course_milestone( + self.test_course_key, + self.relationship_types['REQUIRES'], + milestone + ) + api.add_course_content_milestone( + self.test_course_key, + self.test_content_key, + self.relationship_types['FULFILLS'], + milestone + ) + api.add_user_milestone(self.serialized_test_user, milestone) + self.assertGreater(milestone['id'], 0) + api.remove_milestone(milestone['id']) + + with self.assertNumQueries(5): + milestone = api.add_milestone(milestone_data) + def test_add_milestone_invalid_data_throws_exceptions(self): """ Unit Test: test_add_milestone_invalid_namespaces_throw_exceptions""" with self.assertNumQueries(0): @@ -186,6 +241,35 @@ def test_add_course_milestone(self): ) self.assertEqual(len(fulfiller_milestones), 1) + def test_add_course_milestone_active_exists(self): + """ Unit Test: test_add_course_milestone """ + api.add_course_milestone( + self.test_course_key, + self.relationship_types['REQUIRES'], + self.test_milestone + ) + with self.assertNumQueries(2): + api.add_course_milestone( + self.test_course_key, + self.relationship_types['REQUIRES'], + self.test_milestone + ) + + def test_add_course_milestone_inactive_to_active(self): + """ Unit Test: test_add_course_milestone """ + api.add_course_milestone( + self.test_course_key, + self.relationship_types['REQUIRES'], + self.test_milestone + ) + api.remove_course_milestone(self.test_course_key, self.test_milestone) + with self.assertNumQueries(5): + api.add_course_milestone( + self.test_course_key, + self.relationship_types['REQUIRES'], + self.test_milestone + ) + def test_add_course_milestone_bogus_course_key(self): """ Unit Test: test_add_course_milestone_bogus_course_key """ with self.assertNumQueries(0): @@ -327,7 +411,7 @@ def test_remove_course_milestone(self): self.relationship_types['REQUIRES'] ) self.assertEqual(len(requirer_milestones), 1) - with self.assertNumQueries(3): + with self.assertNumQueries(4): api.remove_course_milestone(self.test_course_key, self.test_milestone) requirer_milestones = api.get_course_milestones(self.test_course_key) self.assertEqual(len(requirer_milestones), 0) @@ -368,6 +452,43 @@ def test_add_course_content_milestone(self): ) self.assertEqual(len(fulfiller_milestones), 1) + def test_add_course_content_milestone_active_exists(self): + """ Unit Test: test_add_course_content_milestone """ + api.add_course_content_milestone( + self.test_course_key, + self.test_content_key, + self.relationship_types['REQUIRES'], + self.test_milestone + ) + with self.assertNumQueries(2): + api.add_course_content_milestone( + self.test_course_key, + self.test_content_key, + self.relationship_types['REQUIRES'], + self.test_milestone + ) + + def test_add_course_content_milestone_inactive_to_active(self): + """ Unit Test: test_add_course_content_milestone """ + api.add_course_content_milestone( + self.test_course_key, + self.test_content_key, + self.relationship_types['REQUIRES'], + self.test_milestone + ) + api.remove_course_content_milestone( + self.test_course_key, + self.test_content_key, + self.test_milestone + ) + with self.assertNumQueries(5): + api.add_course_content_milestone( + self.test_course_key, + self.test_content_key, + self.relationship_types['REQUIRES'], + self.test_milestone + ) + def test_add_course_content_milestone_bogus_content_key(self): """ Unit Test: test_add_course_content_milestone_bogus_content_key """ with self.assertNumQueries(0): @@ -436,7 +557,7 @@ def test_remove_course_content_milestone(self): self.relationship_types['REQUIRES'] ) self.assertEqual(len(requirer_milestones), 1) - with self.assertNumQueries(3): + with self.assertNumQueries(4): api.remove_course_content_milestone( self.test_course_key, self.test_content_key, @@ -468,6 +589,21 @@ def test_add_user_milestone(self): api.add_user_milestone(self.serialized_test_user, self.test_milestone) self.assertTrue(api.user_has_milestone(self.serialized_test_user, self.test_milestone)) + def test_add_user_milestone_active_exists(self): + """ Unit Test: test_add_user_milestone """ + api.add_user_milestone(self.serialized_test_user, self.test_milestone) + with self.assertNumQueries(1): + api.add_user_milestone(self.serialized_test_user, self.test_milestone) + self.assertTrue(api.user_has_milestone(self.serialized_test_user, self.test_milestone)) + + def test_add_user_milestone_inactive_to_active(self): + """ Unit Test: test_add_user_milestone """ + api.add_user_milestone(self.serialized_test_user, self.test_milestone) + api.remove_user_milestone(self.serialized_test_user, self.test_milestone) + with self.assertNumQueries(4): + api.add_user_milestone(self.serialized_test_user, self.test_milestone) + self.assertTrue(api.user_has_milestone(self.serialized_test_user, self.test_milestone)) + def test_add_user_milestone_bogus_user(self): """ Unit Test: test_add_user_milestone_bogus_user """ with self.assertNumQueries(0):