LG-11318 unify SsnController templates#9547
Merged
soniaconnolly merged 13 commits intomainfrom Nov 6, 2023
Merged
Conversation
jmhooper
reviewed
Nov 6, 2023
Contributor
There was a problem hiding this comment.
This is in the comments on both templates. It looks like you have refactored this into a lie.
Merged
jmhooper
approved these changes
Nov 6, 2023
changelog: Internal, Ssn step, unify underlying code for remote and in person ssn pages
It's still failing. We could consider deleting this. If we keep it, it needs to be renamed, and it needs to set up @ssn_form, or that needs to be coded differently
jmhooper
force-pushed
the
sonia-lg-11318-unify-ssn-controllers
branch
from
7f40487 to
6ee1999
Compare
Co-authored-by: Gina Yamada <gina.yamada@gsa.gov>
gina-yamada
approved these changes
Nov 6, 2023
Contributor
gina-yamada
left a comment
gina-yamada
left a comment
There was a problem hiding this comment.
Walked through code changes with Sonia. IPP and remote flow both working as expected.
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
🎫 Ticket
LG-11318
🛠 Summary of changes
Both the remote and in person identity verification flows have an SsnController, and they look and behave the same. This PR gives them a shared template, and makes the controller code more alike.
Other changes:
flash[:error]instead of an@error_messageinstance variable.Differences still include:
We could pull out the common code into an SsnConcern similar to the shared VerifyInfoConcern, but in this case the shared code is short enough that the benefit doesn't outweigh the lost readability. We can revisit after implementing the browser back button and FlowPolicy for this step, since that will simplify the code further and make it more similar.
📜 Testing Plan