From 4056d5214b648cf0941eb0f8e85d17d3894193eb Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 15 Oct 2014 21:25:58 +0200 Subject: [PATCH] Fix incorrect handling of userstory rolepoints update process. The current implementation allows duplicate points values in one project, and because of it, unexpected MultipleObjectsReturned can be raised with previous update_role_points code. The new one fixes this. Additionally, in some circumstances, the project does not have any points with None as value, that causes also unexpected errors. The new implementation fixes it creating a points instance with None as value if it not exists. --- taiga/projects/models.py | 27 ++++++++++++++++----------- tests/integration/test_userstories.py | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/taiga/projects/models.py b/taiga/projects/models.py index c8306c46..092d5b7f 100644 --- a/taiga/projects/models.py +++ b/taiga/projects/models.py @@ -220,22 +220,27 @@ class Project(ProjectDefaults, TaggedMixin, models.Model): if roles.count() == 0: return - # Get point instance that represent a null/undefined - try: - null_points_value = self.points.get(value=None) - except Points.DoesNotExist: - null_points_value = None - # Iter over all project user stories and create # role point instance for new created roles. if user_stories is None: user_stories = self.user_stories.all() - for story in user_stories: - story_related_roles = Role.objects.filter(role_points__in=story.role_points.all())\ - .distinct() - new_roles = roles.exclude(id__in=story_related_roles) - new_rolepoints = [RolePoints(role=role, user_story=story, points=null_points_value) + # Get point instance that represent a null/undefined + # The current model allows dulplicate values. Because + # of it, we should get all poins with None as value + # and use the first one. + # In case of that not exists, creates one for avoid + # unxpected errors. + none_points = list(self.points.filter(value=None)) + if none_points: + null_points_value = none_points[0] + else: + null_points_value = Points.objects.create(name="?", value=None, project=self) + + for us in user_stories: + usroles = Role.objects.filter(role_points__in=us.role_points.all()).distinct() + new_roles = roles.exclude(id__in=usroles) + new_rolepoints = [RolePoints(role=role, user_story=us, points=null_points_value) for role in new_roles] RolePoints.objects.bulk_create(new_rolepoints) diff --git a/tests/integration/test_userstories.py b/tests/integration/test_userstories.py index fc1d31e5..5544a4a0 100644 --- a/tests/integration/test_userstories.py +++ b/tests/integration/test_userstories.py @@ -155,6 +155,29 @@ def test_update_userstory_points(client): assert rp == [(role1.pk, points3.pk), (role2.pk, points1.pk)] +def test_update_userstory_rolepoints_on_add_new_role(client): + # This test is explicitly without assertions. It simple should + # works without raising any exception. + + user1 = f.UserFactory.create() + user2 = f.UserFactory.create() + project = f.ProjectFactory.create(owner=user1) + + role1 = f.RoleFactory.create(project=project) + + member1 = f.MembershipFactory.create(project=project, user=user1, role=role1) + + points1 = f.PointsFactory.create(project=project, value=2) + + us = f.UserStoryFactory.create(project=project, owner=user1) + # url = reverse("userstories-detail", args=[us.pk]) + # client.login(user1) + + role2 = f.RoleFactory.create(project=project, computable=True) + member2 = f.MembershipFactory.create(project=project, user=user2, role=role2) + us.save() + + def test_archived_filter(client): user = f.UserFactory.create() project = f.ProjectFactory.create(owner=user)