diff --git a/config/features.yml b/config/features.yml index 52b070a5f50..46f59132728 100644 --- a/config/features.yml +++ b/config/features.yml @@ -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 diff --git a/modules/ivc_champva/app/services/ivc_champva/file_uploader.rb b/modules/ivc_champva/app/services/ivc_champva/file_uploader.rb index b390e088d8d..4187435dda5 100644 --- a/modules/ivc_champva/app/services/ivc_champva/file_uploader.rb +++ b/modules/ivc_champva/app/services/ivc_champva/file_uploader.rb @@ -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, Array>] 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] 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? @@ -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 diff --git a/modules/ivc_champva/lib/ivc_champva/monitor.rb b/modules/ivc_champva/lib/ivc_champva/monitor.rb index e707e21090f..a0c62b262dc 100644 --- a/modules/ivc_champva/lib/ivc_champva/monitor.rb +++ b/modules/ivc_champva/lib/ivc_champva/monitor.rb @@ -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 diff --git a/modules/ivc_champva/spec/services/file_uploader_spec.rb b/modules/ivc_champva/spec/services/file_uploader_spec.rb index 87f947520f2..e62c39577b6 100644 --- a/modules/ivc_champva/spec/services/file_uploader_spec.rb +++ b/modules/ivc_champva/spec/services/file_uploader_spec.rb @@ -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