[OSDEV-2292] Added stricter ISIC-4 taxonomy validation to v1 production locations endpoints

doc/release/RELEASE-NOTES.md

@@ -20,6 +20,7 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html
 * [Follow-up][OSDEV-2066](https://opensupplyhub.atlassian.net/browse/OSDEV-2066) - Removed the `null=True` parameter from the `Contributor.partner_fields` ManyToManyField definition in the model class to resolve Django system check warning W340. The parameter had no effect on the field behavior as ManyToManyFields store relationships in an intermediary table rather than as a database column that could contain NULL values.
 * [OSDEV-2114](https://opensupplyhub.atlassian.net/browse/OSDEV-2114) - Added the `reindex_locations_with_environmental_data` management command to perform a one-time reindexing of locations that have approved claims containing the new environmental data fields (opening/closing dates, throughput, energy consumption). This ensures that the already contributed environmental data will be displayed on the location profile once the environmental data display feature is released in version `2.17.0`. The command also includes a `--dry-run` flag to preview the affected location IDs before execution.
 * [OSDEV-2225](https://opensupplyhub.atlassian.net/browse/OSDEV-2225) - Apart from implementing the clear button for the opening and closing dates in the environmental data section within the claim form, the `ShowOnly` React component was also rewritten and made fully functional.
+* [OSDEV-2292](https://opensupplyhub.atlassian.net/browse/OSDEV-2292) - Enhanced ISIC-4 validation in `ISIC4EntrySerializer` and `ProductionLocationSchemaSerializer` for `POST /v1/production-locations/` and `PATCH /v1/production-locations/{os_id}` requests to reject empty ISIC-4 objects (at least one field from section, division, group, or class must be provided), enforce strict type checking to ensure all ISIC-4 field values are strings, reject unrecognized fields that are not one of the four valid ISIC-4 field names, and reject duplicate ISIC-4 objects within the same array (duplicates are detected regardless of field order).
 
 ### Bugfix
 * [OSDEV-2277](https://opensupplyhub.atlassian.net/browse/OSDEV-2277) - Introduced a reusable `FormFieldHint` component to display helper text below form field titles. Applied to `Affiliations` and `Certifications` fields to improve user guidance.
@@ -29,6 +30,7 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html
 * [OSDEV-2114](https://opensupplyhub.atlassian.net/browse/OSDEV-2114) - Extended the `GET api/facilities/{os_id}/` endpoint with additional claim environmental data including opening date, closing date, estimated annual throughput, and actual annual energy consumption. The production location profile page now displays these new environmental data points as part of the claim information.
 * [OSDEV-2269](https://opensupplyhub.atlassian.net/browse/OSDEV-2269) - Added URI format support for partner fields with JSON schema. Properties defined with `"format": "uri"` are rendered as clickable links on production location profile pages.
 * [OSDEV-2225](https://opensupplyhub.atlassian.net/browse/OSDEV-2225) - Added a clear button to the opening and closing date inputs in the environmental data section within the claim form to allow users to remove selected opening and closing dates.
+* [OSDEV-2292](https://opensupplyhub.atlassian.net/browse/OSDEV-2292) - Improved ISIC-4 validation for `POST /v1/production-locations/` and `PATCH /v1/production-locations/{os_id}` endpoints to provide clearer error messages when invalid data is submitted, including rejection of empty ISIC-4 objects, non-string values, unrecognized fields, and duplicate entries. API consumers will now receive specific validation errors for previously accepted but invalid ISIC-4 taxonomy data, ensuring only properly formatted ISIC-4 classification objects (with string values for section, division, group, or class fields) are stored in the system.
 
 ### Release instructions
 * Ensure that the following commands are included in the `post_deployment` command:

src/django/api/serializers/v1/isic4_entry_serializer.py

@@ -1,54 +1,89 @@
-from rest_framework import serializers
+from rest_framework.serializers import (
+    Serializer,
+    CharField,
+    ValidationError
+)
 
 
-class ISIC4EntrySerializer(serializers.Serializer):
-    isic_class = serializers.CharField(
-        source='class',
-        required=False,
-        allow_blank=False,
-        error_messages={
-            'required': 'Field class is required for isic_4.',
-            'blank': 'Field class must be a non-empty string.',
-            'invalid': 'Field class must be a valid string.',
-        },
-    )
-    group = serializers.CharField(
+class ISIC4EntrySerializer(Serializer):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        # Define 'class' field directly since it's a Python reserved keyword.
+        # We can't use it as a class attribute, so we add it in the __init__
+        # method.
+        self.fields['class'] = CharField(
+            required=False,
+            allow_blank=False,
+            error_messages={
+                'blank': 'Field class must be a non-empty string.',
+            },
+        )
+
+    group = CharField(
         required=False,
         allow_blank=False,
         error_messages={
-            'required': 'Field group is required for isic_4.',
             'blank': 'Field group must be a non-empty string.',
-            'invalid': 'Field group must be a valid string.',
         },
     )
-    section = serializers.CharField(
+    section = CharField(
         required=False,
         allow_blank=False,
         error_messages={
-            'required': 'Field section is required for isic_4.',
             'blank': 'Field section must be a non-empty string.',
-            'invalid': 'Field section must be a valid string.',
         },
     )
-    division = serializers.CharField(
+    division = CharField(
         required=False,
         allow_blank=False,
         error_messages={
-            'required': 'Field division is required for isic_4.',
             'blank': 'Field division must be a non-empty string.',
-            'invalid': 'Field division must be a valid string.',
         },
     )
 
-    def validate(self, attrs):
+    def to_internal_value(self, data):
+        """Override to check for unknown fields before they're dropped."""
+        if not isinstance(data, dict):
+            raise ValidationError(
+                'ISIC-4 entry must be an object.'
+            )
+
+        field_names = ('class', 'group', 'section', 'division')
+
+        # Check for unknown fields.
+        unknown_fields = set(data.keys()) - set(field_names)
+        if unknown_fields:
+            errors = {}
+            for field in unknown_fields:
+                errors[field] = [
+                    f'Field {field} is not a valid ISIC-4 field. '
+                    f'Only section, division, group, and class '
+                    f'are allowed.'
+                ]
+            raise ValidationError(errors)
+
+        # Check that fields are strings (not numeric or other types).
         errors = {}
-        raw_data = getattr(self, 'initial_data', {}) or {}
-        for field_name in ('class', 'group', 'section', 'division'):
-            if field_name not in raw_data:
+        for field_name in field_names:
+            if field_name not in data:
                 continue
-            if not isinstance(raw_data[field_name], str):
-                errors[field_name] = ['Field '
-                                      f'{field_name} must be a string.']
+            if not isinstance(data[field_name], str):
+                errors[field_name] = [
+                    f'Field {field_name} must be a string.'
+                ]
+
         if errors:
-            raise serializers.ValidationError(errors)
+            raise ValidationError(errors)
+
+        return super().to_internal_value(data)
+
+    def validate(self, attrs):
+        # Check that object is not empty (at least one field required).
+        if not attrs:
+            raise ValidationError(
+                'ISIC-4 object cannot be empty. '
+                'At least one field (section, division, group, or '
+                'class) must be provided.'
+            )
+
         return attrs

src/django/api/serializers/v1/production_location_schema_serializer.py

@@ -62,10 +62,10 @@ class ProductionLocationSchemaSerializer(serializers.Serializer):
         min_length=1,
         max_length=15,
         error_messages={
-            'min_length': 'Provide at least one isic_4 object.',
-            'max_length': 'Provide at most 15 isic_4 objects.',
-            'invalid': 'Field isic_4 must be a list of objects.',
-            'empty': 'Field isic_4 cannot be empty.',
+            'not_a_list': 'Field isic_4 must be a list of objects.',
+            'min_length': 'Provide at least one ISIC-4 object.',
+            'max_length': 'Provide at most 15 ISIC-4 objects.',
+            'empty': 'Field isic_4 cannot be empty if provided.',
         },
     )
 
@@ -95,6 +95,23 @@ def _validate_string_field(self, data, field_name):
             }
         return None
 
+    def validate_isic_4(self, value):
+        """Validate that there are no duplicate ISIC-4 objects."""
+        seen = []
+
+        for isic_obj in value:
+            # Convert to a comparable format (sorted tuple of items)
+            # to detect duplicates regardless of field order.
+            isic_tuple = tuple(sorted(isic_obj.items()))
+            if isic_tuple in seen:
+                raise serializers.ValidationError(
+                    "Duplicate ISIC-4 objects are not allowed "
+                    "within the same array."
+                )
+            seen.append(isic_tuple)
+
+        return value
+
     def validate(self, data):
         errors = []
         for field in self.core_fields:

src/django/api/tests/test_contribution_production_location_schema_serializer.py

@@ -0,0 +1,113 @@
+from django.test import SimpleTestCase
+
+from api.serializers.v1.production_location_post_schema_serializer import (
+    ProductionLocationPostSchemaSerializer,
+)
+
+
+class ContributionProductionLocationSchemaSerializerTest(SimpleTestCase):
+    def setUp(self):
+        self.valid_isic_entry_1 = {
+            'class': '620 - Computer programming, consultancy and related '
+                     'activities',
+            'group': '62 - Computer programming, consultancy and related '
+                     'activities',
+            'section': 'J - Information and communication',
+            'division': '62 - Computer programming, consultancy and related '
+                        'activities',
+        }
+        self.valid_isic_entry_2 = {
+            'class': '561 - Restaurants and mobile food service activities',
+            'group': '56 - Food and beverage service activities',
+            'section': 'I - Accommodation and food service activities',
+            'division': '56 - Food and beverage service activities',
+        }
+        self.base_payload = {
+            'name': 'Test Facility',
+            'address': '123 Test Street',
+            'country': 'US',
+        }
+
+    def test_multiple_isic_entries_allowed(self):
+        payload = {
+            **self.base_payload,
+            'isic_4': [
+                self.valid_isic_entry_1,
+                self.valid_isic_entry_2,
+            ],
+        }
+        serializer = ProductionLocationPostSchemaSerializer(data=payload)
+
+        self.assertTrue(serializer.is_valid(), serializer.errors)
+
+    def test_duplicate_isic_entries_are_rejected(self):
+        '''Test that duplicate ISIC-4 objects in the array are rejected.'''
+        payload = {
+            **self.base_payload,
+            'isic_4': [
+                self.valid_isic_entry_1,
+                self.valid_isic_entry_1,  # Duplicate.
+            ],
+        }
+        serializer = ProductionLocationPostSchemaSerializer(data=payload)
+
+        self.assertFalse(serializer.is_valid())
+        self.assertIn('isic_4', serializer.errors)
+        self.assertIn(
+            'Duplicate ISIC-4 objects are not allowed',
+            str(serializer.errors['isic_4'])
+        )
+
+    def test_duplicate_isic_entries_different_order_are_rejected(self):
+        '''Test that duplicates are detected regardless of field order.'''
+        entry_1 = {
+            'class': '620',
+            'group': '62',
+            'section': 'J',
+            'division': '62',
+        }
+        entry_2 = {
+            'section': 'J',
+            'division': '62',
+            'class': '620',
+            'group': '62',
+        }
+        payload = {
+            **self.base_payload,
+            'isic_4': [entry_1, entry_2],
+        }
+        serializer = ProductionLocationPostSchemaSerializer(data=payload)
+
+        self.assertFalse(serializer.is_valid())
+        self.assertIn('isic_4', serializer.errors)
+        self.assertIn(
+            'Duplicate ISIC-4 objects are not allowed',
+            str(serializer.errors['isic_4'])
+        )
+
+    def test_empty_isic_object_rejected(self):
+        '''Test that empty ISIC-4 objects are rejected.'''
+        payload = {
+            **self.base_payload,
+            'isic_4': [{}],
+        }
+        serializer = ProductionLocationPostSchemaSerializer(data=payload)
+
+        self.assertFalse(serializer.is_valid())
+        self.assertIn('isic_4', serializer.errors)
+        self.assertIn('cannot be empty', str(serializer.errors['isic_4']))
+
+    def test_isic_dict_instead_of_list_rejected(self):
+        '''Test that passing a dict instead of list for isic_4 is rejected.'''
+        payload = {
+            **self.base_payload,
+            'isic_4': self.valid_isic_entry_1,  # Dict instead of list.
+        }
+        serializer = ProductionLocationPostSchemaSerializer(data=payload)
+
+        self.assertFalse(serializer.is_valid())
+        self.assertIn('isic_4', serializer.errors)
+        self.assertIn(
+            'must be a list',
+            str(serializer.errors['isic_4'])
+        )