Index: openacs-4/packages/notifications/tcl/sweep-procs.tcl =================================================================== RCS file: /usr/local/cvsroot/openacs-4/packages/notifications/tcl/sweep-procs.tcl,v diff -u -r1.28 -r1.29 --- openacs-4/packages/notifications/tcl/sweep-procs.tcl 7 May 2018 14:55:20 -0000 1.28 +++ openacs-4/packages/notifications/tcl/sweep-procs.tcl 3 Sep 2024 15:37:39 -0000 1.29 @@ -10,15 +10,27 @@ namespace eval notification::sweep { - ad_proc -public cleanup_notifications {} { + ad_proc -private cleanup_notifications {} { Clean up the notifications that have been sent out (DRB: inefficiently...). } { - # LARS: - # Also sweep the dynamic notification requests that have been sent out - db_dml delete_dynamic_requests {} - # before the killing starts, remove invalid requests - foreach request_id [db_list select_invalid_request_ids {}] { + foreach request_id [db_list select_invalid_request_ids { + select request_id + from notification_requests + where + -- LARS + -- Also sweep the dynamic notification requests that have been sent out + (dynamic_p = 't' and + exists (select 1 + from notifications n, + notification_user_map num + where n.type_id = type_id + and n.object_id = object_id + and num.notification_id = n.notification_id + and num.user_id = user_id)) + + -- or not acs_permission.permission_p(object_id, user_id, 'read') + }] { notification::request::delete -request_id $request_id } @@ -32,18 +44,68 @@ } - ad_proc -public sweep_notifications { + ad_proc -private sweep_notifications { {-interval_id:required} {-batched_p 0} } { This sweeps for notifications in a particular interval } { - # Look for notifications joined against the requests they may match with the right interval_id - # order it by user_id - # make sure the users have not yet received this notification with outer join - # on the mapping table and a null check - set notifications [db_list_of_ns_sets select_notifications {}] + # + # Look for notifications joined against the requests they may + # match with the right interval_id order it by user_id. Make + # sure the users have not yet received this notification with + # outer join on the mapping table and a null check. + # + set notifications [db_list_of_ns_sets select_notifications { + select notifications.notification_id, + notif_subject, + notif_text, + notif_html, + file_ids, + notification_requests.user_id, + request_id, + notifications.type_id, + delivery_method_id, + response_id, + notif_date, + notif_user, + acs_permission.permission_p(notification_requests.object_id, notification_requests.user_id, 'read') as still_valid_p + from notifications + inner join notification_requests on (notifications.type_id = notification_requests.type_id + and notifications.object_id = notification_requests.object_id) + inner join acs_objects on (notification_requests.request_id = acs_objects.object_id) + left outer join notification_user_map on (notification_user_map.notification_id = notifications.notification_id + and notification_user_map.user_id = notification_requests.user_id) + where sent_date is null + and creation_date <= notif_date + and (notif_date is null or notif_date < current_timestamp) + and interval_id = :interval_id + order by notification_requests.user_id, notifications.type_id, notif_date + }] + foreach notif $notifications { + if {![ns_set get $notif still_valid_p]} { + # + # The user has lost permissions on the object, so + # delete this notification. This deletion was done + # before in the highly expensive query in + # "cleanup_notifications" + # + ns_log notice "delete notification [ns_set get $notif request_id]" \ + "for user_id [ns_set get $notif user_id]" \ + "since user has lost rights on object" + + notification::request::delete \ + -request_id [ns_set get $notif request_id] + # + # Remove this tuple from the notification list such we + # do not have to double-check for this. + # + set idx [lsearch $notifications $notif] + set notifications [lreplace $notifications $idx $idx] + } + } + if {$batched_p} { set prev_user_id 0 set prev_type_id 0 @@ -71,9 +133,10 @@ set user_id "" set type_id "" } - - # Check if we have a new user_id and type_id - # if so, batch up previous stuff and send it + # + # Check if we have a new user_id and type_id. If so, + # batch up previous stuff and send it. + # if {$notif eq "STOP" || $user_id != $prev_user_id || $type_id != $prev_type_id} { ns_log Debug "NOTIF-BATCHED: batching things up for $prev_user_id" @@ -137,13 +200,20 @@ } append summary_text "[ns_set get $notif notif_subject]\n" - append summary_html "
  • [ns_set get $notif notif_subject]
    • " + append summary_html \ + "
  • " \ + [ns_set get $notif notif_subject] \ + "
    • " + append batched_content_text \ + "[_ notifications.SUBJECT] [ns_set get $notif notif_subject]\n" \ + [ns_set get $notif notif_text] \ + "\n=====================\n" + append batched_content_html \ + "" \ + "[_ notifications.SUBJECT] [ns_set get $notif notif_subject]\n" \ + " $notif_html

    " - append batched_content_text "[_ notifications.SUBJECT] [ns_set get $notif notif_subject]\n[ns_set get $notif notif_text]\n=====================\n" - append batched_content_html "[_ notifications.SUBJECT] [ns_set get $notif notif_subject]\n $notif_html

    " - lappend batched_file_ids {*}[ns_set get $notif file_ids] - lappend list_of_notification_ids [ns_set get $notif notification_id] # Set the vars @@ -153,7 +223,9 @@ } } else { + # # Unbatched + # foreach notif $notifications { db_transaction { # Send it @@ -168,7 +240,7 @@ -reply_object_id [ns_set get $notif response_id] \ -delivery_method_id [ns_set get $notif delivery_method_id] - # Mark it as sent + # Mark it as sent notification::mark_sent \ -notification_id [ns_set get $notif notification_id] \ -user_id [ns_set get $notif user_id]