refactor(Price tags): fix stats. move constants. cleanup (#653)

open_prices/api/proofs/tests.py

@@ -6,7 +6,6 @@
 from django.urls import reverse
 from PIL import Image
 
-from open_prices.common.constants import PriceTagStatus
 from open_prices.locations import constants as location_constants
 from open_prices.locations.factories import LocationFactory
 from open_prices.prices.factories import PriceFactory
@@ -406,11 +405,11 @@ def setUpTestData(cls):
         cls.price_tag_1 = PriceTagFactory(
             proof=cls.proof,
             price=cls.price,
-            status=PriceTagStatus.linked_to_price.value,
+            status=proof_constants.PriceTagStatus.linked_to_price.value,
         )
         cls.price_tag_2 = PriceTagFactory(proof=cls.proof)
         cls.price_tag_3 = PriceTagFactory(
-            proof=cls.proof_2, status=PriceTagStatus.deleted
+            proof=cls.proof_2, status=proof_constants.PriceTagStatus.deleted
         )
 
     def test_price_tag_list(self):
@@ -556,7 +555,10 @@ def test_price_tag_create_with_price(self):
         self.assertEqual(response.data["created_by"], self.user_session.user.user_id)
         self.assertEqual(response.data["updated_by"], self.user_session.user.user_id)
         self.assertEqual(response.data["price_id"], self.price.id)
-        self.assertEqual(response.data["status"], PriceTagStatus.linked_to_price.value)
+        self.assertEqual(
+            response.data["status"],
+            proof_constants.PriceTagStatus.linked_to_price.value,
+        )
 
 
 class PriceTagUpdateApiTest(TestCase):
@@ -616,7 +618,10 @@ def test_price_tag_set_price_id(self):
         # Price ID was set to the new value
         self.assertEqual(response.data["price_id"], self.price.id)
         # Status was automatically set to linked_to_price
-        self.assertEqual(response.data["status"], PriceTagStatus.linked_to_price.value)
+        self.assertEqual(
+            response.data["status"],
+            proof_constants.PriceTagStatus.linked_to_price.value,
+        )
 
     def test_price_tag_set_invalid_price_id(self):
         self.assertEqual(self.price_tag.price_id, None)
@@ -638,11 +643,13 @@ def test_price_tag_set_status(self):
             self.url,
             content_type="application/json",
             # Price associated with another proof
-            data={"status": PriceTagStatus.not_readable.value},
+            data={"status": proof_constants.PriceTagStatus.not_readable.value},
             headers={"Authorization": f"Bearer {self.user_session.token}"},
         )
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.data["status"], PriceTagStatus.not_readable.value)
+        self.assertEqual(
+            response.data["status"], proof_constants.PriceTagStatus.not_readable.value
+        )
 
     def test_price_tag_invalid_status(self):
         self.assertEqual(self.price_tag.status, None)
@@ -721,5 +728,6 @@ def test_price_tag_delete(self):
         self.assertEqual(response.data, None)
         self.assertEqual(PriceTag.objects.filter(id=self.price_tag.id).count(), 1)
         self.assertEqual(
-            PriceTag.objects.get(id=self.price_tag.id).status, PriceTagStatus.deleted
+            PriceTag.objects.get(id=self.price_tag.id).status,
+            proof_constants.PriceTagStatus.deleted,
         )

open_prices/api/proofs/views.py

@@ -24,7 +24,7 @@
 )
 from open_prices.api.utils import get_source_from_request
 from open_prices.common.authentication import CustomAuthentication
-from open_prices.common.constants import PriceTagStatus
+from open_prices.proofs import constants as proof_constants
 from open_prices.proofs.ml import extract_from_price_tag
 from open_prices.proofs.models import PriceTag, Proof
 from open_prices.proofs.utils import store_file
@@ -175,7 +175,7 @@ def destroy(self, request: Request, *args, **kwargs) -> Response:
                 {"detail": "Cannot delete price tag with associated prices."},
                 status=status.HTTP_403_FORBIDDEN,
             )
-        price_tag.status = PriceTagStatus.deleted
+        price_tag.status = proof_constants.PriceTagStatus.deleted
         price_tag.save()
         return Response(status=status.HTTP_204_NO_CONTENT)
 

open_prices/common/constants.py

@@ -1,16 +1,4 @@
-import enum
-
 from babel.numbers import list_currencies
 
 CURRENCY_LIST = sorted(list_currencies())
 CURRENCY_CHOICES = [(key, key) for key in CURRENCY_LIST]
