From e3b97f543a700d93740483753efee1385fbc70a7 Mon Sep 17 00:00:00 2001 From: Raphael Odini Date: Fri, 25 Oct 2024 21:46:30 +0200 Subject: [PATCH] refactor(Proofs): refactor method that updates proof fields from prices (1 instead of 3). ref #481 --- open_prices/common/tasks.py | 5 +-- open_prices/proofs/models.py | 79 +++++++++++++----------------------- open_prices/proofs/tests.py | 44 ++++++++++---------- 3 files changed, 54 insertions(+), 74 deletions(-) diff --git a/open_prices/common/tasks.py b/open_prices/common/tasks.py index 6daa835f..450436ed 100644 --- 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 index fab7b426..59647546 100644 --- 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 index d524d086..05326d00 100644 --- 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()