diff --git a/doc/release/RELEASE-NOTES.md b/doc/release/RELEASE-NOTES.md index acec7307f..b27ebf64f 100644 --- a/doc/release/RELEASE-NOTES.md +++ b/doc/release/RELEASE-NOTES.md @@ -52,6 +52,7 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html * [OSDEV-1493](https://opensupplyhub.atlassian.net/browse/OSDEV-1493) - Fixed an issue where the backend sorts countries not by `name` but by their `alpha-2 codes` in `GET /api/v1/moderation-events/` endpoint. * [OSDEV-1532](https://opensupplyhub.atlassian.net/browse/OSDEV-1532) - Fixed the date range picker on the `Moderation Queue` page. A Data Moderator can change the Before date even if an Error message is displayed. * [OSDEV-1533](https://opensupplyhub.atlassian.net/browse/OSDEV-1533) - The presentation of the `Moderation Decision Date` in the `Moderation Queue` table has been corrected. If the "status_change_date" is missing in the object, it now displays as "N/A". +* [OSDEV-1556](https://opensupplyhub.atlassian.net/browse/OSDEV-1556) - Fixed validation of `os_id` for PATCH `/api/v1/moderation-events/{moderation_id}/production-locations/{os_id}/` endpoint. ### What's new * [OSDEV-1376](https://opensupplyhub.atlassian.net/browse/OSDEV-1376) - Updated automated emails for closure reports (report_result) to remove the term "Rejected" for an improved user experience. Added link to Closure Policy and instructions for submitting a Reopening Report to make the process easier to understand for users. diff --git a/src/django/api/os_id.py b/src/django/api/os_id.py index 4f5c6195f..dd150f368 100644 --- a/src/django/api/os_id.py +++ b/src/django/api/os_id.py @@ -80,7 +80,7 @@ def validate_os_id(raw_id, raise_on_invalid=True): return True -os_id_regex = re.compile('[A-Z]{2}[0-9]{7}[A-Z0-9]{6}') +os_id_regex = re.compile('^[A-Z]{2}[0-9]{7}[A-Z0-9]{6}$') def string_matches_os_id_format(string): diff --git a/src/django/api/services/moderation_events_service.py b/src/django/api/services/moderation_events_service.py index bc438ad2a..5d229bf54 100644 --- a/src/django/api/services/moderation_events_service.py +++ b/src/django/api/services/moderation_events_service.py @@ -10,7 +10,7 @@ from api.exceptions import GoneException, InternalServerErrorException from api.models.facility.facility import Facility from api.models.moderation_event import ModerationEvent -from api.os_id import validate_os_id +from api.os_id import string_matches_os_id_format from api.serializers.v1.opensearch_common_validators.moderation_id_validator \ import ModerationIdValidator from api.views.v1.utils import create_error_detail @@ -52,7 +52,7 @@ def validate_moderation_status(status): @staticmethod def validate_location_os_id(os_id): - if not validate_os_id(os_id, raise_on_invalid=False): + if not string_matches_os_id_format(os_id): raise ParseError( create_error_detail( field="os_id", diff --git a/src/django/api/tests/test_moderation_events_update_production_location.py b/src/django/api/tests/test_moderation_events_update_production_location.py index 3f3006e3a..7e5a36794 100644 --- a/src/django/api/tests/test_moderation_events_update_production_location.py +++ b/src/django/api/tests/test_moderation_events_update_production_location.py @@ -112,21 +112,34 @@ def test_moderation_event_not_pending(self): def test_invalid_os_id_format(self): self.login_as_superuser() - response = self.client.patch( - self.get_url().replace(self.os_id, "invalid_os_id"), - data=json.dumps({}), - content_type="application/json", - ) - - self.assertEqual(400, response.status_code) - self.assertEqual( - "The request path parameter is invalid.", response.data["detail"] - ) - self.assertEqual("os_id", response.data["errors"][0]["field"]) - self.assertEqual( - APIV1CommonErrorMessages.LOCATION_ID_NOT_VALID, - response.data["errors"][0]["detail"], - ) + invalid_ids = [ + "A1234567ABCDEF", # Less than 15 characters + "ABC1234567ABCDEF", # More than 15 characters + "AB1234567abcdef", # Contains lowercase letters + "AB1234567AB!DEF", # Contains special character + "AB12345X7ABCDEF", # Letter in the digit section + "1234567ABABCDEF", # Starts with digits + "ABCD56789012345", # Too many letters at the start + "AB12345678ABCDEF" # Too many digits + ] + + for invalid_id in invalid_ids: + response = self.client.patch( + self.get_url().replace(self.os_id, invalid_id), + data=json.dumps({}), + content_type="application/json", + ) + + self.assertEqual(400, response.status_code) + self.assertEqual( + "The request path parameter is invalid.", + response.data["detail"] + ) + self.assertEqual("os_id", response.data["errors"][0]["field"]) + self.assertEqual( + APIV1CommonErrorMessages.LOCATION_ID_NOT_VALID, + response.data["errors"][0]["detail"], + ) def test_no_production_location_found_with_os_id(self): self.login_as_superuser()