[ADD] runbot_merge: tag PR to signify state changes

runbot_merge/github.py

@@ -50,6 +50,19 @@ class GH(object):
         self.comment(pr, message)
         self('PATCH', 'pulls/{}'.format(pr), json={'state': 'closed'})
 
+    def change_tags(self, pr, from_, to_):
+        to_add, to_remove = to_ - from_, from_ - to_
+        for t in to_remove:
+            r = self('DELETE', 'issues/{}/labels/{}'.format(pr, t), check=False)
+            r.raise_for_status()
+            # successful deletion or attempt to delete a tag which isn't there
+            # is fine, otherwise trigger an error
+            if r.status_code not in (200, 404):
+                r.raise_for_status()
+
+        if to_add:
+            self('POST', 'issues/{}/labels'.format(pr), json=list(to_add))
+
     def fast_forward(self, branch, sha):
         try:
             self('patch', 'git/refs/heads/{}'.format(branch), json={'sha': sha})

runbot_merge/models/pull_requests.py

@@ -177,8 +177,53 @@ class Project(models.Model):
                     # create staging branch from tmp
                     for r, it in meta.items():
                         it['gh'].set_ref('staging.{}'.format(branch.name), it['head'])
+
+                    # creating the staging doesn't trigger a write on the prs
+                    # and thus the ->staging taggings, so do that by hand
+                    Tagging = self.env['runbot_merge.pull_requests.tagging']
+                    for pr in st.mapped('batch_ids.prs'):
+                        Tagging.create({
+                            'pull_request': pr.number,
+                            'repository': pr.repository.id,
+                            'state_from': pr._tagstate,
+                            'state_to': 'staged',
+                        })
+
                     logger.info("Created staging %s", st)
 
+        Repos = self.env['runbot_merge.repository']
+        ghs = {}
+        self.env.cr.execute("""
+        SELECT
+            t.repository as repo_id,
+            t.pull_request as pr_number,
+            array_agg(t.id) as ids,
+            (array_agg(t.state_from ORDER BY t.id))[1] as state_from,
+            (array_agg(t.state_to ORDER BY t.id DESC))[1] as state_to
+        FROM runbot_merge_pull_requests_tagging t
+        GROUP BY t.repository, t.pull_request
+        """)
+        to_remove = []
+        for repo_id, pr, ids, from_, to_ in self.env.cr.fetchall():
+            repo = Repos.browse(repo_id)
+            from_tags = _TAGS[from_ or False]
+            to_tags = _TAGS[to_ or False]
+
+            gh = ghs.get(repo)
+            if not gh:
+                gh = ghs[repo] = repo.github()
+
+            try:
+                gh.change_tags(pr, from_tags, to_tags)
+            except Exception:
+                _logger.exception(
+                    "Error while trying to change the tags of %s:%s from %s to %s",
+                    repo.name, pr, from_tags, to_tags,
+                )
+            else:
+                to_remove.extend(ids)
+        self.env['runbot_merge.pull_requests.tagging'].browse(to_remove).unlink()
+
     def is_timed_out(self, staging):
         return fields.Datetime.from_string(staging.staged_at) + datetime.timedelta(minutes=self.ci_timeout) < datetime.datetime.now()
 
@@ -505,6 +550,99 @@ class PullRequests(models.Model):
             self._cr, 'runbot_merge_unique_pr_per_target', self._table, ['number', 'target', 'repository'])
         return res
 
