From 2c9e47cb976ac15e1e4a207b282dc98545b57a1e Mon Sep 17 00:00:00 2001
From: Hector Correa <hector@hectorcorrea.com>
Date: Thu, 31 Oct 2024 10:24:09 -0400
Subject: [PATCH] Fixes time out issue on records with very large number of
 provenance messages (#1982)

* First draft at addressing the issue of loading a work with a large number of messages in the provenance

* Batch updates when marking notifications as read. Display onlu the first 100 system notifications.

* Rubocop nitpicking
---
 app/decorators/work_decorator.rb              |  2 +-
 app/models/work.rb                            | 20 ++++++++++++-------
 app/models/work_activity.rb                   |  1 -
 .../works/_work_activity_provenance.html.erb  | 13 +++++++-----
 lib/tasks/works.rake                          | 16 +++++++++++++++
 5 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/app/decorators/work_decorator.rb b/app/decorators/work_decorator.rb
index 458b7b517..8a1b5b90c 100644
--- a/app/decorators/work_decorator.rb
+++ b/app/decorators/work_decorator.rb
@@ -10,7 +10,7 @@ class WorkDecorator
   def initialize(work, current_user)
     @work = work
     @current_user = current_user
-    @changes =  WorkActivity.changes_for_work(work.id)
+    @changes =  WorkActivity.changes_for_work(work.id).order(created_at: :asc)
     @messages = WorkActivity.messages_for_work(work.id).order(created_at: :desc)
     @can_curate = current_user&.can_admin?(group)
   end
diff --git a/app/models/work.rb b/app/models/work.rb
index 475c36602..281a0f4da 100644
--- a/app/models/work.rb
+++ b/app/models/work.rb
@@ -292,13 +292,19 @@ def new_notification_count_for_user(user_id)
   # In practice, the user_id is the id of the current user and therefore this method marks the current's user
   # notifications as read.
   def mark_new_notifications_as_read(user_id)
-    activities.each do |activity|
-      unread_notifications = WorkActivityNotification.where(user_id:, work_activity_id: activity.id, read_at: nil)
-      unread_notifications.each do |notification|
-        notification.read_at = Time.now.utc
-        notification.save
-      end
-    end
+    # Notice that we fetch and update the information in batches
+    # so that we don't issue individual SQL SELECT + SQL UPDATE
+    # for each notification.
+    #
+    # Rails batching information:
+    #   https://guides.rubyonrails.org/active_record_querying.html
+    #   https://api.rubyonrails.org/classes/ActiveRecord/Batches.html
+
+    # Disable this validation since we want to force a SQL UPDATE.
+    # rubocop:disable Rails/SkipsModelValidations
+    now_utc = Time.now.utc
+    WorkActivityNotification.joins(:work_activity).where("user_id=? and work_id=?", user_id, id).in_batches(of: 1000).update_all(read_at: now_utc)
+    # rubocop:enable Rails/SkipsModelValidations
   end
 
   def current_transition
diff --git a/app/models/work_activity.rb b/app/models/work_activity.rb
index 723f17438..1727d2c14 100644
--- a/app/models/work_activity.rb
+++ b/app/models/work_activity.rb
@@ -79,7 +79,6 @@ def users_referenced
 
   def created_by_user
     return nil unless created_by_user_id
-
     User.find(created_by_user_id)
   end
 
diff --git a/app/views/works/_work_activity_provenance.html.erb b/app/views/works/_work_activity_provenance.html.erb
index 569aed4b1..1509f7c96 100644
--- a/app/views/works/_work_activity_provenance.html.erb
+++ b/app/views/works/_work_activity_provenance.html.erb
@@ -4,12 +4,15 @@
     No activity
   <% end %>
   <ul class="beads">
-    <% @work_decorator.changes.sort_by(&:created_at).each do |activity| %>
+    <% @work_decorator.changes.take(100).each_with_index do |activity| %>
       <li class="activity-history-item">
         <%= activity.to_html.html_safe %>
       </li>
     <% end %>
   </ul>
+  <% if @work_decorator.changes.size > 100 %>
+    <p><i>There are <b><%= @work_decorator.changes.size - 100 %></b> more notifications, contact RDSS if you need to view them.</i></p>
+  <% end %>
 
   <% if can_add_provenance_note %>
     <%= form_with url: add_provenance_note_path(@work) do |f| %>
@@ -34,7 +37,7 @@
         <summary  class="remove-see-more">
         Change Label
         <span class="summary-detail">
-        The change label applies a brief descriptor to the provenance log that labels the applied action. 
+        The change label applies a brief descriptor to the provenance log that labels the applied action.
         </span>
         <title>Change Label</title>
         </head>
@@ -55,11 +58,11 @@
       <div class="field">
         <details>
           <summary  class="remove-see-more">
-          Note 
+          Note
           <span class="summary-detail">
-          The note to add to the change history. Markdown can be used. 
+          The note to add to the change history. Markdown can be used.
           </span>
-          
+
           </summary>
         </details>
         <input id="new-provenance-note" name="new-provenance-note">
diff --git a/lib/tasks/works.rake b/lib/tasks/works.rake
index 7791ebce5..c30584c6a 100644
--- a/lib/tasks/works.rake
+++ b/lib/tasks/works.rake
@@ -109,6 +109,22 @@ namespace :works do
     puts work_preservation.preserve!
   end
 
+  # Artificially add a lot of notifications to a work
+  # (Will be used to test https://github.com/pulibrary/pdc_describe/issues/1978 in staging )
+  task :big_provenance, [:work_id] => :environment do |_, args|
+    work_id = args[:work_id].to_i
+    user = User.find_by(uid: "hc8719")
+    datestamp = DateTime.now.to_s
+    (0..2000).each do |_i|
+      WorkActivity.add_work_activity(work_id, "SYSTEM #{datestamp} - #{rand(1_000_000)}", user.id, activity_type: WorkActivity::SYSTEM)
+    end
+    (0..40).each do |_i|
+      WorkActivity.add_work_activity(work_id, "MESSAGE #{datestamp} - #{rand(1_000_000)} @hc8719", user.id, activity_type: WorkActivity::MESSAGE)
+    end
+    work = Work.find(work_id)
+    puts work.activities.count
+  end
+
   desc "Clean up the duplicate globus and data_space files in a work from migration"
   task :migration_cleanup, [:work_id, :force] => :environment do |_, args|
     work_id = args[:work_id].to_i