-
Notifications
You must be signed in to change notification settings - Fork 114
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-12617: Add additional profile-related fields to enhanced Idv events #10270
Conversation
e0a4eec
to
c90896b
Compare
first_letter_requested_at: pending_profile.gpo_verification_pending_at, | ||
hours_since_first_letter: 24, | ||
**ab_test_args, | ||
), | ||
) |
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.
Rather than adding
active_profile_idv_level: nil,
pending_profile_idv_level: nil,
proofing_components: nil,
here, I switched it (and several other specs) to use hash_including
. My feeling is that these tests aren't really testing these extra analytics arguments--those should be covered by a combination of the Idv::AnalyticsEventsEnhancer
spec and the analytics feature spec.
6b22eee
to
98b0c2b
Compare
Summarize the history of a user's profiles in IdV analytics events changelog: Internal, Identity verification, Include profile metadata in analytics logs
Just return the levels, and use .presence to ensure falsey values are nil so they're compacted properly
It's a lot of data and we want to keep average event payload size low.
Adding the following to covered methods: - active_profile_idv_level - pending_profile_idv_level - profile_history Also making sure proofing_components is documented appropriately.
Sprinkle all these new args around a little.
Rather than add `active_profile_idv_level: nil, pending_profile_idv_level: nil` everywhere, I moved many of these over to using hash_including, which I think makes it clearer what these specs actually _care_ about checking.
ad22588
to
81cff75
Compare
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.
LGTM. That cleaned up nicely.
🎫 Ticket
Link to the relevant ticket:
LG-12617
🛠 Summary of changes
This PR updates the
Idv::AnalyticsEventsEnhancer
to add more fields to covered Idv-related events. Previously, we were only addingproofing_components
. With this update we will add:pending_profile_idv_level
: The verification level associated with the user's pending profileactive_profile_idv_level
: The verification level associated with the user's active profileAdditionally, a subset of events will have a new
profile_history
argument added. This argument is an array. Each item is a hash including the following fields:id
active
idv_level
created_at
verified_at
activated_at
in_person_verification_pending_at
gpo_verification_pending_at
fraud_review_pending_at
fraud_rejection_at
fraud_pending_reason
deactivation_reason