Backend/SV: Differentiate failures due to birthdate vs gender mismatch#9182
Backend/SV: Differentiate failures due to birthdate vs gender mismatch#9182tristanlabelle wants to merge 9 commits into
Conversation
Strong Verification failures that came from a birthdate or gender mismatch all emitted a single combined reason, so users couldn't tell which field actually failed. Add SV_FAIL_REASON_WRONG_BIRTHDATE and SV_FAIL_REASON_WRONG_GENDER and, in the failure branch, evaluate matches_birthdate/matches_gender separately to pick a specific reason, keeping the combined reason only when both mismatch. Split the email and push notification copy accordingly. Co-authored-by: Tristan Labelle <tristanlabelle@users.noreply.github.com>
tristanlabelle
changed the title
Web (Vercel)View the Vercel web preview for this branch. Backend
Other
|
839f2fd to
3f6da88
Compare
aapeliv
left a comment
aapeliv
left a comment
There was a problem hiding this comment.
fine as-is if you have verified this works; added some comments
| StrongVerificationAttemptStatus.in_progress_waiting_on_user_in_app: account_pb2.STRONG_VERIFICATION_ATTEMPT_STATUS_IN_PROGRESS_WAITING_ON_USER_IN_APP, | ||
| StrongVerificationAttemptStatus.in_progress_waiting_on_backend: account_pb2.STRONG_VERIFICATION_ATTEMPT_STATUS_IN_PROGRESS_WAITING_ON_BACKEND, | ||
| StrongVerificationAttemptStatus.failed: account_pb2.STRONG_VERIFICATION_ATTEMPT_STATUS_FAILED, | ||
| StrongVerificationAttemptStatus.duplicate: account_pb2.STRONG_VERIFICATION_ATTEMPT_STATUS_FAILED, |
There was a problem hiding this comment.
The API was returning UNKNOWN as the status in this case. In the test I expected FAILED. It seemed like an oversight?
The semantics are not super clear as to whether it's querying that the verification attempt succeeded (they provided a legit passport), in which case this api should return SUCCESS, or that it resulted in obtaining a strong verification, in this case it makes sense to return FAILED.
| SV_FAIL_REASON_UNKNOWN = 0; | ||
| // birthdate doesn't match profile or gender | ||
| // both birthdate and gender don't match the profile | ||
| SV_FAIL_REASON_WRONG_BIRTHDATE_OR_GENDER = 1; |
There was a problem hiding this comment.
then should this be renamed to AND? and also should we update the translation strings?
There was a problem hiding this comment.
That makes sense. I was wondering if to make it into a more generic failure but handling three cases is still manageable.
Anything I can do to verify Strong Verification on top of the tests I added/updated? |
i guess you can't on dev. once it's on stage you can test there though. |
3 participants
Users were confused from the message saying that their passport date of birth or gender mismatched their profile, so add enumerants that differentiate those cases and update notifications.
To avoid duplicating more test code, I refactored the IRIS callback mocking logic (see changes to existing tests).
Also fixes
GetStrongVerificationAttemptStatusreturningUNKNOWNin the duplicate status case.With help from Claude Code
Closes #9174.
Testing
Added tests.
Backend checklist
developif necessary for linear migration historyFor maintainers