Merge pull request #221 from maykinmedia/feature/93-retry-deletion

[#93] Retry deletion (backend)

Author: SilviaAmAm

backend/src/openarchiefbeheer/destruction/models.py

@@ -271,9 +271,10 @@ def process_deletion(self) -> None:
                 )
                 raise exc
 
-            delete_zaak_and_related_objects(
-                zaak=zaak, result_store=ResultStore(store=self)
-            )
+            store = ResultStore(store=self)
+            store.clear_traceback()
+
+            delete_zaak_and_related_objects(zaak=zaak, result_store=store)
 
             zaak.delete()
 

backend/src/openarchiefbeheer/destruction/signals.py

@@ -2,16 +2,20 @@
 from django.db.models.signals import post_save
 from django.dispatch import receiver
 
+from openarchiefbeheer.emails.models import EmailConfig
+
 from .constants import ListRole, ReviewDecisionChoices
-from .models import DestructionListAssignee, DestructionListReview
+from .models import DestructionList, DestructionListAssignee, DestructionListReview
 from .utils import (
+    notify,
     notify_author_changes_requested,
     notify_author_last_review,
     notify_author_positive_review,
     notify_reviewer,
 )
 
 user_assigned = django.dispatch.Signal()
+deletion_failure = django.dispatch.Signal()
 
 
 @receiver(post_save, sender=DestructionListReview)
@@ -46,3 +50,15 @@ def notify_reviewer_of_assignment(sender, assignee, **kwargs):
         return
 
     notify_reviewer(assignee.user, assignee.destruction_list)
+
+
+@receiver(deletion_failure)
+def notify_author_of_failure(sender: DestructionList, **kwargs):
+    config = EmailConfig.get_solo()
+
+    notify(
+        subject=config.subject_error_during_deletion,
+        body=config.body_error_during_deletion,
+        context={"list": sender},
+        recipients=[sender.author.email],
+    )

backend/src/openarchiefbeheer/destruction/tasks.py

@@ -8,6 +8,7 @@
 
 from .constants import InternalStatus, ListItemStatus, ListStatus
 from .models import DestructionList, DestructionListItem, ReviewResponse
+from .signals import deletion_failure
 
 logger = logging.getLogger(__name__)
 
@@ -81,6 +82,8 @@ def handle_processing_error(pk: int) -> None:
     destruction_list.processing_status = InternalStatus.failed
     destruction_list.save()
 
+    deletion_failure.send(sender=destruction_list)
+
 
 @app.task
 def delete_destruction_list_item(pk: int) -> None:

backend/src/openarchiefbeheer/destruction/tests/test_signals.py

@@ -1,12 +1,17 @@
 from unittest.mock import patch
 
 from django.core import mail
-from django.test import TestCase
+from django.test import TestCase, override_settings
 
+from openarchiefbeheer.destruction.tasks import delete_destruction_list
 from openarchiefbeheer.emails.models import EmailConfig
 
-from ..constants import ReviewDecisionChoices
-from .factories import DestructionListReviewFactory
+from ..constants import ListStatus, ReviewDecisionChoices
+from .factories import (
+    DestructionListFactory,
+    DestructionListItemFactory,
+    DestructionListReviewFactory,
+)
 
 
 class SignalsTests(TestCase):
@@ -30,3 +35,34 @@ def test_no_email_sent_if_not_review_created(self, m):
 
         # No extra email sent on update event
         self.assertEqual(len(mail.outbox), 1)
+
+    @override_settings(CELERY_TASK_ALWAYS_EAGER=True)
+    def test_failure_during_deletion_sends_signal(self):
+        destruction_list = DestructionListFactory.create(
+            status=ListStatus.ready_to_delete, author__email="[email protected]"
+        )
+        DestructionListItemFactory.create_batch(2, destruction_list=destruction_list)
+
+        with (
+            patch(
+                "openarchiefbeheer.destruction.models.delete_zaak_and_related_objects",
+                side_effect=Exception,
+            ),
+            patch(
+                "openarchiefbeheer.destruction.utils.EmailConfig.get_solo",
+                return_value=EmailConfig(
+                    subject_error_during_deletion="FAILURE!!",
+                    body_error_during_deletion="ERROR AAAh!",
+                ),
+            ),
+            self.assertRaises(Exception),
+        ):
+            delete_destruction_list(destruction_list)
+
+        self.assertEqual(len(mail.outbox), 1)
+
+        email = mail.outbox[0]
+
+        self.assertEqual(email.subject, "FAILURE!!")
+        self.assertEqual(email.body, "ERROR AAAh!")
+        self.assertEqual(email.to[0], "[email protected]")

backend/src/openarchiefbeheer/destruction/tests/test_tasks.py

@@ -302,3 +302,36 @@ def test_item_skipped_if_already_succeeded(self, logs):
             ),
             logs[0],
         )
+
+    @override_settings(CELERY_TASK_ALWAYS_EAGER=True)
+    def test_processing_list_with_failed_item(self):
+        destruction_list = DestructionListFactory.create(
+            status=ListStatus.ready_to_delete, processing_status=InternalStatus.failed
+        )
+        zaak = ZaakFactory.create()
+        DestructionListItemFactory.create(
+            zaak=zaak.url,
+            destruction_list=destruction_list,
+            processing_status=InternalStatus.failed,
+            internal_results={"traceback": "Some traceback"},
+        )
+
+        with (
+            patch(
+                "openarchiefbeheer.destruction.models.delete_zaak_and_related_objects",
+            ),
+        ):
+            delete_destruction_list(destruction_list)
+
+        destruction_list.refresh_from_db()
+
+        self.assertEqual(destruction_list.processing_status, InternalStatus.succeeded)
+        self.assertEqual(destruction_list.status, ListStatus.deleted)
+
+        item = destruction_list.items.first()
+
+        self.assertEqual(item.processing_status, InternalStatus.succeeded)
+        self.assertEqual(
+            item.internal_results,
+            {"deleted_resources": {}, "resources_to_delete": {}, "traceback": ""},
+        )

backend/src/openarchiefbeheer/emails/admin.py

@@ -38,4 +38,13 @@ class EmailConfigAdmin(SingletonModelAdmin):
                 ]
             },
         ),
+        (
+            _("Templates error during deletion"),
+            {
+                "fields": [
+                    "subject_error_during_deletion",
+                    "body_error_during_deletion",
+                ]
+            },
+        ),
     ]

backend/src/openarchiefbeheer/emails/migrations/0003_emailconfig_body_error_during_deletion_and_more.py

@@ -0,0 +1,32 @@
+# Generated by Django 4.2.14 on 2024-07-26 11:33
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("emails", "0002_emailconfig_body_last_review_and_more"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="emailconfig",
+            name="body_error_during_deletion",
+            field=models.TextField(
+                blank=True,
+                help_text="Body of the email that will be sent to the record manager when an error happened during deletion.",
+                verbose_name="body error during deletion",
+            ),
+        ),
+        migrations.AddField(
+            model_name="emailconfig",
+            name="subject_error_during_deletion",
+            field=models.CharField(
+                blank=True,
+                help_text="Subject of the email that will be sent to the record manager when an error happened during deletion.",
+                max_length=250,
+                verbose_name="subject error during deletion",
+            ),
+        ),
+    ]

backend/src/openarchiefbeheer/emails/models.py

@@ -84,6 +84,23 @@ class EmailConfig(SingletonModel):
         ),
         blank=True,
     )
+    subject_error_during_deletion = models.CharField(
+        max_length=250,
+        verbose_name=_("subject error during deletion"),
+        help_text=_(
+            "Subject of the email that will be sent to the record manager "
+            "when an error happened during deletion."
+        ),
+        blank=True,
+    )
+    body_error_during_deletion = models.TextField(
+        verbose_name=_("body error during deletion"),
+        help_text=_(
+            "Body of the email that will be sent to the record manager "
+            "when an error happened during deletion."
+        ),
+        blank=True,
+    )
 
     class Meta:
         verbose_name = _("email configuration")

