fix: Improve performance by not always prefetching channels and target

This fixed performance for the common use case where people don't use Notification.target to dynamically generate the notification text. By always prefetching the `target` field, it was a performance penalty for everyone. Now you need to explicitly prefetch the target field yourself. See performance.md for more information.

Always prefetching the channels relationship didn't make sense since users don't normally display these fields.

Author: kevinrenskers

docs/performance.md

@@ -4,9 +4,14 @@
 
 The `target` field is a GenericForeignKey that can point to any Django model instance. While convenient, accessing targets requires careful consideration for performance.
 
-When using Django 5.0+, this library automatically includes `.prefetch_related("target")` when using the standard query methods. This efficiently fetches target objects, but only the _direct_ targets - accessing relationships _through_ the target will still cause additional queries.
+By default, the library does **not** prefetch targets to avoid unnecessary queries when they're not needed. If you need to access the target field, you should explicitly prefetch it:
 
-_On Django 4.2, you'll need to manually deal with prefetching the `target` relationship._
+```python
+# Add target prefetch when needed
+notifications = get_notifications(user).prefetch_related("target")
+```
+
+Note that this only fetches the _direct_ targets - accessing relationships _through_ the target will still cause additional queries.
 
 Consider this problematic example that will cause N+1 queries:
 
@@ -37,10 +42,10 @@ class CommentNotificationType(NotificationType):
         return f'{actor_name} commented on your article "{article.title}": "{comment_text}"'
 ```
 
-When displaying a list of 10 notifications, this will execute:
+When displaying a list of 10 notifications, with `.prefetch_related("target")` applied, this will execute:
 
 - 1 query for the notifications
-- 1 query for the target comments (Django 5.0+ only, automatically prefetched)
+- 1 query for the target comments
 - 10 queries for the articles (N+1 problem!)
 
 #### Solution 1: store data in the notification
@@ -59,15 +64,14 @@ send_notification(
 )
 ```
 
-However, this only works if you don’t need to dynamically generate the text - for example to make sure the text is always up to date, or to deal with internationalization.
+However, this only works if you don’t need to dynamically generate the text - for example to make sure the text is always up to date, or to deal with [internationalization](https://github.com/loopwerk/django-generic-notifications/blob/main/docs/multilingual.md).
 
 #### Solution 2: prefetch deeper relationships
 
 If you must access relationships through the target, you can prefetch them:
 
 ```python
-# On Django 5.0+ the library already prefetches targets,
-# but you need to add deeper relationships yourself
+# Prefetch the article relationship through target
 notifications = get_notifications(user).prefetch_related(
     "target__article"  # This prevents the N+1 problem
 )

generic_notifications/models.py

@@ -1,4 +1,3 @@
-import django
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.contrib.contenttypes.fields import GenericForeignKey
@@ -18,13 +17,7 @@ class NotificationQuerySet(models.QuerySet):
 
     def prefetch(self):
         """Prefetch related objects"""
-        qs = self.select_related("recipient", "actor").prefetch_related("channels")
-
-        # Only add target prefetching on Django 5.0+ due to GenericForeignKey limitations
-        if django.VERSION >= (5, 0):
-            qs = qs.prefetch_related("target")
-
-        return qs
+        return self.select_related("recipient", "actor")
 
     def for_channel(self, channel: type[BaseChannel] = WebsiteChannel):
         """Filter notifications by channel"""

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "django-generic-notifications"
-version = "2.0.0"
+version = "2.0.1"
 description = "A flexible, multi-channel notification system for Django applications with built-in support for email digests, user preferences, and extensible delivery channels."
 authors = [
     {name = "Kevin Renskers", email = "kevin@loopwerk.io"},

tests/test_performance.py

@@ -25,7 +25,7 @@ def setUp(self):
 
     def test_get_notifications_queries(self):
         """Test the number of queries made by get_notifications"""
-        with self.assertNumQueries(2):  # 1 for notifications + 1 for channels prefetch
+        with self.assertNumQueries(1):
             notifications = get_notifications(self.user)
             # Force evaluation of the queryset
             list(notifications)
@@ -71,8 +71,8 @@ def test_notification_target_access_queries(self):
             )
 
         # First, evaluate the queryset
-        with self.assertNumQueries(3):  # 1 for notifications + 1 for channels prefetch + 1 for targets
-            notifications = get_notifications(self.user)
+        with self.assertNumQueries(2):  # 1 for notifications + 1 for targets
+            notifications = get_notifications(self.user).prefetch_related("target")
             notifications_list = list(notifications)
 
         # Test accessing target - should be 0 queries since we prefetch target
@@ -104,8 +104,8 @@ def test_notification_target_relationship_access(self):
             )
 
         # First, evaluate the queryset
-        with self.assertNumQueries(3):  # 1 for notifications + 1 for channels prefetch + 1 for targets
-            notifications = get_notifications(self.user)
+        with self.assertNumQueries(2):  # 1 for notifications + 1 for targets
+            notifications = get_notifications(self.user).prefetch_related("target")
             notifications_list = list(notifications)
 
         # Test accessing target.recipient - this WILL cause N+1 queries
@@ -138,7 +138,7 @@ def test_notification_target_relationship_preselect_access(self):
             )
 
         # First, evaluate the queryset
-        with self.assertNumQueries(4):  # 1 for notifications + 1 for channels + 1 for targets + 1 for target recipients
+        with self.assertNumQueries(3):  # 1 for notifications + 1 for targets + 1 for target recipients
             notifications = get_notifications(self.user).prefetch_related("target__recipient")
             notifications_list = list(notifications)
 
@@ -148,3 +148,29 @@ def test_notification_target_relationship_preselect_access(self):
             for notification in notifications_list:
                 if notification.target and hasattr(notification.target, "recipient"):
                     _ = notification.target.recipient.email
+
+    def test_notification_mixed_targets_queries(self):
+        """Test queries with heterogeneous notification.target"""
+        # Create notifications with targets
+        notification = create_notification_with_channels(
+            user=self.user,
+            actor=self.actor,
+            notification_type="test_notification",
+            subject="Test notification 1",
+            text="This is test notification 1",
+            target=self.actor,
+        )
+
+        create_notification_with_channels(
+            user=self.user,
+            actor=self.actor,
+            notification_type="test_notification",
+            subject="Test notification 2",
+            text="This is test notification 2",
+            target=notification,
+        )
+
+        # First, evaluate the queryset
+        with self.assertNumQueries(1):
+            notifications = get_notifications(self.user)
+            _ = list(notifications)