Fix PR push times

cron/poll_pull_requests.py

@@ -63,7 +63,7 @@ def poll_pull_requests(api):
                 voting_window = gh.voting.get_extended_voting_window(api, settings.URN)
 
             # is our PR in voting window?
-            in_window = gh.prs.is_pr_in_voting_window(pr, voting_window)
+            in_window = gh.prs.is_pr_in_voting_window(api, pr, voting_window)
 
             if is_approved:
                 __log.info("PR %d status: will be approved", pr_num)

github_api/prs.py

@@ -122,14 +122,52 @@ def close_pr(api, urn, pr):
     return api("patch", path, json=data)
 
 
-def get_pr_last_updated(pr_data):
+def get_events(api, pr_owner, pr_repo):
+    """
+    a helper for getting the github events on a given repo... useful for
+    finding out the last push on a repo.
+    """
+    # TODO: this only gets us the latest 90 events, should we do more?
+    # i.e. can someone do 90 events on a repo in 30 seconds?
+    events = []
+    for i in range(1, 4):
+        path = "/repos/{pr_owner}/{pr_repo}/events?page={page}".format(
+                pr_owner=pr_owner, pr_repo=pr_repo, page=i)
+        events += api("get", path, json={})
+
+    return events
+
+
+def get_pr_last_updated(api, pr_data):
     """ a helper for finding the utc datetime of the last pr branch
     modifications """
-    repo = pr_data["head"]["repo"]
-    if repo:
-        return arrow.get(repo["pushed_at"])
-    else:
-        return None
+
+    pr_ref = pr_data["head"]["ref"]
+    pr_repo = pr_data["head"]["repo"]["name"]
+    pr_owner = pr_data["user"]["login"]
+
+    events = get_events(api, pr_owner, pr_repo)
+    events = filter(lambda e: e["type"] == "PushEvent", events)
+
+    # Gives the full ref name "ref/heads/my_branch_name", but we just
+    # want my_branch_name, so isolate it...
+    def ref_name(e):
+        '/'.join(e["payload"]["ref"].split("/")[3:]) == pr_ref
+
+    events = list(filter(ref_name, events))
+
+    if len(events) == 0:
+        # if we can't get good data, fall back to repo push time
+        repo = pr_data["head"]["repo"]
+        if repo:
+            return arrow.get(repo["pushed_at"])
+        else:
+            return None
+
+    last_updated = max(sorted(map(lambda e: e["created_at"], events)))
+    last_updated = arrow.get(last_updated)
+
+    return max(last_updated, arrow.get(pr_data["created_at"]))
 
 
 def get_pr_comments(api, urn, pr_num):
@@ -176,7 +214,7 @@ def get_ready_prs(api, urn, window):
         pr_num = pr["number"]
 
         now = arrow.utcnow()
-        updated = get_pr_last_updated(pr)
+        updated = get_pr_last_updated(api, pr)
         if updated is None:
             comments.leave_deleted_comment(api, urn, pr["number"])
             close_pr(api, urn, pr)
@@ -211,17 +249,17 @@ def get_ready_prs(api, urn, window):
                 close_pr(api, urn, pr)
 
 
-def voting_window_remaining_seconds(pr, window):
+def voting_window_remaining_seconds(api, pr, window):
     now = arrow.utcnow()
-    updated = get_pr_last_updated(pr)
+    updated = get_pr_last_updated(api, pr)
     if updated is None:
         return math.inf
     delta = (now - updated).total_seconds()
     return window - delta
 
 
-def is_pr_in_voting_window(pr, window):
-    return voting_window_remaining_seconds(pr, window) <= 0
+def is_pr_in_voting_window(api, pr, window):
+    return voting_window_remaining_seconds(api, pr, window) <= 0
 
 
 def get_pr_reviews(api, urn, pr_num):
@@ -272,7 +310,7 @@ def post_accepted_status(api, urn, pr, voting_window, votes, total, threshold,
                          meritocracy_satisfied):
     sha = pr["head"]["sha"]
 
-    remaining_seconds = voting_window_remaining_seconds(pr, voting_window)
+    remaining_seconds = voting_window_remaining_seconds(api, pr, voting_window)
     remaining_human = misc.seconds_to_human(remaining_seconds)
     votes_summary = formatted_votes_short_summary(votes, total, threshold, meritocracy_satisfied)
 
@@ -284,7 +322,7 @@ def post_rejected_status(api, urn, pr, voting_window, votes, total, threshold,
                          meritocracy_satisfied):
     sha = pr["head"]["sha"]
 
