From 7ad8eee3d002f17e4acceea16630c27eab8f858f Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Tue, 8 Aug 2023 12:20:13 -0400 Subject: [PATCH] feat!: removed historical perspective on users' state --- changelog.d/20230808_121854_nedbat_no_cc.rst | 5 ++ openedx_webhooks/info.py | 29 +---------- requirements/base.in | 1 - tests/test_info.py | 53 ++------------------ tests/test_pull_request_closed.py | 23 --------- tests/test_pull_request_opened.py | 32 ------------ 6 files changed, 9 insertions(+), 134 deletions(-) create mode 100644 changelog.d/20230808_121854_nedbat_no_cc.rst diff --git a/changelog.d/20230808_121854_nedbat_no_cc.rst b/changelog.d/20230808_121854_nedbat_no_cc.rst new file mode 100644 index 00000000..19d4dfba --- /dev/null +++ b/changelog.d/20230808_121854_nedbat_no_cc.rst @@ -0,0 +1,5 @@ +.. A new scriv changelog fragment. + +- Removed: the bot no longer understands the past state of users. This data + hasn't been maintained for the last few years. There's no point relying on + stale data, so the capability is removed. diff --git a/openedx_webhooks/info.py b/openedx_webhooks/info.py index 5fff3449..ca8b6719 100644 --- a/openedx_webhooks/info.py +++ b/openedx_webhooks/info.py @@ -2,14 +2,12 @@ Get information about people, repos, orgs, pull requests, etc. """ import csv -import datetime import logging import re from typing import Dict, Iterable, Optional, Tuple, Union import yaml from glom import glom -from iso8601 import parse_date from openedx_webhooks import settings from openedx_webhooks.lib.github.models import PrId @@ -135,24 +133,6 @@ def get_orgs_file(): orgs[org_data["name"]] = org_data return orgs -def get_person_certain_time(person: Dict, certain_time: datetime.datetime) -> Dict: - """ - Return person data structure for a particular time - - Arguments: - person: dict of GitHub user info from people.yaml. - certain_time: datetime.datetime object used to determine the state of the person. - - """ - # Layer together all of the applicable "before" clauses. - update_person = person.copy() - for before_date in sorted(person.get("before", {}), reverse=True): - if certain_time.date() > before_date: - break - update_person.update(person["before"][before_date]) - return update_person - - def is_internal_pull_request(pull_request: PrDict) -> bool: """ Is this pull request's author internal to the PR's GitHub org? @@ -221,14 +201,7 @@ def _pr_author_data(pull_request: PrDict) -> Optional[Dict]: """ people = get_people_file() author = pull_request["user"]["login"] - if author not in people: - # We don't know this person! - return None - - person = people[author] - created_at = parse_date(pull_request["created_at"]).replace(tzinfo=None) - person = get_person_certain_time(people[author], created_at) - return person + return people.get(author) def is_committer_pull_request(pull_request: PrDict) -> bool: diff --git a/requirements/base.in b/requirements/base.in index d41faf67..c58c790d 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -16,7 +16,6 @@ click cryptography glom gunicorn -iso8601 jira logging_tree oauthlib[signedtoken] diff --git a/tests/test_info.py b/tests/test_info.py index 431fa06d..f26fa34e 100644 --- a/tests/test_info.py +++ b/tests/test_info.py @@ -1,14 +1,12 @@ """ Tests of the functions in info.py """ -from datetime import datetime import pytest from openedx_webhooks.info import ( - get_people_file, get_person_certain_time, + get_people_file, is_committer_pull_request, is_internal_pull_request, is_draft_pull_request, - pull_request_has_cla, get_blended_project_id, ) @@ -49,11 +47,6 @@ def test_ex_edx_employee(make_pull_request): pr = make_pull_request("mmprandom") assert not is_internal_pull_request(pr) -def test_ex_edx_employee_old_pr(make_pull_request): - created_at = datetime(2014, 1, 1) - pr = make_pull_request("jarv", created_at=created_at) - assert is_internal_pull_request(pr) - def test_never_heard_of_you(make_pull_request): pr = make_pull_request("some_random_guy") assert not is_internal_pull_request(pr) @@ -107,55 +100,15 @@ def test_base_branch_committers(make_pull_request): def test_current_person_no_institution(): people = get_people_file() - created_at = datetime.today() - current_person = get_person_certain_time(people["jarv"], created_at) + current_person = people["jarv"] assert "institution" not in current_person assert current_person["agreement"] == "individual" def test_current_person(): people = get_people_file() - created_at = datetime.today() - current_person = get_person_certain_time(people["raisingarizona"], created_at) + current_person = people["raisingarizona"] assert current_person["agreement"] == "none" -def test_updated_person_has_institution(): - people = get_people_file() - created_at = datetime(2014, 1, 1) - updated_person = get_person_certain_time(people["jarv"], created_at) - assert updated_person["institution"] == "edX" - -def test_updated_person(): - # This only works if "before" clauses are layered together properly. - people = get_people_file() - created_at = datetime(2014, 1, 1) - updated_person = get_person_certain_time(people["raisingarizona"], created_at) - assert updated_person["agreement"] == "individual" - -@pytest.mark.parametrize("who, when, cc", [ - ("raisingarizona", datetime(2020, 12, 31), False), - ("raisingarizona", datetime(2015, 12, 31), False), - ("raisingarizona", datetime(2014, 12, 31), True), - ("raisingarizona", datetime(2013, 12, 31), False), - ("hollyhunter", datetime(2020, 12, 31), True), - ("hollyhunter", datetime(2019, 12, 31), False), -]) -def test_old_committer(make_pull_request, who, when, cc): - pr = make_pull_request(who, created_at=when) - assert is_committer_pull_request(pr) == cc - -@pytest.mark.parametrize("user, created_at_args, has_cla", [ - ("nedbat", (2020, 7, 2), True), - ("raisingarizona", (2020, 7, 2), False), - ("raisingarizona", (2017, 1, 1), True), - ("raisingarizona", (2016, 1, 1), False), - ("raisingarizona", (2015, 1, 1), True), - ("never-heard-of-her", (2020, 7, 2), False), -]) -def test_pull_request_has_cla(make_pull_request, user, created_at_args, has_cla): - pr = make_pull_request(user, created_at=datetime(*created_at_args)) - assert pull_request_has_cla(pr) is has_cla - - @pytest.mark.parametrize("title, number", [ ("Please take my change", None), ("[BD-17] Fix typo", 17), diff --git a/tests/test_pull_request_closed.py b/tests/test_pull_request_closed.py index c4b10f1c..dc555f0e 100644 --- a/tests/test_pull_request_closed.py +++ b/tests/test_pull_request_closed.py @@ -1,14 +1,11 @@ """Tests of task/github.py:pull_request_changed for closing pull requests.""" -from datetime import datetime - import pytest from openedx_webhooks.bot_comments import ( BotComment, is_comment_kind, ) -from openedx_webhooks.info import get_bot_username, pull_request_has_cla from openedx_webhooks.tasks.github import pull_request_changed from .helpers import check_issue_link_in_markdown, jira_server, random_text @@ -155,26 +152,6 @@ def test_track_additions_deletions(fake_github, fake_jira, is_merged): assert issue.lines_deleted == 1001 -def test_rescan_closed_with_wrong_cla(fake_github, fake_jira): - # No CLA, because this person had no agreement when the pr was made. - pr = fake_github.make_pull_request( - user="raisingarizona", created_at=datetime(2015, 6, 15), state="closed", merged=True, - ) - assert not pull_request_has_cla(pr.as_json()) - # But the bot comment doesn't indicate "no cla", because people.yaml is wrong. - body = "A ticket: OSPR-1234!\n" - pr.add_comment(user=get_bot_username(), body=body) - - pull_request_changed(pr.as_json()) - - # The fixer will think, the bot comment needs to have no_cla added to it, - # and will want to edit the bot comment. But we shouldn't change existing - # bot comments on closed pull requests. - pr_comments = pr.list_comments() - assert len(pr_comments) == 1 - assert pr_comments[0].body == body - - @pytest.mark.parametrize("new_jira", [None, "https://new-jira.atlassian.net"]) def test_close_jira_pr_with_new_jira( fake_github, requests_mocker, new_jira, is_merged diff --git a/tests/test_pull_request_opened.py b/tests/test_pull_request_opened.py index c31cebd5..292834f6 100644 --- a/tests/test_pull_request_opened.py +++ b/tests/test_pull_request_opened.py @@ -1,7 +1,5 @@ """Tests of tasks/github.py:pull_request_changed for opening pull requests.""" -from datetime import datetime - import pytest from openedx_webhooks import settings @@ -297,36 +295,6 @@ def test_core_committer_pr_opened(has_jira, fake_github, fake_jira): assert pull_request_projects(pr.as_json()) == {settings.GITHUB_OSPR_PROJECT} -def test_old_core_committer_pr_opened(fake_github, fake_jira): - # No-one was a core committer before June 2020. - # This test only asserts the core-committer things, that they are not cc. - pr = fake_github.make_pull_request( - user="felipemontoya", owner="openedx", repo="edx-platform", created_at=datetime(2020, 1, 1), - ) - prj = pr.as_json() - - issue_id, _ = pull_request_changed(prj) - - issue = fake_jira.issues[issue_id] - assert issue.labels == set() - - # Check that the Jira issue was started in "Needs Triage" - assert issue.status == "Needs Triage" - - # Check the GitHub comment that was created. - pr_comments = pr.list_comments() - assert len(pr_comments) == 1 - body = pr_comments[0].body - check_issue_link_in_markdown(body, issue_id) - assert "Thanks for the pull request, @felipemontoya!" in body - assert not is_comment_kind(BotComment.CORE_COMMITTER, body) - assert not is_comment_kind(BotComment.NEED_CLA, body) - assert is_comment_kind(BotComment.OK_TO_TEST, body) - - # Check the GitHub labels that got applied. - assert pr.labels == {"needs triage", "open-source-contribution"} - - EXAMPLE_PLATFORM_MAP_1_2 = { "child": { "id": "14522",