+    @property
+    def _tagstate(self):
+        if self.state == 'ready' and self.staging_id.heads:
+            return 'staged'
+        return self.state
+
+    @api.model
+    def create(self, vals):
+        pr = super().create(vals)
+        self.env['runbot_merge.pull_requests.tagging'].create({
+            'pull_request': pr.number,
+            'repository': pr.repository.id,
+            'state_from': False,
+            'state_to': pr._tagstate,
+        })
+        return pr
+
+    @api.multi
+    def write(self, vals):
+        oldstate = { pr: pr._tagstate for pr in self }
+        w = super().write(vals)
+        for pr in self:
+            before, after = oldstate[pr], pr._tagstate
+            if after != before:
+                self.env['runbot_merge.pull_requests.tagging'].create({
+                    'pull_request': pr.number,
+                    'repository': pr.repository.id,
+                    'state_from': oldstate[pr],
+                    'state_to': pr._tagstate,
+                })
+        return w
+
+    @api.multi
+    def unlink(self):
+        for pr in self:
+            self.env['runbot_merge.pull_requests.tagging'].create({
+                'pull_request': pr.number,
+                'repository': pr.repository.id,
+                'state_from': pr._tagstate,
+                'state_to': False,
+            })
+        return super().unlink()
+
+
+_TAGS = {
+    False: set(),
+    'opened': {'seen 🙂'},
+}
+_TAGS['validated'] = _TAGS['opened'] | {'CI 🤖'}
+_TAGS['approved'] = _TAGS['opened'] | {'r+ 👌'}
+_TAGS['ready'] = _TAGS['validated'] | _TAGS['approved']
+_TAGS['staged'] = _TAGS['ready'] | {'merging 👷'}
+_TAGS['merged'] = _TAGS['ready'] | {'merged 🎉'}
+_TAGS['error'] = _TAGS['opened'] | {'error 🙅'}
+_TAGS['closed'] = _TAGS['opened'] | {'closed 💔'}
+
+class Tagging(models.Model):
+    """
+    Queue of tag changes to make on PRs.
+
+    Several PR state changes are driven by webhooks, webhooks should return
+    quickly, performing calls to the Github API would *probably* get in the
+    way of that. Instead, queue tagging changes into this table whose
+    execution can be cron-driven.
+    """
+    _name = 'runbot_merge.pull_requests.tagging'
+
+    repository = fields.Many2one('pull_request.repository', required=True)
+    # store the PR number (not id) as we need a Tagging for PR objects
+    # being deleted (retargeted to non-managed branches)
+    pull_request = fields.Integer()
+
+    state_from = fields.Selection([
+        ('opened', 'Opened'),
+        ('closed', 'Closed'),
+        ('validated', 'Validated'),
+        ('approved', 'Approved'),
+        ('ready', 'Ready'),
+        ('staged', 'Staged'),
+        ('merged', 'Merged'),
+        ('error', 'Error'),
+    ])
+    state_to = fields.Selection([
+        ('opened', 'Opened'),
+        ('closed', 'Closed'),
+        ('validated', 'Validated'),
+        ('approved', 'Approved'),
+        ('ready', 'Ready'),
+        ('staged', 'Staged'),
+        ('merged', 'Merged'),
+        ('error', 'Error'),
+    ])
+
 class Commit(models.Model):
     """Represents a commit onto which statuses might be posted,
     independent of everything else as commits can be created by

runbot_merge/tests/fake_github/__init__.py

@@ -1,11 +1,11 @@
 import collections
 import io
-import itertools
 import json
 import logging
 import re
 
 import responses
+import werkzeug.urls
 import werkzeug.test
 import werkzeug.wrappers
 
@@ -333,6 +333,31 @@ class Repo(object):
 
         return (200, {})
 
+    def _add_labels(self, r, number):
+        try:
+            pr = self.issues[int(number)]
+        except KeyError:
+            return (404, None)
+
+        pr.labels.update(json.loads(r.body))
+
+        return (200, {})
+
+    def _remove_label(self, r, number, label):
+        print('remove_label', number, label)
+        try:
+            pr = self.issues[int(number)]
+        except KeyError:
+            return (404, None)
+
+        print(pr, pr.labels)
+        try:
+            pr.labels.remove(werkzeug.urls.url_unquote(label))
+        except KeyError:
+            return (404, None)
+        else:
+            return (200, {})
+
     def _do_merge(self, r):
         body = json.loads(r.body) # {base, head, commit_message}
         if not body.get('commit_message'):
@@ -394,6 +419,9 @@ class Repo(object):
         ('POST', r'merges', _do_merge),
 
         ('PATCH', r'pulls/(?P<number>\d+)', _edit_pr),
+
+        ('POST', r'issues/(?P<number>\d+)/labels', _add_labels),
+        ('DELETE', r'issues/(?P<number>\d+)/labels/(?P<label>.+)', _remove_label),
     ]
 
 class Issue(object):
@@ -403,6 +431,7 @@ class Issue(object):
         self._body = body
         self.number = max(repo.issues or [0]) + 1
         self.comments = []
+        self.labels = set()
         repo.issues[self.number] = self
 
     def post_comment(self, body, user):

runbot_merge/tests/test_basic.py

@@ -52,18 +52,24 @@ def test_trivial_flow(env, repo):
         ('number', '=', pr1.number),
     ])
     assert pr.state == 'opened'
+    env['runbot_merge.project']._check_progress()
+    assert pr1.labels == {'seen 🙂'}
     # nothing happened
 
     repo.post_status(c1, 'success', 'legal/cla')
     repo.post_status(c1, 'success', 'ci/runbot')
     assert pr.state == 'validated'
+    env['runbot_merge.project']._check_progress()
+    assert pr1.labels == {'seen 🙂', 'CI 🤖'}
 
     pr1.post_comment('hansen r+', 'reviewer')
     assert pr.state == 'ready'
 
+    # can't check labels here as running the cron will stage it
+
     env['runbot_merge.project']._check_progress()
-    print(pr.read()[0])
     assert pr.staging_id
+    assert pr1.labels == {'seen 🙂', 'CI 🤖', 'r+ 👌', 'merging 👷'}
 
     # get head of staging branch
     staging_head = repo.commit('heads/staging.master')
@@ -72,6 +78,7 @@ def test_trivial_flow(env, repo):
 
     env['runbot_merge.project']._check_progress()
     assert pr.state == 'merged'
+    assert pr1.labels == {'seen 🙂', 'CI 🤖', 'r+ 👌', 'merged 🎉'}
 
     master = repo.commit('heads/master')
     assert master.parents == [m, pr1.head],\
@@ -109,24 +116,25 @@ def test_staging_conflict(env, repo):
     repo.post_status(c3, 'success', 'ci/runbot')
     pr2.post_comment('hansen r+', "reviewer")
     env['runbot_merge.project']._check_progress()
-    pr2 = env['runbot_merge.pull_requests'].search([
+    p_2 = env['runbot_merge.pull_requests'].search([
         ('repository.name', '=', 'odoo/odoo'),
         ('number', '=', 2)
     ])
-    assert pr2.state == 'ready', "PR2 should not have been staged since there is a pending staging for master"
+    assert p_2.state == 'ready', "PR2 should not have been staged since there is a pending staging for master"
+    assert pr2.labels == {'seen 🙂', 'CI 🤖', 'r+ 👌'}
 
     staging_head = repo.commit('heads/staging.master')
     repo.post_status(staging_head.id, 'success', 'ci/runbot')
     repo.post_status(staging_head.id, 'success', 'legal/cla')
     env['runbot_merge.project']._check_progress()
     assert pr1.state == 'merged'
-    assert pr2.staging_id
+    assert p_2.staging_id
 
     staging_head = repo.commit('heads/staging.master')
     repo.post_status(staging_head.id, 'success', 'ci/runbot')
     repo.post_status(staging_head.id, 'success', 'legal/cla')
     env['runbot_merge.project']._check_progress()
-    assert pr2.state == 'merged'
+    assert p_2.state == 'merged'
 
 def test_staging_concurrent(env, repo):
     """ test staging to different targets, should be picked up together """
@@ -184,6 +192,7 @@ def test_staging_merge_fail(env, repo):
         ('number', '=', prx.number)
     ])
     assert pr1.state == 'error'
+    assert prx.labels == {'seen 🙂', 'error 🙅'}
     assert prx.comments == [
         ('reviewer', 'hansen r+'),
         ('<insert current user here>', 'Unable to stage PR (merge conflict)')
@@ -312,6 +321,8 @@ def test_edit(env, repo):
     # FIXME: should a PR retargeted to an unmanaged branch really be deleted?
     prx.base = '2.0'
     assert not pr.exists()
+    env['runbot_merge.project']._check_progress()
+    assert prx.labels == set()
 
     prx.base = '1.0'
     assert env['runbot_merge.pull_requests'].search([