[FIX] runbot_merge: tracking message author on PullRequest events

d4fa1fd353 added tracking to changes
from *comments* (as well as a few hacks around authorship transfer),
however it missed two things:

First, it set the `change-author` during comments handling only, so
changes from the `PullRequest` hook e.g. open, synchronise, close,
edit, don't get attributed to their actual source, and instead just
fall back to uid(1). This is easy enough to fix as the `sender` is
always provided, that can be resolved to a partner which is then set
as the author of whatever changes happen.

Second, I actually missed one of the message hooks: there's both
`_message_log` and `_message_log_batch` and they don't call one
another, so both have to be overridden in order for tracking to be
consistent. In this case, specifically, the *creation* of a tracked
object goes through `_message_log_batch` (since that's a very generic
message and so works on every tracked object created during the
transaction... even though batch has a message per record anyway...)
while *updates* go through `_message_log`.

Fixes #895

conftest.py

@@ -160,6 +160,14 @@ def rolemap(request, config):
 
 @pytest.fixture
 def partners(env, config, rolemap):
+    """This specifically does not create partners for ``user`` and ``other``
+    so they can be generated on-interaction, as "external" users.
+
+    The two differ in that ``user`` has ownership of the org and can manage
+    repos there, ``other`` is completely unrelated to anything so useful to
+    check for interaction where the author only has read access to the reference
+    repositories.
+    """
     m = {}
     for role, u in rolemap.items():
         if role in ('user', 'other'):

runbot_merge/controllers/__init__.py

@@ -254,6 +254,11 @@ def handle_pr(env, event):
         headers.get('X-Github-Delivery'),
         headers.get('User-Agent'),
      )
+    sender = env['res.partner'].search([('github_login', '=', event['sender']['login'])], limit=1)
+    if not sender:
+        sender = env['res.partner'].create({'name': event['sender']['login'], 'github_login': event['sender']['login']})
+    env.cr.precommit.data['change-author'] = sender.id
+
     if event['action'] == 'opened':
         author_name = pr['user']['login']
         author = env['res.partner'].search([('github_login', '=', author_name)], limit=1)

runbot_merge/models/pull_requests.py

@@ -1049,6 +1049,13 @@ class PullRequests(models.Model):
             kw['body'] = html_escape(message)
         return super()._message_log(**kw)
 
+    def _message_log_batch(self, **kw):
+        if author := self.env.cr.precommit.data.get('change-author'):
+            kw['author_id'] = author
+        if message := self.env.cr.precommit.data.get('change-message'):
+            kw['bodies'] = {p.id: html_escape(message) for p in self}
+        return super()._message_log_batch(**kw)
+
     def _pr_acl(self, user):
         if not self:
             return ACL(False, False, False)

runbot_merge/tests/test_basic.py

@@ -65,6 +65,13 @@ def test_trivial_flow(env, repo, page, users, config):
         )
         pr = repo.make_pr(title="gibberish", body="blahblah", target='master', head='other')
 
+        [c2] = repo.make_commits(
+            'other',
+            Commit('forgot a bit', tree={'whee': 'kjfdsh'}),
+            ref='heads/other',
+            make=False,
+        )
+
     pr_id = to_pr(env, pr)
     assert pr_id.state == 'opened'
     env.run_crons()
@@ -79,12 +86,12 @@ def test_trivial_flow(env, repo, page, users, config):
         [e.text_content() for e in pr_dashboard.cssselect('dl.runbot-merge-fields dd')],
     )) == {
         'label': f"{config['github']['owner']}:other",
-        'head': c1,
+        'head': c2,
     }
 
     with repo:
-        repo.post_status(c1, 'success', 'legal/cla')
-        repo.post_status(c1, 'success', 'ci/runbot')
+        repo.post_status(c2, 'success', 'legal/cla')
+        repo.post_status(c2, 'success', 'ci/runbot')
     env.run_crons()
     assert pr_id.state == 'validated'
 
@@ -140,6 +147,7 @@ def test_trivial_flow(env, repo, page, users, config):
     assert repo.read_tree(master) == {
         'a': 'some other content',
         'b': 'a second file',
+        'whee': 'kjfdsh',
     }
     assert master.message == "gibberish\n\nblahblah\n\ncloses {repo.name}#1"\
                              "\n\nSigned-off-by: {reviewer.formatted_email}"\
@@ -156,8 +164,9 @@ def test_trivial_flow(env, repo, page, users, config):
     ])
 
     assert list(messages) == [
-        ('OdooBot', '<p>Pull Request created</p>', []),
-        ('OdooBot', f'<p>statuses changed on {c1}</p>', [('Opened', 'Validated')]),
+        (users['user'], '<p>Pull Request created</p>', []),
+        (users['user'], '', [(c1, c2)]),
+        ('OdooBot', f'<p>statuses changed on {c2}</p>', [('Opened', 'Validated')]),
         # reviewer approved changing the state and setting reviewer as reviewer
         # plus set merge method
         ('Reviewer', '', [