Issue #1228 - Makes labeler route slimmer

- Removes the GET. Flask handles this already for us
- Creates only 1 requests for setting labels instead of two, probably
avoiding race condition mentioned in #1553
- Sends plain/text messages where it is granted.
- Logs the failure when tagging didn’t happen properly
- Renames set_label as set_labels and rationalises the functions

webcompat/webhooks/__init__.py

@@ -4,59 +4,70 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-'''Flask Blueprint for our "webhooks" module, which we use to do cool things
-with GitHub events and actions.
+"""Flask Blueprint for our "webhooks" module.webhooks
 
-See https://developer.github.com/webhooks/ for what is possible.'''
+See https://developer.github.com/webhooks/ for what is possible."""
 
 import json
+import logging
 
 from flask import abort
 from flask import Blueprint
 from flask import request
 
-from helpers import parse_and_set_label
-from helpers import set_label
+from helpers import extract_browser_label
+from helpers import set_labels
 from helpers import signature_check
 
 from webcompat import app
 
-
 webhooks = Blueprint('webhooks', __name__, url_prefix='/webhooks')
 
 
-@webhooks.route('/labeler', methods=['GET', 'POST'])
+@webhooks.route('/labeler', methods=['POST'])
 def hooklistener():
-    '''Listen for the "issues" webhook event, parse the body
-
-       Method posts back labels and dumps data to a local db.
-       Only in response to the 'opened' action, though.
-    '''
-    if request.method == 'GET':
-        abort(403)
-    elif request.method == 'POST':
-        event_type = request.headers.get('X-GitHub-Event')
-        post_signature = request.headers.get('X-Hub-Signature')
-        if post_signature:
-            key = app.config['HOOK_SECRET_KEY']
-            payload = json.loads(request.data)
-            if not signature_check(key, post_signature, request.data):
-                abort(401)
-            if event_type == 'issues':
-                if payload.get('action') == 'opened':
-                    issue_body = payload.get('issue')['body']
-                    issue_title = payload.get('issue')['title']
-                    issue_number = payload.get('issue')['number']
-                    parse_and_set_label(issue_body, issue_number)
-                    # Setting "Needs Triage" label by default
-                    # to all the new issues raised
-                    set_label('status-needstriage', issue_number)
-                    return ('gracias, amigo.', 200)
+    """Listen for the "issues" webhook event.
+
+    - Only in response to the 'opened' action (for now).
+    - Add label needstriage to the issue
+    - Add label for the browser name
+    """
+    event_type = request.headers.get('X-GitHub-Event')
+    post_signature = request.headers.get('X-Hub-Signature')
+    if post_signature:
+        key = app.config['HOOK_SECRET_KEY']
+        payload = json.loads(request.data)
+        if not signature_check(key, post_signature, request.data):
+            abort(401)
+        if event_type == 'issues':
+            if payload.get('action') == 'opened':
+                # Setting "Needs Triage" label by default
+                # to all the new issues raised
+                labels = ['status-needstriage']
+                issue_body = payload.get('issue')['body']
+                issue_number = payload.get('issue')['number']
+                browser_label = extract_browser_label(issue_body)
+                if browser_label:
+                    labels.append(browser_label)
+                # Sending a request to set labels
+                response = set_labels(labels, issue_number)
+                if response.status_code == 200:
+                    return ('gracias, amigo.', 200,
+                            {'Content-Type': 'plain/text'})
                 else:
-                    return ('cool story, bro.', 200)
-            elif event_type == 'ping':
-                return ('pong', 200)
+                    # Logging the ip and url for investigation
+                    log = app.logger
+                    log.setLevel(logging.INFO)
+                    msg = 'failed to set labels on issue {issue}'.format(
+                        issue=issue_number)
+                    log.info(msg)
             else:
-                abort(403)
+                return ('cool story, bro.', 200,
+                        {'Content-Type': 'plain/text'})
+        elif event_type == 'ping':
+            return ('pong', 200, {'Content-Type': 'plain/text'})
         else:
-            abort(401)
+            abort(403)
+    else:
+        # No signature.
+        abort(401)

webcompat/webhooks/helpers.py

@@ -13,17 +13,7 @@
 from webcompat import app
 
 
-def api_post(endpoint, payload, issue):
-    '''Helper method to post junk to GitHub.'''
-    headers = {
-        'Authorization': 'token {0}'.format(app.config['OAUTH_TOKEN'])
-    }
-    uri = 'https://api.github.com/repos/{0}/{1}/{2}'.format(
-        app.config['ISSUES_REPO_URI'], issue, endpoint)
-    requests.post(uri, data=json.dumps(payload), headers=headers)
-
-
-def parse_and_set_label(body, issue_number):
+def extract_browser_label(body):
     '''Parse the labels from the body in comment:
 
     <!-- @browser: value -->. Currently this only handles a single label,
@@ -37,18 +27,21 @@ def parse_and_set_label(body, issue_number):
         # ('browser', 'firefox')
         browser = match_list.groups(0)[1].lower()
         dash_browser = '-'.join(browser.split())
-        set_label('browser-' + dash_browser, issue_number)
+        return 'browser-{name}'.format(name=dash_browser)
+    else:
+        return None
 
 
-def set_label(label, issue_number):
+def set_labels(payload, issue_number):
     '''Do a GitHub POST request to set a label for the issue.'''
     # POST /repos/:owner/:repo/issues/:number/labels
     # ['Label1', 'Label2']
-    payload = [label]
-    api_post('labels', payload, issue_number)
-
-
-def dump_to_db(title, body, issue_number):
+    headers = {
+        'Authorization': 'token {0}'.format(app.config['OAUTH_TOKEN'])
+    }
+    uri = 'https://api.github.com/repos/{0}/{1}/labels'.format(
+        app.config['ISSUES_REPO_URI'], issue_number)
+    return requests.post(uri, data=json.dumps(payload), headers=headers)
 
 
 def compare_digest(x, y):