Skip to content

LG-9971 Add outage before_actions to post-FSM controllers#8517

Merged
amirbey merged 13 commits intomainfrom
amirbey/LG-9971-add-outage-actions-to-post-fsm-controllers
Jun 6, 2023
Merged

LG-9971 Add outage before_actions to post-FSM controllers#8517
amirbey merged 13 commits intomainfrom
amirbey/LG-9971-add-outage-actions-to-post-fsm-controllers

Conversation

@amirbey
Copy link
Contributor

@amirbey amirbey commented May 31, 2023 •
edited by soniaconnolly
Loading

🎫 Ticket

LG-9971

🛠 Summary of changes

Refactor the doc auth controller outage check into a concern which is used in post-FSM controllers.

📜 Testing Plan

Set these variables in application.yml.

vendor_status_sms: full_outage
vendor_status_voice: full_outage
feature_idv_force_gpo_verification_enabled: true

@amirbey amirbey changed the title add outage to all post FSM controllers LG-9971 Add outage before_actions to post-FSM controllers May 31, 2023
@amirbey amirbey self-assigned this May 31, 2023
@amirbey amirbey force-pushed the amirbey/LG-9971-add-outage-actions-to-post-fsm-controllers branch 2 times, most recently from c682769 to c539312 Compare June 2, 2023 16:47
@amirbey amirbey force-pushed the amirbey/LG-9971-add-outage-actions-to-post-fsm-controllers branch from c539312 to 461ea2f Compare June 2, 2023 18:30
@amirbey amirbey force-pushed the amirbey/LG-9971-add-outage-actions-to-post-fsm-controllers branch from d5b9847 to 56cc025 Compare June 5, 2023 14:50
@amirbey amirbey requested review from matthinz and solipet June 5, 2023 16:13
@amirbey amirbey marked this pull request as ready for review June 5, 2023 16:32
@amirbey amirbey requested a review from soniaconnolly June 5, 2023 16:32
solipet
solipet approved these changes Jun 5, 2023
View reviewed changes
Copy link
Contributor

@solipet solipet left a comment

Choose a reason for hiding this comment

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

LGTM!

app/controllers/idv/doc_auth_controller.rb
# step and go straight to document upload
flow_session[:skip_upload_step] = true unless FeatureManagement.idv_allow_hybrid_flow?

session[:vendor_outage_redirect] = current_step
Copy link
Contributor

@solipet solipet Jun 5, 2023

Choose a reason for hiding this comment

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

Yay!! love deleting unused session vars!

soniaconnolly
soniaconnolly reviewed Jun 5, 2023
View reviewed changes
Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

Does OutageConcern need to be added to review_controller and address_controller? Also possibly personal_key_controller although it doesn't include IdvStepConcern (maybe it should).

app/controllers/idv/document_capture_controller.rb
Comment on lines 6 to +7
include IdvStepConcern
include OutageConcern
Copy link
Contributor

@soniaconnolly soniaconnolly Jun 5, 2023

Choose a reason for hiding this comment

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

We talked about including OutageConcern in IdvStepConcern and putting the before action there. Did something end up blocking that?

Copy link
Contributor Author

@amirbey amirbey Jun 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I attempted this suggestion; however, the feature tests kept failing ... perhaps this can be refactored in a future iteration.

app/controllers/idv/phone_controller.rb
before_action :confirm_step_needed
before_action :set_idv_form
# rubocop:disable Rails/LexicallyScopedActionFilter
before_action :check_for_outage, only: :show
Copy link
Contributor

@soniaconnolly soniaconnolly Jun 5, 2023

Choose a reason for hiding this comment

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

Why did rubocop complain about this one and not the others?

@amirbey amirbey requested a review from soniaconnolly June 6, 2023 16:15
soniaconnolly
soniaconnolly approved these changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

LGMT! Discussed and tested locally.

amirbey
amirbey commented Jun 6, 2023
View reviewed changes
app/controllers/idv/in_person/verify_info_controller.rb
include StepUtilitiesConcern
include Steps::ThreatMetrixStepHelper
include VerifyInfoConcern
include OutageConcern
Copy link
Contributor Author

@amirbey amirbey Jun 6, 2023

Choose a reason for hiding this comment

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

added as suggested by @tomas-nava 👍🏿

@amirbey amirbey merged commit 035d146 into main Jun 6, 2023
@amirbey amirbey deleted the amirbey/LG-9971-add-outage-actions-to-post-fsm-controllers branch June 6, 2023 17:02
@solipet solipet mentioned this pull request Jun 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@solipet solipet solipet approved these changes

@matthinz matthinz Awaiting requested review from matthinz

+1 more reviewer

@soniaconnolly soniaconnolly soniaconnolly approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@amirbey amirbey

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@amirbey @soniaconnolly @solipet