From 731a367653cfa3c43ecd4e4de8711e3ab5986775 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Thu, 21 Nov 2024 17:49:19 +0100
Subject: [PATCH 01/15] Base endpoint to list issues

---
 backend/code_review_backend/issues/api.py | 139 +++++++++++++++++++---
 1 file changed, 123 insertions(+), 16 deletions(-)

diff --git a/backend/code_review_backend/issues/api.py b/backend/code_review_backend/issues/api.py
index 173d4b53f..37cac4707 100644
--- a/backend/code_review_backend/issues/api.py
+++ b/backend/code_review_backend/issues/api.py
@@ -6,11 +6,20 @@
 from datetime import date, datetime, timedelta
 
 from django.conf import settings
-from django.db.models import BooleanField, Count, ExpressionWrapper, Prefetch, Q
+from django.db.models import (
+    BooleanField,
+    Count,
+    Exists,
+    ExpressionWrapper,
+    OuterRef,
+    Prefetch,
+    Q,
+)
 from django.db.models.functions import TruncDate
 from django.shortcuts import get_object_or_404
 from django.urls import path
 from django.utils.decorators import method_decorator
+from django.utils.functional import cached_property
 from django.views.decorators.cache import cache_page
 from rest_framework import generics, mixins, routers, status, viewsets
 from rest_framework.exceptions import APIException, ValidationError
@@ -486,6 +495,90 @@ def get_queryset(self):
         return qs.filter(**filters).order_by("created").distinct()
 
 
+class GenericIssueList(generics.ListAPIView):
+    serializer_class = IssueHashSerializer
+    allowed_modes = ["unresolved", "known", "closed"]
+
+    @cached_property
+    def diff(self):
+        return get_object_or_404(
+            Diff.objects.select_related("revision"), id=self.kwargs["diff_id"]
+        )
+
+    @cached_property
+    def mode(self):
+        if (mode := self.kwargs["mode"]) not in self.allowed_modes:
+            raise APIException(
+                detail="mode argument must be one of {self.allowed_modes}"
+            )
+        return mode
+
+    def get_queryset(self):
+        return getattr(self, f"get_{self.mode}")()
+
+    def distinct_issues(self, qs):
+        """
+        Convert a list of issue links to unique couples of (issue_id, issue_hash)
+        """
+        attributes = ("issue_id", "issue__hash")
+        return qs.order_by(*attributes).values_list(*attributes).distinct()
+
+    def get_unresolved(self):
+        """
+        Issues that were linked to a previous diff of the same
+        parent revision, and are still present on the current diff.
+        """
+        return self.distinct_issues(
+            self.diff.revision.issue_links.annotate(
+                unresolved=Exists(
+                    IssueLink.objects.exclude(diff=self.diff).filter(
+                        revision=self.diff.revision,
+                        issue=OuterRef("issue"),
+                    )
+                )
+            ).filter(unresolved=True)
+        )
+
+    def get_known(self):
+        """
+        Issues that have also being detected from mozilla-central.
+        """
+        return self.distinct_issues(
+            self.diff.issue_links.annotate(
+                known=Exists(
+                    IssueLink.objects.filter(
+                        revision__base_repository__slug="mozilla-central",
+                        issue=OuterRef("issue"),
+                    )
+                )
+            ).filter(known=True)
+        )
+
+    def get_closed(self):
+        """
+        Issues that were listed on the previous diff of the same
+        parent revision, but does not exist on the current diff.
+        """
+        # Retrieve the previous diff
+        previous_diff = (
+            self.diff.revision.diffs.filter(created__lt=self.diff.created)
+            .order_by("created")
+            .last()
+        )
+        if not previous_diff:
+            return []
+        return self.distinct_issues(
+            previous_diff.issue_links.annotate(
+                closed=Exists(
+                    IssueLink.objects.filter(
+                        diff=self.diff,
+                        issue=OuterRef("issue"),
+                    )
+                )
+            ).filter(closed=True)
+        )
+
+
 # Build exposed urls for the API
 router = routers.DefaultRouter()
 router.register(r"repository", RepositoryViewSet)
@@ -497,18 +590,32 @@ def get_queryset(self):
 )
 router.register(r"diff", DiffViewSet, basename="diffs")
 router.register(r"diff/(?P<diff_id>\d+)/issues", IssueViewSet, basename="issues")
-urls = router.urls + [
-    path(
-        "revision/<int:revision_id>/issues/",
-        IssueBulkCreate.as_view(),
-        name="revision-issues-bulk",
-    ),
-    path("check/stats/", IssueCheckStats.as_view(), name="issue-checks-stats"),
-    path("check/history/", IssueCheckHistory.as_view(), name="issue-checks-history"),
-    path(
-        "check/<str:repository>/<str:analyzer>/<path:check>/",
-        IssueCheckDetails.as_view(),
-        name="issue-check-details",
-    ),
-    path("issues/<slug:repo_slug>/", IssueList.as_view(), name="repository-issues"),
-]
+
+urls = (
+    [
+        # Prevails on the generic issues endpoint
+        path(
+            "diff/<int:diff_id>/issues/<str:mode>/",
+            GenericIssueList.as_view(),
+            name="issue-list",
+        ),
+    ]
+    + router.urls
+    + [
+        path(
+            "revision/<int:revision_id>/issues/",
+            IssueBulkCreate.as_view(),
+            name="revision-issues-bulk",
+        ),
+        path("check/stats/", IssueCheckStats.as_view(), name="issue-checks-stats"),
+        path(
+            "check/history/", IssueCheckHistory.as_view(), name="issue-checks-history"
+        ),
+        path(
+            "check/<str:repository>/<str:analyzer>/<path:check>/",
+            IssueCheckDetails.as_view(),
+            name="issue-check-details",
+        ),
+        path("issues/<slug:repo_slug>/", IssueList.as_view(), name="repository-issues"),
+    ]
+)

From dff247d0b10654a28a00a48e1c224c8882ff8704 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Fri, 22 Nov 2024 11:35:47 +0100
Subject: [PATCH 02/15] Move new endpoint to API /v2/

---
 backend/code_review_backend/app/urls.py      |   3 +-
 backend/code_review_backend/issues/api_v2.py | 107 +++++++++++++++++++
 2 files changed, 109 insertions(+), 1 deletion(-)
 create mode 100644 backend/code_review_backend/issues/api_v2.py

diff --git a/backend/code_review_backend/app/urls.py b/backend/code_review_backend/app/urls.py
index f6f4cd8c8..e0e5ae5ef 100644
--- a/backend/code_review_backend/app/urls.py
+++ b/backend/code_review_backend/app/urls.py
@@ -10,7 +10,7 @@
 from drf_yasg.views import get_schema_view
 from rest_framework import permissions
 
-from code_review_backend.issues import api
+from code_review_backend.issues import api, api_v2
 
 # Build Swagger schema view
 schema_view = get_schema_view(
@@ -28,6 +28,7 @@
 urlpatterns = [
     path("", lambda request: redirect("docs/", permanent=False)),
     path("v1/", include(api.urls)),
+    path("v2/", include(api_v2.urls)),
     path("admin/", admin.site.urls),
     path(
         r"docs<format>\.json|\.yaml)",
diff --git a/backend/code_review_backend/issues/api_v2.py b/backend/code_review_backend/issues/api_v2.py
new file mode 100644
index 000000000..bd955bfe5
--- /dev/null
+++ b/backend/code_review_backend/issues/api_v2.py
@@ -0,0 +1,107 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+from django.db.models import Exists, OuterRef
+from django.shortcuts import get_object_or_404
+from django.urls import path
+from django.utils.functional import cached_property
+from rest_framework.exceptions import ValidationError
+from rest_framework.generics import ListAPIView
+
+from code_review_backend.issues.models import Diff, IssueLink
+from code_review_backend.issues.serializers import IssueHashSerializer
+
+
+class IssueList(ListAPIView):
+    serializer_class = IssueHashSerializer
+    allowed_modes = ["unresolved", "known", "closed"]
+
+    @cached_property
+    def diff(self):
+        return get_object_or_404(
+            Diff.objects.select_related("revision"), id=self.kwargs["diff_id"]
+        )
+
+    @cached_property
+    def mode(self):
+        if (mode := self.kwargs["mode"]) not in self.allowed_modes:
+            raise ValidationError(
+                detail=f"mode argument must be one of {self.allowed_modes}"
+            )
+        return mode
+
+    def get_queryset(self):
+        return getattr(self, f"get_{self.mode}")()
+
+    def distinct_issues(self, qs):
+        """
+        Convert a list of issue links to unique couples of (issue_id, issue_hash)
+        """
+        attributes = ("issue_id", "issue__hash")
+        return qs.order_by(*attributes).values_list(*attributes).distinct()
+
+    def get_unresolved(self):
+        """
+        Issues that were linked to a previous diff of the same
+        parent revision, and are still present on the current diff.
+        """
+        return self.distinct_issues(
+            self.diff.revision.issue_links.annotate(
+                unresolved=Exists(
+                    IssueLink.objects.exclude(diff=self.diff).filter(
+                        revision=self.diff.revision,
+                        issue=OuterRef("issue"),
+                    )
+                )
+            ).filter(unresolved=True)
+        )
+
+    def get_known(self):
+        """
+        Issues that have also being detected from mozilla-central.
+        """
+        return self.distinct_issues(
+            self.diff.issue_links.annotate(
+                known=Exists(
+                    IssueLink.objects.filter(
+                        revision__base_repository__slug="mozilla-central",
+                        issue=OuterRef("issue"),
+                    )
+                )
+            ).filter(known=True)
+        )
+
+    def get_closed(self):
+        """
+        Issues that were listed on the previous diff of the same
+        parent revision, but does not exist on the current diff.
+        """
+        # Retrieve the previous diff
+        previous_diff = (
+            self.diff.revision.diffs.filter(created__lt=self.diff.created)
+            .order_by("created")
+            .last()
+        )
+        if not previous_diff:
+            return []
+        return self.distinct_issues(
+            previous_diff.issue_links.annotate(
+                closed=Exists(
+                    IssueLink.objects.filter(
+                        diff=self.diff,
+                        issue=OuterRef("issue"),
+                    )
+                )
+            ).filter(closed=True)
+        )
+
+
+urls = [
+    # Prevails on the generic issues endpoint
+    path(
+        "diff/<int:diff_id>/issues/<str:mode>/",
+        IssueList.as_view(),
+        name="issue-list",
+    ),
+]

From 11d2b59552b66c99acdf2d69f80a70e375185093 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Fri, 22 Nov 2024 12:06:02 +0100
Subject: [PATCH 03/15] Add test cases

---
 .../issues/tests/test_api_v2.py               | 151 ++++++++++++++++++
 1 file changed, 151 insertions(+)
 create mode 100644 backend/code_review_backend/issues/tests/test_api_v2.py

diff --git a/backend/code_review_backend/issues/tests/test_api_v2.py b/backend/code_review_backend/issues/tests/test_api_v2.py
new file mode 100644
index 000000000..56a5738c6
--- /dev/null
+++ b/backend/code_review_backend/issues/tests/test_api_v2.py
@@ -0,0 +1,151 @@
+from rest_framework import status
+from rest_framework.test import APITestCase
+
+from code_review_backend.issues.models import Issue, Repository
+
+
+class CreationAPIv2TestCase(APITestCase):
+    def setUp(self):
+        # Create the main repository
+        self.repo_main = Repository.objects.create(
+            slug="mozilla-central", url="https://hg.mozilla.org/mozilla-central"
+        )
+        self.repo_try = Repository.objects.create(
+            slug="myrepo-try", url="http://repo.test/try"
+        )
+
+        self.issues = Issue.objects.bulk_create(
+            [
+                Issue(**attrs)
+                for attrs in [
+                    {"hash": "0" * 32},
+                    {"hash": "1" * 32},
+                    {"hash": "2" * 32},
+                    {"hash": "3" * 32},
+                ]
+            ]
+        )
+
+        # Create historic revision with some issues
+        self.revision_main = self.repo_main.head_revisions.create(
+            phabricator_id=456,
+            phabricator_phid="PHID-REV-XXX",
+            title="Main revision",
+            base_repository=self.repo_main,
+        )
+        self.revision_main.issue_links.create(issue=self.issues[0])
+        self.revision_main.issue_links.create(issue=self.issues[1])
+
+        # Create a new revions, with two successive diffs
+        self.revision_last = self.repo_try.head_revisions.create(
+            phabricator_id=1337,
+            phabricator_phid="PHID-REV-YYY",
+            title="Bug XXX - Yet another bug",
+            base_repository=self.repo_try,
+        )
+        self.diff1 = self.revision_last.diffs.create(
+            id=1,
+            repository=self.repo_try,
+            review_task_id="deadbeef123",
+            phid="PHID-DIFF-xxx",
+        )
+        self.diff1.issue_links.create(revision=self.revision_last, issue=self.issues[0])
+        self.diff1.issue_links.create(revision=self.revision_last, issue=self.issues[2])
+        self.diff1.issue_links.create(revision=self.revision_last, issue=self.issues[3])
+
+        self.diff2 = self.revision_last.diffs.create(
+            id=2,
+            repository=self.repo_try,
+            review_task_id="coffee12345",
+            phid="PHID-DIFF-yyy",
+        )
+        self.diff2.issue_links.create(revision=self.revision_last, issue=self.issues[0])
+        self.diff2.issue_links.create(revision=self.revision_last, issue=self.issues[2])
+
+    def test_list_issues_invalid_mode(self):
+        with self.assertNumQueries(0):
+            response = self.client.get(
+                f"/v2/diff/{self.diff2.id}/issues/test/", format="json"
+            )
+            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertListEqual(
+            response.json(),
+            ["mode argument must be one of ['unresolved', 'known', 'closed']"],
+        )
+
+    def test_list_issues_invalid_diff(self):
+        with self.assertNumQueries(1):
+            response = self.client.get("/v2/diff/424242/issues/known/")
+            self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+    def test_list_issues_known(self):
+        with self.assertNumQueries(3):
+            response = self.client.get(f"/v2/diff/{self.diff2.id}/issues/known/")
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertCountEqual(
+            response.json()["results"],
+            [{"id": str(self.issues[0].id), "hash": self.issues[0].hash}],
+        )
+
+    def test_list_issues_unresolved_first_diff(self):
+        """No issue can be marked as unresolved on a new diff"""
+        with self.assertNumQueries(2):
+            response = self.client.get(f"/v2/diff/{self.diff1.id}/issues/unresolved/")
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertCountEqual(
+            response.json()["results"],
+            [],
+        )
+
+    def test_list_issues_unresolved(self):
+        with self.assertNumQueries(4):
+            response = self.client.get(f"/v2/diff/{self.diff2.id}/issues/unresolved/")
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertCountEqual(
+            response.json()["results"],
+            [{"id": str(self.issues[2].id), "hash": self.issues[2].hash}],
+        )
+
+    def test_list_issues_closed_first_diff(self):
+        with self.assertNumQueries(2):
+            response = self.client.get(f"/v2/diff/{self.diff1.id}/issues/closed/")
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertCountEqual(response.json()["results"], [])
+
+    def test_list_issues_closed(self):
+        with self.assertNumQueries(4):
+            response = self.client.get(f"/v2/diff/{self.diff2.id}/issues/closed/")
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertListEqual(
+            response.json()["results"],
+            [{"id": str(self.issues[3].id), "hash": self.issues[3].hash}],
+        )
+
+    def test_list_issues_new_diff_reopen(self):
+        """
+        Open a new diff with 1 known issue and one that has been closed in the previous diff.
+        Both are listed as new, and the two issues of the previous diff listed as closed.
+        """
+        diff = self.revision_last.diffs.create(
+            id=3,
+            repository=self.repo_try,
+            review_task_id="42",
+            phid="PHID-DIFF-42",
+        )
+        new_issue = Issue.objects.create(hash="4" * 32)
+        diff.issue_links.create(revision=self.revision_last, issue=new_issue)
+        diff.issue_links.create(revision=self.revision_last, issue=self.issues[3])
+        self.assertCountEqual(
+            self.client.get(f"/v2/diff/{diff.id}/issues/unresolved/").json()["results"],
+            [],
+        )
+        self.assertCountEqual(
+            self.client.get(f"/v2/diff/{diff.id}/issues/known/").json()["results"], []
+        )
+        self.assertCountEqual(
+            self.client.get(f"/v2/diff/{diff.id}/issues/closed/").json()["results"],
+            [
+                {"id": str(self.issues[0].id), "hash": self.issues[0].hash},
+                {"id": str(self.issues[2].id), "hash": self.issues[2].hash},
+            ],
+        )

From ff1520500b798b1dae4af1fd9a97599c89d53f8c Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Fri, 22 Nov 2024 17:09:42 +0100
Subject: [PATCH 04/15] Use a specific serializer

---
 backend/code_review_backend/issues/api_v2.py      |  6 +++---
 .../code_review_backend/issues/serializers_v2.py  | 15 +++++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)
 create mode 100644 backend/code_review_backend/issues/serializers_v2.py

diff --git a/backend/code_review_backend/issues/api_v2.py b/backend/code_review_backend/issues/api_v2.py
index bd955bfe5..bd6b37ca3 100644
--- a/backend/code_review_backend/issues/api_v2.py
+++ b/backend/code_review_backend/issues/api_v2.py
@@ -10,11 +10,11 @@
 from rest_framework.generics import ListAPIView
 
 from code_review_backend.issues.models import Diff, IssueLink
-from code_review_backend.issues.serializers import IssueHashSerializer
+from code_review_backend.issues.serializers_v2 import IssueLinkHashSerializer
 
 
 class IssueList(ListAPIView):
-    serializer_class = IssueHashSerializer
+    serializer_class = IssueLinkHashSerializer
     allowed_modes = ["unresolved", "known", "closed"]
 
     @cached_property
@@ -39,7 +39,7 @@ def distinct_issues(self, qs):
         Convert a list of issue links to unique couples of (issue_id, issue_hash)
         """
         attributes = ("issue_id", "issue__hash")
-        return qs.order_by(*attributes).values_list(*attributes).distinct()
+        return qs.order_by(*attributes).values(*attributes).distinct()
 
     def get_unresolved(self):
         """
diff --git a/backend/code_review_backend/issues/serializers_v2.py b/backend/code_review_backend/issues/serializers_v2.py
new file mode 100644
index 000000000..6252d65e1
--- /dev/null
+++ b/backend/code_review_backend/issues/serializers_v2.py
@@ -0,0 +1,15 @@
+from rest_framework import serializers
+
+from code_review_backend.issues.models import IssueLink
+
+
+class IssueLinkHashSerializer(serializers.ModelSerializer):
+    """Serialize an Issue hash from the IssueLink M2M"""
+
+    id = serializers.UUIDField(source="issue_id")
+    hash = serializers.CharField(source="issue__hash", max_length=32)
+
+    class Meta:
+        model = IssueLink
+        fields = ("id", "hash")
+        read_only_fields = ("id", "hash")

From f843d3d7d2e2db1995fabc2f8072b54e24e6322c Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Fri, 22 Nov 2024 17:40:44 +0100
Subject: [PATCH 05/15] Update endpoint

---
 backend/code_review_backend/issues/api_v2.py | 54 ++++++++++++--------
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/backend/code_review_backend/issues/api_v2.py b/backend/code_review_backend/issues/api_v2.py
index bd6b37ca3..bfe0d7ffb 100644
--- a/backend/code_review_backend/issues/api_v2.py
+++ b/backend/code_review_backend/issues/api_v2.py
@@ -2,7 +2,8 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-from django.db.models import Exists, OuterRef
+from django.conf import settings
+from django.db.models import Exists, OuterRef, Value
 from django.shortcuts import get_object_or_404
 from django.urls import path
 from django.utils.functional import cached_property
@@ -32,47 +33,60 @@ def mode(self):
         return mode
 
     def get_queryset(self):
-        return getattr(self, f"get_{self.mode}")()
+        return getattr(self, f"{self.mode}_issues").filter(**{self.mode: True})
 
     def distinct_issues(self, qs):
         """
         Convert a list of issue links to unique couples of (issue_id, issue_hash)
         """
         attributes = ("issue_id", "issue__hash")
+        if "postgresql" in settings.DATABASES["default"]["ENGINE"]:
+            return qs.order_by(*attributes).values(*attributes).distinct(*attributes)
         return qs.order_by(*attributes).values(*attributes).distinct()
 
-    def get_unresolved(self):
+    @property
+    def known_issues(self):
         """
-        Issues that were linked to a previous diff of the same
-        parent revision, and are still present on the current diff.
+        Issues that have also being detected from mozilla-central.
         """
         return self.distinct_issues(
-            self.diff.revision.issue_links.annotate(
-                unresolved=Exists(
-                    IssueLink.objects.exclude(diff=self.diff).filter(
-                        revision=self.diff.revision,
+            self.diff.issue_links.annotate(
+                known=Exists(
+                    IssueLink.objects.filter(
+                        revision__base_repository__slug="mozilla-central",
                         issue=OuterRef("issue"),
                     )
                 )
-            ).filter(unresolved=True)
+            )
         )
 
-    def get_known(self):
+    @property
+    def unresolved_issues(self):
         """
-        Issues that have also being detected from mozilla-central.
+        Issues that were linked to the previous diff of the same
+        parent revision, and are still present on the current diff.
+        Filters out issues that are known by the backend.
         """
+        previous_diff = (
+            self.diff.revision.diffs.filter(created__lt=self.diff.created)
+            .order_by("created")
+            .last()
+        )
+        if not previous_diff:
+            return IssueLink.objects.none().annotate(unresolved=Value("True"))
         return self.distinct_issues(
-            self.diff.issue_links.annotate(
-                known=Exists(
+            self.known_issues.filter(known=False).annotate(
+                unresolved=Exists(
                     IssueLink.objects.filter(
-                        revision__base_repository__slug="mozilla-central",
+                        diff=previous_diff,
                         issue=OuterRef("issue"),
                     )
                 )
-            ).filter(known=True)
+            )
         )
 
-    def get_closed(self):
+    @property
+    def closed_issues(self):
         """
         Issues that were listed on the previous diff of the same
         parent revision, but does not exist on the current diff.
@@ -84,16 +98,16 @@ def get_closed(self):
             .last()
         )
         if not previous_diff:
-            return []
+            return IssueLink.objects.none().annotate(closed=Value("True"))
         return self.distinct_issues(
             previous_diff.issue_links.annotate(
-                closed=Exists(
+                closed=~Exists(
                     IssueLink.objects.filter(
                         diff=self.diff,
                         issue=OuterRef("issue"),
                     )
                 )
-            ).filter(closed=True)
+            )
         )
 
 

From 816f7edf665d2689be79e02f43fe3a5d41a3c286 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Tue, 26 Nov 2024 12:56:55 +0100
Subject: [PATCH 06/15] Remove old endpoint from api_v1

---
 backend/code_review_backend/issues/api.py | 139 +++-------------------
 1 file changed, 16 insertions(+), 123 deletions(-)

diff --git a/backend/code_review_backend/issues/api.py b/backend/code_review_backend/issues/api.py
index 37cac4707..173d4b53f 100644
--- a/backend/code_review_backend/issues/api.py
+++ b/backend/code_review_backend/issues/api.py
@@ -6,20 +6,11 @@
 from datetime import date, datetime, timedelta
 
 from django.conf import settings
-from django.db.models import (
-    BooleanField,
-    Count,
-    Exists,
-    ExpressionWrapper,
-    OuterRef,
-    Prefetch,
-    Q,
-)
+from django.db.models import BooleanField, Count, ExpressionWrapper, Prefetch, Q
 from django.db.models.functions import TruncDate
 from django.shortcuts import get_object_or_404
 from django.urls import path
 from django.utils.decorators import method_decorator
-from django.utils.functional import cached_property
 from django.views.decorators.cache import cache_page
 from rest_framework import generics, mixins, routers, status, viewsets
 from rest_framework.exceptions import APIException, ValidationError
@@ -495,90 +486,6 @@ def get_queryset(self):
         return qs.filter(**filters).order_by("created").distinct()
 
 
-class GenericIssueList(generics.ListAPIView):
-    serializer_class = IssueHashSerializer
-    allowed_modes = ["unresolved", "known", "closed"]
-
-    @cached_property
-    def diff(self):
-        return get_object_or_404(
-            Diff.objects.select_related("revision"), id=self.kwargs["diff_id"]
-        )
-
-    @cached_property
-    def mode(self):
-        if (mode := self.kwargs["mode"]) not in self.allowed_modes:
-            raise APIException(
-                detail="mode argument must be one of {self.allowed_modes}"
-            )
-        return mode
-
-    def get_queryset(self):
-        return getattr(self, f"get_{self.mode}")()
-
-    def distinct_issues(self, qs):
-        """
-        Convert a list of issue links to unique couples of (issue_id, issue_hash)
-        """
-        attributes = ("issue_id", "issue__hash")
-        return qs.order_by(*attributes).values_list(*attributes).distinct()
-
-    def get_unresolved(self):
-        """
-        Issues that were linked to a previous diff of the same
-        parent revision, and are still present on the current diff.
-        """
-        return self.distinct_issues(
-            self.diff.revision.issue_links.annotate(
-                unresolved=Exists(
-                    IssueLink.objects.exclude(diff=self.diff).filter(
-                        revision=self.diff.revision,
-                        issue=OuterRef("issue"),
-                    )
-                )
-            ).filter(unresolved=True)
-        )
-
-    def get_known(self):
-        """
-        Issues that have also being detected from mozilla-central.
-        """
-        return self.distinct_issues(
-            self.diff.issue_links.annotate(
-                known=Exists(
-                    IssueLink.objects.filter(
-                        revision__base_repository__slug="mozilla-central",
-                        issue=OuterRef("issue"),
-                    )
-                )
-            ).filter(known=True)
-        )
-
-    def get_closed(self):
-        """
-        Issues that were listed on the previous diff of the same
-        parent revision, but does not exist on the current diff.
-        """
-        # Retrieve the previous diff
-        previous_diff = (
-            self.diff.revision.diffs.filter(created__lt=self.diff.created)
-            .order_by("created")
-            .last()
-        )
-        if not previous_diff:
-            return []
-        return self.distinct_issues(
-            previous_diff.issue_links.annotate(
-                closed=Exists(
-                    IssueLink.objects.filter(
-                        diff=self.diff,
-                        issue=OuterRef("issue"),
-                    )
-                )
-            ).filter(closed=True)
-        )
-
-
 # Build exposed urls for the API
 router = routers.DefaultRouter()
 router.register(r"repository", RepositoryViewSet)
@@ -590,32 +497,18 @@ def get_closed(self):
 )
 router.register(r"diff", DiffViewSet, basename="diffs")
 router.register(r"diff/(?P<diff_id>\d+)/issues", IssueViewSet, basename="issues")
-
-urls = (
-    [
-        # Prevails on the generic issues endpoint
-        path(
-            "diff/<int:diff_id>/issues/<str:mode>/",
-            GenericIssueList.as_view(),
-            name="issue-list",
-        ),
-    ]
-    + router.urls
-    + [
-        path(
-            "revision/<int:revision_id>/issues/",
-            IssueBulkCreate.as_view(),
-            name="revision-issues-bulk",
-        ),
-        path("check/stats/", IssueCheckStats.as_view(), name="issue-checks-stats"),
-        path(
-            "check/history/", IssueCheckHistory.as_view(), name="issue-checks-history"
-        ),
-        path(
-            "check/<str:repository>/<str:analyzer>/<path:check>/",
-            IssueCheckDetails.as_view(),
-            name="issue-check-details",
-        ),
-        path("issues/<slug:repo_slug>/", IssueList.as_view(), name="repository-issues"),
-    ]
-)
+urls = router.urls + [
+    path(
+        "revision/<int:revision_id>/issues/",
+        IssueBulkCreate.as_view(),
+        name="revision-issues-bulk",
+    ),
+    path("check/stats/", IssueCheckStats.as_view(), name="issue-checks-stats"),
+    path("check/history/", IssueCheckHistory.as_view(), name="issue-checks-history"),
+    path(
+        "check/<str:repository>/<str:analyzer>/<path:check>/",
+        IssueCheckDetails.as_view(),
+        name="issue-check-details",
+    ),
+    path("issues/<slug:repo_slug>/", IssueList.as_view(), name="repository-issues"),
+]

From 3b13119b6e143ff02f43741068109f3076cff4be Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Fri, 6 Dec 2024 16:19:20 +0100
Subject: [PATCH 07/15] Move everything to code_review_backend.issues.v2

---
 backend/code_review_backend/app/urls.py                        | 3 ++-
 backend/code_review_backend/issues/v2/__init__.py              | 0
 backend/code_review_backend/issues/{api_v2.py => v2/api.py}    | 2 +-
 .../issues/{serializers_v2.py => v2/serializers.py}            | 0
 backend/code_review_backend/issues/v2/tests/__init__.py        | 0
 .../{tests/test_api_v2.py => v2/tests/test_list_issues.py}     | 2 +-
 6 files changed, 4 insertions(+), 3 deletions(-)
 create mode 100644 backend/code_review_backend/issues/v2/__init__.py
 rename backend/code_review_backend/issues/{api_v2.py => v2/api.py} (98%)
 rename backend/code_review_backend/issues/{serializers_v2.py => v2/serializers.py} (100%)
 create mode 100644 backend/code_review_backend/issues/v2/tests/__init__.py
 rename backend/code_review_backend/issues/{tests/test_api_v2.py => v2/tests/test_list_issues.py} (99%)

diff --git a/backend/code_review_backend/app/urls.py b/backend/code_review_backend/app/urls.py
index e0e5ae5ef..b7862ab28 100644
--- a/backend/code_review_backend/app/urls.py
+++ b/backend/code_review_backend/app/urls.py
@@ -10,7 +10,8 @@
 from drf_yasg.views import get_schema_view
 from rest_framework import permissions
 
-from code_review_backend.issues import api, api_v2
+from code_review_backend.issues import api
+from code_review_backend.issues.v2 import api as api_v2
 
 # Build Swagger schema view
 schema_view = get_schema_view(
diff --git a/backend/code_review_backend/issues/v2/__init__.py b/backend/code_review_backend/issues/v2/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/backend/code_review_backend/issues/api_v2.py b/backend/code_review_backend/issues/v2/api.py
similarity index 98%
rename from backend/code_review_backend/issues/api_v2.py
rename to backend/code_review_backend/issues/v2/api.py
index bfe0d7ffb..e5a191a4e 100644
--- a/backend/code_review_backend/issues/api_v2.py
+++ b/backend/code_review_backend/issues/v2/api.py
@@ -11,7 +11,7 @@
 from rest_framework.generics import ListAPIView
 
 from code_review_backend.issues.models import Diff, IssueLink
-from code_review_backend.issues.serializers_v2 import IssueLinkHashSerializer
+from code_review_backend.issues.v2.serializers import IssueLinkHashSerializer
 
 
 class IssueList(ListAPIView):
diff --git a/backend/code_review_backend/issues/serializers_v2.py b/backend/code_review_backend/issues/v2/serializers.py
similarity index 100%
rename from backend/code_review_backend/issues/serializers_v2.py
rename to backend/code_review_backend/issues/v2/serializers.py
diff --git a/backend/code_review_backend/issues/v2/tests/__init__.py b/backend/code_review_backend/issues/v2/tests/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/backend/code_review_backend/issues/tests/test_api_v2.py b/backend/code_review_backend/issues/v2/tests/test_list_issues.py
similarity index 99%
rename from backend/code_review_backend/issues/tests/test_api_v2.py
rename to backend/code_review_backend/issues/v2/tests/test_list_issues.py
index 56a5738c6..54dd84064 100644
--- a/backend/code_review_backend/issues/tests/test_api_v2.py
+++ b/backend/code_review_backend/issues/v2/tests/test_list_issues.py
@@ -4,7 +4,7 @@
 from code_review_backend.issues.models import Issue, Repository
 
 
-class CreationAPIv2TestCase(APITestCase):
+class ListIssuesTestCase(APITestCase):
     def setUp(self):
         # Create the main repository
         self.repo_main = Repository.objects.create(

From 9e5f280ae91f1524fe95af5392b082a364aa381b Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Fri, 6 Dec 2024 16:30:07 +0100
Subject: [PATCH 08/15] Suggestions

---
 backend/code_review_backend/issues/v2/api.py  | 42 ++++++++-----------
 .../issues/v2/serializers.py                  |  5 +--
 2 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/backend/code_review_backend/issues/v2/api.py b/backend/code_review_backend/issues/v2/api.py
index e5a191a4e..6713f0cf0 100644
--- a/backend/code_review_backend/issues/v2/api.py
+++ b/backend/code_review_backend/issues/v2/api.py
@@ -2,8 +2,8 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-from django.conf import settings
-from django.db.models import Exists, OuterRef, Value
+from django.core.exceptions import BadRequest
+from django.db.models import Exists, OuterRef
 from django.shortcuts import get_object_or_404
 from django.urls import path
 from django.utils.functional import cached_property
@@ -24,6 +24,16 @@ def diff(self):
             Diff.objects.select_related("revision"), id=self.kwargs["diff_id"]
         )
 
+    @cached_property
+    def previous_diff(self):
+        previous_diff = (
+            self.diff.revision.diffs.filter(created__lt=self.diff.created)
+            .order_by("created")
+            .last()
+        )
+        if not previous_diff:
+            raise BadRequest("No previous diff was found to compare issues")
+
     @cached_property
     def mode(self):
         if (mode := self.kwargs["mode"]) not in self.allowed_modes:
@@ -40,20 +50,18 @@ def distinct_issues(self, qs):
         Convert a list of issue links to unique couples of (issue_id, issue_hash)
         """
         attributes = ("issue_id", "issue__hash")
-        if "postgresql" in settings.DATABASES["default"]["ENGINE"]:
-            return qs.order_by(*attributes).values(*attributes).distinct(*attributes)
-        return qs.order_by(*attributes).values(*attributes).distinct()
+        return qs.order_by(*attributes).values(*attributes).distinct(*attributes)
 
     @property
     def known_issues(self):
         """
-        Issues that have also being detected from mozilla-central.
+        Issues that have also being detected from the base repository (e.g. mozilla-central).
         """
         return self.distinct_issues(
             self.diff.issue_links.annotate(
                 known=Exists(
                     IssueLink.objects.filter(
-                        revision__base_repository__slug="mozilla-central",
+                        revision__base_repository_id=self.diff.revision.base_repository_id,
                         issue=OuterRef("issue"),
                     )
                 )
@@ -67,18 +75,11 @@ def unresolved_issues(self):
         parent revision, and are still present on the current diff.
         Filters out issues that are known by the backend.
         """
-        previous_diff = (
-            self.diff.revision.diffs.filter(created__lt=self.diff.created)
-            .order_by("created")
-            .last()
-        )
-        if not previous_diff:
-            return IssueLink.objects.none().annotate(unresolved=Value("True"))
         return self.distinct_issues(
             self.known_issues.filter(known=False).annotate(
                 unresolved=Exists(
                     IssueLink.objects.filter(
-                        diff=previous_diff,
+                        diff=self.previous_diff,
                         issue=OuterRef("issue"),
                     )
                 )
@@ -91,16 +92,8 @@ def closed_issues(self):
         Issues that were listed on the previous diff of the same
         parent revision, but does not exist on the current diff.
         """
-        # Retrieve the previous diff
-        previous_diff = (
-            self.diff.revision.diffs.filter(created__lt=self.diff.created)
-            .order_by("created")
-            .last()
-        )
-        if not previous_diff:
-            return IssueLink.objects.none().annotate(closed=Value("True"))
         return self.distinct_issues(
-            previous_diff.issue_links.annotate(
+            self.previous_diff.issue_links.annotate(
                 closed=~Exists(
                     IssueLink.objects.filter(
                         diff=self.diff,
@@ -112,7 +105,6 @@ def closed_issues(self):
 
 
 urls = [
-    # Prevails on the generic issues endpoint
     path(
         "diff/<int:diff_id>/issues/<str:mode>/",
         IssueList.as_view(),
diff --git a/backend/code_review_backend/issues/v2/serializers.py b/backend/code_review_backend/issues/v2/serializers.py
index 6252d65e1..e5088fa30 100644
--- a/backend/code_review_backend/issues/v2/serializers.py
+++ b/backend/code_review_backend/issues/v2/serializers.py
@@ -6,10 +6,9 @@
 class IssueLinkHashSerializer(serializers.ModelSerializer):
     """Serialize an Issue hash from the IssueLink M2M"""
 
-    id = serializers.UUIDField(source="issue_id")
-    hash = serializers.CharField(source="issue__hash", max_length=32)
+    id = serializers.UUIDField(source="issue_id", read_only=True)
+    hash = serializers.CharField(source="issue__hash", max_length=32, read_only=True)
 
     class Meta:
         model = IssueLink
         fields = ("id", "hash")
-        read_only_fields = ("id", "hash")

From 6b7943a8381bd1632cb2aa857cf7b9c3502db6f9 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Fri, 6 Dec 2024 16:51:22 +0100
Subject: [PATCH 09/15] Serialize the previous diff ID

---
 backend/code_review_backend/issues/v2/api.py  | 48 ++++++++++++-------
 .../issues/v2/serializers.py                  |  5 ++
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/backend/code_review_backend/issues/v2/api.py b/backend/code_review_backend/issues/v2/api.py
index 6713f0cf0..77cda0d67 100644
--- a/backend/code_review_backend/issues/v2/api.py
+++ b/backend/code_review_backend/issues/v2/api.py
@@ -11,12 +11,33 @@
 from rest_framework.generics import ListAPIView
 
 from code_review_backend.issues.models import Diff, IssueLink
-from code_review_backend.issues.v2.serializers import IssueLinkHashSerializer
+from code_review_backend.issues.v2.serializers import DiffIssuesSerializer
 
 
 class IssueList(ListAPIView):
-    serializer_class = IssueLinkHashSerializer
+    """
+    Returns issues that are linked to a diff, and depending on the selected mode:
+    * known: Issues that have also being detected from the base repository (e.g. mozilla-central).
+        `previous_diff_id` is always returned as null when listing known issues.
+    * unresolved: Issues that were linked to the previous diff of the same parent revision, and are
+        still present on the current diff. Filters out issues that are known by the backend.
+    * closed: Issues that were listed on the previous diff of the same parent revision, but does
+        not exist on the current diff.
+
+    This endpoint does not uses pagination.
+    """
+
     allowed_modes = ["unresolved", "known", "closed"]
+    pagination_class = None
+
+    def get_serializer(self, queryset, many=True):
+        return DiffIssuesSerializer(
+            {
+                "previous_diff_id": self.previous_diff and self.previous_diff.id,
+                "issues": queryset,
+            },
+            context=self.get_serializer_context(),
+        )
 
     @cached_property
     def diff(self):
@@ -26,13 +47,14 @@ def diff(self):
 
     @cached_property
     def previous_diff(self):
-        previous_diff = (
+        if self.mode == "known":
+            # No need to search for a previous diff for known issues
+            return None
+        return (
             self.diff.revision.diffs.filter(created__lt=self.diff.created)
             .order_by("created")
             .last()
         )
-        if not previous_diff:
-            raise BadRequest("No previous diff was found to compare issues")
 
     @cached_property
     def mode(self):
@@ -54,9 +76,6 @@ def distinct_issues(self, qs):
 
     @property
     def known_issues(self):
-        """
-        Issues that have also being detected from the base repository (e.g. mozilla-central).
-        """
         return self.distinct_issues(
             self.diff.issue_links.annotate(
                 known=Exists(
@@ -70,11 +89,8 @@ def known_issues(self):
 
     @property
     def unresolved_issues(self):
-        """
-        Issues that were linked to the previous diff of the same
-        parent revision, and are still present on the current diff.
-        Filters out issues that are known by the backend.
-        """
+        if not self.previous_diff:
+            raise BadRequest("No previous diff was found to compare issues")
         return self.distinct_issues(
             self.known_issues.filter(known=False).annotate(
                 unresolved=Exists(
@@ -88,10 +104,8 @@ def unresolved_issues(self):
 
     @property
     def closed_issues(self):
-        """
-        Issues that were listed on the previous diff of the same
-        parent revision, but does not exist on the current diff.
-        """
+        if not self.previous_diff:
+            raise BadRequest("No previous diff was found to compare issues")
         return self.distinct_issues(
             self.previous_diff.issue_links.annotate(
                 closed=~Exists(
diff --git a/backend/code_review_backend/issues/v2/serializers.py b/backend/code_review_backend/issues/v2/serializers.py
index e5088fa30..c384c4efa 100644
--- a/backend/code_review_backend/issues/v2/serializers.py
+++ b/backend/code_review_backend/issues/v2/serializers.py
@@ -12,3 +12,8 @@ class IssueLinkHashSerializer(serializers.ModelSerializer):
     class Meta:
         model = IssueLink
         fields = ("id", "hash")
+
+
+class DiffIssuesSerializer(serializers.Serializer):
+    previous_diff_id = serializers.UUIDField(allow_null=True, read_only=True)
+    issues = IssueLinkHashSerializer(many=True)

From c935df5900b5833a32ac845b1df975db2019aa9d Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Fri, 6 Dec 2024 17:55:48 +0100
Subject: [PATCH 10/15] Compare head_repository to detect known issues

---
 backend/code_review_backend/issues/v2/api.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/backend/code_review_backend/issues/v2/api.py b/backend/code_review_backend/issues/v2/api.py
index 77cda0d67..8cee8f27c 100644
--- a/backend/code_review_backend/issues/v2/api.py
+++ b/backend/code_review_backend/issues/v2/api.py
@@ -80,7 +80,7 @@ def known_issues(self):
             self.diff.issue_links.annotate(
                 known=Exists(
                     IssueLink.objects.filter(
-                        revision__base_repository_id=self.diff.revision.base_repository_id,
+                        revision__head_repository_id=self.diff.revision.base_repository_id,
                         issue=OuterRef("issue"),
                     )
                 )

From 700f7090ec3355bb443bab11151a0e6266ceefe2 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Fri, 6 Dec 2024 17:56:36 +0100
Subject: [PATCH 11/15] Update tests

---
 .../issues/v2/tests/test_list_issues.py       | 92 ++++++++++++-------
 1 file changed, 58 insertions(+), 34 deletions(-)

diff --git a/backend/code_review_backend/issues/v2/tests/test_list_issues.py b/backend/code_review_backend/issues/v2/tests/test_list_issues.py
index 54dd84064..fc0939268 100644
--- a/backend/code_review_backend/issues/v2/tests/test_list_issues.py
+++ b/backend/code_review_backend/issues/v2/tests/test_list_issues.py
@@ -32,6 +32,7 @@ def setUp(self):
             phabricator_phid="PHID-REV-XXX",
             title="Main revision",
             base_repository=self.repo_main,
+            head_repository=self.repo_main,
         )
         self.revision_main.issue_links.create(issue=self.issues[0])
         self.revision_main.issue_links.create(issue=self.issues[1])
@@ -41,7 +42,8 @@ def setUp(self):
             phabricator_id=1337,
             phabricator_phid="PHID-REV-YYY",
             title="Bug XXX - Yet another bug",
-            base_repository=self.repo_try,
+            base_repository=self.repo_main,
+            head_repository=self.repo_try,
         )
         self.diff1 = self.revision_last.diffs.create(
             id=1,
@@ -79,46 +81,58 @@ def test_list_issues_invalid_diff(self):
             self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     def test_list_issues_known(self):
-        with self.assertNumQueries(3):
+        with self.assertNumQueries(2):
             response = self.client.get(f"/v2/diff/{self.diff2.id}/issues/known/")
             self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertCountEqual(
-            response.json()["results"],
-            [{"id": str(self.issues[0].id), "hash": self.issues[0].hash}],
+        self.maxDiff = None
+        self.assertDictEqual(
+            response.json(),
+            {
+                "previous_diff_id": None,
+                "issues": [
+                    {"id": str(self.issues[0].id), "hash": self.issues[0].hash},
+                ],
+            },
         )
 
     def test_list_issues_unresolved_first_diff(self):
         """No issue can be marked as unresolved on a new diff"""
         with self.assertNumQueries(2):
-            response = self.client.get(f"/v2/diff/{self.diff1.id}/issues/unresolved/")
-            self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertCountEqual(
-            response.json()["results"],
-            [],
-        )
+            response = self.client.get(
+                f"/v2/diff/{self.diff1.id}/issues/unresolved/", format="json"
+            )
+            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
 
     def test_list_issues_unresolved(self):
-        with self.assertNumQueries(4):
+        with self.assertNumQueries(3):
             response = self.client.get(f"/v2/diff/{self.diff2.id}/issues/unresolved/")
             self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertCountEqual(
-            response.json()["results"],
-            [{"id": str(self.issues[2].id), "hash": self.issues[2].hash}],
+        self.assertDictEqual(
+            response.json(),
+            {
+                "previous_diff_id": "1",
+                "issues": [
+                    {"id": str(self.issues[2].id), "hash": self.issues[2].hash},
+                ],
+            },
         )
 
     def test_list_issues_closed_first_diff(self):
+        """No issue can be marked as closed on a new diff"""
         with self.assertNumQueries(2):
             response = self.client.get(f"/v2/diff/{self.diff1.id}/issues/closed/")
-            self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertCountEqual(response.json()["results"], [])
+            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
 
     def test_list_issues_closed(self):
-        with self.assertNumQueries(4):
+        with self.assertNumQueries(3):
             response = self.client.get(f"/v2/diff/{self.diff2.id}/issues/closed/")
             self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertListEqual(
-            response.json()["results"],
-            [{"id": str(self.issues[3].id), "hash": self.issues[3].hash}],
+        self.assertDictEqual(
+            response.json(),
+            {
+                "previous_diff_id": "1",
+                "issues": [{"id": str(self.issues[3].id), "hash": self.issues[3].hash}],
+            },
         )
 
     def test_list_issues_new_diff_reopen(self):
@@ -135,17 +149,27 @@ def test_list_issues_new_diff_reopen(self):
         new_issue = Issue.objects.create(hash="4" * 32)
         diff.issue_links.create(revision=self.revision_last, issue=new_issue)
         diff.issue_links.create(revision=self.revision_last, issue=self.issues[3])
-        self.assertCountEqual(
-            self.client.get(f"/v2/diff/{diff.id}/issues/unresolved/").json()["results"],
-            [],
-        )
-        self.assertCountEqual(
-            self.client.get(f"/v2/diff/{diff.id}/issues/known/").json()["results"], []
-        )
-        self.assertCountEqual(
-            self.client.get(f"/v2/diff/{diff.id}/issues/closed/").json()["results"],
-            [
-                {"id": str(self.issues[0].id), "hash": self.issues[0].hash},
-                {"id": str(self.issues[2].id), "hash": self.issues[2].hash},
-            ],
+        self.assertDictEqual(
+            self.client.get(f"/v2/diff/{diff.id}/issues/unresolved/").json(),
+            {
+                "previous_diff_id": "2",
+                "issues": [],
+            },
+        )
+        self.assertDictEqual(
+            self.client.get(f"/v2/diff/{diff.id}/issues/known/").json(),
+            {
+                "previous_diff_id": None,
+                "issues": [],
+            },
+        )
+        self.assertDictEqual(
+            self.client.get(f"/v2/diff/{diff.id}/issues/closed/").json(),
+            {
+                "previous_diff_id": "2",
+                "issues": [
+                    {"id": str(self.issues[0].id), "hash": self.issues[0].hash},
+                    {"id": str(self.issues[2].id), "hash": self.issues[2].hash},
+                ],
+            },
         )

From 887e1051b27ebcf42001e0d79effc5142847b651 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Thu, 28 Nov 2024 17:31:02 +0100
Subject: [PATCH 12/15] Use backend API v2 to detect known/unresolved/closed
 issues

---
 bot/code_review_bot/backend.py            |  7 ++
 bot/code_review_bot/report/phabricator.py | 90 +++++------------------
 bot/code_review_bot/workflow.py           | 56 +++-----------
 3 files changed, 33 insertions(+), 120 deletions(-)

diff --git a/bot/code_review_bot/backend.py b/bot/code_review_bot/backend.py
index 17019b5b3..8e17f7781 100644
--- a/bot/code_review_bot/backend.py
+++ b/bot/code_review_bot/backend.py
@@ -187,6 +187,13 @@ def list_diff_issues(self, diff_id):
         """
         return list(self.paginate(f"/v1/diff/{diff_id}/issues/"))
 
+    def list_diff_issues_v2(self, diff_id, mode):
+        """
+        List issues for a given dif
+        """
+        assert mode in ("known", "unresolved", "closed")
+        return list(self.paginate(f"/v2/diff/{diff_id}/issues/{mode}"))
+
     def paginate(self, url_path):
         """
         Yield results from a paginated API one by one
diff --git a/bot/code_review_bot/report/phabricator.py b/bot/code_review_bot/report/phabricator.py
index c46dc5486..9781c423f 100644
--- a/bot/code_review_bot/report/phabricator.py
+++ b/bot/code_review_bot/report/phabricator.py
@@ -2,7 +2,6 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-from collections import defaultdict
 from typing import List
 from urllib.parse import urljoin
 
@@ -30,7 +29,7 @@
 IMPORTANT: Found {nb_errors} (error level) that must be fixed before landing.
 """
 
-COMMENT_DIFF_FOLLOWUP = """compared to the previous diff [{diff_id}]({phabricator_diff_url}).
+COMMENT_DIFF_FOLLOWUP = """compared to the previous diff.
 """
 
 COMMENT_RUN_ANALYZERS = """
@@ -110,53 +109,6 @@ def pluralize(word, nb):
         assert isinstance(nb, int)
         return "{} {}".format(nb, nb == 1 and word or word + "s")
 
-    def compare_issues(self, former_diff_id, issues):
-        """
-        Compare new issues depending on their evolution from the
-        previous diff on the same revision.
-        Returns a tuple containing lists of:
-          * Unresolved issues, that are present on both diffs
-          * Closed issues, that were present in the previous diff and are now gone
-        """
-        if not self.backend_api.enabled:
-            logger.warning(
-                "Backend API must be enabled to compare issues with previous diff {former_diff_id}."
-            )
-            return [], []
-
-        # If this is the first diff, there's no need to compare issues with a
-        # previous diff since there is no previous diff.
-        if former_diff_id is None:
-            return [], []
-
-        # Retrieve issues related to the previous diff
-        try:
-            previous_issues = self.backend_api.list_diff_issues(former_diff_id)
-        except Exception as e:
-            logger.warning(
-                f"An error occurred listing issues on previous diff {former_diff_id}: {e}. "
-                "Each issue will be considered as a new case."
-            )
-            previous_issues = []
-
-        # Multiple issues may share a similar hash in case they were
-        # produced by the same linter on the same lines
-        indexed_issues = defaultdict(list)
-        for issue in previous_issues:
-            indexed_issues[issue["hash"]].append(issue)
-
-        # Compare current issues with the previous ones based on the hash
-        unresolved = [
-            issue.on_backend["hash"]
-            for issue in issues
-            if issue.on_backend and issue.on_backend["hash"] in indexed_issues
-        ]
-
-        # All previous issues that are not unresolved are closed
-        closed = [issue for issue in previous_issues if issue["hash"] not in unresolved]
-
-        return unresolved, closed
-
     def publish(self, issues, revision, task_failures, notices, reviewers):
         """
         Publish issues on Phabricator:
@@ -215,18 +167,20 @@ def publish(self, issues, revision, task_failures, notices, reviewers):
             )
             return publishable_issues, patches
 
-        # Compare issues that are not known on the repository to a previous diff
-        older_diff_ids = [
-            diff["id"] for diff in rev_diffs if diff["id"] < revision.diff_id
-        ]
-        former_diff_id = sorted(older_diff_ids)[-1] if older_diff_ids else None
-        unresolved_issues, closed_issues = self.compare_issues(
-            former_diff_id, publishable_issues
-        )
+        if not self.backend_api.enabled:
+            logger.warning(
+                "Backend API must be enabled to compare issues with the previous diff."
+            )
+            unresolved, closed = [], []
+        else:
+            unresolved = self.backend_api.list_diff_issues_v2(
+                revision.diff_id, "unresolved"
+            )
+            closed = self.backend_api.list_diff_issues_v2(revision.diff_id, "closed")
 
         if (
-            len(unresolved_issues) == len(publishable_issues)
-            and not closed_issues
+            len(unresolved) == len(publishable_issues)
+            and not closed
             and not task_failures
             and not notices
         ):
@@ -234,7 +188,7 @@ def publish(self, issues, revision, task_failures, notices, reviewers):
             logger.info(
                 "No new issues nor failures/notices were detected. "
                 "Skipping comment publication (some issues are unresolved)",
-                unresolved_count=len(unresolved_issues),
+                unresolved_count=len(unresolved),
             )
             return publishable_issues, patches
 
@@ -245,9 +199,8 @@ def publish(self, issues, revision, task_failures, notices, reviewers):
             patches,
             task_failures,
             notices,
-            former_diff_id=former_diff_id,
-            unresolved_count=len(unresolved_issues),
-            closed_count=len(closed_issues),
+            unresolved_count=len(unresolved),
+            closed_count=len(closed),
         )
 
         # Publish statistics
@@ -284,7 +237,6 @@ def publish_summary(
         patches,
         task_failures,
         notices,
-        former_diff_id,
         unresolved_count,
         closed_count,
     ):
@@ -300,7 +252,6 @@ def publish_summary(
                 bug_report_url=BUG_REPORT_URL,
                 task_failures=task_failures,
                 notices=notices,
-                former_diff_id=former_diff_id,
                 unresolved=unresolved_count,
                 closed=closed_count,
             ),
@@ -315,7 +266,6 @@ def build_comment(
         notices,
         patches=[],
         task_failures=[],
-        former_diff_id=None,
         unresolved=0,
         closed=0,
     ):
@@ -367,13 +317,7 @@ def build_comment(
                     followup_comment += "and "
             if closed:
                 followup_comment += self.pluralize("defect", closed) + " closed "
-            followup_comment += COMMENT_DIFF_FOLLOWUP.format(
-                phabricator_diff_url=self.phabricator_diff_url.format(
-                    diff_id=former_diff_id
-                ),
-                diff_id=former_diff_id,
-            )
-            comment += followup_comment
+            comment += COMMENT_DIFF_FOLLOWUP
 
         # Add colored warning section
         total_warnings = sum(1 for i in issues if i.level == Level.Warning)
diff --git a/bot/code_review_bot/workflow.py b/bot/code_review_bot/workflow.py
index 48e19e4df..ddc675e2f 100644
--- a/bot/code_review_bot/workflow.py
+++ b/bot/code_review_bot/workflow.py
@@ -3,7 +3,6 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from datetime import datetime, timedelta
-from itertools import groupby
 
 import structlog
 from libmozdata.phabricator import BuildState, PhabricatorAPI
@@ -99,24 +98,8 @@ def run(self, revision):
         # Analyze issues in case the before/after feature is enabled
         if revision.before_after_feature:
             logger.info("Running the before/after feature")
-            # Search a base revision from the decision task
-            decision = self.queue_service.task(settings.try_group_id)
-            base_rev_changeset = (
-                decision.get("payload", {}).get("env", {}).get("GECKO_BASE_REV")
-            )
-            if not base_rev_changeset:
-                logger.warning(
-                    "Base revision changeset could not be fetched from Phabricator, "
-                    "looking for existing issues based on the current date",
-                    task=settings.try_group_id,
-                )
-
-            # Clone local repo when required
-            # as find_previous_issues will build the hashes
-            self.clone_repository(revision)
-
             # Mark know issues to avoid publishing them on this patch
-            self.find_previous_issues(issues, base_rev_changeset)
+            self.find_previous_issues(revision.diff_id, issues)
             new_issues_count = sum(issue.new_issue for issue in issues)
             logger.info(
                 f"Found {new_issues_count} new issues (over {len(issues)} total detected issues)",
@@ -348,39 +331,18 @@ def index(self, revision, **kwargs):
                 },
             )
 
-    def find_previous_issues(self, issues, base_rev_changeset=None):
-        """
-        Look for known issues in the backend matching the given list of issues
-
-        If a base revision ID is provided, compare to issues detected on this revision
-        Otherwise, compare to issues detected on last ingested revision
-        """
+    def find_previous_issues(self, diff_id, issues):
+        """Look for known issues in the backend"""
         assert (
             self.backend_api.enabled
         ), "Backend storage is disabled, comparing issues is not possible"
 
-        current_date = datetime.now().strftime("%Y-%m-%d")
-
-        # Group issues by path, so we only list know issues for the affected files
-        issues_groups = groupby(
-            sorted(issues, key=lambda i: i.path),
-            lambda i: i.path,
-        )
-        logger.info(
-            "Checking for existing issues in the backend",
-            base_revision_changeset=base_rev_changeset,
-        )
-
-        for path, group_issues in issues_groups:
-            known_issues = self.backend_api.list_repo_issues(
-                "mozilla-central",
-                date=current_date,
-                revision_changeset=base_rev_changeset,
-                path=path,
-            )
-            hashes = [issue["hash"] for issue in known_issues]
-            for issue in group_issues:
-                issue.new_issue = bool(issue.hash and issue.hash not in hashes)
+        known_hashes = [
+            issue["hash"]
+            for issue in self.backend_api.list_diff_issues_v2(diff_id, "known")
+        ]
+        for issue in issues:
+            issue.new_issue = bool(issue.hash and issue.hash not in known_hashes)
 
     def find_issues(self, revision, group_id):
         """

From 71deba3158caada89ae40abdb1ce67b9b2d5c9f8 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Mon, 2 Dec 2024 14:46:16 +0100
Subject: [PATCH 13/15] TRASHME: Mock HarborMaster and Phabricator

---
 bot/code_review_bot/report/phabricator.py | 70 +++++++++++++----------
 bot/code_review_bot/workflow.py           | 16 +++---
 2 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/bot/code_review_bot/report/phabricator.py b/bot/code_review_bot/report/phabricator.py
index 9781c423f..3c1b2f8b1 100644
--- a/bot/code_review_bot/report/phabricator.py
+++ b/bot/code_review_bot/report/phabricator.py
@@ -154,9 +154,9 @@ def publish(self, issues, revision, task_failures, notices, reviewers):
             if patch.analyzer.name not in self.analyzers_skipped
         ]
 
-        if publishable_issues:
-            # Publish detected patch's issues on Harbormaster, all at once, as lint issues
-            self.publish_harbormaster(revision, publishable_issues)
+        #if publishable_issues:
+        #    # Publish detected patch's issues on Harbormaster, all at once, as lint issues
+        #    self.publish_harbormaster(revision, publishable_issues)
 
         # Retrieve all diffs for the current revision
         rev_diffs = self.api.search_diffs(revision_phid=revision.phabricator_phid)
@@ -216,19 +216,19 @@ def publish_harbormaster(
         Publish issues through HarborMaster
         either as lint results or unit tests results
         """
-        assert lint_issues or unit_issues, "No issues to publish"
-
-        self.api.update_build_target(
-            revision.build_target_phid,
-            state=BuildState.Work,
-            lint=[issue.as_phabricator_lint() for issue in lint_issues],
-            unit=[issue.as_phabricator_unitresult() for issue in unit_issues],
-        )
-        logger.info(
-            "Updated Harbormaster build state with issues",
-            nb_lint=len(lint_issues),
-            nb_unit=len(unit_issues),
-        )
+        #assert lint_issues or unit_issues, "No issues to publish"
+
+        #self.api.update_build_target(
+        #    revision.build_target_phid,
+        #    state=BuildState.Work,
+        #    lint=[issue.as_phabricator_lint() for issue in lint_issues],
+        #    unit=[issue.as_phabricator_unitresult() for issue in unit_issues],
+        #)
+        #logger.info(
+        #    "Updated Harbormaster build state with issues",
+        #    nb_lint=len(lint_issues),
+        #    nb_unit=len(unit_issues),
+        #)
 
     def publish_summary(
         self,
@@ -243,20 +243,30 @@ def publish_summary(
         """
         Summarize publishable issues through Phabricator comment
         """
-        self.api.comment(
-            revision.phabricator_id,
-            self.build_comment(
-                revision=revision,
-                issues=issues,
-                patches=patches,
-                bug_report_url=BUG_REPORT_URL,
-                task_failures=task_failures,
-                notices=notices,
-                unresolved=unresolved_count,
-                closed=closed_count,
-            ),
-        )
-        logger.info("Published phabricator summary")
+        raise Exception(self.build_comment(
+            revision=revision,
+            issues=issues,
+            patches=patches,
+            bug_report_url=BUG_REPORT_URL,
+            task_failures=task_failures,
+            notices=notices,
+            unresolved=unresolved_count,
+            closed=closed_count,
+        ))
+        #self.api.comment(
+        #    revision.phabricator_id,
+        #    self.build_comment(
+        #        revision=revision,
+        #        issues=issues,
+        #        patches=patches,
+        #        bug_report_url=BUG_REPORT_URL,
+        #        task_failures=task_failures,
+        #        notices=notices,
+        #        unresolved=unresolved_count,
+        #        closed=closed_count,
+        #    ),
+        #)
+        #logger.info("Published phabricator summary")
 
     def build_comment(
         self,
diff --git a/bot/code_review_bot/workflow.py b/bot/code_review_bot/workflow.py
index ddc675e2f..f2d4252c4 100644
--- a/bot/code_review_bot/workflow.py
+++ b/bot/code_review_bot/workflow.py
@@ -267,12 +267,12 @@ def publish(self, revision, issues, task_failures, notices, reviewers):
         )
 
         # Publish final HarborMaster state
-        self.update_status(
-            revision,
-            BuildState.Fail
-            if nb_publishable_errors > 0 or task_failures
-            else BuildState.Pass,
-        )
+        #self.update_status(
+        #    revision,
+        #    BuildState.Fail
+        #    if nb_publishable_errors > 0 or task_failures
+        #    else BuildState.Pass,
+        #)
 
     def index(self, revision, **kwargs):
         """
@@ -513,5 +513,5 @@ def update_status(self, revision, state):
             )
             return
 
-        self.phabricator.update_build_target(revision.build_target_phid, state)
-        logger.info("Updated HarborMaster status", state=state, revision=revision)
+        #self.phabricator.update_build_target(revision.build_target_phid, state)
+        #logger.info("Updated HarborMaster status", state=state, revision=revision)

From b596b995b989e01b7538f341d289e548e233e201 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Mon, 2 Dec 2024 16:30:07 +0100
Subject: [PATCH 14/15] Handle missing diff from the backend

---
 bot/code_review_bot/backend.py  | 12 +++++++++++-
 bot/code_review_bot/workflow.py |  6 ++----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/bot/code_review_bot/backend.py b/bot/code_review_bot/backend.py
index 8e17f7781..f898003b4 100644
--- a/bot/code_review_bot/backend.py
+++ b/bot/code_review_bot/backend.py
@@ -6,6 +6,7 @@
 
 import requests
 import structlog
+from requests.exceptions import HTTPError
 
 from code_review_bot import taskcluster
 from code_review_bot.config import GetAppUserAgent, settings
@@ -192,7 +193,16 @@ def list_diff_issues_v2(self, diff_id, mode):
         List issues for a given dif
         """
         assert mode in ("known", "unresolved", "closed")
-        return list(self.paginate(f"/v2/diff/{diff_id}/issues/{mode}"))
+        try:
+            return list(self.paginate(f"/v2/diff/{diff_id}/issues/{mode}"))
+        except HTTPError as e:
+            if e.response.status_code != 404:
+                logger.warning(
+                    f"Cound not list {mode} issues from the bot: {e}. Skipping"
+                )
+            else:
+                logger.info(f"Diff not found in code review backend: {diff_id}")
+            return []
 
     def paginate(self, url_path):
         """
diff --git a/bot/code_review_bot/workflow.py b/bot/code_review_bot/workflow.py
index f2d4252c4..f953732bf 100644
--- a/bot/code_review_bot/workflow.py
+++ b/bot/code_review_bot/workflow.py
@@ -337,10 +337,8 @@ def find_previous_issues(self, diff_id, issues):
             self.backend_api.enabled
         ), "Backend storage is disabled, comparing issues is not possible"
 
-        known_hashes = [
-            issue["hash"]
-            for issue in self.backend_api.list_diff_issues_v2(diff_id, "known")
-        ]
+        known_issues = self.backend_api.list_diff_issues_v2(diff_id, "known")
+        known_hashes = [issue["hash"] for issue in known_issues]
         for issue in issues:
             issue.new_issue = bool(issue.hash and issue.hash not in known_hashes)
 

From 08e596f9d700c9980688a644ba508601cc3ab594 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Mon, 9 Dec 2024 17:26:15 +0100
Subject: [PATCH 15/15] Rebase

---
 bot/code_review_bot/backend.py | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/bot/code_review_bot/backend.py b/bot/code_review_bot/backend.py
index f898003b4..86ef2e2aa 100644
--- a/bot/code_review_bot/backend.py
+++ b/bot/code_review_bot/backend.py
@@ -190,19 +190,29 @@ def list_diff_issues(self, diff_id):
 
     def list_diff_issues_v2(self, diff_id, mode):
         """
-        List issues for a given dif
+        List issues for a given diff, applying specific filters.
+        Returns:
+          * The ID of the previous diff if it exists (always null for `known` mode).
+          * A list of issues (dict serializing ID and hash) corresponding to the filter.
         """
         assert mode in ("known", "unresolved", "closed")
         try:
-            return list(self.paginate(f"/v2/diff/{diff_id}/issues/{mode}"))
+            data = self.get(f"/v2/diff/{diff_id}/issues/{mode}")
         except HTTPError as e:
             if e.response.status_code != 404:
                 logger.warning(
-                    f"Cound not list {mode} issues from the bot: {e}. Skipping"
+                    f"Could not list {mode} issues from the bot: {e}. Skipping"
                 )
             else:
                 logger.info(f"Diff not found in code review backend: {diff_id}")
-            return []
+            return None, []
+        return data["previous_diff_id"], data["issues"]
+
+    def get(self, url):
+        auth = (self.username, self.password)
+        resp = requests.get(url, auth=auth, headers=GetAppUserAgent())
+        resp.raise_for_status()
+        return resp.json()
 
     def paginate(self, url_path):
         """