[ADD] runbot_merge: reviews support

Reviews are interpreted like comments and can contain any number of commands, with the difference that APPROVED and REQUEST_CHANGES are interpreted as (respectively) r+ and r- prefixes.
2025-03-15 23:45:44 +07:00 · 2018-03-27 16:39:29 +02:00 · 2018-03-27 16:39:29 +02:00 · 7033952913
commit 7033952913
parent 49c8fdbed2
4 changed files with 113 additions and 32 deletions
--- a/runbot_merge/controllers.py
+++ b/runbot_merge/controllers.py
@ -10,7 +10,11 @@ class MergebotController(Controller):
    def index(self):
        event = request.httprequest.headers['X-Github-Event']

-        return EVENTS.get(event, lambda _: "Unknown event {}".format(event))(request.jsonrequest)
+        c = EVENTS.get(event)
+        if c:
+            return c(request.jsonrequest)
+        _logger.warn('Unknown event %s', event)
+        return 'Unknown event {}'.format(event)

 def handle_pr(event):
    if event['action'] in [
@ -191,14 +195,36 @@ def handle_comment(event):

    return pr._parse_commands(partner, event['comment']['body'])

+def handle_review(event):
+    env = request.env(user=1)
+
+    partner = env['res.partner'].search([('github_login', '=', event['review']['user']['login'])])
+    if not partner:
+        _logger.info('ignoring comment from %s: not in system', event['review']['user']['login'])
+        return 'ignored'
+
+    pr = env['runbot_merge.pull_requests'].search([
+        ('number', '=', event['pull_request']['number']),
+        ('repository.name', '=', event['repository']['full_name'])
+    ])
+
+    firstline = ''
+    state = event['review']['state'].lower()
+    if state == 'approved':
+        firstline = pr.repository.project_id.github_prefix + ' r+\n'
+    elif state == 'request_changes':
+        firstline = pr.repository.project_id.github_prefix + ' r-\n'
+
+    return pr._parse_commands(partner, firstline + event['review']['body'])
+
 def handle_ping(event):
    print("Got ping! {}".format(event['zen']))
    return "pong"

 EVENTS = {
-    # TODO: add review?
    'pull_request': handle_pr,
    'status': handle_status,
    'issue_comment': handle_comment,
+    'pull_request_review': handle_review,
    'ping': handle_ping,
 }
--- a/runbot_merge/tests/fake_github/init.py
+++ b/runbot_merge/tests/fake_github/init.py
@ -327,9 +327,9 @@ class Repo(object):
            pr.base = body.get('base')

        if body.get('state') == 'open':
-            self.notify('pull_request', 'reopened', self.name, pr)
+            self.notify('pull_request', 'reopened', pr)
        elif body.get('state') == 'closed':
-            self.notify('pull_request', 'closed', self.name, pr)
+            self.notify('pull_request', 'closed', pr)

        return (200, {})

@ -434,20 +434,20 @@ class PR(Issue):
        self.label = label
        self.state = 'open'

-        repo.notify('pull_request', 'opened', repo.name, self)
+        repo.notify('pull_request', 'opened', self)

    @Issue.title.setter
    def title(self, value):
        old = self.title
        Issue.title.fset(self, value)
-        self.repo.notify('pull_request', 'edited', self.repo.name, self, {
+        self.repo.notify('pull_request', 'edited', self, {
            'title': {'from': old}
        })
    @Issue.body.setter
    def body(self, value):
        old = self.body
        Issue.body.fset(self, value)
-        self.repo.notify('pull_request', 'edited', self.repo.name, self, {
+        self.repo.notify('pull_request', 'edited', self, {
            'body': {'from': old}
        })
    @property
@ -456,22 +456,22 @@ class PR(Issue):
    @base.setter
    def base(self, value):
        old, self._base = self._base, value
-        self.repo.notify('pull_request', 'edited', self.repo.name, self, {
+        self.repo.notify('pull_request', 'edited', self, {
            'base': {'from': {'ref': old}}
        })

    def push(self, sha):
        self.head = sha
-        self.repo.notify('pull_request', 'synchronize', self.repo.name, self)
+        self.repo.notify('pull_request', 'synchronize', self)

    def open(self):
        assert self.state == 'closed'
        self.state = 'open'
-        self.repo.notify('pull_request', 'reopened', self.repo.name, self)
+        self.repo.notify('pull_request', 'reopened', self)

    def close(self):
        self.state = 'closed'
-        self.repo.notify('pull_request', 'closed', self.repo.name, self)
+        self.repo.notify('pull_request', 'closed', self)

    @property
    def commits(self):
@ -480,6 +480,10 @@ class PR(Issue):
        return len({h for h, _ in git.walk_ancestors(store, self.head, False)}
                   - {h for h, _ in git.walk_ancestors(store, target, False)})

+    def post_review(self, state, user, body):
+        self.comments.append((user, "REVIEW %s\n\n%s " % (state, body)))
+        self.repo.notify('pull_request_review', state, self, user, body)
+
 class Commit(object):
    __slots__ = ['tree', 'message', 'author', 'committer', 'parents', 'statuses']
    def __init__(self, tree, message, author, committer, parents):
@ -524,33 +528,39 @@ class Client(werkzeug.test.Client):
            data=json.dumps(data),
        )

-    def pull_request(self, action, repository, pr, changes=None):
+    def pull_request(self, action, pr, changes=None):
        assert action in ('opened', 'reopened', 'closed', 'synchronize', 'edited')
        return self.open(self._make_env(
            'pull_request', {
                'action': action,
-                'pull_request': {
-                    'number': pr.number,
-                    'head': {
-                        'sha': pr.head,
-                        'label': pr.label,
-                    },
-                    'base': {
-                        'ref': pr.base,
-                        'repo': {
-                            'name': repository.split('/')[1],
-                            'full_name': repository,
-                        },
-                    },
-                    'title': pr.title,
-                    'body': pr.body,
-                    'commits': pr.commits,
-                    'user': { 'login': pr.user },
-                },
+                'pull_request': self._pr(pr),
                **({'changes': changes} if changes else {})
            }
        ))

+    def pull_request_review(self, action, pr, user, body):
+        """
+        :type action: 'APPROVED' | 'REQUEST_CHANGES' | 'COMMENT'
+        :type pr: PR
+        :type user: str
+        :type body: str
+        """
+        assert action in ('APPROVED', 'REQUEST_CHANGES', 'COMMENT')
+        return self.open(self._make_env(
+            'pull_request_review', {
+                'review': {
+                    'state': action,
+                    'body': body,
+                    'user': {'login': user},
+                },
+                'pull_request': self._pr(pr),
+                'repository': {
+                    'name': pr.repo.name.split('/')[1],
+                    'full_name': pr.repo.name,
+                }
+            }
+        ))
+
    def status(self, repository, context, state, sha):
        assert state in ('success', 'failure', 'pending')
        return self.open(self._make_env(
@ -573,3 +583,26 @@ class Client(werkzeug.test.Client):
        if isinstance(issue, PR):
            contents['issue']['pull_request'] = { 'url': 'fake' }
        return self.open(self._make_env('issue_comment', contents))
+
+    def _pr(self, pr):
+        """
+        :type pr: PR
+        """
+        return {
+            'number': pr.number,
+            'head': {
+                'sha': pr.head,
+                'label': pr.label,
+            },
+            'base': {
+                'ref': pr.base,
+                'repo': {
+                    'name': pr.repo.name.split('/')[1],
+                    'full_name': pr.repo.name,
+                },
+            },
+            'title': pr.title,
+            'body': pr.body,
+            'commits': pr.commits,
+            'user': {'login': pr.user},
+        }
--- a/runbot_merge/tests/test_basic.py
+++ b/runbot_merge/tests/test_basic.py
@ -32,7 +32,9 @@ def repo(gh, env):
    })
    # need to create repo & branch in env so hook will allow processing them
    return gh.repo('odoo/odoo', hooks=[
-        ((odoo.http.root, '/runbot_merge/hooks'), ['pull_request', 'issue_comment', 'status'])
+        ((odoo.http.root, '/runbot_merge/hooks'), [
+            'pull_request', 'issue_comment', 'status', 'pull_request_review',
+        ])
    ])

 def test_trivial_flow(env, repo):
@ -1120,3 +1122,23 @@ class TestReviewing(object):
            ('repository.name', '=', 'odoo/odoo'),
            ('number', '=', prx.number)
        ]).state == 'ready'
+
+    def test_actual_review(self, env, repo):
+        m = repo.make_commit(None, 'initial', None, tree={'m': 'm'})
+        m2 = repo.make_commit(m, 'second', None, tree={'m': 'm', 'm2': 'm2'})
+        repo.make_ref('heads/master', m2)
+
+        c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'})
+        prx = repo.make_pr('title', 'body', target='master', ctid=c1, user='user')
+        pr = env['runbot_merge.pull_requests'].search([
+            ('repository.name', '=', 'odoo/odoo'),
+            ('number', '=', prx.number)
+        ])
+
+        prx.post_review('COMMENT', 'reviewer', "hansen priority=1")
+        assert pr.priority == 1
+        assert pr.state == 'opened'
+
+        prx.post_review('APPROVED', 'reviewer', "hansen priority=2")
+        assert pr.priority == 2
+        assert pr.state == 'approved'
--- a/runbot_merge/tests/test_multirepo.py
+++ b/runbot_merge/tests/test_multirepo.py
@ -35,7 +35,7 @@ def project(env):
 def repo_a(gh, project):
    project.write({'repo_ids': [(0, 0, {'name': "odoo/a"})]})
    return gh.repo('odoo/a', hooks=[
-        ((odoo.http.root, '/runbot_merge/hooks'), ['pull_request', 'issue_comment', 'status'])
+        ((odoo.http.root, '/runbot_merge/hooks'), ['pull_request', 'issue_comment', 'status', 'pull_request_review'])
    ])

@pytest.fixture