Skip to content

LG-16384: passport cards are not state ids#12310

Merged
solipet merged 3 commits intomainfrom
dprice/lg-16384-passports-are-not-state-ids
Jul 2, 2025
Merged

LG-16384: passport cards are not state ids#12310
solipet merged 3 commits intomainfrom
dprice/lg-16384-passports-are-not-state-ids

Conversation

@solipet
Copy link
Copy Markdown
Contributor

@solipet solipet commented Jul 1, 2025

🎫 Ticket

Link to the relevant ticket:
LG-16384

🛠 Summary of changes

Check for the DocIssueType to see if the presented id is a passport card and set the id_doc_type appropriately.
Fails doc auth when passport card and fails doc auth when passport presented but passports are not enabled.

📜 Testing Plan

Specs pass

@solipet solipet changed the title LG-16384: passports are not state ids LG-16384: passport cards are not state ids Jul 1, 2025
@solipet solipet requested a review from mitchellhenke July 1, 2025 21:33
Copy link
Copy Markdown
Contributor

@theabrad theabrad Jul 2, 2025

Choose a reason for hiding this comment

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

We should also test for passport_card in doc_pii_form_spec.rb. I didn't make a test for this because I tested all the scenarios in doc_pii_form_spec, but this is a nice addition!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 9e55565

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, although other comment suggestions are a good idea!

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.

I found myself doing a double-take on this method name since it's checking for passport cards but named passport_book?. Would passport_card_not_supported? or validate_not_passport_card be clearer?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed it to not_passport_book? in 9e55565

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, per Amir's comment, I changed it back to passport_book? but fail if id_doc_type is anything but 'passport' in 03a86f2

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.

What do you think about extracting this to a constant like PASSPORT_CARD_ISSUE_TYPE? Might help if we need to reference this LexisNexis value elsewhere or if it ever changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agree - adding this to LG-16371

Copy link
Copy Markdown
Contributor

@Mawar2 Mawar2 left a comment

Choose a reason for hiding this comment

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

LGTM - left 2 minor comments but good to merge as-is.

Fixes a bug where passport cards were being recorded with an id_doc_type
of state_id_card.

Also adds guards to reject any passport deemed passed by LN if passports
are disabled on our end.

changelog: Bug Fixes, Passports, Correct the id_doc_type for passport cards.

Co-authored-by: Shane Chesnutt <shane.chesnutt@gsa.gov>
Co-authored-by: William Birdsall <william.birdsall@gsa.gov>
Co-authored-by: Abir Shukla <abir.shukla@gsa.gov>
Co-authored-by: Alexander Bradley <alexander.bradley@gsa.gov>
Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>
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.

Suggested change
if pii_from_doc[:id_doc_type] == 'passport_card'
if pii_from_doc[:id_doc_type] != 'passport'

could be helpful if not 'passport' to make any values not a passport throw the error

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated in 03a86f2

@solipet solipet force-pushed the dprice/lg-16384-passports-are-not-state-ids branch from ee3d336 to 9e55565 Compare July 2, 2025 20:52
Copy link
Copy Markdown
Contributor

@theabrad theabrad left a comment

Choose a reason for hiding this comment

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

LGTM

@solipet solipet merged commit 21b9155 into main Jul 2, 2025
1 check passed
@solipet solipet deleted the dprice/lg-16384-passports-are-not-state-ids branch July 2, 2025 21:28
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.

6 participants