Skip to content

Commit

Permalink
Allow processors to rename file attachments
Browse files Browse the repository at this point in the history
Why are these changes being introduced:

* Duplicate filenames attached to a thesis cannot be preserved
* Sometimes we get duplicate filenames attached to a thesis
* We did not have a way to rename filenames in the application

Relevant ticket(s):

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

How does this address that need:

* Adds a form that allows processors to rename a file attachment

Document any side effects to this change:

* ability model initialize class was auto updated by rubocop to use
  a guard clause
  • Loading branch information
JPrevost committed Oct 13, 2023
1 parent 716268e commit 421507c
Show file tree
Hide file tree
Showing 7 changed files with 212 additions and 15 deletions.
26 changes: 26 additions & 0 deletions app/controllers/file_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
class FileController < ApplicationController
before_action :require_user
before_action :authenticate_user!
load_and_authorize_resource
protect_from_forgery with: :exception

def rename_form
@thesis = Thesis.find(params[:thesis_id])
@attachment = ActiveStorage::Attachment.find(params[:attachment_id])
end

def rename
thesis = Thesis.find(params[:thesis_id])
attachment = ActiveStorage::Attachment.find(params[:attachment_id])

attachment.blob.filename = params[:attachment][:filename]

if attachment.blob.save
flash[:success] = "#{thesis.title} file #{attachment.filename} been updated."
else
flash[:error] = "#{thesis.title} file was unable to be updated"
end

redirect_to thesis_process_path(thesis)
end
end
30 changes: 16 additions & 14 deletions app/models/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@ def initialize(user)
# See the documentation for details:
# https://github.com/CanCanCommunity/cancancan/blob/develop/docs/Defining-Abilities.md

if user.present?
@user = user
# Admin users can do everything for all models
can :manage, :all if user.admin?

# Assign thesis_submitter rights directly to appropriate users as the
# process that follows will not pick them up as it is not an explicitly
# assigned role
transfer_submitter if user.submitter?

# This line matches users' roles with the functions defined below,
# giving them privileges accordingly.
send(@user.role.to_sym)
end
return unless user.present?

@user = user
# Admin users can do everything for all models
can :manage, :all if user.admin?

# Assign thesis_submitter rights directly to appropriate users as the
# process that follows will not pick them up as it is not an explicitly
# assigned role
transfer_submitter if user.submitter?

# This line matches users' roles with the functions defined below,
# giving them privileges accordingly.
send(@user.role.to_sym)
end

# The default; any logged-in user. The use case here is students uploading
Expand Down Expand Up @@ -91,6 +91,8 @@ def processor
can :read, Transfer
can :select, Transfer
can :files, Transfer
can :rename_form, File
can :rename, File

can :read, Hold

Expand Down
4 changes: 4 additions & 0 deletions app/views/file/rename_form.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<%= form_with model: @attachment, url: rename_file_path(thesis_id: @thesis, attachment_id: @attachment), method: 'post' do |form| %>
<%= form.text_field :filename, value: @attachment.filename %>
<%= form.submit %>
<% end %>
5 changes: 4 additions & 1 deletion app/views/thesis/_file_fields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
<td>
<%= link_to rails_blob_path(f.object, disposition: "inline"), target: :_blank do %>
<%= f.object.blob.filename %>
<% end %>
<% end %>
</td>
<td>
<%= link_to rename_file_form_path(thesis_id: @thesis, attachment_id: f.object) do %>Rename<% end %>
</td>
<td>
<%= f.object.blob.created_at.in_time_zone('Eastern Time (US & Canada)').strftime('%b %-d, %Y<br>%l:%M %p').html_safe %>
Expand Down
1 change: 1 addition & 0 deletions app/views/thesis/process_theses.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@
<tr>
<th scope="col">Remove</th>
<th scope="col">File</th>
<th scope="col">Rename file</th>
<th scope="col">Received</th>
<th scope="col">Purpose</th>
<th scope="col">Description</th>
Expand Down
4 changes: 4 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@
get 'thesis/proquest_export_preview', to: 'thesis#proquest_export_preview', as: 'thesis_proquest_export_preview'
get 'thesis/proquest_export', to: 'thesis#proquest_export', as: 'thesis_proquest_export'
get 'thesis/reset_all_publication_errors', to: 'thesis#reset_all_publication_errors', as: 'reset_all_publication_errors'

# Blob file renaming
get 'file/rename/:thesis_id/:attachment_id', to: 'file#rename_form', as: 'rename_file_form'
post 'file/rename/:thesis_id/:attachment_id', to: 'file#rename', as: 'rename_file'

resources :registrar, only: [:new, :create, :show]
resources :thesis, only: [:new, :create, :edit, :show, :update]
Expand Down
157 changes: 157 additions & 0 deletions test/controllers/files_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
require 'test_helper'