-
-
-class PriceTagStatus(enum.IntEnum):
-    deleted = 0
-    linked_to_price = 1
-    not_readable = 2
-    not_price_tag = 3
-
-
-PRICE_TAG_STATUS_CHOICES = [(item.value, item.name) for item in PriceTagStatus]

open_prices/proofs/constants.py

@@ -1,3 +1,5 @@
+import enum
+
 TYPE_PRICE_TAG = "PRICE_TAG"
 TYPE_RECEIPT = "RECEIPT"
 TYPE_GDPR_REQUEST = "GDPR_REQUEST"
@@ -31,3 +33,13 @@
 PRICE_TAG_PREDICTION_TYPE_CHOICES = [
     (PRICE_TAG_EXTRACTION_TYPE, PRICE_TAG_EXTRACTION_TYPE)
 ]
+
+
+class PriceTagStatus(enum.IntEnum):
+    deleted = 0
+    linked_to_price = 1
+    not_readable = 2
+    not_price_tag = 3
+
+
+PRICE_TAG_STATUS_CHOICES = [(item.value, item.name) for item in PriceTagStatus]

open_prices/proofs/models.py

@@ -404,6 +404,14 @@ def proof_post_save_run_ml_models(sender, instance, created, **kwargs):
             )
 
 
+class PriceTagQuerySet(models.QuerySet):
+    def status_unknown(self):
+        return self.filter(status=None)
+
+    def status_linked_to_price(self):
+        return self.filter(status=proof_constants.PriceTagStatus.linked_to_price.value)
+
+
 class PriceTag(models.Model):
     """A single price tag in a proof."""
 
@@ -429,7 +437,7 @@ class PriceTag(models.Model):
         help_text="Coordinates of the bounding box, in the format [y_min, x_min, y_max, x_max]",
     )
     status = models.IntegerField(
-        choices=constants.PRICE_TAG_STATUS_CHOICES,
+        choices=proof_constants.PRICE_TAG_STATUS_CHOICES,
         null=True,
         blank=True,
         help_text="The annotation status",
@@ -463,6 +471,8 @@ class PriceTag(models.Model):
         auto_now=True, help_text="When the tag was last updated"
     )
 
+    objects = models.Manager.from_queryset(PriceTagQuerySet)()
+
     class Meta:
         db_table = "price_tags"
         verbose_name = "Price Tag"
@@ -523,8 +533,8 @@ def clean(self, *args, **kwargs):
                 )
 
             if self.status is None:
-                self.status = constants.PriceTagStatus.linked_to_price.value
-            elif self.status != constants.PriceTagStatus.linked_to_price.value:
+                self.status = proof_constants.PriceTagStatus.linked_to_price.value
+            elif self.status != proof_constants.PriceTagStatus.linked_to_price.value:
                 utils.add_validation_error(
                     validation_errors,
                     "status",

open_prices/proofs/tests.py

@@ -616,7 +616,7 @@ def test_select_proof_image_dir_existing_dir_create_new_dir(self):
             self.assertEqual(selected_dir, images_dir / "0002")
 
 
-class TestPriceTagCreation(TestCase):
+class PriceTagCreationTest(TestCase):
     def test_create_price_tag_invalid_bounding_box_length(self):
         with self.assertRaises(ValidationError) as cm:
             PriceTagFactory(

open_prices/stats/migrations/0004_totalstats_price_tag_count_and_more.py

@@ -16,12 +16,12 @@ class Migration(migrations.Migration):
         ),
         migrations.AddField(
             model_name="totalstats",
-            name="price_tag_linked_to_price_count",
+            name="price_tag_status_linked_to_price_count",
             field=models.PositiveIntegerField(default=0),
         ),
         migrations.AddField(
             model_name="totalstats",
-            name="price_tag_unknown_count",
+            name="price_tag_status_unknown_count",
             field=models.PositiveIntegerField(default=0),
         ),
     ]

open_prices/stats/migrations/0005_rename_price_tag_linked_to_price_count_totalstats_price_tag_status_linked_to_price_count_and_more.py

@@ -0,0 +1,22 @@
+# Generated by Django 5.1.4 on 2024-12-23 10:49
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("stats", "0004_totalstats_price_tag_count_and_more"),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name="totalstats",
+            old_name="price_tag_status_linked_to_price_count",
+            new_name="price_tag_status_linked_to_price_count",
+        ),
+        migrations.RenameField(
+            model_name="totalstats",
+            old_name="price_tag_status_unknown_count",
+            new_name="price_tag_status_unknown_count",
+        ),
+    ]

