From 108c9aea1a1a3a1055f1513ddfdb37e2fe1d7d0f Mon Sep 17 00:00:00 2001 From: Sylvain LE GAL Date: Thu, 14 Apr 2022 01:49:21 +0200 Subject: [PATCH 1/4] [IMP] Search addons maintainers in other branches. Fix : #122 --- environment.sample | 4 ++ newsfragments/183.bugfix | 1 + src/oca_github_bot/config.py | 6 ++ src/oca_github_bot/manifest.py | 55 ++++++++++++++++++- .../tasks/mention_maintainer.py | 2 +- src/oca_github_bot/tasks/merge_bot.py | 2 +- tests/test_manifest.py | 11 ++++ tests/test_mention_maintainer.py | 10 ++-- 8 files changed, 81 insertions(+), 10 deletions(-) create mode 100644 newsfragments/183.bugfix diff --git a/environment.sample b/environment.sample index bfeb8b32..ff75553a 100644 --- a/environment.sample +++ b/environment.sample @@ -80,3 +80,7 @@ OCABOT_TWINE_REPOSITORIES="[('https://pypi.org/simple','https://upload.pypi.org/ # Markdown text to be used to call for maintainers when a PR is made to an # addon that has no declared maintainers. #ADOPT_AN_ADDON_MENTION= + +# List of branches the bot will check to verify if user is the maintainer +# of module(s) +MAINTAINER_CHECK_ODOO_RELEASES=8.0,9.0,10.0,11.0,12.0,13.0,14.0,15.0 diff --git a/newsfragments/183.bugfix b/newsfragments/183.bugfix new file mode 100644 index 00000000..91a7e764 --- /dev/null +++ b/newsfragments/183.bugfix @@ -0,0 +1 @@ +Search for addons maintainers in all the branches of the current repository. diff --git a/src/oca_github_bot/config.py b/src/oca_github_bot/config.py index 0dbd8728..3fac951f 100644 --- a/src/oca_github_bot/config.py +++ b/src/oca_github_bot/config.py @@ -130,3 +130,9 @@ def func_wrapper(*args, **kwargs): ) ADOPT_AN_ADDON_MENTION = os.environ.get("ADOPT_AN_ADDON_MENTION") + +MAINTAINER_CHECK_ODOO_RELEASES = ( + os.environ.get("MAINTAINER_CHECK_ODOO_RELEASES") + and os.environ.get("MAINTAINER_CHECK_ODOO_RELEASES").split(",") + or [] +) diff --git a/src/oca_github_bot/manifest.py b/src/oca_github_bot/manifest.py index 859be80c..330d26fa 100644 --- a/src/oca_github_bot/manifest.py +++ b/src/oca_github_bot/manifest.py @@ -2,9 +2,13 @@ # Distributed under the MIT License (http://opensource.org/licenses/MIT). import ast +import logging import os import re +import requests + +from . import config from .github import git_get_current_branch, github_user_can_push from .process import check_call, check_output @@ -17,6 +21,8 @@ r"(?P
[\"']version[\"']\s*:\s*[\"'])(?P[\d\.]+)(?P[\"'])"
 )
 
+_logger = logging.getLogger(__name__)
+
 
 class NoManifestFound(Exception):
     pass
@@ -189,7 +195,11 @@ def git_modified_addons(addons_dir, ref):
 
 def git_modified_addon_dirs(addons_dir, ref):
     modified_addons, other_changes = git_modified_addons(addons_dir, ref)
-    return [os.path.join(addons_dir, addon) for addon in modified_addons], other_changes
+    return (
+        [os.path.join(addons_dir, addon) for addon in modified_addons],
+        other_changes,
+        modified_addons,
+    )
 
 
 def get_odoo_series_from_version(version):
@@ -220,11 +230,13 @@ def user_can_push(gh, org, repo, username, addons_dir, target_branch):
     return true if username is declared in the maintainers key
     on the target branch, for all addons modified in the current branch
     compared to the target_branch.
+    If the username is not declared as maintainer on the current branch,
+    check if the user is maintainer in other branches.
     """
     gh_repo = gh.repository(org, repo)
     if github_user_can_push(gh_repo, username):
         return True
-    modified_addon_dirs, other_changes = git_modified_addon_dirs(
+    modified_addon_dirs, other_changes, modified_addons = git_modified_addon_dirs(
         addons_dir, target_branch
     )
     if other_changes or not modified_addon_dirs:
@@ -234,6 +246,43 @@ def user_can_push(gh, org, repo, username, addons_dir, target_branch):
     current_branch = git_get_current_branch(cwd=addons_dir)
     try:
         check_call(["git", "checkout", target_branch], cwd=addons_dir)
-        return is_maintainer(username, modified_addon_dirs)
+        result = is_maintainer(username, modified_addon_dirs)
     finally:
         check_call(["git", "checkout", current_branch], cwd=addons_dir)
+
+    if result:
+        return True
+
+    other_branches = config.MAINTAINER_CHECK_ODOO_RELEASES
+    if target_branch in other_branches:
+        other_branches.remove(target_branch)
+
+    return is_maintainer_other_branches(
+        org, repo, username, modified_addons, other_branches
+    )
+
+
+def is_maintainer_other_branches(org, repo, username, modified_addons, other_branches):
+    for addon in modified_addons:
+        is_maintainer = False
+        for branch in other_branches:
+            manifest_file = (
+                float(branch) < 10.0 and "__openerp__.py" or "__manifest__.py"
+            )
+            url = (
+                f"https://raw.githubusercontent.com/{org}/{repo}/{branch}/"
+                f"{addon}/{manifest_file}"
+            )
+            _logger.debug("Looking for maintainers in %s" % url)
+            r = requests.get(
+                url, allow_redirects=True, headers={"Cache-Control": "no-cache"}
+            )
+            if r.ok:
+                manifest = ast.literal_eval(r.content.decode("utf-8"))
+                if username in manifest.get("maintainers", []):
+                    is_maintainer = True
+                    break
+
+        if not is_maintainer:
+            return False
+    return True
diff --git a/src/oca_github_bot/tasks/mention_maintainer.py b/src/oca_github_bot/tasks/mention_maintainer.py
index a7186ad6..4b4649e1 100644
--- a/src/oca_github_bot/tasks/mention_maintainer.py
+++ b/src/oca_github_bot/tasks/mention_maintainer.py
@@ -33,7 +33,7 @@ def mention_maintainer(org, repo, pr, dry_run=False):
                 cwd=clonedir,
             )
             check_call(["git", "checkout", pr_branch], cwd=clonedir)
-            modified_addon_dirs, _ = git_modified_addon_dirs(clonedir, target_branch)
+            modified_addon_dirs, _, _ = git_modified_addon_dirs(clonedir, target_branch)
 
             # Remove not installable addons
             # (case where an addon becomes no more installable).
diff --git a/src/oca_github_bot/tasks/merge_bot.py b/src/oca_github_bot/tasks/merge_bot.py
index 0cbb781c..4d1d2f6d 100644
--- a/src/oca_github_bot/tasks/merge_bot.py
+++ b/src/oca_github_bot/tasks/merge_bot.py
@@ -131,7 +131,7 @@ def _merge_bot_merge_pr(org, repo, merge_bot_branch, cwd, dry_run=False):
     # to the PR.
     check_call(["git", "fetch", "origin", f"refs/pull/{pr}/head:tmp-pr-{pr}"], cwd=cwd)
     check_call(["git", "checkout", f"tmp-pr-{pr}"], cwd=cwd)
-    modified_addon_dirs, _ = git_modified_addon_dirs(cwd, target_branch)
+    modified_addon_dirs, _, _ = git_modified_addon_dirs(cwd, target_branch)
     # Run main branch bot actions before bump version.
     # Do not run the main branch bot if there are no modified addons,
     # because it is dedicated to addons repos.
diff --git a/tests/test_manifest.py b/tests/test_manifest.py
index 78996cd6..65ec2509 100644
--- a/tests/test_manifest.py
+++ b/tests/test_manifest.py
@@ -20,6 +20,7 @@
     is_addon_dir,
     is_addons_dir,
     is_maintainer,
+    is_maintainer_other_branches,
     set_manifest_version,
 )
 
@@ -140,6 +141,7 @@ def test_git_modified_addons(git_clone):
     assert git_modified_addon_dirs(git_clone, "origin/master") == (
         [str(git_clone / "addon")],
         False,
+        {"addon"},
     )
     # add a second addon, and change the first one
     addon2_dir = git_clone / "addon2"
@@ -234,3 +236,12 @@ def test_is_maintainer(tmp_path):
     assert is_maintainer("u2", [addon1, addon2])
     assert not is_maintainer("u2", [addon1, addon2, addon3])
     assert not is_maintainer("u1", [tmp_path / "not_an_addon"])
+
+
+def test_is_maintainer_other_branches():
+    assert is_maintainer_other_branches(
+        "OCA", "mis-builder", "sbidoul", {"mis_builder"}, ["12.0"]
+    )
+    assert not is_maintainer_other_branches(
+        "OCA", "mis-builder", "fpdoo", {"mis_builder"}, ["12.0"]
+    )
diff --git a/tests/test_mention_maintainer.py b/tests/test_mention_maintainer.py
index f505bbe5..ddf93661 100644
--- a/tests/test_mention_maintainer.py
+++ b/tests/test_mention_maintainer.py
@@ -20,7 +20,7 @@ def test_maintainer_mentioned(git_clone, mocker):
     modified_addons_mock = mocker.patch(
         "oca_github_bot.tasks.mention_maintainer.git_modified_addon_dirs"
     )
-    modified_addons_mock.return_value = [addon_dir], False
+    modified_addons_mock.return_value = [addon_dir], False, {addon_name}
     mocker.patch("oca_github_bot.tasks.mention_maintainer.check_call")
     mention_maintainer("org", "repo", "pr")
 
@@ -53,7 +53,7 @@ def pr_edited_addon(_args, **_kwargs):
     modified_addons_mock = mocker.patch(
         "oca_github_bot.tasks.mention_maintainer.git_modified_addon_dirs"
     )
-    modified_addons_mock.return_value = [pre_pr_addon], False
+    modified_addons_mock.return_value = [pre_pr_addon], False, {addon_name}
 
     mocker.patch("oca_github_bot.tasks.mention_maintainer.check_call")
 
@@ -79,7 +79,7 @@ def test_multi_maintainer_one_mention(git_clone, mocker):
     modified_addons_mock = mocker.patch(
         "oca_github_bot.tasks.mention_maintainer.git_modified_addon_dirs"
     )
-    modified_addons_mock.return_value = addon_dirs, False
+    modified_addons_mock.return_value = addon_dirs, False, set(addon_names)
     mocker.patch("oca_github_bot.tasks.mention_maintainer.check_call")
     mention_maintainer("org", "repo", "pr")
 
@@ -105,7 +105,7 @@ def test_pr_by_maintainer_no_mention(git_clone, mocker):
     modified_addons_mock = mocker.patch(
         "oca_github_bot.tasks.mention_maintainer.git_modified_addon_dirs"
     )
-    modified_addons_mock.return_value = addon_dirs, False
+    modified_addons_mock.return_value = addon_dirs, False, set(addon_names)
     mocker.patch("oca_github_bot.tasks.mention_maintainer.check_call")
     mention_maintainer("org", "repo", "pr")
 
@@ -123,7 +123,7 @@ def test_no_maintainer_adopt_module(git_clone, mocker):
     modified_addons_mock = mocker.patch(
         "oca_github_bot.tasks.mention_maintainer.git_modified_addon_dirs"
     )
-    modified_addons_mock.return_value = [addon_dir], False
+    modified_addons_mock.return_value = [addon_dir], False, {addon_name}
     mocker.patch("oca_github_bot.tasks.mention_maintainer.check_call")
 
     with set_config(ADOPT_AN_ADDON_MENTION="Hi {pr_opener}, would you like to adopt?"):

From 99b7c2a7d278056e21442edf35dc994526054d38 Mon Sep 17 00:00:00 2001
From: Sylvain LE GAL 
Date: Fri, 20 May 2022 16:43:47 +0200
Subject: [PATCH 2/4] [IMP] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Stéphane Bidoul 
---
 src/oca_github_bot/manifest.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/oca_github_bot/manifest.py b/src/oca_github_bot/manifest.py
index 330d26fa..1f929ce9 100644
--- a/src/oca_github_bot/manifest.py
+++ b/src/oca_github_bot/manifest.py
@@ -267,11 +267,10 @@ def is_maintainer_other_branches(org, repo, username, modified_addons, other_bra
         is_maintainer = False
         for branch in other_branches:
             manifest_file = (
-                float(branch) < 10.0 and "__openerp__.py" or "__manifest__.py"
+                "__openerp__.py" if float(branch) < 10.0 else "__manifest__.py"
             )
             url = (
-                f"https://raw.githubusercontent.com/{org}/{repo}/{branch}/"
-                f"{addon}/{manifest_file}"
+                f"https://github.com/{org}/{repo}/raw/{branch}/{addon}/{manifest_file}"
             )
             _logger.debug("Looking for maintainers in %s" % url)
             r = requests.get(

From 744b57d35a47f44772ff0f083dbad24f37fe9569 Mon Sep 17 00:00:00 2001
From: Sylvain LE GAL 
Date: Fri, 20 May 2022 16:58:23 +0200
Subject: [PATCH 3/4] [REF] add parse_manifest function

---
 src/oca_github_bot/manifest.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/oca_github_bot/manifest.py b/src/oca_github_bot/manifest.py
index 1f929ce9..21b99974 100644
--- a/src/oca_github_bot/manifest.py
+++ b/src/oca_github_bot/manifest.py
@@ -77,12 +77,15 @@ def get_manifest_path(addon_dir):
     return None
 
 
+def parse_manifest(manifest: bytes)-> dict:
+    return ast.literal_eval(manifest.decode("utf-8"))
+
 def get_manifest(addon_dir):
     manifest_path = get_manifest_path(addon_dir)
     if not manifest_path:
         raise NoManifestFound(f"no manifest found in {addon_dir}")
-    with open(manifest_path, "r") as f:
-        return ast.literal_eval(f.read())
+    with open(manifest_path, "rb") as f:
+        return parse_manifest(f.read())
 
 
 def set_manifest_version(addon_dir, version):
@@ -277,7 +280,7 @@ def is_maintainer_other_branches(org, repo, username, modified_addons, other_bra
                 url, allow_redirects=True, headers={"Cache-Control": "no-cache"}
             )
             if r.ok:
-                manifest = ast.literal_eval(r.content.decode("utf-8"))
+                manifest = parse_manifest(r.content)
                 if username in manifest.get("maintainers", []):
                     is_maintainer = True
                     break

From f73b541028f627bbd265a6b190bf2f69b022b9d0 Mon Sep 17 00:00:00 2001
From: "pre-commit-ci[bot]"
 <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date: Fri, 20 May 2022 15:29:35 +0000
Subject: [PATCH 4/4] [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci
---
 src/oca_github_bot/manifest.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/oca_github_bot/manifest.py b/src/oca_github_bot/manifest.py
index 21b99974..e1ffdad2 100644
--- a/src/oca_github_bot/manifest.py
+++ b/src/oca_github_bot/manifest.py
@@ -77,9 +77,10 @@ def get_manifest_path(addon_dir):
     return None
 
 
-def parse_manifest(manifest: bytes)-> dict:
+def parse_manifest(manifest: bytes) -> dict:
     return ast.literal_eval(manifest.decode("utf-8"))
 
+
 def get_manifest(addon_dir):
     manifest_path = get_manifest_path(addon_dir)
     if not manifest_path: