softdevteam / mattermost-github-integration

GitHub integration for Mattermost
Other
79 stars 50 forks source link

The initial ping from GHE will fail with organizational hooks #51

Open jcaesar opened 5 years ago

jcaesar commented 5 years ago

The cause is rather simply that server.py:41 tries to access a repository entry. The fix can be equally simple.

diff --git a/mattermostgithub/server.py b/mattermostgithub/server.py
index ad26bec..9daee73 100644
--- a/mattermostgithub/server.py
+++ b/mattermostgithub/server.py
@@ -38,7 +38,12 @@ def root():

     msg = ""
     if event == "ping":
-        msg = "ping from %s" % data['repository']['full_name']
+        if 'repository' in data:
+            msg = "ping from repository %s" % data['repository']['full_name']
+        elif 'organization' in data:
+            msg = "ping from organization %s by user %s" % (data['organization']['login'], data['sender']['login'])
+        else:
+            msg = "ominous ping…"
     elif event == "pull_request":
         if data['action'] == "opened":
             msg = PullRequest(data).opened()

But now you have me wondering: are organizational hooks even expected to work?

ptersilie commented 5 years ago

I've been using an organizational hook for years now and haven't had any issues. I wonder if they changed the ping event, because I'm fairly sure this used to work. In that case your change is probably enough to make this work. It seems that none of the other events this integration currently supports uses the organization field (though note that the supported events list is incomplete).