From a44207e95f9cf1b972df5625634513b2e0c14dde Mon Sep 17 00:00:00 2001 From: Simone Rubino Date: Tue, 11 Oct 2022 11:09:11 +0200 Subject: [PATCH] [ADD] migration_pr_check: Remind migration guidelines --- environment.sample | 3 + src/oca_github_bot/config.py | 17 ++ src/oca_github_bot/tasks/__init__.py | 1 + .../tasks/migration_issue_bot.py | 11 +- .../tasks/migration_pr_check.py | 78 +++++++++ src/oca_github_bot/webhooks/__init__.py | 1 + .../webhooks/on_pr_open_migration_check.py | 21 +++ tests/test_migration_pr_check.py | 158 ++++++++++++++++++ 8 files changed, 288 insertions(+), 2 deletions(-) create mode 100644 src/oca_github_bot/tasks/migration_pr_check.py create mode 100644 src/oca_github_bot/webhooks/on_pr_open_migration_check.py create mode 100644 tests/test_migration_pr_check.py diff --git a/environment.sample b/environment.sample index ff75553a..3c9e32f4 100644 --- a/environment.sample +++ b/environment.sample @@ -84,3 +84,6 @@ OCABOT_TWINE_REPOSITORIES="[('https://pypi.org/simple','https://upload.pypi.org/ # 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 + +# Reminder of migration guidelines. +#MIGRATION_GUIDELINES_REMINDER= diff --git a/src/oca_github_bot/config.py b/src/oca_github_bot/config.py index e5a23c3c..c6605646 100644 --- a/src/oca_github_bot/config.py +++ b/src/oca_github_bot/config.py @@ -150,3 +150,20 @@ def func_wrapper(*args, **kwargs): # branches. For previous versions, the setuptools_odoo task is run and generates # setup.py instead of pyproject.toml. GEN_PYPROJECT_MIN_VERSION = os.environ.get("GEN_PYPROJECT_MIN_VERSION", "17.0") + +MIGRATION_GUIDELINES_REMINDER = os.environ.get( + "MIGRATION_GUIDELINES_REMINDER", + """Thanks for the contribution! +This appears to be a migration, \ +so here you have a gentle reminder about the migration guidelines. + +Please preserve commit history following technical method \ +explained in https://github.com/OCA/maintainer-tools/wiki/#migration. +If the jump is between several versions, \ +you have to modify the source branch in the main command \ +to accommodate it to this circumstance. + +You can also take a look on the project \ +https://github.com/OCA/odoo-module-migrator/ to make easier migration. +""", +) diff --git a/src/oca_github_bot/tasks/__init__.py b/src/oca_github_bot/tasks/__init__.py index b9d6be7c..971f113c 100644 --- a/src/oca_github_bot/tasks/__init__.py +++ b/src/oca_github_bot/tasks/__init__.py @@ -6,6 +6,7 @@ main_branch_bot, mention_maintainer, migration_issue_bot, + migration_pr_check, tag_approved, tag_needs_review, tag_ready_to_merge, diff --git a/src/oca_github_bot/tasks/migration_issue_bot.py b/src/oca_github_bot/tasks/migration_issue_bot.py index 0975d9c0..8ad67df3 100644 --- a/src/oca_github_bot/tasks/migration_issue_bot.py +++ b/src/oca_github_bot/tasks/migration_issue_bot.py @@ -12,11 +12,18 @@ from ..utils import hide_secrets -def _create_or_find_branch_milestone(gh_repo, branch): +def _find_branch_milestone(gh_repo, branch): for milestone in gh_repo.milestones(): if milestone.title == branch: return milestone - return gh_repo.create_milestone(branch) + return None + + +def _create_or_find_branch_milestone(gh_repo, branch): + branch_milestone = _find_branch_milestone(gh_repo, branch) + if not branch_milestone: + branch_milestone = gh_repo.create_milestone(branch) + return branch_milestone def _find_issue(gh_repo, milestone, target_branch): diff --git a/src/oca_github_bot/tasks/migration_pr_check.py b/src/oca_github_bot/tasks/migration_pr_check.py new file mode 100644 index 00000000..ae612925 --- /dev/null +++ b/src/oca_github_bot/tasks/migration_pr_check.py @@ -0,0 +1,78 @@ +# Copyright 2022 Simone Rubino - TAKOBI +# Distributed under the MIT License (http://opensource.org/licenses/MIT). +import os.path +import re + +from .. import config, github +from ..config import switchable +from ..manifest import addon_dirs_in +from ..process import check_call +from ..queue import getLogger, task +from .migration_issue_bot import _find_branch_milestone, _find_issue + +_logger = getLogger(__name__) + + +def _get_added_modules(org, repo, gh_pr): + target_branch = gh_pr.base.ref + with github.temporary_clone(org, repo, target_branch) as clonedir: + # We need a list now because otherwise modules + # are yielded when the directory is already changed + existing_addons_paths = list(addon_dirs_in(clonedir, installable_only=True)) + + pr_branch = f"tmp-pr-{gh_pr.number}" + check_call( + ["git", "fetch", "origin", f"refs/pull/{gh_pr.number}/head:{pr_branch}"], + cwd=clonedir, + ) + check_call(["git", "checkout", pr_branch], cwd=clonedir) + pr_addons_paths = addon_dirs_in(clonedir, installable_only=True) + + new_addons_paths = { + addon_path + for addon_path in pr_addons_paths + if addon_path not in existing_addons_paths + } + return new_addons_paths + + +def is_migration_pr(org, repo, pr): + """ + Determine if the PR is a migration. + """ + with github.login() as gh: + gh_repo = gh.repository(org, repo) + gh_pr = gh_repo.pull_request(pr) + target_branch = gh_pr.base.ref + milestone = _find_branch_milestone(gh_repo, target_branch) + gh_migration_issue = _find_issue(gh_repo, milestone, target_branch) + + # The PR is mentioned in the migration issue + pr_regex = re.compile(rf"#({gh_pr.number})") + found_pr = pr_regex.findall(gh_migration_issue.body) + if found_pr: + return True + + # The added module is mentioned in the migration issue + new_addons_paths = _get_added_modules(org, repo, gh_pr) + new_addons = map(os.path.basename, new_addons_paths) + for addon in new_addons: + module_regex = re.compile(rf"- \[[ x]] {addon}") + found_module = module_regex.search(gh_migration_issue.body) + if found_module: + return True + + # The Title of the PR contains [MIG] + pr_title = gh_pr.title + if "[MIG]" in pr_title: + return True + return False + + +@task() +@switchable("comment_migration_guidelines") +def comment_migration_guidelines(org, repo, pr, dry_run=False): + migration_reminder = config.MIGRATION_GUIDELINES_REMINDER + with github.login() as gh: + gh_pr = gh.pull_request(org, repo, pr) + return github.gh_call(gh_pr.create_comment, migration_reminder) diff --git a/src/oca_github_bot/webhooks/__init__.py b/src/oca_github_bot/webhooks/__init__.py index 55e90e44..b096a4d3 100644 --- a/src/oca_github_bot/webhooks/__init__.py +++ b/src/oca_github_bot/webhooks/__init__.py @@ -7,6 +7,7 @@ on_pr_green_label_needs_review, on_pr_open_label_new_contributor, on_pr_open_mention_maintainer, + on_pr_open_migration_check, on_pr_review, on_push_to_main_branch, on_status_merge_bot, diff --git a/src/oca_github_bot/webhooks/on_pr_open_migration_check.py b/src/oca_github_bot/webhooks/on_pr_open_migration_check.py new file mode 100644 index 00000000..708e8d4e --- /dev/null +++ b/src/oca_github_bot/webhooks/on_pr_open_migration_check.py @@ -0,0 +1,21 @@ +# Copyright 2022 Simone Rubino - TAKOBI +# Distributed under the MIT License (http://opensource.org/licenses/MIT). + +import logging + +from ..router import router +from ..tasks.migration_pr_check import comment_migration_guidelines, is_migration_pr + +_logger = logging.getLogger(__name__) + + +@router.register("pull_request", action="opened") +async def on_pr_open_migration_check(event, *args, **kwargs): + """ + Whenever a PR is opened, if it is a migration PR, + remind the migration guidelines. + """ + org, repo = event.data["repository"]["full_name"].split("/") + pr = event.data["pull_request"]["number"] + if is_migration_pr(org, repo, pr): + comment_migration_guidelines.delay(org, repo, pr) diff --git a/tests/test_migration_pr_check.py b/tests/test_migration_pr_check.py new file mode 100644 index 00000000..2d92d7d3 --- /dev/null +++ b/tests/test_migration_pr_check.py @@ -0,0 +1,158 @@ +# Copyright 2022 Simone Rubino - TAKOBI +# Distributed under the MIT License (http://opensource.org/licenses/MIT). + +import pytest + +from oca_github_bot.tasks.migration_pr_check import is_migration_pr + +MIGRATION_PR_PATH = "oca_github_bot.tasks.migration_pr_check" + + +def _get_addons_gen_mock(pr_new_modules=None): + """ + Return a callable that returns a list of modules. + The list contains `pr_new_modules` only after first call. + """ + if pr_new_modules is None: + pr_new_modules = list() + + class AddonsGenMock: + def __init__(self): + self.addons_gen_calls = 0 + + def __call__(self, *args, **kwargs): + # First time, only the existing addons are returned + existing_addons = ["existing_addon"] + if self.addons_gen_calls > 0: + # After that, return also `pr_new_modules` + if pr_new_modules: + existing_addons.extend(pr_new_modules) + self.addons_gen_calls += 1 + return existing_addons + + return AddonsGenMock() + + +@pytest.mark.vcr() +def test_no_new_module(mocker): + """ + If a PR does not add a new module, then it is not a migration. + """ + mocker.patch("%s.github" % MIGRATION_PR_PATH) + mocker.patch("%s.check_call" % MIGRATION_PR_PATH) + + migration_issue = mocker.patch("%s._find_issue" % MIGRATION_PR_PATH) + migration_issue.return_value.body = "Migration Issue Body" + + addons_gen = mocker.patch("%s.addon_dirs_in" % MIGRATION_PR_PATH) + addons_gen.side_effect = _get_addons_gen_mock() + + is_migration = is_migration_pr("org", "repo", "pr") + assert not is_migration + + +@pytest.mark.vcr() +def test_new_module_no_migration(mocker): + """ + If a PR adds a new module but the module is not in the migration issue, + then it is not a migration. + """ + mocker.patch("%s.github" % MIGRATION_PR_PATH) + mocker.patch("%s.check_call" % MIGRATION_PR_PATH) + + migration_issue_body = """ +Modules to migrate: +- [ ] a_module + """ + migration_issue = mocker.patch("%s._find_issue" % MIGRATION_PR_PATH) + migration_issue.return_value.body = migration_issue_body + + addons_gen = mocker.patch("%s.addon_dirs_in" % MIGRATION_PR_PATH) + addons_gen.side_effect = _get_addons_gen_mock() + + is_migration = is_migration_pr("org", "repo", "pr") + assert not is_migration + + +@pytest.mark.vcr() +def test_new_module_migration(mocker): + """ + If a PR adds a new module and the module is in the migration issue, + then it is a migration. + """ + mocker.patch("%s.github" % MIGRATION_PR_PATH) + mocker.patch("%s.check_call" % MIGRATION_PR_PATH) + + addon_name = "migrated_module" + migration_issue_body = f""" +Modules to migrate: +- [ ] {addon_name} + """ + migration_issue = mocker.patch("%s._find_issue" % MIGRATION_PR_PATH) + migration_issue.return_value.body = migration_issue_body + + addons_gen = mocker.patch("%s.addon_dirs_in" % MIGRATION_PR_PATH) + addons_gen.side_effect = _get_addons_gen_mock([addon_name]) + + is_migration = is_migration_pr("org", "repo", "pr") + assert is_migration + + +@pytest.mark.vcr() +def test_migration_comment(mocker): + """ + If a PR adds a new module and it is in the migration issue, + then it is a migration. + """ + github_mock = mocker.patch("%s.github" % MIGRATION_PR_PATH) + mocker.patch("%s.check_call" % MIGRATION_PR_PATH) + + addon_name = "migrated_module" + migration_issue_body = f""" +Modules to migrate: +- [ ] {addon_name} + """ + migration_issue = mocker.patch("%s._find_issue" % MIGRATION_PR_PATH) + migration_issue.return_value.body = migration_issue_body + + addons_gen = mocker.patch("%s.addon_dirs_in" % MIGRATION_PR_PATH) + addons_gen.side_effect = _get_addons_gen_mock([addon_name]) + + gh_context = mocker.MagicMock() + github_mock_login_cm = github_mock.login.return_value + github_mock_login_cm.__enter__.return_value = gh_context + + is_migration = is_migration_pr("org", "repo", "pr") + assert is_migration + + +@pytest.mark.vcr() +def test_pr_title(mocker): + """ + If a PR has [MIG] in its Title, + then it is a migration. + """ + github_mock = mocker.patch("%s.github" % MIGRATION_PR_PATH) + mocker.patch("%s.check_call" % MIGRATION_PR_PATH) + + migration_issue_body = """ + Modules to migrate + """ + migration_issue = mocker.patch("%s._find_issue" % MIGRATION_PR_PATH) + migration_issue.return_value.body = migration_issue_body + + addons_gen = mocker.patch("%s.addon_dirs_in" % MIGRATION_PR_PATH) + addons_gen.side_effect = _get_addons_gen_mock(["another_module"]) + + pr_title = "[MIG] migrated_module" + gh_pr = mocker.MagicMock() + gh_pr.title = pr_title + gh_repo = mocker.MagicMock() + gh_repo.pull_request.return_value = gh_pr + gh_context = mocker.MagicMock() + gh_context.repository.return_value = gh_repo + github_mock_login_cm = github_mock.login.return_value + github_mock_login_cm.__enter__.return_value = gh_context + + is_migration = is_migration_pr("org", "repo", "pr") + assert is_migration