From d6fa608cd1f047bfd039bbce64cc304e6e28054d Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Tue, 10 Mar 2026 17:10:34 -0600 Subject: [PATCH 1/5] Add unique constraint for system notifications and deduplicate existing entries --- .../0262_notifications_system_unique.py | 42 +++++++++++++++++++ dojo/models.py | 5 +++ dojo/notifications/helper.py | 8 ++-- dojo/notifications/views.py | 26 ++++++------ 4 files changed, 64 insertions(+), 17 deletions(-) create mode 100644 dojo/db_migrations/0262_notifications_system_unique.py diff --git a/dojo/db_migrations/0262_notifications_system_unique.py b/dojo/db_migrations/0262_notifications_system_unique.py new file mode 100644 index 00000000000..1a341bce986 --- /dev/null +++ b/dojo/db_migrations/0262_notifications_system_unique.py @@ -0,0 +1,42 @@ +from django.db import migrations, models + + +def deduplicate_system_notifications(apps, schema_editor): + """Remove duplicate Notification rows where user and product are both NULL. + + Keeps the oldest row (lowest pk) for each template value and deletes the rest. + """ + Notifications = apps.get_model("dojo", "Notifications") + # Handle both template=True and template=False system notifications + for template_val in [True, False]: + dupes = ( + Notifications.objects + .filter(user__isnull=True, product__isnull=True, template=template_val) + .order_by("pk") + ) + ids = list(dupes.values_list("pk", flat=True)) + if len(ids) > 1: + # Keep the first, delete the rest + Notifications.objects.filter(pk__in=ids[1:]).delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ("dojo", "0261_remove_url_insert_insert_remove_url_update_update_and_more"), + ] + + operations = [ + migrations.RunPython( + deduplicate_system_notifications, + reverse_code=migrations.RunPython.noop, + ), + migrations.AddConstraint( + model_name="notifications", + constraint=models.UniqueConstraint( + condition=models.Q(user__isnull=True, product__isnull=True), + fields=("template",), + name="notifications_system_unique", + ), + ), + ] diff --git a/dojo/models.py b/dojo/models.py index b5a58140045..dc5f3060463 100644 --- a/dojo/models.py +++ b/dojo/models.py @@ -4326,6 +4326,11 @@ class Notifications(models.Model): class Meta: constraints = [ models.UniqueConstraint(fields=["user", "product"], name="notifications_user_product"), + models.UniqueConstraint( + fields=["template"], + condition=models.Q(user__isnull=True, product__isnull=True), + name="notifications_system_unique", + ), ] indexes = [ models.Index(fields=["user", "product"]), diff --git a/dojo/notifications/helper.py b/dojo/notifications/helper.py index 7318bae91af..2acd41dede6 100644 --- a/dojo/notifications/helper.py +++ b/dojo/notifications/helper.py @@ -117,10 +117,10 @@ def __init__( def _get_notifications_object(self) -> Notifications: """Set the system Notifications object on the class.""" - try: - return Notifications.objects.get(user=None, template=False) - except Exception: - return Notifications() + notifications, _ = Notifications.objects.get_or_create( + user=None, product=None, template=False, + ) + return notifications def _get_system_settings(self) -> System_Settings: """Set the system settings object in the class.""" diff --git a/dojo/notifications/views.py b/dojo/notifications/views.py index 3d96c4ab5e5..323a8b0ccf2 100644 --- a/dojo/notifications/views.py +++ b/dojo/notifications/views.py @@ -3,6 +3,7 @@ import requests from django.contrib import messages from django.core.exceptions import PermissionDenied +from django.db import transaction from django.http import Http404, HttpRequest, HttpResponseRedirect from django.shortcuts import get_object_or_404, render from django.urls import reverse @@ -19,11 +20,10 @@ class SystemNotificationsView(View): def get_notifications(self, request: HttpRequest): - try: - notifications = Notifications.objects.get(user=None, product__isnull=True, template=False) - except Notifications.DoesNotExist: - notifications = Notifications(user=None, template=False) - + with transaction.atomic(): + notifications, _ = Notifications.objects.get_or_create( + user=None, product=None, template=False, + ) return notifications def check_user_permissions(self, request: HttpRequest): @@ -97,10 +97,10 @@ def post(self, request: HttpRequest): class PersonalNotificationsView(SystemNotificationsView): def get_notifications(self, request: HttpRequest): - try: - notifications = Notifications.objects.get(user=request.user, product__isnull=True) - except Notifications.DoesNotExist: - notifications = Notifications(user=request.user) + with transaction.atomic(): + notifications, _ = Notifications.objects.get_or_create( + user=request.user, product=None, template=False, + ) return notifications def check_user_permissions(self, request: HttpRequest): @@ -116,10 +116,10 @@ def set_breadcrumbs(self, request: HttpRequest): class TemplateNotificationsView(SystemNotificationsView): def get_notifications(self, request: HttpRequest): - try: - notifications = Notifications.objects.get(template=True) - except Notifications.DoesNotExist: - notifications = Notifications(user=None, template=True) + with transaction.atomic(): + notifications, _ = Notifications.objects.get_or_create( + user=None, product=None, template=True, + ) return notifications def get_scope(self): From 4b71748e989329fcf4c15a4ea05397e03641eebb Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Tue, 10 Mar 2026 17:35:34 -0600 Subject: [PATCH 2/5] Handle multiple system notifications gracefully by logging a warning and deleting duplicates --- dojo/notifications/helper.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/dojo/notifications/helper.py b/dojo/notifications/helper.py index 2acd41dede6..610993cd3d7 100644 --- a/dojo/notifications/helper.py +++ b/dojo/notifications/helper.py @@ -117,9 +117,25 @@ def __init__( def _get_notifications_object(self) -> Notifications: """Set the system Notifications object on the class.""" - notifications, _ = Notifications.objects.get_or_create( - user=None, product=None, template=False, - ) + try: + notifications, _ = Notifications.objects.get_or_create( + user=None, product=None, template=False, + ) + except Notifications.MultipleObjectsReturned: + notifications = Notifications.objects.filter( + user=None, + product=None, + template=False, + ).first() + logger.warning( + "Multiple system notifications objects found, using the first one with id %s. Cleaning up the duplicate...", + notifications.id, + ) + Notifications.objects.filter( + user=None, + product=None, + template=False, + ).exclude(id=notifications.id).delete() return notifications def _get_system_settings(self) -> System_Settings: From 37e60662b619ee27e76e940b196d749576925c44 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Tue, 10 Mar 2026 17:38:21 -0600 Subject: [PATCH 3/5] Add unique constraint for notifications to prevent duplicates --- ...tifications_notifications_system_unique.py | 17 ++++++++ .../0262_notifications_system_unique.py | 42 ------------------- dojo/db_migrations/max_migration.txt | 2 +- 3 files changed, 18 insertions(+), 43 deletions(-) create mode 100644 dojo/db_migrations/0262_notifications_notifications_system_unique.py delete mode 100644 dojo/db_migrations/0262_notifications_system_unique.py diff --git a/dojo/db_migrations/0262_notifications_notifications_system_unique.py b/dojo/db_migrations/0262_notifications_notifications_system_unique.py new file mode 100644 index 00000000000..2c09e2efdce --- /dev/null +++ b/dojo/db_migrations/0262_notifications_notifications_system_unique.py @@ -0,0 +1,17 @@ +# Generated by Django 5.2.12 on 2026-03-10 23:37 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('dojo', '0261_remove_url_insert_insert_remove_url_update_update_and_more'), + ] + + operations = [ + migrations.AddConstraint( + model_name='notifications', + constraint=models.UniqueConstraint(condition=models.Q(('product__isnull', True), ('user__isnull', True)), fields=('template',), name='notifications_system_unique'), + ), + ] diff --git a/dojo/db_migrations/0262_notifications_system_unique.py b/dojo/db_migrations/0262_notifications_system_unique.py deleted file mode 100644 index 1a341bce986..00000000000 --- a/dojo/db_migrations/0262_notifications_system_unique.py +++ /dev/null @@ -1,42 +0,0 @@ -from django.db import migrations, models - - -def deduplicate_system_notifications(apps, schema_editor): - """Remove duplicate Notification rows where user and product are both NULL. - - Keeps the oldest row (lowest pk) for each template value and deletes the rest. - """ - Notifications = apps.get_model("dojo", "Notifications") - # Handle both template=True and template=False system notifications - for template_val in [True, False]: - dupes = ( - Notifications.objects - .filter(user__isnull=True, product__isnull=True, template=template_val) - .order_by("pk") - ) - ids = list(dupes.values_list("pk", flat=True)) - if len(ids) > 1: - # Keep the first, delete the rest - Notifications.objects.filter(pk__in=ids[1:]).delete() - - -class Migration(migrations.Migration): - - dependencies = [ - ("dojo", "0261_remove_url_insert_insert_remove_url_update_update_and_more"), - ] - - operations = [ - migrations.RunPython( - deduplicate_system_notifications, - reverse_code=migrations.RunPython.noop, - ), - migrations.AddConstraint( - model_name="notifications", - constraint=models.UniqueConstraint( - condition=models.Q(user__isnull=True, product__isnull=True), - fields=("template",), - name="notifications_system_unique", - ), - ), - ] diff --git a/dojo/db_migrations/max_migration.txt b/dojo/db_migrations/max_migration.txt index 5b015714f08..54f4b9cb6d2 100644 --- a/dojo/db_migrations/max_migration.txt +++ b/dojo/db_migrations/max_migration.txt @@ -1 +1 @@ -0261_remove_url_insert_insert_remove_url_update_update_and_more +0262_notifications_notifications_system_unique From 8202059fc59ee19dfdd012869b86cbf6584c19c9 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Tue, 10 Mar 2026 18:48:16 -0600 Subject: [PATCH 4/5] Remove unique constraint for system notifications to simplify notification handling --- ...notifications_notifications_system_unique.py | 17 ----------------- dojo/db_migrations/max_migration.txt | 2 +- dojo/models.py | 5 ----- 3 files changed, 1 insertion(+), 23 deletions(-) delete mode 100644 dojo/db_migrations/0262_notifications_notifications_system_unique.py diff --git a/dojo/db_migrations/0262_notifications_notifications_system_unique.py b/dojo/db_migrations/0262_notifications_notifications_system_unique.py deleted file mode 100644 index 2c09e2efdce..00000000000 --- a/dojo/db_migrations/0262_notifications_notifications_system_unique.py +++ /dev/null @@ -1,17 +0,0 @@ -# Generated by Django 5.2.12 on 2026-03-10 23:37 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('dojo', '0261_remove_url_insert_insert_remove_url_update_update_and_more'), - ] - - operations = [ - migrations.AddConstraint( - model_name='notifications', - constraint=models.UniqueConstraint(condition=models.Q(('product__isnull', True), ('user__isnull', True)), fields=('template',), name='notifications_system_unique'), - ), - ] diff --git a/dojo/db_migrations/max_migration.txt b/dojo/db_migrations/max_migration.txt index 54f4b9cb6d2..d49ec6b6fcb 100644 --- a/dojo/db_migrations/max_migration.txt +++ b/dojo/db_migrations/max_migration.txt @@ -1 +1 @@ -0262_notifications_notifications_system_unique +0261_remove_url_insert_insert_remove_url_update_update_and_more \ No newline at end of file diff --git a/dojo/models.py b/dojo/models.py index dc5f3060463..b5a58140045 100644 --- a/dojo/models.py +++ b/dojo/models.py @@ -4326,11 +4326,6 @@ class Notifications(models.Model): class Meta: constraints = [ models.UniqueConstraint(fields=["user", "product"], name="notifications_user_product"), - models.UniqueConstraint( - fields=["template"], - condition=models.Q(user__isnull=True, product__isnull=True), - name="notifications_system_unique", - ), ] indexes = [ models.Index(fields=["user", "product"]), From ec7d92b7891a136b6a69f851cbb4801f5649071f Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Tue, 10 Mar 2026 20:18:13 -0600 Subject: [PATCH 5/5] Extra logging statement leads to assertion error --- unittests/test_notifications.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/unittests/test_notifications.py b/unittests/test_notifications.py index 2e897702fc4..065f488847b 100644 --- a/unittests/test_notifications.py +++ b/unittests/test_notifications.py @@ -578,7 +578,7 @@ def test_missing_system_webhook(self): with self.assertLogs("dojo.notifications.helper", level="INFO") as cm: manager = WebhookNotificationManger() manager.send_webhooks_notification(event="dummy") - self.assertIn("URLs for Webhooks not configured: skipping system notification", cm.output[0]) + self.assertIn("URLs for Webhooks not configured: skipping system notification", cm.output[1]) def test_missing_personal_webhook(self): # test data contains 2 entries but we need to test missing definition @@ -586,7 +586,7 @@ def test_missing_personal_webhook(self): with self.assertLogs("dojo.notifications.helper", level="INFO") as cm: manager = WebhookNotificationManger() manager.send_webhooks_notification(event="dummy", user=Dojo_User.objects.get(username="admin")) - self.assertIn("URLs for Webhooks not configured for user '(admin)': skipping user notification", cm.output[0]) + self.assertIn("URLs for Webhooks not configured for user '(admin)': skipping user notification", cm.output[1]) def test_system_webhook_inactive(self): self.sys_wh.status = Notification_Webhooks.Status.STATUS_INACTIVE_PERMANENT @@ -594,7 +594,7 @@ def test_system_webhook_inactive(self): with self.assertLogs("dojo.notifications.helper", level="INFO") as cm: manager = WebhookNotificationManger() manager.send_webhooks_notification(event="dummy") - self.assertIn("URL for Webhook 'My webhook endpoint' is not active: Permanently inactive (inactive_permanent)", cm.output[0]) + self.assertIn("URL for Webhook 'My webhook endpoint' is not active: Permanently inactive (inactive_permanent)", cm.output[1]) def test_system_webhook_sucessful(self): with self.assertLogs("dojo.notifications.helper", level="DEBUG") as cm: