-
Notifications
You must be signed in to change notification settings - Fork 115
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
Document analytics properties exempted in controller specs #11220
Conversation
changelog: Internal, Analytics, Document analytics properties
@@ -2652,8 +2777,40 @@ def idv_in_person_proofing_redo_state_id_submitted( | |||
) | |||
end | |||
|
|||
def idv_in_person_proofing_residential_address_submitted(**extra) | |||
track_event('IdV: in person proofing residential address submitted', **extra) | |||
# @param [Boolean] success Whether form validation was successful |
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.
NIt- I did not look at all events but it would be nice if the order matched ie: 3 arg in notes is error_details but is flow_path in args below. Update- maybe lots of additional work and not caused by your changes so ignore if you'd like
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.
I tried to add some consistency here while balancing diff size. I think we'll want to make a pass at standardizing order & descriptions once we've got everything fully documented
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.
Make sense to me- resolving
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, thank you for your service
🛠 Summary of changes
Updates controller specs to remove blanket exemptions for
allowed_extra_analytics
, and adds documentation where existing errors occur in doing so.Most of these are in IdV/IPP.
Why?
allowed_extra_analytics
📜 Testing Plan
Verify build passes.
Double-check that required keyword arguments are always passed.