From 61b92b22245f9323b55a83f006feb38b1e9b78c7 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 9 Aug 2024 10:10:01 +0200 Subject: [PATCH] [IMP] runbot_merge: use native support for tracking messages odoo/odoo@1341d52 added native support for tracking / overriding tracking message so we don't need `change-message `anymore. The calls had to be modified a bit as `_track_set_log_message` has to be invoked on the records for which the tracking message is being set / updated. Sadly the same for authors was only added in 16.3 *and* it's unsuitable for our needs as we want to set the author without knowing the scope of affected records (at least in the controller). That way hopefully in 17.0 we can remove the `_message_compute_author` override and things should work out. And maybe for 19.0 we can get the ability to set a per-model (or even global) fallback author into standard. --- runbot_merge/controllers/__init__.py | 2 +- runbot_merge/models/__init__.py | 1 + runbot_merge/models/mail_thread.py | 32 ++++++++++++++++++++++++++++ runbot_merge/models/pull_requests.py | 32 ++++------------------------ 4 files changed, 38 insertions(+), 29 deletions(-) create mode 100644 runbot_merge/models/mail_thread.py diff --git a/runbot_merge/controllers/__init__.py b/runbot_merge/controllers/__init__.py index 8b18a846..b25e9a55 100644 --- a/runbot_merge/controllers/__init__.py +++ b/runbot_merge/controllers/__init__.py @@ -258,7 +258,7 @@ def handle_pr(env, event): 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 + env['runbot_merge.pull_requests']._track_set_author(sender.id, fallback=True) if event['action'] == 'opened': author_name = pr['user']['login'] diff --git a/runbot_merge/models/__init__.py b/runbot_merge/models/__init__.py index f15f98f2..ac19320c 100644 --- a/runbot_merge/models/__init__.py +++ b/runbot_merge/models/__init__.py @@ -1,3 +1,4 @@ +from . import mail_thread from . import ir_actions from . import res_partner from . import project diff --git a/runbot_merge/models/mail_thread.py b/runbot_merge/models/mail_thread.py new file mode 100644 index 00000000..d788e49f --- /dev/null +++ b/runbot_merge/models/mail_thread.py @@ -0,0 +1,32 @@ +from collections import ChainMap + +from odoo import models +from odoo.tools import ConstantMapping + + +class MailThread(models.AbstractModel): + _inherit = 'mail.thread' + + def _track_set_author(self, author, *, fallback=False): + """ Set the author of the tracking message. """ + if not self._track_get_fields(): + return + authors = self.env.cr.precommit.data.setdefault(f'mail.tracking.author.{self._name}', {}) + if fallback: + details = authors + if isinstance(authors, ChainMap): + details = authors.maps[0] + self.env.cr.precommit.data[f'mail.tracking.author.{self._name}'] = ChainMap( + details, + ConstantMapping(author), + ) + else: + for id_ in self.ids: + authors[id_] = author + + def _message_compute_author(self, author_id=None, email_from=None, raise_on_email=True): + if author_id is None and self and 'id' in self: + t = self.env.cr.precommit.data.get(f'mail.tracking.author.{self._name}', {}) + if len(authors := {t.get(r.id) for r in self}) == 1: + author_id = authors.pop() + return super()._message_compute_author(author_id, email_from, raise_on_email) diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index c2b243bb..b39b6681 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -954,7 +954,7 @@ For your own safety I've ignored *everything in your entire comment*. cmdstr = ', '.join(map(str, cmds)) if not rejections: _logger.info("%s (%s) applied %s", login, name, cmdstr) - self.env.cr.precommit.data['change-author'] = author.id + self._track_set_author(author.id, fallback=True) return 'applied ' + cmdstr self.env.cr.rollback() @@ -1121,27 +1121,6 @@ For your own safety I've ignored *everything in your entire comment*. self.env.ref('runbot_merge.check_linked_prs_status')._trigger() return None - def message_post(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['body'] = html_escape(message) - return super().message_post(**kw) - - def _message_log(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['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) @@ -1946,6 +1925,7 @@ class Commit(models.Model): self.flush(['to_check'], c) if pr: pr._validate(c.statuses) + pr._track_set_log_message(html_escape(f"statuses changed on {sha}")) if stagings: stagings._notify(c) @@ -1956,8 +1936,6 @@ class Commit(models.Model): _logger.exception("Failed to apply commit %s (%s)", c, sha) self.env.cr.rollback() else: - self.env.cr.precommit.data['change-message'] = \ - f"statuses changed on {sha}" self.env.cr.commit() _sql_constraints = [ @@ -2199,9 +2177,8 @@ class Stagings(models.Model): def fail(self, message, prs=None): _logger.info("Staging %s failed: %s", self, message) - self.env.cr.precommit.data['change-message'] =\ - f'staging {self.id} failed: {message}' prs = prs or self.batch_ids.prs + prs._track_set_log_message(f'staging {self.id} failed: {message}') prs.error = True for pr in prs: self.env.ref('runbot_merge.pr.staging.fail')._send( @@ -2317,9 +2294,8 @@ class Stagings(models.Model): 'reason': str(e.__cause__ or e.__context__ or e) }) else: - self.env.cr.precommit.data['change-message'] =\ - f'staging {self.id} succeeded' prs = self.mapped('batch_ids.prs') + prs._track_set_log_message(f'staging {self.id} succeeded') logger.info( "%s FF successful, marking %s as merged", self, prs