Skip to content

Commit

Permalink
feat!: removed historical perspective on users' state
Browse files Browse the repository at this point in the history
  • Loading branch information
Ned Batchelder committed Aug 8, 2023
1 parent 51cc2fa commit 7ad8eee
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 134 deletions.
5 changes: 5 additions & 0 deletions changelog.d/20230808_121854_nedbat_no_cc.rst
Original file line number Diff line number Diff line change
@@ -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.
29 changes: 1 addition & 28 deletions openedx_webhooks/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion requirements/base.in
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ click
cryptography
glom
gunicorn
iso8601
jira
logging_tree
oauthlib[signedtoken]
Expand Down
53 changes: 3 additions & 50 deletions tests/test_info.py
Original file line number Diff line number Diff line change
@@ -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,
)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand Down
23 changes: 0 additions & 23 deletions tests/test_pull_request_closed.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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<!-- comment:external_pr -->"
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
Expand Down
32 changes: 0 additions & 32 deletions tests/test_pull_request_opened.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 7ad8eee

Please sign in to comment.