-
Notifications
You must be signed in to change notification settings - Fork 129
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-15294: Integrate Flow Policy with IPP steps - State ID and Address #11815
LG-15294: Integrate Flow Policy with IPP steps - State ID and Address #11815
Conversation
395e2cc
to
43e41ef
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.
Approved! I did leave feedback about specs, and would be curious to see your response.
expect(subject).to have_actions( | ||
:before, | ||
:confirm_in_person_state_id_step_complete, | ||
) | ||
expect(subject).to have_actions( |
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.
Since you added before_action :confirm_step_allowed
to the address_controller, would you be up for adding a related expectation? (I was also going to say that the separate expect statements here could be combined. However, I looked through the codebase, and it doesn't look like we have a set pattern of having one expect statement for multiple before actions, or having separate expect statements for each before action. Either way is fine by me.)
@@ -26,7 +26,7 @@ | |||
) |
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 left a similar comment on another controller spec: what are your thoughts on adding an expectation to check for the before_action confirm_step_allowed
?
7e58171
to
8f36dfc
Compare
changelog: Internal, In-person proofing, integrates the flow policy into the state id and address controllers
…ta is being properly cleared via flowpolicy
8f36dfc
to
bcc109b
Compare
🎫 Ticket
Link to the relevant ticket:
LG-15294
🛠 Summary of changes
During FSM clean-up we discovered that the flow policy was not yet being fully used within the in_person/state_id_controller and in_person/address_controller. The step_info was included but it wasn't yet being called. These changes address that by adding the confirm_step_allowed before_action and refactoring the undo_step lambdas in both the state_id_controller and address_controller, so they could be properly called in the clear_future_steps! Flow Policy step.
📜 Testing Plan
There is no new or changed functionality for manual testing - you will want to test the IPP flow to make sure everything is working as usual.