Skip to content

LG-8867: New page to inform user of GPO only#8011

Merged
solipet merged 35 commits intomainfrom
doug-lg-8867
Mar 24, 2023
Merged

LG-8867: New page to inform user of GPO only#8011
solipet merged 35 commits intomainfrom
doug-lg-8867

Conversation

@solipet
Copy link
Contributor

@solipet solipet commented Mar 16, 2023

🎫 Ticket

LG-8867

🛠 Summary of changes

Introduces a new interstitial page that a user starting IDV will see once before the Welcome page informing the user that due to technical difficulties, we can only allow address verification via a letter.

This page is shown to a user if any of the following config flags are set:

  • vendor_status_sms: full_outage
  • vendor_status_voice: full_outage
  • feature_idv_force_gpo_verification_enabled: true

Additionally, the user is not presented with an option to do the hybrid flow if any of the following config flags are set:

  • vendor_status_sms: full_outage
  • vendor_status_voice: full_outage
  • feature_idv_hybrid_flow_enabled: false

👀 Screenshots

Coming from a service provider: Interstitial-with-SP
Hitting the /verify path directly (not available in production): Interstitial-without-SP
Spanish translation: Interstitial-with-SP-Spanish
French translation: Interstitial-with-SP-French

@solipet solipet requested a review from a team March 16, 2023 21:24
Copy link
Contributor

Choose a reason for hiding this comment

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

Should gpo_only? and allow_hybrid_flow? go in FeatureManagement, since they depend on vendor status and configuration? I have some similar code for an idv_available? method in #7970 that I put in there for that reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We renamed this class OutageStatus since the outage could be predicated on a vendor outage or a feature flag config option.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I think I preferred the separation of VendorStatus and FeatureManagement. To me it is not necessarily clear what OutageStatus is reporting on -- who is having the outage? Is it us or a vendor?

I think there's a clear separation between:

  • VendorStatus - which vendors are operational right now? Which classes (e.g. idv) of vendors are having trouble right now
  • FeatureManagement - given the larger state of the application, including configuration and vendor availability, what features are currently available to users?

Copy link
Contributor

@eric-gade eric-gade Mar 23, 2023

Choose a reason for hiding this comment

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

"Vendor" isn't really the right term either, though, because we aren't referring to vendors, but capabilities (ie, sms or voice).

As a middle ground, might I suggest we keep OutageStatus for services we use that are switched to out (phone etc) and FeatureManagement for user-facing features we want to be able to switch off, as @matthinz has suggested? In other words, let's move the gpo and hybrid screen switched to FeatureManagement but keep the rest as-is. Does that make sense?

Example:

  def self.idv_hybrid_flow_enabled?
    return false if not IdentityConfig.store.feature_idv_hybrid_flow_enabled
    return false if OutageStatus.new.any_phone_vendor_outage?
    true
  end

Copy link
Contributor

Choose a reason for hiding this comment

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

That works for me. I also wonder about bringing the :sms and :voice vendors more in line with the other ones (by actually naming them in ruby source, similar to Idv vendors) might be better long-term, but I'm happy to punt that to another day.

@eric-gade eric-gade force-pushed the doug-lg-8867 branch 3 times, most recently from 8a5aa2a to 84bbd6a Compare March 17, 2023 18:09
@solipet solipet force-pushed the doug-lg-8867 branch 2 times, most recently from a9bb2dd to 10bca70 Compare March 20, 2023 22:28
@solipet solipet force-pushed the doug-lg-8867 branch 2 times, most recently from c924aa2 to 6a01b2e Compare March 22, 2023 20:47
Copy link
Contributor

@matthinz matthinz left a comment

Choose a reason for hiding this comment

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

If the user restarts IdV (via a "Start Over" button), should we send them back to the outage notice screen instead of the welcome screen?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this SSN context block needed here? I think I don't see how it relates.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is testing the version of the SSN step that is removed from the FSM (which is feature flagged) to make sure it's routing correctly. I'm not sure why it's showing up in this diff though. Something must have got moved around

Copy link
Contributor

Choose a reason for hiding this comment

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

That feature flag is deleted in main, so this file is out of date.

Copy link
Contributor

@jmax-gsa jmax-gsa left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 this is a good name

Copy link
Contributor

@matthinz matthinz left a comment

Choose a reason for hiding this comment

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

LGTM pending removing the ssn controller stuff in verify_info_step_spec.rb (and rebasing on main I guess). Nice work!

eric-gade and others added 8 commits March 24, 2023 10:05
Co-authored-by: Doug Price <douglas.price@gsa.gov>

changelog: User-Facing Improvements, vendor outage warnings, Pinpoint
vendor outage screen
-- What
1. Updated header locales
2. Added session checking to controller for vendor_status
3. Added continue button
4. Updated the working test for page display
See added failing verify_info spec
Co-authored-by: Alex Bradley <alexander.bradley@gsa.gov>
solipet and others added 25 commits March 24, 2023 10:05
Also removing unused locales
-- What
1. Updated the Outage show view to use the StatusPageComponent, for
better uniformity;
2. Added a controller helper to determine which url to link to in the
"Exit" button. In the case where there is an SP present and it has a
valid return_to_sp_url set, the exit button will navigate to that
URL. In all other cases it will return the user to the account page.

NOTE: We had to change the yml configuration for local testing SPs so
that it had a correct return_to_sp_url that we could actually test
against.

Co-authored-by: Douglas Price <douglas.price@gsa.gov>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
if GPO verification is not enabled, redirect to vendor outage blocking page.
@solipet solipet merged commit 4a1eb8b into main Mar 24, 2023
@solipet solipet deleted the doug-lg-8867 branch March 24, 2023 14:41
@aduth aduth mentioned this pull request Mar 27, 2023
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.

7 participants