Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] Search addons maintainers in other branches. Fix : #122 #183

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions environment.sample
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions newsfragments/183.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Search for addons maintainers in all the branches of the current repository.
6 changes: 6 additions & 0 deletions src/oca_github_bot/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
)
62 changes: 57 additions & 5 deletions src/oca_github_bot/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -17,6 +21,8 @@
r"(?P<pre>[\"']version[\"']\s*:\s*[\"'])(?P<version>[\d\.]+)(?P<post>[\"'])"
)

_logger = logging.getLogger(__name__)


class NoManifestFound(Exception):
pass
Expand Down Expand Up @@ -71,12 +77,16 @@ 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):
Expand Down Expand Up @@ -189,7 +199,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):
Expand Down Expand Up @@ -220,11 +234,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:
Expand All @@ -234,6 +250,42 @@ 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 = (
"__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

if not is_maintainer:
return False
return True
2 changes: 1 addition & 1 deletion src/oca_github_bot/tasks/mention_maintainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
2 changes: 1 addition & 1 deletion src/oca_github_bot/tasks/merge_bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions tests/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
is_addon_dir,
is_addons_dir,
is_maintainer,
is_maintainer_other_branches,
set_manifest_version,
)

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"]
)
10 changes: 5 additions & 5 deletions tests/test_mention_maintainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand All @@ -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")

Expand All @@ -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")

Expand All @@ -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?"):
Expand Down