[FIX] Updating instances with incorrect data should return errors (#15)

* feat ⭐ return errors if the instances to be updated are invalid

* feat ⭐ add tests for error handling

oscar_odin/mappings/context.py

@@ -170,7 +170,8 @@ def bulk_update_or_create_foreign_keys(self):
             Model = field.related_model
             fields = self.get_fields_to_update(Model)
             if fields is not None:
-                Model.objects.bulk_update(instances, fields=fields)
+                validated_instances_to_update = self.validate_instances(instances)
+                Model.objects.bulk_update(validated_instances_to_update, fields=fields)
 
     def bulk_update_or_create_instances(self, instances):
         (
@@ -193,11 +194,12 @@ def bulk_update_or_create_instances(self, instances):
 
         fields = self.get_fields_to_update(self.Model)
         if fields is not None:
-            for instance in instances_to_update:
+            validated_instances_to_update = self.validate_instances(instances_to_update)
+            for instance in validated_instances_to_update:
                 # This should be removed once support for django 3.2 is dropped
                 # pylint: disable=protected-access
                 instance._prepare_related_fields_for_save("bulk_update")
-            self.Model.objects.bulk_update(instances_to_update, fields=fields)
+            self.Model.objects.bulk_update(validated_instances_to_update, fields=fields)
 
     def bulk_update_or_create_one_to_many(self):
         for relation, product, instances in self.get_all_o2m_instances:
@@ -213,7 +215,10 @@ def bulk_update_or_create_one_to_many(self):
         for relation, instances in instances_to_update.items():
             fields = self.get_fields_to_update(relation.related_model)
             if fields is not None:
-                relation.related_model.objects.bulk_update(instances, fields=fields)
+                validated_instances_to_update = self.validate_instances(instances)
+                relation.related_model.objects.bulk_update(
+                    validated_instances_to_update, fields=fields
+                )
 
     def bulk_update_or_create_many_to_many(self):
         m2m_to_create, m2m_to_update = self.get_all_m2m_relations
@@ -227,7 +232,10 @@ def bulk_update_or_create_many_to_many(self):
         for relation, instances in m2m_to_update.items():
             fields = self.get_fields_to_update(relation.related_model)
             if fields is not None:
-                relation.related_model.objects.bulk_update(instances, fields=fields)
+                validated_instances_to_update = self.validate_instances(instances)
+                relation.related_model.objects.bulk_update(
+                    validated_instances_to_update, fields=fields
+                )
 
         for relation, values in self.many_to_many_items.items():
             Through = getattr(self.Model, relation.name).through
@@ -326,12 +334,18 @@ def bulk_update_or_create_product_attributes(self, instances):
         if attributes_to_delete:
             ProductAttributeValue.objects.filter(pk__in=attributes_to_delete).delete()
         if attributes_to_update:
+            validated_attributes_to_update = self.validate_instances(
+                attributes_to_update
+            )
             ProductAttributeValue.objects.bulk_update(
-                attributes_to_update, fields_to_be_updated, batch_size=500
+                validated_attributes_to_update, fields_to_be_updated, batch_size=500
             )
         if attributes_to_create:
+            validated_attributes_to_create = self.validate_instances(
+                attributes_to_create
+            )
             ProductAttributeValue.objects.bulk_create(
-                attributes_to_create, batch_size=500, ignore_conflicts=False
+                validated_attributes_to_create, batch_size=500, ignore_conflicts=False
             )
 
     def bulk_update_or_create_instances(self, instances):

tests/reverse/test_catalogue.py

@@ -551,3 +551,129 @@ def test_non_existing_parent_childs(self):
 
         with self.assertRaises(OscarOdinException):
             products_to_db(child_product)
+
+
+class SingleProductErrorHandlingTest(TestCase):
+    @property
+    def image(self):
+        img = PIL.Image.new(mode="RGB", size=(200, 200))
+        output = io.BytesIO()
+        img.save(output, "jpeg")
+        return output
+
+    def test_error_handling_on_product_operations(self):
+        product_class = ProductClassResource(
+            slug="klaas", name="Klaas", requires_shipping=True, track_stock=True
+        )
+
+        partner = Partner.objects.create(name="klaas")
+
+        Category.add_root(name="Hatsie", slug="batsie", is_public=True, code="1")
+        Category.add_root(name="henk", slug="klaas", is_public=True, code="2")
+
+        # Incorrect data for creating product
+        product_resource = ProductResource(
+            upc="1234323-2",
+            title="asdf2",
+            slug="asdf-asdfasdf2",
+            description="description",
+            structure=Product.STANDALONE,
+            is_discountable=True,
+            price=D("20"),
+            availability=2,
+            currency="EUR",
+            partner=partner,
+            product_class=product_class,
+            images=[
+                ImageResource(
+                    caption="gekke caption",
+                    display_order="top",
+                    original=File(self.image, name="harrie.jpg"),
+                ),
+                ImageResource(
+                    caption="gekke caption 2",
+                    display_order=1,
+                    original=File(self.image, name="vats.jpg"),
+                ),
+            ],
+            categories=[CategoryResource(code="2")],
+        )
+        _, errors = products_to_db(product_resource)
+        self.assertEqual(len(errors), 1)
+        self.assertEqual(
+            errors[0].message_dict['display_order'][0],
+            "“top” value must be an integer."
+        )
+
+        # Correct Data for creating product
+        product_resource = ProductResource(
+            upc="1234323-2",
+            title="asdf2",
+            slug="asdf-asdfasdf2",
+            description="description",
+            structure=Product.STANDALONE,
+            is_discountable=True,
+            price="20",
+            availability=2,
+            currency="EUR",
+            partner=partner,
+            product_class=product_class,
+            images=[
+                ImageResource(
+                    caption="gekke caption",
+                    display_order=0,
+                    original=File(self.image, name="harrie.jpg"),
+                ),
+                ImageResource(
+                    caption="gekke caption 2",
+                    display_order=1,
+                    original=File(self.image, name="vats.jpg"),
+                ),
+            ],
+            categories=[CategoryResource(code="2")],
+        )
+        _, errors = products_to_db(product_resource)
+        self.assertEqual(len(errors), 0)
+
+        # Incorrect data for updating product
+        product_resource = ProductResource(
+            upc="1234323-2",
+            title="asdf2",
+            slug="asdf-asdfasdf2",
+            description="description",
+            structure=Product.STANDALONE,
+            is_discountable=53,
+            price="expensive",
+            availability=2,
+            currency="EUR",
+            partner=partner,
+            product_class=product_class,
+            images=[
+                ImageResource(
+                    caption="gekke caption",
+                    display_order=0,
+                    original=File(self.image, name="harrie.jpg"),
+                ),
+                ImageResource(
+                    caption="gekke caption 2",
+                    display_order="Alphabet",
+                    original=File(self.image, name="vats.jpg"),
+                ),
+            ],
+            categories=[CategoryResource(code="2")],
+        )
+        _, errors = products_to_db(product_resource)
+
+        self.assertEqual(len(errors), 3)
+        self.assertEqual(
+            errors[0].message_dict['is_discountable'][0],
+            "“53” value must be either True or False."
+        )
+        self.assertEqual(
+            errors[1].message_dict['display_order'][0],
+            "“Alphabet” value must be an integer."
+        )
+        self.assertEqual(
+            errors[2].message_dict['price'][0],
+            "“expensive” value must be a decimal number."
+        )