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

100496 Throw 500 if any files fail to make it to S3 - IVC CHAMPVA forms #20348

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions config/features.yml
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ features:
champva_pdf_decrypt:
actor_type: user
description: Enables unlocking password protected file submissions in IVC CHAMPVA forms.
champva_require_all_s3_success:
actor_type: user
description: Enables raising a StandardError if any supporting documents fail to upload to S3 in IVC CHAMPVA forms.
check_in_experience_enabled:
actor_type: user
description: Enables the health care check-in experiences
Expand Down
30 changes: 17 additions & 13 deletions modules/ivc_champva/app/services/ivc_champva/file_uploader.rb
michaelclement marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,12 @@ def initialize(form_id, metadata, file_paths, insert_db_row = false) # rubocop:d
#
# The return value reflects whether or not all files were successfully uploaded.
#
# If successful, it will return:
# - An array containing a single HTTP status code and an optional error message,
# e.g. [200] | [400, 'No such file']
#
# If any uploads yield non-200 statuses when submitted to S3, it will return:
# - An array of arrays of statuses and optional error messages corresponding to the individual
# uploads.
# E.g., for two documents uploaded, one successful and one failed, it returns:
# [[200], [400, 'No such file or directory']]
#
# @return [Array<Integer, String>, Array<Array<Integer, String>>] Either an array of arrays, or
# an array with a status code and an optional error message string.
# If successful, it will return an array containing a single HTTP status code and an
# optional error message, e.g. [200] | [400, 'No such file']
#
# If any uploads yield non-200 statuses when submitted to S3, it raise a StandardError.
#
# @return [Array<Integer, String>] An array with a status code and an optional error message string.
def handle_uploads
results = @metadata['attachment_ids'].zip(@file_paths).map do |attachment_id, file_path|
next if file_path.blank?
Expand All @@ -50,11 +44,21 @@ def handle_uploads
response_status
end.compact

all_success = results.all? { |(status, _)| status == 200 }
s3_err = nil
all_success = results.all? do |(status, err)|
s3_err = err if err # Collect last error present for logging purposes
status == 200
end

if all_success
generate_and_upload_meta_json
elsif Flipper.enabled?(:champva_require_all_s3_success, @current_user)
monitor.track_s3_upload_error(@metadata['uuid'], s3_err)
# Stop this submission in its tracks - entries will still be added to database
# for these files, but user will see error on the FE saying submission failed.
raise StandardError, "IVC ChampVa Forms - failed to upload all documents for submission: #{s3_err}"
else
# array of arrays, e.g.: [[200], [400, 'S3 error']]
results
end
end
Expand Down
12 changes: 12 additions & 0 deletions modules/ivc_champva/lib/ivc_champva/monitor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,17 @@ def track_failed_send_zsf_notification_to_pega(form_uuid, template_id)
"#{STATS_KEY}.failed_send_zsf_notification_to_pega",
call_location: caller_locations.first, **additional_context)
end

##
# Logs UUID and S3 error message when supporting docs fail to reach S3.
#
# @param [String] form_uuid UUID of the form submission with failed uploads
# @param [String] s3_err Error message received from a failed upload to S3
def track_s3_upload_error(form_uuid, s3_err)
additional_context = { form_uuid:, s3_err: }
track_request('warn', "IVC ChampVa Forms - failed to upload all documents for submission: #{form_uuid}",
"#{STATS_KEY}.s3_upload_error",
call_location: caller_locations.first, **additional_context)
end
end
end
17 changes: 16 additions & 1 deletion modules/ivc_champva/spec/services/file_uploader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,24 @@
end
end

context 'when at least one PDF upload fails' do
context 'when at least one PDF upload fails and champva_require_all_s3_success flipper is enabled:' do
before do
allow(uploader).to receive(:upload).and_return([400, 'Upload failed'])
allow(Flipper).to receive(:enabled?).with(:champva_require_all_s3_success, @current_user).and_return(true)
end

it 'raises an error' do
# Updated this test to account for new error being raised. This is so submissions are blocked
# from completing if any files fail to make it to S3. Formerly, the expectation was:
# `expect(uploader.handle_uploads).to eq([[400, 'Upload failed'], [400, 'Upload failed']])`
expect { uploader.handle_uploads }.to raise_error(StandardError, /Upload failed/)
end
end

context 'when at least one PDF upload fails and champva_require_all_s3_success flipper is disabled:' do
before do
allow(uploader).to receive(:upload).and_return([400, 'Upload failed'])
allow(Flipper).to receive(:enabled?).with(:champva_require_all_s3_success, @current_user).and_return(false)
end

it 'returns an array of upload results' do
Expand Down
Loading