Merge pull request #336 from NHSDigital/DTOSS-10448-remove-validation-errors-and-retry

DTOSS-10448: Remove validation errors and retry

Author: Harriethw

…s/management/commands/command_helpers.py …ommands/helpers/message_batch_helpers.pymanage_breast_screening/notifications/management/commands/command_helpers.py renamed to manage_breast_screening/notifications/management/commands/helpers/message_batch_helpers.py manage_breast_screening/notifications/management/commands/command_helpers.py renamed to manage_breast_screening/notifications/management/commands/helpers/message_batch_helpers.py

@@ -1,4 +1,5 @@
 import json
+import re
 from datetime import datetime
 from zoneinfo import ZoneInfo
 
@@ -14,8 +15,6 @@
 
 TZ_INFO = ZoneInfo("Europe/London")
 RECOVERABLE_STATUS_CODES = [
-    # Validation error
-    400,
     # Client side issue
     408,
     # Retried too early
@@ -29,6 +28,8 @@
     # Issue with backend services
     504,
 ]
+VALIDATION_ERROR_STATUS_CODE = 400
+MESSAGE_PATH_REGEX = r"(?<=\/data\/attributes\/messages\/)(\d*)(?=\/)"
 
 
 class MessageBatchHelpers:
@@ -61,10 +62,45 @@ def mark_batch_as_failed(
                     }
                 )
             )
+        elif response.status_code == VALIDATION_ERROR_STATUS_CODE:
+            MessageBatchHelpers.process_validation_errors(message_batch, retry_count)
         else:
             message_batch.status = MessageBatchStatusChoices.FAILED_UNRECOVERABLE.value
         message_batch.save()
 
         for message in message_batch.messages.all():
             message.status = MessageStatusChoices.FAILED.value
+            message.sent_at = datetime.now(tz=TZ_INFO)
             message.save()
+
+    @staticmethod
+    def process_validation_errors(message_batch: MessageBatch, retry_count: int = 0):
+        message_batch_errors = json.loads(message_batch.nhs_notify_errors).get("errors")
+
+        messages = list(Message.objects.filter(batch=message_batch).all())
+        for error in message_batch_errors:
+            message_index_result = re.search(
+                MESSAGE_PATH_REGEX, error["source"]["pointer"]
+            )
+            if message_index_result is not None:
+                message_index = int(message_index_result.group(0))
+                message = messages[message_index]
+                message.batch = None
+                if message.nhs_notify_errors is None:
+                    message.nhs_notify_errors = json.dumps([error])
+                else:
+                    message.nhs_notify_errors = json.dumps(
+                        json.loads(message.nhs_notify_errors) + [error]
+                    )
+                message.save()
+
+        message_batch.status = MessageBatchStatusChoices.FAILED_RECOVERABLE.value
+        message_batch.save()
+        Queue.RetryMessageBatches().add(
+            json.dumps(
+                {
+                    "message_batch_id": str(message_batch.id),
+                    "retry_count": retry_count,
+                }
+            )
+        )

manage_breast_screening/notifications/management/commands/retry_failed_message_batch.py

@@ -5,7 +5,7 @@
 
 from django.core.management.base import BaseCommand, CommandError
 
-from manage_breast_screening.notifications.management.commands.command_helpers import (
+from manage_breast_screening.notifications.management.commands.helpers.message_batch_helpers import (
     MessageBatchHelpers,
 )
 from manage_breast_screening.notifications.models import (

manage_breast_screening/notifications/management/commands/send_message_batch.py

@@ -3,7 +3,7 @@
 
 from django.core.management.base import BaseCommand, CommandError
 
-from manage_breast_screening.notifications.management.commands.command_helpers import (
+from manage_breast_screening.notifications.management.commands.helpers.message_batch_helpers import (
     MessageBatchHelpers,
 )
 from manage_breast_screening.notifications.models import (

manage_breast_screening/notifications/migrations/0016_add_notify_errors_to_messages.py

@@ -0,0 +1,18 @@
+# Generated by Django 5.2.5 on 2025-09-02 10:13
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('notifications', '0015_add_nbss_id_as_index_to_appointments'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='message',
+            name='nhs_notify_errors',
+            field=models.JSONField(blank=True, null=True),
+        ),
+    ]

manage_breast_screening/notifications/models.py

@@ -95,6 +95,7 @@ class Message(models.Model):
     status = models.CharField(
         max_length=50, choices=MessageStatusChoices, default="pending_enrichment"
     )
+    nhs_notify_errors = models.JSONField(blank=True, null=True)
 
     appointment = models.ForeignKey(
         "notifications.Appointment", on_delete=models.PROTECT

manage_breast_screening/notifications/tests/integration/test_retry_message_batches_from_queue.py

@@ -1,13 +1,15 @@
 import json
 import uuid
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
 import pytest
+import requests
 
 from manage_breast_screening.notifications.management.commands.retry_failed_message_batch import (
     Command,
 )
 from manage_breast_screening.notifications.models import Message, MessageBatch
+from manage_breast_screening.notifications.services.api_client import ApiClient
 from manage_breast_screening.notifications.services.queue import Queue
 from manage_breast_screening.notifications.tests.factories import (
     MessageBatchFactory,
@@ -59,3 +61,50 @@ def test_message_batch_is_retried_successfully(
         assert message_batch_actual[0].status == "sent"
         messages_actual = Message.objects.filter(id=message.id)
         assert messages_actual[0].status == "delivered"
+
+    @patch.object(
+        ApiClient, "send_message_batch", return_value=MagicMock(spec=requests.Response)
+    )
+    @pytest.mark.django_db
+    def test_invalid_messages_are_removed(
+        self, mock_send_message_batch, mock_jwt_encode, routing_plan_id, monkeypatch
+    ):
+        monkeypatch.setenv(
+            "QUEUE_STORAGE_CONNECTION_STRING", Helpers().azurite_connection_string()
+        )
+        notify_errors = {
+            "errors": [
+                {
+                    "status": 400,
+                    "source": {
+                        "pointer": "/data/attributes/messages/1/recipient/nhsNumber"
+                    },
+                }
+            ]
+        }
+
+        mock_send_message_batch.return_value.status_code = 400
+        mock_send_message_batch.return_value.json.return_value = json.dumps(
+            notify_errors
+        )
+
+        invalid_message = MessageFactory(status="failed")
+        ok_message_1 = MessageFactory(status="failed")
+        ok_message_2 = MessageFactory(status="failed")
+        message_batch = MessageBatchFactory(
+            routing_plan_id=routing_plan_id,
+            status="failed_recoverable",
+            messages=[ok_message_1, invalid_message, ok_message_2],
+        )
+        queue = Queue.RetryMessageBatches()
+        with Helpers().queue_listener(queue, Command().handle):
+            queue.add(
+                json.dumps(
+                    {"message_batch_id": str(message_batch.id), "retry_count": "0"}
+                )
+            )
+
+        invalid_message.refresh_from_db()
+        assert invalid_message.status == "failed"
+        assert invalid_message.batch is None
+        assert invalid_message.nhs_notify_errors == json.dumps(notify_errors["errors"])