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

Adds report for thesis files with no defined purpose #795

Merged
merged 1 commit into from
Oct 4, 2021

Conversation

matt-bernhardt
Copy link
Member

@matt-bernhardt matt-bernhardt commented Sep 28, 2021

This adds a new report page for files with no defined purpose. The table includes links to the processing form for that thesis, so staff can assign a purpose.

More details in the commit message...

Ticket

https://mitlibraries.atlassian.net/browse/ETD-418

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • ANDI or Wave has been run in accordance to
    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)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

NO

@matt-bernhardt matt-bernhardt marked this pull request as draft September 28, 2021 20:52
@mitlib mitlib temporarily deployed to thesis-submit-pr-795 September 28, 2021 20:52 Inactive
@coveralls
Copy link

coveralls commented Sep 28, 2021

Coverage Status

Coverage increased (+0.05%) to 95.92% when pulling 889f2fa on etd-418-files-without-purpose into fff0764 on main.

@matt-bernhardt matt-bernhardt force-pushed the etd-418-files-without-purpose branch from 9daa653 to d6019c6 Compare October 1, 2021 17:57
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-795 October 1, 2021 17:57 Inactive
@matt-bernhardt matt-bernhardt force-pushed the etd-418-files-without-purpose branch from d6019c6 to 59c4bb7 Compare October 1, 2021 19:23
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-795 October 1, 2021 19:24 Inactive
@matt-bernhardt matt-bernhardt force-pushed the etd-418-files-without-purpose branch from 59c4bb7 to a0299a3 Compare October 1, 2021 19:40
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-795 October 1, 2021 19:41 Inactive
** Why are these changes being introduced:

* Staff users need a way to confirm that all files attached to theses
  have had their purpose identified.

** Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/etd-418

** How does this address that need:

* This creates a new report page, /report/files, which lists all files
  which have no defined value in their purpose field. The page can be
  filtered by academic term. Entries appear on this report with a link
  back to the processing form for that thesis, so that staff can assign
  a purpose for them.

** Document any side effects to this change:

* The tests for this report end up repeating a setup method,
  attach_files, which was written first for the thesis tests. This
  repetition is not ideal, but IMO necessary.
* The implementation for this report is slightly different than other
  reports, in that the controller calls the specific list method
  directly, rather than receiving a collection of lists. This makes the
  template more bound to this given list, rather than having a generic
  list partial.
@matt-bernhardt matt-bernhardt force-pushed the etd-418-files-without-purpose branch from a0299a3 to 889f2fa Compare October 1, 2021 20:00
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-795 October 1, 2021 20:00 Inactive
@matt-bernhardt matt-bernhardt marked this pull request as ready for review October 1, 2021 20:02
@JPrevost JPrevost self-assigned this Oct 4, 2021
@@ -6,6 +6,14 @@ class ReportController < ApplicationController

include ThesisHelper

def files
Copy link
Member

Choose a reason for hiding this comment

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

We may want to consider a more explicit name for this. In the future looking at this method it won't be clear that this is really for the files_with_no_defined_purpose versus some generic files route without having to poke around further.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is fair, but I'd like to move this to a different name once we have an idea of what the alternative report is going to be. Looking at the current reporting epic in Jira, no other requested report seems likely to overlap with this method.

@matt-bernhardt matt-bernhardt merged commit a9954f1 into main Oct 4, 2021
@matt-bernhardt matt-bernhardt deleted the etd-418-files-without-purpose branch October 4, 2021 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants