Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LG-11461 add FlowPolicy#undo_steps_from! and add to #update through document capture #9593

Merged
merged 12 commits into from
Nov 15, 2023
Merged
4 changes: 4 additions & 0 deletions app/controllers/concerns/idv_step_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,8 @@ def url_for_latest_step
step_info = flow_policy.info_for_latest_step
url_for(controller: step_info.controller, action: step_info.action)
end

def clear_invalid_steps!
flow_policy.undo_steps_from_controller!(controller: self.class)
end
end
2 changes: 2 additions & 0 deletions app/controllers/idv/agreement_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def show
end

def update
clear_invalid_steps!
skip_to_capture if params[:skip_hybrid_handoff]

result = Idv::ConsentForm.new.submit(consent_form_params)
Expand Down Expand Up @@ -48,6 +49,7 @@ def self.step_info
controller: controller_name,
next_steps: [:hybrid_handoff, :document_capture, :phone_question, :how_to_verify],
preconditions: ->(idv_session:, user:) { idv_session.welcome_visited },
undo_step: ->(idv_session:, user:) { idv_session.idv_consent_given = nil },
)
end

Expand Down
5 changes: 5 additions & 0 deletions app/controllers/idv/document_capture_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ def show
end

def update
clear_invalid_steps!
idv_session.redo_document_capture = nil # done with this redo
# Not used in standard flow, here for data consistency with hybrid flow.
document_capture_session.confirm_ocr
Expand Down Expand Up @@ -60,6 +61,10 @@ def self.step_info
controller: controller_name,
next_steps: [:success], # [:ssn],
preconditions: ->(idv_session:, user:) { idv_session.flow_path == 'standard' },
undo_step: ->(idv_session:, user:) do
idv_session.pii_from_doc = nil
idv_session.invalidate_in_person_pii_from_user!
end,
)
end

Expand Down
2 changes: 2 additions & 0 deletions app/controllers/idv/how_to_verify_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def show
end

def update
clear_invalid_steps!
result = Idv::HowToVerifyForm.new.submit(how_to_verify_form_params)

analytics.idv_doc_auth_how_to_verify_submitted(
Expand All @@ -39,6 +40,7 @@ def self.step_info
preconditions: ->(idv_session:, user:) do
self.enabled?
end,
undo_step: ->(idv_session:, user:) {}, # clear any saved data
)
end

Expand Down
2 changes: 2 additions & 0 deletions app/controllers/idv/hybrid_handoff_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def show
end

def update
clear_invalid_steps!
irs_attempts_api_tracker.idv_document_upload_method_selected(
upload_method: params[:type],
)
Expand All @@ -41,6 +42,7 @@ def self.step_info
controller: controller_name,
next_steps: [:link_sent, :document_capture],
preconditions: ->(idv_session:, user:) { idv_session.idv_consent_given },
undo_step: ->(idv_session:, user:) { idv_session.flow_path = nil },
)
end

Expand Down
5 changes: 5 additions & 0 deletions app/controllers/idv/link_sent_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def show
end

def update
clear_invalid_steps!
analytics.idv_doc_auth_link_sent_submitted(**analytics_arguments)

return render_document_capture_cancelled if document_capture_session&.cancelled_at
Expand All @@ -46,6 +47,10 @@ def self.step_info
controller: controller_name,
next_steps: [:success], # [:ssn],
preconditions: ->(idv_session:, user:) { idv_session.flow_path == 'hybrid' },
undo_step: ->(idv_session:, user:) do
idv_session.pii_from_doc = nil
idv_session.invalidate_in_person_pii_from_user!
end,
)
end

Expand Down
3 changes: 3 additions & 0 deletions app/controllers/idv/phone_question_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ def show
end

def phone_with_camera
clear_invalid_steps!
idv_session.phone_with_camera = true
analytics.idv_doc_auth_phone_question_submitted(**analytics_arguments)

redirect_to idv_hybrid_handoff_url
end

def phone_without_camera
clear_invalid_steps!
idv_session.flow_path = 'standard'
idv_session.phone_with_camera = false
analytics.idv_doc_auth_phone_question_submitted(**analytics_arguments)
Expand All @@ -39,6 +41,7 @@ def self.step_info
AbTests::IDV_PHONE_QUESTION.bucket(user.uuid) == :show_phone_question &&
idv_session.idv_consent_given
end,
undo_step: ->(idv_session:, user:) { idv_session.phone_with_camera = nil },
)
end

Expand Down
2 changes: 2 additions & 0 deletions app/controllers/idv/welcome_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def show
end

def update
clear_invalid_steps!
analytics.idv_doc_auth_welcome_submitted(**analytics_arguments)

create_document_capture_session
Expand All @@ -39,6 +40,7 @@ def self.step_info
controller: controller_name,
next_steps: [:agreement],
preconditions: ->(idv_session:, user:) { true },
undo_step: ->(idv_session:, user:) { idv_session.welcome_visited = nil },
)
end

Expand Down
18 changes: 18 additions & 0 deletions app/policies/idv/flow_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ def info_for_latest_step
steps[latest_step]
end

def undo_steps_from_controller!(controller:)
controller_name = controller.ancestors.include?(ApplicationController) ?
controller.controller_name : controller
Copy link
Contributor

Choose a reason for hiding this comment

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

is controller a Class or an instance?

If it's a Class we can do < to check subclass instead of iterating ancestors

Suggested change
controller_name = controller.ancestors.include?(ApplicationController) ?
controller.controller_name : controller
controller_name = controller < ApplicationController ?
controller.controller_name : controller

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! Cool syntax, if a bit opaque. Updated.

key = controller_to_key(controller: controller_name)
undo_steps_from!(key: key)
end

private

def latest_step(current_step: :root)
Expand All @@ -39,6 +46,7 @@ def steps
controller: AccountsController.controller_name,
next_steps: [:welcome],
preconditions: ->(idv_session:, user:) { true },
undo_step: ->(idv_session:, user:) { true },
),
welcome: Idv::WelcomeController.step_info,
agreement: Idv::AgreementController.step_info,
Expand All @@ -54,6 +62,16 @@ def step_allowed?(key:)
steps[key].preconditions.call(idv_session: idv_session, user: user)
end

def undo_steps_from!(key:)
return if key == :success

steps[key].undo_step.call(idv_session: idv_session, user: user)

steps[key].next_steps.each do |next_step|
undo_steps_from!(key: next_step)
end
end

def controller_to_key(controller:)
steps.keys.each do |key|
return key if steps[key].controller == controller
Expand Down
13 changes: 10 additions & 3 deletions app/policies/idv/step_info.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ module Idv
class StepInfo
include ActiveModel::Validations

attr_reader :key, :controller, :action, :next_steps, :preconditions
attr_reader :key, :controller, :action, :next_steps, :preconditions, :undo_step

validates :controller, presence: true
validates :action, presence: true
validate :next_steps_validation, :preconditions_validation
validate :next_steps_validation, :preconditions_validation, :undo_step_validation

def initialize(key:, controller:, next_steps:, preconditions:, action: :show)
def initialize(key:, controller:, next_steps:, preconditions:, undo_step:, action: :show)
@key = key
@controller = controller
@action = action
@next_steps = next_steps
@preconditions = preconditions
@undo_step = undo_step

raise ArgumentError unless valid?
end
Expand All @@ -29,5 +30,11 @@ def preconditions_validation
errors.add(:preconditions, type: :invalid_argument, message: 'preconditions must be a Proc')
end
end

def undo_step_validation
soniaconnolly marked this conversation as resolved.
Show resolved Hide resolved
unless undo_step.is_a?(Proc)
errors.add(:undo_step, type: :invalid_argument, message: 'undo_step must be a Proc')
end
end
end
end
6 changes: 6 additions & 0 deletions app/services/idv/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,12 @@ def has_pii_from_user_in_flow_session
user_session.dig('idv/in_person', :pii_from_user)
end

def invalidate_in_person_pii_from_user!
if user_session.dig('idv/in_person', :pii_from_user)
user_session['idv/in_person'][:pii_from_user] = nil
end
end

def document_capture_complete?
pii_from_doc || has_pii_from_user_in_flow_session || verify_info_step_complete?
end
Expand Down
6 changes: 6 additions & 0 deletions spec/controllers/idv/agreement_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@
}.compact
end

it 'invalidates future steps' do
expect(subject).to receive(:clear_invalid_steps!)

put :update, params: params
end

it 'sends analytics_submitted event with consent given' do
put :update, params: params

Expand Down
6 changes: 6 additions & 0 deletions spec/controllers/idv/document_capture_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@
end
let(:result) { { success: true, errors: {} } }

it 'invalidates future steps' do
expect(subject).to receive(:clear_invalid_steps!)

