mattermost / mattermost-plugin-jira

JIRA plugin for Mattermost 🔌
https://mattermost.gitbook.io/plugin-jira/
Apache License 2.0
98 stars 128 forks source link

Add support for DM subscriptions based on "watching" an issue in Jira #507

Open mickmister opened 4 years ago

mickmister commented 4 years ago

Summary

In order to get a DM notification from the Jira plugin, the user has to meet certain criteria like being assigned or mentioned. If a user is "watching" an issue in Jira's system, they could want to receive notifications for any changes on the issue. This can be a system-wide setting combined with a user-specific setting.

Additional Info

We can use IssueService.GetWatchers(issueID string) during the webhook event processing to compare against our users' settings.

Note that this does not look possible in the GitHub plugin based on GitHub's documented API.

haardikdharma10 commented 4 years ago

Hi @mickmister, I'd like to work on this. Can you help me on getting started?

mickmister commented 4 years ago

@haardikdharma10 Sure, thanks for your interest in the ticket!

First, you'll need to set up your development environment. Here's some docs to help get started. Join the Jira plugin channel on our community server and we can discuss the feature or any questions you might have!

mickmister commented 4 years ago

I'm thinking we will add some functionality added to line 60 below, that will call a new function defined on *Plugin to handle the watchers.

https://github.com/mattermost/mattermost-plugin-jira/blob/36bcf271f93be5f241c3b0e212b38405662cd61a/server/webhook_worker.go#L56-L63

The function should do the following:

Here's some code to create a new notification to append to the array:

https://github.com/mattermost/mattermost-plugin-jira/blob/36bcf271f93be5f241c3b0e212b38405662cd61a/server/webhook_parser.go#L302-L308

haardikdharma10 commented 4 years ago

Thanks for such a brief explanation @mickmister, really appreciated. After going through the docs and some minimal research, first question I have here is, we can have 2 things here, a type called 'Watcher' and a type called 'Watches' wherein the latter will look something like this:

type Watches struct {
    Self       string     `json:"self,omitempty" structs:"self,omitempty"`
    WatchCount int        `json:"watchCount,omitempty" structs:"watchCount,omitempty"`
    IsWatching bool       `json:"isWatching,omitempty" structs:"isWatching,omitempty"`
    Watchers   []*Watcher `json:"watchers,omitempty" structs:"watchers,omitempty"`
}

This will represent a type of how many and which users are "observing" a Jira issue to track the status / updates.

Watcher, which will look something like this:

type Watcher struct {
    Self        string `json:"self,omitempty" structs:"self,omitempty"`
    Name        string `json:"name,omitempty" structs:"name,omitempty"`
    AccountID   string `json:"accountId,omitempty" structs:"accountId,omitempty"`
    DisplayName string `json:"displayName,omitempty" structs:"displayName,omitempty"`
    Active      bool   `json:"active,omitempty" structs:"active,omitempty"`
}

This will represent a common user that "observes" the issue. Do we need both of them or only one?

The function signature, according to me should be somewhat like: func (s *IssueService) GetWatchers(issueID string) (*[]User, *Response, error)

The REST API docs here were also helpful (unfortunately, there is no ready code snippet in go). With this, I am still confused on how to go about the second bullet you mentioned and the array part. I think I have understood the job in bits and pieces and just want to integrate things and get it done!

mickmister commented 4 years ago

@haardikdharma10 Thanks for the explanations of your strategy. I have some questions and suggestions:

The REST API docs here were also helpful (unfortunately, there is no ready code snippet in go).

In the description, I had mentioned IssueService, but I actually meant to say was the service at client.Jira.Issue when using the Jira client from go-jira. We have a name collision of internal and external library names for IssueService there. There is a GetWatchers function here that will return what we need. It actually has the exact same signature as the one you suggested

func (s *IssueService) GetWatchers(issueID string) (*[]User, *Response, error) {

I'm not sure why we need to add the Watches and Watcher datatypes. Can you please explain what the roles of these datatypes are? When/why are they accessed, etc. This is how I see the process going for handling the users watching the ticket:


In one of the code snippets in my previous comment, there is a variable wh that has a notifications field, that has been filled by ParseWebhook. This notifications field is the array I've been referring to:

https://github.com/mattermost/mattermost-plugin-jira/blob/36bcf271f93be5f241c3b0e212b38405662cd61a/server/webhook.go#L30-L38

At this point, it is filled with any users that were mentioned in a comment, or a separate notification in the array for the assignee of the ticket. We basically want to add any watchers to this array, but avoid adding any duplicates that are already in the array. Editing the wh.notifications array should be all we need to do.

Please let me know your thoughts @haardikdharma10 🙂

mickmister commented 4 years ago

Hi @haardikdharma10, how are you doing? Do you have any thoughts or questions on my proposal above?

mickmister commented 3 years ago

Hi @haardikdharma10, hope everything is well. Is it okay if I unassign you from this ticket, so another developer may work work on the ticket?

haardikdharma10 commented 3 years ago

Hey @mickmister, sure.

wiersgallak commented 1 year ago

Closing issues due to inactivity. This issue can be re-opened with more interest from our community.

mickmister commented 1 year ago

@wiersgallak Heads up that issues that are intended to be closed due to not being part of planned work, should be closed via the "Close as not planned" option in GitHub's UI. Otherwise it can be mistaken as "completed".

CleanShot 2023-07-06 at 14 35 49

Also, this issue in particular has an open PR from Brightscout https://github.com/mattermost/mattermost-plugin-jira/pull/872, so maybe it shouldn't be closed. This feature is very high impact. It's currently just waiting on QA review, though currently not prioritized or testable due to https://github.com/mattermost/mattermost-plugin-jira/issues/945

wiersgallak commented 1 year ago

Thanks for catching it and letting me know the appropriate process. Noted for future updates.