From 8e707c662dd998cf58dc91ba17abbfc0ddfb78a7 Mon Sep 17 00:00:00 2001 From: Sylvain LE GAL Date: Fri, 1 Jul 2022 14:53:30 +0200 Subject: [PATCH 1/2] [IMP] make _set_lines_issue working on empty issue --- src/oca_github_bot/tasks/migration_issue_bot.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/oca_github_bot/tasks/migration_issue_bot.py b/src/oca_github_bot/tasks/migration_issue_bot.py index 7d75334a..e7420491 100644 --- a/src/oca_github_bot/tasks/migration_issue_bot.py +++ b/src/oca_github_bot/tasks/migration_issue_bot.py @@ -58,6 +58,10 @@ def _set_lines_issue(gh_pr, issue, module): lines.append(new_line) added = True lines.append(line) + + # make the addition working on an empty migration issue + if not added: + lines.append(new_line) return lines From 4ba3d9138ce0a64e4a90766e2e639683a7aa376c Mon Sep 17 00:00:00 2001 From: Sylvain LE GAL Date: Sat, 2 Jul 2022 19:57:11 +0200 Subject: [PATCH 2/2] [REF] refactor to improve _set_lines_issue coverage --- newsfragments/190.bugfix | 1 + .../tasks/migration_issue_bot.py | 14 ++++--- tests/test_migration_issue_bot.py | 37 ++++++++++++++++--- 3 files changed, 40 insertions(+), 12 deletions(-) create mode 100644 newsfragments/190.bugfix diff --git a/newsfragments/190.bugfix b/newsfragments/190.bugfix new file mode 100644 index 00000000..59e823b0 --- /dev/null +++ b/newsfragments/190.bugfix @@ -0,0 +1 @@ +Make the command ``/ocabot migration`` working when migration issue doesn't contain migration module lines. diff --git a/src/oca_github_bot/tasks/migration_issue_bot.py b/src/oca_github_bot/tasks/migration_issue_bot.py index e7420491..0837fc33 100644 --- a/src/oca_github_bot/tasks/migration_issue_bot.py +++ b/src/oca_github_bot/tasks/migration_issue_bot.py @@ -29,12 +29,12 @@ def _find_issue(gh_repo, milestone, target_branch): return issue -def _set_lines_issue(gh_pr, issue, module): +def _set_lines_issue(gh_pr_user_login, gh_pr_number, issue_body, module): lines = [] added = False module_list = False - new_line = f"- [ ] {module} - By @{gh_pr.user.login} - #{gh_pr.number}" - for line in issue.body.split("\n"): + new_line = f"- [ ] {module} - By @{gh_pr_user_login} - #{gh_pr_number}" + for line in issue_body.split("\n"): if added: # Bypass the checks for faster completion lines.append(line) continue @@ -62,7 +62,7 @@ def _set_lines_issue(gh_pr, issue, module): # make the addition working on an empty migration issue if not added: lines.append(new_line) - return lines + return "\n".join(lines) @task() @@ -113,8 +113,10 @@ def migration_issue_start(org, repo, pr, username, module=None, dry_run=False): ) return # Change issue to add the PR in the module list - lines = _set_lines_issue(gh_pr, issue, module) - issue.edit(body="\n".join(lines)) + new_body = _set_lines_issue( + gh_pr.user.login, gh_pr.number, issue.body, module + ) + issue.edit(body=new_body) except Exception as e: github.gh_call( gh_pr.create_comment, diff --git a/tests/test_migration_issue_bot.py b/tests/test_migration_issue_bot.py index 67cb9d9c..94ee09fa 100644 --- a/tests/test_migration_issue_bot.py +++ b/tests/test_migration_issue_bot.py @@ -31,9 +31,34 @@ def test_find_issue(gh): @pytest.mark.vcr() def test_set_lines_issue(gh): - repo = _get_repository(gh, "OCA", "contract") - milestone = _create_or_find_branch_milestone(repo, "14.0") - issue = _find_issue(repo, milestone, "14.0") - gh_pr = gh.pull_request("OCA", "contract", 705) - lines = _set_lines_issue(gh_pr, issue, "contract") - assert len(lines) > 0 + module = "mis_builder" + gh_pr_user_login = "sbidoul" + gh_pr_number = 11 + + body_transformation = [ + ( + "Issue with list but not the module\n" + "- [ ] a_module_1 - By @legalsylvain - #1\n" + "- [ ] z_module_1 - By @pedrobaeza - #2", + f"Issue with list but not the module\n" + f"- [ ] a_module_1 - By @legalsylvain - #1\n" + f"- [ ] {module} - By @{gh_pr_user_login} - #{gh_pr_number}\n" + f"- [ ] z_module_1 - By @pedrobaeza - #2", + ), + ( + f"Issue with list containing the module\n" + f"- [x] {module} - By @legalsylvain - #1\n" + f"- [ ] z_module_1 - By @pedrobaeza - #2", + f"Issue with list containing the module\n" + f"- [x] {module} - By @{gh_pr_user_login} - #{gh_pr_number}\n" + f"- [ ] z_module_1 - By @pedrobaeza - #2", + ), + ( + "Issue with no list", + f"Issue with no list\n" + f"- [ ] {module} - By @{gh_pr_user_login} - #{gh_pr_number}", + ), + ] + for (old_body, new_body_expected) in body_transformation: + new_body = _set_lines_issue(gh_pr_user_login, gh_pr_number, old_body, module) + assert new_body == new_body_expected