Skip to content

LG-4944: track whether the user edits their address or not#5484

Merged
solipet merged 4 commits intomainfrom
dprice-lg-4944-log-edited-address
Oct 14, 2021
Merged

LG-4944: track whether the user edits their address or not#5484
solipet merged 4 commits intomainfrom
dprice-lg-4944-log-edited-address

Conversation

@solipet
Copy link
Contributor

@solipet solipet commented Oct 8, 2021

Context: we pre-populate address for identity resolution step (LexisNexis Instant Verify) with the values from Acuant (scanned from ID)

We let users change the address, and this PR adds logging to track which users go through proofing with the scanned address vs an edited address.

The new key address_edited will appear in both the IdV: address submitted and IdV: doc auth optional verify_wait submitted events, i.e., when the user is done with the address editor, and when they submit their name/address/ssn for verification.

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting... this argument wasn't doing anything before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope and it wasn't even a user, but a hash with a user and previous_params, neither of which were ever accessed. ¯\_(ツ)_/¯

Copy link
Contributor

Choose a reason for hiding this comment

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

Trying to wrap my head around the second half of this condition: If they're both blank, wouldn't they be equal to each other, thus short-circuit in the first part? Or is this accounting for differences between nil and '' ? If so, could a .to_s in send(key).to_s != @pii[key].to_s work too?

Copy link
Contributor

Choose a reason for hiding this comment

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

is params all the keys of PII? or just the address keys?

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 was thinking of accounting for the differences between nil or "" and ' '. nil.to_s is equal to "", but if they happen to accidentally hit a space in the address2 field, this will still catch that.

In 8f754ac I switched this to use unless - removes the negatives in the logic which hopefully makes it easier to grok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zachmargolis - on line 36, an error is raised if any non-address params are included.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we not just use .present? here and simplify that way rather than going the unless route?

Something like:
if send(key) != @pii[key] && (send(key).present? || @pii[key].present?)

I think that's logically equivalent. If new and old aren't the same and at least one of them is present it has been modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amathews-fs sounds good - updated in fb19708

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like sometimes we have a idv/capture_doc key in here for the hybrid flow? does this always exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is past the point where the hybrid flow rejoins the regular doc_auth flow so I assume so. I tried it out locally in the hybrid flow and it worked.

Copy link
Contributor

@amathews-fs amathews-fs left a comment

Choose a reason for hiding this comment

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

LGTM!

@solipet solipet force-pushed the dprice-lg-4944-log-edited-address branch from fb19708 to df4a378 Compare October 14, 2021 14:05
@solipet solipet merged commit 022e134 into main Oct 14, 2021
@solipet solipet deleted the dprice-lg-4944-log-edited-address branch October 14, 2021 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants