From 494f18300a513988391b3e650799f4c51c8fdd28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Wed, 26 Nov 2014 19:09:10 +0100 Subject: [PATCH] Fix notification of objects without permissions --- taiga/projects/__init__.py | 17 +++++++ taiga/projects/apps.py | 47 +++++++++++++++++++ taiga/projects/models.py | 59 +----------------------- taiga/projects/notifications/services.py | 20 ++++++++ taiga/projects/signals.py | 55 ++++++++++++++++++++++ tests/integration/test_github_hook.py | 33 +++++++++---- tests/integration/test_notifications.py | 35 +++++++++----- 7 files changed, 188 insertions(+), 78 deletions(-) create mode 100644 taiga/projects/apps.py diff --git a/taiga/projects/__init__.py b/taiga/projects/__init__.py index e69de29b..48278e73 100644 --- a/taiga/projects/__init__.py +++ b/taiga/projects/__init__.py @@ -0,0 +1,17 @@ +# 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 . + +default_app_config = "taiga.projects.apps.ProjectsAppConfig" diff --git a/taiga/projects/apps.py b/taiga/projects/apps.py new file mode 100644 index 00000000..232efc60 --- /dev/null +++ b/taiga/projects/apps.py @@ -0,0 +1,47 @@ +# 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.apps import AppConfig +from django.apps import apps +from django.db.models import signals + +from . import signals as handlers + + +class ProjectsAppConfig(AppConfig): + name = "taiga.projects" + verbose_name = "Projects" + + def ready(self): + # On membership object is deleted, update role-points relation. + signals.pre_delete.connect(handlers.membership_post_delete, + sender=apps.get_model("projects", "Membership"), + dispatch_uid='membership_pre_delete') + + # On membership object is deleted, update watchers of all objects relation. + signals.post_delete.connect(handlers.update_watchers_on_membership_post_delete, + sender=apps.get_model("projects", "Membership"), + dispatch_uid='update_watchers_on_membership_post_delete') + + # On membership object is deleted, update watchers of all objects relation. + signals.post_save.connect(handlers.create_notify_policy, + sender=apps.get_model("projects", "Membership"), + dispatch_uid='create-notify-policy') + + # On project object is created apply template. + signals.post_save.connect(handlers.project_post_save, + sender=apps.get_model("projects", "Project"), + dispatch_uid='project_post_save') diff --git a/taiga/projects/models.py b/taiga/projects/models.py index 61137e06..4c615d05 100644 --- a/taiga/projects/models.py +++ b/taiga/projects/models.py @@ -31,12 +31,10 @@ from djorm_pgarray.fields import TextArrayField from taiga.permissions.permissions import ANON_PERMISSIONS, USER_PERMISSIONS from taiga.base.tags import TaggedMixin -from taiga.users.models import Role from taiga.base.utils.slug import slugify_uniquely from taiga.base.utils.dicts import dict_sum from taiga.base.utils.sequence import arithmetic_progression from taiga.base.utils.slug import slugify_uniquely_for_queryset -from taiga.projects.notifications.services import create_notify_policy_if_not_exists from . import choices @@ -674,6 +672,8 @@ class ProjectTemplate(models.Model): self.default_owner_role = self.roles[0].get("slug", None) def apply_to_project(self, project): + Role = apps.get_model("users", "Role") + if project.id is None: raise Exception("Project need an id (must be a saved project)") @@ -783,58 +783,3 @@ class ProjectTemplate(models.Model): project.default_severity = Severity.objects.get(name=self.default_options["severity"], project=project) return project - - -# On membership object is deleted, update role-points relation. -@receiver(signals.pre_delete, sender=Membership, dispatch_uid='membership_pre_delete') -def membership_post_delete(sender, instance, using, **kwargs): - instance.project.update_role_points() - - -# On membership object is deleted, update watchers of all objects relation. -@receiver(signals.post_delete, sender=Membership, dispatch_uid='update_watchers_on_membership_post_delete') -def update_watchers_on_membership_post_delete(sender, instance, using, **kwargs): - models = [apps.get_model("userstories", "UserStory"), - apps.get_model("tasks", "Task"), - apps.get_model("issues", "Issue")] - - # `user_id` is used beacuse in some momments - # instance.user can contain pointer to now - # removed object from a database. - for model in models: - model.watchers.through.objects.filter(user_id=instance.user_id).delete() - - -# On membership object is deleted, update watchers of all objects relation. -@receiver(signals.post_save, sender=Membership, dispatch_uid='create-notify-policy') -def create_notify_policy(sender, instance, using, **kwargs): - if instance.user: - create_notify_policy_if_not_exists(instance.project, instance.user) - - -@receiver(signals.post_save, sender=Project, dispatch_uid='project_post_save') -def project_post_save(sender, instance, created, **kwargs): - """ - Populate new project dependen default data - """ - if not created: - return - - if instance._importing: - return - - template = getattr(instance, "creation_template", None) - if template is None: - template = ProjectTemplate.objects.get(slug=settings.DEFAULT_PROJECT_TEMPLATE) - template.apply_to_project(instance) - - instance.save() - - try: - owner_role = instance.roles.get(slug=template.default_owner_role) - except Role.DoesNotExist: - owner_role = instance.roles.first() - - if owner_role: - Membership.objects.create(user=instance.owner, project=instance, role=owner_role, - is_owner=True, email=instance.owner.email) diff --git a/taiga/projects/notifications/services.py b/taiga/projects/notifications/services.py index ba08918e..ecd3a2e8 100644 --- a/taiga/projects/notifications/services.py +++ b/taiga/projects/notifications/services.py @@ -32,6 +32,7 @@ from taiga.projects.history.choices import HistoryType from taiga.projects.history.services import (make_key_from_model_object, get_last_snapshot_for_key, get_model_from_key) +from taiga.permissions.service import user_has_perm from taiga.users.models import User from .models import HistoryChangeNotification @@ -121,6 +122,23 @@ def analize_object_for_watchers(obj:object, history:object): obj.watchers.add(user) +def _filter_by_permissions(obj, user): + UserStory = apps.get_model("userstories", "UserStory") + Issue = apps.get_model("issues", "Issue") + Task = apps.get_model("tasks", "Task") + WikiPage = apps.get_model("wiki", "WikiPage") + + if isinstance(obj, UserStory): + return user_has_perm(user, "view_us", obj) + elif isinstance(obj, Issue): + return user_has_perm(user, "view_issues", obj) + elif isinstance(obj, Task): + return user_has_perm(user, "view_tasks", obj) + elif isinstance(obj, WikiPage): + return user_has_perm(user, "view_wiki_pages", obj) + return False + + def get_users_to_notify(obj, *, discard_users=None) -> list: """ Get filtered set of users to notify for specified @@ -149,6 +167,8 @@ def get_users_to_notify(obj, *, discard_users=None) -> list: if discard_users: candidates = candidates - set(discard_users) + candidates = filter(partial(_filter_by_permissions, obj), candidates) + return frozenset(candidates) diff --git a/taiga/projects/signals.py b/taiga/projects/signals.py index e61722cb..e4366d8d 100644 --- a/taiga/projects/signals.py +++ b/taiga/projects/signals.py @@ -14,7 +14,11 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from django.apps import apps +from django.conf import settings + from taiga.projects.services.tags_colors import update_project_tags_colors_handler, remove_unused_tags +from taiga.projects.notifications.services import create_notify_policy_if_not_exists #################################### @@ -35,3 +39,54 @@ def update_project_tags_when_create_or_edit_taggable_item(sender, instance, **kw def update_project_tags_when_delete_taggable_item(sender, instance, **kwargs): remove_unused_tags(instance.project) instance.project.save() + +def membership_post_delete(sender, instance, using, **kwargs): + instance.project.update_role_points() + + +def update_watchers_on_membership_post_delete(sender, instance, using, **kwargs): + models = [apps.get_model("userstories", "UserStory"), + apps.get_model("tasks", "Task"), + apps.get_model("issues", "Issue")] + + # `user_id` is used beacuse in some momments + # instance.user can contain pointer to now + # removed object from a database. + for model in models: + model.watchers.through.objects.filter(user_id=instance.user_id).delete() + + +def create_notify_policy(sender, instance, using, **kwargs): + if instance.user: + create_notify_policy_if_not_exists(instance.project, instance.user) + + +def project_post_save(sender, instance, created, **kwargs): + """ + Populate new project dependen default data + """ + if not created: + return + + if instance._importing: + return + + + template = getattr(instance, "creation_template", None) + if template is None: + ProjectTemplate = apps.get_model("projects", "ProjectTemplate") + template = ProjectTemplate.objects.get(slug=settings.DEFAULT_PROJECT_TEMPLATE) + template.apply_to_project(instance) + + instance.save() + + Role = apps.get_model("users", "Role") + try: + owner_role = instance.roles.get(slug=template.default_owner_role) + except Role.DoesNotExist: + owner_role = instance.roles.first() + + if owner_role: + Membership = apps.get_model("projects", "Membership") + Membership.objects.create(user=instance.owner, project=instance, role=owner_role, + is_owner=True, email=instance.owner.email) diff --git a/tests/integration/test_github_hook.py b/tests/integration/test_github_hook.py index bb902d15..da3d6ca6 100644 --- a/tests/integration/test_github_hook.py +++ b/tests/integration/test_github_hook.py @@ -75,8 +75,10 @@ def test_push_event_detected(client): def test_push_event_issue_processing(client): creation_status = f.IssueStatusFactory() + role = f.RoleFactory(project=creation_status.project, permissions=["view_issues"]) + membership = f.MembershipFactory(project=creation_status.project, role=role, user=creation_status.project.owner) new_status = f.IssueStatusFactory(project=creation_status.project) - issue = f.IssueFactory.create(status=creation_status, project=creation_status.project) + issue = f.IssueFactory.create(status=creation_status, project=creation_status.project, owner=creation_status.project.owner) payload = {"commits": [ {"message": """test message test TG-%s #%s ok @@ -93,8 +95,10 @@ def test_push_event_issue_processing(client): def test_push_event_task_processing(client): creation_status = f.TaskStatusFactory() + role = f.RoleFactory(project=creation_status.project, permissions=["view_tasks"]) + membership = f.MembershipFactory(project=creation_status.project, role=role, user=creation_status.project.owner) new_status = f.TaskStatusFactory(project=creation_status.project) - task = f.TaskFactory.create(status=creation_status, project=creation_status.project) + task = f.TaskFactory.create(status=creation_status, project=creation_status.project, owner=creation_status.project.owner) payload = {"commits": [ {"message": """test message test TG-%s #%s ok @@ -111,8 +115,10 @@ def test_push_event_task_processing(client): def test_push_event_user_story_processing(client): creation_status = f.UserStoryStatusFactory() + role = f.RoleFactory(project=creation_status.project, permissions=["view_us"]) + membership = f.MembershipFactory(project=creation_status.project, role=role, user=creation_status.project.owner) new_status = f.UserStoryStatusFactory(project=creation_status.project) - user_story = f.UserStoryFactory.create(status=creation_status, project=creation_status.project) + user_story = f.UserStoryFactory.create(status=creation_status, project=creation_status.project, owner=creation_status.project.owner) payload = {"commits": [ {"message": """test message test TG-%s #%s ok @@ -130,8 +136,10 @@ def test_push_event_user_story_processing(client): def test_push_event_processing_case_insensitive(client): creation_status = f.TaskStatusFactory() + role = f.RoleFactory(project=creation_status.project, permissions=["view_tasks"]) + membership = f.MembershipFactory(project=creation_status.project, role=role, user=creation_status.project.owner) new_status = f.TaskStatusFactory(project=creation_status.project) - task = f.TaskFactory.create(status=creation_status, project=creation_status.project) + task = f.TaskFactory.create(status=creation_status, project=creation_status.project, owner=creation_status.project.owner) payload = {"commits": [ {"message": """test message test tg-%s #%s ok @@ -291,12 +299,17 @@ def test_issues_event_bad_issue(client): def test_issue_comment_event_on_existing_issue_task_and_us(client): - issue = f.IssueFactory.create(external_reference=["github", "http://github.com/test/project/issues/11"]) - take_snapshot(issue, user=issue.owner) - task = f.TaskFactory.create(project=issue.project, external_reference=["github", "http://github.com/test/project/issues/11"]) - take_snapshot(task, user=task.owner) - us = f.UserStoryFactory.create(project=issue.project, external_reference=["github", "http://github.com/test/project/issues/11"]) - take_snapshot(us, user=us.owner) + project = f.ProjectFactory() + role = f.RoleFactory(project=project, permissions=["view_tasks", "view_issues", "view_us"]) + membership = f.MembershipFactory(project=project, role=role, user=project.owner) + user = f.UserFactory() + + issue = f.IssueFactory.create(external_reference=["github", "http://github.com/test/project/issues/11"], owner=project.owner, project=project) + take_snapshot(issue, user=user) + task = f.TaskFactory.create(external_reference=["github", "http://github.com/test/project/issues/11"], owner=project.owner, project=project) + take_snapshot(task, user=user) + us = f.UserStoryFactory.create(external_reference=["github", "http://github.com/test/project/issues/11"], owner=project.owner, project=project) + take_snapshot(us, user=user) payload = { "action": "created", diff --git a/tests/integration/test_notifications.py b/tests/integration/test_notifications.py index 997c0d5b..097f5263 100644 --- a/tests/integration/test_notifications.py +++ b/tests/integration/test_notifications.py @@ -101,11 +101,16 @@ def test_analize_object_for_watchers(): def test_users_to_notify(): project = f.ProjectFactory.create() - issue = f.IssueFactory.create(project=project) + role1 = f.RoleFactory.create(project=project, permissions=['view_issues']) + role2 = f.RoleFactory.create(project=project, permissions=[]) - member1 = f.MembershipFactory.create(project=project) - member2 = f.MembershipFactory.create(project=project) - member3 = f.MembershipFactory.create(project=project) + member1 = f.MembershipFactory.create(project=project, role=role1) + member2 = f.MembershipFactory.create(project=project, role=role1) + member3 = f.MembershipFactory.create(project=project, role=role1) + member4 = f.MembershipFactory.create(project=project, role=role1) + member5 = f.MembershipFactory.create(project=project, role=role2) + + issue = f.IssueFactory.create(project=project, owner=member4.user) policy_model_cls = apps.get_model("notifications", "NotifyPolicy") @@ -147,12 +152,20 @@ def test_users_to_notify(): assert len(users) == 2 assert users == {member1.user, issue.get_owner()} + # Test with watchers without permissions + issue.watchers.add(member5.user) + users = services.get_users_to_notify(issue) + assert len(users) == 2 + assert users == {member1.user, issue.get_owner()} + + def test_send_notifications_using_services_method(settings, mail): settings.CHANGE_NOTIFICATIONS_MIN_INTERVAL = 1 project = f.ProjectFactory.create() - member1 = f.MembershipFactory.create(project=project) - member2 = f.MembershipFactory.create(project=project) + role = f.RoleFactory.create(project=project, permissions=['view_issues', 'view_us', 'view_tasks', 'view_wiki_pages']) + member1 = f.MembershipFactory.create(project=project, role=role) + member2 = f.MembershipFactory.create(project=project, role=role) history_change = MagicMock() history_change.user = {"pk": member1.user.pk} @@ -170,7 +183,7 @@ def test_send_notifications_using_services_method(settings, mail): history_delete.type = HistoryType.delete # Issues - issue = f.IssueFactory.create(project=project) + issue = f.IssueFactory.create(project=project, owner=member2.user) take_snapshot(issue) services.send_notifications(issue, history=history_create) @@ -183,7 +196,7 @@ def test_send_notifications_using_services_method(settings, mail): # Userstories - us = f.UserStoryFactory.create() + us = f.UserStoryFactory.create(project=project, owner=member2.user) take_snapshot(us) services.send_notifications(us, history=history_create) @@ -195,7 +208,7 @@ def test_send_notifications_using_services_method(settings, mail): history=history_delete) # Tasks - task = f.TaskFactory.create() + task = f.TaskFactory.create(project=project, owner=member2.user) take_snapshot(task) services.send_notifications(task, history=history_create) @@ -207,7 +220,7 @@ def test_send_notifications_using_services_method(settings, mail): history=history_delete) # Wiki pages - wiki = f.WikiPageFactory.create() + wiki = f.WikiPageFactory.create(project=project, owner=member2.user) take_snapshot(wiki) services.send_notifications(wiki, history=history_create) @@ -230,7 +243,7 @@ def test_resource_notification_test(client, settings, mail): user1 = f.UserFactory.create() user2 = f.UserFactory.create() project = f.ProjectFactory.create(owner=user1) - role = f.RoleFactory.create(project=project) + role = f.RoleFactory.create(project=project, permissions=["view_issues"]) member1 = f.MembershipFactory.create(project=project, user=user1, role=role, is_owner=True) member2 = f.MembershipFactory.create(project=project, user=user2, role=role) issue = f.IssueFactory.create(owner=user2, project=project)