diff --git a/runbot_merge/controllers/__init__.py b/runbot_merge/controllers/__init__.py index 10d815ef..dce688ed 100644 --- a/runbot_merge/controllers/__init__.py +++ b/runbot_merge/controllers/__init__.py @@ -431,7 +431,9 @@ def _handle_comment(env, repo, issue, comment, target=None): if not repository.project_id._find_commands(comment['body'] or ''): return "No commands, ignoring" - pr = env['runbot_merge.pull_requests']._get_or_schedule(repo, issue, target=target) + pr = env['runbot_merge.pull_requests']._get_or_schedule( + repo, issue, target=target, commenter=comment['user']['login'], + ) if not pr: return "Unknown PR, scheduling fetch" diff --git a/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv b/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv index eab2a241..b140ec43 100644 --- a/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv +++ b/runbot_merge/data/runbot_merge.pull_requests.feedback.template.csv @@ -16,7 +16,7 @@ runbot_merge.pr.load.unmanaged,"Branch `{pr[base][ref]}` is not within my remit, pr: pull request (github object) Repository: repository object (???)" -runbot_merge.pr.load.fetched,"{pr.ping}I didn't know about this PR and had to retrieve its information, you may have to re-approve it as I didn't see previous commands.","Notifies that we did retrieve an unknown PR (either by request or as side effect of an interaction). +runbot_merge.pr.load.fetched,"{ping}I didn't know about this PR and had to retrieve its information, you may have to re-approve it as I didn't see previous commands.","Notifies that we did retrieve an unknown PR (either by request or as side effect of an interaction). Pr: pr object we just created" runbot_merge.pr.branch.disabled,"{pr.ping}the target branch {pr.target.name!r} has been disabled, you may want to close this PR.","Notifies that the target branch for this PR was deactivated. diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index c0bf5f4e..d301523f 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -107,7 +107,14 @@ All substitutions are tentatively applied sequentially to the input. self._cr, 'runbot_merge_unique_repo', self._table, ['name']) return res - def _load_pr(self, number, *, closing=False, squash=False): + def _load_pr( + self, + number: int, + *, + closing: bool = False, + squash: bool = False, + ping: str | None = None, + ): gh = self.github() # fetch PR object and handle as *opened* @@ -238,7 +245,10 @@ All substitutions are tentatively applied sequentially to the input. self.env.ref('runbot_merge.pr.load.fetched')._send( repository=self, pull_request=number, - format_args={'pr': pr_id}, + format_args={ + 'pr': pr_id, + 'ping': ping or pr_id.ping, + }, ) def having_branch(self, branch): @@ -635,7 +645,15 @@ class PullRequests(models.Model): return json.loads(self.overrides) return {} - def _get_or_schedule(self, repo_name, number, *, target=None, closing=False) -> PullRequests | None: + def _get_or_schedule( + self, + repo_name: str, + number: int, + *, + target: str | None = None, + closing: bool = False, + commenter: str | None = None, + ) -> PullRequests | None: repo = self.env['runbot_merge.repository'].search([('name', '=', repo_name)]) if not repo: source = self.env['runbot_merge.events_sources'].search([('repository', '=', repo_name)]) @@ -682,6 +700,7 @@ class PullRequests(models.Model): 'repository': repo.id, 'number': number, 'closing': closing, + 'commenter': commenter, }) def _iter_ancestors(self) -> Iterator[PullRequests]: @@ -2518,6 +2537,7 @@ class FetchJob(models.Model): number = fields.Integer(required=True, group_operator=None) closing = fields.Boolean(default=False) commits_at = fields.Datetime(index="btree_not_null") + commenter = fields.Char() @api.model_create_multi def create(self, vals_list): @@ -2545,7 +2565,12 @@ class FetchJob(models.Model): f.active = False self.env.cr.execute("SAVEPOINT runbot_merge_before_fetch") try: - f.repository._load_pr(f.number, closing=f.closing, squash=bool(f.commits_at)) + f.repository._load_pr( + f.number, + closing=f.closing, + squash=bool(f.commits_at), + ping=f.commenter and f'@{f.commenter} ', + ) except Exception: self.env.cr.execute("ROLLBACK TO SAVEPOINT runbot_merge_before_fetch") _logger.exception("Failed to load pr %s, skipping it", f.number) diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index 9fee6f6f..fa1e72f0 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -3414,7 +3414,7 @@ class TestUnknownPR: (users['reviewer'], 'hansen r+'), (users['reviewer'], 'hansen r+'), seen(env, prx, users), - (users['user'], f"@{users['user']} I didn't know about this PR and had to " + (users['user'], f"@{users['reviewer']} I didn't know about this PR and had to " "retrieve its information, you may have to " "re-approve it as I didn't see previous commands."), ] @@ -3470,7 +3470,7 @@ class TestUnknownPR: # reviewer is set because fetch replays all the comments (thus # setting r+ and reviewer) but then syncs the head commit thus # unsetting r+ but leaving the reviewer - (users['user'], f"@{users['user']} I didn't know about this PR and had to retrieve " + (users['user'], f"@{users['reviewer']} I didn't know about this PR and had to retrieve " "its information, you may have to re-approve it " "as I didn't see previous commands."), ]