From f83caab1e3ba6be027065df5365a6e3b78b1dbf0 Mon Sep 17 00:00:00 2001
From: Sylvain LE GAL <sylvain.legal@grap.coop>
Date: Sat, 2 Jul 2022 19:57:11 +0200
Subject: [PATCH] [REF] refactor to improve _set_lines_issue coverage

---
 .../tasks/migration_issue_bot.py              | 14 ++++---
 tests/test_migration_issue_bot.py             | 37 ++++++++++++++++---
 2 files changed, 39 insertions(+), 12 deletions(-)

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