Finding notes cascading deletes (#10636)

* finding-notes-cascading-deletes first pass at cascading deletes for notes/notehistory

* finding-notes-cascading-deletes remove unused code

* finding-notes-cascading-deletes linter cleanup

* finding-notes-cascading-deletes retrigger actions

Author: dogboat

dojo/apps.py

@@ -72,14 +72,19 @@ def ready(self):
         # Load any signals here that will be ready for runtime
         # Importing the signals file is good enough if using the reciever decorator
         import dojo.announcement.signals  # noqa: F401
+        import dojo.benchmark.signals  # noqa: F401
+        import dojo.cred.signals  # noqa: F401
         import dojo.endpoint.signals  # noqa: F401
         import dojo.engagement.signals  # noqa: F401
         import dojo.finding_group.signals  # noqa: F401
+        import dojo.notes.signals  # noqa: F401
         import dojo.product.signals  # noqa: F401
         import dojo.product_type.signals  # noqa: F401
+        import dojo.risk_acceptance.signals  # noqa: F401
         import dojo.sla_config.helpers  # noqa: F401
         import dojo.tags_signals  # noqa: F401
         import dojo.test.signals  # noqa: F401
+        import dojo.tool_product.signals  # noqa: F401
 
 
 def get_model_fields_with_extra(model, extra_fields=()):

dojo/benchmark/signals.py

@@ -0,0 +1,14 @@
+import logging
+
+from django.db.models.signals import pre_delete
+from django.dispatch import receiver
+
+from dojo.models import Benchmark_Product
+from dojo.notes.helper import delete_related_notes
+
+logger = logging.getLogger(__name__)
+
+
+@receiver(pre_delete, sender=Benchmark_Product)
+def benchmark_product_pre_delete(sender, instance, **kwargs):
+    delete_related_notes(instance)

dojo/cred/signals.py

@@ -0,0 +1,14 @@
+import logging
+
+from django.db.models.signals import pre_delete
+from django.dispatch import receiver
+
+from dojo.models import Cred_User
+from dojo.notes.helper import delete_related_notes
+
+logger = logging.getLogger(__name__)
+
+
+@receiver(pre_delete, sender=Cred_User)
+def cred_user_pre_delete(sender, instance, **kwargs):
+    delete_related_notes(instance)

dojo/engagement/signals.py

@@ -1,12 +1,13 @@
 from auditlog.models import LogEntry
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
-from django.db.models.signals import post_delete, post_save, pre_save
+from django.db.models.signals import post_delete, post_save, pre_delete, pre_save
 from django.dispatch import receiver
 from django.urls import reverse
 from django.utils.translation import gettext as _
 
 from dojo.models import Engagement
+from dojo.notes.helper import delete_related_notes
 from dojo.notifications.helper import create_notification
 
 
@@ -55,3 +56,8 @@ def engagement_post_delete(sender, instance, using, origin, **kwargs):
                             url=reverse('view_product', args=(instance.product.id, )),
                             recipients=[instance.lead],
                             icon="exclamation-triangle")
+
+
+@receiver(pre_delete, sender=Engagement)
+def engagement_pre_delete(sender, instance, **kwargs):
+    delete_related_notes(instance)

dojo/finding/helper.py

@@ -23,6 +23,7 @@
     Vulnerability_Id,
     Vulnerability_Id_Template,
 )
+from dojo.notes.helper import delete_related_notes
 from dojo.utils import get_current_user, mass_model_updater, to_str_typed
 
 logger = logging.getLogger(__name__)
@@ -402,8 +403,8 @@ def finding_pre_delete(sender, instance, **kwargs):
     logger.debug('finding pre_delete: %d', instance.id)
     # this shouldn't be necessary as Django should remove any Many-To-Many entries automatically, might be a bug in Django?
     # https://code.djangoproject.com/ticket/154
-
     instance.found_by.clear()
+    delete_related_notes(instance)
 
 
 def finding_delete(instance, **kwargs):

dojo/notes/helper.py

@@ -0,0 +1,11 @@
+import logging
+
+logger = logging.getLogger(__name__)
+
+
+def delete_related_notes(obj):
+    if not hasattr(obj, 'notes'):
+        logger.warning(f"Attempted to delete notes from object type {type(obj)} without 'notes' attribute.")
+        return
+    logging.debug(f"Deleting {obj.notes.count()} notes for {type(obj).__name__} {obj.id}")
+    obj.notes.all().delete()

dojo/notes/signals.py

@@ -0,0 +1,18 @@
+import logging
+
+from django.db.models.signals import pre_delete
+from django.dispatch import receiver
+
+from dojo.models import Notes
+
+logger = logging.getLogger(__name__)
+
+
+def delete_note_history(note):
+    logging.debug(f"Deleting history for note {note.id}")
+    note.history.all().delete()
+
+
+@receiver(pre_delete, sender=Notes)
+def note_pre_delete(sender, instance, **kwargs):
+    delete_note_history(instance)

dojo/risk_acceptance/helper.py

@@ -98,9 +98,6 @@ def delete(eng, risk_acceptance):
     eng.risk_acceptance.remove(risk_acceptance)
     eng.save()
 
-    for note in risk_acceptance.notes.all():
-        note.delete()
-
     risk_acceptance.path.delete()
     risk_acceptance.delete()
 

dojo/risk_acceptance/signals.py

@@ -0,0 +1,14 @@
+import logging
+
+from django.db.models.signals import pre_delete
+from django.dispatch import receiver
+
+from dojo.models import Risk_Acceptance
+from dojo.notes.helper import delete_related_notes
+
+logger = logging.getLogger(__name__)
+
+
+@receiver(pre_delete, sender=Risk_Acceptance)
+def risk_acceptance_pre_delete(sender, instance, **kwargs):
+    delete_related_notes(instance)

dojo/test/signals.py

@@ -3,12 +3,13 @@
 from auditlog.models import LogEntry
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
-from django.db.models.signals import post_delete, pre_save
+from django.db.models.signals import post_delete, pre_delete, pre_save
 from django.dispatch import receiver
 from django.urls import reverse
 from django.utils.translation import gettext as _
 
 from dojo.models import Finding, Test
+from dojo.notes.helper import delete_related_notes
 from dojo.notifications.helper import create_notification
 
 
@@ -49,3 +50,8 @@ def update_found_by_for_findings(sender, instance, **kwargs):
             for find in findings:
                 find.found_by.remove(old_test_type)
                 find.found_by.add(new_test_type)
+
+
+@receiver(pre_delete, sender=Test)
+def test_pre_delete(sender, instance, **kwargs):
+    delete_related_notes(instance)