LG-9333 Preserve the SSN when returning to the SSN controller by jmhooper · Pull Request #8197 · 18F/identity-idp

jmhooper · 2023-04-12T20:29:22Z

The SSN controller allows a user to enter or update their SSN. Prior to this commit the previous value of the SSN was not maintained when the user was updating their SSN.

This commit attempts to fix that by making use of the SSN form that is used in the controller. The template is changed to eventually read the SSN from the form.

Additionally, the form is used in a pattern that better matches the pattern you expect a form object to be used in. This was not the case before because of constraints that the flow state machine placed on HTML forms.

jmhooper · 2023-04-12T20:29:52Z

This is currently WIP. I need to put together a test case for this behavior. I also look forward to seeing what weird side effects it causes once CI runs.

aduth · 2023-04-12T21:08:50Z

app/views/shared/_ssn_field.html.erb

        pattern: '^\d{3}-?\d{2}-?\d{4}$',
        maxlength: 11,
-        input_html: { class: 'ssn-toggle usa-input', value: '' },
+        input_html: { class: 'ssn-toggle usa-input', value: f.object.ssn },


Very happy to see use the form for this, since I think it means we can just let SimpleForm do its thing and not need to explicitly provide the value at all?

Also, fairly certain that usa-input should be assigned by default now.

Suggested change

input_html: { class: 'ssn-toggle usa-input', value: f.object.ssn },

input_html: { class: 'ssn-toggle' },

It looks like unfortunately I do have to specify the value because the underlying component does not appear to tie into simple form. It is used in a log of places so I am hesitant to change it here since that may get out of hand.

