From 08d148f84ea8039d869fca680d59b2c722943d25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Odini?= Date: Mon, 23 Dec 2024 12:22:23 +0100 Subject: [PATCH] refactor(Price tags): fix stats. move constants. cleanup (#653) --- open_prices/api/proofs/tests.py | 24 ++++++++++++------- open_prices/api/proofs/views.py | 4 ++-- open_prices/common/constants.py | 12 ---------- open_prices/proofs/constants.py | 12 ++++++++++ open_prices/proofs/models.py | 16 ++++++++++--- open_prices/proofs/tests.py | 2 +- ...004_totalstats_price_tag_count_and_more.py | 4 ++-- ...g_status_linked_to_price_count_and_more.py | 22 +++++++++++++++++ open_prices/stats/models.py | 19 +++++++-------- open_prices/stats/tests.py | 11 ++++----- 10 files changed, 82 insertions(+), 44 deletions(-) create mode 100644 open_prices/stats/migrations/0005_rename_price_tag_linked_to_price_count_totalstats_price_tag_status_linked_to_price_count_and_more.py diff --git a/open_prices/api/proofs/tests.py b/open_prices/api/proofs/tests.py index 8c40ce45..7b9fb2ff 100644 --- a/open_prices/api/proofs/tests.py +++ b/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, ) diff --git a/open_prices/api/proofs/views.py b/open_prices/api/proofs/views.py index 6a9e88a7..6b74d01e 100644 --- a/open_prices/api/proofs/views.py +++ b/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) diff --git a/open_prices/common/constants.py b/open_prices/common/constants.py index 29c3f1c9..87326065 100644 --- a/open_prices/common/constants.py +++ b/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] diff --git a/open_prices/proofs/constants.py b/open_prices/proofs/constants.py index 48e9cf1a..cb4ea05b 100644 --- a/open_prices/proofs/constants.py +++ b/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] diff --git a/open_prices/proofs/models.py b/open_prices/proofs/models.py index 4a360d85..259b3410 100644 --- a/open_prices/proofs/models.py +++ b/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", diff --git a/open_prices/proofs/tests.py b/open_prices/proofs/tests.py index f02d3aec..8f2af417 100644 --- a/open_prices/proofs/tests.py +++ b/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( diff --git a/open_prices/stats/migrations/0004_totalstats_price_tag_count_and_more.py b/open_prices/stats/migrations/0004_totalstats_price_tag_count_and_more.py index ec802b72..89e3714c 100644 --- a/open_prices/stats/migrations/0004_totalstats_price_tag_count_and_more.py +++ b/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), ), ] diff --git a/open_prices/stats/migrations/0005_rename_price_tag_linked_to_price_count_totalstats_price_tag_status_linked_to_price_count_and_more.py b/open_prices/stats/migrations/0005_rename_price_tag_linked_to_price_count_totalstats_price_tag_status_linked_to_price_count_and_more.py new file mode 100644 index 00000000..891ccffe --- /dev/null +++ b/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", + ), + ] diff --git a/open_prices/stats/models.py b/open_prices/stats/models.py index e8eca917..568628eb 100644 --- a/open_prices/stats/models.py +++ b/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,8 +26,8 @@ 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 = ( @@ -37,6 +35,7 @@ class TotalStats(SingletonModel): + 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): diff --git a/open_prices/stats/tests.py b/open_prices/stats/tests.py index 4f85769b..e171b912 100644 --- a/open_prices/stats/tests.py +++ b/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)