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
Next Next commit
LG-14196 | idv_consent_given -> idv_consent_given_at
This is part 2, which moves everything over to using the new
timestamp. Most things only care about the boolean state, so I added
a method.
  • Loading branch information
n1zyy committed Aug 29, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit dd5dc66ecf9be4d3d0f350390759fec51656dc10
7 changes: 3 additions & 4 deletions app/controllers/idv/agreement_controller.rb
Original file line number Diff line number Diff line change
@@ -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

@@ -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)

@@ -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 &&
@@ -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,
)
2 changes: 1 addition & 1 deletion app/controllers/idv/document_capture_controller.rb
Original file line number Diff line number Diff line change
@@ -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'
2 changes: 1 addition & 1 deletion app/controllers/idv/how_to_verify_controller.rb
Original file line number Diff line number Diff line change
@@ -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:) {
2 changes: 1 addition & 1 deletion app/controllers/idv/hybrid_handoff_controller.rb
Original file line number Diff line number Diff line change
@@ -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)
6 changes: 5 additions & 1 deletion app/services/idv/session.rb
Original file line number Diff line number Diff line change
@@ -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
@@ -291,6 +290,11 @@ 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] &&
session[:idv_consent_given_at] > (Time.zone.now - 1.month))
n1zyy marked this conversation as resolved.
Show resolved Hide resolved
end

private

attr_reader :user_session
2 changes: 1 addition & 1 deletion spec/controllers/idv/address_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion spec/controllers/idv/agreement_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion spec/controllers/idv/enter_password_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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]
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
@@ -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
@@ -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
2 changes: 1 addition & 1 deletion spec/controllers/idv/hybrid_handoff_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions spec/controllers/idv/link_sent_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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)
@@ -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
2 changes: 1 addition & 1 deletion spec/controllers/idv/otp_verification_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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]
2 changes: 1 addition & 1 deletion spec/controllers/idv/phone_errors_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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'
9 changes: 5 additions & 4 deletions spec/policies/idv/flow_policy_spec.rb
Original file line number Diff line number Diff line change
@@ -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'
@@ -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
@@ -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'
@@ -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
2 changes: 1 addition & 1 deletion spec/support/flow_policy_helper.rb
Original file line number Diff line number Diff line change
@@ -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