-    remaining_seconds = voting_window_remaining_seconds(pr, voting_window)
+    remaining_seconds = voting_window_remaining_seconds(api, pr, voting_window)
     remaining_human = misc.seconds_to_human(remaining_seconds)
     votes_summary = formatted_votes_short_summary(votes, total, threshold, meritocracy_satisfied)
 
@@ -296,7 +334,7 @@ def post_pending_status(api, urn, pr, voting_window, votes, total, threshold,
                         meritocracy_satisfied):
     sha = pr["head"]["sha"]
 
-    remaining_seconds = voting_window_remaining_seconds(pr, voting_window)
+    remaining_seconds = voting_window_remaining_seconds(api, pr, voting_window)
     remaining_human = misc.seconds_to_human(remaining_seconds)
     votes_summary = formatted_votes_short_summary(votes, total, threshold, meritocracy_satisfied)
 

tests/github_api/prs_test.py

@@ -5,19 +5,42 @@
 from github_api import prs, API
 
 
-def create_mock_pr(number, title, pushed_at):
+def create_mock_pr(number, title, pushed_at, created_at=None):
     return {
         "number": number,
         "title": title,
         "statuses_url": "statuses_url/{}".format(number),
         "head": {
             "repo": {
-                "pushed_at": pushed_at
-            }
-        }
+                "pushed_at": pushed_at,
+                "name": "test_repo",
+            },
+            "ref": "test_ref"
+        },
+        "user": {
+            "login": "test_user",
+        },
+        "created_at": created_at,
     }
 
 
+def create_mock_events(events):
+
+    def produce_event(event):
+        if event[0] == "PushEvent":
+            return {
+                    "type": "PushEvent",
+                    "payload": {
+                        "ref": "/ref/heads/%s" % event[1],
+                        },
+                    "created_at": event[2],
+                    }
+        else:
+            return {"type": event[0]}
+
+    return list(map(produce_event, events))
+
+
 class TestPRMethods(unittest.TestCase):
     def test_statuses_returns_passed_travis_build(self):
         test_data = [[{"state": "success",
@@ -103,3 +126,48 @@ def get_is_mergeable_side_effect(api, urn, pr_num):
         ready_prs_list = [pr for pr in ready_prs]
         self.assertTrue(len(ready_prs_list) is 1)
         self.assertTrue(ready_prs_list[0].get("number") is 11)
+
+    @patch("github_api.prs.get_events")
+    def test_get_pr_last_updated_with_early_events(self, mock_get_events):
+        mock_get_events.return_value = \
+            create_mock_events([("PushEvent", "test_ref", "2017-01-01T00:00:10Z"),
+                                ("PushEvent", "blah", "2017-01-03T00:00:10Z")])
+
+        api = MagicMock()
+        api.BASE_URL = "api_base_url"
+
+        last_updated = prs.get_pr_last_updated(api,
+                                               create_mock_pr(10, "OK", "2017-01-05T00:00:00Z",
+                                                              "2017-01-02T00:00:00Z"))
+
+        self.assertEqual(last_updated, arrow.get("2017-01-02T00:00:00Z"))
+
+    @patch("github_api.prs.get_events")
+    def test_get_pr_last_updated_without_events(self, mock_get_events):
+        mock_get_events.return_value = \
+                create_mock_events([("PublicEvent",),
+                                    ("PushEvent", "blah", "2017-01-03T00:00:10Z")])
+
+        api = MagicMock()
+        api.BASE_URL = "api_base_url"
+
+        last_updated = prs.get_pr_last_updated(api,
+                                               create_mock_pr(10, "OK", "2017-01-05T00:00:00Z",
+                                                              "2017-01-02T00:00:00Z"))
+
+        self.assertEqual(last_updated, arrow.get("2017-01-05T00:00:00Z"))
+
+    @patch("github_api.prs.get_events")
+    def test_get_pr_last_updated_with_events(self, mock_get_events):
+        mock_get_events.return_value = \
+            create_mock_events([("PushEvent", "test_ref", "2017-01-04T00:00:10Z"),
+                                ("PushEvent", "blah", "2017-01-03T00:00:10Z")])
+
+        api = MagicMock()
+        api.BASE_URL = "api_base_url"
+
+        last_updated = prs.get_pr_last_updated(api,
+                                               create_mock_pr(10, "OK", "2017-01-05T00:00:00Z",
+                                                              "2017-01-02T00:00:00Z"))
+
+        self.assertEqual(last_updated, arrow.get("2017-01-04T00:00:10Z"))