LG-7663 email analytics for success and failure emails#7156
LG-7663 email analytics for success and failure emails#7156svalexander merged 10 commits intomainfrom
Conversation
svalexander
changed the title
| def email_analytics_attributes(enrollment, delay_time) | ||
| { | ||
| timestamp: Time.zone.now, | ||
| user_id: enrollment.user_id, | ||
| service_provider: enrollment.issuer, | ||
| delay_time_seconds: delay_time, |
There was a problem hiding this comment.
Since the delivery parameter method is in-scope from this method, what would you think about calling it directly rather than expecting it to be passed as a parameter, optionally refactoring out a method to just get the delay value?
| def email_analytics_attributes(enrollment, delay_time) | |
| { | |
| timestamp: Time.zone.now, | |
| user_id: enrollment.user_id, | |
| service_provider: enrollment.issuer, | |
| delay_time_seconds: delay_time, | |
| def email_analytics_attributes(enrollment) | |
| { | |
| timestamp: Time.zone.now, | |
| user_id: enrollment.user_id, | |
| service_provider: enrollment.issuer, | |
| delay_time_seconds: mail_delivery_delay_hours.in_seconds, |
app/services/analytics_events.rb
Outdated
| # Tracks emails that are initiated during GetUspsProofingResultsJob | ||
| # @param [String] email_version success, failed or failed fraud | ||
| def idv_in_person_usps_proofing_results_job_email_initiated( | ||
| email_version:, |
There was a problem hiding this comment.
Unsure if it was prescribed in the ticket, but "version" here had me thinking something incremented like "v1", "v2". What would you think about a "email name", "email type", "email variant", or "email template" ?
There was a problem hiding this comment.
good point, i think email type is clearer than version
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
Couple comments, but LGTM 👍
| context 'email_analytics_attributes' do | ||
| before(:each) do | ||
| stub_request_passed_proofing_results | ||
| end | ||
| it 'logs message with email analytics attributes' do | ||
| freeze_time do | ||
| job.perform(Time.zone.now) | ||
| expect(job_analytics).to have_logged_event( | ||
| 'GetUspsProofingResultsJob: Success or failure email initiated', | ||
| timestamp: Time.zone.now, | ||
| user_id: pending_enrollment.user_id, | ||
| service_provider: pending_enrollment.issuer, | ||
| delay_time_seconds: 3600, | ||
| ) | ||
| end | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Personally email_analytics_attributes seems to be a good candidate for a private method, since we're primarily concerned with each of the specific email types, and the helper method is an implementation detail that we could choose to refactor or remove altogether. As a private method, I don't think we'd need to test it at all in isolation like this, but I'd suggest enhancing the assertions below for the specific email types to ensure they include all the details we're looking for.
There was a problem hiding this comment.
i separated out the email attributes from the earlier test for job analytics so I could use a completed enrollment rather then the pending enrollment used for the test for the other job analytics.
| send_failed_fraud_email(enrollment.user, enrollment) | ||
| analytics(user: enrollment.user).idv_in_person_usps_proofing_results_job_email_initiated( | ||
| **email_analytics_attributes(enrollment), | ||
| email_type: 'Failed fraud suspected email type', |
There was a problem hiding this comment.
The trailing " email type" in these strings feels unnecessary since it's already part of the property name.
| email_type: 'Failed fraud suspected email type', | |
| email_type: 'Failed fraud suspected', |
svalexander
changed the title
2 participants
🎫 Ticket
Lg-7663
🛠 Summary of changes
This pr is to add analytics to Cloudwatch around the success and fail emails in the GetUspsProofingResultsJob. Because we are using a service to send the emails we cannot know for sure that they are sent, but we can know when we've initiated/queued the job. When we handle a failed or successful status we call the functions that send the appropriate emails, this pr adds an analytic's event for emails when we handle those statuses. The analytics event is called idv_in_person_usps_proofing_results_job_email_initiated and it accepts email_analytics_attributes. These attributes include:
timestamp: time email was initiated,
user_id: user id attached to the enrollment,
service_provider: found in enrollment's issuer field,
delay_time_seconds: wait time for email to be sent,
📜 Testing Plan
Provide a checklist of steps to confirm the changes.
UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_results_response
👀 Screenshots
If relevant, include a screenshot or screen capture of the changes.
Success email analytics:
Failed fraud suspected email analytics:
Failed email analytics: