Use null_distincts for IssueLink constraints

.taskcluster.yml

@@ -202,7 +202,10 @@ tasks:
                   - "git clone --quiet ${repository} /src && cd /src && git checkout ${head_rev} -b checks &&
                     cd /src/tools && ${pip_install} . &&
                     cd /src/backend && ${pip_install} . && ${pip_install} -r requirements-dev.txt &&
+                    ./ci/setup_postgres.sh &&
                     ./manage.py test && ./manage.py makemigrations --check --noinput --dry-run -v 3"
+                env:
+                  DATABASE_URL: postgres://[email protected]/code-review
               metadata:
                 name: "Code Review Backend checks: unit tests"
                 description: Check python code with Django tests

backend/ci/pg_hba.conf

@@ -0,0 +1,5 @@
+# Database administrative login by Unix domain socket
+local   all             postgres                                peer
+
+# Only tester user can connect locally, without password
+host		all		tester		127.0.0.1/32		trust

backend/ci/setup_postgres.sh

@@ -0,0 +1,19 @@
+#!/bin/bash -ex
+export PG_VERSION=15
+
+# Silent apt
+export DEBIAN_FRONTEND=noninteractive
+
+# Install postgresql
+apt update
+apt install -qq -y postgresql-$PG_VERSION
+
+# Setup access rights
+cp $(dirname $0)/pg_hba.conf /etc/postgresql/$PG_VERSION/main/pg_hba.conf
+
+# Start postgresql
+pg_ctlcluster $PG_VERSION main start
+
+# Create user & database
+su postgres -c 'createuser --createdb tester'
+su postgres -c 'createdb --owner=tester code-review'

backend/code_review_backend/issues/migrations/0015_unique_issue_link_positions.py

@@ -0,0 +1,28 @@
+# Generated by Django 5.1.1 on 2024-11-25 14:35
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("issues", "0014_unique_hash"),
+    ]
+
+    operations = [
+        migrations.RemoveConstraint(
+            model_name="issuelink",
+            name="issue_link_unique_revision",
+        ),
+        migrations.RemoveConstraint(
+            model_name="issuelink",
+            name="issue_link_unique_diff",
+        ),
+        migrations.AddConstraint(
+            model_name="issuelink",
+            constraint=models.UniqueConstraint(
+                fields=("issue", "revision", "diff", "line", "nb_lines", "char"),
+                name="issue_link_unique",
+                nulls_distinct=False,
+            ),
+        ),
+    ]

backend/code_review_backend/issues/models.py

@@ -171,16 +171,10 @@ class IssueLink(models.Model):
 
     class Meta:
         constraints = [
-            # Two constraints are required as Null values are not compared for unicity
-            models.UniqueConstraint(
-                fields=["issue", "revision", "line", "nb_lines", "char"],
-                name="issue_link_unique_revision",
-                condition=Q(diff__isnull=True),
-            ),
             models.UniqueConstraint(
                 fields=["issue", "revision", "diff", "line", "nb_lines", "char"],
-                name="issue_link_unique_diff",
-                condition=Q(diff__isnull=False),
+                name="issue_link_unique",
+                nulls_distinct=False,
             ),
         ]
 

backend/code_review_backend/issues/tests/test_issue.py

@@ -5,6 +5,7 @@
 from datetime import datetime
 from unittest.mock import patch
 
+from django.db.utils import IntegrityError
 from django.test import TestCase
 from django.urls import reverse
 from rest_framework import status
@@ -211,3 +212,63 @@ def test_list_repository_issues_date_no_match(self):
         data = response.json()
         self.assertEqual(data["count"], 0)
         self.assertEqual(data["results"], [])
+
+    def test_issue_link_unicity_all_values(self):
+        """
+        Check that the unique constraints are respected when all positioning values are set
+        """
+        self.revision.issue_links.create(
+            issue=self.err_issue, line=20, nb_lines=2, char=1
+        )
+        with self.assertRaises(IntegrityError):
+            self.revision.issue_links.create(
+                issue=self.err_issue, line=20, nb_lines=2, char=1
+            )
+
+    def test_issue_link_unicity_no_line(self):
+        """
+        Check that the unique constraints are respected when line is not set
+        """
+        self.revision.issue_links.create(
+            issue=self.err_issue, line=None, nb_lines=2, char=1
+        )
+        with self.assertRaises(IntegrityError):
+            self.revision.issue_links.create(
+                issue=self.err_issue, line=None, nb_lines=2, char=1
+            )
+
+    def test_issue_link_unicity_no_nb_lines(self):
+        """
+        Check that the unique constraints are respected when nb_lines is not set
+        """
+        self.revision.issue_links.create(
+            issue=self.err_issue, line=100, nb_lines=None, char=1
+        )
+        with self.assertRaises(IntegrityError):
+            self.revision.issue_links.create(
+                issue=self.err_issue, line=100, nb_lines=None, char=1
+            )
+
+    def test_issue_link_unicity_no_char(self):
+        """
+        Check that the unique constraints are respected when char is not set
+        """
+        self.revision.issue_links.create(
+            issue=self.err_issue, line=100, nb_lines=42, char=None
+        )
+        with self.assertRaises(IntegrityError):
+            self.revision.issue_links.create(
+                issue=self.err_issue, line=100, nb_lines=42, char=None
+            )
+
+    def test_issue_link_unicity_no_position(self):
+        """
+        Check that the unique constraints are respected when no positioning args are set
+        """
+        self.revision.issue_links.create(
+            issue=self.err_issue, line=None, nb_lines=None, char=None
+        )
+        with self.assertRaises(IntegrityError):
+            self.revision.issue_links.create(
+                issue=self.err_issue, line=None, nb_lines=None, char=None
+            )