[FIX] Ping maintainers when module is migrated

src/oca_github_bot/manifest.py

@@ -7,8 +7,6 @@
 import re
 from typing import Tuple
 
-import requests
-
 from . import config
 from .github import git_get_current_branch, github_user_can_push
 from .process import check_call, check_output
@@ -38,13 +36,13 @@ def is_addons_dir(addons_dir, installable_only=False):
     return any(addon_dirs_in(addons_dir, installable_only))
 
 
-def is_addon_dir(addon_dir, installable_only=False):
+def is_addon_dir(addon_dir, installable_only=False, cwd=None):
     """Test if a directory contains an Odoo addon."""
     if not installable_only:
-        return bool(get_manifest_path(addon_dir))
+        return bool(get_manifest_path(addon_dir, cwd=cwd))
     else:
         try:
-            return get_manifest(addon_dir).get("installable", True)
+            return get_manifest(addon_dir, cwd=cwd).get("installable", True)
         except NoManifestFound:
             return False
 
@@ -70,9 +68,11 @@ def get_manifest_file_name(addon_dir):
     return None
 
 
-def get_manifest_path(addon_dir):
+def get_manifest_path(addon_dir, cwd=None):
+    if cwd is None:
+        cwd = os.getcwd()
     for manifest_name in MANIFEST_NAMES:
-        manifest_path = os.path.join(addon_dir, manifest_name)
+        manifest_path = os.path.join(cwd, addon_dir, manifest_name)
         if os.path.exists(manifest_path):
             return manifest_path
     return None
@@ -82,8 +82,8 @@ 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)
+def get_manifest(addon_dir, cwd=None):
+    manifest_path = get_manifest_path(addon_dir, cwd=cwd)
     if not manifest_path:
         raise NoManifestFound(f"no manifest found in {addon_dir}")
     with open(manifest_path, "rb") as f:
@@ -99,13 +99,11 @@ def set_manifest_version(addon_dir, version):
         f.write(manifest)
 
 
-def is_maintainer(username, addon_dirs):
-    for addon_dir in addon_dirs:
-        try:
-            manifest = get_manifest(addon_dir)
-        except NoManifestFound:
-            return False
-        maintainers = manifest.get("maintainers", [])
+def is_maintainer(username, addon_dirs, main_branches=None, cwd=None):
+    addon_to_maintainers = get_maintainers(
+        addon_dirs, main_branches=main_branches, cwd=cwd
+    )
+    for _addon, maintainers in addon_to_maintainers.items():
         if username not in maintainers:
             return False
     return True
@@ -247,46 +245,72 @@ def user_can_push(gh, org, repo, username, addons_dir, target_branch):
     if other_changes or not modified_addon_dirs:
         return False
     # if we are modifying addons only, then the user must be maintainer of
