refactor(IDX): update comment logic

reusable_workflows/check_cla/check_cla_issue.py

@@ -20,7 +20,7 @@ def main() -> None:
 
     cla_signed = cla.check_if_cla_signed(issue, user)
     if not cla_signed:
-        cla.comment_on_issue(issue)
+        cla.leave_failed_comment_on_issue(issue)
     else:
         cla.handle_cla_signed(issue, user)
 

reusable_workflows/check_cla/check_cla_pr.py

@@ -12,33 +12,30 @@
 APPROVED_LABEL = "cla:agreed"
 GH_WORKFLOW_LABEL = "cla:gh-wf-pending"
 
-# keep all old bot names for backwards compatibility
-CLA_BOT_NAMES = ["cla-idx-bot[bot]", "sa-github-api", "dfinity-droid-prod[bot]"]
-
 
 class CLAHandler:
     def __init__(self, gh: github3.login) -> None:
         self.cla_repo = gh.repository(owner="dfinity", repository="cla")
         self.cla_link = f"{self.cla_repo.html_url}/blob/main/CLA.md"
 
-    def check_comment_already_exists(
-        self, comments: github3.structs.GitHubIterator
+    @staticmethod
+    def check_if_comment_already_exists(
+        search_comment: str, comments: github3.structs.GitHubIterator
     ) -> bool:
         for comment in comments:
-            if comment.user.login in CLA_BOT_NAMES:
+            if search_comment == comment.body:
                 return True
         return False
 
-    def comment_on_issue(self, issue: GHIssue):
+    def leave_failed_comment_on_issue(self, issue: GHIssue) -> None:
         # check if bot has already left a message to avoid spam
         issue_comments = issue.comments()
-        bot_comment = self.check_comment_already_exists(issue_comments)
-        if not bot_comment:
+        if not self.check_if_comment_already_exists(messages.FAILED_COMMENT, issue_comments):
             issue.create_comment(messages.FAILED_COMMENT)
 
-    def comment_on_pr(self, pr: GHPullRequest, pr_comment):
-        bot_comment = self.check_comment_already_exists(pr.issue_comments())
-        if not bot_comment:
+    def comment_on_pr(self, pr: GHPullRequest, pr_comment: str) -> None:
+        pr_comments = pr.comments()
+        if not self.check_if_comment_already_exists(pr_comment, pr_comments):
             pr.create_comment(pr_comment)
 
     def check_if_cla_signed(self, issue: GHIssue, user: str) -> bool:
@@ -56,7 +53,7 @@ def check_if_cla_signed(self, issue: GHIssue, user: str) -> bool:
 
     def get_cla_issue(self, user: str) -> Optional[GHIssue]:
         for issue in self.cla_repo.issues():
-            if issue.title == f"cla: @{user}" and issue.user.login in CLA_BOT_NAMES:
+            if issue.title == f"cla: @{user}":
                 return issue
         print(f"No CLA issue for {user}")
         return None  # to make linter happy

reusable_workflows/tests/test_cla_issue.py

@@ -33,7 +33,7 @@ def test_end_to_end_cla_signed(cla_mock, gh_login_mock):
     main()
 
     cla.check_if_cla_signed.assert_called_with(issue, "username")
-    cla.comment_on_issue.assert_not_called()
+    cla.leave_failed_comment_on_issue.assert_not_called()
     cla.handle_cla_signed.assert_called_once()
 
 
@@ -56,7 +56,7 @@ def test_end_to_end_cla_not_signed(cla_mock, gh_login_mock):
     main()
 
     cla.check_if_cla_signed.assert_called_with(issue, "username")
-    cla.comment_on_issue.assert_called_once()
+    cla.leave_failed_comment_on_issue.assert_called_once()
     cla.handle_cla_signed.assert_not_called()
 
 

reusable_workflows/tests/test_cla_pr.py

@@ -6,6 +6,7 @@
 from shared.messages import (
     AGREED_MESSAGE,
     CLA_AGREEMENT_MESSAGE,
+    FAILED_COMMENT,
     USER_AGREEMENT_MESSAGE,
 )
 from check_cla.check_cla_pr import CLAHandler, main
@@ -28,15 +29,14 @@ def test_bot_comment_exists():
     cla = CLAHandler(mock.Mock())
     comments_iterator = mock.Mock()
     comment1 = mock.Mock()
-    comment1.user.login = "username"
+    comment1.body = "comment1"
     comment2 = mock.Mock()
-    comment2.user.login = "sa-github-api"
+    comment2.body = "comment2"
     comments_iterator.__iter__ = mock.Mock(
-        return_value=iter([comment1, comment2, comment1])
+        return_value=iter([comment1, comment2])
     )
-    #  comments_iterator.return_value = [comment1, comment2, comment1]
 
-    bot_comment = cla.check_comment_already_exists(comments_iterator)
+    bot_comment = cla.check_if_comment_already_exists("comment1", comments_iterator)
 
     assert bot_comment is True
 
@@ -45,14 +45,25 @@ def test_no_bot_comment():
     cla = CLAHandler(mock.Mock())
     issue_comments = mock.Mock()
     comment1 = mock.Mock()
-    comment1.user.login = "username"
-    issue_comments.__iter__ = mock.Mock(return_value=iter([comment1, comment1]))
+    comment1.body = "comment"
+    issue_comments.__iter__ = mock.Mock(return_value=iter([comment1]))
 
-    bot_comment = cla.check_comment_already_exists(issue_comments)
+    bot_comment = cla.check_if_comment_already_exists("comment2", issue_comments)
 
     assert bot_comment is False
 
 
+def test_leave_failed_comment_on_issue():
+    cla = CLAHandler(mock.Mock())
+    issue = mock.Mock()
+    issue.comments.return_value = mock.Mock()
+    cla.check_if_comment_already_exists = mock.Mock(return_value=False)
+
+    cla.leave_failed_comment_on_issue(issue)
+
+    issue.create_comment.assert_called_once_with(FAILED_COMMENT)
+
+
 def test_cla_is_signed(capfd):
     cla = CLAHandler(mock.Mock())
     issue = mock.Mock()
@@ -88,7 +99,6 @@ def test_cla_is_not_signed(capfd):
     cla = CLAHandler(mock.Mock())
     issue = mock.Mock()
     comment = mock.Mock()
-    comment.user.login = "bot"
     issue.comments.return_value = [mock.Mock(), comment]
 
     response = cla.check_if_cla_signed(issue, "username")
@@ -103,7 +113,6 @@ def test_get_cla_issue_success():
     cla_repo = mock.Mock()
     issue = mock.Mock()
     issue.title = "cla: @username"
-    issue.user.login = "sa-github-api"
     cla_repo.issues.return_value = [mock.Mock(), issue]
     cla.cla_repo = cla_repo