From a0299a36a371652942a9e3fa202b7f728872618f Mon Sep 17 00:00:00 2001 From: Matt Bernhardt Date: Tue, 28 Sep 2021 16:51:44 -0400 Subject: [PATCH] Adds a report of files with no defined purpose ** 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. --- app/controllers/report_controller.rb | 8 ++ app/models/ability.rb | 1 + app/models/report.rb | 10 ++ app/views/report/_files_empty.html.erb | 3 + .../report/_files_without_purpose.html.erb | 14 +++ app/views/report/files.html.erb | 29 +++++ app/views/shared/_report_submenu.html.erb | 3 + config/routes.rb | 1 + test/controllers/report_controller_test.rb | 104 ++++++++++++++++++ 9 files changed, 173 insertions(+) create mode 100644 app/views/report/_files_empty.html.erb create mode 100644 app/views/report/_files_without_purpose.html.erb create mode 100644 app/views/report/files.html.erb diff --git a/app/controllers/report_controller.rb b/app/controllers/report_controller.rb index 36477ae9..c244e47e 100644 --- a/app/controllers/report_controller.rb +++ b/app/controllers/report_controller.rb @@ -6,6 +6,14 @@ class ReportController < ApplicationController include ThesisHelper + def files + report = Report.new + theses = Thesis.all + @terms = report.extract_terms theses + subset = filter_theses_by_term theses + @list = report.list_unattached_files subset + end + def index report = Report.new @terms = Thesis.pluck(:grad_date).uniq.sort diff --git a/app/models/ability.rb b/app/models/ability.rb index 65c26e0b..c9c48bc8 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -53,6 +53,7 @@ def processor can :manage, :submitter + can :files, Report can :index, Report can :term, Report diff --git a/app/models/report.rb b/app/models/report.rb index 47253f8a..ac650ad2 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -252,6 +252,16 @@ def extract_terms(collection) collection.pluck(:grad_date).uniq.sort end + def list_unattached_files(collection) + result = [] + collection.joins(:files_attachments).order(:grad_date).uniq.each do |record| + record.files.where(purpose: nil).each do |file| + result.push(file) + end + end + result + end + def table_copyright(collection) result = {} collection.group(:copyright).count.each do |record| diff --git a/app/views/report/_files_empty.html.erb b/app/views/report/_files_empty.html.erb new file mode 100644 index 00000000..ce45e78b --- /dev/null +++ b/app/views/report/_files_empty.html.erb @@ -0,0 +1,3 @@ + + There are no files without an assigned purpose within the selected term. + diff --git a/app/views/report/_files_without_purpose.html.erb b/app/views/report/_files_without_purpose.html.erb new file mode 100644 index 00000000..bc057266 --- /dev/null +++ b/app/views/report/_files_without_purpose.html.erb @@ -0,0 +1,14 @@ + + <%= link_to(files_without_purpose.blob[:filename],thesis_process_path(files_without_purpose[:record_id])) %> + + <% files_without_purpose.record.authors.each do |author| %> + <%= author.user.display_name %>
+ <% end %> + + + <% files_without_purpose.record.departments.each do |dept| %> + <%= dept.name_dw %>
+ <% end %> + + <%= files_without_purpose[:description] %> + diff --git a/app/views/report/files.html.erb b/app/views/report/files.html.erb new file mode 100644 index 00000000..c7f3cbb1 --- /dev/null +++ b/app/views/report/files.html.erb @@ -0,0 +1,29 @@ +<%= content_for(:title, "Thesis Reporting | MIT Libraries") %> + +
+
+

Files without purpose

