[GH-618]: Added slack attachment for webhook posts, in order to comment, edit, and close issues.

[Nityanand13]

Summary

Whenever a user creates an issue on github then three actions will be shown in the custom post created by the plugin's bot. The three actions are:

• Comment
• Edit
• Close

With the help of the above mentioned actions user can create a comment, edit, close, and reopen a github issue from the mattermost. As you see we haven't made a reopen button so when a user closes an issue then this close button will turn to reopen.

Ticket Link

Issue: #618

[mattermost-build]

Hello @Nityanand13,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

[hanzei]

Using the Comment feature posts this message in the thread, which is misleading as no message from mattermost got attached.

[Nityanand13]

That is a very nifty feature tada

One broad question before I dive into the code too much: The implementation currently uses a custom post and a webapp model. Could this feature also be implemented using interactive dialogues and post actions? That way it's supported on mobile.

@hanzei I think we can not implement this feature using interactive dialogues because in that we do not have any case in which we can select more than one option for a particular field. For here if we want to edit an issue then there are fields like labels in which there might be a case in which we need to select more than one option from the list which would not be possible using interactive dialogues.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 1.32450% with 596 lines in your changes missing coverage. Please review.

Project coverage is 14.79%. Comparing base (da4c4df) to head (70ae885). Report is 10 commits behind head on master.

:exclamation: Current head 70ae885 differs from pull request most recent head 39fe3cf

Please upload reports for the commit 39fe3cf to get more accurate results.

Files	Patch %	Lines
server/plugin/api.go	2.02%	385 Missing and 2 partials :warning:
server/plugin/utils.go	0.00%	117 Missing :warning:
server/plugin/webhook.go	0.00%	58 Missing :warning:
server/plugin/command.go	0.00%	18 Missing :warning:
server/plugin/plugin.go	0.00%	13 Missing :warning:
server/plugin/mm_34646_token_refresh.go	0.00%	3 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #636      +/-   ##
==========================================
- Coverage   16.16%   14.79%   -1.37%     
==========================================
  Files          17       15       -2     
  Lines        6021     5563     -458     
==========================================
- Hits          973      823     -150     
+ Misses       5003     4697     -306     
+ Partials       45       43       -2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[mattermost-build]

This PR has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!

[hanzei]

@Nityanand13 @Kshitij-Katiyar Could one of you please resolve the merge conflicts?

[Kshitij-Katiyar]

@Nityanand13 @Kshitij-Katiyar Could one of you please resolve the merge conflicts?

@hanzei sure

[mickmister]

@asaadmahmood Can I get your opinion on this feature?

@ayusht2810 Are you able to get this running on a Cloud server so we can give access to others for review?

[ayusht2810]

@ayusht2810 Are you able to get this running on a Cloud server so we can give access to others for review?

@mickmister Do you mean to create a cloud server and deploy the build for this PR? If yes, what should I use for client ID and client secret for OAuth?

[mickmister]

@ayusht2810 You should be able to use the Use Preregistered OAuth Application feature here. Then /github connect should just work without any further configuration.

[ayusht2810]

@mickmister Created a cloud server for testing and review of the PR and configured the plugin on it as well. https://github-issues-test.test.mattermost.cloud

[mickmister]

@asaadmahmood Would you be up for taking a look at this feature? @ayusht2810 can give you access to the cloud server if you want to review the feature in action

[hanzei]

Happy to take a look afterwards

[ayusht2810]

@hanzei fixed the review fixes and resolved the conflicts.

[mattermost-build]

This PR has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!

[mickmister]

@ayusht2810 I'd like to make sure the UX for this feature is approved before going forward with the PR. It seems off to me to have a destructive button ("Close" in this case) be so readily available and visible. That's typically not what I would expect to see in this context. Maybe we can have a "Edit" button, which can open a modal to give all the options? Are you able to provide more screenshots and/or gifs of the feature working?

[ayusht2810]

@mickmister Below are different screenshots for the feature. Please let me know if anything else is needed:

• Issue creation post
• Create comment modal
• Comment added post
• Update issue modal
• Issue updated added post
• Updated post after after updating issue (same post as previous)
• Close issue modal
• Updated post after closing issue
• Reopen modal

[mickmister]

@matthewbirtch This feature was suggested by a previous PM on the integrations, to essentially mirror slack's integration that has this functionality https://github.com/mattermost/mattermost-plugin-github/issues/618

The main thing I'm weary of in this feature is that we are surfacing a Close button on the post, when that's not an action a user would typically want to take within Mattermost. Personally I think we should remove the Close button. Do you have any thoughts on this?

[matthewbirtch]

@matthewbirtch This feature was suggested by a previous PM on the integrations, to essentially mirror slack's integration that has this functionality #618

The main thing I'm weary of in this feature is that we are surfacing a Close button on the post, when that's not an action a user would typically want to take within Mattermost. Personally I think we should remove the Close button. Do you have any thoughts on this?

@mickmister I don't have a problem with the Close button, to be honest. Because it launches a modal, it's not really prone to misclicks. If someone thinks they have enough context within Mattermost to be able to close an issue, I don't have a big problem with giving them that ability. What are your specific concerns?

I have some unrelated thoughts about tweaks to styling, but I'll reserve that until it's ready for that.

[mickmister]

I don't have a problem with the Close button, to be honest. Because it launches a modal, it's not really prone to misclicks. If someone thinks they have enough context within Mattermost to be able to close an issue, I don't have a big problem with giving them that ability. What are your specific concerns?

@matthewbirtch That all makes sense. My main concern is the "danger" color in the channel. These posts are mostly meant to be informational, and user actions will mostly not be taken. Having the danger passively "at your fingertips" seems alarming to me. I think the likelihood of closing is much lower than commenting and editing as well.

[matthewbirtch]

@matthewbirtch That all makes sense. My main concern is the "danger" color in the channel. These posts are mostly meant to be informational, and user actions will mostly not be taken. Having the danger passively "at your fingertips" seems alarming to me. I think the likelihood of closing is much lower than commenting and editing as well.

Fair points @mickmister. However, the purpose of this PR is to introduce more interactivity to the post to allow users to take action. This could reduce the need to switch out to GitHub and remain in Mattermost. If we're allowing editing or commenting, why not closing? We could remove the danger color though - that might make it less alarming. I'd be ok with that since it's not immediately destructive (due to the modal that opens) and would reduce the prominance it creates.

[matthewbirtch]

Is it possible to get this up on a test server to play with as well? The button styles don't look quite right to me either.

[raghavaggarwal2308]

@matthewbirtch Please refer this message: https://hub.mattermost.com/partners/pl/bdbujx3ebibuumiifmymwtuj8c

[raghavaggarwal2308]

@matthewbirtch Updated the post acc to your suggestion. Please re-review.

[matthewbirtch]

@matthewbirtch Updated the post acc to your suggestion. Please re-review.

Thanks @raghavaggarwal2308, the screenshot looks good, but could you update the test server? These changes aren't showing for me there.

[raghavaggarwal2308]

Thanks @raghavaggarwal2308, the screenshot looks good, but could you update the test server? These changes aren't showing for me there.

@matthewbirtch The test server is updated now.

[matthewbirtch]

Thanks @raghavaggarwal2308. A few details:

1. Since this post shows when a new issue is created, I wonder if we need a line at the beginning of these posts that says '{User} created a new issue in {owner/repo}'. Otherwise it doesn't really provide a reason why this is getting posted in the channel.

2. I'm also thinking that the details of the issue should show within a card UI element.

3. I'm seeing extra space above the buttons if there are no assignee/label fields showing. See comparison below:

4. Also, I'm wondering why we show the issue number first in the title, but in the hover popover, we show the issue number after the name and not bolded. I think we should be consistent here and show the issue number after the title.

5. On the mobile app, I'm seeing extra space below the issue title and the title is not linked like it is on the webapp. The title is also inconsistent with webapp as it doesn't include the issue number as part of the title. In addition, I think there should be a paragraph space here above the labels/assignee fields. Also noticed these fields are appearing in a different order than on the webapp.

6. When I update an issue from the edit modal, a reply is posted that says Updated GitHub issue #80 from a message. I think we can remove 'from a message' as it seems odd in this context.

Let me know if any of this is out of scope for this PR

[raghavaggarwal2308]

@matthewbirtch All these suggestions will take some time to fix. Also, we will have to handle styling of many things:

1. Adding the username and org link in the pretext. This will become quire easy in a slack attachment.
2. Rendering as a card.

. What are your opinions on replacing the custom post with a slack attachment for webapp as well?

[matthewbirtch]

@matthewbirtch All these suggestions will take some time to fix. Also, we will have to handle styling of many things:

1. Adding the username and org link in the pretext. This will become quire easy in a slack attachment.
2. Rendering as a card.

. What are your opinions on replacing the custom post with a slack attachment for webapp as well?

I think that could work quite well and makes a lot of sense to me. @mickmister any objections to this?

[mickmister]

@raghavaggarwal2308 Sounds good to me :+1: