-
Notifications
You must be signed in to change notification settings - Fork 66
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
Save icn in uploadsubmission tracking record #20643
base: master
Are you sure you want to change the base?
Conversation
# attempt to use consumer's file number field to look up the claiments ICN | ||
icn = find_icn(metadata['fileNumber']) | ||
metadata['ICN'] = icn if icn.present? | ||
|
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.
Here is my mistake in the original code, i guess(???) i thought that the metadata hash above was the same metadata we store in our tracking record. It's not, the metadata below is dynamic, we build it in memory and send it to EMMS as part of our req to them.
# Attempt to use consumer supplied file number field to look up the claiments ICN | ||
# asap for tracking consumer's impacted | ||
icn = find_icn(parts) | ||
@upload.update(metadata: @upload.metadata.merge({ 'ICN' => icn })) if icn.present? |
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.
Search and save icn in our UploadSubmission model. Doing this as soon as practical, even if further along in this process we find issues or blow up, we will still have the ICN around in the errored UploadSubmission database record for support and Veterans Impacted type of stats.
@@ -60,14 +66,11 @@ def download_and_process | |||
validate_metadata(parts[META_PART_NAME], @upload.consumer_id, @upload.guid, | |||
submission_version: @upload.metadata['version'].to_i) | |||
metadata = perfect_metadata(@upload, parts, timestamp) | |||
metadata['ICN'] = icn if icn.present? |
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.
Pass ICN along to EMMS in the metadata we send them as part of our upload request.
end | ||
|
||
def find_icn(parts) | ||
consumer_file_number = read_original_metadata_file_number(parts) | ||
|
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 added this little helper method to attempt to just go after the file_number field in the metadata that the consumer sends us.
UploadValidator is called below and also goes after the fileNumber field and validates it's presence and format, however, it's all grouped up with validating all the other metadata fields, so for example, if a metadata field other than fileNumber were missing, UploadValidator would throw an exception and i'd never get the fileNumber field back even if it were valid. I want to record icn regardless of the state of the other metadata fields the consumer provided
Summary
No flipper flag, just a feature gap in existing code
We recently were asked to start supplying an ICN on a best effort basis by one of our upstream services, EMMS.
I added the code to lookup ICN, and we did send it to our downstream, however, i neglected to save the ICN to our tracking record. This PR adds functionality to save the ICN in our metadata field. This will be used for consumer support when needed, and for stats around veterans impacted for the Benefits Intake API.
Team banana-peels, we own this code
Related issue(s)
API-44676
Testing done
Unit test were modified to confirm ICN is stored in metadata and no adverse impacts if ICN is not available
Screenshots
Note: Optional
What areas of the site does it impact?
(Describe what parts of the site are impacted andifcode touched other areas)
Acceptance criteria
Requested Feedback
(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?