Also, it looks like removing usa-input troubles the a11y specs (ref: https://gitlab.login.gov/lg/identity-idp/-/jobs/413342)

Your test failure might have been a flake, because when I check out that commit and run in locally it passes. I can also confirm upon inspecting the page that the usa-input class is in there already by default

Just as a data point, I reverted 2b26e7f and ran spec/features/accessibility locally and it passed a-ok

Hmm, lemme try again

It looks like unfortunately I do have to specify the value because the underlying component does not appear to tie into simple form.

Not sure if it's what you meant by underlying component, but I spent some time digging into this since I thought it should "just work". Turns out that SimpleForm eventually delegates to Rails' built-in password_field tag builder, which sets the value to nil by default.

https://github.com/rails/rails/blob/v7.0.4.3/actionview/lib/action_view/helpers/tags/password_field.rb#L8

For non-password fields, it probably would work fine without the explicit value to use the value from the SimpleForm object.

The SSN controller allows a user to enter or update their SSN. Prior to this commit the previous value of the SSN was not maintained when the user was updating their SSN. This commit attempts to fix that by making use of the SSN form that is used in the controller. The template is changed to eventually read the SSN from the form. Additionally, the form is used in a pattern that better matches the pattern you expect a form object to be used in. This was not the case before because of constraints that the flow state machine placed on HTML forms. changelog: Improvements, Proofing workflow, The user's SSN is preserved for users who enter and SSN then continue to the verify step but return to the SSN in the unsupervised remote proofing flow.

soniaconnolly

LGTM with one query.

soniaconnolly · 2023-04-14T16:26:19Z

spec/features/idv/doc_auth/verify_info_step_spec.rb

    expect(page).to have_current_path(idv_ssn_path)
+    expect(
+      find_field(t('idv.form.ssn_label_html')).value,
+    ).to eq(DocAuthHelper::GOOD_SSN.gsub(/\D/, ''))


Not clear to me why this gsub is needed. Are you expecting the field to be blank? Could you use GOOD_SSN_MASKED?

I'm looking at the value of the field which is the SSN without the dashes.

* Remove unused configuration redis_throttle_alternate_url and redis_throttle_alternate_pool_write_enabled configurations (#8211) changelog: Internal, Configuration, Remove unused Redis migration configuration * Override tag style to leave case unchanged and make text bold (#8212) * Override tag style to leave case unchanged and make text bold Per Andrew Duthie the Login style for tags is Title case and bold, but the design system is still all caps, so override it for idp for now. changelog: User-facing Improvements, Design, change 'tag' style to be bold and preserve case * LG-9333 Preserve the SSN when returning to the SSN controller (#8197) The SSN controller allows a user to enter or update their SSN. Prior to this commit the previous value of the SSN was not maintained when the user was updating their SSN. This commit attempts to fix that by making use of the SSN form that is used in the controller. The template is changed to eventually read the SSN from the form. Additionally, the form is used in a pattern that better matches the pattern you expect a form object to be used in. This was not the case before because of constraints that the flow state machine placed on HTML forms. changelog: Improvements, Proofing workflow, The user's SSN is preserved for users who enter and SSN then continue to the verify step but return to the SSN in the unsupervised remote proofing flow. * LG-8714 Remove UserDecorator (#8204) * Moving UserDecorator methods to User * Updating all references to user.decorate Also: - Move UserDecorator specs to User spec (with adjustments) - Delete old UserDecorator class and spe Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> * Jskinne3 lg 9361 mobile local md (#8217) * Transfer mobile guide from old branch * Link to mobile docs from other docs * Check spelling * [skip changelog] * Auto-generate README * Soften USB hub wording becase it works for some people * Seperate mobile device and VM instructions * Configure the DDP mock client to respond with a failed result on `no_result` (#8214) In a previous commit we changed the DDP proofer to respond with an exception result when the result from DDP included an unexpected status (ref: #8149). This includes when the result is nil. This commit changes the DDP mock's behavior to align with the DDP proofer's behavior. [skip changelog] * LG-9034 Log TrueID decision product status (#8195) * LG-9034 Log TrueID decision product status Capture decision product status for TrueID responses Decision product status contains the result of TrueID after decisioning. Prior to this change, product status was only being captured for @productType='TrueID' and ignoring product status for @productType='TrueID_Decision' * [skip changelog] * successful if TrueID_Decision product does not exist * refactor obtaining decision_product for reuse * happy linting * Update spec/services/doc_auth/lexis_nexis/responses/true_id_response_spec.rb Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> * remove unnecessary expect statements since the check is done a few lines below * remove line no longer used in test --------- Co-authored-by: AmirReavis-Bey <amirreavis-bey@fcoh2j-wyp9w9mv.localdomain> Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> * LG-9488: Entry controller for hybrid mobile document capture flow (#8209) * Add placeholder CaptureDocController * Add controller to handle entry into hybrid flow Controller looks at `document-capture-session` and does a "light login" to enable the user to start document capture. changelog: Internal, Flow state machine removal, Add controller for entry into hybrid mobile document capture * Refactor feature flag check out to HybridMobileConcern * Add tests around the 'Doc Auth' event * Send links to new hybrid mobile experience Going with /verify/documents?document-capture-session=xxxxx as a temporary url for now * CaptureDocController -> DocumentCaptureController * Tweak doc capture entry url naming & add comment * idv_hybrid_mobile_document_capture_entry_url -> idv_hybrid_mobile_entry_url * Refactor EntryController not to use before_action * Move HybridMobileConcern into Idv::HybridMobile namespace * WHOOPS need to see why this wasn't breaking any tests * Remove extra redirect Rails doesn't preserve the querystring when redirecting routes, and this is just an unnecessary extra step anyway * Add feature spec for mobile hybrid flow entry * Tweaks to capture complete step spec Ultimately we'll merge this with a doc capture step, presumably * Fix email confirmation 500 (#8224) * add failing spec * Use strong parameters in email confirmation controller to fix 500 error changelog: Bug Fixes, Email Confirmation, Use strong parameters in email confirmation controller to fix 500 error * LG-9321: New field on Enrollment Outcomes for GetUspsProofingResultsJob Summary (#8216) * LG-9321 Add data to usps proof results job * LG-9321 Added one more test * LG-9321 Fix lint issues * changelog: Internal, In-person-proofing, new metric on enrollment outcomes summary for GetUspsProofingResultsJob * LG-9321 Add arg to round() for precision * LG-9321 Update round to have more precision Co-authored-by: Tim Bradley <90272033+NavaTim@users.noreply.github.com> * LG-9321 Remove initial unnecessary assignment Co-authored-by: Tim Bradley <90272033+NavaTim@users.noreply.github.com> --------- Co-authored-by: Tim Bradley <90272033+NavaTim@users.noreply.github.com> --------- Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov> Co-authored-by: Eric Gade <105373963+eric-gade@users.noreply.github.com> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> Co-authored-by: John Skiles Skinner <john.skinner@gsa.gov> Co-authored-by: Amir Reavis-Bey <1261794+amirbey@users.noreply.github.com> Co-authored-by: AmirReavis-Bey <amirreavis-bey@fcoh2j-wyp9w9mv.localdomain> Co-authored-by: Matt Hinz <matt.hinz@gsa.gov> Co-authored-by: gina-yamada <125507397+gina-yamada@users.noreply.github.com> Co-authored-by: Tim Bradley <90272033+NavaTim@users.noreply.github.com>

The SSN controller allows a user to enter or update their SSN. Prior to this commit the previous value of the SSN was not maintained when the user was updating their SSN. This commit attempts to fix that by making use of the SSN form that is used in the controller. The template is changed to eventually read the SSN from the form. Additionally, the form is used in a pattern that better matches the pattern you expect a form object to be used in. This was not the case before because of constraints that the flow state machine placed on HTML forms. changelog: Improvements, Proofing workflow, The user's SSN is preserved for users who enter and SSN then continue to the verify step but return to the SSN in the unsupervised remote proofing flow.

aduth reviewed Apr 12, 2023

View reviewed changes

jmhooper force-pushed the jmhooper-delete-fsm-ssn-template-preserve-ssn branch from 4c35df7 to ca281df Compare April 14, 2023 14:56

jmhooper added 5 commits April 14, 2023 10:58

fix existing tests

2682750

specs for the form

49cc6cf

update an existing feature spec to check for pre-fill

95fe779

remove extraneous css

3de110d

put back use-input class

2b26e7f

jmhooper marked this pull request as ready for review April 14, 2023 15:41

jmhooper requested a review from a team April 14, 2023 15:41

soniaconnolly approved these changes Apr 14, 2023

View reviewed changes

remove usa-input class

c3c7d07

jmhooper merged commit 0ce4192 into main Apr 14, 2023

jmhooper deleted the jmhooper-delete-fsm-ssn-template-preserve-ssn branch April 14, 2023 17:22

jmdembe mentioned this pull request Apr 18, 2023

Deploy RC 273 to Production #8230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LG-9333 Preserve the SSN when returning to the SSN controller#8197

LG-9333 Preserve the SSN when returning to the SSN controller#8197
jmhooper merged 7 commits intomainfrom
jmhooper-delete-fsm-ssn-template-preserve-ssn

jmhooper commented Apr 12, 2023

Uh oh!

jmhooper commented Apr 12, 2023

Uh oh!

aduth Apr 12, 2023

Uh oh!

jmhooper Apr 14, 2023

Uh oh!

jmhooper Apr 14, 2023

Uh oh!

eric-gade Apr 14, 2023

Uh oh!

matthinz Apr 14, 2023

Uh oh!

jmhooper Apr 14, 2023

Uh oh!

aduth Apr 17, 2023

Uh oh!

soniaconnolly left a comment

Uh oh!

soniaconnolly Apr 14, 2023

Uh oh!

jmhooper Apr 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	input_html: { class: 'ssn-toggle usa-input', value: f.object.ssn },
	input_html: { class: 'ssn-toggle' },

Conversation

jmhooper commented Apr 12, 2023

Uh oh!

jmhooper commented Apr 12, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soniaconnolly left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants