Skip to content

LG-13009 - consolidate poor quality and not live messaging#10536

Merged
amirbey merged 7 commits intomainfrom
amirbey/LG-13009-expected-face-error-msg
May 2, 2024
Merged

LG-13009 - consolidate poor quality and not live messaging#10536
amirbey merged 7 commits intomainfrom
amirbey/LG-13009-expected-face-error-msg

Conversation

@amirbey
Copy link
Contributor

@amirbey amirbey commented May 1, 2024

🎫 Ticket

Link to the relevant ticket:
LG-13009

🛠 Summary of changes

Consolidate duplicate messaging for selfie not live and selfie poor quality.

AC-1 error messages already handled
AC 2 for LG-13009 resolved in PR #10450

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • submit doc auth w/ selfie images with a Liveness: PoorQuality face error message and verify Try taking a photo of yourself again. Make sure your whole face is clear and visible in the photo. is displayed
  • submit doc auth w/ selfie images with Liveness: Error face error message and verify general selfie failure message Try taking your photos again. Make sure all of your photos are clear and in focus. is displayed

👀 Screenshots

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

Liveness Failure:

LG-13009-liveness-not-live

Portrait Match Filure/Liveness Error:

LG-13009-liveness-error

@amirbey amirbey self-assigned this May 1, 2024
@amirbey amirbey marked this pull request as ready for review May 1, 2024 14:54
@amirbey amirbey force-pushed the amirbey/LG-13009-expected-face-error-msg branch from a8c53f4 to b3bee0a Compare May 1, 2024 16:29
Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch!

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL blank includes nil!

Copy link
Contributor

Choose a reason for hiding this comment

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

this method name was very confusing

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.

This mostly looks good to me. I tried manually testing and the only potential issue I saw was that in a liveness error, I only saw the inline error on the selfie section, while your screenshots showed it in all 3 places. I'm not sure if that's big enough to block this work, but I thought I would mention it here in case it's unexpected.

@amirbey
Copy link
Contributor Author

amirbey commented May 2, 2024

This mostly looks good to me. I tried manually testing and the only potential issue I saw was that in a liveness error, I only saw the inline error on the selfie section, while your screenshots showed it in all 3 places. I'm not sure if that's big enough to block this work, but I thought I would mention it here in case it's unexpected.

Thanks for the review Brittany! As a sanity check, I just retested with FaceErrorMessage: Liveness: Error and saw the inline error on all 3 image fields and FaceErrorMessage: Liveness: NotLive displayed the inline error only on the selfie field 👍🏿 ... Lmk if you want revisit this and if there's still a concern. Thanks 🙏🏿

@amirbey amirbey force-pushed the amirbey/LG-13009-expected-face-error-msg branch from 8214768 to d717b49 Compare May 2, 2024 16:00
@amirbey amirbey force-pushed the amirbey/LG-13009-expected-face-error-msg branch from d717b49 to 130e41c Compare May 2, 2024 17:04
@amirbey amirbey merged commit f9f029a into main May 2, 2024
@amirbey amirbey deleted the amirbey/LG-13009-expected-face-error-msg branch May 2, 2024 18:19
samathad2023 pushed a commit that referenced this pull request May 11, 2024
* resolve conflict encountered during main rebase

* consolidate selfie not_live and poor_quality i18n

* update spec key for liveness error message

* [skip changelog]

* happy linting

* remove dup tests and add scenarios

* key no longer exists
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.

2 participants