class FilesControllerTest < ActionDispatch::IntegrationTest
test 'anonymous users are prompted to log in when accessing rename form' do
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
get "/file/rename/#{@thesis.id}/#{@attachment.id}"

assert_response :redirect
assert_redirected_to '/login'
end

test 'basic can not access file rename form' do
sign_in users(:basic)
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
get "/file/rename/#{@thesis.id}/#{@attachment.id}"
assert_redirected_to '/'
follow_redirect!
assert_select 'div.alert', text: 'Not authorized.', count: 1
end

test 'transfer_submitter can not access file rename form' do
sign_in users(:transfer_submitter)
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
get "/file/rename/#{@thesis.id}/#{@attachment.id}"
assert_redirected_to '/'
follow_redirect!
assert_select 'div.alert', text: 'Not authorized.', count: 1
end

test 'thesis processors can access file rename form' do
sign_in users(:processor)
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
get "/file/rename/#{@thesis.id}/#{@attachment.id}"
assert_response :success
end

test 'thesis admins can access file rename form' do
sign_in users(:thesis_admin)
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
get "/file/rename/#{@thesis.id}/#{@attachment.id}"
assert_response :success
end

test 'thesis admins can rename blob filename' do
sign_in users(:thesis_admin)
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
assert_equal('a_pdf.pdf', @attachment.blob.filename.to_s)

post rename_file_path(@thesis, @attachment),
params: { attachment: { filename: 'renamed_a_pdf_too.pdf' } }
follow_redirect!
assert_equal thesis_process_path(theses(:publication_review)), path

assert_select '.alert-banner.success',
text: 'MyReadyThesis file renamed_a_pdf_too.pdf been updated.', count: 1

@attachment.reload

assert_equal('renamed_a_pdf_too.pdf', @attachment.blob.filename.to_s)
end

test 'thesis processor can rename blob filename' do
sign_in users(:processor)
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
assert_equal('a_pdf.pdf', @attachment.blob.filename.to_s)

post rename_file_path(@thesis, @attachment),
params: { attachment: { filename: 'renamed_a_pdf_too.pdf' } }
follow_redirect!
assert_equal thesis_process_path(theses(:publication_review)), path

assert_select '.alert-banner.success',
text: 'MyReadyThesis file renamed_a_pdf_too.pdf been updated.', count: 1

@attachment.reload

assert_equal('renamed_a_pdf_too.pdf', @attachment.blob.filename.to_s)
end

test 'basic user can not rename blob filename' do
sign_in users(:basic)
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
assert_equal('a_pdf.pdf', @attachment.blob.filename.to_s)

post rename_file_path(@thesis, @attachment),
params: { attachment: { filename: 'renamed_a_pdf_too.pdf' } }
follow_redirect!
assert_equal root_path, path

assert_select 'div.alert', text: 'Not authorized.', count: 1

@attachment.reload

assert_equal('a_pdf.pdf', @attachment.blob.filename.to_s)
end

test 'transfer_submitter user can not rename blob filename' do
sign_in users(:transfer_submitter)
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
assert_equal('a_pdf.pdf', @attachment.blob.filename.to_s)

post rename_file_path(@thesis, @attachment),
params: { attachment: { filename: 'renamed_a_pdf_too.pdf' } }
follow_redirect!
assert_equal root_path, path

assert_select 'div.alert', text: 'Not authorized.', count: 1

@attachment.reload

assert_equal('a_pdf.pdf', @attachment.blob.filename.to_s)
end

test 'anonymous user can not rename blob filename' do
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
assert_equal('a_pdf.pdf', @attachment.blob.filename.to_s)

post rename_file_path(@thesis, @attachment),
params: { attachment: { filename: 'renamed_a_pdf_too.pdf' } }
assert_redirected_to '/login'

@attachment.reload

assert_equal('a_pdf.pdf', @attachment.blob.filename.to_s)
end

test 'renaming a file does not change the file checksum' do
sign_in users(:thesis_admin)
@thesis = theses(:publication_review)
@attachment = @thesis.files.first
assert_equal('a_pdf.pdf', @attachment.blob.filename.to_s)
assert_equal('KADsjJnGD1sVUgvqyZOaRg==', @attachment.checksum)

post rename_file_path(@thesis, @attachment),
params: { attachment: { filename: 'renamed_a_pdf_too.pdf' } }
follow_redirect!
assert_equal thesis_process_path(theses(:publication_review)), path

assert_select '.alert-banner.success',
text: 'MyReadyThesis file renamed_a_pdf_too.pdf been updated.', count: 1

@attachment.reload

assert_equal('renamed_a_pdf_too.pdf', @attachment.blob.filename.to_s)
assert_equal('KADsjJnGD1sVUgvqyZOaRg==', @attachment.checksum)
end
end

0 comments on commit 421507c

Please sign in to comment.