-
Notifications
You must be signed in to change notification settings - Fork 128
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
Create classes for tracking Attempts API events #11716
base: main
Are you sure you want to change the base?
Conversation
changelog: Internal, Attempts API, Create class for tracking Attempts API events
app/services/attempts_api/tracker.rb
Outdated
if metadata.has_key?(:failure_reason) && | ||
(metadata[:failure_reason].blank? || | ||
metadata[:success].present?) | ||
metadata.delete(:failure_reason) |
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.
should we do metadata.dup
so we're not mutating an input?
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've replaced metadata.delete
with using metadata.except
which should have the same effect of creating a new hash.
003640a
to
031b693
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.
i don't feel strongly about zach's suggestions but they're worth considering!
de27854
to
5d29b99
Compare
5d29b99
to
3f86703
Compare
🎫 Ticket
https://gitlab.login.gov/lg-teams/Melba/protocols-backlog/-/issues/142
🛠 Summary of changes
A follow up to #11692 to create an interface to begin logging events to the previously implemented storage. This is also heavily based on the 2022 implementation with similar changes to accommodate multiple service providers. This PR creates an initial stub module for the list of events as well.