diff --git a/changelog.d/20230824_154702_kyle_rm_survey.rst b/changelog.d/20230824_154702_kyle_rm_survey.rst new file mode 100644 index 00000000..d0faca65 --- /dev/null +++ b/changelog.d/20230824_154702_kyle_rm_survey.rst @@ -0,0 +1,4 @@ +.. A new scriv changelog fragment. + +- Removed: The bot on longer posts a survey link when a PR is closed or merged. + diff --git a/docs/details.rst b/docs/details.rst index 1bfd9caf..b64bd634 100644 --- a/docs/details.rst +++ b/docs/details.rst @@ -128,18 +128,10 @@ fields in the issue. It removes the GitHub label for the old Jira status, and adds the GitHub label for the new Jira status. -When a pull request is closed ------------------------------ - -The bot leaves a comment asking the author to complete a survey about the pull -request. - - When a pull request is re-opened -------------------------------- -The bot deletes the "please complete a survey" comment that was added when the -pull request was closed. The Jira issue is returned to the state it was in +The Jira issue is returned to the state it was in when the pull request was closed. diff --git a/openedx_webhooks/bot_comments.py b/openedx_webhooks/bot_comments.py index 627d8c38..d9e1578f 100644 --- a/openedx_webhooks/bot_comments.py +++ b/openedx_webhooks/bot_comments.py @@ -30,7 +30,6 @@ class BotComment(Enum): NEED_CLA = auto() BLENDED = auto() END_OF_WIP = auto() - SURVEY = auto() NO_CONTRIBUTIONS = auto() @@ -52,11 +51,6 @@ class BotComment(Enum): BotComment.END_OF_WIP: [ "", ], - BotComment.SURVEY: [ - "", - "/1FAIpQLSceJOyGJ6JOzfy6lyR3T7EW_71OWUnNQXp68Fymsk3MkNoSDg/viewform", - "", - ], BotComment.NO_CONTRIBUTIONS: [ "", ], @@ -141,41 +135,9 @@ def github_blended_pr_comment( ) -SURVEY_URL = ( - 'https://docs.google.com/forms/d/e' - '/1FAIpQLSceJOyGJ6JOzfy6lyR3T7EW_71OWUnNQXp68Fymsk3MkNoSDg/viewform' - '?usp=pp_url' - '&entry.1671973413={repo_full_name}' - '&entry.867055334={pull_request_url}' - '&entry.1484655318={contributor_url}' - '&entry.752974735={created_at}' - '&entry.1917517419={closed_at}' - '&entry.2133058324={is_merged}' -) - def _format_datetime(datetime_string): return arrow.get(datetime_string).format('YYYY-MM-DD+HH:mm') -def github_end_survey_comment(pull_request: PrDict) -> str: - """ - Create a "please fill out this survey" comment. - """ - is_merged = pull_request.get("merged", False) - url = SURVEY_URL.format( - repo_full_name=pull_request["base"]["repo"]["full_name"], - pull_request_url=pull_request["html_url"], - contributor_url=pull_request["user"]["html_url"], - created_at=_format_datetime(pull_request["created_at"]), - closed_at=_format_datetime(pull_request["closed_at"]), - is_merged="Yes" if is_merged else "No", - ) - return render_template( - "github_end_survey.md.j2", - user=pull_request["user"]["login"], - is_merged=is_merged, - survey_url=url, - ) - def no_contributions_thanks(pull_request: PrDict) -> str: """ diff --git a/openedx_webhooks/tasks/pr_tracking.py b/openedx_webhooks/tasks/pr_tracking.py index 377d3d4e..0cdb25cc 100644 --- a/openedx_webhooks/tasks/pr_tracking.py +++ b/openedx_webhooks/tasks/pr_tracking.py @@ -20,7 +20,6 @@ github_blended_pr_comment, github_community_pr_comment, github_community_pr_comment_closed, - github_end_survey_comment, no_contributions_thanks, ) from openedx_webhooks.cla_check import ( @@ -96,9 +95,6 @@ class PrCurrentInfo: # The text of the first bot comment. bot_comment0_text: Optional[str] = None - # The comment id of the survey comment, if any. - bot_survey_comment_id: Optional[str] = None - # The last-seen state stored in the first bot comment. last_seen_state: Dict = field(default_factory=dict) # And aggregate of all data stored on all bot comments. @@ -192,8 +188,6 @@ def current_support_state(pr: PrDict) -> PrCurrentInfo: for comment_id, snips in BOT_COMMENT_INDICATORS.items(): if any(snip in body for snip in snips): current.bot_comments.add(comment_id) - if comment_id == BotComment.SURVEY: - current.bot_survey_comment_id = comment["id"] current.all_bot_state.update(extract_data_from_comment(body)) on_our_jira, jira_id = get_jira_issue_key(prid) @@ -326,10 +320,6 @@ def desired_support_state(pr: PrDict) -> Optional[PrDesiredInfo]: desired.jira_status = "Merged" elif state == "reopened": desired.jira_status = "pre-close" # Not a real Jira status. - desired.bot_comments_to_remove.add(BotComment.SURVEY) - - if state in ["closed", "merged"]: - desired.bot_comments.add(BotComment.SURVEY) if "additions" in pr: desired.jira_extra_fields["Github Lines Added"] = pr["additions"] @@ -602,8 +592,6 @@ def _fix_bot_comment(self, comment_kwargs: Dict) -> None: if BotComment.WELCOME_CLOSED in needed_comments: comment_body += github_community_pr_comment_closed(self.pr, jira_id, **comment_kwargs) needed_comments.remove(BotComment.WELCOME_CLOSED) - if BotComment.SURVEY in self.desired.bot_comments: - self.desired.bot_comments.remove(BotComment.SURVEY) if BotComment.BLENDED in needed_comments: comment_body += github_blended_pr_comment( @@ -648,19 +636,8 @@ def _add_bot_comments(self): """ needed_comments = self.desired.bot_comments - self.current.bot_comments - BOT_COMMENTS_FIRST - if BotComment.SURVEY in needed_comments: - body = github_end_survey_comment(self.pr) - if self.desired.jira_status == "Rejected": - # For close/re-open cycling, remember what Jira was before the close. - body += format_data_for_comment({"jira-pre-close": self.desired.jira_previous_status}) - self.actions.add_comment_to_pull_request(comment_body=body) - needed_comments.remove(BotComment.SURVEY) - self.happened = True - - if BotComment.SURVEY in self.desired.bot_comments_to_remove: - if self.current.bot_survey_comment_id: - self.actions.delete_comment_on_pull_request(comment_id=self.current.bot_survey_comment_id) - self.happened = True + # Handle needed_comments here. Pop them from the set once handled. + # (Currently no comments are handled this way.) assert needed_comments == set(), f"Couldn't make comments: {needed_comments}" diff --git a/openedx_webhooks/templates/github_community_pr_comment_closed.md.j2 b/openedx_webhooks/templates/github_community_pr_comment_closed.md.j2 deleted file mode 100644 index a486fff5..00000000 --- a/openedx_webhooks/templates/github_community_pr_comment_closed.md.j2 +++ /dev/null @@ -1,24 +0,0 @@ -{% filter replace("\n", " ")|trim %} -Although this pull request is already -{% if is_merged %}merged{% else %}closed{% endif %}, -{% if issue_key %} -I've created -[{{ issue_key }}]({{ jira_server }}/browse/{{ issue_key }}) -so that we can track it in Jira. -{% else %} -I'm still watching it for updates. -{% endif %} -{% endfilter %} - -{% filter replace("\n", " ")|trim %} -There is nothing you have to do. No action is needed from your side. -{% if is_merged %} -Thanks again for your contribution. -{% else %} -Thanks. -{% endif %} -{% endfilter %} - - - - diff --git a/openedx_webhooks/templates/github_end_survey.md.j2 b/openedx_webhooks/templates/github_end_survey.md.j2 deleted file mode 100644 index b5f39e1d..00000000 --- a/openedx_webhooks/templates/github_end_survey.md.j2 +++ /dev/null @@ -1,12 +0,0 @@ -{% filter replace("\n", " ")|trim %} -{% if is_merged %} -@{{ user }} 🎉 Your pull request was merged! - -Please take a moment -{% else %} -@{{ user }} Even though your pull request wasn’t merged, please take a moment -{% endif %} -to [answer a two question survey]({{ survey_url }}) so we can improve your experience in the future. -{% endfilter %} - - diff --git a/tests/test_pr_comments.py b/tests/test_pr_comments.py index d1f4ca4b..4122c722 100644 --- a/tests/test_pr_comments.py +++ b/tests/test_pr_comments.py @@ -8,7 +8,6 @@ BotComment, is_comment_kind, github_community_pr_comment, - github_end_survey_comment, extract_data_from_comment, format_data_for_comment, ) @@ -36,27 +35,6 @@ def test_community_pr_comment_no_cla(fake_github, fake_jira): check_good_markdown(comment) -def test_survey_pr_comment(fake_github, is_merged): - with freeze_time("2021-08-31 15:30:12"): - pr = fake_github.make_pull_request(user="FakeUser") - with freeze_time("2021-09-01 01:02:03"): - pr.close(merge=is_merged) - prj = pr.as_json() - comment = github_end_survey_comment(prj) - assert "@FakeUser" in comment - assert "/1FAIpQLSceJOyGJ6JOzfy6lyR3T7EW_71OWUnNQXp68Fymsk3MkNoSDg/viewform" in comment - assert "&entry.1671973413=an-org/a-repo" in comment - assert "&entry.752974735=2021-08-31+15:30" in comment - assert "&entry.1917517419=2021-09-01+01:02" in comment - if is_merged: - assert "Your pull request was merged!" in comment - assert "&entry.2133058324=Yes" in comment - else: - assert "Even though your pull request wasn’t merged" in comment - assert "&entry.2133058324=No" in comment - check_good_markdown(comment) - - COMMENT_DATA = { "hello": 1, "what": "-- that's what --\nbye.", diff --git a/tests/test_pull_request_closed.py b/tests/test_pull_request_closed.py index dc555f0e..a4166cf1 100644 --- a/tests/test_pull_request_closed.py +++ b/tests/test_pull_request_closed.py @@ -55,10 +55,6 @@ def test_external_pr_closed(is_merged, has_jira, fake_jira, closed_pull_request) # We moved the Jira issue to Merged or Rejected. expected_status = "Merged" if is_merged else "Rejected" assert fake_jira.issues[issue_key].status == expected_status - pr_comments = pr.list_comments() - body = pr_comments[-1].body - assert "survey" in body - assert is_comment_kind(BotComment.SURVEY, body) def test_external_pr_closed_but_issue_deleted(is_merged, has_jira, fake_jira, closed_pull_request): @@ -73,7 +69,7 @@ def test_external_pr_closed_but_issue_deleted(is_merged, has_jira, fake_jira, cl assert anything_happened pr_comments = pr.list_comments() - assert len(pr_comments) == 4 # 1 welcome, closed_pull_request makes two, 1 survey + assert len(pr_comments) == 3 # 1 welcome, closed_pull_request makes two # We leave the old issue id in the comment. body = pr_comments[0].body check_issue_link_in_markdown(body, old_issue_key) @@ -124,13 +120,13 @@ def test_cc_pr_closed(fake_github, fake_jira, is_merged): pull_request_changed(pr.as_json()) pr_comments = pr.list_comments() - assert len(pr_comments) == 2 # 1 welcome, 1 survey + assert len(pr_comments) == 1 # welcome # Processing it again won't change anything. pull_request_changed(pr.as_json()) pr_comments = pr.list_comments() - assert len(pr_comments) == 2 # 1 welcome, 1 survey + assert len(pr_comments) == 1 # welcome def test_track_additions_deletions(fake_github, fake_jira, is_merged): diff --git a/tests/test_pull_request_opened.py b/tests/test_pull_request_opened.py index a6009bbb..108c39a0 100644 --- a/tests/test_pull_request_opened.py +++ b/tests/test_pull_request_opened.py @@ -145,9 +145,7 @@ def test_external_pr_opened_no_cla(has_jira, fake_github, fake_jira): issue_id2, anything_happened2 = close_and_reopen_pr(pr) assert issue_id2 == issue_id assert anything_happened2 is True - # Now there is one comment: closing the PR added a survey comment, but - # re-opening it deleted it. - assert len(pr.list_comments()) == 1 + assert len(pr.list_comments()) == 0 if has_jira: issue = fake_jira.issues[issue_id] @@ -210,10 +208,8 @@ def test_external_pr_opened_with_cla(has_jira, fake_github, fake_jira): issue_id2, anything_happened2 = close_and_reopen_pr(pr) assert issue_id2 == issue_id assert anything_happened2 is True - # Now there is one comment: closing the PR added a survey comment, but - # re-opening it deleted it. pr_comments = pr.list_comments() - assert len(pr_comments) == 1 + assert len(pr_comments) == 0 if has_jira: issue = fake_jira.issues[issue_id]