Fix: watchers not saved when creating issues/tasks/user stories via API ([Bug] #138)

.vscode/settings.json

@@ -0,0 +1,3 @@
+{
+    "python-envs.defaultEnvManager": "ms-python.python:system"
+}

taiga/projects/notifications/mixins.py

@@ -42,7 +42,7 @@ def pre_conditions_on_save(self, obj)
     """
 
     _not_notify = False
-    _old_watchers = None
+    _old_watchers = []
     _old_mentions = []
 
     @detail_route(methods=["POST"])
@@ -294,8 +294,14 @@ def restore_object(self, attrs, instance=None):
         new_watcher_ids = attrs.pop("watchers", None)
         obj = super(EditableWatchedResourceSerializer, self).restore_object(attrs, instance)
 
-        # A partial update can exclude the watchers field or if the new instance can still not be saved
-        if instance is None or new_watcher_ids is None:
+        # A partial update can exclude the watchers field
+        if new_watcher_ids is None:
+            return obj
+
+        # On creation the object hasn't been persisted yet so M2M relations can't be set.
+        # Defer watcher assignment to save().
+        if instance is None:
+            self._pending_watcher_ids = list(new_watcher_ids)
             return obj
 
         new_watcher_ids = set(new_watcher_ids)
@@ -330,6 +336,15 @@ def to_native(self, obj):
 
     def save(self, **kwargs):
         obj = super(EditableWatchedResourceSerializer, self).save(**kwargs)
+
+        # Apply watchers that were deferred during object creation (when instance was None)
+        pending_watcher_ids = getattr(self, '_pending_watcher_ids', None)
+        if pending_watcher_ids is not None:
+            adding_users = get_user_model().objects.filter(id__in=pending_watcher_ids)
+            for user in adding_users:
+                services.add_watcher(obj, user)
+            self._pending_watcher_ids = None
+
         self.fields["watchers"] = WatchersField(required=False)
         obj.watchers = [user.id for user in obj.get_watchers()]
         return obj

tests/integration/test_watch_issues.py

@@ -137,3 +137,94 @@ def test_remove_issue_watcher(client):
     assert response.status_code == 200
     assert response.data['watchers'] == []
     assert response.data['is_watcher'] == False
+
+
+def test_create_issue_with_watchers_via_api(client):
+    """
+    Regression test for: watchers specified at creation time via the API
+    must be persisted (bug: they were silently discarded).
+    """
+    owner = f.UserFactory.create()
+    watcher = f.UserFactory.create()
+    project = f.ProjectFactory.create(owner=owner)
+    f.MembershipFactory.create(project=project, user=owner, is_admin=True)
+    f.MembershipFactory.create(project=project, user=watcher, is_admin=True)
+    url = reverse("issues-list")
+
+    data = {
+        "subject": "Issue with watchers",
+        "project": project.id,
+        "watchers": [watcher.id],
+    }
+    client.login(owner)
+    response = client.json.post(url, json.dumps(data))
+
+    assert response.status_code == 201
+    assert watcher.id in response.data['watchers']
+    assert response.data['total_watchers'] >= 1
+
+
+def test_create_issue_with_multiple_watchers_via_api(client):
+    """
+    Multiple watchers specified at creation time should all be persisted.
+    """
+    owner = f.UserFactory.create()
+    watcher1 = f.UserFactory.create()
+    watcher2 = f.UserFactory.create()
+    project = f.ProjectFactory.create(owner=owner)
+    f.MembershipFactory.create(project=project, user=owner, is_admin=True)
+    f.MembershipFactory.create(project=project, user=watcher1, is_admin=True)
+    f.MembershipFactory.create(project=project, user=watcher2, is_admin=True)
+    url = reverse("issues-list")
+
+    data = {
+        "subject": "Issue with multiple watchers",
+        "project": project.id,
+        "watchers": [watcher1.id, watcher2.id],
+    }
+    client.login(owner)
+    response = client.json.post(url, json.dumps(data))
+
+    assert response.status_code == 201
+    assert watcher1.id in response.data['watchers']
+    assert watcher2.id in response.data['watchers']
+
+
+def test_create_issue_without_watchers_still_works(client):
+    """
+    Regression: creating an issue without specifying watchers should still succeed.
+    """
+    owner = f.UserFactory.create()
+    project = f.ProjectFactory.create(owner=owner)
+    f.MembershipFactory.create(project=project, user=owner, is_admin=True)
+    url = reverse("issues-list")
+
+    data = {
+        "subject": "Issue without watchers",
+        "project": project.id,
+    }
+    client.login(owner)
+    response = client.json.post(url, json.dumps(data))
+
+    assert response.status_code == 201
+    assert response.data['watchers'] == [] or response.data['watchers'] is not None
+
+
+def test_update_issue_watchers_still_works(client):
+    """
+    Regression: updating watchers on an existing issue via PATCH must still work.
+    """
+    owner = f.UserFactory.create()
+    watcher = f.UserFactory.create()
+    project = f.ProjectFactory.create(owner=owner)
+    f.MembershipFactory.create(project=project, user=owner, is_admin=True)
+    f.MembershipFactory.create(project=project, user=watcher, is_admin=True)
+    issue = f.create_issue(owner=owner, project=project)
+    url = reverse("issues-detail", args=(issue.id,))
+
+    client.login(owner)
+    data = {"version": issue.version, "watchers": [watcher.id]}
+    response = client.json.patch(url, json.dumps(data))
+
+    assert response.status_code == 200
+    assert watcher.id in response.data['watchers']

tests/integration/test_watch_tasks.py

@@ -135,3 +135,28 @@ def test_remove_task_watcher(client):
     assert response.status_code == 200
     assert response.data['watchers'] == []
     assert response.data['is_watcher'] == False
+
+
+def test_create_task_with_watchers_via_api(client):
+    """
+    Regression test: watchers specified at task creation time via the API
+    must be persisted (same root bug as issues).
+    """
+    owner = f.UserFactory.create()
+    watcher = f.UserFactory.create()
+    project = f.ProjectFactory.create(owner=owner)
+    f.MembershipFactory.create(project=project, user=owner, is_admin=True)
+    f.MembershipFactory.create(project=project, user=watcher, is_admin=True)
+    url = reverse("tasks-list")
+
+    data = {
+        "subject": "Task with watchers",
+        "project": project.id,
+        "watchers": [watcher.id],
+    }
+    client.login(owner)
+    response = client.json.post(url, json.dumps(data))
+
+    assert response.status_code == 201
+    assert watcher.id in response.data['watchers']
+    assert response.data['total_watchers'] >= 1

tests/integration/test_watch_userstories.py

@@ -135,3 +135,28 @@ def test_remove_user_story_watcher(client):
     assert response.status_code == 200
     assert response.data['watchers'] == []
     assert response.data['is_watcher'] == False
+
+
+def test_create_userstory_with_watchers_via_api(client):
+    """
+    Regression test: watchers specified at user story creation time via the API
+    must be persisted (same root bug as issues).
+    """
+    owner = f.UserFactory.create()
+    watcher = f.UserFactory.create()
+    project = f.ProjectFactory.create(owner=owner)
+    f.MembershipFactory.create(project=project, user=owner, is_admin=True)
+    f.MembershipFactory.create(project=project, user=watcher, is_admin=True)
+    url = reverse("userstories-list")
+
+    data = {
+        "subject": "User story with watchers",
+        "project": project.id,
+        "watchers": [watcher.id],
+    }
+    client.login(owner)
+    response = client.json.post(url, json.dumps(data))
+
+    assert response.status_code == 201
+    assert watcher.id in response.data['watchers']
+    assert response.data['total_watchers'] >= 1