Skip to content

Commit

Permalink
[CRIMAP-687] evidence download failure logging (#447)
Browse files Browse the repository at this point in the history
  • Loading branch information
hiboabd committed Oct 31, 2023
1 parent 2d35155 commit 261aa13
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 8 deletions.
15 changes: 13 additions & 2 deletions app/controllers/casework/documents_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,23 @@ class CannotDownloadDocNotPartOfApp < StandardError; end
def show; end

def download
presign_download = Datastore::Documents::Download.new(document: @document).call
redirect_to(presign_download.url, allow_other_host: true) if presign_download
presign_download = Datastore::Documents::Download.new(document: @document, log_context: log_context).call

if presign_download.respond_to?(:url)
redirect_to(presign_download.url, allow_other_host: true)
else
set_flash(:cannot_download_try_again, file_name: @document.filename, success: false)
redirect_to crime_application_path(params[:crime_application_id])
end
end

private

def log_context
{ caseworker_id: current_user_id, caseworker_ip: request.remote_ip, file_type: @document.content_type,
s3_object_key: @document.s3_object_key }
end

def set_crime_application
@crime_application = ::CrimeApplication.find(params[:crime_application_id])
end
Expand Down
7 changes: 4 additions & 3 deletions app/services/datastore/documents/download.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ module Documents
class Download
PRESIGNED_URL_EXPIRES_IN = 15 # seconds

attr_accessor :document
attr_accessor :document, :log_context

def initialize(document:)
def initialize(document:, log_context:)
@document = document
@log_context = log_context
end

def call
Rails.error.handle(fallback: -> { false }) do
Rails.error.handle(fallback: -> { false }, context: @log_context, severity: :error) do
DatastoreApi::Requests::Documents::PresignDownload.new(
object_key:, expires_in:, response_content_disposition:
).call
Expand Down
2 changes: 1 addition & 1 deletion config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
config.cache_store = :null_store

# Raise exceptions instead of rendering exception templates.
config.action_dispatch.show_exceptions = true
config.action_dispatch.show_exceptions = :all

# Disable request forgery protection in test environment.
config.action_controller.allow_forgery_protection = false
Expand Down
1 change: 1 addition & 0 deletions config/locales/en/casework.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ en:
cannot_send_back_when_marked_as_ready: This application was already marked as ready for assessment
cannot_download_unless_assigned: You must assign this application to your list to download files
cannot_download_doc_uploaded_to_another_app: File must be uploaded to current application to download
cannot_download_try_again: "%{file_name} could not be downloaded – try again"

primary_navigation:
your_list: "Your list (%{count})"
Expand Down
3 changes: 2 additions & 1 deletion spec/services/datastore/documents/download_spec.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
require 'rails_helper'

RSpec.describe Datastore::Documents::Download do
subject(:download_service) { described_class.new(document:) }
subject(:download_service) { described_class.new(document:, log_context:) }

include_context 'when downloading a document'

let(:document) { instance_double(Document, filename:, s3_object_key:) }
let(:log_context) { { caseworker_id: 1, caseworker_ip: '123.0001' } }

describe '#call' do
context 'when a document download link is retrieved successfully' do
Expand Down
4 changes: 3 additions & 1 deletion spec/shared_contexts/when_downloading_a_document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
'https://localhost.localstack.cloud:4566/crime-apply-documents-dev/42/WtpJTOwsQ2?response-content-disposition=attachment%3B%20filename%3Dtest.pdf&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=test%2F20231005%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20231005T112229Z&X-Amz-Expires=15&X-Amz-SignedHeaders=host&X-Amz-Signature=ca6081d4060ab8fc692f53b1746740f13a6bf3e46a6df0cf0acc06ed2367084e'
end

let(:expected_return) { { status: 201, body: { url: presign_download_url }.to_json } }

before do
stub_request(:put, 'https://datastore-api-stub.test/api/v1/documents/presign_download')
.with(body: expected_query)
.to_return(status: 201, body: { url: presign_download_url }.to_json)
.to_return(expected_return)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@
expect(page).not_to have_content('Mark as ready for MAAT')
end
end
end

describe 'Evidence download' do
context 'when a user attempts to download supporting evidence' do
it 'raises error if document is not part of current application' do
visit download_documents_path(crime_application_id: application_id, id: '321/hijklm5678')
Expand All @@ -52,5 +54,14 @@
expect(page).to have_current_path(presign_download_url)
end
end

context 'when there is an error with obtaining the download url' do
let(:expected_return) { { status: 500 } }

it 'displays a message if there is an error obtaining the url' do
click_on 'Download file (pdf, 12 Bytes)'
expect(page).to have_content('test.pdf could not be downloaded – try again')
end
end
end
end

0 comments on commit 261aa13

Please sign in to comment.