Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use actioncable to track indexing progress #3182

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
539 changes: 532 additions & 7 deletions app/assets/javascripts/spotlight/spotlight.esm.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion app/assets/javascripts/spotlight/spotlight.esm.js.map

Large diffs are not rendered by default.

539 changes: 532 additions & 7 deletions app/assets/javascripts/spotlight/spotlight.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion app/assets/javascripts/spotlight/spotlight.js.map

Large diffs are not rendered by default.

30 changes: 30 additions & 0 deletions app/components/spotlight/progress_bar_component.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<div data-behavior='progress-panel' data-exhibit-id="<%= exhibit.id %>" data-monitor-url="<%= helpers.actioncable? ? false : helpers.monitor_exhibit_bulk_updates_path(exhibit) %>">
<div class="card index-status border-info mb-3">
<h3 class="card-header bg-info text-white h4"><%= t("heading", scope: translation_field) %></h3>
<div class="card-body">
<p data-behavior='monitor-start'>
<span class="text-muted" data-behavior='date'></span> <%= t("begin_html", scope: translation_field) %>
</p>

<% if I18n.exists?("current_html", scope: translation_field) %>
<p data-behavior='monitor-current'>
<span class="text-muted" data-behavior='date'></span> <%= t("current_html", scope: translation_field) %>
</p>
<% end %>

<% if I18n.exists?("completed_html", scope: translation_field) %>
<p data-behavior='monitor-completed'>
<span class="text-muted" data-behavior='date'></span> <%= t("completed_html", scope: translation_field) %>
</p>
<% end %>

<p class="bg-warning" data-behavior='monitor-error' style='display:none;'>
<%= t("error", scope: translation_field) %>
</p>

<div class="progress">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. This is a bootstrap class: https://getbootstrap.com/docs/4.0/components/progress/. Changing this to it breaks the CSS.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the semantic element better for accessibility than a bootstrap component?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. I think if we are trying to solve the accessibility issue then we should open a new ticket. This is the progress bar that was used in app/views/spotlight/catalog/_reindex_progress_panel.html.erb and app/views/spotlight/bulk_updates/_progress_panel.html.erb

<div class="progress-bar progress-bar-striped active" role="progressbar" aria-valuenow="" aria-valuemin="0" aria-valuemax=""></div>
</div>
</div>
</div>
</div>
14 changes: 14 additions & 0 deletions app/components/spotlight/progress_bar_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# frozen_string_literal: true

module Spotlight
# Renders progress bar html which is updated by progress_monitor.js
class ProgressBarComponent < ViewComponent::Base
attr_reader :exhibit, :translation_field

def initialize(exhibit:, translation_field:)
@exhibit = exhibit
@translation_field = translation_field
super
end
end
end
17 changes: 17 additions & 0 deletions app/helpers/spotlight/actioncable_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# frozen_string_literal: true

module Spotlight
##
# General helper for checking and using ActionCable
module ActioncableHelper
def actioncable?
defined?(ActionCable.server) && ActionCable.server.config.cable.present?
end

def ws_broadcast(channel, data)
return unless actioncable?

ActionCable.server.broadcast channel, data
end
end
end
1 change: 1 addition & 0 deletions app/helpers/spotlight/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module ApplicationHelper
include MetaHelper
include CropHelper
include LanguagesHelper
include ActioncableHelper

##
# Give the application name a chance to include the exhibit title
Expand Down
25 changes: 18 additions & 7 deletions app/javascript/spotlight/admin/progress_monitor.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,31 @@
import consumer from "../channels/consumer"

