-
Notifications
You must be signed in to change notification settings - Fork 116
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
LG-10206 Move STEP_INDICATOR_STEP constants out of InPersonFlow #11607
LG-10206 Move STEP_INDICATOR_STEP constants out of InPersonFlow #11607
Conversation
@@ -83,11 +83,7 @@ def pii_locked? | |||
end | |||
|
|||
def step_indicator_steps | |||
if in_person_proofing? |
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 was probably just missed in the IPP GPO removal, I did some manual testing and this condition should never be true. There is no longer a need for IPP GPO steps so I updated this block.
changelog: Internal, In-person proofing, Move STEP_INDICATOR_STEP constants out of InPersonFlow
492f001
to
246f36a
Compare
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.
I tested manually. On the state id page, I killed the server while it was running on main, switched to main, and was able to successfully move forward in the app. Caveat: I didn't manually test the hybrid flow. Also, I tested ipp, but not gpo.
else | ||
StepIndicatorConcern::STEP_INDICATOR_STEPS_GPO | ||
end | ||
StepIndicatorConcern::STEP_INDICATOR_STEPS_GPO |
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.
👏🏻 Thanks for investigating this after our slack conversation.
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.
I think we talked about this before the hackathon- you will delete STEP_INDICATOR_STEPS in InPersonFlow (and probably STEP_INDICATOR_STEPS_GPO) in a follow up PR because of 50/50 state.
This looks good to me. I don't see other instances of Idv::Flows::InPersonFlow::STEP_INDICATOR_STEPS
being used in the code. I walked through IPP, GPO, Remote, and hybrid. Steps in step indicator are what I'd expect and I move through flow as I'd expect.
Nice work Jenny 🚀
🎫 Ticket
Link to the relevant ticket:
LG-10206
🛠 Summary of changes
Moves STEP_INDICATOR_STEPS from InPersonFlow into StepIndicatorConcern. InPersonFlow is FSM-related and will get deleted in a future clean-up PR, so these constants need to be moved as they are still used for the IPP step indicator. References to the old Idv::Flows::InPersonFlow::STEP_INDICATOR_STEPS are updated to reference the new constants.
Note: For 50/50 state management, the old constants in the InPersonFlow are not being removed in this PR.
📜 Testing Plan
No new features or behavior was added - regression testing would be good: