From 6cb377b110b63f6ac9918381ebf81259d7c9b971 Mon Sep 17 00:00:00 2001
From: Ned Batchelder <ned@edx.org>
Date: Wed, 1 Nov 2023 17:00:31 -0400
Subject: [PATCH] fix: don't add a no-contributions comment to pull requests
 being closed. #273

---
 .../20231101_165935_nedbat_issue_273.rst      |  4 ++
 openedx_webhooks/tasks/pr_tracking.py         |  6 ++-
 tests/test_pull_request_closed.py             | 40 +++++++++++++++++++
 tests/test_pull_request_opened.py             |  2 +-
 4 files changed, 50 insertions(+), 2 deletions(-)
 create mode 100644 changelog.d/20231101_165935_nedbat_issue_273.rst

diff --git a/changelog.d/20231101_165935_nedbat_issue_273.rst b/changelog.d/20231101_165935_nedbat_issue_273.rst
new file mode 100644
index 00000000..84392ab9
--- /dev/null
+++ b/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.
diff --git a/openedx_webhooks/tasks/pr_tracking.py b/openedx_webhooks/tasks/pr_tracking.py
index ec0c6bd3..d7ed237c 100644
--- a/openedx_webhooks/tasks/pr_tracking.py
+++ b/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)
         })
diff --git a/tests/test_pull_request_closed.py b/tests/test_pull_request_closed.py
index d736f4de..490aaba0 100644
--- a/tests/test_pull_request_closed.py
+++ b/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
diff --git a/tests/test_pull_request_opened.py b/tests/test_pull_request_opened.py
index 6efc2358..aba5eed2 100644
--- a/tests/test_pull_request_opened.py
+++ b/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()