refactor(Proofs): refactor method that updates proof fields from pric…

…es (1 instead of 3). ref #481
openfoodfacts · Oct 25, 2024 · e3b97f5 · e3b97f5
1 parent 8bd55e9
commit e3b97f5
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 74 deletions.
diff --git a/open_prices/common/tasks.py b/open_prices/common/tasks.py
@@ -87,13 +87,12 @@ def update_location_counts_task():
 def fix_proof_fields_task():
     """
     Proofs uploaded via the (old) mobile app lack location/date/currency fields
-    Fix these fields using the proof's prices
+    Fill these fields using the proof's prices
     """
     for proof in Proof.objects.with_stats().filter(
         price_count_annotated__gte=1, location=None
     ):
-        for field in Proof.FIX_PRICE_FIELDS:
-            getattr(proof, f"set_missing_{field}_from_prices")()
+        proof.set_missing_fields_from_prices()
 
 
 def dump_db_task():

diff --git a/open_prices/proofs/models.py b/open_prices/proofs/models.py
@@ -29,7 +29,7 @@ def has_type_single_shop(self):
         return self.filter(type__in=proof_constants.TYPE_SINGLE_SHOP_LIST)
 
     def has_type_shopping_session(self):
-        return self.filter(type=proof_constants.TYPE_SHOPPING_SESSION_LIST)
+        return self.filter(type__in=proof_constants.TYPE_SHOPPING_SESSION_LIST)
 
     def has_prices(self):
         return self.filter(price_count__gt=0)
@@ -271,55 +271,34 @@ def update_location(self, location_osm_id, location_osm_type):
         if new_location:
             new_location.update_price_count()
 
-    def set_missing_location_from_prices(self):
-        if self.is_type_single_shop and not self.location and self.prices.exists():
-            proof_prices_location_list = list(
-                self.prices.values_list("location", flat=True).distinct()
-            )
-            if len(proof_prices_location_list) == 1:
-                location = self.prices.first().location
-                self.location_osm_id = location.osm_id
-                self.location_osm_type = location.osm_type
-                self.save()
-            else:
-                print(
-                    "different locations",
-                    self,
-                    self.prices.count(),
-                    proof_prices_location_list,
-                )
-
-    def set_missing_date_from_prices(self):
-        if self.is_type_single_shop and not self.date and self.prices.exists():
-            proof_prices_dates_list = list(
-                self.prices.values_list("date", flat=True).distinct()
-            )
-            if len(proof_prices_dates_list) == 1:
-                self.date = self.prices.first().date
-                self.save()
-            else:
-                print(
-                    "different dates",
-                    self,
-                    self.prices.count(),
-                    proof_prices_dates_list,
-                )
-
-    def set_missing_currency_from_prices(self):
-        if self.is_type_single_shop and not self.currency and self.prices.exists():
-            proof_prices_currencies_list = list(
-                self.prices.values_list("currency", flat=True).distinct()
-            )
-            if len(proof_prices_currencies_list) == 1:
-                self.currency = self.prices.first().currency
-                self.save()
-            else:
-                print(
-                    "different currencies",
-                    self,
-                    self.prices.count(),
-                    proof_prices_currencies_list,
-                )
+    def set_missing_fields_from_prices(self):
+        fields_to_update = list()
+        if self.is_type_single_shop and self.prices.exists():
+            for field in Proof.FIX_PRICE_FIELDS:
+                if not getattr(self, field):
+                    proof_prices_field_list = list(
+                        self.prices.values_list(field, flat=True).distinct()
+                    )
+                    if len(proof_prices_field_list) == 1:
+                        if field == "location":
+                            location = self.prices.first().location
+                            self.location_osm_id = location.osm_id
+                            self.location_osm_type = location.osm_type
+                            fields_to_update.extend(
+                                ["location_osm_id", "location_osm_type"]
+                            )
+                        else:
+                            setattr(self, field, getattr(self.prices.first(), field))
+                            fields_to_update.append(field)
+                    else:
+                        print(
+                            f"different {field}s",
+                            self,
+                            f"{self.prices.count()} prices",
+                            proof_prices_field_list,
+                        )
+        if len(fields_to_update):
+            self.save()
 
 
 @receiver(signals.post_delete, sender=Proof)

diff --git a/open_prices/proofs/tests.py b/open_prices/proofs/tests.py
@@ -239,40 +239,42 @@ def test_update_location(self):
         self.assertEqual(self.proof_price_tag.price_count, 2)
         self.assertEqual(self.proof_price_tag.location.price_count, 2)
 
-    def test_set_missing_location_from_prices(self):
+    def test_set_missing_fields_from_prices(self):
         self.proof_receipt.refresh_from_db()
         self.assertTrue(self.proof_receipt.location is None)
-        self.assertEqual(self.proof_receipt.price_count, 1)
-        self.proof_receipt.set_missing_location_from_prices()
-        self.assertEqual(self.proof_receipt.location, self.location)
-
-    def test_set_missing_date_from_prices(self):
-        self.proof_receipt.refresh_from_db()
         self.assertTrue(self.proof_receipt.date is None)
+        self.assertTrue(self.proof_receipt.currency is None)
         self.assertEqual(self.proof_receipt.price_count, 1)
-        self.proof_receipt.set_missing_date_from_prices()
+        self.proof_receipt.set_missing_fields_from_prices()
+        self.assertEqual(self.proof_receipt.location, self.location)
         self.assertEqual(
             self.proof_receipt.date, self.proof_receipt.prices.first().date
         )
-
-    def test_set_missing_currency_from_prices(self):
-        self.proof_receipt.refresh_from_db()
-        self.assertTrue(self.proof_receipt.currency is None)
-        self.assertEqual(self.proof_receipt.price_count, 1)
-        self.proof_receipt.set_missing_currency_from_prices()
         self.assertEqual(
             self.proof_receipt.currency, self.proof_receipt.prices.first().currency
         )
 
 
 class ProofModelUpdateTest(TestCase):
-    def test_proof_update(self):
-        location = LocationFactory(**LOCATION_OSM_NODE_652825274)
-        proof_price_tag = ProofFactory(
+    @classmethod
+    def setUpTestData(cls):
+        cls.location = LocationFactory(**LOCATION_OSM_NODE_652825274)
+        cls.proof_price_tag = ProofFactory(
             type=proof_constants.TYPE_PRICE_TAG,
-            location_osm_id=location.osm_id,
-            location_osm_type=location.osm_type,
+            location_osm_id=cls.location.osm_id,
+            location_osm_type=cls.location.osm_type,
+            currency="EUR",
+            date="2024-06-30",
+        )
+        PriceFactory(
+            proof_id=cls.proof_price_tag.id,
+            location_osm_id=cls.proof_price_tag.location.osm_id,
+            location_osm_type=cls.proof_price_tag.location.osm_type,
+            price=1.0,
             currency="EUR",
+            date="2024-06-30",
         )
-        proof_price_tag.currency = "USD"
-        proof_price_tag.save()
+
+    def test_proof_update(self):
+        self.proof_price_tag.currency = "USD"
+        self.proof_price_tag.save()