From bc95282cfdc1497f4be4aa6f2feaea34a4b06d5e Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 23 Oct 2014 20:45:11 +0200 Subject: [PATCH] Add proper validation of watchers for issues, tasks and userstories serializers. --- taiga/projects/issues/serializers.py | 3 +- taiga/projects/notifications/validators.py | 45 ++++++ taiga/projects/tasks/serializers.py | 3 +- taiga/projects/userstories/serializers.py | 3 +- tests/factories.py | 33 ++++- tests/integration/test_notifications.py | 162 +++++++++++++++++++++ 6 files changed, 239 insertions(+), 10 deletions(-) create mode 100644 taiga/projects/notifications/validators.py diff --git a/taiga/projects/issues/serializers.py b/taiga/projects/issues/serializers.py index 1a270ee5..d3f5d740 100644 --- a/taiga/projects/issues/serializers.py +++ b/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") diff --git a/taiga/projects/notifications/validators.py b/taiga/projects/notifications/validators.py new file mode 100644 index 00000000..5088d446 --- /dev/null +++ b/taiga/projects/notifications/validators.py @@ -0,0 +1,45 @@ +# Copyright (C) 2014 Andrey Antukh +# Copyright (C) 2014 Jesús Espino +# Copyright (C) 2014 David Barragán +# 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 . + +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 diff --git a/taiga/projects/tasks/serializers.py b/taiga/projects/tasks/serializers.py index 24b679d4..20bd17dc 100644 --- a/taiga/projects/tasks/serializers.py +++ b/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") diff --git a/taiga/projects/userstories/serializers.py b/taiga/projects/userstories/serializers.py index cc7d1638..06aaed22 100644 --- a/taiga/projects/userstories/serializers.py +++ b/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") diff --git a/tests/factories.py b/tests/factories.py index a833b996..c2d0f7cf 100644 --- a/tests/factories.py +++ b/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()) - project = ProjectFactory.create(owner=owner) + 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()) - project = ProjectFactory.create(owner=owner) + 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) diff --git a/tests/integration/test_notifications.py b/tests/integration/test_notifications.py index 09447dda..411f67bd 100644 --- a/tests/integration/test_notifications.py +++ b/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