fix: don't add a no-contributions comment to pull requests being closed. Fixes #273

changelog.d/20231101_165935_nedbat_issue_273.rst

@@ -0,0 +1,4 @@
+.. A new scriv changelog fragment.
+
+- Don't add a "no contributions accepted" comment if the pull request is being
+  closed. It's needlessly discouraging. Fixed #273.

openedx_webhooks/tasks/pr_tracking.py

@@ -290,7 +290,7 @@ def desired_support_state(pr: PrDict) -> PrDesiredInfo:
         if state in ["closed", "merged"]:
             desired.bot_comments.add(BotComment.SURVEY)
 
-    if desired.is_refused:
+    if desired.is_refused and state not in ["closed", "merged"]:
         desired.bot_comments.add(BotComment.NO_CONTRIBUTIONS)
 
     return desired
@@ -479,6 +479,10 @@ def _fix_bot_comment(self) -> None:
             needed_comments.remove(BotComment.END_OF_WIP)
         # BTW, we never have WELCOME_CLOSED in desired.bot_comments
 
+        if not comment_body:
+            # No body, no comment to make.
+            return
+
         comment_body += format_data_for_comment({
             "draft": is_draft_pull_request(self.pr)
         })

tests/test_pull_request_closed.py

@@ -6,6 +6,11 @@
     BotComment,
     is_comment_kind,
 )
+from openedx_webhooks.cla_check import (
+    CLA_CONTEXT,
+    CLA_STATUS_GOOD,
+    CLA_STATUS_NO_CONTRIBUTIONS,
+)
 from openedx_webhooks.tasks.github import pull_request_changed
 from .helpers import check_issue_link_in_markdown, random_text
 
@@ -98,3 +103,38 @@ def test_cc_pr_closed(fake_github, fake_jira, is_merged):
 
     pr_comments = pr.list_comments()
     assert len(pr_comments) == 2    # 1 welcome, 1 survey
+
+
+@pytest.mark.parametrize("user", ["tusbar", "feanil"])
+def test_pr_in_nocontrib_repo_closed(fake_github, user):
+    repo = fake_github.make_repo("edx", "some-public-repo")
+    pr = repo.make_pull_request(user=user)
+    pr.close(merge=False)
+    result = pull_request_changed(pr.as_json())
+    assert not result.jira_issues
+    # We don't want the bot to write a comment on a closed pull request.
+    assert len(pr.list_comments()) == 0
+    # User has a cla, but we aren't accepting contributions.
+    assert pr.status(CLA_CONTEXT) == CLA_STATUS_NO_CONTRIBUTIONS
+
+
+def test_pr_closed_after_employee_leaves(fake_github, mocker):
+    # Ned is internal.
+    pr = fake_github.make_pull_request("edx", user="nedbat")
+    result = pull_request_changed(pr.as_json())
+
+    assert not result.jira_issues
+    assert len(pr.list_comments()) == 0
+    assert pr.status(CLA_CONTEXT) == CLA_STATUS_GOOD
+
+    # Ned is fired for malfeasance.
+    mocker.patch("openedx_webhooks.tasks.pr_tracking.is_internal_pull_request", lambda pr: False)
+
+    # His PR is closed.
+    pr.close(merge=False)
+    result = pull_request_changed(pr.as_json())
+
+    assert not result.jira_issues
+    # We don't want the bot to write a comment on a closed pull request.
+    assert len(pr.list_comments()) == 0
+    assert pr.status(CLA_CONTEXT) == CLA_STATUS_NO_CONTRIBUTIONS

tests/test_pull_request_opened.py

@@ -70,7 +70,7 @@ def test_pr_in_nocontrib_repo_opened(fake_github, user):
     assert len(pr_comments) == 1
     body = pr_comments[0].body
     assert is_comment_kind(BotComment.NO_CONTRIBUTIONS, body)
-    # tusbar has a cla, but we aren't accepting contributions.
+    # User has a cla, but we aren't accepting contributions.
     assert pr.status(CLA_CONTEXT) == CLA_STATUS_NO_CONTRIBUTIONS
     assert pull_request_projects(pr.as_json()) == set()