put :update
end

it 'sends analytics_submitted event' do
allow(result).to receive(:success?).and_return(true)
allow(subject).to receive(:handle_stored_result).and_return(result)
Expand Down
8 changes: 8 additions & 0 deletions spec/controllers/idv/how_to_verify_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,12 @@
expect(response).to render_template :show
end
end

describe '#update' do
it 'invalidates future steps' do
expect(subject).to receive(:clear_invalid_steps!)

put :update
end
end
end
10 changes: 8 additions & 2 deletions spec/controllers/idv/hybrid_handoff_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@
end

context 'hybrid_handoff already visited' do
it 'shows hybrid_handoff' do
it 'shows hybrid_handoff for standard' do
subject.idv_session.flow_path = 'standard'

get :show

expect(response).to render_template :show
end

it 'shows hybrid_handoff' do
it 'shows hybrid_handoff for hybrid' do
subject.idv_session.flow_path = 'hybrid'

get :show
Expand Down Expand Up @@ -220,6 +220,12 @@

let(:document_capture_session_uuid) { '09228b6d-dd39-4925-bf82-b69104095517' }

it 'invalidates future steps' do
expect(subject).to receive(:clear_invalid_steps!)

put :update, params: params
end

it 'sends analytics_submitted event for hybrid' do
put :update, params: params

Expand Down
6 changes: 6 additions & 0 deletions spec/controllers/idv/link_sent_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@
}.merge(ab_test_args)
end

it 'invalidates future steps' do
expect(subject).to receive(:clear_invalid_steps!)

put :update
end

it 'sends analytics_submitted event' do
put :update

Expand Down
12 changes: 12 additions & 0 deletions spec/controllers/idv/phone_question_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,12 @@
describe '#phone_with_camera' do
let(:analytics_name) { :idv_doc_auth_phone_question_submitted }

it 'invalidates future steps' do
expect(subject).to receive(:clear_invalid_steps!)

get :phone_with_camera
end

it 'redirects to hybrid handoff' do
get :phone_with_camera

Expand All @@ -166,6 +172,12 @@
describe '#phone_without_camera' do
let(:analytics_name) { :idv_doc_auth_phone_question_submitted }

it 'invalidates future steps' do
expect(subject).to receive(:clear_invalid_steps!)

get :phone_without_camera
end

it 'redirects to document_capture' do
get :phone_without_camera

Expand Down
6 changes: 6 additions & 0 deletions spec/controllers/idv/welcome_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@
expect(@analytics).to have_logged_event(analytics_name, analytics_args)
end

it 'invalidates future steps' do
expect(subject).to receive(:clear_invalid_steps!)

put :update
end

it 'creates a document capture session' do
expect { put :update }.
to change { subject.idv_session.document_capture_session_uuid }.from(nil)
Expand Down
4 changes: 3 additions & 1 deletion spec/features/idv/doc_auth/redo_document_capture_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@
complete_hybrid_handoff_step

visit idv_verify_info_url

expect(page).to have_current_path(idv_document_capture_path)
DocAuth::Mock::DocAuthMockClient.reset!
attach_and_submit_images
complete_verify_step

expect(page).to have_current_path(idv_phone_path)
Expand Down
2 changes: 1 addition & 1 deletion spec/features/idv/end_to_end_idv_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ def try_to_go_back_from_hybrid_handoff
expect(current_path).to eql(idv_welcome_path)
complete_welcome_step
expect(page).to have_current_path(idv_agreement_path)
expect(page).to have_checked_field(
expect(page).not_to have_checked_field(
t('doc_auth.instructions.consent', app_name: APP_NAME),
visible: :all,
)
Expand Down
18 changes: 18 additions & 0 deletions spec/policies/idv/flow_policy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,24 @@
end
end

context '#undo_steps_from_controller!' do
context 'user is on document_capture step' do
before do
idv_session.welcome_visited = true
idv_session.idv_consent_given = true
idv_session.flow_path = 'standard'
end

it 'user goes back and submits welcome' do
subject.undo_steps_from_controller!(controller: Idv::WelcomeController)

expect(idv_session.welcome_visited).to be_nil
expect(idv_session.idv_consent_given).to be_nil
expect(idv_session.flow_path).to be_nil
end
end
end

context 'each step in the flow' do
before do
allow(Idv::PhoneConfirmationSession).to receive(:from_h).
Expand Down
Loading