1658841 - Webhooks should include comments added to a bug

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, enhancement)

Description

[Lisset Cuevas]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:79.0) Gecko/20100101 Firefox/79.0

Comment 1

[Lisset Cuevas]

Currently the Push extension does not work with comments and do not include messages that have only a comment added to a bug. It only queues a message if other attributes are changed.

Comment 2

[Lisset Cuevas]

Should there be a new event like "When a comment is added" or should there be included in create or change events?

Comment 3

[David Lawrence [:dkl]]

We could keep this simple and just add it as part of bug.modify event. We add a field to the changes called 'comment' and the 'added' part is the comment itself.

"event": {
      "action": "change",
      "time": "2020-07-24T20:11:22",
      "user": {
          "id": 1,
            "login": "[e-mail]",
            "real_name": "Vagrant User"
      },
      "changes": [
          {
            "field": "comment",
            "removed": "",
            "added": "This is a new comment"
            }
        ],
}

The above is for changed bugs. For new bugs that are created we could add a "description" field that has the first comment since that is also not included at this time. Might be useful to have that as well for the consumers.
I imagine it could be a problem if the comment is really long. We could maybe cut it off at a certain number of characters and add a "truncated":1 to the change attributes. Either way it would be the raw text and not rendered markdown html like you would see in the bug page.

Comment 4

[Lisset Cuevas]

At how many characters should it be cut?

Comment 5

[David Lawrence [:dkl]]

(In reply to Lisset Cuevas from comment #4)

At how many characters should it be cut?

After some research, JSON itself is not really limited on how much text can be used inside a variable. It is mostly to do with the servers that are receiving the data and normally that should be fine. The data is broken up into 8K chunks as it is transmitted across the internet and put back together on the receiving server.

Also looking at Bugzilla/WebService/Bug.pm comments(), we do not truncate the comment text at all currently and noone has complained really.

Let's just leave the entire unformatted comment text inside the JSON data for now and see how it goes. If we ever do have to truncate the text later, we can include a comment_id value that can be used to get the full comment text in a separate REST API call.

Comment 6

[Lisset Cuevas]

Attachment: GitHub Pull Request

Comment 7

[Lisset Cuevas]

I was seeing the current payload of a push message when a comment is created and when an attachment is created and I think it’s better to create two new events, comment create and attachment create and send the same current payload. Is it ok if I do that?
And also we can add the full comment info in the comment payload.

Comment 8

[Lisset Cuevas]

The current payload of a push message when a comment is created is:

{
    "event": {
        "change_set": "14.1598401096.24698",
        "action": "create",
        "time": "2020-08-26T00:18:16",
        "routing_key": "comment.create",
        "target": "comment",
        "user": {
            "real_name": "Vagrant User",
            "id": 1,
            "login": "[e-mail]"
        }
    },
    "comment": {
        "id": 4,
        "body": "",
        "number": 1,
        "bug": { 
                       ...
         }
}

And the current payload of a push message when a attachment is created is:

{
    "event": {
        "change_set": "14.1598401095.9712",
        "time": "2020-08-26T00:18:16",
        "action": "create",
        "routing_key": "attachment.create",
        "user": {
            "login": "[e-mail]",
            "real_name": "Vagrant User",
            "id": 1
        },
        "target": "attachment"
    },
    "attachment": {
        "is_obsolete": false,
        "description": "GitHub Pull Request",
        "is_private": false,
        "is_patch": false,
        "content_type": "text/x-github-pull-request",
        "creation_time": "2020-08-26T00:18:15",
        "id": 3,
        "file_name": "file_4.txt",
        "flags": [],
        "last_change_time": "2020-08-26T00:18:15",
        "bug": {
          ...
         }
}

And of course we'd add the extra parameters of webhooks :)

Comment 9

[David Lawrence [:dkl]]

(In reply to Lisset Cuevas from comment #7)

I was seeing the current payload of a push message when a comment is created and when an attachment is created and I think it’s better to create two new events, comment create and attachment create and send the same current payload. Is it ok if I do that?
And also we can add the full comment info in the comment payload.

Hmm. I think that will be fine. We will need to add two new events to the preferences page for webhooks for the new events. So the new list of choices would be in my opinion:

[ ] When a new bug is created
[ ] When an existing bug is modified
[ ] When a new comment is created
[ ] When a new attachment is created

So are you proposing that if a comment is added at the same time that a bug is modified that the user will receive two separate messages, one for the bug modifications and the second one for the comment that was added? I think that is OK and workable from the client end if so.

Comment 10

[David Lawrence [:dkl]]

Merged to master. Closing.