Skip to content
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

98887 Callback implementation for IVC emails - IVC CHAMPVA forms #20334

Conversation

michaelclement
Copy link
Contributor

@michaelclement michaelclement commented Jan 16, 2025

Summary

This PR adds a custom callback class for use with IVC emails sent through VA Notify. The main purpose of the callback is to improve our Data Dog displays of emails being sent so we can monitor successful/failed deliveries and how many emails are sent per form.

This PR is part of an ongoing effort. To fully test this callback, the callback class needs to be merged into staging. I've added a feature flag so this can be safely tested without impacting production.

Related issue(s)

Testing done

  • New code is covered by unit tests
  • Prior to this callback, there was not an IVC specific log trail for confirmation emails being sent
  • To verify the new monitor method works as intended, the code changes may be pasted into the staging argo terminal and the monitor class instantiated + track_email_sent invoked. However, to verify the VA Notify integration the code has to be fully merged into staging so that whichever node receives the VA Notify callback will have the latest code.

Screenshots

NA

What areas of the site does it impact?

IVC CHAMPVA forms

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

Requested Feedback

NA

@va-vfs-bot va-vfs-bot temporarily deployed to 98887-callback-implementation-to-monitor-confirmation-emails-sent-by-va-notify/main/main January 16, 2025 21:51 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 98887-callback-implementation-to-monitor-confirmation-emails-sent-by-va-notify/main/main January 17, 2025 16:11 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 98887-callback-implementation-to-monitor-confirmation-emails-sent-by-va-notify/main/main January 17, 2025 16:20 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 98887-callback-implementation-to-monitor-confirmation-emails-sent-by-va-notify/main/main January 17, 2025 16:29 Inactive
reiting
reiting previously approved these changes Jan 17, 2025
@michaelclement michaelclement marked this pull request as ready for review January 17, 2025 16:58
@michaelclement michaelclement requested review from a team as code owners January 17, 2025 16:58
@stevenjcumming
Copy link
Contributor

@michaelclement there's a failing test

@michaelclement michaelclement marked this pull request as draft January 17, 2025 18:27
@va-vfs-bot va-vfs-bot temporarily deployed to 98887-callback-implementation-to-monitor-confirmation-emails-sent-by-va-notify/main/main January 17, 2025 18:58 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 98887-callback-implementation-to-monitor-confirmation-emails-sent-by-va-notify/main/main January 17, 2025 20:30 Inactive
@michaelclement michaelclement marked this pull request as ready for review January 21, 2025 14:06
@michaelclement michaelclement merged commit a437280 into master Jan 21, 2025
26 of 27 checks passed
@michaelclement michaelclement deleted the 98887-callback-implementation-to-monitor-confirmation-emails-sent-by-va-notify branch January 21, 2025 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants