Merge pull request #138 from taigaio/watchers-validation-bug

Add proper validation of watchers for issues, tasks and userstories serializers.

taiga/projects/issues/serializers.py

@@ -19,11 +19,12 @@ from rest_framework import serializers
 from taiga.base.serializers import Serializer, PickleField, NeighborsSerializerMixin
 from taiga.mdrender.service import render as mdrender
 from taiga.projects.validators import ProjectExistsValidator
+from taiga.projects.notifications.validators import WatchersValidator
 
 from . import models
 
 
-class IssueSerializer(serializers.ModelSerializer):
+class IssueSerializer(WatchersValidator, serializers.ModelSerializer):
     tags = PickleField(required=False)
     is_closed = serializers.Field(source="is_closed")
     comment = serializers.SerializerMethodField("get_comment")

taiga/projects/notifications/validators.py

@@ -0,0 +1,45 @@
+# Copyright (C) 2014 Andrey Antukh <niwi@niwi.be>
+# Copyright (C) 2014 Jesús Espino <jespinog@gmail.com>
+# Copyright (C) 2014 David Barragán <bameda@dbarragan.com>
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# 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 django.utils.translation import ugettext_lazy as _
+from rest_framework import serializers
+
+
+class WatchersValidator:
+    def validate_watchers(self, attrs, source):
+        users = attrs[source]
+
+        # Try obtain a valid project
+        if self.object is None and "project" in attrs:
+            project = attrs["project"]
+        elif self.object:
+            project = self.object.project
+        else:
+            project = None
+
+        # If project is empty in all conditions, continue
+        # without errors, because other validator should
+        # validate the empty project field.
+        if not project:
+            return attrs
+
+        # Check if incoming watchers are contained
+        # in project members list
+        result = set(users).difference(set(project.members.all()))
+        if result:
+            raise serializers.ValidationError("Watchers contains invalid users")
+
+        return attrs

taiga/projects/tasks/serializers.py

@@ -21,11 +21,12 @@ from taiga.mdrender.service import render as mdrender
 from taiga.projects.validators import ProjectExistsValidator, TaskStatusExistsValidator
 from taiga.projects.milestones.validators import SprintExistsValidator
 from taiga.projects.userstories.validators import UserStoryExistsValidator
+from taiga.projects.notifications.validators import WatchersValidator
 
 from . import models
 
 
-class TaskSerializer(serializers.ModelSerializer):
+class TaskSerializer(WatchersValidator, serializers.ModelSerializer):
     tags = PickleField(required=False, default=[])
     comment = serializers.SerializerMethodField("get_comment")
     milestone_slug = serializers.SerializerMethodField("get_milestone_slug")

taiga/projects/userstories/serializers.py

@@ -22,6 +22,7 @@ from taiga.base.serializers import Serializer, PickleField, NeighborsSerializerM
 from taiga.mdrender.service import render as mdrender
 from taiga.projects.validators import ProjectExistsValidator, UserStoryStatusExistsValidator
 from taiga.projects.userstories.validators import UserStoryExistsValidator
+from taiga.projects.notifications.validators import WatchersValidator
 
 from . import models
 
@@ -36,7 +37,7 @@ class RolePointsField(serializers.WritableField):
         return json.loads(obj)
 
 
-class UserStorySerializer(serializers.ModelSerializer):
+class UserStorySerializer(WatchersValidator, serializers.ModelSerializer):
     tags = PickleField(default=[], required=False)
     points = RolePointsField(source="role_points", required=False)
     total_points = serializers.SerializerMethodField("get_total_points")

tests/factories.py

@@ -394,8 +394,14 @@ class AttachmentFactory(Factory):
 
 def create_issue(**kwargs):
     "Create an issue and along with its dependencies."
-    owner = kwargs.pop("owner", UserFactory())
+    owner = kwargs.pop("owner", None)
+    if owner is None:
+        owner = UserFactory.create()
+
+    project = kwargs.pop("project", None)
+    if project is None:
         project = ProjectFactory.create(owner=owner)
