LG-12393: barcode attention should only be shown when image upload is successful

[amirbey]

🎫 Ticket

Link to the relevant ticket:
LG-12393

🛠 Summary of changes

Only present BarcodeWarning when the image upload is successful as in the doc auth and pii validation is successful.

📜 Testing Plan

• Submit doc auth image/yml that contains invalid pii and an attention doc auth result.
• Veify the failed ID screen is shown with the option to Try Again

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Before:

After:

[aduth]

I thought we only did anything with this if it was unsuccessful?

Noting how the use of it...

identity-idp/app/javascript/packages/document-capture/services/upload.ts

Lines 119 to 121 in 4926cdd

	if (result.ocr_pii) {
	error.pii = result.ocr_pii;
	}

...is within a block to check for unsuccessful:

identity-idp/app/javascript/packages/document-capture/services/upload.ts

Line 102 in 4926cdd

if (!result.success) {

[amirbey]

👋🏿 @aduth ... for the BarcodeAttentionWarning screen to appear, the barcode attention should be the only reason why the result is failed

[night-jellyfish]

This probably doesn't matter too much - but I tried out your testing plan. On your branch it worked perfectly, just as described!

On main, I did see a little bit of different behavior than your "before" screenshot, so I just wanted to note it here. Instead of seeing the "technical difficulties" message, I got a "barcode" warning screen showing me the pii from the yml, including that the dob was missing, but no dob message.

On both main and your branch I used a yml file that had an attention with barcode message and a missing DOB.

Ultimately since the end result was the same as expected, it's probably not too important, but wanted to note it.

[dawei-nava]

@night-jellyfish , i think there is some front end logic check attention with barcode to display it or not.

[dawei-nava]

@night-jellyfish
On front end, we have error and pii(which is the ocr_pii from backend).

identity-idp/app/javascript/packages/document-capture/components/review-issues-step.tsx

Lines 112 to 114 in e479616

	if (!hasDismissed && pii) {
	return <BarcodeAttentionWarning onDismiss={onWarningPageDismissed} pii={pii} />;
	}

Based on LG-5918, it seems suggesting that these ocr_pii fields missing/error can come with Attention with barcode read as the only error(Attention), while other pii errors should cause some cross checking errors too(meaning barcode read is not the only error).

So I am wondering whether we are testing some pii scenarios(not related firstname, lastname and dob, OCRed fileds, like missing address) that were thought not realistic from LG-5918.

@aduth, do you have any insight or memory on this part?

[night-jellyfish]

I feel like I should probably know this...but what is OCR?

I tried doing a quick search but came across references that seemed to range from a concept to a specific org, so I wanted to clarify.

[dawei-nava]

@night-jellyfish , I think OCR here means optical character recognization?

[night-jellyfish]

This is curious to me, especially given my note above - is there a reason you changed from no dob to no address? I would have thought all pii would behave the same.

[night-jellyfish]

I also found that this is the only place we use the no_dob.yml.

If we are switching to no_address.yml should we remove the un-used no_dob?

[amirbey]

while debugging this and trying to understand the issue, there was a period where I thought the underlying issue was something different and had created the address file along the way. Having gotten to the bottom of the issue, I understand that it doesn't matter whether it's dob or address. I left the address as because I figured it'd be good to have more test files.

[eileen-nava]

I probably would have fixed this by making a change to review-issues-step.tsx and changing this line to if (errors.length == 0 && !hasDismissed && pii) { , but changing it in the presenter works, too.

LGTM! 👏🏻