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

[vlad-shapik]

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).

[barecheck]

React App | Jest test suite - Code coverage report

Total: 39.1%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[barecheck]

Dedupe Hub App | Unittest test suite - Code coverage report

Total: 55.73%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[barecheck]

Countries App | Unittest test suite - Code coverage report

Total: 100%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[barecheck]

Django App | Unittest test suite - Code coverage report

Total: 81.57%

Your code coverage diff: 0.06% ▴

✅ All code changes are covered

[coderabbitai]

📝 Walkthrough

Walkthrough

Enhanced ISIC-4 validation for production location endpoints with new constraints: rejecting empty objects, enforcing string types for all fields, preventing unrecognized fields, and detecting duplicates. Includes serializer updates, validators, and comprehensive test coverage.

Changes

Cohort / File(s)	Summary
Release Notes doc/release/RELEASE-NOTES.md	Added OSDEV-2292 entry documenting ISIC-4 validation enhancements for POST and PATCH production-locations endpoints.
ISIC-4 Entry Serializer src/django/api/serializers/v1/isic4_entry_serializer.py	Refactored to use runtime-defined 'class' field (reserved keyword). Added to_internal_value override for unknown-field rejection and type validation. Added validate method to enforce non-empty objects. Updated error messaging.
Production Location Schema Serializer src/django/api/serializers/v1/production_location_schema_serializer.py	Updated isic_4 field error messages. Added validate_isic_4 validator to detect and reject duplicate ISIC-4 objects.
ISIC-4 Serializer Tests src/django/api/tests/test_isic4_entry_serializer.py	Renamed test method to reflect empty-object rejection. Expanded validation tests for blank fields, unknown fields, numeric values, and error reporting. Removed ProductionLocationSchemaIsic4Test class.
Production Location Schema Tests src/django/api/tests/test_contribution_production_location_schema_serializer.py	New test module validating ISIC-4 behavior: multiple entries, duplicate rejection, empty-object rejection, list-type enforcement.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• ISIC4EntrySerializer validation logic: Custom to_internal_value with multi-step validation (type checking, unknown field detection, field validation), along with __init__ dynamic field setup and validate method—verify correctness of validation order and error handling
• Duplicate detection algorithm: Ensure the tuple-normalization approach in validate_isic_4 correctly identifies duplicates regardless of field order
• Test coverage: Confirm test cases adequately cover the new validation paths and edge cases

Possibly related PRs

• [Backmerge][OSDEV-2285] Adjust multiupload for isic_4 fields #829: Modifies ISIC-4 and production-location schema serializers to change ISIC-4 input handling and validation
• [OSDEV-2122][Climate Trace] Allow users to PATCH v1/production-locations without 'name', 'address', 'country' #728: Updates ProductionLocationSchemaSerializer with core fields handling and required-field toggling
• [OSDEV-1806] Fix parent company validation #543: Refactors validation behavior within ProductionLocationSchemaSerializer

Suggested reviewers

• VadimKovalenkoSNF
• protsack-stephan
• roman-stolar

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly and specifically summarizes the main change: stricter ISIC-4 validation for v1 production location endpoints, with precise focus on the feature area (ISIC-4 taxonomy) and the endpoint scope (v1 production locations).
Description check	✅ Passed	The PR description is comprehensive and directly related to the changeset, detailing all four specific validation enhancements (empty object rejection, type checking, unrecognized field rejection, and duplicate detection) that align with the code changes across multiple files.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch OSDEV-2292-rba-taxonomy-isic-4-validation-allows-empty-objects-invalid-fields-and-numeric-values

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 097bc96 and 17965fb.

📒 Files selected for processing (5)

• doc/release/RELEASE-NOTES.md (2 hunks)
• src/django/api/serializers/v1/isic4_entry_serializer.py (1 hunks)
• src/django/api/serializers/v1/production_location_schema_serializer.py (2 hunks)
• src/django/api/tests/test_contribution_production_location_schema_serializer.py (1 hunks)
• src/django/api/tests/test_isic4_entry_serializer.py (2 hunks)

🧰 Additional context used

🧠 Learnings (8)

