LG-9865: Add same_address_as_id to in person verify visited analytics#8585
LG-9865: Add same_address_as_id to in person verify visited analytics#8585gina-yamada merged 7 commits intomainfrom
Conversation
| def extra_analytics_properties | ||
| extra = { | ||
| pii_like_keypaths: [[:same_address_as_id], [:state_id, :state_id_jurisdiction]], | ||
| } | ||
| unless flow_session[:pii_from_user]&.[](:same_address_as_id).nil? | ||
| extra[:same_address_as_id] = | ||
| flow_session[:pii_from_user][:same_address_as_id].to_s == 'true' | ||
| end | ||
| extra |
There was a problem hiding this comment.
Eventually you'll want this method in a shared concern as you move more steps out of the FSM, but it makes sense to leave it here for now.
There was a problem hiding this comment.
I created an issue to handle this in the next step.
| irs_reproofing: false, | ||
| step: 'verify', | ||
| same_address_as_id: true, | ||
| pii_like_keypaths: [[:same_address_as_id], [:state_id, :state_id_jurisdiction]], |
There was a problem hiding this comment.
I just checked, and the first thing Analytics#track_events does is attributes.delete(:pii_like_keypaths), so that's why they don't show up in the feature specs.
| } | ||
| unless flow_session[:pii_from_user]&.[](:same_address_as_id).nil? | ||
| extra[:same_address_as_id] = | ||
| flow_session[:pii_from_user][:same_address_as_id].to_s == 'true' |
There was a problem hiding this comment.
Just curious, but why are we coercing to a string here? Do we not know the type of :same_address_as_id?
In the spec it's a boolean, so if that's the case then you shouldn't have to coerce the type or compare the value.
There was a problem hiding this comment.
@JackRyan1989 This bit of code was taken from in_person_flow.rb, I kept it the exact same as is it just missing on analytics when moving verify out of the flow state machine (discovered in the delete PR). There is a ticket, LG-9543, to refactor all string instances of same_address_as_id to a boolean because both are being used. That ticket was started but presented roadblocks. How do you feel about that being handled in ticket LG-9543?
There was a problem hiding this comment.
Sounds good!
Super nit: Maybe change the "same_address_as_id" to be a string in the test then? This is not a blocking comment.
There was a problem hiding this comment.
This is not needed. Analytic data is a boolean. @JackRyan1989 and I jumped on a call.
tomas-nava
changed the title
sheldon-b
left a comment
sheldon-b
left a comment
There was a problem hiding this comment.
Couple of minor comments, otherwise lgtm!
Updated comment to include more detail for following line of code Co-authored-by: Sheldon Bachstein <sheldon.bachstein@gsa.gov>
Co-authored-by: Sheldon Bachstein <sheldon.bachstein@gsa.gov>
|
Thanks @sheldon-b |
4 participants
🎫 Ticket
LG-9865 Work for Remove In Person Verify
🛠 Summary of changes
Add same_address_as_id to in person verify visited analytics