Skip to content

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

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.

Sign up for GitHub

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

Merged
amirbey merged 1 commit into from
Feb 20, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/presenters/image_upload_response_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def attention_with_barcode?
end

@night-jellyfish night-jellyfish Feb 16, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Screenshot 2024-02-16 at 10 46 18 AM

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 dawei-nava Feb 16, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@dawei-nava dawei-nava Feb 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?


def ocr_pii

@aduth aduth Feb 15, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 amirbey Feb 15, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@night-jellyfish night-jellyfish Feb 16, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 dawei-nava Feb 16, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

return unless success?
return unless attention_with_barcode? && @form_response.respond_to?(:pii_from_doc)
@form_response.pii_from_doc&.slice(:first_name, :last_name, :dob)
end
Expand Down
9 changes: 4 additions & 5 deletions spec/features/idv/doc_auth/document_capture_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,15 @@
attach_images(
Rails.root.join(
'spec', 'fixtures',
'ial2_test_credential_barcode_attention_no_dob.yml'
'ial2_test_credential_barcode_attention_no_address.yml'

@night-jellyfish night-jellyfish Feb 16, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 night-jellyfish Feb 16, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 amirbey Feb 16, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

),
)
submit_images

expect(page).to have_content(t('doc_auth.errors.barcode_attention.heading'))
click_idv_continue

# should show try again
expect(page).to have_content(t('doc_auth.errors.alerts.address_check'))
expect(page).to have_current_path(idv_document_capture_path)

click_try_again
attach_images
submit_images
expect(page).to have_current_path(idv_ssn_path)
Expand Down
14 changes: 14 additions & 0 deletions spec/fixtures/ial2_test_credential_barcode_attention_no_address.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
document:
first_name: Jane
last_name: Doe
middle_name: Q
city: Bayside
state: NY
zipcode: '11364'
dob: 10/06/1938
phone: +1 314-555-1212
state_id_jurisdiction: 'NY'
state_id_number: 'S59397998'
failed_alerts:
- name: 2D Barcode Read
result: Attention