LG-14673: Add SP info to fraud review events #11390
Conversation
changelog: Internal, Fraud review, Attribute fraud review analytics events to SPs.
Update Analytics to always attempt to log SP, even when there is no associated request.
| @@ -21,6 +21,7 @@ def track_event(event, attributes = {}) | |||
| event_properties: attributes.except(:user_id).compact, | |||
| new_event: first_event_this_session?, | |||
| path: request&.path, | |||
| service_provider: sp, | |||
There was a problem hiding this comment.
I had to triple check, but I forgot what the type of sp was as an attribute, since we're here, WDYT of adding type documentation to the initialzer for this class like
# @param sp [String] issuer string for the service provider for this analytics event
There was a problem hiding this comment.
On that note, would it be worthwhile to rename the sp param to sp_issuer or just issuer?
There was a problem hiding this comment.
Yeah, I agree with that... probably not going to do in this PR but standardizing usage of sp vs issuer vs sp_issuer across the codebase would be great. I think I'd be in favor of:
sp = A ServiceProvider instance
sp_issuer = sp.issuer, an issuer string
If include_missing is specified and there are no missing users, don't log an event for them
matthinz
force-pushed
the
matthinz/14673-fraud-sp-info
branch
from
f765520 to
330161e
Compare
Include service_provider in analytics_attributes now that it does not depend on request being non-nil.
| profile_fraud_review_pending_at: profile_fraud_review_pending_at, | ||
| profile_age_in_seconds: profile_age_in_seconds, | ||
| profile_fraud_review_pending_at:, | ||
| profile_age_in_seconds: profile&.profile_age_in_seconds, |
There was a problem hiding this comment.
(suggestion, non-blocking): I think it is worthwhile keeping the profile attr wrapping methods to streamline reading as profile attributes are inconsistently accessed with safe navigation (see line 281 where we don't use safe navigation).
🎫 Ticket
Link to the relevant ticket:
LG-14673
🛠 Summary of changes
When an operator executes the
review-passorreview-rejectscripts, they log an analytics event. This PR updates those analytics events to be attributed to the SP that was associated with the initial profile creation.📜 Testing Plan
Provide a checklist of steps to confirm the changes.
bin/action-account review-reject $(bin/rails runner 'puts User.last.uuid')properties.service_providerkey, set to the SP you were originally going through IdV for.bin/action-account review-pass $(bin/rails runner 'puts User.last.uuid'properties.service_providerkey, set to the SP you were originally going through IdV for.