Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions app/models/profile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ def self.gpo_verification_pending
end

def self.in_person_verification_pending
where.not(in_person_verification_pending_at: nil)
where.not(in_person_verification_pending_at: nil).
left_outer_joins(:in_person_enrollment).where(in_person_enrollment: { status: :pending })
end

# Instance methods
Expand Down Expand Up @@ -184,7 +185,7 @@ def fraud_deactivation_reason?
end

def in_person_verification_pending?
in_person_verification_pending_at.present?
in_person_verification_pending_at.present? && in_person_enrollment&.pending? != false
end

def deactivate_due_to_gpo_expiration
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/concerns/idv_step_concern_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ def show
end

context 'with pending in person enrollment' do
let(:user) { create(:user, :with_pending_in_person_enrollment, :fully_registered) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

before do
allow(IdentityConfig.store).to receive(:in_person_proofing_enabled).and_return(true)
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/idv/cancellations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@
end

context 'with in person enrollment' do
let(:user) { build(:user, :with_pending_in_person_enrollment) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

before do
allow(IdentityConfig.store).to receive(:in_person_proofing_enabled).and_return(true)
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/idv/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
end

context 'with in person enrollment' do
let(:user) { build(:user, :with_pending_in_person_enrollment) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

it 'logs idv_start_over event with extra analytics attributes for barcode step' do
delete :destroy, params: { step: 'barcode', location: '' }
Expand Down Expand Up @@ -101,7 +101,7 @@
end

context 'with in person enrollment' do
let(:user) { build(:user, :with_pending_in_person_enrollment) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

before do
allow(IdentityConfig.store).to receive(:in_person_proofing_enabled).and_return(true)
Expand Down
3 changes: 3 additions & 0 deletions spec/factories/profiles.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@
trait :in_person_verification_pending do
in_person_verification_pending_at { 15.days.ago }
idv_level { :legacy_in_person }
in_person_enrollment do
association(:in_person_enrollment, :pending, user:, profile: instance)
end
end

trait :fraud_pending_reason do
Expand Down
6 changes: 3 additions & 3 deletions spec/factories/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,9 @@
end

trait :with_pending_in_person_enrollment do
after :build do |user|
profile = create(:profile, :with_pii, :in_person_verification_pending, user: user)
create(:in_person_enrollment, :pending, user: user, profile: profile)
fully_registered
profiles do
[association(:profile, :with_pii, :in_person_verification_pending, user: instance)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ooo love this cleanup to use the factory associations better

Copy link
Copy Markdown
Contributor Author

@aduth aduth Aug 22, 2024

Choose a reason for hiding this comment

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

Yeah, to be honest it was a bit of a nightmare to figure out how to implement this correctly, but it does feel a lot more reliable / portable, and supports overriding trait attributes in a way that's not possible with the lifecycle hooks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same! Big fan. 🤩

end
end

Expand Down
44 changes: 35 additions & 9 deletions spec/models/profile_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,10 @@
end

describe '#in_person_verification_pending?' do
it 'returns true if the in_person_verification_pending_at is present' do
profile = create(
:profile,
:in_person_verification_pending,
user: user,
)
let(:profile) { create(:profile, :in_person_verification_pending, user:) }
subject(:result) { profile.in_person_verification_pending? }

it 'returns true if the in_person_verification_pending_at is present' do
allow(profile).to receive(:update!).and_raise(RuntimeError)

expect(profile.activated_at).to be_nil
Expand All @@ -66,6 +63,27 @@
expect(profile.initiating_service_provider).to be_nil
expect(profile.verified_at).to be_nil
end

context 'with unlinked establishing enrollment' do
# When marking profile as pending in-person proofing, the enrollment hasn't yet been linked to
# the profile.
#
# See: Idv::Session#create_profile_from_applicant_with_password

before do
Comment on lines +71 to +73
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

style, I'd remove the space between?

Suggested change
# See: Idv::Session#create_profile_from_applicant_with_password
before do
# See: Idv::Session#create_profile_from_applicant_with_password
before do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm, yeah. The padding line was intentional to try to "link" it more with the context block than the before block. But maybe in that case the comment should live above the context line? Not sure how I feel about this.

Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Aug 22, 2024

Choose a reason for hiding this comment

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

I guess my thinking is the context description is what we want to happen, the before block (or sometimes a let) is how we want that to happen, and in this case, the comment clarifies the how

profile.in_person_enrollment.update(status: :establishing, profile: nil)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When would a Profile be associated with an InPersonEnrollment and the InPersonEnrollment have profile set to nil?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does the comment above help clarify?

# When marking profile as pending in-person proofing, the enrollment hasn't yet been linked to
# the profile.
#
# See: Idv::Session#create_profile_from_applicant_with_password

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe clarify that it's still linked to the user via the in_person_enrollment.user_id?

end

it { is_expected.to eq(true) }
end

context 'with expired enrollment' do
before do
profile.in_person_enrollment.update(status: :expired)
end

it { is_expected.to eq(false) }
end
end

describe '#encrypt_pii' do
Expand Down Expand Up @@ -1140,9 +1158,17 @@
end

describe '.in_person_verification_pending' do
it 'returns only in_person_verification_pending Profiles' do
user.profiles.create(in_person_verification_pending_at: Time.zone.now)
user.profiles.create(in_person_verification_pending_at: nil)
it 'returns only profiles pending in person verification with unexpired enrollment' do
user.profiles << [
create(:profile, :in_person_verification_pending),
create(:profile),
create(
:profile,
:in_person_verification_pending,
in_person_enrollment: create(:in_person_enrollment, :expired, user:),
),
]

expect(user.profiles.in_person_verification_pending.count).to eq 1
end
end
Expand Down
6 changes: 3 additions & 3 deletions spec/presenters/account_show_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@
it { is_expected.to eq(false) }

context 'with user pending ipp verification' do
let(:user) { build(:user, :with_pending_in_person_enrollment) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

it { is_expected.to eq(true) }
end
Expand Down Expand Up @@ -232,7 +232,7 @@
end

context 'with user pending ipp verification' do
let(:user) { build(:user, :with_pending_in_person_enrollment) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

it { is_expected.to eq(true) }
end
Expand All @@ -245,7 +245,7 @@
end

describe '#formatted_ipp_due_date' do
let(:user) { build(:user, :with_pending_in_person_enrollment) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

subject(:formatted_ipp_due_date) { presenter.formatted_ipp_due_date }

Expand Down
4 changes: 2 additions & 2 deletions spec/views/accounts/_identity_verification.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
let(:ipp_sp_name) { 'IPP SP' }
let(:ipp_sp_issuer) { 'urn:gov:gsa:openidconnect:sp:ipp_sp' }
let(:ipp_sp) { create(:service_provider, issuer: ipp_sp_issuer, friendly_name: ipp_sp_name) }
let(:user) { build(:user, :with_pending_in_person_enrollment) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

before do
ipp_sp
Expand Down Expand Up @@ -82,7 +82,7 @@
end

context 'with user pending ipp verification' do
let(:user) { build(:user, :with_pending_in_person_enrollment) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

it 'shows pending badge' do
expect(rendered).to have_content(t('account.index.verification.pending_badge'))
Expand Down
36 changes: 34 additions & 2 deletions spec/views/accounts/show.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
end

context 'when current user has ipp pending profile' do
let(:user) { build(:user, :with_pending_in_person_enrollment) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

it 'renders idv partial' do
expect(render).to render_template(partial: 'accounts/_identity_verification')
Expand All @@ -111,10 +111,42 @@
end
end

context 'when current user has an in_person_enrollment that was failed' do
let(:vtr) { ['Pe'] }
let(:sp_name) { 'sinatra-test-app' }
let(:user) { create(:user, :with_pending_in_person_enrollment, :with_multiple_emails) }

before do
# Make the in_person_enrollment and associated profile failed
in_person_enrollment = user.in_person_enrollments.first
in_person_enrollment.update!(status: :failed, status_check_completed_at: Time.zone.now)
end

it 'renders the idv partial' do
expect(render).to render_template(partial: 'accounts/_identity_verification')
end
end

context 'when current user has an in_person_enrollment that was cancelled' do
let(:vtr) { ['Pe'] }
let(:sp_name) { 'sinatra-test-app' }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

before do
# Make the in_person_enrollment and associated profile cancelled
in_person_enrollment = user.in_person_enrollments.first
in_person_enrollment.update!(status: :cancelled, status_check_completed_at: Time.zone.now)
end

it 'renders the idv partial' do
expect(render).to render_template(partial: 'accounts/_identity_verification')
end
end

context 'when current user has an in_person_enrollment that expired' do
let(:vtr) { ['Pe'] }
let(:sp_name) { 'sinatra-test-app' }
let(:user) { build(:user, :with_pending_in_person_enrollment) }
let(:user) { create(:user, :with_pending_in_person_enrollment) }

before do
# Expire the in_person_enrollment and associated profile
Expand Down