-
Notifications
You must be signed in to change notification settings - Fork 4
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
First part of Transfer processing #697
Conversation
00d56d5
to
f0d6a9a
Compare
f0d6a9a
to
27f9060
Compare
2e1120a
to
24d2a1a
Compare
<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"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if our theme gem should allow <% content_for :additional_css do %>
to better handle this use case?
(No action needed, just thinking out loud).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was on the point of asking why I couldn't find a layout in this app - didn't think to ask about the theme gem. Yes, I think that would be a good feature to add for a use case like this.
(I don't think that blocks this merging, though right? We can do that via maintenance?)
app/views/transfer/_empty.html.erb
Outdated
@@ -0,0 +1,3 @@ | |||
<tr class="empty"> | |||
<td colspan="6">There were no transfers matching your parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
td
not closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
smacks head D'oh! I'm going to push something to fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that theme gem thing wasn't intended to be included in this work.
** 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.
cbd14df
to
54d382e
Compare
This implements the first few steps of the Transfer Processing workflow:
Developer
our guide and
all issues introduced by these changes have been resolved or opened as new
issues (link to those issues in the Pull Request details above)
Code Reviewer
(not just this pull request message)
Requires database migrations?
NO
Includes new or updated dependencies?
NO