remarkbox/lib/notify.py

@@ -151,6 +151,37 @@ def get_email_notifications(dbsession, frequency):
     return notification_dict
 
 
+def filter_orphaned_notifications(notification_dict):
+    """
+    Filter out notifications with null node_event from the notification dictionary.
+
+    Args:
+        notification_dict: Dictionary mapping user_id to list of notifications
+
+    Returns:
+        dict: Filtered notification dictionary with orphaned notifications removed
+    """
+    filtered_dict = {}
+    total_orphaned = 0
+
+    for user_id, notifications in notification_dict.items():
+        valid_notifications = []
+        for notification in notifications:
+            if notification.node_event is None:
+                total_orphaned += 1
+                log.warning(f"Skipping orphaned notification {notification.id} for user {user_id}")
+            else:
+                valid_notifications.append(notification)
+
+        if valid_notifications:
+            filtered_dict[user_id] = valid_notifications
+
+    if total_orphaned > 0:
+        log.warning(f"Filtered out {total_orphaned} orphaned notifications")
+
+    return filtered_dict
+
+
 def deliver_scheduled_notifications(request=None):
     from datetime import datetime
     from pyramid.scripting import prepare
@@ -160,12 +191,14 @@ def deliver_scheduled_notifications(request=None):
         request = env["request"]
 
         notification_dict = get_email_notifications(request.dbsession, "daily")
-        send_digest_notifications(request, notification_dict, "daily")
+        filtered_dict = filter_orphaned_notifications(notification_dict)
+        send_digest_notifications(request, filtered_dict, "daily")
 
         # Send weekly on Monday.
         if datetime.today().weekday() == 0:
             notification_dict = get_email_notifications(request.dbsession, "weekly")
-            send_digest_notifications(request, notification_dict, "weekly")
+            filtered_dict = filter_orphaned_notifications(notification_dict)
+            send_digest_notifications(request, filtered_dict, "weekly")
     
 
 def send_immediate_notifications(request, notifications):
@@ -210,10 +243,14 @@ def group_notifications_by_root(notifications):
     """
     group notifications by root node, where the key is
     the root node and the value is a list of notification objects.
+    Skips orphaned notifications (where node_event is None).
     """
     groups = defaultdict(list)
     for notification in notifications:
-        groups[notification.node_event.node.root].append(notification)
+        if notification.node_event is not None:
+            groups[notification.node_event.node.root].append(notification)
+        else:
+            log.warning(f"Skipping orphaned notification {notification.id} in group_notifications_by_root")
     return groups
 
 

remarkbox/scripts/delete_disabled_nodes.py

@@ -4,6 +4,7 @@ import transaction
 import logging
 from pyramid.paster import bootstrap, setup_logging
 from ..models import Node, get_tm_session, now_timestamp
+from ..models.notification import NodeEventNotification
 
 from . import base_parser
 
@@ -29,10 +30,40 @@ def is_node_anonymized(node):
     )
 
 
+def cleanup_orphaned_notifications(dbsession):
+    """
+    Clean up any existing orphaned NodeEventNotification records.
+    
+    Args:
+        dbsession: Database session
+        
+    Returns:
+        int: Number of orphaned notifications cleaned up
+    """
+    # Find notifications with null node_event references
+    orphaned = (
+        dbsession.query(NodeEventNotification)
+        .filter(NodeEventNotification.node_event_id.is_(None))
+        .all()
+    )
+    
+    count = len(orphaned)
+    if count > 0:
+        print(f"Found {count} orphaned notifications, cleaning them up...")
+        for notification in orphaned:
+            dbsession.delete(notification)
+        print(f"Cleaned up {count} orphaned notifications")
+    else:
+        print("No orphaned notifications found")
+    
+    return count
+
+
 def delete_disabled_nodes(request):
     """
     Delete disabled leaf nodes and anonymize disabled parent nodes while preserving children.
     Skips re-anonymizing already anonymized parent nodes.
+    Also cleans up any existing orphaned notifications.
 
     Args:
         request: Pyramid request object with transaction manager
@@ -46,6 +77,10 @@ def delete_disabled_nodes(request):
             request.registry["dbsession_factory"], transaction.manager
         )
 
+        # First, clean up any existing orphaned notifications
+        print("Checking for orphaned notifications...")
+        orphaned_count = cleanup_orphaned_notifications(dbsession)
+
         # Fetch all disabled nodes in one query
         disabled_nodes = dbsession.query(Node).filter(Node.disabled == True).all()
         print(f"Found {len(disabled_nodes)} disabled nodes.")
@@ -92,8 +127,16 @@ def delete_disabled_nodes(request):
                 anonymized_count += 1
             else:
                 # Delete leaf node and its related data
-                # Handle events deletion
+                # Handle events deletion and cleanup associated notifications
                 for event in node.events:
+                    # Delete all notifications associated with this event
+                    notifications = dbsession.query(NodeEventNotification).filter(
+                        NodeEventNotification.node_event_id == event.id
+                    ).all()
+                    for notification in notifications:
+                        dbsession.delete(notification)
+                    
+                    # Delete the event itself
                     dbsession.delete(event)
 
                 # Handle watchers deletion with a loop
@@ -123,7 +166,7 @@ def delete_disabled_nodes(request):
 
         # Print summary of operations
         print(
-            f"Summary: Deleted {deleted_count} leaf nodes, anonymized {anonymized_count} parent nodes with children, skipped {skipped_count} already anonymized nodes"
+            f"Summary: Cleaned up {orphaned_count} orphaned notifications, deleted {deleted_count} leaf nodes, anonymized {anonymized_count} parent nodes with children, skipped {skipped_count} already anonymized nodes"
         )
 
         if remaining_disabled_nodes > 0:
@@ -150,7 +193,7 @@ def main():
     """
     # Set up argument parser with description
     parser = base_parser(
-        "Delete disabled leaf nodes and anonymize disabled parent nodes with children."
+        "Delete disabled leaf nodes, anonymize disabled parent nodes with children, and clean up orphaned notifications."
     )
     args = parser.parse_args()
 

