diff --git a/metrics/github/prs.py b/metrics/github/prs.py index f939c0df..2337f513 100644 --- a/metrics/github/prs.py +++ b/metrics/github/prs.py @@ -38,10 +38,7 @@ def calculate_counts(prs_by_repo, predicate): for repo, prs in prs_by_repo.items(): for pr in prs: start = pr["created_on"] - end = min( - repo.archived_on if repo.archived_on else date.today(), - pr["closed_on"] if pr["closed_on"] else date.today(), - ) + end = pr["closed_on"] if pr["closed_on"] else date.today() for day in iter_days(start, end): if predicate(pr, day): counts[(repo.org, repo.name, pr["author"], day)] += 1 diff --git a/metrics/github/query.py b/metrics/github/query.py index bd771b86..f5d4d1e6 100644 --- a/metrics/github/query.py +++ b/metrics/github/query.py @@ -32,7 +32,7 @@ def repos(client, org): org, raw_repo["name"], date_from_iso(raw_repo["createdAt"]), - date_from_iso(raw_repo["archivedAt"]), + raw_repo["archivedAt"] is not None, raw_repo["hasVulnerabilityAlertsEnabled"], ) @@ -42,12 +42,9 @@ class Repo: org: str name: str created_on: date - archived_on: date | None + is_archived: bool = False has_vulnerability_alerts_enabled: bool = False - def is_archived(self): - return bool(self.archived_on) - def team_repos(client, org, team): """The API doesn't make it easy for us to get all the information we need about repos in diff --git a/metrics/github/repos.py b/metrics/github/repos.py index 7502b849..2c648227 100644 --- a/metrics/github/repos.py +++ b/metrics/github/repos.py @@ -29,7 +29,7 @@ def get_repo_ownership(client, orgs): def _active_repos(client, org): - return [repo for repo in query.repos(client, org) if not repo.is_archived()] + return [repo for repo in query.repos(client, org) if not repo.is_archived] # GitHub slugs for the teams we're interested in diff --git a/metrics/github/security.py b/metrics/github/security.py index 8ae1283b..587b8c7e 100644 --- a/metrics/github/security.py +++ b/metrics/github/security.py @@ -34,8 +34,7 @@ def vulnerabilities(client, org, to_date): for repo in repos.tech_repos(client, org): vulns = list(map(Vulnerability.from_dict, query.vulnerabilities(client, repo))) - end = min(to_date, repo.archived_on) if repo.archived_on else to_date - for day in dates.iter_days(repo.created_on, end): + for day in dates.iter_days(repo.created_on, to_date): closed_vulns = sum(1 for v in vulns if v.is_closed_on(day)) open_vulns = sum(1 for v in vulns if v.is_open_on(day)) diff --git a/tests/metrics/github/test_prs.py b/tests/metrics/github/test_prs.py index 90c811ee..ac1abf2b 100644 --- a/tests/metrics/github/test_prs.py +++ b/tests/metrics/github/test_prs.py @@ -34,25 +34,6 @@ def test_makes_counts_for_every_day_between_pr_creation_and_now(): } -def test_drops_repos_after_archiving(): - repo_ = repo( - "an-org", - "a-repo", - archived_on=YESTERDAY, - ) - prs = { - repo_: [ - pr(author="an-author", created_on=TWO_DAYS_AGO), - ] - } - - counts = calculate_counts(prs, true) - assert counts == { - ("an-org", "a-repo", "an-author", TWO_DAYS_AGO): 1, - ("an-org", "a-repo", "an-author", YESTERDAY): 1, - } - - def test_counts_prs(): prs = { repo("an-org", "a-repo"): [ @@ -144,12 +125,12 @@ def test_was_merged_in_on(): assert not was_merged_on(pr(merged_on=TOMORROW), TODAY) -def repo(org, name, archived_on=None): +def repo(org, name, is_archived=False): return Repo( org, name, created_on=date.min, - archived_on=archived_on, + is_archived=is_archived, has_vulnerability_alerts_enabled=False, ) diff --git a/tests/metrics/github/test_repos.py b/tests/metrics/github/test_repos.py index 2a6690a4..013e41de 100644 --- a/tests/metrics/github/test_repos.py +++ b/tests/metrics/github/test_repos.py @@ -77,7 +77,7 @@ def test_excludes_archived_repos(patch): patch( "repos", [ - repo("opensafely-core", "other-repo", archived_on=date.min), + repo("opensafely-core", "other-repo", is_archived=True), ], ) assert len(repos.tech_repos(None, "opensafely-core")) == 0 @@ -121,7 +121,7 @@ def test_looks_up_ownership_across_orgs(patch): def test_ignores_ownership_of_archived_repos(patch): - patch("repos", [repo("the_org", "the_repo", archived_on=date.min)]) + patch("repos", [repo("the_org", "the_repo", is_archived=True)]) patch( "team_repos", {"the_org": {"team-rex": ["the_repo"], "team-rap": [], "tech-shared": []}}, @@ -139,11 +139,11 @@ def test_returns_none_for_unknown_ownership(patch): ] -def repo(org, name, archived_on=None): +def repo(org, name, is_archived=False): return Repo( org=org, name=name, created_on=date.min, - archived_on=archived_on, + is_archived=is_archived, has_vulnerability_alerts_enabled=False, ) diff --git a/tests/metrics/github/test_security.py b/tests/metrics/github/test_security.py index d242dce3..283ff252 100644 --- a/tests/metrics/github/test_security.py +++ b/tests/metrics/github/test_security.py @@ -46,35 +46,11 @@ def test_vulnerability_closed_on_is_closed(): assert v.is_closed_on(date(2023, 10, 29)) -def test_vulnerabilities_ignores_archived_repos_after_archive_date(monkeypatch): - archive_date = date(2022, 1, 3) - - def fake_repos(client, org): - return [ - Repo( - "anything", - "anything", - created_on=date(2022, 1, 1), - archived_on=archive_date, - ) - ] - - monkeypatch.setattr(security.repos, "tech_repos", fake_repos) - - def fake_vulnerabilities(client, repo): - return [] - - monkeypatch.setattr(security.query, "vulnerabilities", fake_vulnerabilities) - - result = security.vulnerabilities({}, "org", date(2022, 1, 10)) - assert result[-1]["time"] == archive_date - - def test_vulnerabilities(monkeypatch): def fake_repos(client, org): return [ - Repo(org, "test", date(2023, 10, 13), None, True), - Repo(org, "test2", date(2023, 10, 13), None, True), + Repo(org, "test", date(2023, 10, 13), False, True), + Repo(org, "test2", date(2023, 10, 13), False, True), ] monkeypatch.setattr(security.repos, "tech_repos", fake_repos)