Skip to content

Fix Acuant Error Handler Function Naming#9149

Merged
charleyf merged 7 commits intomainfrom
charley/fix-acuant-error-handler
Sep 6, 2023
Merged

Fix Acuant Error Handler Function Naming#9149
charleyf merged 7 commits intomainfrom
charley/fix-acuant-error-handler

Conversation

@charleyf
Copy link
Contributor

@charleyf charleyf commented Sep 5, 2023

🎫 Ticket

https://cm-jira.usa.gov/browse/LG-10900

🛠 Summary of changes

This work changes the name of an error handling function to what the AcuantSDK expects.

📜 Testing Plan

Extracted from the slack thread about this bug here.

  • Change your settings like this and restart your dev server. You can also do 11.8.2 to test that version only.
idv_acuant_sdk_version_default: '11.9.1'
idv_acuant_sdk_version_alternate: '11.9.1'
  • Get to the document upload page with FRONT and BACK image zones.
  • Click the "FRONT" upload zone, and deny the camera permissions.
  • Before this PR, you should get a white screen with an X in the corner. Clicking the "X" then trying to upload an image again doesn't give you the camera permissions popup again.
  • After this PR, you should be returned to the document upload screen and see an error. Trying to upload again should give you the camera permissions popup again.

👀 Screenshots

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

Camera permissions popup

IMG_0417

Before: White screen with X in the corner

image (1)

After: Error message

IMG_0416

@charleyf charleyf changed the title Fix error handler naming Fix Acuant Error Handler Function Naming Sep 5, 2023
@charleyf charleyf marked this pull request as ready for review September 6, 2023 14:21
@charleyf charleyf requested a review from eileen-nava September 6, 2023 14:23
Copy link
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

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

This worked when I tested it locally! Below I uploaded a video of me locally testing it. Thanks, Charley! 🎉 (I am also going to request that another @18F/identity-timnit-engineers team member review it, to be thorough.)

SDkBugFixWorked.mov

@charleyf Could you please fix the tests that broke due to the change from onFailure to onError, like the acuant capture jsx tests? Thanks!

@eileen-nava eileen-nava requested a review from a team September 6, 2023 16:06
Copy link
Contributor

@night-jellyfish night-jellyfish left a comment

Choose a reason for hiding this comment

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

I tested both before and after the changes and saw the expected difference! 🎉 I will approve once CI passes.

Copy link
Contributor

@night-jellyfish night-jellyfish left a comment

Choose a reason for hiding this comment

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

Thank you for fixing the tests!

@eileen-nava eileen-nava self-requested a review September 6, 2023 18:54
Copy link
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! 💯

@charleyf charleyf merged commit b230556 into main Sep 6, 2023
@charleyf charleyf deleted the charley/fix-acuant-error-handler branch September 6, 2023 20:41
@aduth aduth mentioned this pull request Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants