-
Notifications
You must be signed in to change notification settings - Fork 166
Lg 7265 - Inherited Proofing - Display user data on Please Verify page #7065
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
6d48735
Display user data on the please verify page (#7265)
holytoastr afc984a
Clean up specs
holytoastr 42d11df
Further clean up
holytoastr 6182654
Remove remote proofing check
holytoastr 4bd51ae
Minor styling fixes - quick
holytoastr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| <%# | ||
| locals: | ||
| pii - user's information | ||
| remote_identity_proofing - true if we're doing RIDP, false if we're doing IPP | ||
| step_url - generator for step URLs | ||
| %> | ||
| <% title t('inherited_proofing.headings.verify_information') %> | ||
|
|
||
| <%= render PageHeadingComponent.new.with_content(t('titles.idv.verify_info')) %> | ||
| <div class='margin-top-4 margin-bottom-2'> | ||
| <div class="grid-row grid-gap grid-gap-2 padding-bottom-1"> | ||
| <dl class="grid-col-fill margin-bottom-0"> | ||
| <div> | ||
| <dt class="display-inline"> <%= t('idv.form.first_name') %>: </dt> | ||
| <dd class="display-inline margin-left-0"> <%= pii[:first_name].upcase %> </dd> | ||
| </div> | ||
| <div> | ||
| <dt class="display-inline"> <%= t('idv.form.last_name') %>: </dt> | ||
| <dd class="display-inline margin-left-0"> <%= pii[:last_name].upcase %> </dd> | ||
| </div> | ||
| <div> | ||
| <dt class="display-inline"> <%= t('idv.form.dob') %>: </dt> | ||
| <dd class="display-inline margin-left-0"> <%= pii[:dob] %> </dd> | ||
| </div> | ||
| </dl> | ||
| </div> | ||
| <div class="grid-row grid-gap grid-gap-2 padding-bottom-1 padding-top-1 border-y border-primary-light"> | ||
| <dl class='grid-col-fill margin-bottom-0'> | ||
| <div> | ||
| <dt class="display-inline"> <%= t('idv.form.address1') %>: </dt> | ||
| <dd class="display-inline margin-left-0"> <%= pii[:address1].upcase %> </dd> | ||
| </div> | ||
| <div> | ||
| <dt class="display-inline"> <%= t('idv.form.city') %>: </dt> | ||
| <dd class="display-inline margin-left-0"> <%= pii[:city].upcase %> </dd> | ||
| </div> | ||
| <div> | ||
| <dt class="display-inline"> <%= t('idv.form.state') %>: </dt> | ||
| <dd class="display-inline margin-left-0"> <%= pii[:state].upcase %> </dd> | ||
| </div> | ||
| <div> | ||
| <dt class="display-inline"> <%= t('idv.form.zipcode') %>: </dt> | ||
| <dd class="display-inline margin-left-0"> <%= pii[:zipcode] %> </dd> | ||
| </div> | ||
| </dl> | ||
| </div> | ||
| <div class="grid-row grid-gap grid-gap-2 padding-top-1"> | ||
| <div class='grid-col-fill'> | ||
| <%= t('idv.form.ssn') %>: | ||
| <%= render( | ||
| 'shared/masked_text', | ||
| text: SsnFormatter.format(pii[:ssn]), | ||
| masked_text: SsnFormatter.format_masked(pii[:ssn]), | ||
| accessible_masked_text: t( | ||
| 'idv.accessible_labels.masked_ssn', | ||
| first_number: pii[:ssn][0], | ||
| last_number: pii[:ssn][-1], | ||
| ), | ||
| toggle_label: t('forms.ssn.show'), | ||
| ) %> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <% javascript_packs_tag_once 'form-steps-wait' %> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
57 changes: 57 additions & 0 deletions
57
spec/features/idv/inherited_proofing/verify_info_step_spec.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| require 'rails_helper' | ||
|
|
||
| feature 'inherited proofing verify info' do | ||
| include IdvHelper | ||
| include DocAuthHelper | ||
| include_context 'va_user_context' | ||
|
|
||
| before do | ||
| allow(IdentityConfig.store).to receive(:va_inherited_proofing_mock_enabled).and_return true | ||
| allow_any_instance_of(Idv::InheritedProofingController).to \ | ||
| receive(:va_inherited_proofing?).and_return true | ||
| allow_any_instance_of(Idv::InheritedProofingController).to \ | ||
| receive(:va_inherited_proofing_auth_code).and_return auth_code | ||
| end | ||
|
|
||
| let(:auth_code) { Idv::InheritedProofing::Va::Mocks::Service::VALID_AUTH_CODE } | ||
|
|
||
| before do | ||
| sign_in_and_2fa_user | ||
| complete_inherited_proofing_steps_before_verify_step | ||
| end | ||
|
|
||
| describe 'page content' do | ||
| it 'renders the Continue button' do | ||
| expect(page).to have_button(t('inherited_proofing.buttons.continue')) | ||
| end | ||
|
|
||
| it 'renders content' do | ||
| expect(page).to have_content(t('titles.idv.verify_info')) | ||
| expect(page).to have_link(t('inherited_proofing.troubleshooting.options.get_va_help')) | ||
| end | ||
| end | ||
|
|
||
| describe 'user info' do | ||
| it "displays the user's personal information" do | ||
| expect(page).to have_text user_attributes[:first_name].upcase | ||
| expect(page).to have_text user_attributes[:last_name].upcase | ||
| expect(page).to have_text user_attributes[:birth_date] | ||
| end | ||
|
|
||
| it "displays the user's address" do | ||
| expect(page).to have_text user_attributes[:address][:street].upcase | ||
| expect(page).to have_text user_attributes[:address][:city].upcase | ||
| expect(page).to have_text user_attributes[:address][:state].upcase | ||
| expect(page).to have_text user_attributes[:address][:zip] | ||
| end | ||
|
|
||
| it "obfuscates the user's ssn" do | ||
| expect(page).to have_text '1**-**-***9' | ||
| end | ||
|
|
||
| it "can display the user's ssn when selected" do | ||
| check 'Show Social Security number' | ||
| expect(page).to have_text '123-45-6789' | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
28 changes: 0 additions & 28 deletions
28
spec/views/idv/inherited_proofing/verify_info.html.erb_spec.rb
This file was deleted.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be using the existing shared template, rather than duplicating it?
https://github.com/18F/identity-idp/blob/main/app/views/idv/shared/_verify.html.erb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were enough differences between the two templates that I felt it would be better to use our own. (For at least the time being.) I would, ideally, like to move us to the existing shared one in the future but for MVP functionality this was the cleanest option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the differences? It looks like it may only be the titles, right? We could have a
inheritted_proofinglocal or something like that to switch those. I think an even easier approach would be to check with @benjaminchait and @artfulaction if they are okay with using the titles from the existing "Verify your information" screen to keep things consistent.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not allowing users to edit their addresses or ssn at this time, for example. I could put some conditionals in but it started to look really messy and I didn't want to 💩 up the shared template when I wasn't 100% sure we weren't going to restore editing later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an inconsistency/old version I didn't catch earlier, but yes, the title should read "Verify your information" instead of "Please verify your information". And yes, the addresses or SSN are not edited at this time, so I defer to y'all on whatever is easiest with the template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another minor thing is the the user info should be displayed in all caps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@artfulaction Thanks for the careful eye! I have changed the title and put the user info in all caps. (The
.upcasecall is a quick fix until we get through MVP and can abstract that out of the view.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@holytoastr @artfulaction -
This seems fine, but going forward--I'd encourage us not to transform the data we get from the VA.
I believe IDV displays whatever is read from an identity document (eg. your name and address are passed to Login.gov in ALL CAPS) but if the VA passes data in Title Case, I'd encourage us to display it that way. (Since this is already merged, don't worry about changing it for now--but if/when we revisit, I'd encourage us to consider whether we should just display what we received from the wire.)