Fix wrong handling role points update.

Additionally it moves the logic from serializer to resource.

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 <http://www.gnu.org/licenses/>.
 
+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)

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()
 

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 = """
+    data = "User Story #1\nUser Story #2\n"
-User Story #1
-User Story #2
-"""
     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 = """
+    data = "User Story #1\nUser Story #2\n"
-User Story #1
-User Story #2
-"""
 
     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)]
+
+