Skip to content

LG-16378: ID_TYPE_SLUGS for stateID vs passports#12301

Merged
amirbey merged 5 commits intomainfrom
amirbey/LG-16378-ID_TYPE_SLUGS-passports
Jun 30, 2025
Merged

LG-16378: ID_TYPE_SLUGS for stateID vs passports#12301
amirbey merged 5 commits intomainfrom
amirbey/LG-16378-ID_TYPE_SLUGS-passports

Conversation

@amirbey
Copy link
Contributor

@amirbey amirbey commented Jun 29, 2025

🎫 Ticket

LG-16378

🛠 Summary of changes

@amirbey amirbey changed the title Amirbey/lg 16378 id type slugs passports LG-16378: ID_TYPE_SLUGS for stateID vs passports Jun 29, 2025
@amirbey amirbey self-assigned this Jun 29, 2025
@amirbey amirbey requested review from mitchellhenke and solipet June 29, 2025 12:48
@amirbey amirbey marked this pull request as ready for review June 29, 2025 12:48
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Newline before it block

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add two contexts one for when passports are not enabled and one for when they are enabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't there be an update to the Lexis Nexis response. I feel like it should have a similar setup with id_type_supported? So that even if Lexis Nexis comes back with a passport we reject it. This leads me to think we need to ensure that we limit the supported types depending on which flow the user is in. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

This would most likely need to be new ticket

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the update to the LN response is handled here

Copy link
Contributor

Choose a reason for hiding this comment

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

Since you are making changes in here can you fix the indentation. The ends at the end and the private keyword seem to be off.

@amirbey amirbey force-pushed the amirbey/LG-16378-ID_TYPE_SLUGS-passports branch from 051de7f to 15cf12e Compare June 30, 2025 16:18
Copy link
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

@amirbey amirbey merged commit 8303c3e into main Jun 30, 2025
1 check passed
@amirbey amirbey deleted the amirbey/LG-16378-ID_TYPE_SLUGS-passports branch June 30, 2025 16:39
Copy link
Contributor

@solipet solipet left a comment

Choose a reason for hiding this comment

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

One suggestion, but LGTM

Comment on lines 11 to 15
ID_TYPE_SLUGS = {
'Identification Card' => 'state_id_card',
'Drivers License' => 'drivers_license',
'Passport' => 'passport',
}.freeze
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe define ID_TYPE_SLUGS after STATE_ID_TYPE_SLUGS so we eliminate the repeated strings?

Suggested change
ID_TYPE_SLUGS = {
'Identification Card' => 'state_id_card',
'Drivers License' => 'drivers_license',
'Passport' => 'passport',
}.freeze
ID_TYPE_SLUGS = STATE_ID_TYPE_SLUGS.merge({
'Passport' => 'passport'
}.freeze

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