From 28109e0f9ebc915915af754b8a737e328c262e6b Mon Sep 17 00:00:00 2001 From: Eeshan Garg Date: Wed, 1 May 2019 20:37:36 -0230 Subject: [PATCH] webhooks/jira: Ignore comment_created message. comment_created payloads may not contain the required issue data to format a useful notification, therefore, it is better to handle issue comments through issue_updated events (which we already do). Fixes: #11995. --- zerver/webhooks/jira/doc.md | 8 +- .../jira/fixtures/comment_created.json | 93 ------------------- zerver/webhooks/jira/tests.py | 8 -- zerver/webhooks/jira/view.py | 11 ++- 4 files changed, 15 insertions(+), 105 deletions(-) delete mode 100644 zerver/webhooks/jira/fixtures/comment_created.json diff --git a/zerver/webhooks/jira/doc.md b/zerver/webhooks/jira/doc.md index 308821f16e..33ef6315df 100644 --- a/zerver/webhooks/jira/doc.md +++ b/zerver/webhooks/jira/doc.md @@ -14,11 +14,17 @@ These instructions apply to Atlassian Cloud's hosted JIRA, and JIRA Server versi 1. Set **Name** to a name of your choice, such as `Zulip`. Set **Status** to **Enabled**, and set **URL** to the URL constructed above. Select the events you'd like to be notified about, and click **Create**. We - support the following events: + only support the following **Issue** events: * when an issue is created * when an issue is deleted * when an issue is updated + !!! tip "" + If you'd like to be notified of issue comments, you should enable **Issue** + updates; Zulip ignores Jira's webhook **Comment** events as + they are redundant with the Issue events and do not have + complete information in some versions of Jira. + {!congrats.md!} ![](/static/images/integrations/jira/001.png) diff --git a/zerver/webhooks/jira/fixtures/comment_created.json b/zerver/webhooks/jira/fixtures/comment_created.json deleted file mode 100644 index aa4a0c9d23..0000000000 --- a/zerver/webhooks/jira/fixtures/comment_created.json +++ /dev/null @@ -1,93 +0,0 @@ -{ - "timestamp":1546892751184, - "webhookEvent":"comment_created", - "comment":{ - "self":"https://zulipintegrations.atlassian.net/rest/api/2/issue/10000/comment/10000", - "id":"10000", - "author":{ - "self":"https://zulipintegrations.atlassian.net/rest/api/2/user?accountId=5a1b0e47748e770ed11f338c", - "name":"admin", - "key":"admin", - "accountId":"5a1b0e47748e770ed11f338c", - "avatarUrls":{ - "48x48":"https://avatar-cdn.atlassian.com/cd181af88d928dab53c55600c9f7551d?s=48&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fcd181af88d928dab53c55600c9f7551d%3Fd%3Dmm%26s%3D48%26noRedirect%3Dtrue", - "24x24":"https://avatar-cdn.atlassian.com/cd181af88d928dab53c55600c9f7551d?s=24&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fcd181af88d928dab53c55600c9f7551d%3Fd%3Dmm%26s%3D24%26noRedirect%3Dtrue", - "16x16":"https://avatar-cdn.atlassian.com/cd181af88d928dab53c55600c9f7551d?s=16&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fcd181af88d928dab53c55600c9f7551d%3Fd%3Dmm%26s%3D16%26noRedirect%3Dtrue", - "32x32":"https://avatar-cdn.atlassian.com/cd181af88d928dab53c55600c9f7551d?s=32&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fcd181af88d928dab53c55600c9f7551d%3Fd%3Dmm%26s%3D32%26noRedirect%3Dtrue" - }, - "displayName":"Eeshan Garg", - "active":true, - "timeZone":"America/St_Johns" - }, - "body":"Leaving a comment here! :)", - "updateAuthor":{ - "self":"https://zulipintegrations.atlassian.net/rest/api/2/user?accountId=5a1b0e47748e770ed11f338c", - "name":"admin", - "key":"admin", - "accountId":"5a1b0e47748e770ed11f338c", - "avatarUrls":{ - "48x48":"https://avatar-cdn.atlassian.com/cd181af88d928dab53c55600c9f7551d?s=48&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fcd181af88d928dab53c55600c9f7551d%3Fd%3Dmm%26s%3D48%26noRedirect%3Dtrue", - "24x24":"https://avatar-cdn.atlassian.com/cd181af88d928dab53c55600c9f7551d?s=24&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fcd181af88d928dab53c55600c9f7551d%3Fd%3Dmm%26s%3D24%26noRedirect%3Dtrue", - "16x16":"https://avatar-cdn.atlassian.com/cd181af88d928dab53c55600c9f7551d?s=16&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fcd181af88d928dab53c55600c9f7551d%3Fd%3Dmm%26s%3D16%26noRedirect%3Dtrue", - "32x32":"https://avatar-cdn.atlassian.com/cd181af88d928dab53c55600c9f7551d?s=32&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fcd181af88d928dab53c55600c9f7551d%3Fd%3Dmm%26s%3D32%26noRedirect%3Dtrue" - }, - "displayName":"Eeshan Garg", - "active":true, - "timeZone":"America/St_Johns" - }, - "created":"2019-01-07T16:55:51.184-0330", - "updated":"2019-01-07T16:55:51.184-0330", - "jsdPublic":true - }, - "issue":{ - "id":"10000", - "self":"https://zulipintegrations.atlassian.net/rest/api/2/issue/10000", - "key":"ZUL-1", - "fields":{ - "summary":"A minor issue", - "issuetype":{ - "self":"https://zulipintegrations.atlassian.net/rest/api/2/issuetype/10001", - "id":"10001", - "description":"Stories track functionality or features expressed as user goals.", - "iconUrl":"https://zulipintegrations.atlassian.net/secure/viewavatar?size=xsmall&avatarId=10315&avatarType=issuetype", - "name":"Story", - "subtask":false, - "avatarId":10315 - }, - "project":{ - "self":"https://zulipintegrations.atlassian.net/rest/api/2/project/10000", - "id":"10000", - "key":"ZUL", - "name":"zulipintegrations", - "projectTypeKey":"software", - "avatarUrls":{ - "48x48":"https://zulipintegrations.atlassian.net/secure/projectavatar?avatarId=10324", - "24x24":"https://zulipintegrations.atlassian.net/secure/projectavatar?size=small&avatarId=10324", - "16x16":"https://zulipintegrations.atlassian.net/secure/projectavatar?size=xsmall&avatarId=10324", - "32x32":"https://zulipintegrations.atlassian.net/secure/projectavatar?size=medium&avatarId=10324" - } - }, - "assignee":null, - "priority":{ - "self":"https://zulipintegrations.atlassian.net/rest/api/2/priority/3", - "iconUrl":"https://zulipintegrations.atlassian.net/images/icons/priorities/medium.svg", - "name":"Medium", - "id":"3" - }, - "status":{ - "self":"https://zulipintegrations.atlassian.net/rest/api/2/status/10001", - "description":"", - "iconUrl":"https://zulipintegrations.atlassian.net/", - "name":"Backlog", - "id":"10001", - "statusCategory":{ - "self":"https://zulipintegrations.atlassian.net/rest/api/2/statuscategory/2", - "id":2, - "key":"new", - "colorName":"blue-gray", - "name":"To Do" - } - } - } - } -} diff --git a/zerver/webhooks/jira/tests.py b/zerver/webhooks/jira/tests.py index 7f041fbca5..b7767d7971 100644 --- a/zerver/webhooks/jira/tests.py +++ b/zerver/webhooks/jira/tests.py @@ -99,14 +99,6 @@ Adding a comment. Oh, what a comment it is!""" self.send_and_test_stream_message('commented_v1', expected_topic, expected_message) self.send_and_test_stream_message('commented_v2', expected_topic, expected_message) - def test_comment_created_event_type(self) -> None: - expected_topic = "ZUL-1: A minor issue" - expected_message = """Eeshan Garg **added comment to** [ZUL-1](https://zulipintegrations.atlassian.net/browse/ZUL-1): - - -Leaving a comment here! :)""" - self.send_and_test_stream_message('comment_created', expected_topic, expected_message) - def test_comment_edited(self) -> None: expected_topic = "BUG-15: New bug with hook" expected_message = """Leo Franchi **edited comment on** [BUG-15](http://lfranchi.com:8080/browse/BUG-15) (assigned to **Othello, the Moor of Venice**): diff --git a/zerver/webhooks/jira/view.py b/zerver/webhooks/jira/view.py index f76abb04fc..0dfc13547b 100644 --- a/zerver/webhooks/jira/view.py +++ b/zerver/webhooks/jira/view.py @@ -13,9 +13,15 @@ from zerver.lib.webhooks.common import check_send_webhook_message, \ from zerver.models import Realm, UserProfile, get_user_by_delivery_email IGNORED_EVENTS = [ - 'comment_deleted', # we handle issue_update event instead - 'issuelink_created', + # The reason we don't handle any comment_* events is that, depending + # on the JIRA version, comment payloads may or may not contain the + # required information about the issue that was commented on. It + # is better to receive comment notifications via the issue_updated + # event, which seems to contain all necessary data across JIRA versions. + 'comment_deleted', + 'comment_created', 'comment_updated', + 'issuelink_created', 'attachment_created', 'issuelink_deleted', 'sprint_started', @@ -230,7 +236,6 @@ JIRA_CONTENT_FUNCTION_MAPPER = { "jira:issue_created": handle_created_issue_event, "jira:issue_deleted": handle_deleted_issue_event, "jira:issue_updated": handle_updated_issue_event, - "comment_created": handle_updated_issue_event } @api_key_only_webhook_view("JIRA")