[Tracker-discuss] [issue622] set_roundup_user selects wrong username

[Ezio Melotti]

Ezio Melotti added the comment:

The attached patch fixes the issue.
I couldn't find any method that checks for exact matches, so I kept filter and then checked the GH username manually.  There is a lookup method, but IIUC it only works on the "key field", which in this case is the 'username' (not the GH name).  I was able to use that to lookup python-dev.
The patch also includes tests for the 3 possible scenarios.

----------
status: unread -> chatting

_______________________________________________________
PSF Meta Tracker <metatracker at psf.upfronthosting.co.za>
<http://psf.upfronthosting.co.za/roundup/meta/issue622>
_______________________________________________________
-------------- next part --------------
diff --git a/roundup/github.py b/roundup/github.py
--- a/roundup/github.py
+++ b/roundup/github.py
@@ -141,13 +141,17 @@
         """
         github_username = self.get_github_username()
         user_ids = self.db.user.filter(None, {'github': github_username})
-        if not user_ids:
-            # set bpobot as userid when none is found
-            user_ids = self.db.user.filter(None, {'username': 'python-dev'})
-            if not user_ids:
+        for user_id in user_ids:
+            if self.db.user.get(user_id, 'github') == github_username:
+                break  # found the right user id
+        else:
+            # set python-dev as user id when none is found
+            try:
+                user_id = self.db.user.lookup('python-dev')
+            except KeyError:
                 # python-dev does not exists, anonymous will be used instead
                 return
-        username = self.db.user.get(user_ids[0], 'username')
+        username = self.db.user.get(user_id, 'username')
         self.db.setCurrentUser(username)
 
     def dispatch(self):
diff --git a/test/test_github.py b/test/test_github.py
--- a/test/test_github.py
+++ b/test/test_github.py
@@ -173,6 +173,43 @@
         status = self.db.pull_request.get(prs[0], 'status')
         self.assertEqual(status, "open")
 
+    def testPullRequestEventForUsername(self):
+        # Make sure that only a exact github username match
+        dummy_client = self._make_client("pullrequestevent.txt")
+        self.db.user.create(username="foo", github="_python")
+        self.db.user.create(username="bar", github="a_python")
+        self.db.user.create(username="baz", github="python2")
+        self.db.user.create(username="expected", github="python")
+        handler = GitHubHandler(dummy_client)
+        handler.dispatch()
+        prs = self.db.issue.get('1', 'pull_requests')
+        user_id = self.db.pull_request.get(prs[0], 'creator')
+        self.assertEqual(self.db.user.get(user_id, 'username'), 'expected')
+
+    def testPullRequestEventForUsernameWithNoMatchUsesPythonDev(self):
+        # Make sure that python-dev is picked if there are no matches
+        dummy_client = self._make_client("pullrequestevent.txt")
+        self.db.user.create(username="foo", github="_python")
+        self.db.user.create(username="bar", github="a_python")
+        self.db.user.create(username="python-dev", github="python2")
+        handler = GitHubHandler(dummy_client)
+        handler.dispatch()
+        prs = self.db.issue.get('1', 'pull_requests')
+        user_id = self.db.pull_request.get(prs[0], 'creator')
+        self.assertEqual(self.db.user.get(user_id, 'username'), 'python-dev')
+
+    def testPullRequestEventForUsernameWithNoMatchUsesAnonymous(self):
+        # Make sure that anonymous is picked with no match and no python-dev
+        dummy_client = self._make_client("pullrequestevent.txt")
+        self.db.user.create(username="foo", github="_python")
+        self.db.user.create(username="bar", github="a_python")
+        self.db.user.create(username="baz", github="python2")
+        handler = GitHubHandler(dummy_client)
+        handler.dispatch()
+        prs = self.db.issue.get('1', 'pull_requests')
+        user_id = self.db.pull_request.get(prs[0], 'creator')
+        self.assertEqual(self.db.user.get(user_id, 'username'), 'anonymous')
+
     def testPullRequestEventForMultipleIssueReferenceInTitle(self):
         dummy_client = self._make_client("pullrequestevent3.txt")
         self.db.issue.create(title="Issue 2")