open_prices/stats/models.py

@@ -2,8 +2,6 @@
 from django.utils import timezone
 from solo.models import SingletonModel
 
-from open_prices.common.constants import PriceTagStatus
-
 
 class TotalStats(SingletonModel):
     PRICE_COUNT_FIELDS = [
@@ -28,15 +26,16 @@ class TotalStats(SingletonModel):
     ]
     PRICE_TAG_COUNT_FIELDS = [
         "price_tag_count",
-        "price_tag_unknown_count",
-        "price_tag_linked_to_price_count",
+        "price_tag_status_unknown_count",
+        "price_tag_status_linked_to_price_count",
     ]
     USER_COUNT_FIELDS = ["user_count", "user_with_price_count"]
     COUNT_FIELDS = (
         PRICE_COUNT_FIELDS
         + PRODUCT_COUNT_FIELDS
         + LOCATION_COUNT_FIELDS
         + PROOF_COUNT_FIELDS
+        + PRICE_TAG_COUNT_FIELDS
         + USER_COUNT_FIELDS
     )
 
@@ -56,8 +55,8 @@ class TotalStats(SingletonModel):
     proof_type_gdpr_request_count = models.PositiveIntegerField(default=0)
     proof_type_shop_import_count = models.PositiveIntegerField(default=0)
     price_tag_count = models.PositiveIntegerField(default=0)
-    price_tag_unknown_count = models.PositiveIntegerField(default=0)
-    price_tag_linked_to_price_count = models.PositiveIntegerField(default=0)
+    price_tag_status_unknown_count = models.PositiveIntegerField(default=0)
+    price_tag_status_linked_to_price_count = models.PositiveIntegerField(default=0)
     user_count = models.PositiveIntegerField(default=0)
     user_with_price_count = models.PositiveIntegerField(default=0)
 
@@ -115,10 +114,10 @@ def update_price_tag_stats(self):
         from open_prices.proofs.models import PriceTag
 
         self.price_tag_count = PriceTag.objects.count()
-        self.price_tag_unknown_count = PriceTag.objects.filter(status=None).count()
-        self.price_tag_linked_to_price_count = PriceTag.objects.filter(
-            status=PriceTagStatus.linked_to_price.value
-        ).count()
+        self.price_tag_status_unknown_count = PriceTag.objects.status_unknown().count()
+        self.price_tag_status_linked_to_price_count = (
+            PriceTag.objects.status_linked_to_price().count()
+        )
         self.save(update_fields=self.PRICE_TAG_COUNT_FIELDS + ["updated"])
 
     def update_user_stats(self):

open_prices/stats/tests.py

@@ -1,7 +1,6 @@
 from django.db import IntegrityError
 from django.test import TestCase
 
-from open_prices.common.constants import PriceTagStatus
 from open_prices.locations.factories import LocationFactory
 from open_prices.prices import constants as price_constants
 from open_prices.prices.factories import PriceFactory
@@ -77,7 +76,7 @@ def setUpTestData(cls):
         PriceTagFactory(
             proof=cls.proof_price_tag,
             price=cls.price,
-            status=PriceTagStatus.linked_to_price.value,
+            status=proof_constants.PriceTagStatus.linked_to_price.value,
         )
 
     def test_update_price_stats(self):
@@ -124,13 +123,13 @@ def test_update_proof_stats(self):
 
     def test_update_price_tag_stats(self):
         self.assertEqual(self.total_stats.price_tag_count, 0)
-        self.assertEqual(self.total_stats.price_tag_unknown_count, 0)
-        self.assertEqual(self.total_stats.price_tag_linked_to_price_count, 0)
+        self.assertEqual(self.total_stats.price_tag_status_unknown_count, 0)
+        self.assertEqual(self.total_stats.price_tag_status_linked_to_price_count, 0)
         # update_price_tag_stats() will update price_tag_counts
         self.total_stats.update_price_tag_stats()
         self.assertEqual(self.total_stats.price_tag_count, 2)
-        self.assertEqual(self.total_stats.price_tag_unknown_count, 1)
-        self.assertEqual(self.total_stats.price_tag_linked_to_price_count, 1)
+        self.assertEqual(self.total_stats.price_tag_status_unknown_count, 1)
+        self.assertEqual(self.total_stats.price_tag_status_linked_to_price_count, 1)
 
     def test_update_user_stats(self):
         self.assertEqual(self.total_stats.user_count, 0)