Update UniqueConstraints. Update serializer. Add tests

open_prices/api/locations/filters.py

@@ -22,4 +22,4 @@ class LocationFilter(django_filters.FilterSet):
 
     class Meta:
         model = Location
-        fields = ["price_count"]
+        fields = ["type", "price_count"]

open_prices/api/locations/serializers.py

@@ -13,3 +13,15 @@ class LocationCreateSerializer(serializers.ModelSerializer):
     class Meta:
         model = Location
         fields = Location.CREATE_FIELDS
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        # with UniqueConstraints, DRF sets some fields as required
+        for field in (
+            Location.TYPE_OSM_MANDATORY_FIELDS + Location.TYPE_ONLINE_MANDATORY_FIELDS
+        ):
+            self.fields[field].required = False
+
+    # with UniqueConstraints, DRF wrongly validates.
+    # Leave it to the model's save()
+    validators = []

open_prices/api/locations/tests.py

@@ -27,6 +27,11 @@
     "osm_name": "Auchan",
     "price_count": 50,
 }
+LOCATION_ONLINE_DECATHLON = {
+    "type": location_constants.TYPE_ONLINE,
+    "website_url": "https://www.decathlon.fr/",
+    "price_count": 15,
+}
 
 
 class LocationListApiTest(TestCase):
@@ -68,18 +73,27 @@ def setUpTestData(cls):
         LocationFactory(**LOCATION_OSM_NODE_652825274)
         LocationFactory(**LOCATION_OSM_NODE_6509705997)
         LocationFactory(**LOCATION_OSM_WAY_872934393)
+        LocationFactory(**LOCATION_ONLINE_DECATHLON)
 
     def test_location_list_filter_by_osm_name(self):
         url = self.url + "?osm_name__like=monop"
         response = self.client.get(url)
         self.assertEqual(response.data["total"], 1)
         self.assertEqual(response.data["items"][0]["osm_name"], "Monoprix")
 
+    def test_location_list_filter_by_type(self):
+        url = self.url + "?type=ONLINE"
+        response = self.client.get(url)
+        self.assertEqual(response.data["total"], 1)
+        self.assertEqual(
+            response.data["items"][0]["type"], location_constants.TYPE_ONLINE
+        )
+
     def test_location_list_filter_by_price_count(self):
         # exact price_count
         url = self.url + "?price_count=15"
         response = self.client.get(url)
-        self.assertEqual(response.data["total"], 1)
+        self.assertEqual(response.data["total"], 1 + 1)
         self.assertEqual(response.data["items"][0]["price_count"], 15)
         # lte / gte
         url = self.url + "?price_count__gte=20"
@@ -88,7 +102,7 @@ def test_location_list_filter_by_price_count(self):
         self.assertEqual(response.data["items"][0]["price_count"], 50)
         url = self.url + "?price_count__lte=20"
         response = self.client.get(url)
-        self.assertEqual(response.data["total"], 2)
+        self.assertEqual(response.data["total"], 3)
         self.assertEqual(response.data["items"][0]["price_count"], 15)
 
 
@@ -135,7 +149,7 @@ class LocationCreateApiTest(TestCase):
     def setUpTestData(cls):
         cls.url = reverse("api:locations-list")
 
-    def test_location_create(self):
+    def test_location_create_osm(self):
         response = self.client.post(
             self.url, LOCATION_OSM_NODE_652825274, content_type="application/json"
         )
@@ -147,6 +161,14 @@ def test_location_create(self):
         )  # ignored (and post_save signal disabled)
         self.assertEqual(response.data["price_count"], 0)  # ignored
 
+    def test_location_create_online(self):
+        response = self.client.post(
+            self.url, LOCATION_ONLINE_DECATHLON, content_type="application/json"
+        )
+        self.assertEqual(response.status_code, 201)
+        self.assertEqual(response.data["website_url"], "https://www.decathlon.fr/")
+        self.assertEqual(response.data["price_count"], 0)  # ignored
+
 
 class LocationUpdateApiTest(TestCase):
     @classmethod

open_prices/locations/constants.py

@@ -18,7 +18,7 @@
 
 WEBSITE_URL_OK_LIST = [
     "https://www.decathlon.fr/",
-    "https://www.decathlon.fr",
-    "www.decathlon.fr/",
-    "www.decathlon.fr",
+    "https://www.alltricks.fr",
+    "www.ekosport.fr/",
+    "www.auvieuxcampeur.fr",
 ]

open_prices/locations/migrations/0002_location_type_location_website_url_alter_location_osm_id_alter_location_osm_type.py

@@ -39,4 +39,24 @@ class Migration(migrations.Migration):
                 null=True,
             ),
         ),
