Skip to content

LG-15980 Add passport info section to verify info form#12181

Merged
shanechesnutt-ft merged 1 commit intomainfrom
sc/LG-15980
May 19, 2025
Merged

LG-15980 Add passport info section to verify info form#12181
shanechesnutt-ft merged 1 commit intomainfrom
sc/LG-15980

Conversation

@shanechesnutt-ft
Copy link
Copy Markdown
Contributor

@shanechesnutt-ft shanechesnutt-ft commented May 14, 2025

🎫 Ticket

Link to the relevant ticket:
LG-15980

🛠 Summary of changes

Add passport info section to verify info form

📜 Testing Plan

Scenario: Verify Info page in IPP Passport Flow

  • Login through the oidc sinatra application selecting the Identity Verified level of service.
  • Create a new account
  • Complete the ID-IPP Passport flow reaching the Verify Info page
  • Ensure Passport Book (Surname, First name, Date of Birth), Address, SSN info is displayed
  • Ensure no update link is present in the passport section
  • Ensure Address info can be updated using the update link
  • Ensure SSN info can be updated using the update link
  • Ensure Verify Info page matches Figma design

Scenario: Verify Info page in IPP State-ID Flow

  • Login through the oidc sinatra application selecting the Identity Verified level of service.
  • Create a new account
  • Complete the ID-IPP State-ID flow reaching the Verify Info page
  • Ensure State-ID, Address, SSN info is displayed
  • Ensure State-ID info can be updated using the update link
  • Ensure Address info can be updated using the update link
  • Ensure SSN info can be updated using the update link

👀 Screenshots

Verify Info for Passport Screenshot 2025-05-19 at 10 27 13 AM
Verify Info for State-ID Screenshot 2025-05-19 at 10 25 40 AM

@shanechesnutt-ft shanechesnutt-ft marked this pull request as ready for review May 15, 2025 18:59
@shanechesnutt-ft shanechesnutt-ft force-pushed the sc/LG-15985 branch 2 times, most recently from 41700dc to 7e03df3 Compare May 16, 2025 13:11
Base automatically changed from sc/LG-15985 to main May 16, 2025 13:24
@shanechesnutt-ft shanechesnutt-ft force-pushed the sc/LG-15980 branch 2 times, most recently from d6b2c1a to e23d1fe Compare May 16, 2025 14:58
@gina-yamada
Copy link
Copy Markdown
Contributor

gina-yamada commented May 16, 2025

✅ Ensure Verify Info page matches Figma design
There are slight variations. The mocks might just be out of date. For example, the data displayed is all caps in the mocks but not on your branch. (It bet they are not on main.) Also, the SSN section display is a little different. Both seem out of scope. I think the Figma probably just needs to be updated. I would not make any changes in your branch. New ticket if that should be changed. Shane and Annie worked together on an agreed solution. Screenshots in the description of this PR were updated since this comment. See comments below. Marking as done.
✅ Testing plan
✅ Tested diff scenarios with same address as ID and diff address as ID
✅ Expected step nav
✅ Expected page navigation

@gina-yamada gina-yamada self-requested a review May 19, 2025 13:43
Copy link
Copy Markdown
Contributor

@gina-yamada gina-yamada left a comment

Choose a reason for hiding this comment

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

I tested with the flags off as well. Working as I'd expect. Nice work Shane!

@anniehirshman-gsa
Copy link
Copy Markdown
Contributor

❌ Ensure Verify Info page matches Figma design There are slight variations. The mocks might just be out of date. For example, the data displayed is all caps in the mocks but not on your branch. (It bet they are not on main.) Also, the SSN section display is a little different.

Thanks for flagging these!

  • Data in all caps: this should be cap/lc reflecting what the user inputs - I'll make a note to update this in Figma
  • SSN section: Should not display the second "Social Security number" label
  • Additionally, the current section headings have 8px top/bottom padding, which makes them misaligned from the "Update" button, and the overall spacing above and below the hrs is not even (should be 16px)

If it's easy to fix the SSN label and spacing to match Figma now, that would be ideal, but keep me posted if it needs more time and a separate ticket. Thanks again!!

Copy link
Copy Markdown
Contributor

@anniehirshman-gsa anniehirshman-gsa left a comment

Choose a reason for hiding this comment

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

Screenshot LGTM - with one comment about (preexisting) inconsistencies with Figma that would be great to fix if possible

@shanechesnutt-ft
Copy link
Copy Markdown
Contributor Author

Screenshot LGTM - with one comment about (preexisting) inconsistencies with Figma that would be great to fix if possible

@anniehirshman-gsa I pushed up a fix for those inconsistencies with Figma. I also updated my screenshots with the changes that I have made.

changelog: Upcoming Features, In-person Proofing, Add passport info section to verify info form
@shanechesnutt-ft shanechesnutt-ft merged commit 4bb3b4b into main May 19, 2025
1 check passed
@shanechesnutt-ft shanechesnutt-ft deleted the sc/LG-15980 branch May 19, 2025 17:15
Copy link
Copy Markdown
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

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

I know this is already merged, I just wanted to chime in and say LGTM. 👍🏻


def enrolled_with_passport_book?
enrollment.document_type == InPersonEnrollment::DOCUMENT_TYPE_PASSPORT_BOOK
enrollment.passport_book?
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.

👏🏻 This is helpful for clarity.


let(:ssn_pii) do
{
ssn: '666' + Faker::Number.number(digits: 6).to_s,
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.

Nice use of a prefix and Faker to ensure the ssn will be recognized as fake. 👍🏻

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.

5 participants