Make sure calls to fetch_commit_yaml are Owner type (#823)

Co-authored-by: michelletran-codecov <[email protected]>

Author: JerrySentry

api/public/v2/compare/views.py

@@ -117,7 +117,7 @@ def components(self, request, *args, **kwargs):
         Returns component comparisons
         """
         comparison = self.get_object()
-        components = commit_components(comparison.head_commit, request.user)
+        components = commit_components(comparison.head_commit, self.owner)
         component_comparisons = [
             ComponentComparison(comparison, component) for component in components
         ]

api/public/v2/component/views.py

@@ -40,7 +40,7 @@ def list(self, request, *args, **kwargs):
         """
         commit = self.get_commit()
         report = commit.full_report
-        components = commit_components(commit, request.user)
+        components = commit_components(commit, self.owner)
         components_with_coverage = []
         for component in components:
             component_report = component_filtered_report(report, [component])

api/public/v2/report/views.py

@@ -98,7 +98,7 @@ def filter_report(
             component = next(
                 (
                     component
-                    for component in commit_components(commit, self.request.user)
+                    for component in commit_components(commit, self.owner)
                     if component.component_id == component_id
                 ),
                 None,

api/public/v2/tests/test_api_compare_viewset.py

@@ -715,6 +715,7 @@ def test_components_comparison(
             content_type="application/json",
         )
 
+        commit_components.assert_called_once_with(self.head, self.org)
         assert res.status_code == 200
         assert res.json() == [
             {

api/public/v2/tests/test_api_component_viewset.py

@@ -72,11 +72,11 @@ def _request_components(self):
     @patch("api.public.v2.component.views.commit_components")
     @patch("shared.reports.api_report_service.build_report_from_commit")
     def test_component_list(
-        self, build_report_from_commit, commit_compontents, get_repo_permissions
+        self, build_report_from_commit, commit_components, get_repo_permissions
     ):
         get_repo_permissions.return_value = (True, True)
         build_report_from_commit.side_effect = [sample_report()]
-        commit_compontents.return_value = [
+        commit_components.return_value = [
             Component(
                 component_id="foo",
                 paths=[r".*foo"],
@@ -94,6 +94,7 @@ def test_component_list(
         ]
 
         res = self._request_components()
+        commit_components.assert_called_once_with(self.commit, self.org)
         assert res.status_code == 200
         assert res.json() == [
             {"component_id": "foo", "name": "Foo", "coverage": 62.5},
@@ -103,11 +104,11 @@ def test_component_list(
     @patch("api.public.v2.component.views.commit_components")
     @patch("shared.reports.api_report_service.build_report_from_commit")
     def test_component_list_no_coverage(
-        self, build_report_from_commit, commit_compontents, get_repo_permissions
+        self, build_report_from_commit, commit_components, get_repo_permissions
     ) -> None:
         get_repo_permissions.return_value = (True, True)
         build_report_from_commit.side_effect = [empty_report()]
-        commit_compontents.return_value = [
+        commit_components.return_value = [
             Component(
                 component_id="foo",
                 paths=[r".*foo"],
@@ -118,6 +119,7 @@ def test_component_list_no_coverage(
         ]
 
         res = self._request_components()
+        commit_components.assert_called_once_with(self.commit, self.org)
         assert res.status_code == 200
         assert res.json() == [
             {

api/public/v2/tests/test_report_viewset.py

@@ -971,6 +971,7 @@ def test_report_component(
         build_report_from_commit.return_value = flags_report()
 
         res = self._request_report(component_id="foo")
+        commit_components.assert_called_once_with(self.commit1, self.org)
         assert res.status_code == 200
         assert res.json() == {
             "totals": {

api/public/v2/tests/test_totals_viewset.py

@@ -617,6 +617,7 @@ def test_report_component(
         build_report_from_commit.return_value = sample_report()
 
         res = self._request_report(component_id="foo")
+        commit_components.assert_called_once_with(self.commit1, self.org)
         assert res.status_code == 200
         assert res.json() == {
             "totals": {

services/tests/test_yaml.py

@@ -1,5 +1,6 @@
 from unittest.mock import patch
 
+import pytest
 from django.test import TransactionTestCase
 from shared.torngit.exceptions import TorngitObjectNotFoundError
 
@@ -31,3 +32,27 @@ def test_when_commit_has_no_yaml(self, mock_fetch_yaml):
         )
         config = yaml.final_commit_yaml(self.commit, None)
         assert config["codecov"]["require_ci_to_pass"] is True
+
+    @patch("services.yaml.fetch_current_yaml_from_provider_via_reference")
+    def test_when_commit_has_yaml_with_wrongly_typed_owner_arg(self, mock_fetch_yaml):
+        mock_fetch_yaml.return_value = """
+        codecov:
+          notify:
+            require_ci_to_pass: no
+        """
+        with pytest.raises(TypeError) as exc_info:
+            yaml.final_commit_yaml(self.commit, "something else")
+        assert (
+            str(exc_info.value)
+            == "fetch_commit_yaml owner arg must be Owner or None. Provided: <class 'str'>"
+        )
+
+    @patch("services.yaml.fetch_current_yaml_from_provider_via_reference")
+    def test_when_commit_has_yaml_with_owner(self, mock_fetch_yaml):
+        mock_fetch_yaml.return_value = """
+        codecov:
+          notify:
+            require_ci_to_pass: no
+        """
+        config = yaml.final_commit_yaml(self.commit, self.org)
+        assert config["codecov"]["require_ci_to_pass"] is False

services/yaml.py

@@ -25,6 +25,16 @@ def fetch_commit_yaml(commit: Commit, owner: Owner | None) -> Dict | None:
     Fetches the codecov.yaml file for a particular commit from the service provider.
     Service provider API request is made on behalf of the given `owner`.
     """
+
+    # There's been a lot of instances where this function is called where the owner arg is not Owner type
+    # Previously this issue is masked by the catch all exception handling. Most badly typed calls have
+    # been addressed, but there might still be some entrypoints unaccounted for, will fix new discoveries
+    # from this assertion being raised.
+    if owner is not None and not isinstance(owner, Owner):
+        raise TypeError(
+            f"fetch_commit_yaml owner arg must be Owner or None. Provided: {type(owner)}"
+        )
+
     try:
         repository_service = RepoProviderService().get_adapter(
             owner=owner, repo=commit.repository

upload/tests/views/test_empty_upload.py

@@ -4,6 +4,7 @@
 from rest_framework.test import APIClient
 from shared.yaml.user_yaml import UserYaml
 
+from codecov_auth.tests.factories import OrganizationLevelTokenFactory
 from core.tests.factories import CommitFactory, RepositoryFactory
 from upload.views.uploads import CanDoCoverageUploadsPermission
 
@@ -481,3 +482,101 @@ def test_empty_upload_no_auth(db, mocker):
         == "Failed token authentication, please double-check that your repository token matches in the Codecov UI, "
         "or review the docs https://docs.codecov.com/docs/adding-the-codecov-token"
     )
+
+
+@patch("services.yaml.fetch_commit_yaml")
+@patch("services.task.TaskService.notify")
+@patch("services.repo_providers.RepoProviderService.get_adapter")
+def test_empty_upload_commit_yaml_org_token(
+    mock_repo_provider_service, notify_mock, fetch_yaml_mock, db, mocker
+):
+    mocker.patch.object(
+        CanDoCoverageUploadsPermission, "has_permission", return_value=True
+    )
+    mock_repo_provider_service.return_value = MockedProviderAdapter(
+        ["README.md", "codecov.yml", "template.txt", "base.py"]
+    )
+    fetch_yaml_mock.return_value = None
+
+    repository = RepositoryFactory(
+        name="the_repo", author__username="codecov", author__service="github"
+    )
+    commit = CommitFactory(repository=repository)
+    org_token = OrganizationLevelTokenFactory.create(owner=repository.author)
+    repository.save()
+    commit.save()
+    org_token.save()
+
+    client = APIClient()
+    url = reverse(
+        "new_upload.empty_upload",
+        args=[
+            "github",
+            "codecov::::the_repo",
+            commit.commitid,
+        ],
+    )
+    response = client.post(
+        url,
+        headers={"Authorization": f"token {org_token.token}"},
+    )
+    response_json = response.json()
+    assert response.status_code == 200
+    assert (
+        response_json.get("result")
+        == "Some files cannot be ignored. Triggering failing notifications."
+    )
+    assert response_json.get("non_ignored_files") == ["base.py"]
+    notify_mock.assert_called_once_with(
+        repoid=repository.repoid, commitid=commit.commitid, empty_upload="fail"
+    )
+
+    fetch_yaml_mock.assert_called_once_with(commit, repository.author)
+
+
+@patch("services.yaml.fetch_commit_yaml")
+@patch("services.task.TaskService.notify")
+@patch("services.repo_providers.RepoProviderService.get_adapter")
+def test_empty_upload_ommit_yaml_repo_token(
+    mock_repo_provider_service, notify_mock, fetch_yaml_mock, db, mocker
+):
+    mocker.patch.object(
+        CanDoCoverageUploadsPermission, "has_permission", return_value=True
+    )
+    mock_repo_provider_service.return_value = MockedProviderAdapter(
+        ["README.md", "codecov.yml", "template.txt", "base.py"]
+    )
+    fetch_yaml_mock.return_value = None
+
+    repository = RepositoryFactory(
+        name="the_repo", author__username="codecov", author__service="github"
+    )
+    commit = CommitFactory(repository=repository)
+    repository.save()
+    commit.save()
+
+    client = APIClient()
+    url = reverse(
+        "new_upload.empty_upload",
+        args=[
+            "github",
+            "codecov::::the_repo",
+            commit.commitid,
+        ],
+    )
+    response = client.post(
+        url,
+        headers={"Authorization": f"token {repository.upload_token}"},
+    )
+    response_json = response.json()
+    assert response.status_code == 200
+    assert (
+        response_json.get("result")
+        == "Some files cannot be ignored. Triggering failing notifications."
+    )
+    assert response_json.get("non_ignored_files") == ["base.py"]
+    notify_mock.assert_called_once_with(
+        repoid=repository.repoid, commitid=commit.commitid, empty_upload="fail"
+    )
+
+    fetch_yaml_mock.assert_called_once_with(commit, repository.author)