-
Notifications
You must be signed in to change notification settings - Fork 115
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-14988 Send additional attributes to AAMVA #11565
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you're just implementing a feature we were asked to implement, but as this is written this would discriminate against non-binary folks.
I know we are not enforcing a match initially, but because it's all behind one feature flag, I fear some day we're going to look at aggregate proofing rate, see that this PR doesn't hurt it, and switch this over to match.
I'd like to ask that we remove the sex parameter for this until AAMVA fixes their API.
Withdrawn. I missed the key part where this is handled.
# | ||
# The height is provided in feet-inches (i.e. 5 foot 10 inches is presented as "510"). | ||
# | ||
[(height / 12).to_s, (height % 12).to_s].join('') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to zero-pad inches? E.g., if someone is 61" tall, are they 51
or 501
?
aamva_applicant = Proofing::Aamva::Applicant.from_proofer_applicant(proofer_applicant) | ||
|
||
# This is intended to describe 6'1" | ||
expect(aamva_applicant[:height]).to eq('61') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, provided this matches what AAMVA says, this answers my first question about padding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D'oh! I am a silly goose and misunderstood this. AAMVA's API may not represent all values a person can have on their ID, but that is in fact handled by this code as an optional attribute anyway.
That was the only objection I had to this, so please enjoy this about-face approval. 😄
context 'when the sex is blank' do | ||
it 'does not send a sex code value' do | ||
applicant.sex = nil | ||
expect(subject.body).to_not include('<aa:PersonSexCode>') | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Also noting that this test specifically covers the concern I had erroneously raised.)
We are planning on sending additional attributes that we read off of documents to AAMVA. This commit modifies the AAMVA client to do so. The new attributes are: - Middle name - Name suffix - Sex - Height - Weight - Eye color These attributes are only sent if they are available in the applicant. For most of the attributes we do not currently read them from the document so they will not be sent until we enable the feature to do so. The exception to this is middle name. For that reason a feature flag controls whether we send middle name. We will log whether these attributes are sent and whether they are validated. None of these attributes are "required attributes" so a user will still pass if they are absent or if they do not match. changelog: Internal, AAMVA DLDV, Send additional attributes to AAMVA
258ca64
to
73a5452
Compare
We are planning on sending additional attributes that we read off of documents to AAMVA. This commit modifies the AAMVA client to do so. The new attributes are:
These attributes are only sent if they are available in the applicant. For most of the attributes we do not currently read them from the document so they will not be sent until we enable the feature to do so. The exception to this is middle name. For that reason a feature flag controls whether we send middle name.
We will log whether these attributes are sent and whether they are validated. None of these attributes are "required attributes" so a user will still pass if they are absent or if they do not match.