Skip to content

LG-9018 Puerto Rico address guidance#7964

Merged
soniaconnolly merged 10 commits intomainfrom
sonia-lg-9018-puerto-rico-address-guidance
Mar 10, 2023
Merged

LG-9018 Puerto Rico address guidance#7964
soniaconnolly merged 10 commits intomainfrom
sonia-lg-9018-puerto-rico-address-guidance

Conversation

@soniaconnolly
Copy link
Contributor

🎫 Ticket

LG-9018

🛠 Summary of changes

Puerto Rico residents have a very low success rate with Lexisnexis Instant Verify. It appears that it expects the Urbanization part of the address to be on line 2. To gather data and take a first step toward helping Puerto Rico residents complete identity verification successfully, add guidance on how to edit their address to the Update Address screen.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Create account
  • Upload Puerto Rico-based ID or yml file
  • Click Update Address and note extra banner and Example hints
  • Change address to non-Puerto Rico address.
  • Click Update Address again and note no banner, no Example hints

👀 Screenshots

After:

Screenshot 2023-03-09 at 4 22 49 PM

Puerto Rico residents have a very low success rate with Lexisnexis Instant Verify. It appears that it expects the Urbanization part of the address to be on line 2. To gather data and take a first step toward helping Puerto Rico residents complete identity verification successfully, add guidance on how to edit their address.

changelog: User-facing Improvements, Identity Verification, Add guidance on how to edit Puerto Rico addresses for improved IdV success
@soniaconnolly soniaconnolly requested a review from a team March 10, 2023 00:39
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

end

def address_line1_hint
I18n.t('forms.example') + ' 150 Calle A Apt 3' if puerto_rico_address?
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. I feel like we'd typically put these strings like ' 150 Calle A Apt 3' in i18n? I know this is specifically for one state, and would probably be the same across languages, but it seems wrong to have a plain string like this
  2. I feel like the trailing if is a bit sneaky here, the combo of the + makes me wonder which part the if "attaches" to?

Maybe we could write this as:

Suggested change
I18n.t('forms.example') + ' 150 Calle A Apt 3' if puerto_rico_address?
safe_join([I18n.t('forms.example'), '150 Calle A Apt 3'], ' ') if puerto_rico_address?

(we might have to include ActionView::Helpers::OutputSafety for safe_join)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree the bare string looks odd, but we don't want to translate street names. I followed the pattern of the ssn field. This uses the existing "Example" translation rather than adding a string to each yml file for each hint on the page, which is why I hesitate to add them all.

I also agree the trailing if raises questions about precedence. I'm happy to use a block instead if rubocop will allow. Do we need safe_join since there's no HTML or user input involved?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing that Rubocop will say the line is too short for the block form of if. And no, we don'd need safe_join... unless t('forms.example') ever contains HTML, which it might one day? 🤷

in the meantime this might be a good compromise for readability

Suggested change
I18n.t('forms.example') + ' 150 Calle A Apt 3' if puerto_rico_address?
"#{I18n.t('forms.example')} 150 Calle A Apt 3" if puerto_rico_address?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, did that.

Comment on lines +121 to +123
attach_file 'Front of your ID', File.expand_path(yml_file)
attach_file 'Back of your ID', File.expand_path(yml_file)
click_on 'Submit'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use the t helper here to avoid duplicating the source of truth for UI strings (and in the off-chance we'd ever want to run tests in a language other than English).

Copy link
Contributor Author

@soniaconnolly soniaconnolly Mar 10, 2023

Choose a reason for hiding this comment

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

This yml_file is a proofing yml for document capture. Were you thinking it was a translation file, or is there a way to use t helpers to find proofing ymls? I can update the name.

Or were you referring to 'Front of your ID' ? I can certainly put in the t string for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe Andrew is referring to .* of your ID and Submit here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right, the Submit too!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, meaning to swap the hard-coded "Front of your ID", "Back of your ID", and "Submit" labels with whatever the equivalent t call would be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, did that here, and in a few more places. Also this reminded me to call this helper method in the test I originally copied the code from!

@soniaconnolly soniaconnolly merged this pull request into main Mar 10, 2023
@soniaconnolly soniaconnolly deleted the sonia-lg-9018-puerto-rico-address-guidance branch March 10, 2023 17:55
mitchellhenke pushed a commit that referenced this pull request Mar 14, 2023
Puerto Rico residents have a very low success rate with Lexisnexis Instant Verify. It appears that it expects the Urbanization part of the address to be on line 2. To gather data and take a first step toward helping Puerto Rico residents complete identity verification successfully, add guidance on how to edit their address.

changelog: User-facing Improvements, Identity Verification, Add guidance on how to edit Puerto Rico addresses for improved IdV success

* Change address entry field labels and fix newline in alert msg

* Show guidance only if state is PR, not if issuer is PR

* Add AddressPresenter to conditionally show edit field hints

* New address for Puerto Rico yml fixture

* Change AddressPresenter to take @pii

* Update banner text per discussion with Princess

* Replace a few explicit strings in specs with I18n.t lookups

* Make hint strings more readable
zachmargolis pushed a commit that referenced this pull request Mar 14, 2023
Puerto Rico residents have a very low success rate with Lexisnexis Instant Verify. It appears that it expects the Urbanization part of the address to be on line 2. To gather data and take a first step toward helping Puerto Rico residents complete identity verification successfully, add guidance on how to edit their address.

changelog: User-facing Improvements, Identity Verification, Add guidance on how to edit Puerto Rico addresses for improved IdV success

* Change address entry field labels and fix newline in alert msg

* Show guidance only if state is PR, not if issuer is PR

* Add AddressPresenter to conditionally show edit field hints

* New address for Puerto Rico yml fixture

* Change AddressPresenter to take @pii

* Update banner text per discussion with Princess

* Replace a few explicit strings in specs with I18n.t lookups

* Make hint strings more readable
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.

4 participants