Remove send link step by kbighorse · Pull Request #7929 · 18F/identity-idp

kbighorse · 2023-03-06T18:54:38Z

🎫 Ticket

Link to the relevant ticket.
https://cm-jira.usa.gov/browse/LG-8903

🛠 Summary of changes

This PR removes a number of files relating to the send_link step in the Flow State Machine, including specs, source files, and views.

soniaconnolly

Nice work! I added some questions & comments.

I pulled it down to try it, and it worked fine with computer upload. When I tried to use the hybrid flow locally with the telephony helper I got logged out on both "desktop" and "mobile." Does it work for you?

db/schema.rb

spec/features/idv/doc_auth/link_sent_step_spec.rb

spec/features/idv/hybrid_flow_spec.rb

spec/features/idv/in_person_spec.rb

soniaconnolly · 2023-03-13T23:28:54Z

spec/support/features/doc_auth_helper.rb

From explorations during test hacking, I don't think the wait: 20 is having any effect. Recommend removing this change and setting CAPYBARA_WAIT_TIME_SECONDS to a higher value in your local config, as described by @jmax-gsa here.

Capybara will not be able to find the label unless we put some kind of wait in here

Capybara will not be able to find the label unless we put some kind of wait in here

Why is that?

I had assumed it's because it takes an extra bit of time for the elements on the page to load (due to React maybe?). Several of the helper methods that deal with clicking things have short wait calls -- here is one example

I'd agree that typically I'd only expect to have to wait if there were some dynamic content shown after page load, but the consent screen is pretty static, which is why it's surprising/alarming to me as well.

There is a spinner button on the agreement page. I'm not sure if that's new or what -- maybe it has to do with checking for mobile on the frontend

Ah, in that case, could we use the click_spinner_button_and_wait helper instead?

identity-idp/spec/support/features/interaction_helper.rb

Lines 2 to 5 in 6d2a452

def click_spinner_button_and_wait(...)

click_on(...)

expect(page).to have_no_css('lg-spinner-button.spinner-button--spinner-active', wait: 10)

end

Edit: Although the wait being on the interaction with the checkbox would not imply to me that it's an issue with the button 🤔

I found the issue is that the preceding step uses a spinner button but the complete_welcome_step helper doesn't wait for the button to resolve. The wait is necessary for the label here only because the welcome step may not have finished submitting by the time the assertion is made, but this helper shouldn't need to be concerned with details of the welcome step.

I've got a related pull request already in the works that I'll include a change for this.

for posterity: #8013

app/services/idv/actions/cancel_send_link_action.rb

app/services/idv/steps/send_link_step.rb

spec/features/idv/hybrid_flow_spec.rb

app/services/idv/steps/upload_step.rb

soniaconnolly · 2023-03-14T21:32:25Z

I pulled it down to try it, and it worked fine with computer upload. When I tried to use the hybrid flow locally with the telephony helper I got logged out on both "desktop" and "mobile." Does it work for you?

@kbighorse and I looked at this together, and getting logged out seems to be an artifact of testing "desktop" and "mobile" on the same browser. When we did desktop on Chrome and mobile on Firefox we saw it work.

-- What This new helper method will first clear an input before inserting text into it during capybara tests.

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

changelog: User-Facing Improvements, Updating Hybrid Handoff, finishing removal of feature-flagged combined hybrid handoff

…ng removal of feature-flagged combined hybrid handoff (LG-8903)

soniaconnolly

LGTM!

* Add columns to duplicate SSN reporting (LG-9191) (#7995) * Add profile_active column * Add count_active_ssn_fingerprint column * Handle nil activated_at values changelog: Internal, Reporting, Add report for duplicate SSNs * LG-9138: Allow state ID address fields in user PII (#7961) * Allow state ID address fields in user PII changelog: Upcoming features, Double address verification, Allow state ID address fields in user PII * Add more Pii attribute comments * Use existing state_id_jurisdiction field * Allow state_id_jurisdiction in feature tests * Allow PII for other events * Update brakeman gem (#7998) changelog: Internal, Security, Update brakeman gem * Clean up `ResolutionProofingJob` tests (#7996) The `ResolutionProofingJob` is the class that does much of the legwork during the verify step. Specifically, it does the following: 1. Communicates with ThreatMetrix if necessary 2. Communicates with InstantVerify 3. Communicates with AAMVA 4. Combines results through the `ResolutionResultAdjudicator` The tests for this class have been sophisticated and have combined a number of different ways of mocking out responses from vendors (e.g. webmocks or stubs of the vendor classes). This change attempts to simplify some of this by creating a helper for stubbing responses from vendors. Those helpers are used to re-implement simpler examples for the logic the tests test. Ideally this will allow us to more quickly make changes to the `ResolutionProofingJob`. [skip changelog] * Reduce device/event transactions by wrapping multiple writes in one transaction (#7999) changelog: Internal, Database, Reduce device/event transactions by wrapping multiple writes in one transaction * Don't make a request to AAMVA if verification is guaranteed to fail (#7997) * Don't make a request to AAMVA if verification is guaranteed to fail We implemented a feature that allowed us to use the response we get from AAMVA to cover attributes that fail the LexisNexis check during resolution. Specifically, the following attributes can be covered by AAMVA: - Address - Dob - State ID Number As part of this change we started sending requests to AAMVA even if the LexisNexis response failed. Previously this occurred even if the failed LexisNexis attributes could not be covered by the AAMVA response. This commit attempts to shed some load on AAMVA's service by not making requests to them if the attributes in a failed LexisNexis response cannot be covered by AAMVA. [skip changelog] * Disable primary database prepared statements if using database socket connection (#7990) changelog: Internal, Database, Disable primary database prepared statements if using database socket connection * Change the regular expression we use to determine if get to yes is viable (#8002) Before we use the get-to-yes feature we need to make sure that the base lexis nexis error refers to the scoring model verification. This is what implies that the checks failed and we can fallback to AAMVA. LexisNexis uses the code priority.scoring.model.verification.fail for some of its workflows. This commit changes the regex we use to match against the code to match that code. [skip changelog] * Disable advisory locks when using pgbouncer socket (#8003) Related PRs #7990 and #7967 This disables advisory locks when a DB socket is used as `pgbouncer` does not support advisory lock use in transaction mode. changelog: Internal, Database, Disable primary database advisory locks if using database socket connection * Remove `skip_legacy_state` method (#7991) * Remove `skip_legacy_state` method The `skip_legacy_state` method had 2 major problems: 1. The name implies that the new state is somehow a legacy state 2. It is hidden in the `check_ssn` code which makes it difficult to understand how a user moves from the verify info controller to the phone controller This method attempts to clean this up by removing `skip_legacy_state` method and moving its logic into more reasonable places in the verify info concern or into the `Idv::Session`. [skip changelog] * Update app/services/idv/session.rb Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> * delint --------- Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> * Add additional prompts to UUID lookup and review rake tasks (#8001) changelog: Internal, utilities, Reuqest more information from users when calling investigative rake tasks Co-authored-by: Paul Hirsch <paul.hirsch@gsa.gov> * LG-9020 update ssn content (#8004) * add strings * update ssn views with changed content * changelog: User-Facing Improvements, ssn page, update text * Update have_description accessibility matcher for feature spec compatibility (#8010) changelog: Internal, Automated Testing, Improve compatibility of accessibility description matcher * Remove send link step (#7929) * attempt to squash everything * Adding clear_and_fill_in helper -- What This new helper method will first clear an input before inserting text into it during capybara tests. * Removing unused analytics event methods Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * Changelog changelog: User-Facing Improvements, Updating Hybrid Handoff, finishing removal of feature-flagged combined hybrid handoff * changelog: User-Facing Improvements, Updating Hybrid Handoff, finishing removal of feature-flagged combined hybrid handoff (LG-8903) * Restore `pgcrypto` extension * Remove redundant assignment * Decrease find wait time in feature spec --------- Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov> Co-authored-by: eric-gade <eric.gade@gsa.gov> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> * LG-9050: Add password error handling (#7989) * Add additional feedback to password error handling changelog: User-Facing Improvements, Registration, Add additional feedback to password error handling * LG-9023: Remove phone helper text (#7917) * Remove phone example number text changelog: User-Facing Improvements, Authentication, Remove phone helper text * LG-9206 | Analytics logs irs_session_id (#8007) changelog: Internal, Analytics, Log irs_session_id with analytics * Lg 9089 user data ack (#7974) * LG-9089 Add IRS text to localdev.yml for testing changelog: Internal, Analytics, Add Policy Redirect * Remove arcgis_search_enabled feature flag (#8006) * Remove arcgis_search_enabled feature flag Why: This feature is now enabled in production and would not be expected to be reverted to pilot facilities, and its continued existence in the code creates confusion for other developers. * changelog: Internal, Feature Flags, Remove code associated with live features * Remove unused locale strings * LG-8887: Update Ready to Verify language to be simpler, more branded (#8009) * changelog: Improvements, In-Person Proofing, Update language to be simpler and branded * Add param * Remove custom initializers from GoodJob classes (#8015) **Why**: They interfere with the jobs being instantiated by the scheduler changelog: Internal, Reporting, Fix scheduled SSN and Daily Dropoff report jobs --------- Co-authored-by: Sheldon Bachstein <sheldon.bachstein@gsa.gov> Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov> Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov> Co-authored-by: Paul Hirsch <59626817+pauldoomgov@users.noreply.github.com> Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov> Co-authored-by: Stephen Shelton <stephen.shelton@gsa.gov> Co-authored-by: Paul Hirsch <paul.hirsch@gsa.gov> Co-authored-by: Shannon A <20867088+svalexander@users.noreply.github.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Kimball Bighorse <kimball.bighorse@gsa.gov> Co-authored-by: eric-gade <eric.gade@gsa.gov> Co-authored-by: jc-gsa <104452882+jc-gsa@users.noreply.github.com> Co-authored-by: Matt Wagner <mattwagner@navapbc.com> Co-authored-by: Luis H. Matos <ThatSpaceGuy@users.noreply.github.com> Co-authored-by: Matt Gardner <wilburnforce@gmail.com>

This reverts commit 8340d0a.

This reverts commit 8340d0a. This caused errors in prod from the send_link route, which shouldn't be happening. Possibly because of the 50/50 state. Reverting pending further research.

* LG-8887: In-Person Proofing: Add Login.gov logo above the Barcode (#7956) * WIP commit of half-baked changes for showing logo These are not yet functional * changelog: User-Facing Improvements, In-Person Proofing, Display Login.gov logo above barcode in email * Include asset_url * Revert "Include asset_url" This reverts commit a4612d2. * Revert "changelog: User-Facing Improvements, In-Person Proofing, Display Login.gov logo above barcode in email" This reverts commit 2f5326c. * Revert "WIP commit of half-baked changes for showing logo" This reverts commit c8d3af5. * WIP * Display miniature logo inside mailer and component view * Add a little padding * Drop skip_pipeline; simplify styles * Add image assertion * Allow alternative extensions --------- Co-authored-by: Sheldon Bachstein <sheldon.bachstein@gsa.gov> * LG-8788: Refactor address search controller (#7820) * changelog: Internal, In-Person Proofing, refactor some error reporting (analytics * Add server error to address search controller * refactor --------- Co-authored-by: Eileen McFarland <eileenmcfarland@navapbc.com> * Revert "Remove send link step (#7929)" (#8019) This reverts commit 8340d0a. This caused errors in prod from the send_link route, which shouldn't be happening. Possibly because of the 50/50 state. Reverting pending further research. * Correct analytics name send_link to link_sent (#8022) * Correct analytics name for link sent event to 'IdV: doc auth link_sent submitted' from 'IdV: doc auth send_link submitted' [skip changelog] * Add email support test case to mailer shared examples (#8017) * Add email support test case to mailer shared examples changelog: Internal, Automated Testing, Enhance mailer tests to check for email client support features * Link support reference For future recall * Fix spec flake in GpoConfirmationUploader (#8024) changelog: Internal, Tests, Fix test that flaked due to clock drift * Clean up spec to stop stubbing .new * Revert "Merge pull request #8016 from 18F/stages-rc-2023-03-17-revert" This reverts commit 4c8419f, reversing changes made to 3741fd7. --------- Co-authored-by: Matt Gardner <wilburnforce@gmail.com> Co-authored-by: Sheldon Bachstein <sheldon.bachstein@gsa.gov> Co-authored-by: Eileen McFarland <eileenmcfarland@navapbc.com> Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

kbighorse requested a review from eric-gade March 6, 2023 21:07

kbighorse changed the title ~~changelog: Identity verification, Remove old hybrid handoff steps and…~~ Remove send link step Mar 7, 2023

eric-gade force-pushed the LG-8903-remove-hybrid-handoff-steps branch from de6ece4 to 6709c76 Compare March 13, 2023 19:13

kbighorse marked this pull request as ready for review March 13, 2023 20:28

kbighorse requested a review from soniaconnolly March 13, 2023 20:28

soniaconnolly reviewed Mar 13, 2023

View reviewed changes

aduth reviewed Mar 14, 2023

View reviewed changes

mitchellhenke force-pushed the main branch from 69c4ff8 to 3f7e670 Compare March 14, 2023 17:22

jmhooper force-pushed the LG-8903-remove-hybrid-handoff-steps branch from c260f39 to 910cd42 Compare March 14, 2023 18:02

attempt to squash everything

57d5022

jmhooper force-pushed the LG-8903-remove-hybrid-handoff-steps branch from 910cd42 to 57d5022 Compare March 14, 2023 18:03

eric-gade and others added 10 commits March 15, 2023 10:25

Adding clear_and_fill_in helper

c7f6e1c

-- What This new helper method will first clear an input before inserting text into it during capybara tests.

Removing unused analytics event methods

c8d166f

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

Changelog

75bcb0f

changelog: User-Facing Improvements, Updating Hybrid Handoff, finishing removal of feature-flagged combined hybrid handoff

changelog: User-Facing Improvements, Updating Hybrid Handoff, finishi…

dd4c2c7

…ng removal of feature-flagged combined hybrid handoff (LG-8903)

Merge branch 'main' into LG-8903-remove-hybrid-handoff-steps

f5f2c7f

Merge branch 'main' into LG-8903-remove-hybrid-handoff-steps

82d0f13

Restore pgcrypto extension

9def818

Remove redundant assignment

c3b8ef2

Decrease find wait time in feature spec

b60b329

Merge branch 'main' into LG-8903-remove-hybrid-handoff-steps

ecaa131

kbighorse requested review from aduth and soniaconnolly March 16, 2023 19:40

soniaconnolly approved these changes Mar 16, 2023

View reviewed changes

Merge branch 'main' into LG-8903-remove-hybrid-handoff-steps

a7817d9

kbighorse merged commit 8340d0a into main Mar 16, 2023

kbighorse deleted the LG-8903-remove-hybrid-handoff-steps branch March 16, 2023 20:52

zachmargolis mentioned this pull request Mar 17, 2023

Deploy RC 263 #8014

Merged

soniaconnolly added a commit that referenced this pull request Mar 17, 2023

Revert "Remove send link step (#7929)"

30862ca

This reverts commit 8340d0a.

soniaconnolly mentioned this pull request Mar 17, 2023

Revert "Remove send link step" #8019

Merged

soniaconnolly mentioned this pull request Mar 20, 2023

Conditionally remove SendLink step from DocAuthFlow #8033

Merged

kbighorse restored the LG-8903-remove-hybrid-handoff-steps branch March 22, 2023 18:17

	def click_spinner_button_and_wait(...)
	click_on(...)
	expect(page).to have_no_css('lg-spinner-button.spinner-button--spinner-active', wait: 10)
	end

Conversation

kbighorse commented Mar 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎫 Ticket

🛠 Summary of changes

Uh oh!

soniaconnolly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

soniaconnolly Mar 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-gade Mar 15, 2023

Choose a reason for hiding this comment

Uh oh!

aduth Mar 15, 2023

Choose a reason for hiding this comment

Uh oh!

eric-gade Mar 15, 2023

Choose a reason for hiding this comment

Uh oh!

aduth Mar 15, 2023

Choose a reason for hiding this comment

Uh oh!

eric-gade Mar 15, 2023

Choose a reason for hiding this comment

Uh oh!

aduth Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aduth Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kbighorse Mar 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

soniaconnolly commented Mar 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soniaconnolly left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kbighorse commented Mar 6, 2023 •

edited

Loading

soniaconnolly Mar 13, 2023 •

edited

Loading

aduth Mar 15, 2023 •

edited

Loading

aduth Mar 16, 2023 •

edited

Loading

soniaconnolly commented Mar 14, 2023 •

edited

Loading