Update

Author: rohitvinnakota-codecov

api/public/v2/pull/serializers.py

@@ -1,10 +1,14 @@
+from typing import Dict, Optional
+
 from rest_framework import serializers
 
 from api.public.v2.owner.serializers import OwnerSerializer
-from api.shared.commit.serializers import CommitTotalsSerializer
-from compare.models import CommitComparison
+from api.shared.commit.serializers import (
+    CommitTotalsSerializer,
+    PatchCoverageSerializer,
+)
 from core.models import Pull, PullStates
-from services.comparison import ComparisonReport
+from services.comparison import CommitComparisonService, ComparisonReport
 
 
 class PullSerializer(serializers.ModelSerializer):
@@ -37,38 +41,26 @@ class Meta:
         )
         fields = read_only_fields
 
-    def get_patch(self, obj: Pull):
-        # 1) Fetch the CommitComparison for (compared_to, head)
-        comparison_qs = CommitComparison.objects.filter(
-            base_commit__commitid=obj.compared_to,
-            compare_commit__commitid=obj.head,
-            base_commit__repository_id=obj.repository_id,
-            compare_commit__repository_id=obj.repository_id,
-        ).select_related("compare_commit", "base_commit")
-
-        commit_comparison = comparison_qs.first()
+    def get_patch(self, obj: Pull) -> Optional[Dict[str, float]]:
+        commit_comparison = CommitComparisonService.get_commit_comparison_for_pull(obj)
         if not commit_comparison or not commit_comparison.is_processed:
             return None
-
-        # 2) Wrap it in ComparisonReport
         cr = ComparisonReport(commit_comparison)
-
-        # 3) Summation of patch coverage across impacted files
         hits = misses = partials = 0
         for f in cr.impacted_files:
             pc = f.patch_coverage
             if pc:
                 hits += pc.hits
                 misses += pc.misses
                 partials += pc.partials
-
         total_branches = hits + misses + partials
-        if total_branches == 0:
-            return None
-
-        return dict(
+        coverage = 0
+        if total_branches != 0:
+            coverage = round(100 * hits / total_branches, 2)
+        data = dict(
             hits=hits,
             misses=misses,
             partials=partials,
-            coverage=round(100 * hits / total_branches, 2),
+            coverage=coverage,
         )
+        return PatchCoverageSerializer(data).data

api/public/v2/tests/test_api_pull_viewset.py

@@ -32,6 +32,7 @@ def setUp(self):
         )
         self.client = APIClient()
         self.client.force_login_owner(self.current_owner)
+        self.no_patch_response = dict(hits=0, misses=0, partials=0, coverage=0)
 
     def test_list(self):
         res = self.client.get(
@@ -59,7 +60,7 @@ def test_list(self):
                     "state": "open",
                     "ci_passed": None,
                     "author": None,
-                    "patch": None,
+                    "patch": self.no_patch_response,
                 },
                 {
                     "pullid": self.pulls[0].pullid,
@@ -70,7 +71,7 @@ def test_list(self):
                     "state": "open",
                     "ci_passed": None,
                     "author": None,
-                    "patch": None,
+                    "patch": self.no_patch_response,
                 },
             ],
             "total_pages": 1,
@@ -102,7 +103,7 @@ def test_list_state(self):
                     "state": "closed",
                     "ci_passed": None,
                     "author": None,
-                    "patch": None,
+                    "patch": self.no_patch_response,
                 }
             ],
             "total_pages": 1,
@@ -133,7 +134,7 @@ def test_list_start_date(self):
                     "state": "open",
                     "ci_passed": None,
                     "author": None,
-                    "patch": None,
+                    "patch": self.no_patch_response,
                 }
             ],
             "total_pages": 1,
@@ -161,7 +162,7 @@ def test_list_cursor_pagination(self):
                 "state": "open",
                 "ci_passed": None,
                 "author": None,
-                "patch": None,
+                "patch": self.no_patch_response,
             }
         ]
         assert data["previous"] is None
@@ -179,7 +180,7 @@ def test_list_cursor_pagination(self):
                 "state": "open",
                 "ci_passed": None,
                 "author": None,
-                "patch": None,
+                "patch": self.no_patch_response,
             }
         ]
         assert data["previous"] is not None
@@ -209,7 +210,7 @@ def test_retrieve(self, get_repo_permissions):
             "state": "open",
             "ci_passed": None,
             "author": None,
-            "patch": None,
+            "patch": self.no_patch_response,
         }
 
     @patch("api.shared.permissions.RepositoryArtifactPermissions.has_permission")
@@ -305,7 +306,7 @@ def test_pull_with_valid_super_token(self):
             "state": "open",
             "ci_passed": None,
             "author": None,
-            "patch": None,
+            "patch": self.no_patch_response,
         }
 
     @patch("api.public.v2.pull.serializers.ComparisonReport")
@@ -371,4 +372,4 @@ def test_retrieve_with_patch_coverag_no_branches(
         )
         assert res.status_code == 200
         data = res.json()
-        assert data["patch"] is None
+        assert data["patch"] is self.no_patch_response

api/shared/commit/serializers.py

@@ -21,6 +21,13 @@ def get_coverage(self, totals) -> float:
         return 0
 
 
+class PatchCoverageSerializer(serializers.Serializer):
+    hits = serializers.IntegerField()
+    misses = serializers.IntegerField()
+    partials = serializers.IntegerField()
+    coverage = serializers.FloatField()
+
+
 class CommitTotalsSerializer(BaseTotalsSerializer):
     files = serializers.IntegerField(source="f")
     lines = serializers.IntegerField(source="n")

services/comparison.py

@@ -20,7 +20,7 @@
 from shared.utils.merge import LineType, line_type
 
 from compare.models import CommitComparison
-from core.models import Commit
+from core.models import Commit, Pull
 from reports.models import CommitReport
 from services import ServiceException
 from services.redis_configuration import get_redis_connection
@@ -1186,7 +1186,8 @@ def update_base_report_with_pseudo_diff(self):
 class CommitComparisonService:
     """
     Utilities for determining whether a commit comparison needs to be recomputed
-    (and enqueueing that recompute when necessary)
+    (and enqueueing that recompute when necessary), and fetching associated comparisons
+    for pulls
     """
 
     def __init__(self, commit_comparison: CommitComparison):
@@ -1246,6 +1247,16 @@ def _load_commit(self, commit_id: int) -> Optional[Commit]:
             .first()
         )
 
+    @staticmethod
+    def get_commit_comparison_for_pull(obj: Pull):
+        comparison_qs = CommitComparison.objects.filter(
+            base_commit__commitid=obj.compared_to,
+            compare_commit__commitid=obj.head,
+            base_commit__repository_id=obj.repository_id,
+            compare_commit__repository_id=obj.repository_id,
+        ).select_related("compare_commit", "base_commit")
+        return comparison_qs.first()
+
     @classmethod
     def fetch_precomputed(self, repo_id: int, keys: List[Tuple]) -> QuerySet:
         comparison_table = CommitComparison._meta.db_table