-    # all of them on the target branch
-    current_branch = git_get_current_branch(cwd=addons_dir)
-    try:
-        check_call(["git", "checkout", target_branch], cwd=addons_dir)
-        result = is_maintainer(username, modified_addon_dirs)
-    finally:
-        check_call(["git", "checkout", current_branch], cwd=addons_dir)
+    # all of them
+    main_branches = [target_branch] + config.MAINTAINER_CHECK_ODOO_RELEASES
+    return is_maintainer(
+        username, modified_addon_dirs, main_branches=main_branches, 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)
+def get_maintainers_current_branch(addon_dirs, cwd=None):
+    """Get maintainer for each addon in `addon_dirs`.
 
-    return is_maintainer_other_branches(
-        org, repo, username, modified_addons, other_branches
-    )
+    :return: Dictionary {'addon_dir': <set of addon's maintainers>}
+    """
+    addon_to_maintainers = dict()
+    for addon_dir in addon_dirs:
+        try:
+            manifest = get_manifest(addon_dir, cwd=cwd)
+        except NoManifestFound:
+            maintainers = set()
+        else:
+            maintainers = manifest.get("maintainers", set())
+        addon_to_maintainers[addon_dir] = maintainers
+    return addon_to_maintainers
 
 
-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 = (
-                "__openerp__.py" if float(branch) < 10.0 else "__manifest__.py"
-            )
-            url = (
-                f"https://github.com/{org}/{repo}/raw/{branch}/{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 = parse_manifest(r.content)
-                if username in manifest.get("maintainers", []):
-                    is_maintainer = True
-                    break
+def get_maintainers(addon_dirs, main_branches=None, cwd=None):
+    if cwd is None:
+        cwd = os.getcwd()
+    current_branch = git_get_current_branch(cwd=cwd)
+    if main_branches is None:
+        main_branches = [current_branch]
 
-        if not is_maintainer:
-            return False
-    return True
+    branch_to_addon_to_maintainers = {}
+    for branch in main_branches:
+        try:
+            check_call(["git", "checkout", branch], cwd=cwd)
+            branch_to_addon_to_maintainers[branch] = get_maintainers_current_branch(
+                addon_dirs,
+                cwd=cwd,
+            )
+        finally:
+            check_call(["git", "checkout", current_branch], cwd=cwd)
+
+    # Transform
+    # {
+    #     "16.0": {
+    #         "date_range": {
+    #             "user_1",
+    #         },
+    #     },
+    #     "14.0": {
+    #         "date_range": {
+    #             "user_2",
+    #         },
+    #     }
+    # }
+    # to
+    # {
+    #     "date_range": {
+    #         "user_1",
+    #         "user_2",
+    #     },
+    # }
+
+    result = {}
+    for _branch, addon_to_maintainers in branch_to_addon_to_maintainers.items():
+        for addon, maintainers in addon_to_maintainers.items():
+            addon_maintainers = result.setdefault(addon, set())
+            addon_maintainers.update(maintainers)
+    return result

src/oca_github_bot/tasks/mention_maintainer.py

@@ -5,7 +5,7 @@
 from ..config import switchable
 from ..manifest import (
     addon_dirs_in,
-    get_manifest,
+    get_maintainers,
     git_modified_addon_dirs,
     is_addon_dir,
 )
@@ -24,7 +24,13 @@ def mention_maintainer(org, repo, pr, dry_run=False):
         with github.temporary_clone(org, repo, target_branch) as clonedir:
             # Get maintainers existing before the PR changes
             addon_dirs = addon_dirs_in(clonedir, installable_only=True)
-            maintainers_dict = get_maintainers(addon_dirs)
+            other_branches = config.MAINTAINER_CHECK_ODOO_RELEASES
+            main_branches = [target_branch] + other_branches
+            maintainers_dict = get_maintainers(
+                addon_dirs,
+                main_branches=main_branches,
+                cwd=clonedir,
+            )
 
             # Get list of addons modified in the PR.
             pr_branch = f"tmp-pr-{pr}"
@@ -38,9 +44,30 @@ def mention_maintainer(org, repo, pr, dry_run=False):
             # Remove not installable addons
             # (case where an addon becomes no more installable).
             modified_addon_dirs = [
-                d for d in modified_addon_dirs if is_addon_dir(d, installable_only=True)
+                d
+                for d in modified_addon_dirs
+                if is_addon_dir(d, installable_only=True, cwd=clonedir)
             ]
 
+            # Get maintainer of new addons in other branches
+            if other_branches:
+                new_addons = {
+                    addon for addon in modified_addon_dirs if addon not in addon_dirs
+                }
+                if new_addons:
+                    new_maintainers_dict = get_maintainers(
+                        new_addons,
+                        main_branches=other_branches,
+                        cwd=clonedir,
+                    )
+                    maintainers_dict.update(
+                        {
+                            new_addon: new_maintainers_dict[new_addon]
+                            for new_addon in new_addons
+                        }
+                    )
+                    modified_addon_dirs.extend(new_addons)
+
         modified_addons_maintainers = set()
         for modified_addon in modified_addon_dirs:
             addon_maintainers = maintainers_dict.get(modified_addon, list())
@@ -81,15 +108,3 @@ def get_adopt_mention(pr_opener):
     if config.ADOPT_AN_ADDON_MENTION:
         return config.ADOPT_AN_ADDON_MENTION.format(pr_opener=pr_opener)
     return None
-
-
-def get_maintainers(addon_dirs):
-    """Get maintainer for each addon in `addon_dirs`.
-
-    :return: Dictionary {'addon_dir': <list of addon's maintainers>}
-    """
-    addon_maintainers_dict = dict()
-    for addon_dir in addon_dirs:
-        maintainers = get_manifest(addon_dir).get("maintainers", [])
-        addon_maintainers_dict.setdefault(addon_dir, maintainers)
-    return addon_maintainers_dict

tests/test_manifest.py

@@ -1,6 +1,6 @@
 # Copyright (c) ACSONE SA/NV 2018
 # Distributed under the MIT License (http://opensource.org/licenses/MIT).
-
+import json
 import subprocess
 
 import pytest
@@ -20,7 +20,6 @@
     is_addon_dir,
     is_addons_dir,
     is_maintainer,
-    is_maintainer_other_branches,
     set_manifest_version,
 )
 
@@ -242,10 +241,44 @@ def test_is_maintainer(tmp_path):
     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"]
+def test_is_maintainer_other_branches(git_clone):
+    addon_name = "addon_maintainer_other_branches"
+
+    # create `addon_name` on `other_branch` with maintainer `maintainer`
+    maintainer = "maintainer"
+    other_branch = "other_branch_maintainer"
+    subprocess.check_call(["git", "checkout", "-b", other_branch], cwd=git_clone)
+    addon1_dir = git_clone / addon_name
+    addon1_dir.mkdir(exist_ok=True)
+    manifest = {
+        "name": addon_name,
+        "maintainers": [
+            maintainer,
+        ],
+    }
+    (addon1_dir / "__manifest__.py").write_text(json.dumps(manifest))
+    subprocess.check_call(["git", "add", addon_name], cwd=git_clone)
+    subprocess.check_call(
+        ["git", "commit", "-m", "[BOT] Add with maintainer"], cwd=git_clone
+    )
+
+    # create `addon_name` on current branch with no maintainers
+    branch = "current_branch_no_maintainer"
+    subprocess.check_call(["git", "checkout", "-b", branch], cwd=git_clone)
+    addon1_dir = git_clone / addon_name
+    addon1_dir.mkdir(exist_ok=True)
+    manifest = {
+        "name": addon_name,
+    }
+    (addon1_dir / "__manifest__.py").write_text(json.dumps(manifest))
+    subprocess.check_call(["git", "add", addon_name], cwd=git_clone)
+    subprocess.check_call(
+        ["git", "commit", "-m", "[BOT] Add without maintainer"], cwd=git_clone
+    )
+
+    assert is_maintainer(
+        maintainer, {addon_name}, main_branches=[other_branch], cwd=git_clone
     )
-    assert not is_maintainer_other_branches(
-        "OCA", "mis-builder", "fpdoo", {"mis_builder"}, ["12.0"]
+    assert not is_maintainer(
+        "fpdoo", {addon_name}, main_branches=[other_branch], cwd=git_clone
     )