Skip to content

Commit

Permalink
Fixes time out issue on records with very large number of provenance …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
hectorcorrea authored Oct 31, 2024
1 parent f57814b commit 2c9e47c
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 14 deletions.
2 changes: 1 addition & 1 deletion app/decorators/work_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 13 additions & 7 deletions app/models/work.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion app/models/work_activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 8 additions & 5 deletions app/views/works/_work_activity_provenance.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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| %>
Expand All @@ -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>
Expand All @@ -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">
Expand Down
16 changes: 16 additions & 0 deletions lib/tasks/works.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2c9e47c

Please sign in to comment.