+ + <%= render 'shared/defined_terms_filter' %> + + + + + + + + + + + + + <%= render(partial: 'report/files_without_purpose', collection: @list) || render('files_empty') %> + +
FileAuthorsDepartmentsDescription
+ +
+ + +
diff --git a/app/views/shared/_report_submenu.html.erb b/app/views/shared/_report_submenu.html.erb index 3c7c8c7e..8321f239 100644 --- a/app/views/shared/_report_submenu.html.erb +++ b/app/views/shared/_report_submenu.html.erb @@ -8,6 +8,9 @@ <% if can?(:select, Thesis) %>
  • <%= link_to("Theses with co-authors", thesis_deduplicate_path) %>
    • <% end %> + <% if can?(:files, Report) %> +
  • <%= link_to("Files without purpose", report_files_path) %>
    • + <% end %> diff --git a/config/routes.rb b/config/routes.rb index 39ca704e..18dc4e81 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -20,6 +20,7 @@ end get 'report', to: 'report#index', as: 'report_index' + get 'report/files', to: 'report#files', as: 'report_files' get 'report/term', to: 'report#term', as: 'report_term' get 'thesis/confirm', to: 'thesis#confirm', as: 'thesis_confirm' get 'thesis/deduplicate', to: 'thesis#deduplicate', as: 'thesis_deduplicate' diff --git a/test/controllers/report_controller_test.rb b/test/controllers/report_controller_test.rb index eca9af23..0a3b0171 100644 --- a/test/controllers/report_controller_test.rb +++ b/test/controllers/report_controller_test.rb @@ -1,6 +1,23 @@ require 'test_helper' class ReportControllerTest < ActionDispatch::IntegrationTest + def attach_files(tr, th) + # Ideally our fixtures would have already-attached files, but they do not + # yet. So we attach two files to a transfer and thesis record, to prepare + # for tests with an accurate set of file attachments. + f1 = Rails.root.join('test', 'fixtures', 'files', 'a_pdf.pdf') + f2 = Rails.root.join('test', 'fixtures', 'files', 'a_pdf.pdf') + tr.files.attach(io: File.open(f1), filename: 'a_pdf.pdf') + tr.files.attach(io: File.open(f2), filename: 'a_pdf.pdf') + tr.save + tr.reload + th.files.attach(tr.files.first.blob) + th.files.attach(tr.files.second.blob) + th.save + th.reload + end + + # ~~~~~~~~~~~~~~~~~~~~ Report dashboard ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ test 'summary report exists' do sign_in users(:admin) get report_index_path @@ -120,4 +137,91 @@ class ReportControllerTest < ActionDispatch::IntegrationTest assert_select '.card-overall .message', text: '2 thesis records', count: 1 assert_response :success end + + # ~~~~~~~~~~~~~~~~~~~~ Files without purpose report ~~~~~~~~~~~~~~~~~~~~~~~~ + test 'files report exists' do + sign_in users(:admin) + get report_files_path + assert_response :success + end + + test 'anonymous users are prompted to log in by files report' do + # Note that nobody is signed in. + get report_files_path + assert_response :redirect + end + + test 'basic users cannot see files report' do + sign_in users(:basic) + get report_files_path + assert_redirected_to '/' + follow_redirect! + assert_select 'div.alert', text: 'Not authorized.', count: 1 + end + + test 'submitters cannot see files report' do + sign_in users(:transfer_submitter) + get report_files_path + assert_redirected_to '/' + follow_redirect! + assert_select 'div.alert', text: 'Not authorized.', count: 1 + end + + test 'processors can see files report' do + sign_in users(:processor) + get report_files_path + assert_response :success + end + + test 'thesis_admins can see files report' do + sign_in users(:thesis_admin) + get report_files_path + assert_response :success + end + + test 'admins can see files report' do + sign_in users(:admin) + get report_files_path + assert_response :success + end + + test 'default files report is empty' do + sign_in users(:processor) + get report_files_path + assert_select 'table tbody td', text: 'There are no files without an assigned purpose within the selected term.', count: 1 + end + + test 'files have no default purpose, and appear on the files report' do + sign_in users(:processor) + xfer = transfers(:valid) + thesis = theses(:one) + attach_files(xfer, thesis) + get report_files_path + assert_select 'table tbody td', text: 'a_pdf.pdf', count: 2 + end + + test 'files report can be filtered by term' do + sign_in users(:processor) + xfer = transfers(:valid) + thesis = theses(:one) + attach_files(xfer, thesis) + get report_files_path + assert_select 'table tbody td', text: 'a_pdf.pdf', count: 2 + get report_files_path, params: { graduation: '2018-09-01' } + assert_select 'table tbody td', text: 'There are no files without an assigned purpose within the selected term.', count: 1 + end + + test 'files disappear from files report when a purpose is set' do + sign_in users(:processor) + xfer = transfers(:valid) + thesis = theses(:one) + attach_files(xfer, thesis) + get report_files_path + assert_select 'table tbody td', text: 'a_pdf.pdf', count: 2 + file = thesis.files.first + file.purpose = 0 + file.save + get report_files_path + assert_select 'table tbody td', text: 'a_pdf.pdf', count: 1 + end end