Skip to content

LG-15779: verify docv id type#11895

Merged
amirbey merged 5 commits intomainfrom
amirbey/LG-15779-docv-id-type
Feb 19, 2025
Merged

LG-15779: verify docv id type#11895
amirbey merged 5 commits intomainfrom
amirbey/LG-15779-docv-id-type

Conversation

@amirbey
Copy link
Copy Markdown
Contributor

@amirbey amirbey commented Feb 18, 2025

🎫 Ticket

Link to the relevant ticket:
LG-15779

🛠 Summary of changes

Check the document type received in the verification data to confirm either a license or state ID was authenticated. If not, send the user to the unaccepted ID type error page.

📜 Testing Plan

  • Automated test as the DocV Sandbox environment does not return an invalid doc type.

👀 Screenshots

Screenshot 2025-02-19 at 8 22 47 AM

@amirbey amirbey changed the title verify docv id type LG-15779: verify docv id type Feb 18, 2025
@amirbey amirbey marked this pull request as ready for review February 19, 2025 13:19
Copy link
Copy Markdown
Contributor

@AShukla-GSA AShukla-GSA left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@jmax-gsa jmax-gsa left a comment

Choose a reason for hiding this comment

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

Mostly looks nice. I added one concrete suggestion about the error messaging. Approving; take or leave the suggestion.

(Also, the hordes of parallel if-elsif-elsifs and switch statements for our different kinds of errors really want to be an error class of some sort, but today is not that day.)

LGTM

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.

👍

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.

For network errors with no additional information available, we just say network: true. Do we want to follow the same pattern here, and say unaccepted_id_type: true? (If we have more information, of course, we could say something like {unaccepted_id_type: 'library_card'}, but I don't think we do here.)

@amirbey amirbey force-pushed the amirbey/LG-15779-docv-id-type branch from d902185 to df774c5 Compare February 19, 2025 16:12
@amirbey amirbey merged commit 003bdc5 into main Feb 19, 2025
2 checks passed
@amirbey amirbey deleted the amirbey/LG-15779-docv-id-type branch February 19, 2025 16:36
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