Skip to content

Commit

Permalink
Add Background processing and UI Forms for CSV exports (#198)
Browse files Browse the repository at this point in the history
This change adds the scaffolding to let an administrator:
1. Select 1-3 objects for a CSV export
2. Submit an Export job
3. Run the job in the background
4. View the results
5. Download the exported file

We expect to provide a different UX for selecting items for export,
so the submission form is very bare-bones but functional.

The status (show) page for exports need additional design but provides
enough functionality to download the exported CSV for testing.
  • Loading branch information
mark-dce authored Aug 29, 2022
1 parent dfc8b80 commit c3653bd
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 11 deletions.
20 changes: 18 additions & 2 deletions app/controllers/exports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,28 @@ def index
end

def new
super
redirect_to new_export_path
@job = Export.new
end

def show
super
add_breadcrumb "##{@job.id} - #{I18n.t('tenejo.admin.sidebar.exports')}", @job
end

def create
@job = Export.new(job_params.merge({ user: current_user, status: :submitted }))
respond_to do |format|
if @job.save
BatchExportJob.perform_later(@job)
format.html { redirect_to @job }
format.json { render :show, status: :created, location: @job }
end
end
end

private

def job_params
params.require(:export).permit(identifiers: [])
end
end
10 changes: 10 additions & 0 deletions app/jobs/batch_export_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# frozen_string_literal: true
class BatchExportJob < ApplicationJob
queue_as :default

def perform(export_job)
# probably not optimal, but enough until we get
# import hammered out
Tenejo::CsvExporter.new(export_job).run
end
end
20 changes: 16 additions & 4 deletions app/lib/tenejo/csv_exporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,34 @@

module Tenejo
class CsvExporter
EXPORT_HEADERS = ([:primary_identifier, :error, :class, :title] + Tenejo::CsvImporter.collection_attributes_to_copy.keys + Tenejo::CsvImporter.work_attributes_to_copy.keys).uniq
EXCLUDE_FROM_EXPORT = [:date_modified, :identifier, :label, :arkivo_checksum, :state].freeze
HEADER_ROW = (([:primary_identifier, :error, :class, :title] \
+ Tenejo::CsvImporter.collection_attributes_to_copy.keys \
+ Tenejo::CsvImporter.work_attributes_to_copy.keys).uniq \
- EXCLUDE_FROM_EXPORT).freeze

def initialize(export_job)
@export = export_job
end

def run
@export.status = :in_progress
@export.save
output = CSV.generate(encoding: 'UTF-8', write_headers: true) do |csv|
csv << EXPORT_HEADERS # Header row
csv << HEADER_ROW
csv << CSV::Row.new([:primary_identifier, :error], ["missing", "No identifiers provided"]) if @export.identifiers.empty?
@export.identifiers.each do |id|
csv << serialize(id)
end
end

# TODO: remove this after refactoring Tenejo metadata to rename primary_identifier and identifer
output.gsub!('primary_identifier,error,class,', 'identifier,error,object type,')

@export.manifest.attach(io: StringIO.new(output), filename: export_name, content_type: 'text/csv')
@export.status = :complete
@export.completed_at = Time.current
@export.save
end

private
Expand All @@ -37,8 +49,8 @@ def export_name
def serialize(id)
obj = ActiveFedora::Base.where(primary_identifier_ssi: id).last
return CSV::Row.new([:primary_identifier, :error], [id, "No match for identifier"]) unless obj
values = EXPORT_HEADERS.map { |attr| pack_field(obj.try(attr)) }
CSV::Row.new(EXPORT_HEADERS, values)
values = HEADER_ROW.map { |attr| pack_field(obj.try(attr)) }
CSV::Row.new(HEADER_ROW, values)
end

# Handle multi-value fields and normalize empty fields regardless of underlying class
Expand Down
32 changes: 32 additions & 0 deletions app/views/exports/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<h1>New <%= @job.class %></h1>

<%= form_with(model: @job, local: true) do |form| %>
<% if @job.errors.any? %>
<div id="error_explanation">
<h2><%= pluralize(@job.errors.count, "error") %> prohibited this job from being saved:</h2>

<ul>
<% @job.errors.full_messages.each do |message| %>
<li><%= message %></li>
<% end %>
</ul>
</div>
<% end %>

<div class="field">
<%= form.label :identifiers %>
<%= form.text_field :identifiers, id: 'export_identifiers_0', value: @job.identifiers[0], multiple: true %>
</div>
<div class="field">
<%= form.label :identifiers %>
<%= form.text_field :identifiers, id: 'export_identifiers_1', value: @job.identifiers[1], multiple: true %>
</div>
<div class="field">
<%= form.label :identifiers %>
<%= form.text_field :identifiers, id: 'export_identifiers_2', value: @job.identifiers[2], multiple: true %>
</div>
<br/>
<div class="actions">
<%= form.submit 'Submit', type: 'submit' %>
</div>
<% end %>
10 changes: 5 additions & 5 deletions spec/lib/tenejo/csv_exporter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
export.save
described_class.new(export).run
rows = CSV.parse(export.manifest.download, headers: true)
expect(rows.first['primary_identifier']).to eq 'invalid_id'
expect(rows.first['identifier']).to eq 'invalid_id'
expect(rows.first['error']).to eq 'No match for identifier'
end

Expand All @@ -48,16 +48,16 @@
rows = CSV.parse(export.manifest.download, headers: true)

# Collection COL001
expect(rows[0]['primary_identifier']).to eq 'COL001'
expect(rows[0]['identifier']).to eq 'COL001'
expect(rows[0]['error']).to be_nil
expect(rows[0]['title']).to include 'Test collection'
expect(rows[0]['class']).to eq "Collection"
expect(rows[0]['object type']).to eq "Collection"

# Work WRK001
expect(rows[1]['primary_identifier']).to eq 'WRK001'
expect(rows[1]['identifier']).to eq 'WRK001'
expect(rows[1]['error']).to be_nil
expect(rows[1]['title']).to include 'Test work'
expect(rows[1]['class']).to eq "Work"
expect(rows[1]['object type']).to eq "Work"
end
end

Expand Down
53 changes: 53 additions & 0 deletions spec/requests/export_request_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# frozen_string_literal: true
require 'rails_helper'

RSpec.describe "/exports", type: :request do
let(:admin) { FactoryBot.create(:user, :admin) }
let(:export) { Export.new(user: admin) }

before do
sign_in admin
end

describe "GET /index" do
it "redirects to /jobs/index" do
get exports_path
expect(response).to redirect_to jobs_path
end
end

describe "GET /new" do
it "renders a successful response" do
get new_export_path
expect(response).to render_template('exports/new')
end
end

describe "GET /show" do
it "displays info for an export job" do
export.save!
get export_path export
expect(response).to render_template('exports/show')
end
end

describe "POST /create" do
it "creates a new export job" do
expect {
post exports_path, params: { export: { identifiers: [] } }
}.to change(Export, :count).by(1)
end

it "queues a new background job" do
ActiveJob::Base.queue_adapter = :test
expect {
post exports_path, params: { export: { identifiers: [] } }
} .to enqueue_job(BatchExportJob).with(Job.last).on_queue(:default)
end

it "redirects to the submitted export show view" do
post exports_path, params: { export: { identifiers: [] } }
expect(response).to redirect_to Export.last
end
end
end
41 changes: 41 additions & 0 deletions spec/routing/exports_routing_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# frozen_string_literal: true
require "rails_helper"

RSpec.describe ExportsController, type: :routing do
describe "routing" do
it "routes to #index" do
expect(get: "/exports").to route_to("exports#index")
end

it "routes to #new" do
expect(get: "/exports/new").to route_to("exports#new")
end

it "routes to #show" do
expect(get: "/exports/1").to route_to("exports#show", id: "1")
end

it "routes to #create" do
expect(post: "/exports").to route_to("exports#create")
end

# Preflight jobs are not editable after creation
context 'invalid routes' do
it "does not route to #edit" do
expect(get: "/exports/1/edit").not_to be_routable
end

it "does not route to #update via PUT" do
expect(put: "/exports/1").not_to be_routable
end

it "does not route to #update via PATCH" do
expect(patch: "/exports/1").not_to be_routable
end

it "does not route to #destroy" do
expect(delete: "/exports/1").not_to be_routable
end
end
end
end

0 comments on commit c3653bd

Please sign in to comment.