-
Notifications
You must be signed in to change notification settings - Fork 62
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
Api 40300 526 bd upload refactor #18730
Open
mchristiansonVA
wants to merge
24
commits into
master
Choose a base branch
from
API-40300-526-BD-upload-refactor
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 9 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
f91dd7f
Initial commit, work in progress
mchristiansonVA 095520a
Incremental commit
mchristiansonVA afe2a14
Merge remote-tracking branch 'origin/master' into API-40300-526-BD-up…
mchristiansonVA e1ff3ea
Incremental commit
mchristiansonVA 01936b1
Misc cleanup, remove unused code
mchristiansonVA b1369b6
Add conditional for new BD method
mchristiansonVA 8f90288
Merge remote-tracking branch 'origin/master' into API-40300-526-BD-up…
mchristiansonVA 5995f6b
Fix name of body payload for Faraday
mchristiansonVA 7dcd073
Remove handling for participant_id from 526 flow
mchristiansonVA 5a8ad0d
Add new base class, begin moving shared methods
mchristiansonVA 18988a4
Merge remote-tracking branch 'origin/master' into API-40300-526-BD-up…
mchristiansonVA 8214b66
Move additional shared logic to base class
mchristiansonVA 3842246
Merge remote-tracking branch 'origin/master' into API-40300-526-BD-up…
mchristiansonVA b0bc84d
Misc cleanup, tweaks for doc type name and form name
mchristiansonVA bb8f096
Fix tests for BD upload
mchristiansonVA 47779aa
Add new flipper expectation
mchristiansonVA b4772e3
Rubocop params warning fix
mchristiansonVA b99445b
Update BD spec to exercise new upload_document method
mchristiansonVA 2a26eb5
Add spec for DisabilityDocumentService
mchristiansonVA 3ca3ebb
Rubocop fix string quotes
mchristiansonVA b4d796d
Add test for DocumentServiceBase
mchristiansonVA adea26f
Fix test for upload_document
mchristiansonVA b30dbbe
Merge branch 'master' into API-40300-526-BD-upload-refactor
mchristiansonVA 59efb8c
Merge remote-tracking branch 'origin/master' into API-40300-526-BD-up…
mchristiansonVA File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
79 changes: 79 additions & 0 deletions
79
...claims_api/app/services/claims_api/disability_compensation/disability_document_service.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
# frozen_string_literal: true | ||
|
||
module ClaimsApi | ||
module DisabilityCompensation | ||
class DisabilityDocumentService < ServiceBase | ||
LOG_TAG = '526_v2_Disability_Document_service' | ||
|
||
def create_upload(claim:, pdf_path:, doc_type: 'L122', action: 'post', original_filename: nil) | ||
unless File.exist? pdf_path | ||
ClaimsApi::Logger.log('benefits_documents', detail: "Error creating upload doc: #{file_path} doesn't exist, | ||
claim_id: #{claim.id}") | ||
raise Errno::ENOENT, pdf_path | ||
end | ||
|
||
body = generate_upload_body(claim:, doc_type:, pdf_path:, action:, original_filename:) | ||
ClaimsApi::BD.new.upload_document(claim_id: claim.id, doc_type:, body:) | ||
end | ||
|
||
private | ||
|
||
## | ||
# Generate form body to upload a document | ||
# | ||
# @return {parameters, file} | ||
def generate_upload_body(claim:, doc_type:, pdf_path:, action:, original_filename: nil) | ||
payload = {} | ||
auth_headers = claim.auth_headers | ||
veteran_name = compact_veteran_name(auth_headers['va_eauth_firstName'], auth_headers['va_eauth_lastName']) | ||
birls_file_num = auth_headers['va_eauth_birlsfilenumber'] | ||
claim_id = claim.evss_id | ||
file_name = generate_file_name(doc_type:, veteran_name:, claim_id:, original_filename:, action:) | ||
tracked_item_ids = claim.tracked_items&.map(&:to_i) if claim&.has_attribute?(:tracked_items) | ||
data = build_body(doc_type:, file_name:, claim_id:, | ||
file_number: birls_file_num, tracked_item_ids:) | ||
|
||
fn = Tempfile.new('params') | ||
File.write(fn, data.to_json) | ||
payload[:parameters] = Faraday::UploadIO.new(fn, 'application/json') | ||
payload[:file] = Faraday::UploadIO.new(pdf_path.to_s, 'application/pdf') | ||
payload | ||
end | ||
|
||
def compact_veteran_name(first_name, last_name) | ||
[first_name, last_name].compact_blank.join('_') | ||
end | ||
|
||
def generate_file_name(doc_type:, veteran_name:, claim_id:, original_filename:, action:) | ||
if action == 'post' && doc_type == 'L122' | ||
"#{[veteran_name, claim_id, '526EZ'].compact_blank.join('_')}.pdf" | ||
else | ||
filename = get_original_supporting_doc_file_name(original_filename) | ||
"#{[veteran_name, claim_id, filename].compact_blank.join('_')}.pdf" | ||
end | ||
end | ||
|
||
## | ||
# DisabilityCompensationDocuments method create_unique_filename adds a random 11 digit | ||
# hex string to the original filename, so we remove that to yield the user-submitted | ||
# filename to use as part of the filename uploaded to the BD service. | ||
def get_original_supporting_doc_file_name(original_filename) | ||
file_extension = File.extname(original_filename) | ||
base_filename = File.basename(original_filename, file_extension) | ||
base_filename[0...-12] | ||
end | ||
|
||
def build_body(options = {}) | ||
data = { | ||
systemName: 'VA.gov', | ||
docType: options[:doc_type], | ||
fileName: options[:file_name], | ||
trackedItemIds: options[:tracked_item_ids].presence || [] | ||
} | ||
data[:claimId] = options[:claim_id] unless options[:claim_id].nil? | ||
data[:fileNumber] = options[:file_number] unless options[:file_number].nil? | ||
{ data: } | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned this method will basically need to be duplicated in the other service files (if I am seeing the direction correctly.
Wondering if there is a way to move this to the service_base perhaps and make it universal enough that we can invoke this one method from the different service files?
I just feel like this method should still be shared, acknowledging of course that the direction may become clearer with another service in the mix to see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good callout here. just had a sync with Matt and I and we were in agreeance 100%. I like the idea of an intermediary service, somewhat generic, and specific to the document functionality. The original idea was to move to servicebase but with that service, it handles most the root level and generic functionalities. The thought now is to have a DocumentService < ServiceBase that would handle some of these shared, generic document specific methods, and then we'll just have a Service for each form type, hopefully with just a singular "extraction" method to get the values needed to call the rest of the services within the DocumentService, which will be the builders.
Hopefully that makes sense.