backend/src/openarchiefbeheer/fixtures/default_emails.json

@@ -12,7 +12,9 @@
       "subject_last_review": "Vernietigingslijst klaar om vernietigd te worden",
       "body_last_review": "Beste {{ user }}, \r\n\r\nReviewer {{ last_reviewer }} heeft de Vernietigingslijst {{ list }} goedgekeurd. Alle reviewers hebben de lijst goedgekeurd dus die is klaar om te worden vernietigd.",
       "subject_changes_requested": "Voorstel voor wijziging van uw vernietigingslijst",
-      "body_changes_requested": "Beste {{ user }},\r\n\r\nEr is een voorstel tot aanpassing van uw vernietigingslijst {{ list }}. U kunt de lijst en de voorgestelde wijziging in de Open-Archiefbeheer web app bekijken en af te handelen."
+      "body_changes_requested": "Beste {{ user }},\r\n\r\nEr is een voorstel tot aanpassing van uw vernietigingslijst {{ list }}. U kunt de lijst en de voorgestelde wijziging in de Open-Archiefbeheer web app bekijken en af te handelen.",
+      "subject_error_during_deletion": "Fout tijdens vernietiging",
+      "body_error_during_deletion": "Beste {{ user }},\r\n\r\nEr is een fout opgetreden tijdens het vernietigen van vernietigingslijst {{ list }}. Probeer het opnieuw of neem contact op met de IT afdeling."
     }
   }
 ]

backend/src/openarchiefbeheer/utils/results_store.py

@@ -6,19 +6,21 @@
 class InternalResults(TypedDict):
     deleted_resources: dict[str, list]
     resources_to_delete: dict[str, list]
+    traceback: str = ""
 
 
 class Store(Protocol):
     internal_results: InternalResults
 
     def save(self) -> None: ...
+    def refresh_from_db(self) -> None: ...
 
 
 @dataclass
 class ResultStore:
     store: Store
 
-    def _get_internal_results(self) -> InternalResults:
+    def get_internal_results(self) -> InternalResults:
         results = self.store.internal_results
         if not results.get("deleted_resources"):
             results["deleted_resources"] = defaultdict(list)
@@ -27,37 +29,45 @@ def _get_internal_results(self) -> InternalResults:
             results["resources_to_delete"] = defaultdict(list)
         return results
 
+    def refresh_from_db(self) -> None:
+        self.store.refresh_from_db()
+
     def add_deleted_resource(self, resource_type: str, value: str) -> None:
-        results = self._get_internal_results()
+        results = self.get_internal_results()
         results["deleted_resources"][resource_type].append(value)
         self.save()
 
     def save(self) -> None:
         self.store.save()
 
-    def has_deleted_resource(self, resource_type: str, value: str) -> bool:
-        results = self._get_internal_results()
-
-        return value in results["deleted_resources"][resource_type]
-
     def has_resource_to_delete(self, resource_type: str) -> bool:
-        results = self._get_internal_results()
+        results = self.get_internal_results()
 
         return (
             resource_type in results["resources_to_delete"]
             and len(results["resources_to_delete"][resource_type]) > 0
         )
 
     def add_resource_to_delete(self, resource_type: str, value: str) -> None:
-        results = self._get_internal_results()
+        results = self.get_internal_results()
 
         results["resources_to_delete"][resource_type].append(value)
         self.save()
 
     def get_resources_to_delete(self, resource_type: str) -> list[str]:
-        results = self._get_internal_results()
+        results = self.get_internal_results()
         return results["resources_to_delete"][resource_type]
 
     def clear_resources_to_delete(self, resource_type: str) -> None:
-        results = self._get_internal_results()
+        results = self.get_internal_results()
         del results["resources_to_delete"][resource_type]
+
+    def add_traceback(self, formatted_traceback: str) -> None:
+        results = self.get_internal_results()
+        results["traceback"] = formatted_traceback
+        self.save()
+
+    def clear_traceback(self):
+        results = self.get_internal_results()
+        results["traceback"] = ""
+        self.save()