LG-14100 Include identity-verified status in account reset delete event #11236
Conversation
…atus in account reset delete event
kevinsmaster5
force-pushed
the
kmas-lg-14100-include-identity-verified-reset-delete
branch
from
c74daee
to
b6d2df6
Compare
spec/controllers/account_reset/delete_account_controller_spec.rb
Outdated
Show resolved
Hide resolved
spec/controllers/account_reset/delete_account_controller_spec.rb
Outdated
Show resolved
Hide resolved
spec/controllers/account_reset/delete_account_controller_spec.rb
Outdated
Show resolved
Hide resolved
| def extra_analytics_attributes | ||
| { | ||
| user_id: user.uuid, | ||
| email: user.email_addresses.take&.email, | ||
| account_age_in_days: account_age, | ||
| account_confirmed_at: user.confirmed_at, | ||
| mfa_method_counts: mfa_method_counts, | ||
| proofing_components: profile_components, |
There was a problem hiding this comment.
I hate to switch it up at this point, but I'm thinking it may be better off to just use (or at least start with using) the Profile#idv_level value (mentioned in Slack). Looking at some real-world logging, proofing components alone can't tell us if the user was proofed with biometric. Conversely, idv_level can't tell us if they proofed with GPO, but I think it's probably more pertinent to the event that we know "legacy" vs. with biometric vs in-person.
Plus, strings are a bit easier to work with than a hash 😅
| proofing_components: profile_components, | |
| profile_idv_level: user.active_profile&.idv_level, |
There was a problem hiding this comment.
That worked great. Thanks also to @matthinz for the suggestion.
spec/factories/users.rb
Outdated
| @@ -209,6 +217,7 @@ | |||
| :with_pii, | |||
| idv_level: :unsupervised_with_selfie, | |||
| user: user, | |||
| proofing_components: { document_check: 'mock', document_type: 'biometric' }, | |||
There was a problem hiding this comment.
To my previous comment, these aren't really accurate to the proofing components of a biometric proofed profile in the real-world. Maybe we can just revert the proofing_components changes in this file
| it 'logs info about user with a verified by mail account' do | ||
| user = create(:user, :proofed_with_gpo) | ||
| create_account_reset_request_for(user) | ||
| grant_request(user) | ||
| session[:granted_token] = AccountResetRequest.first.granted_token | ||
|
|
||
| delete :delete | ||
|
|
||
| expect(@analytics).to have_logged_event( | ||
| 'Account Reset: delete', | ||
| user_id: user.uuid, | ||
| success: true, | ||
| errors: {}, | ||
| mfa_method_counts: { phone: 1 }, | ||
| profile_idv_level: 'legacy_unsupervised', | ||
| identity_verified: true, | ||
| account_age_in_days: 0, | ||
| account_confirmed_at: user.confirmed_at, | ||
| ) | ||
| end | ||
|
|
There was a problem hiding this comment.
I think we could drop this since we're not expecting anything distinguishable from GPO anymore.
| it 'logs info about user with a verified by mail account' do | |
| user = create(:user, :proofed_with_gpo) | |
| create_account_reset_request_for(user) | |
| grant_request(user) | |
| session[:granted_token] = AccountResetRequest.first.granted_token | |
| delete :delete | |
| expect(@analytics).to have_logged_event( | |
| 'Account Reset: delete', | |
| user_id: user.uuid, | |
| success: true, | |
| errors: {}, | |
| mfa_method_counts: { phone: 1 }, | |
| profile_idv_level: 'legacy_unsupervised', | |
| identity_verified: true, | |
| account_age_in_days: 0, | |
| account_confirmed_at: user.confirmed_at, | |
| ) | |
| end |
There was a problem hiding this comment.
That makes sense, thank you!
🎫 Ticket
Link to the relevant ticket:
LG-14100
🛠 Summary of changes
Add Identity Verified details to 'Account Reset: delete' event.
📜 Testing Plan
Provide a checklist of steps to confirm the changes.
Reduce wait time for Account Reset: delete by setting in your local application.yml
account_reset_wait_period_days: 0identity_verified:
profile_idv_level: (if applicable)
👀 Screenshots