remarkbox/scripts/send_node_digest_notifications.py

@@ -14,7 +14,7 @@ import logging
 import transaction
 from pyramid.paster import bootstrap, setup_logging
 
-from ..lib.notify import deliver_scheduled_notifications
+from ..lib.notify import get_email_notifications, send_digest_notifications, filter_orphaned_notifications
 from ..models import get_tm_session
 from ..models.notification import NodeEventNotification
 from ..models.meta import now_timestamp
@@ -55,6 +55,32 @@ def should_have_been_sent(notification):
     return False
 
 
+def safe_deliver_scheduled_notifications(request, dbsession):
+    """
+    Deliver scheduled notifications with protection against orphaned notifications.
+    """
+    from datetime import datetime
+
+    # Temporarily replace request.dbsession with our transaction-managed session
+    old_dbsession = request.dbsession
+    request.dbsession = dbsession
+
+    try:
+        # Get daily notifications and filter out orphaned ones
+        notification_dict = get_email_notifications(dbsession, "daily")
+        filtered_dict = filter_orphaned_notifications(notification_dict)
+        send_digest_notifications(request, filtered_dict, "daily")
+
+        # Send weekly on Monday
+        if datetime.today().weekday() == 0:
+            notification_dict = get_email_notifications(dbsession, "weekly")
+            filtered_dict = filter_orphaned_notifications(notification_dict)
+            send_digest_notifications(request, filtered_dict, "weekly")
+    finally:
+        # Restore original dbsession
+        request.dbsession = old_dbsession
+
+
 def get_unsent_notification_count(request):
     """
     Get count of unsent notifications using a fresh database session.
@@ -77,6 +103,7 @@ def get_unsent_notification_count(request):
 def mark_ready_notifications_as_sent(request):
     """
     Mark notifications as sent if they were ready to be delivered based on frequency rules.
+    Only marks non-orphaned notifications (those with valid node_event).
 
     Args:
         request: Pyramid request object
@@ -88,36 +115,42 @@ def mark_ready_notifications_as_sent(request):
         dbsession = get_tm_session(
             request.registry["dbsession_factory"], transaction.manager
         )
-        
+
         # Get all unsent notifications
         unsent_notifications = dbsession.query(NodeEventNotification).filter(
             NodeEventNotification.sent == False
         ).all()
-        
+
         total_unsent = len(unsent_notifications)
-        
+
         if total_unsent == 0:
             return total_unsent, 0, 0
-        
+
         # Only mark notifications that should have been sent based on frequency
+        # and are not orphaned (have valid node_event)
         ready_notification_ids = [
-            n.id for n in unsent_notifications 
-            if should_have_been_sent(n)
+            n.id for n in unsent_notifications
+            if should_have_been_sent(n) and n.node_event is not None
         ]
-        
+
+        # Count orphaned notifications separately
+        orphaned_count = sum(1 for n in unsent_notifications if n.node_event is None)
+        if orphaned_count > 0:
+            log.warning(f"Found {orphaned_count} orphaned notifications that will not be marked as sent")
+
         marked_as_sent = 0
         if ready_notification_ids:
             marked_as_sent = dbsession.query(NodeEventNotification).filter(
                 NodeEventNotification.id.in_(ready_notification_ids)
             ).update({
-                'sent': True, 
+                'sent': True,
                 'updated_timestamp': now_timestamp()
             }, synchronize_session=False)
-            
+
             # Ensure changes are persisted
             dbsession.flush()
             transaction.commit()
-        
+
         left_for_later = total_unsent - marked_as_sent
         return total_unsent, marked_as_sent, left_for_later
 
@@ -142,10 +175,14 @@ def main():
                 log.info("No notifications to process")
                 return
             
-            # Deliver scheduled notifications
+            # Deliver scheduled notifications with orphaned notification protection
             log.info("Delivering scheduled notifications...")
-            deliver_scheduled_notifications(request)
-            transaction.commit()
+            with transaction.manager:
+                dbsession = get_tm_session(
+                    request.registry["dbsession_factory"], transaction.manager
+                )
+                safe_deliver_scheduled_notifications(request, dbsession)
+                transaction.commit()
             
             # Check final state and mark ready notifications as sent
             log.info("Checking for notifications that need to be marked as sent...")