From 4502fdc4115a99dd2598adbe5a6a4764e4a1e094 Mon Sep 17 00:00:00 2001 From: Raphael Odini Date: Wed, 6 Nov 2024 15:39:46 +0100 Subject: [PATCH] temp --- open_prices/api/proofs/tests.py | 9 ++++--- open_prices/proofs/models.py | 48 ++++++++++++++++++++------------- open_prices/proofs/tests.py | 35 +++++++++++++++++------- 3 files changed, 60 insertions(+), 32 deletions(-) diff --git a/open_prices/api/proofs/tests.py b/open_prices/api/proofs/tests.py index 68d7f66a..65da6c8d 100644 --- a/open_prices/api/proofs/tests.py +++ b/open_prices/api/proofs/tests.py @@ -214,15 +214,14 @@ def test_proof_create(self): def test_proof_create_with_location_id(self): location_osm = LocationFactory(**LOCATION_OSM_NODE_652825274) location_online = LocationFactory(type=location_constants.TYPE_ONLINE) - # with location_id, location_osm_id & location_osm_type: OK + # with location_id, location_osm_id & location_osm_type: NOK response = self.client.post( self.url, {**self.data, "location_id": location_osm.id}, headers={"Authorization": f"Bearer {self.user_session.token}"}, ) - self.assertEqual(response.status_code, 201) - self.assertEqual(response.data["location"]["id"], location_osm.id) - # with just location_id (OSM): NOK + self.assertEqual(response.status_code, 400) + # with just location_id (OSM): OK data = self.data.copy() del data["location_osm_id"] del data["location_osm_type"] @@ -232,6 +231,7 @@ def test_proof_create_with_location_id(self): headers={"Authorization": f"Bearer {self.user_session.token}"}, ) self.assertEqual(response.status_code, 400) + self.assertEqual(response.data["location"]["id"], location_osm.id) # with just location_id (ONLINE): OK data = self.data.copy() del data["location_osm_id"] @@ -242,6 +242,7 @@ def test_proof_create_with_location_id(self): headers={"Authorization": f"Bearer {self.user_session.token}"}, ) self.assertEqual(response.status_code, 201) + self.assertEqual(response.data["location"]["id"], location_online.id) def test_proof_create_with_app_name(self): for params, result in [ diff --git a/open_prices/proofs/models.py b/open_prices/proofs/models.py index a420c391..489d7246 100644 --- a/open_prices/proofs/models.py +++ b/open_prices/proofs/models.py @@ -130,9 +130,9 @@ def clean(self, *args, **kwargs): "Should not be in the future", ) # location rules - # - allow passing a location_id - # - location_osm_id should be set if location_osm_type is set - # - location_osm_type should be set if location_osm_id is set + # - allow passing a location_id (should exist) + # - on create: cannot pass both location_id and location_osm_id/type + # - location_osm_id should be set if location_osm_type is set (and vice-versa) # noqa # - some location fields should match the proof fields (on create) if self.location_id: location = None @@ -148,33 +148,43 @@ def clean(self, *args, **kwargs): ) if location: - if location.type == location_constants.TYPE_ONLINE: + if not self.id: # additional rules on create if self.location_osm_id: validation_errors = utils.add_validation_error( validation_errors, "location_osm_id", - "Can only be set if location type is OSM", + "Cannot be set if location_id is filled", ) if self.location_osm_type: validation_errors = utils.add_validation_error( validation_errors, "location_osm_type", - "Can only be set if location type is OSM", + "Cannot be set if location_id is filled", ) - elif location.type == location_constants.TYPE_OSM: - if not self.id: # skip these checks on update - for LOCATION_FIELD in Proof.DUPLICATE_LOCATION_FIELDS: - location_field_value = getattr( - self.location, LOCATION_FIELD.replace("location_", "") + if location.type == location_constants.TYPE_ONLINE: + if self.location_osm_id: + validation_errors = utils.add_validation_error( + validation_errors, + "location_osm_id", + "Can only be set if location type is OSM", ) - if location_field_value: - proof_field_value = getattr(self, LOCATION_FIELD) - if str(location_field_value) != str(proof_field_value): - validation_errors = utils.add_validation_error( - validation_errors, - "location", - f"Location {LOCATION_FIELD} ({location_field_value}) does not match the proof {LOCATION_FIELD} ({proof_field_value})", - ) + if self.location_osm_type: + validation_errors = utils.add_validation_error( + validation_errors, + "location_osm_type", + "Can only be set if location type is OSM", + ) + else: # cleanup: allows update (only by location_id) + self.location_osm_id = ( + None + if location.type == location_constants.TYPE_ONLINE + else location.osm_id + ) + self.location_osm_type = ( + None + if location.type == location_constants.TYPE_ONLINE + else location.osm_type + ) else: if self.location_osm_id: if not self.location_osm_type: diff --git a/open_prices/proofs/tests.py b/open_prices/proofs/tests.py index 59622c8d..f4be9bdf 100644 --- a/open_prices/proofs/tests.py +++ b/open_prices/proofs/tests.py @@ -28,6 +28,11 @@ "osm_type": location_constants.OSM_TYPE_NODE, "osm_name": "Carrefour", } +LOCATION_ONLINE_DECATHLON = { + "type": location_constants.TYPE_ONLINE, + "website_url": "https://www.decathlon.fr/", + "price_count": 15, +} class ProofModelSaveTest(TestCase): @@ -84,21 +89,16 @@ def test_proof_location_validation(self): ValidationError, ProofFactory, location_id=location_osm.id, - location_osm_id=None, # needed - location_osm_type=None, # needed + location_osm_id=652825274, # should be None ) self.assertRaises( ValidationError, ProofFactory, location_id=location_online.id, - location_osm_id=LOCATION_OSM_ID_OK, # should be None + location_osm_id=652825274, # should be None + location_osm_type=LOCATION_OSM_TYPE_OK, # should be None ) # location_id ok - ProofFactory( - location_id=location_osm.id, - location_osm_id=location_osm.osm_id, - location_osm_type=location_osm.osm_type, - ) ProofFactory( location_id=location_online.id, location_osm_id=None, location_osm_type=None ) @@ -273,6 +273,7 @@ class ProofModelUpdateTest(TestCase): def setUpTestData(cls): cls.location_osm_1 = LocationFactory(**LOCATION_OSM_NODE_652825274) cls.location_osm_2 = LocationFactory(**LOCATION_OSM_NODE_6509705997) + cls.location_osm_3 = LocationFactory(**LOCATION_ONLINE_DECATHLON) cls.proof_price_tag = ProofFactory( type=proof_constants.TYPE_PRICE_TAG, location_osm_id=cls.location_osm_1.osm_id, @@ -299,15 +300,31 @@ def test_proof_update(self): self.proof_price_tag.date = "2024-07-01" self.proof_price_tag.save() self.assertEqual(str(self.proof_price_tag.prices.first().date), "2024-07-01") - # location + # location: osm_id & osm_type: ignored self.proof_price_tag.location_osm_id = self.location_osm_2.osm_id self.proof_price_tag.location_osm_type = self.location_osm_2.osm_type self.proof_price_tag.save() self.proof_price_tag.refresh_from_db() + self.assertEqual(self.proof_price_tag.location, self.location_osm_1) # same + self.assertEqual( + self.proof_price_tag.prices.first().location, self.location_osm_1 + ) # same + # location: id (OSM) + self.proof_price_tag.location_id = self.location_osm_2.id + self.proof_price_tag.save() + self.proof_price_tag.refresh_from_db() self.assertEqual(self.proof_price_tag.location, self.location_osm_2) self.assertEqual( self.proof_price_tag.prices.first().location, self.location_osm_2 ) + # location: id (ONLINE) + self.proof_price_tag.location_id = self.location_osm_3.id + self.proof_price_tag.save() + self.proof_price_tag.refresh_from_db() + self.assertEqual(self.proof_price_tag.location, self.location_osm_3) + self.assertEqual( + self.proof_price_tag.prices.first().location, self.location_osm_3 + ) class RunOCRTaskTest(TestCase):