Skip to content

LG-10289: Fix cropping error message#8849

Closed
night-jellyfish wants to merge 2 commits intomainfrom
brittany/lg-10289-fix-cropping-error-message
Closed

LG-10289: Fix cropping error message#8849
night-jellyfish wants to merge 2 commits intomainfrom
brittany/lg-10289-fix-cropping-error-message

Conversation

@night-jellyfish
Copy link
Contributor

🎫 Ticket

LG-10289

🛠 Summary of changes

Before this change, when the camera cropping failed in the SDK, the user
would get a Camera failed to start, please try again. error message.

This is confusing because to get to the cropping failure, the user's
camera would have had to start. In the case of a cropping error, we want
to give a general error message instead.

📜 Testing Plan

  • Follow the flow through to the upload image section.
  • When uploading an image, upload it in such a way that you cause a cropping error.
  • Make sure that the error message you see is "Oops, something went wrong. Please try again." and not "Camera failed to start, please try again."

Brittany Greaner added 2 commits July 24, 2023 19:44
[Ticket](https://cm-jira.usa.gov/browse/LG-10289)

Before this change, when the camera cropping failed in the SDK, the user
would get a `Camera failed to start, please try again.` error message.

This is confusing because to get to the cropping failure, the user's
camera would have had to start. In the case of a cropping error, we want
to give a general error message instead.
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Left a question, but looks good 👍

);

initialize({
start: sinon.stub().callsArgWithAsync(1, undefined, 'start-fail-code'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these parameters accurate for what is sent when a crop error occurs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @aduth, I am responding to feedback for this PR because Brittany will be out of office for a bit. Do you happen to know how to trigger a crop error? I wanted to follow up on your question. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not aware of how to trigger it. Part of the reason for the question was trying to understand where in the Acuant source this actually occurs, since the other error expected types were originally sourced by following all of the failure paths.

@eileen-nava
Copy link
Contributor

Just posting this for visibility, @charleyf is going to take over this ticket now that he is back from being OOO and Brittany is OOO. He will probably start a new PR.

charleyf added a commit that referenced this pull request Jul 25, 2023
charleyf added a commit that referenced this pull request Aug 25, 2023
#8860)

* Adopt test from earlier PR (#8849)

* Lint to fix import ordering

* Extract onAcuantImageCaptureFailure function

* Give cropping errors a more general error message.

* Add comment about cropping failure callback

* Remove incorrect comment

* Re-add test and fix test naming
@night-jellyfish
Copy link
Contributor Author

This work has been done in #8860.

@night-jellyfish night-jellyfish deleted the brittany/lg-10289-fix-cropping-error-message branch September 6, 2023 16:02
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