From 9920c3fd9cf003e1df89893694177d1d3eef7877 Mon Sep 17 00:00:00 2001 From: Oliver Date: Fri, 5 May 2023 14:06:19 +1000 Subject: [PATCH] Metadata fix (#4725) * Add 'clean' method to MetadataMixin class - Ensure that the "metadata" is a valid dict object * Add "overwrite" option for set_metadata method * Update unit tests * full_clean -> clean * Cleanup * Fix for MetadataMixin * Updates for unit tests * Test --- InvenTree/InvenTree/models.py | 27 ++++++++++++++++++++++++--- InvenTree/build/test_build.py | 2 +- InvenTree/company/tests.py | 4 ++-- InvenTree/label/tests.py | 1 - InvenTree/order/test_sales_order.py | 2 -- InvenTree/order/tests.py | 9 ++++++++- InvenTree/part/test_bom_item.py | 1 - InvenTree/part/test_param.py | 1 - InvenTree/part/test_part.py | 1 - InvenTree/report/tests.py | 2 +- InvenTree/stock/tests.py | 1 - 11 files changed, 36 insertions(+), 15 deletions(-) diff --git a/InvenTree/InvenTree/models.py b/InvenTree/InvenTree/models.py index 385a5c8c53..5f17a031a5 100644 --- a/InvenTree/InvenTree/models.py +++ b/InvenTree/InvenTree/models.py @@ -60,6 +60,27 @@ class MetadataMixin(models.Model): """Meta for MetadataMixin.""" abstract = True + def save(self, *args, **kwargs): + """Save the model instance, and perform validation on the metadata field.""" + self.validate_metadata() + super().save(*args, **kwargs) + + def clean(self, *args, **kwargs): + """Perform model validation on the metadata field.""" + super().clean() + + self.validate_metadata() + + def validate_metadata(self): + """Validate the metadata field.""" + + # Ensure that the 'metadata' field is a valid dict object + if self.metadata is None: + self.metadata = {} + + if type(self.metadata) is not dict: + raise ValidationError({'metadata': _('Metadata must be a python dict object')}) + metadata = models.JSONField( blank=True, null=True, verbose_name=_('Plugin Metadata'), @@ -80,16 +101,16 @@ class MetadataMixin(models.Model): return self.metadata.get(key, backup_value) - def set_metadata(self, key: str, data, commit: bool = True): + def set_metadata(self, key: str, data, commit: bool = True, overwrite: bool = False): """Save the provided metadata under the provided key. Args: key (str): Key for saving metadata data (Any): Data object to save - must be able to be rendered as a JSON string commit (bool, optional): If true, existing metadata with the provided key will be overwritten. If false, a merge will be attempted. Defaults to True. + overwrite (bool): If true, delete existing metadata before adding new value """ - if self.metadata is None: - # Handle a null field value + if overwrite or self.metadata is None: self.metadata = {} self.metadata[key] = data diff --git a/InvenTree/build/test_build.py b/InvenTree/build/test_build.py index d6788d38ec..5279894cf1 100644 --- a/InvenTree/build/test_build.py +++ b/InvenTree/build/test_build.py @@ -579,7 +579,7 @@ class BuildTest(BuildTestBase): for model in [Build, BuildItem]: p = model.objects.first() - self.assertIsNone(p.metadata) + self.assertEqual(len(p.metadata.keys()), 0) self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123) diff --git a/InvenTree/company/tests.py b/InvenTree/company/tests.py index 4c83864297..bc9c51fa81 100644 --- a/InvenTree/company/tests.py +++ b/InvenTree/company/tests.py @@ -135,7 +135,7 @@ class CompanySimpleTest(TestCase): def test_metadata(self): """Unit tests for the metadata field.""" p = Company.objects.first() - self.assertIsNone(p.metadata) + self.assertIn(p.metadata, [None, {}]) self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123) @@ -227,7 +227,7 @@ class ManufacturerPartSimpleTest(TestCase): """Unit tests for the metadata field.""" for model in [ManufacturerPart, SupplierPart]: p = model.objects.first() - self.assertIsNone(p.metadata) + self.assertIn(p.metadata, [None, {}]) self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123) diff --git a/InvenTree/label/tests.py b/InvenTree/label/tests.py index 523364ff1f..810b348b5d 100644 --- a/InvenTree/label/tests.py +++ b/InvenTree/label/tests.py @@ -135,7 +135,6 @@ class LabelTest(InvenTreeAPITestCase): """Unit tests for the metadata field.""" for model in [StockItemLabel, StockLocationLabel, PartLabel]: p = model.objects.first() - self.assertIsNone(p.metadata) self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123) diff --git a/InvenTree/order/test_sales_order.py b/InvenTree/order/test_sales_order.py index 2a42abc08c..d957d7c1f8 100644 --- a/InvenTree/order/test_sales_order.py +++ b/InvenTree/order/test_sales_order.py @@ -303,8 +303,6 @@ class SalesOrderTest(TestCase): for model in [SalesOrder, SalesOrderLineItem, SalesOrderExtraLine, SalesOrderShipment]: p = model.objects.first() - self.assertIsNone(p.metadata) - self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123) diff --git a/InvenTree/order/tests.py b/InvenTree/order/tests.py index b0ed86418b..b3870f1fb4 100644 --- a/InvenTree/order/tests.py +++ b/InvenTree/order/tests.py @@ -390,7 +390,14 @@ class OrderTest(TestCase): """Unit tests for the metadata field.""" for model in [PurchaseOrder, PurchaseOrderLineItem, PurchaseOrderExtraLine]: p = model.objects.first() - self.assertIsNone(p.metadata) + + # Setting metadata to something *other* than a dict will fail + with self.assertRaises(django_exceptions.ValidationError): + p.metadata = 'test' + p.save() + + # Reset metadata to known state + p.metadata = {} self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123) diff --git a/InvenTree/part/test_bom_item.py b/InvenTree/part/test_bom_item.py index 5148ea7048..cda8029009 100644 --- a/InvenTree/part/test_bom_item.py +++ b/InvenTree/part/test_bom_item.py @@ -250,7 +250,6 @@ class BomItemTest(TestCase): """Unit tests for the metadata field.""" for model in [BomItem]: p = model.objects.first() - self.assertIsNone(p.metadata) self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123) diff --git a/InvenTree/part/test_param.py b/InvenTree/part/test_param.py index 8b8906e0ea..7844503d94 100644 --- a/InvenTree/part/test_param.py +++ b/InvenTree/part/test_param.py @@ -47,7 +47,6 @@ class TestParams(TestCase): """Unit tests for the metadata field.""" for model in [PartParameterTemplate]: p = model.objects.first() - self.assertIsNone(p.metadata) self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123) diff --git a/InvenTree/part/test_part.py b/InvenTree/part/test_part.py index d502b659f8..e1a7db6006 100644 --- a/InvenTree/part/test_part.py +++ b/InvenTree/part/test_part.py @@ -277,7 +277,6 @@ class PartTest(TestCase): """Unit tests for the metadata field.""" for model in [Part]: p = model.objects.first() - self.assertIsNone(p.metadata) self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123) diff --git a/InvenTree/report/tests.py b/InvenTree/report/tests.py index 90b72f3980..4adb0aa12d 100644 --- a/InvenTree/report/tests.py +++ b/InvenTree/report/tests.py @@ -299,7 +299,7 @@ class ReportTest(InvenTreeAPITestCase): if self.model is not None: p = self.model.objects.first() - self.assertIsNone(p.metadata) + self.assertEqual(p.metadata, {}) self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123) diff --git a/InvenTree/stock/tests.py b/InvenTree/stock/tests.py index 05e7e201e0..b9f66f2df3 100644 --- a/InvenTree/stock/tests.py +++ b/InvenTree/stock/tests.py @@ -921,7 +921,6 @@ class StockTest(StockTestBase): """Unit tests for the metadata field.""" for model in [StockItem, StockLocation]: p = model.objects.first() - self.assertIsNone(p.metadata) self.assertIsNone(p.get_metadata('test')) self.assertEqual(p.get_metadata('test', backup_value=123), 123)