diff --git a/libraries/github.py b/libraries/github.py index f5b18562..94374087 100644 --- a/libraries/github.py +++ b/libraries/github.py @@ -7,7 +7,7 @@ import structlog from fastcore.xtras import obj2dict from ghapi.all import GhApi, paged -from .models import Category, Issue, Library +from .models import Category, Issue, Library, PullRequest from .utils import parse_date logger = structlog.get_logger() @@ -381,4 +381,53 @@ class GithubUpdater: def update_prs(self): self.logger.info("updating_repo_prs") - # raise ValueError("testing!") + + prs_data = repo_prs(self.owner, self.library.name, state="all") + + for pr_dict in prs_data: + + # Get the date information + closed_at = None + merged_at = None + created_at = None + modified_at = None + + if pr_dict.get("closed_at"): + closed_at = parse_date(pr_dict["closed_at"]) + + if pr_dict.get("merged_at"): + merged_at = parse_date(pr_dict["merged_at"]) + + if pr_dict.get("created_at"): + created_at = parse_date(pr_dict["created_at"]) + + if pr_dict.get("updated_at"): + modified_at = parse_date(pr_dict["updated_at"]) + + try: + pull_request, created = PullRequest.objects.update_or_create( + library=self.library, + github_id=pr_dict["id"], + defaults={ + "title": pr_dict["title"][:255], + "number": pr_dict["number"], + "is_open": pr_dict["state"] == "open", + "closed": closed_at, + "merged": merged_at, + "created": created_at, + "modified": modified_at, + "data": obj2dict(pr_dict), + }, + ) + except Exception as e: + logger.exception( + "update_prs_error_skipped_pr", + pr_github_id=pr_dict.get("id"), + exc_msg=str(e), + ) + logger.info( + "pull_request_updated_successfully", + pr_id=pull_request.id, + created_pr=created, + pr_github_id=pull_request.github_id, + ) diff --git a/libraries/tests/fixtures.py b/libraries/tests/fixtures.py index 15724074..a8a85bc2 100644 --- a/libraries/tests/fixtures.py +++ b/libraries/tests/fixtures.py @@ -101,6 +101,49 @@ def github_api_repo_issues_response(db): ] +@pytest.fixture +def github_api_repo_prs_response(db): + """Returns the response from GhApi().pulls.list, already paged""" + return [ + dict2obj( + { + "title": "Improve logging", + "number": 1, + "state": "closed", + "closed_at": "2022-04-11T12:38:24Z", + "merged_at": "2022-04-11T12:38:24Z", + "created_at": "2022-04-11T11:41:02Z", + "updated_at": "2022-04-11T12:38:25Z", + "id": 5898798798, + } + ), + dict2obj( + { + "title": "Fix a test", + "number": 2, + "state": "open", + "closed_at": "2022-04-11T12:38:24Z", + "merged_at": "2022-04-11T12:38:24Z", + "created_at": "2022-04-11T11:41:02Z", + "updated_at": "2022-04-11T12:38:25Z", + "id": 7395968281, + } + ), + dict2obj( + { + "title": "Add a new feature", + "number": 3, + "state": "closed", + "closed_at": "2022-04-11T12:38:24Z", + "merged_at": "2022-04-11T12:38:24Z", + "created_at": "2022-04-11T11:41:02Z", + "updated_at": "2022-04-11T12:38:25Z", + "id": 7492027464, + } + ), + ] + + @pytest.fixture def boost_module(): return {"module": "rational", "url": "rational"} diff --git a/libraries/tests/test_github.py b/libraries/tests/test_github.py index 447dfc6b..fe88aff8 100644 --- a/libraries/tests/test_github.py +++ b/libraries/tests/test_github.py @@ -7,7 +7,7 @@ from ghapi.all import GhApi from model_bakery import baker from libraries.github import GithubUpdater, LibraryUpdater, get_api -from libraries.models import Issue, Library +from libraries.models import Issue, Library, PullRequest def test_get_api(): @@ -16,9 +16,6 @@ def test_get_api(): # GithubUpdater tests -@pytest.mark.skip(reason="Method not yet written") -def test_update_prs(): - pass def test_update_issues_new(tp, library, github_api_repo_issues_response): @@ -56,7 +53,7 @@ def test_update_issues_new(tp, library, github_api_repo_issues_response): def test_update_issues_existing(tp, library, github_api_repo_issues_response): - """GithubUpdater.update_issues()""" + """Test that GithubUpdater.update_issues() updates existing issues when appropriate""" existing_issue_data = github_api_repo_issues_response[0] old_title = "Old title" issue = baker.make( @@ -84,7 +81,7 @@ def test_update_issues_existing(tp, library, github_api_repo_issues_response): def test_update_issues_long_title(tp, library, github_api_repo_issues_response): - """GithubUpdater.update_issues()""" + """Test that GithubUpdater.update_issues() handles long title gracefully""" new_issues_count = len(github_api_repo_issues_response) expected_count = Issue.objects.count() + new_issues_count title = "sample" * 100 @@ -106,6 +103,70 @@ def test_update_issues_long_title(tp, library, github_api_repo_issues_response): assert issue.title == expected_title +def test_update_prs_new(tp, library, github_api_repo_prs_response): + """Test that GithubUpdater.update_prs() imports new PRs appropriately""" + new_prs_count = len(github_api_repo_prs_response) + expected_count = PullRequest.objects.count() + new_prs_count + + with patch("libraries.github.repo_prs") as repo_prs_mock: + updater = GithubUpdater(library=library) + github_api_repo_prs_response[0]["title"] = "sample" * 100 + repo_prs_mock.return_value = github_api_repo_prs_response + updater.update_prs() + + assert PullRequest.objects.count() == expected_count + ids = [pr.id for pr in github_api_repo_prs_response] + pulls = PullRequest.objects.filter(library=library, github_id__in=ids) + assert pulls.exists() + assert pulls.count() == expected_count + + # Test the values of a sample PR + gh_pull = github_api_repo_prs_response[0] + pr = pulls.get(github_id=gh_pull.id) + assert pr.title == gh_pull.title[:255] + assert pr.number == gh_pull.number + if gh_pull.state == "open": + assert pr.is_open + else: + assert not pr.is_open + assert pr.data == gh_pull + + expected_closed = parse(gh_pull["closed_at"]) + expected_created = parse(gh_pull["created_at"]) + expected_modified = parse(gh_pull["updated_at"]) + assert pr.closed == expected_closed + assert pr.created == expected_created + assert pr.modified == expected_modified + + +def test_update_prs_existing(tp, library, github_api_repo_prs_response): + """Test that GithubUpdater.update_prs() updates existing PRs when appropriate""" + existing_pr_data = github_api_repo_prs_response[0] + old_title = "Old title" + pull = baker.make( + PullRequest, library=library, github_id=existing_pr_data.id, title=old_title + ) + + # Make sure we are expected one fewer new PRs, since we created one in advance + new_prs_count = len(github_api_repo_prs_response) + expected_count = PullRequest.objects.count() + new_prs_count - 1 + + with patch("libraries.github.repo_prs") as repo_prs_mock: + updater = GithubUpdater(library=library) + repo_prs_mock.return_value = github_api_repo_prs_response + updater.update_prs() + + assert PullRequest.objects.count() == expected_count + ids = [pr.id for pr in github_api_repo_prs_response] + pulls = PullRequest.objects.filter(library=library, github_id__in=ids) + assert pulls.exists() + assert pulls.count() == expected_count + + # Test that the existing PR updated + pull.refresh_from_db() + assert pull.title == existing_pr_data.title + + # LibraryUpdater tests diff --git a/libraries/tests/test_views.py b/libraries/tests/test_views.py index 735327f0..396f5ba1 100644 --- a/libraries/tests/test_views.py +++ b/libraries/tests/test_views.py @@ -14,10 +14,28 @@ def test_library_detail(library, tp): tp.response_200(response) -def test_library_detail_issues_context(tp, library): +def test_library_detail_context_get_closed_prs_count(tp, library): """ GET /libraries/{repo}/ - Test that the custom context vars appear as expected + Test that the custom closed_prs_count var appears as expected + """ + # Create open and closed PRs for this library, and another random PR + lib2 = baker.make("libraries.Library", slug="sample") + baker.make("libraries.PullRequest", library=library, is_open=True) + baker.make("libraries.PullRequest", library=library, is_open=False) + baker.make("libraries.PullRequest", library=lib2, is_open=True) + url = tp.reverse("library-detail", library.slug) + response = tp.get(url) + tp.response_200(response) + assert "closed_prs_count" in response.context + # Verify that the count only includes the one open PR for this library + assert response.context["closed_prs_count"] == 1 + + +def test_library_detail_context_get_open_issues_count(tp, library): + """ + GET /libraries/{repo}/ + Test that the custom open_issues_count var appears as expected """ # Create open and closed issues for this library, and another random issue lib2 = baker.make("libraries.Library", slug="sample") diff --git a/libraries/views.py b/libraries/views.py index d4c867eb..330eed67 100644 --- a/libraries/views.py +++ b/libraries/views.py @@ -1,6 +1,6 @@ from django.views.generic import DetailView, ListView -from .models import Category, Issue, Library +from .models import Category, Issue, Library, PullRequest class CategoryMixin: @@ -59,8 +59,12 @@ class LibraryDetail(CategoryMixin, DetailView): def get(self, request, *args, **kwargs): self.object = self.get_object() context = self.get_context_data(object=self.object) + context["closed_prs_count"] = self.get_closed_prs_count(self.object) context["open_issues_count"] = self.get_open_issues_count(self.object) return self.render_to_response(context) + def get_closed_prs_count(self, obj): + return PullRequest.objects.filter(library=obj, is_open=True).count() + def get_open_issues_count(self, obj): return Issue.objects.filter(library=obj, is_open=True).count() diff --git a/templates/libraries/detail.html b/templates/libraries/detail.html index 6a0bcc9e..a586eaa7 100644 --- a/templates/libraries/detail.html +++ b/templates/libraries/detail.html @@ -63,7 +63,7 @@