-
Notifications
You must be signed in to change notification settings - Fork 61
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
Strip fileno before validating #18497
base: master
Are you sure you want to change the base?
Changes from all commits
9f13aac
388f54d
05b24dd
6b3d05d
1adaa04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,7 @@ def validate_metadata(metadata_input, consumer_id, upload_guid, submission_versi | |
if rejected.present? | ||
raise VBADocuments::UploadError.new(code: 'DOC102', detail: "Non-string values for keys: #{rejected.join(',')}") | ||
end | ||
if (FILE_NUMBER_REGEX =~ metadata['fileNumber']).nil? | ||
if (FILE_NUMBER_REGEX =~ metadata['fileNumber']&.strip).nil? | ||
raise VBADocuments::UploadError.new(code: 'DOC102', detail: 'Non-numeric or invalid-length fileNumber') | ||
end | ||
|
||
|
@@ -76,6 +76,7 @@ def validate_documents(parts, pdf_validator_options = VBADocuments::DocumentRequ | |
|
||
def perfect_metadata(model, parts, timestamp) | ||
metadata = JSON.parse(parts['metadata']) | ||
metadata['fileNumber'] = metadata['fileNumber']&.strip if metadata.key?('fileNumber') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since execution won't reach
|
||
metadata['source'] = "#{model.consumer_name} via VA API" | ||
metadata['receiveDt'] = timestamp.in_time_zone('US/Central').strftime('%Y-%m-%d %H:%M:%S') | ||
metadata['uuid'] = model.guid | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
{ | ||
"veteranFirstName": "Jane", | ||
"veteranLastName": "Doe", | ||
"fileNumber": "012345678", | ||
"fileNumber": " 012345678 ", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than modifying this file which is intended to be the "model" version of metadata, have you considered modifying the value in the spec file prior to using it instead? Something like this:
|
||
"zipCode": "94402", | ||
"source": "Vets.gov", | ||
"docType": "21-22", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -191,6 +191,8 @@ | |
expect(capture_body).to have_key('document') | ||
metadata = JSON.parse(capture_body['metadata']) | ||
expect(metadata['uuid']).to eq(upload.guid) | ||
# leading\trailing whitespace should have been removed | ||
expect(metadata['fileNumber']).to eq(metadata['fileNumber']&.strip) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since having whitespace in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea, i had the same thought..i was trying to use an existing test to save unit test run time(I've noticed that PRs are starting to time out on unit tests in vets-api), but, yea, you pushed me back to a distinct unit test for file number with leading\trailing white space....more unit test power Scotty |
||
updated = VBADocuments::UploadSubmission.find_by(guid: upload.guid) | ||
expect(updated.status).to eq('received') | ||
end | ||
|
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.
The safe navigation operator (
&.
) isn't needed here because L48 will raise an exception if there are missing keys, and L53 will raise an exception if the value isn't a string. Thereforemetadata['fileNumber']
won't ever benil
if L55 is reached.