mattermost-plugin-jira icon indicating copy to clipboard operation
mattermost-plugin-jira copied to clipboard

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

Open mickmister opened this issue 4 years ago • 11 comments

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.

mickmister avatar Mar 24 '20 04:03 mickmister

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

haardikdharma10 avatar Oct 09 '20 09:10 haardikdharma10

@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 avatar Oct 09 '20 09:10 mickmister

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:

  • Fetch the watchers of the issue using a JIRA API client. The wh variable can be used to get the issue key.
  • For each watcher, check if they have their notification settings in our system set to receive watcher notifications. This will be a new user config setting we will introduce to our JIRA User model's settings, adjustable through a slash command like /jira settings watching on
  • Add each appropriate user to the wh.notifications array
  • Avoid duplicate users that are already in the array

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

mickmister avatar Oct 09 '20 16:10 mickmister

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!

haardikdharma10 avatar Oct 09 '20 19:10 haardikdharma10

@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:

  • Create a new user setting in the plugin here, which will be a boolean of "does the user want to receive MM notifications for tickets they are 'watching' in Jira." Let's call this setting WatchingNotifications. You can see how this can be configured by observing how the Notifications setting is currently handled in the project.
  • When we receive a webhook event, call the GetWatchers function to get the users watching
  • For every user that is watching the ticket, fetch their WatchingNotifications setting from our database.
  • For each one that has their setting true, append their user info to the array

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 avatar Oct 10 '20 21:10 mickmister

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

mickmister avatar Oct 19 '20 18:10 mickmister

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?

mickmister avatar Nov 16 '20 23:11 mickmister

Hey @mickmister, sure.

haardikdharma10 avatar Nov 17 '20 10:11 haardikdharma10

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

wiersgallak avatar Jun 14 '23 01:06 wiersgallak

@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

mickmister avatar Jul 06 '23 18:07 mickmister

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

wiersgallak avatar Jul 06 '23 19:07 wiersgallak