Skip to content

Commit

Permalink
Create Transfer Processing Queue with tests
Browse files Browse the repository at this point in the history
** Why are these changes being introduced:

* The first step of the Transfer processing workflow is to allow staff
  to identify which Transfer they want to process (out of all available)
  and get to the page that will allow them to process the submitted
  files.

** Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/ETD-276

** How does this address that need:

* This defines a transfer selection screen (/transfer/select) where
  staff can see the list of available transfer records. This display can
  be filtered by graduation month (or by arbitrary user input). When the
  user clicks on any transfer in the list, they are taken to the (blank)
  detail page for that transfer. Future work will further develop that
  interface.
* This PR also includes relevant tests for the navigation, the new
  path, and the existing ability model.

** Document any side effects to this change:

* Part of the UI for this transfer selection screen is enabled by a
  jQuery plugin, DataTables. This plugin, while very useful out of the
  box, does increase our reliance on the jQuery ecosystem. A future
  discussion in EngX may lead to us needing to re-develop this screen
  using either Vue or native Javascript.
  • Loading branch information
matt-bernhardt committed May 3, 2021
1 parent 7d9130e commit 54d382e
Show file tree
Hide file tree
Showing 12 changed files with 162 additions and 7 deletions.
4 changes: 4 additions & 0 deletions app/controllers/transfer_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ def create
end
end

def select
@transfer = Transfer.all
end

def show
@transfer = Transfer.find(params[:id])
end
Expand Down
1 change: 1 addition & 0 deletions app/models/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ def processor
can :stats, Thesis

can :read, Transfer
can :select, Transfer
can :read, Hold
end

Expand Down
3 changes: 3 additions & 0 deletions app/views/layouts/_site_nav.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
<% if can? :create, Transfer %>
<%= nav_link_to("Transfer theses", new_transfer_path) %>
<% end %>
<% if can? :select, Transfer %>
<%= nav_link_to("Process transfers", transfer_select_path) %>
<% end %>
<% if can? :create, Registrar %>
<%= nav_link_to("Upload CSV", new_registrar_path) %>
<% end %>
Expand Down
3 changes: 3 additions & 0 deletions app/views/transfer/_empty.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<tr class="empty">
<td colspan="6">There were no transfers matching your parameters.</td>
</tr>
8 changes: 8 additions & 0 deletions app/views/transfer/_transfer.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<tr>
<td data-sort="<%= transfer.created_at %>"><%= link_to transfer.created_at.in_time_zone('Eastern Time (US & Canada)').strftime('%b %-d, %Y<br>%l:%M %p').html_safe, url_for(transfer) %></td>
<td data-sort="<%= transfer.grad_date %>"><%= transfer.graduation_month[...3] %> <%= transfer.graduation_year %></td>
<td><%= transfer.department.name_dw %></td>
<td><%= transfer.user.display_name %></td>
<td><%= transfer.files.count %></td>
<td><%= transfer.note.present? ? "<span title='#{transfer.note}'>#{transfer.note[..10]}...</span>".html_safe : "" %></td>
</tr>
63 changes: 63 additions & 0 deletions app/views/transfer/select.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<%= content_for(:title, "Transfer Processing | MIT Libraries") %>

<% content_for :additional_js do %>
<script src="https://cdn.datatables.net/1.10.24/js/jquery.dataTables.min.js"></script>
<% end %>

<link href="https://cdn.datatables.net/1.10.24/css/jquery.dataTables.min.css" rel="stylesheet">

<h3 class="title title-page">Transfer Processing Queue</h3>

<div id="term-list" class="filter-row">
<button data-filter="*">Show<br>all</button>
</div>

<table class="table" id="transferQueue">
<thead>
<tr>
<th scope="col">Transfer date</th>
<th scope="col">Degree date</th>
<th scope="col">Department</th>
<th scope="col">Dept. Admin</th>
<th scope="col">Files</th>
<th scope="col">Notes</th>
</tr>
</thead>
<tbody>
<%= render(partial: 'transfer/transfer', collection: @transfers) || render('empty') %>
</tbody>
</table>

<script type="text/javascript">
$(document).ready( function () {
if( document.getElementById('transferQueue').getElementsByClassName('empty').length === 0 ) {
var table = $('#transferQueue').DataTable({
"order": [[ 1, "asc" ]]
});

// Populate filter buttons with found values
var terms = [...new Set( table.columns(1).data()[0] )];
terms.forEach(element => {
document
.getElementById("term-list")
.insertAdjacentHTML("beforeend", `
<button data-filter="${element}">${element.replace(' ', '<br>')}</button>
`);
});

// Perform filtering when buttons are clicked
$(".filter-row button").click(function() {
var needle = $(this).data("filter");
$.fn.dataTable.ext.search.push(
function( settings, data, dataIndex ) {
return ( data[1] === needle || needle === "*" )
? true
: false
}
);
table.draw();
$.fn.dataTable.ext.search.pop();
});
}
});
</script>
8 changes: 7 additions & 1 deletion app/views/transfer/show.html.erb
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
<%= content_for(:title, "Thesis Transfer Submission | MIT Libraries") %>
<%= content_for(:title, "Transfer Processing | MIT Libraries") %>

