From a3cb48cf8e9fbb01c965f12800032d1c672c2ab9 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 15 Oct 2014 19:49:16 +0200 Subject: [PATCH] Fix wrong handling role points update. Additionally it moves the logic from serializer to resource. --- taiga/projects/userstories/api.py | 38 +++++++++++-- taiga/projects/userstories/serializers.py | 13 ----- tests/integration/test_userstories.py | 65 +++++++++++++++++++---- 3 files changed, 89 insertions(+), 27 deletions(-) diff --git a/taiga/projects/userstories/api.py b/taiga/projects/userstories/api.py index f4b604ed..f1a87c9a 100644 --- a/taiga/projects/userstories/api.py +++ b/taiga/projects/userstories/api.py @@ -14,9 +14,13 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from contextlib import suppress + +from django.apps import apps from django.db import transaction from django.utils.translation import ugettext as _ from django.shortcuts import get_object_or_404 +from django.core.exceptions import ObjectDoesNotExist from rest_framework.response import Response from rest_framework import status @@ -62,6 +66,35 @@ class UserStoryViewSet(OCCResourceMixin, HistoryResourceMixin, WatchedResourceMi qs = qs.select_related("milestone", "project") return qs + def pre_save(self, obj): + # This is very ugly hack, but having + # restframework is the only way to do it. + # NOTE: code moved as is from serializer + # to api because is not serializer logic. + related_data = getattr(obj, "_related_data", {}) + self._role_points = related_data.pop("role_points", None) + + if not obj.id: + obj.owner = self.request.user + + super().pre_save(obj) + + def post_save(self, obj, created=False): + # Code related to the hack of pre_save method. Rather, + # this is the continuation of it. + + Points = apps.get_model("projects", "Points") + RolePoints = apps.get_model("userstories", "RolePoints") + + if self._role_points: + with suppress(ObjectDoesNotExist): + for role_id, points_id in self._role_points.items(): + role_points = RolePoints.objects.get(role__id=role_id, user_story_id=obj.pk) + role_points.points = Points.objects.get(id=points_id, project_id=obj.project_id) + role_points.save() + + super().post_save(obj, created) + @list_route(methods=["POST"]) def bulk_create(self, request, **kwargs): serializer = serializers.UserStoriesBulkSerializer(data=request.DATA) @@ -145,8 +178,3 @@ class UserStoryViewSet(OCCResourceMixin, HistoryResourceMixin, WatchedResourceMi return response - def pre_save(self, obj): - if not obj.id: - obj.owner = self.request.user - - super().pre_save(obj) diff --git a/taiga/projects/userstories/serializers.py b/taiga/projects/userstories/serializers.py index ef2607e2..cc7d1638 100644 --- a/taiga/projects/userstories/serializers.py +++ b/taiga/projects/userstories/serializers.py @@ -52,19 +52,6 @@ class UserStorySerializer(serializers.ModelSerializer): depth = 0 read_only_fields = ('created_date', 'modified_date') - def save_object(self, obj, **kwargs): - role_points = obj._related_data.pop("role_points", None) - super().save_object(obj, **kwargs) - - points_modelcls = apps.get_model("projects", "Points") - - if role_points: - for role_id, points_id in role_points.items(): - role_points = obj.role_points.get(role__id=role_id) - role_points.points = points_modelcls.objects.get(id=points_id, - project=obj.project) - role_points.save() - def get_total_points(self, obj): return obj.get_total_points() diff --git a/tests/integration/test_userstories.py b/tests/integration/test_userstories.py index 63721eca..61d31535 100644 --- a/tests/integration/test_userstories.py +++ b/tests/integration/test_userstories.py @@ -1,3 +1,4 @@ +import copy from unittest import mock from django.core.urlresolvers import reverse @@ -11,10 +12,7 @@ pytestmark = pytest.mark.django_db def test_get_userstories_from_bulk(): - data = """ -User Story #1 -User Story #2 -""" + data = "User Story #1\nUser Story #2\n" userstories = services.get_userstories_from_bulk(data) assert len(userstories) == 2 @@ -23,10 +21,7 @@ User Story #2 def test_create_userstories_in_bulk(): - data = """ -User Story #1 -User Story #2 -""" + data = "User Story #1\nUser Story #2\n" with mock.patch("taiga.projects.userstories.services.db") as db: userstories = services.create_userstories_in_bulk(data) @@ -41,7 +36,9 @@ def test_update_userstories_order_in_bulk(): with mock.patch("taiga.projects.userstories.services.db") as db: services.update_userstories_order_in_bulk(data, "backlog_order", project) - db.update_in_bulk_with_ids.assert_called_once_with([1, 2], [{"backlog_order": 1}, {"backlog_order": 2}], + db.update_in_bulk_with_ids.assert_called_once_with([1, 2], + [{"backlog_order": 1}, + {"backlog_order": 2}], model=models.UserStory) @@ -108,3 +105,53 @@ def test_api_update_backlog_order_in_bulk(client): assert response1.status_code == 204, response.data assert response2.status_code == 204, response.data assert response3.status_code == 204, response.data + + +from taiga.projects.userstories.serializers import UserStorySerializer + + +def test_update_userstory_points(client): + user1 = f.UserFactory.create() + user2 = f.UserFactory.create() + project = f.ProjectFactory.create(owner=user1) + + role1 = f.RoleFactory.create(project=project) + role2 = f.RoleFactory.create(project=project) + + member = f.MembershipFactory.create(project=project, user=user1, role=role1) + member = f.MembershipFactory.create(project=project, user=user2, role=role2) + + points1 = f.PointsFactory.create(project=project, value=None) + points2 = f.PointsFactory.create(project=project, value=1) + points3 = f.PointsFactory.create(project=project, value=2) + + us = f.UserStoryFactory.create(project=project, owner=user1) + url = reverse("userstories-detail", args=[us.pk]) + usdata = UserStorySerializer(us).data + + client.login(user1) + + # Api should ignore invalid values + data = {} + data["version"] = usdata["version"] + data["points"] = copy.copy(usdata["points"]) + data["points"].update({'2000':points3.pk}) + + response = client.json.patch(url, json.dumps(data)) + assert response.status_code == 200, response.data + + # Api should save successful + data = {} + data["version"] = usdata["version"] + data["points"] = copy.copy(usdata["points"]) + data["points"].update({str(role1.pk):points3.pk}) + + response = client.json.patch(url, json.dumps(data)) + assert response.status_code == 200, response.data + + us = models.UserStory.objects.get(pk=us.pk) + rp = list(us.role_points.values_list("role_id", "points_id")) + + assert rp == [(role1.pk, points3.pk), (role2.pk, points1.pk)] + +