Detecting errors properly on points

taiga/projects/userstories/api.py

@@ -16,8 +16,6 @@
 # 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 _
@@ -126,18 +124,26 @@ class UserStoryViewSet(OCCResourceMixin, VotedResourceMixin, HistoryResourceMixi
         super().pre_save(obj)
 
     def post_save(self, obj, created=False):
-        # Code related to the hack of pre_save method. Rather,
+        # Code related to the hack of pre_save method. Rather, this is the continuation of it.
-        # 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):
+            Points = apps.get_model("projects", "Points")
-                for role_id, points_id in self._role_points.items():
+            RolePoints = apps.get_model("userstories", "RolePoints")
-                    role_points = RolePoints.objects.get(role__id=role_id, user_story_id=obj.pk)
+
+            for role_id, points_id in self._role_points.items():
+                try:
+                    role_points = RolePoints.objects.get(role__id=role_id, user_story_id=obj.pk,
+                                                         role__computable=True)
+                except (ValueError, RolePoints.DoesNotExist):
+                    raise exc.BadRequest({"points": _("Invalid role id '{role_id}'").format(
+                                                                            role_id=role_id)})
+
+                try:
                     role_points.points = Points.objects.get(id=points_id, project_id=obj.project_id)
-                    role_points.save()
+                except (ValueError, Points.DoesNotExist):
+                    raise exc.BadRequest({"points": _("Invalid points id '{points_id}'").format(
+                                                                             points_id=points_id)})
+
+                role_points.save()
 
         super().post_save(obj, created)
 

tests/integration/test_userstories.py

@@ -248,33 +248,42 @@ def test_update_userstory_points(client):
     f.PointsFactory.create(project=project, value=1)
     points3 = f.PointsFactory.create(project=project, value=2)
 
-    us = f.UserStoryFactory.create(project=project, owner=user1, status__project=project, milestone__project=project)
+    us = f.UserStoryFactory.create(project=project,owner=user1, status__project=project,
+                                   milestone__project=project)
     usdata = UserStorySerializer(us).data
 
     url = reverse("userstories-detail", args=[us.pk])
 
     client.login(user1)
 
-    # Api should ignore invalid values
+    # invalid role
     data = {}
     data["version"] = usdata["version"]
     data["points"] = copy.copy(usdata["points"])
-    data["points"].update({'2000': points3.pk})
+    data["points"].update({"222222": points3.pk})
 
     response = client.json.patch(url, json.dumps(data))
-    assert response.status_code == 200, str(response.content)
+    assert response.status_code == 400
-    assert response.data["points"] == usdata['points']
+
+    # invalid point
+    data = {}
+    data["version"] = usdata["version"]
+    data["points"] = copy.copy(usdata["points"])
+    data["points"].update({str(role1.pk): "999999"})
+
+    response = client.json.patch(url, json.dumps(data))
+    assert response.status_code == 400
 
     # Api should save successful
     data = {}
-    data["version"] = usdata["version"] + 1
+    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))
     us = models.UserStory.objects.get(pk=us.pk)
     usdatanew = UserStorySerializer(us).data
-    assert response.status_code == 200
+    assert response.status_code == 200, str(response.content)
     assert response.data["points"] == usdatanew['points']
     assert response.data["points"] != usdata['points']