Skip to content

LG-13577 Save Sponsor ID on new Enrollments #10859

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.

Sign up for GitHub

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

Merged
gina-yamada merged 9 commits into from
Jul 1, 2024
Merged

LG-13577 Save Sponsor ID on new Enrollments #10859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7c49014
Save sponsor_id on new IPP enrollments
gina-yamada Jun 24, 2024
e8bdc8b
Fix linter errors
gina-yamada Jun 24, 2024
6fed24e
Add addi check before setting sponsor_id
gina-yamada Jun 24, 2024
9347f9c
changelog: Internal, In-Person Proofing, Save Sponsor Id on IPP Enrol…
gina-yamada Jun 24, 2024
a159529
Remove config var checks
gina-yamada Jun 25, 2024
c553ff7
Remove white space inside it description of spec
gina-yamada Jun 25, 2024
39b487a
Merge branch 'main' of github.com:18F/identity-idp into yamada/LG-135…
gina-yamada Jun 26, 2024
93c0b38
Merge branch 'main' of github.com:18F/identity-idp into yamada/LG-135…
gina-yamada Jun 28, 2024
7baef31
Merge branch 'main' of github.com:18F/identity-idp into yamada/LG-135…
gina-yamada Jul 1, 2024
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
6 changes: 6 additions & 0 deletions app/services/usps_in_person_proofing/enrollment_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ def schedule_in_person_enrollment(user:, pii:, is_enhanced_ipp:, opt_in: nil)
enrollment_code = create_usps_enrollment(enrollment, pii, is_enhanced_ipp)
return unless enrollment_code

if is_enhanced_ipp
Copy link
Copy Markdown
Contributor

@eileen-nava eileen-nava Jun 26, 2024

Choose a reason for hiding this comment

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

💯

enrollment.sponsor_id = IdentityConfig.store.usps_eipp_sponsor_id
else
enrollment.sponsor_id = IdentityConfig.store.usps_ipp_sponsor_id
end

# update the enrollment to status pending
enrollment.enrollment_code = enrollment_code
enrollment.status = :pending
Expand Down
24 changes: 23 additions & 1 deletion spec/services/usps_in_person_proofing/enrollment_helper_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
end
let(:proofer) { UspsInPersonProofing::Mock::Proofer.new }
let(:is_enhanced_ipp) { false }
let(:usps_ipp_sponsor_id) { '2718281828' }

before(:each) do
stub_request_token
Expand All @@ -38,6 +39,7 @@
allow(subject).to receive(:analytics).and_return(subject_analytics)
allow(IdentityConfig.store).to receive(:usps_ipp_transliteration_enabled).
and_return(usps_ipp_transliteration_enabled)
allow(IdentityConfig.store).to receive(:usps_ipp_sponsor_id).and_return(usps_ipp_sponsor_id)
end

describe '#schedule_in_person_enrollment' do
Expand Down Expand Up @@ -179,10 +181,14 @@
end
end

it 'sets enrollment status to pending and sets established at date and unique id' do
it <<~STR.squish do
sets enrollment status to pending, sponsor_id to usps_ipp_sponsor_id,
and sets established at date and unique id
Copy link
Copy Markdown
Contributor

@shanechesnutt-ft shanechesnutt-ft Jun 26, 2024

Choose a reason for hiding this comment

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

What if to clean this up instead of having a multi-line description you break it out into multiple it blocks by wrapping this into a context. Maybe something like this?

      context 'when the in person enrollment is not enhanced ipp' do
        before do
          subject.schedule_in_person_enrollment(user:, pii:, is_enhanced_ipp:)
        end

        it 'sets enrollment status to pending' do
          expect(user.in_person_enrollments.first.status).to eq(InPersonEnrollment::STATUS_PENDING)
        end

        it 'sets sponsor_id to usps_ipp_sponsor_id' do
          expect(user.in_person_enrollments.first.sponsor_id).to eq(usps_ipp_sponsor_id)
        end

        it 'sets established at date' do
          expect(user.in_person_enrollments.first.enrollment_established_at).to_not be_nil
        end

        it 'sets unique id' do
          expect(user.in_person_enrollments.first.unique_id).to_not be_nil
        end
      end

This does however slightly increases the speed of the tests by 0.25 seconds, but I do think it helps with readability.

Copy link
Copy Markdown
Contributor Author

@gina-yamada gina-yamada Jun 28, 2024

Choose a reason for hiding this comment

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

@shanechesnutt-ft this is good suggestion. I think I will keep the existing pattern and thus speed over readability. If others have strong opinions in preference over readability, we can rework at a later time. I appreciate your feedback though. 🙏

Copy link
Copy Markdown
Contributor

@mitchellhenke mitchellhenke Jun 28, 2024

Choose a reason for hiding this comment

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

I tend to agree with leaning towards the single test. I think the intent of each test is reflected well enough in the expect statements.

STR
subject.schedule_in_person_enrollment(user:, pii:, is_enhanced_ipp:)

expect(user.in_person_enrollments.first.status).to eq(InPersonEnrollment::STATUS_PENDING)
expect(user.in_person_enrollments.first.sponsor_id).to eq(usps_ipp_sponsor_id)
expect(user.in_person_enrollments.first.enrollment_established_at).to_not be_nil
expect(user.in_person_enrollments.first.unique_id).to_not be_nil
end
Expand Down Expand Up @@ -331,10 +337,26 @@
allow(proofer).to receive(:request_enroll).and_call_original
end
context 'when the user is going through enhanced ipp' do
let!(:enrollment) do
create(
:in_person_enrollment,
user: user,
service_provider: service_provider,
status: :establishing,
profile: nil,
)
end

it 'creates an enhanced ipp enrollment' do
expect(proofer).to receive(:request_enroll).with(applicant, is_enhanced_ipp)
subject.create_usps_enrollment(enrollment, pii, is_enhanced_ipp)
end

it 'saves sponsor_id on the enrollment to the usps_eipp_sponsor_id' do
subject.schedule_in_person_enrollment(user:, pii:, is_enhanced_ipp:)

expect(user.in_person_enrollments.first.sponsor_id).to eq(usps_eipp_sponsor_id)
Copy link
Copy Markdown
Contributor

@eileen-nava eileen-nava Jun 26, 2024

Choose a reason for hiding this comment

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

👏🏻

end
end
end

Expand Down