Skip to content

LG-10091 FSM/SSN - Move Extra Analytic Properties to A Shared Concern#8865

Merged
gina-yamada merged 13 commits intomainfrom
gyamada/LG-10091-extra-prop
Jul 31, 2023
Merged

LG-10091 FSM/SSN - Move Extra Analytic Properties to A Shared Concern#8865
gina-yamada merged 13 commits intomainfrom
gyamada/LG-10091-extra-prop

Conversation

@gina-yamada
Copy link
Contributor

@gina-yamada gina-yamada commented Jul 26, 2023

🎫 Ticket

LG-10091 FSM/SSN - Move extra analytics properties to a shared concern

🛠 Summary of changes

  • Move extra analytics properties to a shared concern (so that each of the steps in the non-FSM version can use it)
  • Refactor app/controllers/idv/in_person/verify_info_controller.rb to use shared concern
  • Added extra analytics properties to non-FSM ssn step so it would have same_address_as_id

📜 Testing Plan

  1. Run through the steps👇 on main branch with in_person_ssn_info_controller_enabled set to false in application.yml. This shows the analytic property currently being added to the FSM version on the SSN step. (See Scenario 1 in screenshots)

  2. Run through the steps👇 on main branch with in_person_ssn_info_controller_enabled set to true in application.yml. This shows the analytic property currently not being added to the non-FSM version on the SSN step. (See Scenario 2 in screenshots)

  3. Run through the steps👇 on gyamada/LG-10091-extra-prop branch with in_person_ssn_info_controller_enabled set to true in application.yml. This shows the analytic property has been added to the non-FSM version on the SSN step. (See Scenario 3 in screenshots)

  4. Run through the team Ada's flow on gyamada/LG-10091-extra-prop. This shows the analytic property has not been added to the SSN step for the doc auth flow. (See Scenario 4 in screenshots)

Steps:

  1. Set in_person_ssn_info_controller_enabled to false in application.yml
  2. Open file events.log inside log
  3. Start server to run locally.
  4. Create an account. Confirm email. Set up a password. Set up Authentication method. Change url to /verify to Get started verifying your identity. Upload photos to fail proofing and proceed to Try in person.
  5. Fill out forms until you land on url /verify/in_person/verify_info
  6. Open up events.log and search for ssn. View the properties for both IdV: doc auth ssn visited and Idv: doc auth ssn submitted. Both should have field same_address_as_id and the value should match what your selection was.

👀 Screenshots

Scenario 1: Screenshot 2023-07-28 at 10 25 55 AM
Scenario 2: Screenshot 2023-07-28 at 10 30 12 AM
Scenario 3: Screenshot 2023-07-26 at 8 55 28 AM
Scenario 4: Screenshot 2023-07-28 at 10 18 07 AM

@gina-yamada gina-yamada marked this pull request as ready for review July 28, 2023 17:13
@gina-yamada gina-yamada requested a review from svalexander July 28, 2023 17:13
@gina-yamada
Copy link
Contributor Author

gina-yamada commented Jul 28, 2023

To whoever reviews this pull request- Please run rpec/features/idv/steps/in_person/ssn_spec.rb and report your findings. I can run this file 3 times and get different pass/fail results. In the flow you will eventually hit complete_location_step which has a pause for the location list to disappear and a rescue for Selenium (a stale element reference). It is inconsistent locally for me. Maybe slow internet but wouldn't mind a check to see if okay. Inconsistent failing tests are the worst. Wondering if this is a concern if other experience this locally too?

May need to add Webdrivers::Chromedriver.required_version = '114.0.5735.90' to railers_helper.rb on line 99, see Slack thread.

let(:flow_session) do
{ 'document_capture_session_uuid' => 'fd14e181-6fb1-4cdc-92e0-ef66dad0df4e',
'pii_from_user' => Idp::Constants::MOCK_IDV_APPLICANT_SAME_ADDRESS_AS_ID_WITH_NO_SSN.dup,
:threatmetrix_session_id => 'c90ae7a5-6629-4e77-b97c-f1987c2df7d0',
Copy link
Contributor

Choose a reason for hiding this comment

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

do we not need tests for threatmetrix?

Copy link
Contributor Author

@gina-yamada gina-yamada Jul 28, 2023

Choose a reason for hiding this comment

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

@svalexander Good point! I can see threatmetric_session_id on flow_session on ssn step for both fsm and non-fsm so I put threatmetrix_session_id back onto flow_session (for test data) and added two tests.

See commit 073e2f2

Copy link
Contributor

@svalexander svalexander left a comment

Choose a reason for hiding this comment

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

LGTM. Running the ssn spec passed for me (I ran 3 times).

@gina-yamada
Copy link
Contributor Author

gina-yamada commented Jul 28, 2023

LGTM. Running the ssn spec passed for me (I ran 3 times).

Thanks for running them! My internet has been slow today and I think that may have caused these inconsistencies. I dug into them but never once observed failure when the pipeline ran them

@gina-yamada gina-yamada merged commit 1e09def into main Jul 31, 2023
@gina-yamada gina-yamada deleted the gyamada/LG-10091-extra-prop branch July 31, 2023 14:46
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.

2 participants