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-14196 | idv_consent_given -> idv_consent_given_at #11168

Merged
merged 10 commits into from
Sep 6, 2024
7 changes: 3 additions & 4 deletions app/controllers/idv/agreement_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def show
)

@consent_form = Idv::ConsentForm.new(
idv_consent_given: idv_session.idv_consent_given,
idv_consent_given: idv_session.idv_consent_given?,
)
end

Expand All @@ -27,7 +27,7 @@ def update
skip_to_capture if params[:skip_hybrid_handoff]

@consent_form = Idv::ConsentForm.new(
idv_consent_given: idv_session.idv_consent_given,
idv_consent_given: idv_session.idv_consent_given?,
)
result = @consent_form.submit(consent_form_params)

Expand All @@ -36,7 +36,6 @@ def update
)

if result.success?
idv_session.idv_consent_given = true
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to keep these around for 1 more release cycle. Old instances will still be inspecting idv_session.idv_consent_given. We need to stop reading from these everywhere before we can stop writing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, good catch! The first PR started setting the timestamp, allowing this to begin reading it immediately, but I overlooked the inverse which will occur in the 50/50 state.

idv_session.idv_consent_given_at = Time.zone.now

if IdentityConfig.store.in_person_proofing_opt_in_enabled &&
Expand All @@ -57,7 +56,7 @@ def self.step_info
next_steps: [:hybrid_handoff, :document_capture, :how_to_verify],
preconditions: ->(idv_session:, user:) { idv_session.welcome_visited },
undo_step: ->(idv_session:, user:) do
idv_session.idv_consent_given = nil
idv_session.idv_consent_given_at = nil
idv_session.skip_hybrid_handoff = nil
end,
)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/idv/document_capture_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def handle_stored_result

