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

#1770 Send All Email Statuses to VA Profile #1889

Merged
merged 21 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/pull_request_template.md
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a couple links to our PR template for easier referencing of our docs since we were talking about how we might best follow our guidelines in the last process improvement meeting.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding the links

Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ Please provide relevant information and / or links to demonstrate the functional
- [ ] PR has a detailed description, including links to specific documentation
- [ ] I have added the [appropriate labels](https://github.com/department-of-veterans-affairs/notification-api/blob/master/.github/release.yaml) to the PR.
- [ ] I did not remove any parts of the template, such as checkboxes even if they are not used
- [ ] My code follows the style guidelines of this project
- [ ] My code follows the [style guidelines](https://github.com/department-of-veterans-affairs/vanotify-team/blob/master/Process/style_guide.md) of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to any documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] I have added tests that prove my fix is effective or that my feature works. [Testing guidelines](https://github.com/department-of-veterans-affairs/vanotify-team/blob/master/Process/testing_guide.md)
- [ ] I have ensured the latest master is merged into my branch and all checks are green prior to review
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules
Expand Down
2 changes: 2 additions & 0 deletions .talismanrc
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,6 @@ fileignoreconfig:
checksum: 21d6c62402baa8ec3451c2d00cf5ffdff96398344d78603f93e3479e711305fb
- filename: tests/app/notifications/test_send_notifications.py
checksum: 12f223d78afccf1d48b586c53ca061121ad0666aae4c0e464b47e88531bd630e
- filename: tests/app/va/va_profile/test_va_profile_client.py
checksum: faac058194f6d02342f46fdf7551c246caa58ae164cea973ff1f7089fee664fc
version: "1.0"
1 change: 1 addition & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ def create_app(application):
application.config['VA_PROFILE_URL'],
application.config['VANOTIFY_SSL_CERT_PATH'],
application.config['VANOTIFY_SSL_KEY_PATH'],
application.config['VA_PROFILE_TOKEN'],
statsd_client,
)
mpi_client.init_app(
Expand Down
67 changes: 65 additions & 2 deletions app/celery/process_ses_receipts_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@
from sqlalchemy.orm.exc import NoResultFound
import enum
import requests
from app import notify_celery, statsd_client
from app import DATETIME_FORMAT, notify_celery, statsd_client, va_profile_client
from app.celery.exceptions import AutoRetryException
from app.celery.service_callback_tasks import publish_complaint
from app.config import QueueNames
from app.clients.email.aws_ses import get_aws_responses
from app.dao import notifications_dao, services_dao, templates_dao
from app.feature_flags import FeatureFlag, is_feature_enabled
from app.models import (
Notification,
EMAIL_TYPE,
KEY_TYPE_NORMAL,
NOTIFICATION_SENDING,
Expand Down Expand Up @@ -151,7 +154,7 @@ def sns_smtp_callback_handler():

@notify_celery.task(bind=True, name='process-ses-result', max_retries=5, default_retry_delay=300)
@statsd(namespace='tasks')
def process_ses_results( # noqa: C901
def process_ses_results( # noqa: C901 (too complex 14 > 10)
self,
response,
):
Expand Down Expand Up @@ -202,6 +205,9 @@ def process_ses_results( # noqa: C901

notifications_dao.dao_update_notification(notification)
check_and_queue_callback_task(notification)

check_and_queue_va_profile_email_status_callback(notification)

return

# Prevent regressing bounce status
Expand Down Expand Up @@ -251,6 +257,8 @@ def process_ses_results( # noqa: C901

check_and_queue_callback_task(notification)

check_and_queue_va_profile_email_status_callback(notification)

return True

except Retry:
Expand Down Expand Up @@ -342,3 +350,58 @@ def process_ses_smtp_results(
current_app.logger.exception(e)
current_app.logger.error('Error processing SES SMTP results: %s', type(e))
self.retry(queue=QueueNames.RETRY)


def check_and_queue_va_profile_email_status_callback(notification: Notification) -> None:
"""This checks the feature flag is enabled and queues the celery task if it is. Otherwise, it only logs a message."""

if is_feature_enabled(FeatureFlag.VA_PROFILE_EMAIL_STATUS_ENABLED):
send_email_status_to_va_profile.apply_async([notification], queue=QueueNames.CALLBACKS)
else:
current_app.logger.info('Email status not sent to VA Profile, feature flag disabled')


@notify_celery.task(
bind=True,
name='send-email-status-to-va-profile',
throws=(AutoRetryException,),
autoretry_for=(AutoRetryException,),
max_retries=60,
retry_backoff=True,
retry_backoff_max=3600,
)
def send_email_status_to_va_profile(task, notification: Notification) -> None:
"""
This function collects the information from the email notification to send to VA Profile and calls the
VAProfileClient method to send the information to VA Profile.

:param notification: the email notification to collect data from
"""

current_app.logger.debug('Sending email status to VA Profile, collecting data...')
notification_data = {
'id': str(notification.id), # this is the notification id
'reference': notification.client_reference,
'to': notification.to, # this is the recipient's contact info (email)
'status': notification.status, # this will specify the delivery status of the notification
'status_reason': notification.status_reason, # populated if there's additional context on the delivery status
'created_at': notification.created_at.strftime(DATETIME_FORMAT),
'completed_at': notification.updated_at.strftime(DATETIME_FORMAT) if notification.updated_at else None,
'sent_at': notification.sent_at.strftime(DATETIME_FORMAT) if notification.sent_at else None,
'notification_type': notification.notification_type, # this is the channel/type of notification (email)
'provider': notification.sent_by, # email provider
}

try:
va_profile_client.send_va_profile_email_status(notification_data)
except requests.Timeout:
# logging in send_va_profile_email_status
raise AutoRetryException
except requests.exceptions.RequestException:
# logging exception in send_va_profile_email_status
current_app.logger.error(
'Exception caused notification status to NOT be sent to VA Profile for notification %s',
notification_data.get('id'),
)

# the error is being handled by not retrying this celery task
5 changes: 1 addition & 4 deletions app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ class Config(object):
GRANICUS_TOKEN = os.environ.get('GRANICUS_TOKEN', '')
GRANICUS_URL = os.environ.get('GRANICUS_URL', 'https://tms.govdelivery.com')
VA_PROFILE_URL = os.environ.get('VA_PROFILE_URL', 'https://int.vaprofile.va.gov')
VA_PROFILE_TOKEN = os.environ.get('VA_PROFILE_TOKEN', '')
MPI_URL = os.environ.get('MPI_URL', 'https://ps.dev.iam.va.gov')

VA_ONSITE_URL = os.environ.get('VA_ONSITE_URL', 'https://staging-api.va.gov')
Expand Down Expand Up @@ -449,10 +450,6 @@ class Config(object):
SESSION_COOKIE_SECURE = str(True) == os.getenv('SESSION_COOKIE_SECURE', 'False')
SESSION_COOKIE_SAMESITE = 'Lax'

# Feature flags
GOVDELIVERY_EMAIL_CLIENT_ENABLED = True
SWITCH_SLOW_SMS_PROVIDER_ENABLED = False

# Google Analytics
GOOGLE_ANALYTICS_ENABLED = str(True) == (os.getenv('GOOGLE_ANALYTICS_ENABLED', 'False'))
GOOGLE_ANALYTICS_URL = os.getenv('GOOGLE_ANALYTICS_URL', 'https://www.google-analytics.com/collect')
Expand Down
1 change: 1 addition & 0 deletions app/feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class FeatureFlag(Enum):
VA_SSO_ENABLED = 'VA_SSO_ENABLED'
V3_ENABLED = 'V3_ENABLED'
COMP_AND_PEN_MESSAGES_ENABLED = 'COMP_AND_PEN_MESSAGES_ENABLED'
VA_PROFILE_EMAIL_STATUS_ENABLED = 'VA_PROFILE_EMAIL_STATUS_ENABLED'
VA_PROFILE_V3_USE_PROFILE_API_V3 = 'VA_PROFILE_V3_USE_PROFILE_API_V3'
VA_PROFILE_V3_COMBINE_CONTACT_INFO_AND_PERMISSIONS_LOOKUP = (
'VA_PROFILE_V3_COMBINE_CONTACT_INFO_AND_PERMISSIONS_LOOKUP'
Expand Down
42 changes: 41 additions & 1 deletion app/va/va_profile/va_profile_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class PhoneNumberType(Enum):
TEMPORARY = 'TEMPORARY'

@staticmethod
def valid_type_values():
def valid_type_values() -> list[str]:
return [PhoneNumberType.MOBILE.value, PhoneNumberType.HOME.value]


Expand All @@ -40,12 +40,14 @@ def init_app(
va_profile_url,
ssl_cert_path,
ssl_key_path,
va_profile_token,
statsd_client,
):
self.logger = logger
self.va_profile_url = va_profile_url
self.ssl_cert_path = ssl_cert_path
self.ssl_key_path = ssl_key_path
self.va_profile_token = va_profile_token
self.statsd_client = statsd_client

def get_email(
Expand Down Expand Up @@ -264,3 +266,41 @@ def _validate_response(
):
if response.get('messages'):
self._raise_no_contact_info_exception(bio_type, va_profile_id, response.get(self.TX_AUDIT_ID))

def send_va_profile_email_status(self, notification_data: dict) -> None:
"""
This method sends notification status data to VA Profile. This is part of our integration to help VA Profile
provide better service by letting them know which emails are good, and which ones result in bounces.

:param notification_data: the data to include with the POST request to VA Profile

Raises:
requests.Timeout: if the request to VA Profile times out
RequestException: if something unexpected happens when sending the request
"""

headers = {'Authorization': f'Bearer {self.va_profile_token}'}
url = self.va_profile_url + '/contact-information-vanotify/notify/status'

self.logger.debug(
'Sending email status to VA Profile using url: %s | notification: %s', url, notification_data.get('id')
)

# make POST request to VA Profile endpoint for notification statuses
# raise errors if they occur, they will be handled by the calling function
try:
requests.post(url, json=notification_data, headers=headers, timeout=(3.05, 1))
except requests.Timeout:
self.logger.info(
'Request timeout attempting to send email status for notification %s to VA Profile, retrying',
notification_data.get('id'),
)
raise
except requests.exceptions.RequestException as e:
self.logger.exception(
'Unexpected request exception attempting to send email status for notification %s to VA Profile.'
' Exception: %s',
notification_data.get('id'),
e,
)
raise
11 changes: 9 additions & 2 deletions cd/application-deployment/dev/vaec-celery-task-definition.json
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@
"name": "VA_PROFILE_URL",
"value": "https://int.vaprofile.va.gov"
},
{
"name": "VA_PROFILE_EMAIL_STATUS_ENABLED",
"value": "True"
},
{
"name": "VANOTIFY_SSL_CERT_PATH",
"value": "/app/certs/vanotify_ssl_cert.pem"
Expand Down Expand Up @@ -310,6 +314,10 @@
{
"name": "VA_ONSITE_SECRET",
"valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/onsite/notification-priv"
},
{
"name": "VA_PROFILE_TOKEN",
"valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/va-profile/auth-token"
}
],
"command": [
Expand Down Expand Up @@ -438,5 +446,4 @@
"value": "NOTIFY"
}
]
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@
"name": "GA4_URL",
"value": "https://www.google-analytics.com/mp/collect"
},

{
"name": "VA_ONSITE_URL",
"value": "https://staging-api.va.gov"
Expand All @@ -98,6 +97,10 @@
"name": "VA_PROFILE_URL",
"value": "https://qa.vaprofile.va.gov"
},
{
"name": "VA_PROFILE_EMAIL_STATUS_ENABLED",
"value": "False"
},
{
"name": "VANOTIFY_SSL_CERT_PATH",
"value": "/app/certs/vanotify_ssl_cert.pem"
Expand Down Expand Up @@ -263,6 +266,10 @@
{
"name": "VETEXT_PASSWORD",
"valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/vetext/password"
},
{
"name": "VA_PROFILE_TOKEN",
"valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/va-profile/auth-token"
}
],
"command": [
Expand Down
11 changes: 9 additions & 2 deletions cd/application-deployment/prod/vaec-celery-task-definition.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@
"name": "VA_PROFILE_URL",
"value": "https://www.vaprofile.va.gov"
},
{
"name": "VA_PROFILE_EMAIL_STATUS_ENABLED",
"value": "False"
},
{
"name": "VANOTIFY_SSL_CERT_PATH",
"value": "/app/certs/vanotify_ssl_cert.pem"
Expand Down Expand Up @@ -274,6 +278,10 @@
{
"name": "VA_ONSITE_SECRET",
"valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/onsite/notification-priv"
},
{
"name": "VA_PROFILE_TOKEN",
"valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/va-profile/auth-token"
}
],
"command": [
Expand Down Expand Up @@ -402,5 +410,4 @@
"value": "NOTIFY"
}
]
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@
"name": "VA_PROFILE_URL",
"value": "https://qa.vaprofile.va.gov"
},
{
"name": "VA_PROFILE_EMAIL_STATUS_ENABLED",
"value": "False"
},
{
"name": "VANOTIFY_SSL_CERT_PATH",
"value": "/app/certs/vanotify_ssl_cert.pem"
Expand Down Expand Up @@ -290,6 +294,10 @@
{
"name": "VA_ONSITE_SECRET",
"valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/onsite/notification-priv"
},
{
"name": "VA_PROFILE_TOKEN",
"valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/va-profile/auth-token"
}
],
"command": [
Expand Down Expand Up @@ -418,5 +426,4 @@
"value": "NOTIFY"
}
]
}

}
Loading