From 8a0a811821a86bd9b884fc0cba67fe057561988e Mon Sep 17 00:00:00 2001 From: Viggo de Vries Date: Fri, 23 Feb 2024 14:37:25 +0100 Subject: [PATCH 1/6] Implement productrecommendations --- oscar_odin/mappings/catalogue.py | 12 +++++++ oscar_odin/mappings/context.py | 30 ++++++++++------ oscar_odin/resources/catalogue.py | 6 ++++ tests/reverse/test_catalogue.py | 60 +++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 10 deletions(-) diff --git a/oscar_odin/mappings/catalogue.py b/oscar_odin/mappings/catalogue.py index 1fa6dd7..7de086e 100644 --- a/oscar_odin/mappings/catalogue.py +++ b/oscar_odin/mappings/catalogue.py @@ -289,6 +289,13 @@ def stockrecords( return [] + @odin.map_list_field + def recommended_products(self, values): + if values: + return RecommendedProductToModel.apply(values) + + return [] + @odin.map_field def product_class(self, value) -> ProductClassModel: if not value or self.source.structure == ProductModel.CHILD: @@ -297,6 +304,11 @@ def product_class(self, value) -> ProductClassModel: return ProductClassToModel.apply(value) +class RecommendedProductToModel(OscarBaseMapping): + from_obj = resources.catalogue.ProductRecommentation + to_obj = ProductModel + + class ParentToModel(OscarBaseMapping): from_obj = resources.catalogue.ParentProduct to_obj = ProductModel diff --git a/oscar_odin/mappings/context.py b/oscar_odin/mappings/context.py index 3bc8678..cf0fd4d 100644 --- a/oscar_odin/mappings/context.py +++ b/oscar_odin/mappings/context.py @@ -278,21 +278,23 @@ def bulk_update_or_create_many_to_many(self): m2m_to_create, m2m_to_update, _ = self.get_all_m2m_relations # Create many to many's - for relation, instances in m2m_to_create.items(): + for relation, instances_to_create in m2m_to_create.items(): fields = self.get_fields_to_update(relation.related_model) if fields is not None: - validated_m2m_instances = self.validate_instances(instances) - relation.related_model.objects.bulk_create(validated_m2m_instances) + if relation.related_model != self.Model: + instances_to_create = self.validate_instances(instances_to_create) + relation.related_model.objects.bulk_create(instances_to_create) # Update many to many's - for relation, instances in m2m_to_update.items(): + for relation, instances_to_update in m2m_to_update.items(): fields = self.get_fields_to_update(relation.related_model) if fields is not None: - validated_instances_to_update = self.validate_instances( - instances, fields=fields - ) + if relation.related_model != self.Model: + instances_to_update = self.validate_instances( + instances_to_update, fields=fields + ) relation.related_model.objects.bulk_update( - validated_instances_to_update, fields=fields + instances_to_update, fields=fields ) for relation, values in self.many_to_many_items.items(): @@ -319,7 +321,10 @@ def bulk_update_or_create_many_to_many(self): # Delete throughs if no instances are passed for the field if self.delete_related: Through.objects.filter( - product_id__in=to_delete_throughs_product_ids + **{ + "%s_id__in" + % relation.m2m_field_name(): to_delete_throughs_product_ids + } ).all().delete() if throughs: @@ -341,7 +346,12 @@ def bulk_update_or_create_many_to_many(self): # Delete remaining non-existing through models if self.delete_related: Through.objects.filter( - product_id__in=[item[0] for item in bulk_troughs.keys()] + **{ + "%s_id__in" + % relation.m2m_field_name(): [ + item[0] for item in bulk_troughs.keys() + ] + } ).exclude(id__in=bulk_troughs.values()).delete() # Save only new through models diff --git a/oscar_odin/resources/catalogue.py b/oscar_odin/resources/catalogue.py index a731336..04f439b 100644 --- a/oscar_odin/resources/catalogue.py +++ b/oscar_odin/resources/catalogue.py @@ -85,6 +85,10 @@ class ParentProduct(OscarCatalogue): upc: str +class ProductRecommentation(OscarCatalogue): + upc: str + + class Product(OscarCatalogue): """A product within Django Oscar.""" @@ -111,6 +115,8 @@ class Product(OscarCatalogue): attributes: Dict[str, Union[Any, None]] categories: List[Category] + recommended_products: List[ProductRecommentation] + date_created: Optional[datetime] date_updated: Optional[datetime] diff --git a/tests/reverse/test_catalogue.py b/tests/reverse/test_catalogue.py index 99ee1f7..917acdd 100644 --- a/tests/reverse/test_catalogue.py +++ b/tests/reverse/test_catalogue.py @@ -15,6 +15,7 @@ ProductClass as ProductClassResource, Category as CategoryResource, ParentProduct as ParentProductResource, + ProductRecommentation as ProductRecommentationResource, ) from oscar_odin.exceptions import OscarOdinException from oscar_odin.mappings.constants import ( @@ -23,6 +24,7 @@ STOCKRECORD_NUM_ALLOCATED, PRODUCTIMAGE_ORIGINAL, PRODUCT_TITLE, + PRODUCT_UPC, PRODUCT_DESCRIPTION, PRODUCTCLASS_REQUIRESSHIPPING, ) @@ -33,6 +35,7 @@ ProductImage = get_model("catalogue", "ProductImage") Category = get_model("catalogue", "Category") Partner = get_model("partner", "Partner") +ProductRecommendation = get_model("catalogue", "ProductRecommendation") class SingleProductReverseTest(TestCase): @@ -482,6 +485,63 @@ def test_create_product_with_related_fields(self): self.assertEqual(prd2.attr.harrie, 1) +class ProductRecommendationTest(TestCase): + def setUp(self): + super().setUp() + ProductClass.objects.create( + name="Klaas", slug="klaas", requires_shipping=True, track_stock=True + ) + Partner.objects.create(name="klaas") + + def test_recommendation(self): + product_resource = [ + ProductResource( + upc="recommended_product1", + title="asdf2", + slug="asdf-asdfasdf2", + description="description", + structure=Product.STANDALONE, + product_class=ProductClassResource(slug="klaas"), + ), + ProductResource( + upc="recommended_product2", + title="asdf2", + slug="asdf-asdasdfasdf2", + description="description", + structure=Product.STANDALONE, + product_class=ProductClassResource(slug="klaas"), + ), + ] + + _, errors = products_to_db(product_resource) + self.assertEqual(len(errors), 0) + + product_resource = ProductResource( + upc="harses", + title="asdf2", + slug="asdf-asdfas23df2", + description="description", + structure=Product.STANDALONE, + product_class=ProductClassResource(slug="klaas"), + recommended_products=[ + ProductRecommentationResource(upc="recommended_product1"), + ProductRecommentationResource(upc="recommended_product2"), + ], + ) + + _, errors = products_to_db(product_resource, fields_to_update=[PRODUCT_UPC]) + self.assertEqual(len(errors), 0) + + prd = Product.objects.get(upc="harses") + + self.assertEquals(ProductRecommendation.objects.count(), 2) + self.assertEquals(prd.recommended_products.count(), 2) + self.assertEquals( + sorted(list(prd.recommended_products.values_list("upc", flat=True))), + sorted(["recommended_product1", "recommended_product2"]), + ) + + class ParentChildTest(TestCase): def setUp(self): super().setUp() From 4b8dfaaa4323014583c41638dd9e767044876666 Mon Sep 17 00:00:00 2001 From: Viggo de Vries Date: Fri, 23 Feb 2024 15:01:54 +0100 Subject: [PATCH 2/6] equal ofcourse --- tests/reverse/test_catalogue.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/reverse/test_catalogue.py b/tests/reverse/test_catalogue.py index 917acdd..4f54e0a 100644 --- a/tests/reverse/test_catalogue.py +++ b/tests/reverse/test_catalogue.py @@ -534,9 +534,9 @@ def test_recommendation(self): prd = Product.objects.get(upc="harses") - self.assertEquals(ProductRecommendation.objects.count(), 2) - self.assertEquals(prd.recommended_products.count(), 2) - self.assertEquals( + self.assertEqual(ProductRecommendation.objects.count(), 2) + self.assertEqual(prd.recommended_products.count(), 2) + self.assertEqual( sorted(list(prd.recommended_products.values_list("upc", flat=True))), sorted(["recommended_product1", "recommended_product2"]), ) From a79cae104f1bd2bd6daba3909dcd85d9189f1a1b Mon Sep 17 00:00:00 2001 From: Viggo de Vries Date: Mon, 26 Feb 2024 11:32:56 +0100 Subject: [PATCH 3/6] Make flag to not update related objects from the same type --- oscar_odin/mappings/context.py | 63 +++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/oscar_odin/mappings/context.py b/oscar_odin/mappings/context.py index cf0fd4d..9164c47 100644 --- a/oscar_odin/mappings/context.py +++ b/oscar_odin/mappings/context.py @@ -58,6 +58,8 @@ class ModelMapperContext(dict): Model = None errors = None + update_related_models_same_type = True + def __init__(self, Model, *args, delete_related=False, **kwargs): super().__init__(*args, **kwargs) self.foreign_key_items = defaultdict(list) @@ -187,12 +189,11 @@ def bulk_update_or_create_foreign_keys(self): Model = field.related_model fields = self.get_fields_to_update(Model) if fields is not None: - validated_instances_to_update = self.validate_instances( - instances, fields=fields - ) - Model.objects.bulk_update( - validated_instances_to_update, fields=fields - ) + if self.update_related_models_same_type or Model != self.Model: + instances_to_update = self.validate_instances( + instances, fields=fields + ) + Model.objects.bulk_update(instances_to_update, fields=fields) def bulk_update_or_create_instances(self, instances): ( @@ -232,23 +233,29 @@ def bulk_update_or_create_one_to_many(self): instances_to_create, instances_to_update, identities = self.get_o2m_relations - for relation, instances in instances_to_create.items(): + for relation, instances_to_create in instances_to_create.items(): fields = self.get_fields_to_update(relation.related_model) if fields is not None: - validated_instances_to_create = self.validate_instances(instances) - relation.related_model.objects.bulk_create( - validated_instances_to_create - ) + if ( + self.update_related_models_same_type + or relation.related_model != self.Model + ): + instances_to_create = self.validate_instances(instances_to_create) + relation.related_model.objects.bulk_create(instances_to_create) - for relation, instances in instances_to_update.items(): + for relation, instances_to_update in instances_to_update.items(): fields = self.get_fields_to_update(relation.related_model) if fields is not None: - validated_instances_to_update = self.validate_instances( - instances, fields=fields - ) - relation.related_model.objects.bulk_update( - validated_instances_to_update, fields=fields - ) + if ( + self.update_related_models_same_type + or relation.related_model != self.Model + ): + instances_to_update = self.validate_instances( + instances_to_update, fields=fields + ) + relation.related_model.objects.bulk_update( + instances_to_update, fields=fields + ) if self.delete_related: for relation, keys in identities.items(): @@ -281,21 +288,27 @@ def bulk_update_or_create_many_to_many(self): for relation, instances_to_create in m2m_to_create.items(): fields = self.get_fields_to_update(relation.related_model) if fields is not None: - if relation.related_model != self.Model: + if ( + self.update_related_models_same_type + or relation.related_model != self.Model + ): instances_to_create = self.validate_instances(instances_to_create) - relation.related_model.objects.bulk_create(instances_to_create) + relation.related_model.objects.bulk_create(instances_to_create) # Update many to many's for relation, instances_to_update in m2m_to_update.items(): fields = self.get_fields_to_update(relation.related_model) if fields is not None: - if relation.related_model != self.Model: + if ( + self.update_related_models_same_type + or relation.related_model != self.Model + ): instances_to_update = self.validate_instances( instances_to_update, fields=fields ) - relation.related_model.objects.bulk_update( - instances_to_update, fields=fields - ) + relation.related_model.objects.bulk_update( + instances_to_update, fields=fields + ) for relation, values in self.many_to_many_items.items(): fields = self.get_fields_to_update(relation.related_model) @@ -374,6 +387,8 @@ def bulk_save(self, instances, fields_to_update, identifier_mapping): class ProductModelMapperContext(ModelMapperContext): + update_related_models_same_type = False + @property def get_fk_relations(self): to_create, to_update = super().get_fk_relations From 53bb00181636da86c149b1b186e56def6cbad7e5 Mon Sep 17 00:00:00 2001 From: Viggo de Vries Date: Mon, 26 Feb 2024 11:36:03 +0100 Subject: [PATCH 4/6] Change around the logic --- oscar_odin/mappings/context.py | 64 ++++++++++++++++------------------ 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/oscar_odin/mappings/context.py b/oscar_odin/mappings/context.py index 9164c47..5246632 100644 --- a/oscar_odin/mappings/context.py +++ b/oscar_odin/mappings/context.py @@ -182,18 +182,14 @@ def bulk_update_or_create_foreign_keys(self): field.related_model.objects.bulk_create(validated_fk_instances) for field, instances in instances_to_update.items(): - # We don't update parent details. If we want this then we will have to - # provide other product fields in the ParentProductResource too along with - # the upc, which is not useful in most cases. - if not field.name == "parent": - Model = field.related_model + Model = field.related_model + if self.update_related_models_same_type or Model != self.Model: fields = self.get_fields_to_update(Model) if fields is not None: - if self.update_related_models_same_type or Model != self.Model: - instances_to_update = self.validate_instances( - instances, fields=fields - ) - Model.objects.bulk_update(instances_to_update, fields=fields) + instances_to_update = self.validate_instances( + instances, fields=fields + ) + Model.objects.bulk_update(instances_to_update, fields=fields) def bulk_update_or_create_instances(self, instances): ( @@ -234,22 +230,22 @@ def bulk_update_or_create_one_to_many(self): instances_to_create, instances_to_update, identities = self.get_o2m_relations for relation, instances_to_create in instances_to_create.items(): - fields = self.get_fields_to_update(relation.related_model) - if fields is not None: - if ( - self.update_related_models_same_type - or relation.related_model != self.Model - ): + if ( + self.update_related_models_same_type + or relation.related_model != self.Model + ): + fields = self.get_fields_to_update(relation.related_model) + if fields is not None: instances_to_create = self.validate_instances(instances_to_create) relation.related_model.objects.bulk_create(instances_to_create) for relation, instances_to_update in instances_to_update.items(): - fields = self.get_fields_to_update(relation.related_model) - if fields is not None: - if ( - self.update_related_models_same_type - or relation.related_model != self.Model - ): + if ( + self.update_related_models_same_type + or relation.related_model != self.Model + ): + fields = self.get_fields_to_update(relation.related_model) + if fields is not None: instances_to_update = self.validate_instances( instances_to_update, fields=fields ) @@ -286,23 +282,23 @@ def bulk_update_or_create_many_to_many(self): # Create many to many's for relation, instances_to_create in m2m_to_create.items(): - fields = self.get_fields_to_update(relation.related_model) - if fields is not None: - if ( - self.update_related_models_same_type - or relation.related_model != self.Model - ): + if ( + self.update_related_models_same_type + or relation.related_model != self.Model + ): + fields = self.get_fields_to_update(relation.related_model) + if fields is not None: instances_to_create = self.validate_instances(instances_to_create) relation.related_model.objects.bulk_create(instances_to_create) # Update many to many's for relation, instances_to_update in m2m_to_update.items(): - fields = self.get_fields_to_update(relation.related_model) - if fields is not None: - if ( - self.update_related_models_same_type - or relation.related_model != self.Model - ): + if ( + self.update_related_models_same_type + or relation.related_model != self.Model + ): + fields = self.get_fields_to_update(relation.related_model) + if fields is not None: instances_to_update = self.validate_instances( instances_to_update, fields=fields ) From c690467f54cfb476cfdff6b86b7bbe7d4905a95e Mon Sep 17 00:00:00 2001 From: Viggo de Vries Date: Mon, 26 Feb 2024 11:43:41 +0100 Subject: [PATCH 5/6] Raise odin exception when trough object creation fails --- oscar_odin/mappings/context.py | 10 ++++++++-- tests/reverse/test_catalogue.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/oscar_odin/mappings/context.py b/oscar_odin/mappings/context.py index 5246632..0d54245 100644 --- a/oscar_odin/mappings/context.py +++ b/oscar_odin/mappings/context.py @@ -363,8 +363,14 @@ def bulk_update_or_create_many_to_many(self): } ).exclude(id__in=bulk_troughs.values()).delete() - # Save only new through models - Through.objects.bulk_create(throughs.values()) + try: + # Save only new through models + Through.objects.bulk_create(throughs.values()) + except ValueError as e: + raise OscarOdinException( + "Failed creating Trough models for %s. Maybe the related model does NOT exist?" + % relation.name + ) from e def bulk_save(self, instances, fields_to_update, identifier_mapping): self.fields_to_update = fields_to_update diff --git a/tests/reverse/test_catalogue.py b/tests/reverse/test_catalogue.py index 4f54e0a..6bd1cb4 100644 --- a/tests/reverse/test_catalogue.py +++ b/tests/reverse/test_catalogue.py @@ -541,6 +541,37 @@ def test_recommendation(self): sorted(["recommended_product1", "recommended_product2"]), ) + def test_recommendation_non_existing(self): + product_resource = [ + ProductResource( + upc="recommended_product1", + title="asdf2", + slug="asdf-asdfasdf2", + description="description", + structure=Product.STANDALONE, + product_class=ProductClassResource(slug="klaas"), + ), + ] + + _, errors = products_to_db(product_resource) + self.assertEqual(len(errors), 0) + + product_resource = ProductResource( + upc="harses", + title="asdf2", + slug="asdf-asdfas23df2", + description="description", + structure=Product.STANDALONE, + product_class=ProductClassResource(slug="klaas"), + recommended_products=[ + ProductRecommentationResource(upc="recommended_product1"), + ProductRecommentationResource(upc="recommended_product2"), + ], + ) + + with self.assertRaises(OscarOdinException): + products_to_db(product_resource) + class ParentChildTest(TestCase): def setUp(self): From 6993525b1b1636b391b0bc176260a9a4262653b2 Mon Sep 17 00:00:00 2001 From: Viggo de Vries Date: Mon, 26 Feb 2024 13:54:19 +0100 Subject: [PATCH 6/6] Add test for deleting recommendation --- tests/reverse/test_deleting_related.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/reverse/test_deleting_related.py b/tests/reverse/test_deleting_related.py index 1f5003a..18f910f 100644 --- a/tests/reverse/test_deleting_related.py +++ b/tests/reverse/test_deleting_related.py @@ -14,6 +14,7 @@ Image as ImageResource, ProductClass as ProductClassResource, Category as CategoryResource, + ProductRecommentation as ProductRecommentationResource, ) from oscar_odin.mappings.constants import ( CATEGORY_CODE, @@ -247,6 +248,8 @@ def test_deleting_product_related_models(self): def test_deleting_all_related_models(self): partner = Partner.objects.get(name="klaas") + + Product.objects.create(upc="recommended_product1") product_resource = ProductResource( upc="1234323-2", @@ -274,6 +277,9 @@ def test_deleting_all_related_models(self): original=File(self.image, name="vats.jpg"), ), ], + recommended_products=[ + ProductRecommentationResource(upc="recommended_product1"), + ], categories=[CategoryResource(code="1"), CategoryResource(code="2")], attributes={"henk": "Klaas", "harrie": 1}, ) @@ -286,6 +292,7 @@ def test_deleting_all_related_models(self): self.assertEqual(prd.stockrecords.count(), 1) self.assertEqual(prd.categories.count(), 2) self.assertEqual(prd.attribute_values.count(), 2) + self.assertEqual(prd.recommended_products.count(), 1) product_resource = ProductResource( upc="1234323-2", @@ -303,6 +310,7 @@ def test_deleting_all_related_models(self): self.assertEqual(prd.stockrecords.count(), 0) self.assertEqual(prd.categories.count(), 0) self.assertEqual(prd.attribute_values.count(), 0) + self.assertEqual(prd.recommended_products.count(), 0) def test_partial_deletion_of_one_to_many_related_models(self): partner = Partner.objects.get(name="klaas")