+
     defaults = {
         "project": project,
         "owner": owner,
@@ -412,8 +418,14 @@ def create_issue(**kwargs):
 
 def create_task(**kwargs):
     "Create a task and along with its dependencies."
-    owner = kwargs.pop("owner", UserFactory())
+    owner = kwargs.pop("owner", None)
+    if not owner:
+        owner = UserFactory.create()
+
+    project = kwargs.pop("project", None)
+    if project is None:
         project = ProjectFactory.create(owner=owner)
+
     defaults = {
         "project": project,
         "owner": owner,
@@ -460,12 +472,19 @@ def create_invitation(**kwargs):
 
 def create_userstory(**kwargs):
     "Create an user story along with its dependencies"
-    project = kwargs.pop("project", create_project())
+
+    owner = kwargs.pop("owner", None)
+    if not owner:
+        owner = UserFactory.create()
+
+    project = kwargs.pop("project", None)
+    if project is None:
+        project = ProjectFactory.create(owner=owner)
 
     defaults = {
         "project": project,
-        "owner": project.owner,
-        "milestone": MilestoneFactory.create(project=project, owner=project.owner)
+        "owner": owner,
+        "milestone": MilestoneFactory.create(project=project, owner=owner)
     }
     defaults.update(kwargs)
 

tests/integration/test_notifications.py

@@ -26,6 +26,9 @@ from .. import factories as f
 from taiga.projects.notifications import services
 from taiga.projects.notifications.choices import NotifyLevel
 from taiga.projects.history.choices import HistoryType
+from taiga.projects.issues.serializers import IssueSerializer
+from taiga.projects.userstories.serializers import UserStorySerializer
+from taiga.projects.tasks.serializers import TaskSerializer
 
 
 pytestmark = pytest.mark.django_db
@@ -246,3 +249,162 @@ def test_resource_notification_test(client, mail):
         response = client.delete(url)
         assert response.status_code == 204
         assert len(mail.outbox) == 2
+
+
+def test_watchers_assignation_for_issue(client):
+    user1 = f.UserFactory.create()
+    user2 = f.UserFactory.create()
+    project1 = f.ProjectFactory.create(owner=user1)
+    project2 = f.ProjectFactory.create(owner=user2)
+    role1 = f.RoleFactory.create(project=project1)
+    role2 = f.RoleFactory.create(project=project2)
+    member1 = f.MembershipFactory.create(project=project1, user=user1, role=role1)
+    member2 = f.MembershipFactory.create(project=project2, user=user2, role=role2)
+
+    client.login(user1)
+
+    issue = f.create_issue(project=project1, owner=user1)
+    data = {"version": issue.version,
+            "watchers": [user1.pk]}
+
+    url = reverse("issues-detail", args=[issue.pk])
+    response = client.json.patch(url, json.dumps(data))
+    assert response.status_code == 200, response.content
+
+
+    issue = f.create_issue(project=project1, owner=user1)
+    data = {"version": issue.version,
+            "watchers": [user1.pk, user2.pk]}
+
+    url = reverse("issues-detail", args=[issue.pk])
+    response = client.json.patch(url, json.dumps(data))
+    assert response.status_code == 400
+
+    issue = f.create_issue(project=project1, owner=user1)
+    data = dict(IssueSerializer(issue).data)
+    data["id"] = None
+    data["version"] = None
+    data["watchers"] = [user1.pk, user2.pk]
+
+    url = reverse("issues-list")
+    response = client.json.post(url, json.dumps(data))
+    assert response.status_code == 400
+
+    # Test the impossible case when project is not
+    # exists in create request, and validator works as expected
+    issue = f.create_issue(project=project1, owner=user1)
+    data = dict(IssueSerializer(issue).data)
+
+    data["id"] = None
+    data["watchers"] = [user1.pk, user2.pk]
+    data["project"] = None
+
+    url = reverse("issues-list")
+    response = client.json.post(url, json.dumps(data))
+    assert response.status_code == 400
+
+
+def test_watchers_assignation_for_task(client):
+    user1 = f.UserFactory.create()
+    user2 = f.UserFactory.create()
+    project1 = f.ProjectFactory.create(owner=user1)
+    project2 = f.ProjectFactory.create(owner=user2)
+    role1 = f.RoleFactory.create(project=project1)
+    role2 = f.RoleFactory.create(project=project2)
+    member1 = f.MembershipFactory.create(project=project1, user=user1, role=role1)
+    member2 = f.MembershipFactory.create(project=project2, user=user2, role=role2)
+
+    client.login(user1)
+
+    task = f.create_task(project=project1, owner=user1)
+    data = {"version": task.version,
+            "watchers": [user1.pk]}
+
+    url = reverse("tasks-detail", args=[task.pk])
+    response = client.json.patch(url, json.dumps(data))
+    assert response.status_code == 200, response.content
+
+
+    task = f.create_task(project=project1, owner=user1)
+    data = {"version": task.version,
+            "watchers": [user1.pk, user2.pk]}
+
+    url = reverse("tasks-detail", args=[task.pk])
+    response = client.json.patch(url, json.dumps(data))
+    assert response.status_code == 400
+
+    task = f.create_task(project=project1, owner=user1)
+    data = dict(TaskSerializer(task).data)
+    data["id"] = None
+    data["version"] = None
+    data["watchers"] = [user1.pk, user2.pk]
+
+    url = reverse("tasks-list")
+    response = client.json.post(url, json.dumps(data))
+    assert response.status_code == 400
+
+    # Test the impossible case when project is not
+    # exists in create request, and validator works as expected
+    task = f.create_task(project=project1, owner=user1)
+    data = dict(TaskSerializer(task).data)
+
+    data["id"] = None
+    data["watchers"] = [user1.pk, user2.pk]
+    data["project"] = None
+
+    url = reverse("tasks-list")
+    response = client.json.post(url, json.dumps(data))
+    assert response.status_code == 400
+
+
+def test_watchers_assignation_for_us(client):
+    user1 = f.UserFactory.create()
+    user2 = f.UserFactory.create()
+    project1 = f.ProjectFactory.create(owner=user1)
+    project2 = f.ProjectFactory.create(owner=user2)
+    role1 = f.RoleFactory.create(project=project1)
+    role2 = f.RoleFactory.create(project=project2)
+    member1 = f.MembershipFactory.create(project=project1, user=user1, role=role1)
+    member2 = f.MembershipFactory.create(project=project2, user=user2, role=role2)
+
+    client.login(user1)
+
+    us = f.create_userstory(project=project1, owner=user1)
+    data = {"version": us.version,
+            "watchers": [user1.pk]}
+
+    url = reverse("userstories-detail", args=[us.pk])
+    response = client.json.patch(url, json.dumps(data))
+    assert response.status_code == 200
+
+
+    us = f.create_userstory(project=project1, owner=user1)
+    data = {"version": us.version,
+            "watchers": [user1.pk, user2.pk]}
+
+    url = reverse("userstories-detail", args=[us.pk])
+    response = client.json.patch(url, json.dumps(data))
+    assert response.status_code == 400
+
+    us = f.create_userstory(project=project1, owner=user1)
+    data = dict(UserStorySerializer(us).data)
+    data["id"] = None
+    data["version"] = None
+    data["watchers"] = [user1.pk, user2.pk]
+
+    url = reverse("userstories-list")
+    response = client.json.post(url, json.dumps(data))
+    assert response.status_code == 400
+
+    # Test the impossible case when project is not
+    # exists in create request, and validator works as expected
+    us = f.create_userstory(project=project1, owner=user1)
+    data = dict(UserStorySerializer(us).data)
+
+    data["id"] = None
+    data["watchers"] = [user1.pk, user2.pk]
+    data["project"] = None
+
+    url = reverse("userstories-list")
+    response = client.json.post(url, json.dumps(data))
+    assert response.status_code == 400

tests/integration/test_tasks.py

@@ -47,6 +47,7 @@ def test_api_update_task_tags(client):
 
 def test_api_create_in_bulk_with_status(client):
     us = f.create_userstory()
+    us.project.default_task_status = f.TaskStatusFactory.create(project=us.project)
     url = reverse("tasks-bulk-create")
     data = {
         "bulk_tasks": "Story #1\nStory #2",
@@ -68,6 +69,7 @@ def test_api_create_invalid_task(client):
     # But the User Story is not associated with the milestone
     us_milestone = f.MilestoneFactory.create()
     us = f.create_userstory(milestone=us_milestone)
+    us.project.default_task_status = f.TaskStatusFactory.create(project=us.project)
     task_milestone = f.MilestoneFactory.create(project=us.project, owner=us.owner)
 
     url = reverse("tasks-list")