Mm 378 - GitHub - Check for an existing webhook when subscribing to a repo

[sibasankarnayak]

when the [owner / repo] is subscribed , check if have setup any webhook if not let user know they need to do it

ticket here https://github.com/mattermost/mattermost-plugin-github/issues/378

[mattermod]

Hello @sibasankar-demansol,

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.

[codecov-commenter]

Codecov Report

Merging #466 (fec357d) into master (431bac0) will increase coverage by 0.14%. The diff coverage is 20.08%.

@@            Coverage Diff             @@
##           master     #466      +/-   ##
==========================================
+ Coverage   18.49%   18.63%   +0.14%     
==========================================
  Files          12       12              
  Lines        3320     3541     +221     
==========================================
+ Hits          614      660      +46     
- Misses       2667     2842     +175     
  Partials       39       39              

Impacted Files	Coverage Δ
server/plugin/command.go	6.49% <0.00%> (-2.27%)	:arrow_down:
server/plugin/plugin.go	4.08% <20.00%> (+0.19%)	:arrow_up:
server/plugin/template.go	95.94% <100.00%> (+0.72%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 431bac0...fec357d. Read the comment docs.

[mickmister]

@hanzei There's an issue here @sibasankar-demansol ran into where the OAuth token does not have permissions to create webhooks. We currently request the scopes:

• repo
• notifications
• read:org

Looking at the available scopes, we can add these for admin users:

• read:repo_hook
• write:repo_hook
• admin:repo_hook
• admin:org_hook

We can then have a /github connect admin command to apply these scopes. I also think we should be storing the scopes in the GitHubUserInfo struct, so we always know what permissions a user has inside of GitHub.

@hanzei Let me know your thoughts on this, and I'll make a separate ticket for the task.

[sibasankarnayak]

While testing this change I'm running into a similar issue as in #456.

Is there anything specific requirement to test this PR?

Can you let me know the Command you are using for ?

[hanzei]

@sibasankarnayak /github hooks add matterpoll/matterpoll some_secret

[sibasankarnayak]

matterpoll

@sibasankarnayak /github hooks add matterpoll/matterpoll some_secret

@hanzei seems the org/repo doesn't exist , this 404 error occurs only in 2 case

1. trying to create out of scope
2. passed invalid org/repo

you need to pass a valid org/repo i.e. /github webhook add sibasankarnayak/mattermost-plugin-github some_secret htpps://callback_url

[mickmister]

@sanjaydemansol @maisnamrajusingh For any requests you've made on this PR that have been fulfilled, please click "Resolve Conversation" at the bottom of the request's discussion

[maisnamrajusingh]

@mickmister doing now. will update you once i sync with @sibasankarnayak on this.

[mattermod]

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!

/cc @jasonblais @jfrerich @emilyacook

[mickmister]

@hanzei I'm seeing this feature as an opportunity to support multiple webhook secrets. There was recently a request for this functionality. https://community-daily.mattermost.com/core/pl/uf5fzfmu97bbfqfw1qgjjebh4h

Out of scope of this PR, but if we move secrets to the kv store instead of plugin settings, any webhooks we automate the creation of we can store a separate secret for.

@sibasankarnayak Something else that comes to mind: Are we able to put some sort of "id" on the webhook request, so we can know if this plugin was the creator of the webhook? Then when we fetch the webhooks, we can definitively know which ones are for our Mattermost instance.

[mickmister]

There are a lot of unresolved comments on this PR. @sanjaydemansol @maisnamrajusingh Can you please resolve the conversations for any requests you've made on this PR that have been fulfilled? Please click "Resolve Conversation" at the bottom of the request's discussion.

[dipak-demansol]

@mickmister some days ago this PR assigned directly to me by siba but as per your recent comment can you pls tell me should i test this PR?

[mattermod]

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!

/cc @jasonblais @jfrerich @emilyacook