+        migrations.AlterUniqueTogether(
+            name="location",
+            unique_together=set(),
+        ),
+        migrations.AddConstraint(
+            model_name="location",
+            constraint=models.UniqueConstraint(
+                condition=models.Q(("type", "OSM")),
+                fields=("osm_id", "osm_type"),
+                name="unique_osm_constraint",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="location",
+            constraint=models.UniqueConstraint(
+                condition=models.Q(("type", "ONLINE")),
+                fields=("website_url",),
+                name="unique_online_constraint",
+            ),
+        ),
     ]

open_prices/locations/models.py

@@ -1,7 +1,7 @@
 from django.conf import settings
 from django.core.validators import ValidationError
 from django.db import models
-from django.db.models import Count, signals
+from django.db.models import Count, Q, UniqueConstraint, signals
 from django.dispatch import receiver
 from django.utils import timezone
 from django_q.tasks import async_task
@@ -76,7 +76,18 @@ class Location(models.Model):
     class Meta:
         # managed = False
         db_table = "locations"
-        unique_together = ["osm_id", "osm_type"]
+        constraints = [
+            UniqueConstraint(
+                name="unique_osm_constraint",
+                fields=["osm_id", "osm_type"],
+                condition=Q(type=location_constants.TYPE_OSM),
+            ),
+            UniqueConstraint(
+                name="unique_online_constraint",
+                fields=["website_url"],
+                condition=Q(type=location_constants.TYPE_ONLINE),
+            ),
+        ]
         verbose_name = "Location"
         verbose_name_plural = "Locations"
 

open_prices/locations/tests.py

@@ -64,13 +64,13 @@ def test_location_osm_id_type_validation(self):
                     osm_id=6509705997,
                     osm_type=LOCATION_OSM_TYPE_NOT_OK,
                 )
-        # must be unique
+        # unique constraint
         self.assertRaises(
             ValidationError,
             LocationFactory,
             type=location_constants.TYPE_OSM,
             osm_id=6509705997,
-            osm_type=location_constants.OSM_TYPE_NODE,
+            osm_type=location_constants.OSM_TYPE_OK_LIST[0],
         )
 
     def test_location_decimal_truncate_on_create(self):
@@ -97,6 +97,13 @@ def test_location_online_validation(self):
                 LocationFactory(
                     type=location_constants.TYPE_ONLINE, website_url=WEBSITE_URL
                 )
+        # unique constraint
+        self.assertRaises(
+            ValidationError,
+            LocationFactory,
+            type=location_constants.TYPE_ONLINE,
+            website_url=location_constants.WEBSITE_URL_OK_LIST[0],
+        )
 
 
 class LocationQuerySetTest(TestCase):

open_prices/proofs/tests.py

@@ -35,29 +35,29 @@ def test_proof_location_validation(self):
         # both location_osm_id & location_osm_type not set
         ProofFactory(location_osm_id=None, location_osm_type=None)
         # location_osm_id
-        for LOCATION_OSM_OSM_ID_OK in location_constants.OSM_ID_OK_LIST:
+        for LOCATION_OSM_ID_OK in location_constants.OSM_ID_OK_LIST:
             ProofFactory(
-                location_osm_id=LOCATION_OSM_OSM_ID_OK,
+                location_osm_id=LOCATION_OSM_ID_OK,
                 location_osm_type=location_constants.OSM_TYPE_NODE,
             )
-        for LOCATION_OSM_OSM_ID_NOT_OK in location_constants.OSM_ID_NOT_OK_LIST:
+        for LOCATION_OSM_ID_NOT_OK in location_constants.OSM_ID_NOT_OK_LIST:
             self.assertRaises(
                 ValidationError,
                 ProofFactory,
-                location_osm_id=LOCATION_OSM_OSM_ID_NOT_OK,
+                location_osm_id=LOCATION_OSM_ID_NOT_OK,
                 location_osm_type=location_constants.OSM_TYPE_NODE,
             )
         # location_osm_type
-        for LOCATION_OSM_OSM_TYPE_OK in location_constants.OSM_TYPE_OK_LIST:
+        for LOCATION_OSM_TYPE_OK in location_constants.OSM_TYPE_OK_LIST:
             ProofFactory(
-                location_osm_id=652825274, location_osm_type=LOCATION_OSM_OSM_TYPE_OK
+                location_osm_id=652825274, location_osm_type=LOCATION_OSM_TYPE_OK
             )
-        for LOCATION_OSM_OSM_TYPE_NOT_OK in location_constants.OSM_TYPE_NOT_OK_LIST:
+        for LOCATION_OSM_TYPE_NOT_OK in location_constants.OSM_TYPE_NOT_OK_LIST:
             self.assertRaises(
                 ValidationError,
                 ProofFactory,
                 location_osm_id=652825274,
-                location_osm_type=LOCATION_OSM_OSM_TYPE_NOT_OK,
+                location_osm_type=LOCATION_OSM_TYPE_NOT_OK,
             )