def allow_direct_ipp?
return false unless idv_session.welcome_visited &&
idv_session.idv_consent_given
idv_session.idv_consent_given?
# not allowed when no step param and action:show(get request)
return false if params[:step].blank? || params[:action].to_s != 'show' ||
idv_session.flow_path == 'hybrid'
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/idv/how_to_verify_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def self.step_info
next_steps: [:hybrid_handoff, :document_capture],
preconditions: ->(idv_session:, user:) do
self.enabled? &&
idv_session.idv_consent_given &&
idv_session.idv_consent_given? &&
idv_session.service_provider&.in_person_proofing_enabled
end,
undo_step: ->(idv_session:, user:) {
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/idv/hybrid_handoff_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def self.step_info
controller: self,
next_steps: [:link_sent, :document_capture],
preconditions: ->(idv_session:, user:) {
idv_session.idv_consent_given &&
idv_session.idv_consent_given? &&
(self.selected_remote(idv_session: idv_session) || # from opt-in screen
# back from ipp doc capture screen
idv_session.skip_doc_auth_from_handoff)
Expand Down
2 changes: 1 addition & 1 deletion app/forms/idv/how_to_verify_form.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# frozen_string_literal: true
se# frozen_string_literal: true
n1zyy marked this conversation as resolved.
Show resolved Hide resolved

module Idv
class HowToVerifyForm
Expand Down
5 changes: 4 additions & 1 deletion app/services/idv/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ class Session
gpo_code_verified
had_barcode_attention_error
had_barcode_read_failure
idv_consent_given
idv_consent_given_at
idv_phone_step_document_capture_session_uuid
mail_only_warning_shown
Expand Down Expand Up @@ -291,6 +290,10 @@ def desktop_selfie_test_mode_enabled?
IdentityConfig.store.doc_auth_selfie_desktop_test_mode
end

def idv_consent_given?
Copy link
Member

Choose a reason for hiding this comment

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

Smart to just make this a predicate method

!!session[:idv_consent_given_at]
end

private

attr_reader :user_session
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/idv/address_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
stub_sign_in(user)
stub_analytics
subject.idv_session.welcome_visited = true
subject.idv_session.idv_consent_given = true
subject.idv_session.idv_consent_given_at = Time.zone.now
subject.idv_session.flow_path = 'standard'
subject.idv_session.pii_from_doc = pii_from_doc
end
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/idv/agreement_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@
it 'does not set IDV consent flags' do
put :update, params: params

expect(subject.idv_session.idv_consent_given).to be_nil
expect(subject.idv_session.idv_consent_given?).to eq(false)
expect(subject.idv_session.idv_consent_given_at).to be_nil
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/idv/enter_password_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
allow(IdentityConfig.store).to receive(:usps_mock_fallback).and_return(false)
allow(subject).to receive(:ab_test_analytics_buckets).and_return(ab_test_args)
subject.idv_session.welcome_visited = true
subject.idv_session.idv_consent_given = true
subject.idv_session.idv_consent_given_at = Time.zone.now
subject.idv_session.flow_path = 'standard'
subject.idv_session.pii_from_doc = Pii::StateId.new(**Idp::Constants::MOCK_IDV_APPLICANT)
subject.idv_session.ssn = Idp::Constants::MOCK_IDV_APPLICANT_WITH_PHONE[:ssn]
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/idv/how_to_verify_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
allow(subject).to receive(:ab_test_analytics_buckets).and_return(ab_test_args)
allow(subject.idv_session).to receive(:service_provider).and_return(service_provider)
subject.idv_session.welcome_visited = true
subject.idv_session.idv_consent_given = true
subject.idv_session.idv_consent_given_at = Time.zone.now
end

describe 'before_actions' do
Expand Down Expand Up @@ -133,7 +133,7 @@

context 'agreement step not completed' do
before do
subject.idv_session.idv_consent_given = nil
subject.idv_session.idv_consent_given_at = nil
end

it 'redirects to agreement path' do
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/idv/hybrid_handoff_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@

context 'agreement step is not complete' do
before do
subject.idv_session.idv_consent_given = nil
subject.idv_session.idv_consent_given_at = nil
end

it 'redirects to idv_agreement_url' do
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/idv/link_sent_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
before do
stub_sign_in(user)
subject.idv_session.welcome_visited = true
subject.idv_session.idv_consent_given = true
subject.idv_session.idv_consent_given_at = Time.zone.now
subject.idv_session.flow_path = 'hybrid'
stub_analytics
allow(subject).to receive(:ab_test_analytics_buckets).and_return(ab_test_args)
Expand Down Expand Up @@ -87,7 +87,7 @@
context 'flow_path is standard' do
it 'redirects to idv_document_capture_url' do
subject.idv_session.welcome_visited = true
subject.idv_session.idv_consent_given = true
subject.idv_session.idv_consent_given_at = Time.zone.now
subject.idv_session.flow_path = 'standard'

get :show
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/idv/otp_verification_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
sign_in(user)
stub_verify_steps_one_and_two(user)
subject.idv_session.welcome_visited = true
subject.idv_session.idv_consent_given = true
subject.idv_session.idv_consent_given_at = Time.zone.now
subject.idv_session.flow_path = 'standard'
subject.idv_session.pii_from_doc = Pii::StateId.new(**Idp::Constants::MOCK_IDV_APPLICANT)
subject.idv_session.ssn = Idp::Constants::MOCK_IDV_APPLICANT_WITH_PHONE[:ssn]
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/idv/phone_errors_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
if user
stub_sign_in(user)
subject.idv_session.welcome_visited = true
subject.idv_session.idv_consent_given = true
subject.idv_session.idv_consent_given_at = Time.zone.now
subject.idv_session.flow_path = 'standard'
subject.idv_session.pii_from_doc = Pii::StateId.new(**Idp::Constants::MOCK_IDV_APPLICANT)
subject.idv_session.ssn = '123-45-6789'
Expand Down
9 changes: 5 additions & 4 deletions spec/policies/idv/flow_policy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
idv_session.welcome_visited = true
idv_session.document_capture_session_uuid = SecureRandom.uuid

idv_session.idv_consent_given = true
idv_session.idv_consent_given_at = Time.zone.now
idv_session.skip_hybrid_handoff = true

idv_session.flow_path = 'standard'
Expand All @@ -56,7 +56,7 @@
expect(idv_session.welcome_visited).not_to be_nil
expect(idv_session.document_capture_session_uuid).not_to be_nil

expect(idv_session.idv_consent_given).to be_nil
expect(idv_session.idv_consent_given_at).to be_nil
expect(idv_session.skip_hybrid_handoff).to be_nil

expect(idv_session.flow_path).to be_nil
Expand All @@ -77,7 +77,7 @@
idv_session.welcome_visited = true
idv_session.document_capture_session_uuid = SecureRandom.uuid

idv_session.idv_consent_given = true
idv_session.idv_consent_given_at = Time.zone.now
idv_session.skip_hybrid_handoff = true

idv_session.flow_path = 'standard'
Expand Down Expand Up @@ -130,7 +130,8 @@
end.not_to change {
idv_session.welcome_visited
idv_session.document_capture_session_uuid
idv_session.idv_consent_given
idv_session.idv_consent_given_at
idv_session.idv_consent_given?
idv_session.skip_hybrid_handoff

idv_session.flow_path
Expand Down
2 changes: 1 addition & 1 deletion spec/support/flow_policy_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def stub_step(key:, idv_session:)
when :welcome
idv_session.welcome_visited = true
when :agreement
idv_session.idv_consent_given = true
idv_session.idv_consent_given_at = Time.zone.now
when :how_to_verify
idv_session.skip_doc_auth = false
idv_session.skip_doc_auth_from_how_to_verify = false
Expand Down