export default class {
connect() {
var monitorElements = $('[data-behavior="progress-panel"]');
var defaultRefreshRate = 3000;
var panelContainer;
var pollers = [];

$(monitorElements).each(function() {
panelContainer = $(this);
panelContainer.hide();
var monitorUrl = panelContainer.data('monitorUrl');
var refreshRate = panelContainer.data('refreshRate') || defaultRefreshRate;
pollers.push(
setInterval(function() {
checkMonitorUrl(monitorUrl);
}, refreshRate)
);

if (monitorUrl){
var refreshRate = panelContainer.data('refreshRate') || 3000;
pollers.push(
setInterval(function() {
checkMonitorUrl(monitorUrl);
}, refreshRate)
);
} else {
consumer.subscriptions.create({ channel: "ProgressChannel"}, {
received(data) {
if (data.exhibit_id != panelContainer.data('exhibit-id')) return;
updateMonitorPanel(data);
}
});
}
});

// Clear the intervals on turbolink:click event (e.g. when the user navigates away from the page)
Expand Down
6 changes: 6 additions & 0 deletions app/javascript/spotlight/channels/consumer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Action Cable provides the framework to deal with WebSockets in Rails.
// You can generate new channels where WebSocket features live using the `bin/rails generate channel` command.

import { createConsumer } from "@rails/actioncable"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this file belongs in the engine. It should be in the host application. By doing that you can remove the @rollup/plugin-node-resolve

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are suggesting this goes in the install generator?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it already in the actioncabel generator?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Okay so you are suggesting I put the

  1. rails generate channel NameOfChannel into the install generator. Remove app/channels/application_cable/channel.rb, app/channels/application_cable/connection.rb, and app/javascript/channels/consumer.js.
  2. Either make the name of channel ProgressChanel and update the app/channels/progress_channel.rb to include the right stream in the generator or make it a random channel that doesn't get used

I am guessing for existing apps we are going to need to include instructions on how to install. I am not quite sure how this works with existing apps.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is the path I would use. Yes, we would need instructions for how to install this. It should include something like "Install and configure Redis and actioncable" (assuming we're not doing Solid + Rails 8).


export default createConsumer()
9 changes: 9 additions & 0 deletions app/jobs/spotlight/process_bulk_updates_csv_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,31 @@ module Spotlight
###
class ProcessBulkUpdatesCsvJob < Spotlight::ApplicationJob
include Spotlight::JobTracking
include ActioncableHelper
with_job_tracking(resource: ->(job) { job.arguments.first })

# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
def perform(exhibit, bulk_update)
errors = 0
header_converter = ->(header) { header } # Use raw header for columns (since they are configured)
csv_path = bulk_update.file.current_path
started_at = Time.zone.now

File.open(csv_path) do |f|
progress&.total = f.each_line.count(&:present?) - 1 # ignore the header

::CSV.table(f, header_converters: header_converter).each do |row|
process_row(exhibit, row)
progress&.increment
data = { exhibit_id: exhibit.id, finished: progress.progress == progress.total,
completed: progress.progress, total: progress.total, errors: errors,
updated_at: Time.zone.now, started_at: started_at }
ws_broadcast('progress_channel', data)
rescue StandardError => e
job_tracker.append_log_entry(type: :error, exhibit: exhibit, message: e.to_s)
errors += 1
data['errors'] = errors
ws_broadcast('progress_channel', data)
mark_job_as_failed!
end

Expand Down
9 changes: 9 additions & 0 deletions app/jobs/spotlight/reindex_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module Spotlight
# Reindex the given resources or exhibits
class ReindexJob < Spotlight::ApplicationJob
include Spotlight::JobTracking
include ActioncableHelper
with_job_tracking(resource: ->(job) { job.exhibit })

include Spotlight::LimitConcurrency
Expand Down Expand Up @@ -38,12 +39,20 @@ def perform(exhibit_or_resources, start: nil, finish: nil, **)
errors += 1
end

started_at = Time.zone.now

resource_list(exhibit_or_resources, start: start, finish: finish).each do |resource|
resource.reindex(touch: false, commit: false, job_tracker: job_tracker, additional_data: job_data, on_error: error_handler) do |*|
progress&.increment
data = { exhibit_id: resource.exhibit_id, finished: progress.progress == progress.total,
completed: progress.progress, total: progress.total, errors: errors,
updated_at: Time.zone.now, started_at: started_at, other: resource }
ws_broadcast('progress_channel', data)
end
rescue StandardError => e
error_handler.call(Struct.new(:source).new(resource), e, nil)
data['errors'] = errors
ws_broadcast('progress_channel', data)
end

job_tracker.append_log_entry(
Expand Down
19 changes: 0 additions & 19 deletions app/views/spotlight/bulk_updates/_progress_panel.html.erb

This file was deleted.

4 changes: 1 addition & 3 deletions app/views/spotlight/bulk_updates/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@

<%= curation_page_title t(:".header") %>

<div data-behavior='progress-panel' data-monitor-url="<%= monitor_exhibit_bulk_updates_path(current_exhibit) %>">
<%= render 'progress_panel' %>
</div>
<%= render Spotlight::ProgressBarComponent.new(exhibit: current_exhibit, translation_field: 'spotlight.bulk_updates.progress_panel') %>

<div role="tabpanel">
<ul class="nav nav-tabs" role="tablist">
Expand Down
4 changes: 1 addition & 3 deletions app/views/spotlight/catalog/_admin_header.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
<% if can? :manage, Spotlight::Resource.new(exhibit: current_exhibit) %>
<div data-behavior='progress-panel' data-monitor-url="<%= monitor_exhibit_resources_path(current_exhibit) %>">
<%= render 'reindex_progress_panel' %>
</div>
<%= render Spotlight::ProgressBarComponent.new(exhibit: current_exhibit, translation_field: 'spotlight.catalog.reindex_progress_panel') %>
<div class="add-items-nav clearfix">
<div class="float-right float-end">
<%= link_to t('.reindex'), reindex_all_exhibit_resources_path(current_exhibit),
Expand Down
19 changes: 0 additions & 19 deletions app/views/spotlight/catalog/_reindex_progress_panel.html.erb

This file was deleted.

5 changes: 5 additions & 0 deletions lib/generators/spotlight/install_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ def add_helper
end
end

def add_actioncable
system 'rails generate channel ProgressChannel'
gsub_file 'app/channels/progress_channel.rb', '# stream_from "some_channel"', 'stream_from "progress_channel"'
end

def add_model_mixin
if File.exist? File.expand_path('app/models/solr_document.rb', destination_root)
inject_into_file 'app/models/solr_document.rb', after: 'include Blacklight::Solr::Document' do
Expand Down
Loading
Loading