From d7da328b9f20add5ed3cbd8140e8a8028538792a Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 30 Jan 2025 11:47:18 +0100 Subject: [PATCH] [IMP] runbot_merge: relax requirements for setting the merge method Not entirely sure about just allowing any PR author to set the merge method as it gives them a lot more control over commits (via the PR message), and I'm uncertain about the ramifications of doing that. However if the author of the PR is classified as an employee (via a provisioned user linked to the github account) then allow it. Should not be prone to abuse at least. Fixes #1036 --- runbot_merge/changelog/2025-01/merge-method-acl.md | 7 +++++++ runbot_merge/models/pull_requests.py | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 runbot_merge/changelog/2025-01/merge-method-acl.md diff --git a/runbot_merge/changelog/2025-01/merge-method-acl.md b/runbot_merge/changelog/2025-01/merge-method-acl.md new file mode 100644 index 00000000..3256039e --- /dev/null +++ b/runbot_merge/changelog/2025-01/merge-method-acl.md @@ -0,0 +1,7 @@ +IMP: allow non-reviewer employees to set the merge method on the PRs they create + +Previously this was only allowed for reviewers actual (note: not necessarily +the PR's own), but when the reviewer is busy and the PR is blocked on just the +merge method it can be frustrating. However because the merge method has commit +impliciation via the PR description (squash/merge/rebase-merge) giving access to +any author might be a bit too relaxed. diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 1012f281..b0cf7fce 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -731,6 +731,7 @@ class PullRequests(models.Model): source_author = self.source_id._pr_acl(author).is_author # nota: 15.0 `has_group` completely doesn't work if the recordset is empty super_admin = is_admin and author.user_ids and author.user_ids.has_group('runbot_merge.group_admin') + is_employee = (is_author or source_author) and author.user_ids and author.user_ids.has_group('base.group_user') help_list: list[type(commands.Command)] = list(filter(None, [ commands.Help, @@ -743,7 +744,7 @@ class PullRequests(models.Model): is_author and commands.Limit, source_author and self.source_id and commands.Close, - is_reviewer and commands.MergeMethod, + (is_reviewer or is_employee) and commands.MergeMethod, is_reviewer and commands.Delegate, is_admin and commands.Priority, @@ -854,7 +855,7 @@ For your own safety I've ignored *everything in your entire comment*. self.unstage("unreviewed (r-) by %s", login) else: msg = "r- makes no sense in the current PR state." - case commands.MergeMethod() if is_reviewer: + case commands.MergeMethod() if (is_reviewer or is_employee): self.merge_method = command.value explanation = next(label for value, label in type(self).merge_method.selection if value == command.value) self.env.ref("runbot_merge.command.method")._send(