Skip to content

LG-9041 Add address line 2 from TrueID#7912

Merged
soniaconnolly merged 6 commits intomainfrom
sonia-lg-9041-address-line2-trueid
Mar 2, 2023
Merged

LG-9041 Add address line 2 from TrueID#7912
soniaconnolly merged 6 commits intomainfrom
sonia-lg-9041-address-line2-trueid

Conversation

@soniaconnolly
Copy link
Contributor

@soniaconnolly soniaconnolly commented Mar 1, 2023

🎫 Ticket

LG-9041

🛠 Summary of changes

When Address Line 2 is available from TrueID, save it in pii_from_doc and add address_line2_present to the results to be logged. This follows the example from @matthinz PR #7906 for Acuant.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Set doc auth to go through TrueID in staging
  • Test with fake ID with second address line, and one without
  • Confirm that verification completes successfully
  • Confirm that analytics contain address_line2_present as expected on the idv_doc_auth_submitted_image_upload_vendor analytics event

@soniaconnolly soniaconnolly requested review from a team and matthinz March 1, 2023 23:12
changelog: User-Facing Improvements, Identity verification, Ensure that second line of address is read from identity documents (TrueID)
Comment on lines +652 to +656
{
"Group": "IDAUTH_FIELD_DATA",
"Name": "Fields_AddressLine2",
"Values": [{"Value": "APT 3E"}]
},
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know much about how these tests + fixtures are structured, but is it required that we add Fields_AddressLine2 to all the JSON fixtures, or could we get away with adding it to the success ones only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure which would be better. It's not required to add to the other ones and I can take it back out.

Comment on lines +131 to +134
body_no_line2 = JSON.parse(LexisNexisFixtures.true_id_response_success_2).tap do |json|
json['Products'].first['ParameterDetails'] = json['Products'].first['ParameterDetails'].
select { |f| f['Name'] != 'Fields_AddressLine2' }
end.to_json
Copy link
Contributor

Choose a reason for hiding this comment

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

usually we don't define variables at context-scope? I would either put this inside its own let or move it inside the block for `let(:success_response_noline2)

Comment on lines +141 to +143
it 'notes that address line 2 was not present' do
expect(response.to_h).to include(address_line2_present: false)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a spec that checks that pii_from_doc includes address2? Or is that for a separate step?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good points! Thanks for pointing to improved tests. Fixed with d0ede82.

Put variable inside let block, and test for address2 in pii_from_doc
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

let(:response) { described_class.new(success_response_no_line2, config) }

it 'notes that address line 2 was not present' do
expect(response.pii_from_doc).not_to include(address2: 'APT 3E')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
expect(response.pii_from_doc).not_to include(address2: 'APT 3E')
expect(response.pii_from_doc[:address2]).to eq(nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense! I used .to be_nil and that worked.

Copy link
Contributor

Choose a reason for hiding this comment

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

big fan

@soniaconnolly soniaconnolly merged commit 5ecb74b into main Mar 2, 2023
@soniaconnolly soniaconnolly deleted the sonia-lg-9041-address-line2-trueid branch March 2, 2023 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants