Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: remove the survey link #253

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog.d/20230824_154702_kyle_rm_survey.rst
Original file line number Diff line number Diff line change
@@ -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.

10 changes: 1 addition & 9 deletions docs/details.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.


Expand Down
38 changes: 0 additions & 38 deletions openedx_webhooks/bot_comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ class BotComment(Enum):
NEED_CLA = auto()
BLENDED = auto()
END_OF_WIP = auto()
SURVEY = auto()
NO_CONTRIBUTIONS = auto()


Expand All @@ -52,11 +51,6 @@ class BotComment(Enum):
BotComment.END_OF_WIP: [
"<!-- comment:end_of_wip -->",
],
BotComment.SURVEY: [
"<!-- comment:end_survey -->",
"/1FAIpQLSceJOyGJ6JOzfy6lyR3T7EW_71OWUnNQXp68Fymsk3MkNoSDg/viewform",
"<!-- comment:no_survey_needed -->",
],
BotComment.NO_CONTRIBUTIONS: [
"<!-- comment:no-contributions -->",
],
Expand Down Expand Up @@ -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:
"""
Expand Down
27 changes: 2 additions & 25 deletions openedx_webhooks/tasks/pr_tracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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}"

Expand Down

This file was deleted.

12 changes: 0 additions & 12 deletions openedx_webhooks/templates/github_end_survey.md.j2

This file was deleted.

22 changes: 0 additions & 22 deletions tests/test_pr_comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
BotComment,
is_comment_kind,
github_community_pr_comment,
github_end_survey_comment,
extract_data_from_comment,
format_data_for_comment,
)
Expand Down Expand Up @@ -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.",
Expand Down
10 changes: 3 additions & 7 deletions tests/test_pull_request_closed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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)
Expand Down Expand Up @@ -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):
Expand Down
8 changes: 2 additions & 6 deletions tests/test_pull_request_opened.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down