Skip to content

Conditionally remove SendLink step from DocAuthFlow#8033

Merged
soniaconnolly merged 2 commits intomainfrom
sonia-lg-8903-remove-send-link-from-flow
Mar 21, 2023
Merged

Conditionally remove SendLink step from DocAuthFlow#8033
soniaconnolly merged 2 commits intomainfrom
sonia-lg-8903-remove-send-link-from-flow

Conversation

@soniaconnolly
Copy link
Contributor

🎫 Ticket

LG-8903

🛠 Summary of changes

When the doc_auth_combined_hybrid_handoff_enabled feature flag is enabled, the SendLink step is not used. Conditionally remove it from the DocAuthFlow so that the send_link url is not accessed in the 50/50 state when the feature flag and SendLink code are fully removed.

This is an attempt to avoid the 500 errors we saw when we deployed #7929 last week. This code is a temporary bridge and will be removed when we re-deploy the big cleanup PR.

Co-authored-by: Kimball Bighorse kimball.bighorse@gsa.gov

[skip changelog]

When the doc_auth_combined_hybrid_handoff_enabled feature flag is enabled, the SendLink step is not used. Conditionally remove it from the DocAuthFlow so that the send_link url is not accessed in the 50/50 state when the feature flag and SendLink code are fully removed.

Co-authored-by: Kimball Bighorse <kimball.bighorse@gsa.gov>

[skip changelog]
@soniaconnolly soniaconnolly requested review from a team, eric-gade, jmhooper and kbighorse March 20, 2023 21:08
Comment on lines +8 to +9
**(IdentityConfig.store.doc_auth_combined_hybrid_handoff_enabled ?
{} : { send_link: Idv::Steps::SendLinkStep }),
Copy link
Contributor

Choose a reason for hiding this comment

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

just wanted to flag that adding conditional definitions of constants can be tricky for testing, because the tests could be created with the wrong value. From the description, it sounded like this is temporary, so I'm not too worried

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. In this case I know at least some of the specs were working because I had the values in the wrong order to start with and it made a test fail.

UploadStep spec was testing SendLinkStep instead. Update to test the right step.

Co-authored-by: Eric Gade <eric.gade@gsa.gov>
Co-authored-by: Kimball Bighorse <kimball.bighorse@gsa.gov>
@soniaconnolly soniaconnolly merged commit fa78eda into main Mar 21, 2023
@soniaconnolly soniaconnolly deleted the sonia-lg-8903-remove-send-link-from-flow branch March 21, 2023 18:41
jmhooper pushed a commit that referenced this pull request Mar 22, 2023
* Temporary! Conditionally remove SendLink step from DocAuthFlow

When the doc_auth_combined_hybrid_handoff_enabled feature flag is enabled, the SendLink step is not used. Conditionally remove it from the DocAuthFlow so that the send_link url is not accessed in the 50/50 state when the feature flag and SendLink code are fully removed.

[skip changelog]

* Fix UploadStep step spec

UploadStep spec was testing SendLinkStep instead. Update to test the right step.

Co-authored-by: Eric Gade <eric.gade@gsa.gov>
Co-authored-by: Kimball Bighorse <kimball.bighorse@gsa.gov>
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.

5 participants