📓 Common learnings

Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 728
File: src/django/api/moderation_event_actions/creation/location_contribution/processors/production_location_data_processor.py:156-163
Timestamp: 2025-09-09T09:02:07.291Z
Learning: For PATCH requests to `/api/v1/production-locations/{os_id}/`, the OS ID is always present as a URL parameter, so `event_dto.os` should never be None during validation in the ProductionLocationDataProcessor.

Learnt from: mazursasha1990
Repo: opensupplyhub/open-supply-hub PR: 488
File: src/django/api/views/v1/opensearch_query_builder/opensearch_query_director.py:114-119
Timestamp: 2025-01-29T08:51:47.803Z
Learning: In the Open Supply Hub codebase, request parameter validation should be handled in the serializer classes (e.g., ProductionLocationsSerializer) rather than in the query builders or directors.

Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 470
File: src/react/src/components/Contribute/ProductionLocationInfo.jsx:680-692
Timestamp: 2025-01-23T11:10:17.929Z
Learning: The ProductionLocationInfo component in Open Supply Hub implements field-level validation for name, address and country fields using Material-UI's TextField validation patterns and touch states, providing real-time feedback to users.

Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 598
File: src/django/api/serializers/v1/additional_identifiers_serializer.py:0-0
Timestamp: 2025-04-22T20:16:21.889Z
Learning: The validation logic for AdditionalIdentifiersSerializer will be implemented in a future PR. The current implementation is intentionally a placeholder.

📚 Learning: 2024-11-13T07:21:28.686Z

Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 391
File: src/django/api/serializers/v1/opensearch_common_validators/moderation_id_validator.py:20-25
Timestamp: 2024-11-13T07:21:28.686Z
Learning: In the `ModerationIdValidator` class located at `src/django/api/serializers/v1/opensearch_common_validators/moderation_id_validator.py`, it's not necessary to add input size validation for the `moderation_id` list. OpenSearch already imposes a page size limit, which mitigates potential DoS attacks from large inputs.

Applied to files:

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

📚 Learning: 2024-11-12T11:15:04.794Z

Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 391
File: src/django/api/serializers/v1/opensearch_common_validators/date_range_validator.py:9-11
Timestamp: 2024-11-12T11:15:04.794Z
Learning: In `src/django/api/serializers/v1/opensearch_common_validators/date_range_validator.py`, date format validation is not required within the `DateRangeValidator` class, as date validation is handled elsewhere in the codebase.

Applied to files:

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

📚 Learning: 2024-11-26T04:59:12.296Z

Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 420
File: doc/release/RELEASE-NOTES.md:38-38
Timestamp: 2024-11-26T04:59:12.296Z
Learning: For endpoints that haven't been released to end users, it's acceptable to document API changes under the 'Bugfix' section in the release notes.

Applied to files:

• doc/release/RELEASE-NOTES.md

📚 Learning: 2025-06-02T13:24:57.659Z

Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 641
File: doc/release/RELEASE-NOTES.md:6-35
Timestamp: 2025-06-02T13:24:57.659Z
Learning: The Open Supply Hub team keeps placeholders in release notes until code freeze, then fills in the actual content once all changes are finalized for the release.

Applied to files:

• doc/release/RELEASE-NOTES.md

📚 Learning: 2025-09-09T09:02:07.291Z

Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 728
File: src/django/api/moderation_event_actions/creation/location_contribution/processors/production_location_data_processor.py:156-163
Timestamp: 2025-09-09T09:02:07.291Z
Learning: For PATCH requests to `/api/v1/production-locations/{os_id}/`, the OS ID is always present as a URL parameter, so `event_dto.os` should never be None during validation in the ProductionLocationDataProcessor.

Applied to files:

• doc/release/RELEASE-NOTES.md

📚 Learning: 2025-04-22T20:16:21.889Z

Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 598
File: src/django/api/serializers/v1/additional_identifiers_serializer.py:0-0
Timestamp: 2025-04-22T20:16:21.889Z
Learning: The validation logic for AdditionalIdentifiersSerializer will be implemented in a future PR. The current implementation is intentionally a placeholder.

Applied to files:

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

📚 Learning: 2024-12-12T14:59:19.694Z

Learnt from: mazursasha1990
Repo: opensupplyhub/open-supply-hub PR: 438
File: src/django/api/tests/test_moderation_events_add_production_location.py:21-65
Timestamp: 2024-12-12T14:59:19.694Z
Learning: In `src/django/api/tests/test_moderation_events_add_production_location.py`, the tests have been refactored, and the use of `POST` methods is intentional. Future suggestions to change HTTP methods in these tests may not be necessary.

Applied to files:

• src/django/api/tests/test_contribution_production_location_schema_serializer.py
• src/django/api/tests/test_isic4_entry_serializer.py

🧬 Code graph analysis (2)

src/django/api/tests/test_contribution_production_location_schema_serializer.py (2)

src/django/api/serializers/v1/production_location_post_schema_serializer.py (1)

• ProductionLocationPostSchemaSerializer (5-10)

src/django/api/tests/test_isic4_entry_serializer.py (1)

• setUp (7-16)

src/django/api/tests/test_isic4_entry_serializer.py (1)

src/django/api/serializers/v1/isic4_entry_serializer.py (1)

• ISIC4EntrySerializer (8-89)

🪛 Ruff (0.14.7)

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

107-110: Avoid specifying long messages outside the exception class

(TRY003)

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

47-49: Avoid specifying long messages outside the exception class

(TRY003)

---

83-87: Avoid specifying long messages outside the exception class

(TRY003)

🔇 Additional comments (10)

src/django/api/serializers/v1/production_location_schema_serializer.py (2)

98-113: LGTM! Duplicate detection logic is correct.

The implementation properly normalizes ISIC-4 objects to sorted tuples for order-independent comparison. Using a list for seen is acceptable given the max_length=15 constraint on the field.

---

64-69: Clear, domain-specific error messages.

The updated error messages are user-friendly and clearly describe the validation constraints.

doc/release/RELEASE-NOTES.md (2)

23-23: Release notes accurately document the ISIC-4 validation enhancements.

The Code/API changes entry comprehensively describes all four validation improvements: rejecting empty objects, enforcing string types, rejecting unrecognized fields, and detecting duplicates.

---

33-33: Good user-facing description of the validation improvements.

The "What's new" entry effectively communicates the benefits to API consumers, noting they will now receive specific validation errors for previously accepted but invalid data.

src/django/api/tests/test_contribution_production_location_schema_serializer.py (1)

1-113: Comprehensive test coverage for ISIC-4 validation at the schema level.

The tests effectively cover the key validation scenarios:

• Multiple valid entries are accepted
• Exact duplicates are rejected
• Order-independent duplicates are detected
• Empty objects are rejected
• Type validation (dict vs list)

The test structure is clean with good reuse of base_payload and valid_isic_entry fixtures.

src/django/api/tests/test_isic4_entry_serializer.py (2)

27-37: Test correctly updated to reflect the new validation behavior.

The renamed test test_missing_fields_are_not_allowed properly validates that empty ISIC-4 objects are now rejected with an appropriate error message.

---

49-136: Thorough test coverage for the new entry-level validations.

The new tests effectively cover:

• Blank class field rejection (important since it's added at runtime)
• Unknown field detection (single and multiple)
• Numeric value rejection (single and multiple fields)
• Error prioritization when only unknown fields are present

The test_numeric_values_for_multiple_fields_are_rejected test at lines 109-121 is particularly valuable as it verifies that valid fields don't produce spurious errors alongside invalid ones.

src/django/api/serializers/v1/isic4_entry_serializer.py (3)

8-20: Good solution for handling the reserved keyword class.

Adding the class field at runtime in __init__ is the correct approach since class is a Python reserved keyword. The comment clearly explains the reasoning.

---

44-78: Well-structured validation with proper error aggregation.

The to_internal_value override correctly:

1. Validates input is a dict
2. Detects unknown fields before they're silently dropped by DRF
3. Validates string types for known fields
4. Aggregates multiple errors per category

The validation order is appropriate - checking unknown fields first, then type validation, ensures clear error messages.

---

80-89: Non-empty validation correctly placed in validate method.

The emptiness check in validate runs after to_internal_value, which ensures that unknown-field-only payloads get the unknown field error (not the empty error), as verified by the test test_only_unknown_fields_shows_unknown_field_error.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[roman-stolar]

LGTM

[VadimKovalenkoSNF]

Approved.