<h3 class="title title-page">Transfer file list</h3>

<p><%= link_to "Back to Transfer queue", transfer_select_path %></p>

<p>This will be the display of all files in this Transfer, for processing into Thesis records.</p>
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
}

get 'transfer/confirm', to: 'transfer#confirm', as: 'transfer_confirm'
get 'transfer/select', to: 'transfer#select', as: 'transfer_select'
resources :transfer, only: [:new, :create, :show]

devise_scope :user do
Expand Down
56 changes: 56 additions & 0 deletions test/controllers/transfer_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,60 @@ class TransferControllerTest < ActionDispatch::IntegrationTest
assert_match "Error saving transfer", response.body
assert_equal original_count, Transfer.count
end

# ~~~~~~~~~~~~~~~~~~~~~~~~~~ transfer processing queue ~~~~~~~~~~~~~~~~~~~~~
test 'transfer processing queue exists' do
sign_in users(:admin)
get transfer_select_path
assert_response :success
end

test 'anonymous users cannot see transfer queue' do
# Note that nobody signed in.
get transfer_select_path
assert_response :redirect
end

test 'basic users cannot see transfer queue' do
sign_in users(:basic)
get transfer_select_path
assert_redirected_to '/'
follow_redirect!
assert_select 'div.alert', text: 'Not authorized.', count: 1
end

test 'transfer submitters cannot see transfer queue' do
sign_in users(:transfer_submitter)
get transfer_select_path
assert_redirected_to '/'
follow_redirect!
assert_select 'div.alert', text: 'Not authorized.', count: 1
end

test 'processors can see transfer queue' do
sign_in users(:processor)
get transfer_select_path
assert_response :success
end

test 'thesis admins can set transfer queue' do
sign_in users(:thesis_admin)
get transfer_select_path
assert_response :success
end

test 'admins can see transfer queue' do
sign_in users(:admin)
get transfer_select_path
assert_response :success
end

test 'transfer processing queue has links to transfer pages' do
sign_in users(:thesis_admin)
get transfer_select_path
expected_transfers = Transfer.all
expected_transfers.each do |t|
assert @response.body.include? transfer_path(t)
end
end
end
5 changes: 0 additions & 5 deletions test/fixtures/transfers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,3 @@ alsovalid:
department: one
grad_date: 2020-05-01
user: thesis_admin

baduser:
department: one
grad_date: 2020-05-01
user: invalid
13 changes: 13 additions & 0 deletions test/integration/nav_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ def teardown
end
end

test 'transfer selection nav' do
mock_auth(users(:admin))
get transfer_select_path
assert_select('.current') do |value|
assert(value.text.include?('Process transfers'))
end
end

# Basic user navigation
test 'basic navigation' do
mock_auth(users(:basic))
Expand All @@ -81,6 +89,7 @@ def teardown
assert_select "a[href=?]", process_path, count: 0
assert_select "a[href=?]", stats_path, count: 0
assert_select "a[href=?]", new_transfer_path, count: 0
assert_select "a[href=?]", transfer_select_path, count: 0
assert_select "a[href=?]", new_registrar_path, count: 0
assert_select "a[href=?]", harvest_path, count: 0
assert_select "a[href=?]", admin_root_path, count: 0
Expand All @@ -100,6 +109,7 @@ def teardown
# Navigation should not include:
assert_select "a[href=?]", process_path, count: 0
assert_select "a[href=?]", stats_path, count: 0
assert_select "a[href=?]", transfer_select_path, count: 0
assert_select "a[href=?]", new_registrar_path, count: 0
assert_select "a[href=?]", harvest_path, count: 0
assert_select "a[href=?]", admin_root_path, count: 0
Expand All @@ -115,6 +125,7 @@ def teardown
assert_select "a[href=?]", root_path
assert_select "a[href=?]", thesis_start_path
# assert_select "a[href=?]", new_transfer_path
assert_select "a[href=?]", transfer_select_path
assert_select "a[href=?]", process_path
assert_select "a[href=?]", stats_path
assert_select "a[href=?]", new_registrar_path, count: 0
Expand All @@ -132,6 +143,7 @@ def teardown
assert_select "a[href=?]", root_path
assert_select "a[href=?]", thesis_start_path
assert_select "a[href=?]", new_transfer_path
assert_select "a[href=?]", transfer_select_path
assert_select "a[href=?]", process_path
assert_select "a[href=?]", stats_path
assert_select "a[href=?]", new_registrar_path
Expand All @@ -149,6 +161,7 @@ def teardown
assert_select "a[href=?]", root_path
assert_select "a[href=?]", thesis_start_path
assert_select "a[href=?]", new_transfer_path
assert_select "a[href=?]", transfer_select_path
assert_select "a[href=?]", process_path
assert_select "a[href=?]", stats_path
assert_select "a[href=?]", new_registrar_path
Expand Down
4 changes: 3 additions & 1 deletion test/models/transfer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ class TransferTest < ActiveSupport::TestCase
end

test 'needs valid user' do
@transfer = transfers(:baduser)
@transfer = transfers(:valid)
assert @transfer.valid?
@transfer.user = nil
assert @transfer.invalid?
end

Expand Down

0 comments on commit 54d382e

Please sign in to comment.