-
Notifications
You must be signed in to change notification settings - Fork 206
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
Initial Commit of Evidence/Ancillary Documents RFC #93432
base: master
Are you sure you want to change the base?
Conversation
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.
Looks good and has some great content. I like where this is going! I left a few comments to start some conversations.
* Document extension supported | ||
* Document not password protected (or password collected if so, since we NEED to check this here) | ||
|
||
All other checks should be done by the system downstream that is ultimately responsible for accepting the file or not. We should not try to maintain separate lists of requirements that can get out of sync. We should, after initial validation above, check the file against a validation endpoint for the downstream system. |
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.
We could try to word this as the "furthest synchronous downstream source of truth" with the thought being that a synchronous document validation response is needed to alert the user.
This would mean that the accepting service would be required to do as much validation as possible when there is a lack of usable downstream validation.
I agree that there is ideally a single source of truth on valid documents, we should do what we can to alert the veteran that their document will not be accepted.
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.
Agree. If a validation for a known requirement is done async, that validation needs to be pulled up to a sync path.
[Benefits Documents API](https://developer.va.gov/explore/api/benefits-documents/docs?version=current) is preferred as it directly puts the documents into the system(s) of record (VBMS/BGS). | ||
|
||
[Benefits Intake API](https://developer.va.gov/explore/api/benefits-intake/docs?version=current) goes to Central Mail, similar to if something was mailed in or faxed, it has more human and system touch-points and therefore is slower and more prone to error(s). | ||
|
||
For some documents, this is required, such as 21-4142, since it requires manual action be done by a person. This is also appropriate for documents which fail to go the primary/Documents API path (due to error, data issues/discrepancies, downtimes, network issues, etc) |
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.
Should we call out exceptions (Such as the Decision Reviews API?)
|
||
Exhaustion can be determined by systematic polling and maintaining of statuses via a state system/machine, or database records. | ||
We should maintain the state of every document we send via polling, until it reaches a final state. | ||
We should poll until the state represents that the document made it into the system of record. |
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 think we should poll until the state represents finality. So we poll until either success or unrecoverable error has occurred.
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.
For cases where something is stuck in a state, should we keep polling forever? I am inclined to say maybe, as that may force whatever system is holding it up, or where it is stuck to be investigated? Either way we will need to support "after some amount of time, send the user a notification we have not yet been able to deliver your doc".
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.
Agreed. I think the statement after this in the doc covers that. There needs to be some sort of "time out" but it may be form dependent. If we wait 14 days then ask the veteran to resubmit, but on day 15 their document is accepted, it could have an impact on what they are trying to do.
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 can of worms I'm trying to open, because I really dislike doc upload being async AND happening after claim establishment while also not being guaranteed to succeed:
- IF BIP Claims Evidence API provides synchronous responses, AND
- IF BIP Claims Evidence API allows evidence upload to happen before and separately from evidence association with a claim ID (and I believe they do],
- THEN would the evidence upload experience be less faulty and less of a pain to monitor if we redesigned the evidence submission experience so that we make sure all evidence given by a veteran uploads successfully before establishing the claim? Then, for example, if an evidence upload fails, our web UX will tell them what failed and why to give them a chance to fix/reupload their evidence or we provide a path that says "I understand I'm submitting my claim without the following evidence: [list of file names]". We could either do this as they're pressing submit or immediately as they're uploading that piece of evidence (probably this latter option preferred for tighter feedback loop).
I'm leaving separate the question here of whether we'd use BIP directly or whether we'd ask LH to expose these capabilities to us.
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.
Do both, benefits intake API and document upload API require pooling?
|
||
* Reduced code complexity | ||
* Align on shared strategy | ||
* Consistent fronend experience for the end users |
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.
sp! "frontend"
|
||
Any document that exhausts all sending options, requires action on the systems part to notify the user of this failure, prompting the user to reattempt some other way. | ||
|
||
Monitoring of each and every total-failure/exhaustion event should be setup, with investigation and remediation of the error casuing the failure to be a top priority. Remediation of the CAUSE of the error is to prevent the same error from affecting future users, this is not the same as resending or reattempting after failure, since that will not be acceptable after failure emails start going out, as it will cause duplicates to be sent. (TBD on if this is true or not, duplicates may be better than nothing if the user does not act on the email to reattempt, this is a VA business choice). |
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.
sp! "causing the failure"
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.
Decision was made that once we start sending emails, with confirmation that email was sent, we/VFS will not attempt to resubmit.
|
||
|
||
#### Unauthenticated User Document Uploads | ||
As of now we are not sure the unauthenticated document upload should be any different that authenitcated. We still have a duty to ensure that the document gets where it was going, and potentially a duty to notify the uploader if we cannot. |
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.
typos: "different than authenticated"
Event bus would be a more ideal solution. Polling is often messy or sloppy and not great from an engineering standpoint. However, since most APIs already support a status or introspection endpoint, and also since most APIs are not able to support/prioritize event bus support, this was ruled out as not feasible, even though it is a better solution technically. | ||
|
||
##### Webhooks | ||
Webhooks would be a more ideal solution. Polling is often messy or sloppy and not great from an engineering standpoint. However, since most APIs already support a status or introspection endpoint, and also since most APIs are not able to support/prioritize adding webhook/callback support, this was ruled out as not feasible, even though it is a better solution technically. It would also be a slightly higher level of effort as callback endpoints would need made in vets-api, but we do not belive that to be the major considerationa s to why this was not selected. |
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.
typo: "major considerationa s to why"
|
||
|
||
#### Other considerations | ||
All documents that cannot go their primary path (Documents API) should have a backup path/failsafe by attempting submission via the benefits intake API, unless your primary path is the benefits API or something that uses the benefits API downstream. |
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.
benefits API or something that uses the benefits API
should say
Benefits Intake API or something that uses the Benefits Intake API
|
||
All other checks should be done by the system downstream that is ultimately responsible for accepting the file or not. We should not try to maintain separate lists of requirements that can get out of sync. We should, after initial validation above, check the file against a validation endpoint for the downstream system. | ||
|
||
You may think, "why not just actually upload it here and rely on the error checking there, instead of uploading twice (once to validate and once to upload)". In places where this is possible, this should be done, however many evidence documents cannot be uploaded without first knowing a claim ID which must be sent with the document to be uploaded, which is not yet known at document upload time, and only later known after initial form submission. |
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.
many evidence documents cannot be uploaded without first knowing a claim ID which must be sent with the document to be uploaded
Is this also true with the Claim Evidence API? My naïve read of their Swagger doc makes it seem like file upload and associating a file with a claim ID are separate endpoints.
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.
Re-adding some context from Slack - in the near term we're using LH & polling, but in the longer term, the enablement team is interested in whether directly using BIP Claims Evidence API would be an improvement for our products with regards to developer experience, product simplicity, more features, less middleware, etc.
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.
Im not familiar with the BIPS api very much, Ive never used it, but the Lighthouse endpoints that do use it, uploads are comprised of 2 calls, one to plop in the file to the eFolder, and another to associate said file with a particular claim ID.
For 526 related docs, https://developer.va.gov/explore/api/benefits-documents/docs?version=current Benefits documents api, requires a claim ID. One portion of what the call does, which is upload to efolder, might not require that, but the call overall that va.gov makes to lighthouse, often requires it.
Could we bypass lighthouse and make our own calls directly to BIPS or other apis? Sure. But we've been given direction to prefer/standardize to using lighthouse when possible/supported, from what I am aware.
No description provided.