Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

LG-12375 state id controller #10457

Merged
merged 10 commits into from
Apr 19, 2024
Merged

Conversation

svalexander
Copy link
Contributor

@svalexander svalexander commented Apr 17, 2024

馃帿 Ticket

Link to the relevant ticket:
LG-12375

馃洜 Summary of changes

This pr adds the GET route for the the controller version of the state id page, as well as the controller, view and specs.

馃摐 Testing Plan

Provide a checklist of steps to confirm the changes.

Testing fsm flow

  • Go through ipp flow and arrive at state id page
  • Confirm url is /in_person/state_id
  • Change url to /in_person_proofing/state_id
  • Confirm 404 page is shown

Testing controller flow

  • In config/application.yml file change in_person_state_id_controller_enabled to true
  • Stop server, run make update and then make run
  • Go through ipp flow and arrive at state id page
  • Confirm url is /in_person_proofing/state_id
  • Change url to /in_person/state_id
  • Confirm state id page is shown
  • Go back to controller view
  • Confirm form appears as it should
  • From Issuing state select Florida
  • Confirm Florida hint text appears under id number
  • From Issuing state select Texas
  • Confirm Texas hint text appears under id number
  • From state select Puerto Rico
  • Confirm Puerto Rico hint appears under address line and address line 2
  • Fill out form and submit
  • Confirm submission does not yet work

@svalexander svalexander requested review from a team and racingspider April 18, 2024 13:46
}
end

# update Idv::DocumentCaptureController.step_info.next_steps to include
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the DocumentCaptureController it currently does not include :ipp_address as a next step. I think it should though since the address step is no longer managed by the FSM and has it's own controller, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think so, but I thought it might be something to address in the pr that adds the update route since this pr doesn't allow you to navigate past the state id page

Copy link
Contributor

@JackRyan1989 JackRyan1989 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great so far! Are there feature specs that will need to be updated as a result of this work?

key: :ipp_state_id,
controller: self,
next_steps: [:ipp_address, :ipp_ssn],
preconditions: ->(idv_session:, user:) { user.establishing_in_person_enrollment },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just want to note that the before action; redirect_unless_enrollment, uses current_user whereas this precondition is using user. Is there a difference between these variables?

app/controllers/idv/in_person/state_id_controller.rb Outdated Show resolved Hide resolved
pii_from_user.delete(:zipcode)
end

def copy_state_id_address_to_residential_address(pii_from_user)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here, I don't think this method is being called anywhere?

redirect_url(step)
end

def redirect_url(step)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense. Basically if someone is in the FSM, and we have the state_id_controller_enabled, then bump the user over to the controller based page. I think the thing that is throwing me off is that since this is the last step that's using the FSM, how would a user get into the FSM if the state id controller is enabled?
Just thinking out loud, not saying anything about removing this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a good question. I think the InPersonController allows us to enter the fsm?

Copy link
Contributor

@JackRyan1989 JackRyan1989 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comment about my ignorance.

Copy link
Contributor

@JackRyan1989 JackRyan1989 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manually testing looks good. Way to go!

@svalexander svalexander merged commit 96dcede into main Apr 19, 2024
2 checks passed
@svalexander svalexander deleted the shannon/lg-12375-state-id-controller branch April 19, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants