LG-10427 prevent doc image resubmission validation

[dawei-nava]

🎫 Ticket

LG-10427 : Prevent the user from submitting an image that failed validations on a past attempt to submit

🛠 Summary of changes

1. Store failed doc auth image fingerprints in document capture session result, both front and back.
2. During image upload, verify the submitted image has not been failed in current capture session.
3. On the front end, image files are also checked when a user attempts to choose a previously failed image.
4. New inline error message displayed for corresponding image.
5. New analytics event generated to indicate there is attempt to do so, along with image metadata.

Note: failed image means failures due to quality or business reason, not because network errors.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Step 1: Initiate doc auth workflow.
• Step 2: Submit an image or a test yaml file that cause failure.
• Step 3: Submit a different image a test yaml file that cause failure.
• Step 4: Choose the first failed file again, the inline error message should show.
• Step 5: Repeat similar on mobile.

Note: For mobile captured image(whether by SDK or by native camera), it should not trigger this, since rarely images taken at different time will be exactly the same.

👀 Screenshots

Before:

After:

[eileen-nava]

I left one question. I will finish reviewing this on Monday. The test coverage looks good for api_image_upload_form.rb. 👍🏻

[eileen-nava]

👍🏻

[eileen-nava]

🙌🏻

[eileen-nava]

I find lines 418-421 a little hard to follow.

My current understanding is that if errors_hash.with_indifferent_access.dig(:front) returns something, then front_fingerprint would be set to equal extra_attributes[:front_image_fingerprint].

I don't understand the purpose of the || !side_error portion of

front_fingerprint = (errors_hash.with_indifferent_access.dig(:front) || !side_error) ? extra_attributes[:front_image_fingerprint] : nil

My current understanding is that if neither side had any errors, meaning side_error=false, then(errors_hash.with_indifferent_access.dig(:front) || !side_error) would equal nil || true which would cause front_fingerprint to equal extra_attributes[:front_image_fingerprint].... But I thought we'd want it to equal nil, if there are no errors associated with either side?

Could you please help me to grok this section of the code? Thanks!

[dawei-nava]

@eileen-nava , refactored it to be more clear.

[kellular]

Hi! The screenshot for the English translation is not loading:

[eileen-nava]

I left my main piece of feedback on app/javascript/packages/document-capture/components/acuant-capture.tsx. I noticed some unexpected behavior when doing manual QA.

Test coverage looks good. 👍🏻

[eileen-nava]

I want to be sure I understand the need for this update to the guard statement. Were you trying to ensure that the rate limiter didn't get stale attempts data?

[dawei-nava]

@eileen-nava, yes, because the timing of the extra_attributes called, it may cache stale data ( the attempts).

[eileen-nava]

Thanks for leaving this comment. At first the ^ operator threw me a little bit.

[dawei-nava]

@eileen-nava , have to google to find out this XOR logic operator.

[eileen-nava]

@dawei-nava I tested locally and attempted to resubmit a yml file that had already failed. Inline errors did display that said You already tried this image, and it failed. Please try adding a different image. Awesome!

However, I was still able to click "submit" and proceed to the next error screen when there were inline errors displaying above both upload zones. Is it possible to prevent the user from clicking submit and proceeding when there are inline errors present above the upload zones? Thanks!

[night-jellyfish]

I saw the same behavior. I also saw that if I reused both the same "front" and "back" image, the inline error only showed on the "back" image upload section. I'm not sure but I think we'd want it to display in both sections, or if only one, the "front" section first.

In addition, when I tried again with just a failure on the "front" image upload (ial2_test_credential_forces_error.yml) but a successful image for the "back" (ial2_test_credential.yml), validation succeeded and I moved on to the SSN page. If I switched those images around (ial2_test_credential_forces_error.yml for the back and ial2_test_credential.yml for the front) , the submission fails as expected.

The combination of these two makes me wonder if something is broken with the front upload section, but I'm going to test on main first to make sure this is a problem with this branch.

[night-jellyfish]

Hmm...I'm seeing the second issue in main.

[dawei-nava]

@night-jellyfish , that's because the mock client you cannot really testing two different files when upload image.

[dawei-nava]

@eileen-nava, that's the original intention, seems there is a issue with ordering/extra stuff. Should work now.

[eileen-nava]

@dawei-nava Thanks! I am logging off now but will look at this tomorrow. 👍🏻

[night-jellyfish]

Thanks for the info, Dawei!

I pulled the updates and tested again with ial2_test_credential_no_dob.yml and saw the following:

1. I now saw the inline error message on the front image (yay!)
2. Unfortunately I was still able to submit the first time I re-added the same images (so, the second time I uploaded the images)
3. If I tried a second time to re-add the images (so, third time uploading the same images) then I could not hit submit and it moved my cursor focus to the top image select.

[night-jellyfish]

Interestingly, if I try again but use ial2_test_credential_forces_error.yml for both front and back, I see the opposite effect:

1. I don't see the inline error for the front image (it says "Image updated" happily in green)
2. But, I am not allowed to submit when I re-add that file (as expected)

So I'm seeing different results with different types of errors / yml files. Maybe this is a yml issue?

[dawei-nava]

@night-jellyfish
With that file, first time to submit:

Looks like it classifes only as a back side error.

Second time:

It alerts it is duplicate submission for the back only.

Note: Sometimes it has cached/not compiled javascript files, run bundle exec rake javascript:build to manually rebuild javascript packs and force browser reload.

[dawei-nava]

@eileen-nava issue should have been resolved.

[aduth]

Using // @ts-ignore here masks the fact that the type is invalid, so will effectively be treated as an any.

We should avoid @ts-ignore by making this a valid type.

Alternatively, I've considered in the past I'd to pull in a TypeScript utility package like type-fest, which has various JSON types we could use here? Maybe we can add that as a devDependencies for the project.

https://github.com/sindresorhus/type-fest#json

[dawei-nava]

Revert back to string.

[aduth]

Can we use a more useful type than any here, if we know it? We should avoid any as much as we can.

[dawei-nava]

I use a anoymous type here and thought it should be a type of ImageAnalyticsPayload, but not 100% sure.

[eileen-nava]

approved, but I'd prefer if we did not use @ts-ignore

[dawei-nava]

@kellular , are you good with the wording/ui?

[dawei-nava]

approved, but I'd prefer if we did not use @ts-ignore

@eileen-nava forgot to remove after cleanup.

[kellular]

The screenshots and new error messages LGTM