From 3b9363fb787f21ed45e039d0fe851d20da389cc1 Mon Sep 17 00:00:00 2001
From: Aaron Meurer <asmeurer@gmail.com>
Date: Fri, 1 Mar 2019 16:26:34 -0700
Subject: [PATCH] Always add the person who opened the PR in the authors list

Otherwise a PR by someone commits without a login, plus commits from someone else
won't credit the person who opened the PR. Making a PR from someone else's
branch without adding commits to it is rare.
---
 fix_authors.py                 |   3 +-
 sympy_bot/tests/test_webapp.py | 125 +++++++++++++++++++++++++++++++++
 sympy_bot/webapp.py            |   4 +-
 3 files changed, 128 insertions(+), 4 deletions(-)

diff --git a/fix_authors.py b/fix_authors.py
index 3e803da..5383b44 100755
--- a/fix_authors.py
+++ b/fix_authors.py
@@ -64,8 +64,7 @@ def main():
             if commit['author']:
                 users.add(commit['author']['login'])
 
-        if not users:
-            users = {pull_request.json()['head']['user']['login']}
+        users.add(pull_request.json()['head']['user']['login'])
 
         pr_users[pr] = users
 
diff --git a/sympy_bot/tests/test_webapp.py b/sympy_bot/tests/test_webapp.py
index bc36a3c..18ef9d1 100644
--- a/sympy_bot/tests/test_webapp.py
+++ b/sympy_bot/tests/test_webapp.py
@@ -1742,3 +1742,128 @@ async def test_bad_version_file(action):
     }
     assert patch_urls == []
     assert patch_data == []
+
+
+@parametrize('action', ['opened', 'reopened', 'synchronize', 'edited'])
+@parametrize('include_extra', [True, False])
+async def test_no_user_logins_in_commits(action, include_extra):
+    event_data = {
+        'pull_request': {
+            'number': 1,
+            'state': 'open',
+            'merged': False,
+            'comments_url': comments_url,
+            'commits_url': commits_url,
+            'head': {
+                'user': {
+                    'login': 'asmeurer',
+                    },
+            },
+            'base': {
+                'repo': {
+                    'contents_url': contents_url,
+                    'html_url': html_url,
+                },
+            },
+            'body': valid_PR_description,
+            'statuses_url': statuses_url,
+        },
+        'action': action,
+    }
+
+
+    commits = [
+        {
+            'author': None,
+            'commit': {
+                'message': "A good commit",
+            },
+            'sha': 'a109f824f4cb2b1dd97cf832f329d59da00d609a',
+        },
+    ]
+    if include_extra:
+        commits += [
+            {
+                'author': {
+                    'login': 'certik',
+                },
+                'commit': {
+                    'message': "A good commit",
+                },
+                'sha': 'a109f824f4cb2b1dd97cf832f329d59da00d609a',
+            },
+        ]
+
+    # No comment from sympy-bot
+    comments = [
+        {
+            'user': {
+                'login': 'asmeurer',
+            },
+        },
+        {
+            'user': {
+                'login': 'certik',
+            },
+        },
+    ]
+
+    version_file = {
+        'content': base64.b64encode(b'__version__ = "1.2.1.dev"\n'),
+        }
+
+    getiter = {
+        commits_url: commits,
+        comments_url: comments,
+    }
+
+    getitem = {
+        version_url: version_file,
+    }
+    post = {
+        comments_url: {
+            'html_url': comment_html_url,
+        },
+        statuses_url: {},
+    }
+
+    event = _event(event_data)
+
+    gh = FakeGH(getiter=getiter, getitem=getitem, post=post)
+
+    await router.dispatch(event, gh)
+
+    getitem_urls = gh.getitem_urls
+    getiter_urls = gh.getiter_urls
+    post_urls = gh.post_urls
+    post_data = gh.post_data
+    patch_urls = gh.patch_urls
+    patch_data = gh.patch_data
+
+    assert getiter_urls == list(getiter)
+    assert getitem_urls == list(getitem)
+    assert post_urls == [comments_url, statuses_url]
+    assert len(post_data) == 2
+    # Comments data
+    assert post_data[0].keys() == {"body"}
+    comment = post_data[0]["body"]
+    assert ":white_check_mark:" in comment
+    assert ":x:" not in comment
+    assert "new trig solvers" in comment
+    assert "error" not in comment
+    assert "https://github.com/sympy/sympy-bot" in comment
+    for line in valid_PR_description:
+        assert line in comment
+    assert "good order" in comment
+    assert "@asmeurer" in comment
+    if include_extra:
+        assert "@certik" in comment
+    # Statuses data
+    assert post_data[1] == {
+        "state": "success",
+        "target_url": comment_html_url,
+        "description": "The release notes look OK",
+        "context": "sympy-bot/release-notes",
+    }
+    assert patch_urls == []
+    assert patch_data == []
diff --git a/sympy_bot/webapp.py b/sympy_bot/webapp.py
index 9b29c26..d6fb2de 100644
--- a/sympy_bot/webapp.py
+++ b/sympy_bot/webapp.py
@@ -82,8 +82,8 @@ async def pull_request_comment(event, gh):
         if BEGIN_RELEASE_NOTES in message or END_RELEASE_NOTES in message:
             header_in_message = commit['sha']
 
-    if not users:
-        users = {event.data['pull_request']['head']['user']['login']}
+
+    users.add(event.data['pull_request']['head']['user']['login'])
 
     users = sorted(users)
     contents_url = event.data['pull_request']['base']['repo']['contents_url']