Merge pull request #173 from taigaio/fix/notifications

Fix notification of objects without permissions

taiga/projects/__init__.py

@@ -0,0 +1,17 @@
+# 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/>.
+
+default_app_config = "taiga.projects.apps.ProjectsAppConfig"

taiga/projects/apps.py

@@ -0,0 +1,47 @@
+# 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.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')

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)

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)
 
 

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

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"])
+    project = f.ProjectFactory()
-    take_snapshot(issue, user=issue.owner)
+    role = f.RoleFactory(project=project, permissions=["view_tasks", "view_issues", "view_us"])
-    task = f.TaskFactory.create(project=issue.project, external_reference=["github", "http://github.com/test/project/issues/11"])
+    membership = f.MembershipFactory(project=project, role=role, user=project.owner)
-    take_snapshot(task, user=task.owner)
+    user = f.UserFactory()
-    us = f.UserStoryFactory.create(project=issue.project, external_reference=["github", "http://github.com/test/project/issues/11"])
+
-    take_snapshot(us, user=us.owner)
+    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",

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)
+    member1 = f.MembershipFactory.create(project=project, role=role1)
-    member2 = f.MembershipFactory.create(project=project)
+    member2 = f.MembershipFactory.create(project=project, role=role1)
-    member3 = f.MembershipFactory.create(project=project)
+    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)
+    role = f.RoleFactory.create(project=project, permissions=['view_issues', 'view_us', 'view_tasks', 'view_wiki_pages'])
-    member2 = f.MembershipFactory.create(project=project)
+    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)