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

Use pinpoint for designated templates #2152

Merged
merged 43 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
9740520
rough in pinpoint
sastels Apr 5, 2024
064e36d
add pool_id env var
sastels Apr 5, 2024
b9d0d3a
sending with pinpoint pool
sastels Apr 9, 2024
0fa34c1
use pinpoint pool for specified templates
sastels Apr 9, 2024
7e69706
format
sastels Apr 9, 2024
b3d12e5
tweak
sastels Apr 9, 2024
fe263c6
Merge branch 'main' into rough-in-pinpoint
sastels Apr 9, 2024
d18ce27
add configuration set
sastels Apr 11, 2024
fceab9f
add task for processing pinpoint receipts
sastels Apr 12, 2024
b8ceb91
add mocks for pinpoint receipt task testing
sastels Apr 12, 2024
1a6ba9e
rough in test_process_pinpoint_receipts_tasks.py
sastels Apr 12, 2024
6e3d339
make tests pass
sastels Apr 12, 2024
ca2da8c
add explicit return
sastels Apr 12, 2024
7ec198b
Merge branch 'main' into rough-in-pinpoint
sastels Apr 12, 2024
3db48e8
tweak
sastels Apr 18, 2024
d2e8471
tweak
sastels Apr 22, 2024
c8eb6c5
Merge branch 'main' into rough-in-pinpoint
sastels Apr 22, 2024
6c090ce
working now
sastels Apr 22, 2024
3f59f27
add new env vars to .env.examples
sastels Apr 23, 2024
646b86d
Update .env.example
sastels Apr 24, 2024
bde0f68
Update .env.example
sastels Apr 24, 2024
12f539f
Update app/clients/sms/aws_pinpoint.py
sastels Apr 24, 2024
965fbff
Update app/config.py
sastels Apr 24, 2024
26d4419
Update app/delivery/send_to_providers.py
sastels Apr 24, 2024
5710df2
Update app/config.py
sastels Apr 24, 2024
28524bb
Merge branch 'main' into rough-in-pinpoint
sastels Apr 24, 2024
efa0df9
add typing / docstring for determine_pinpoint_status
sastels Apr 24, 2024
d1612c2
wip add pattern matching
sastels Apr 25, 2024
c08cc14
matchy-matchy
sastels Apr 25, 2024
aed401f
Merge branch 'main' into rough-in-pinpoint
sastels May 6, 2024
e205bc1
formatting
sastels May 6, 2024
41e7eaa
add if/else alternative to matching
sastels May 6, 2024
40e4c87
fix case
sastels May 6, 2024
f09cec5
Merge branch 'main' into rough-in-pinpoint
sastels May 6, 2024
fde4812
add return
sastels May 6, 2024
040a361
use new callback.pinpoint metrics
sastels May 6, 2024
da4006f
remove redundant CliendError catching
sastels May 6, 2024
188d0b6
Merge branch 'main' into rough-in-pinpoint
sastels May 6, 2024
b4e2bd8
format
sastels May 6, 2024
afec912
refactor process_pinpoint_results tests
sastels May 7, 2024
337c594
if/else ftw
sastels May 7, 2024
7fb51e1
format
sastels May 7, 2024
7723702
Merge branch 'main' into rough-in-pinpoint
sastels May 8, 2024
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
5 changes: 4 additions & 1 deletion app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
)
from app.clients.salesforce.salesforce_client import SalesforceClient
from app.clients.sms.aws_sns import AwsSnsClient
from app.clients.sms.aws_pinpoint import AwsPinpointClient
from app.dbsetup import RoutingSQLAlchemy
from app.encryption import CryptoSigner
from app.json_provider import NotifyJSONProvider
Expand All @@ -45,6 +46,7 @@
notify_celery = NotifyCelery()
aws_ses_client = AwsSesClient()
aws_sns_client = AwsSnsClient()
aws_pinpoint_client = AwsPinpointClient()
signer_notification = CryptoSigner()
signer_personalisation = CryptoSigner()
signer_complaint = CryptoSigner()
Expand Down Expand Up @@ -107,6 +109,7 @@ def create_app(application, config=None):
statsd_client.init_app(application)
logging.init_app(application, statsd_client)
aws_sns_client.init_app(application, statsd_client=statsd_client)
aws_pinpoint_client.init_app(application, statsd_client=statsd_client)
aws_ses_client.init_app(application.config["AWS_REGION"], statsd_client=statsd_client)
notify_celery.init_app(application)

Expand All @@ -120,7 +123,7 @@ def create_app(application, config=None):

performance_platform_client.init_app(application)
document_download_client.init_app(application)
clients.init_app(sms_clients=[aws_sns_client], email_clients=[aws_ses_client])
clients.init_app(sms_clients=[aws_sns_client, aws_pinpoint_client], email_clients=[aws_ses_client])

if application.config["FF_SALESFORCE_CONTACT"]:
salesforce_client.init_app(application)
Expand Down
92 changes: 92 additions & 0 deletions app/clients/sms/aws_pinpoint.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import boto3
from botocore.exceptions import ClientError
Fixed Show fixed Hide fixed
import phonenumbers
from time import monotonic
from app.clients.sms import SmsClient


class AwsPinpointClient(SmsClient):
'''
AWS Pinpoint SMS client
'''

def init_app(self, current_app, statsd_client, *args, **kwargs):
self._client = boto3.client('pinpoint-sms-voice-v2', region_name="ca-central-1")
super(SmsClient, self).__init__(*args, **kwargs)
Fixed Show fixed Hide fixed
self.current_app = current_app
self.name = 'pinpoint'
self.statsd_client = statsd_client

def get_name(self):
return self.name

def send_sms(self, to, content, reference, multi=True, sender=None):


# The phone number or short code to send the message from. The phone number
# or short code that you specify has to be associated with your Amazon Pinpoint
# account. For best results, specify long codes in E.164 format.
# originationNumber = sender

# The recipient's phone number. For best results, you should specify the
# phone number in E.164 format.
# destinationNumber = "+14255550142"

# The Amazon Pinpoint project/application ID to use when you send this message.
# Make sure that the SMS channel is enabled for the project or application
# that you choose.
applicationId = self.current_app.config['AWS_PINPOINT_APP_ID']
Fixed Show fixed Hide fixed

# The type of SMS message that you want to send. If you plan to send
# time-sensitive content, specify TRANSACTIONAL. If you plan to send
# marketing-related content, specify PROMOTIONAL.
messageType = "TRANSACTIONAL"

# The registered keyword associated with the originating short code.
registeredKeyword = self.current_app.config['AWS_PINPOINT_KEYWORD']
# The sender ID to use when sending the message. Support for sender ID
# varies by country or region. For more information, see
# https://docs.aws.amazon.com/pinpoint/latest/userguide/channels-sms-countries.html
# senderId = "MySenderID"

matched = False

for match in phonenumbers.PhoneNumberMatcher(to, "US"):
Copy link
Member

Choose a reason for hiding this comment

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

The US parameter is to say the phone number has an American phone number format?

Copy link
Collaborator Author

@sastels sastels May 6, 2024

Choose a reason for hiding this comment

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

It's used as the default region, ie if you don't start your number with a region code then we assume it's US / CA.

matched = True
Dismissed Show dismissed Hide dismissed
to = phonenumbers.format_number(match.number, phonenumbers.PhoneNumberFormat.E164)
destinationNumber = to

try:
start_time = monotonic()

# from https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/pinpoint-sms-voice-v2/client/send_text_message.html
response = self._client.send_text_message(
DestinationPhoneNumber=destinationNumber,
OriginationIdentity=self.pool_id,
MessageBody=content,
MessageType=messageType,
Keyword=registeredKeyword
)

# this will be true if the OriginationIdentity does not exist in pinpoint
if response['MessageResponse']['Result'][destinationNumber]['StatusCode'] == 400:
self.statsd_client.incr("clients.pinpoint.error")
raise Exception(response['MessageResponse']['Result'][destinationNumber]['StatusMessage'])
except ClientError as e:
self.statsd_client.incr("clients.pinpoint.error")
raise Exception(e)
except Exception as e:
Copy link
Member

Choose a reason for hiding this comment

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

I think we can combine these two if the logic is exactly the same.

Suggested change
except Exception as e:
except (ClientError, Exception) as e:

There might be syntactic sugar to avoid the tuple and rather use the | operator nowadays, not sure..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤔 a ClientError is an Exception so I think we can just get rid of the ClientError bit alltogether!

self.statsd_client.incr("clients.pinpoint.error")
raise Exception(e)
finally:
elapsed_time = monotonic() - start_time
self.current_app.logger.info("AWS Pinpoint request finished in {}".format(elapsed_time))
self.statsd_client.timing("clients.pinpoint.request-time", elapsed_time)
self.statsd_client.incr("clients.pinpoint.success")

return response['MessageResponse']['Result'][destinationNumber]['MessageId']

if not matched:
self.statsd_client.incr("clients.pinpoint.error")
self.current_app.logger.error("No valid numbers found in {}".format(to))
raise ValueError("No valid numbers found for SMS delivery")
8 changes: 7 additions & 1 deletion app/delivery/send_to_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@
notification.international,
notification.reply_to_text,
)



template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__

Expand Down Expand Up @@ -334,7 +336,7 @@
dao_update_notification(notification)


def provider_to_use(notification_type, notification_id, international=False, sender=None):

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns Note

Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
active_providers_in_order = [
p for p in get_provider_details_by_notification_type(notification_type, international) if p.active
]
Expand All @@ -343,7 +345,11 @@
current_app.logger.error("{} {} failed as no active providers".format(notification_type, notification_id))
raise Exception("No active {} providers".format(notification_type))

return clients.get_client_by_name_and_type(active_providers_in_order[0].identifier, notification_type)
# return clients.get_client_by_name_and_type(active_providers_in_order[0].identifier, notification_type)

if notification_type == SMS_TYPE:
return clients.get_client_by_name_and_type('pinpoint', notification_type)



def get_html_email_options(service: Service):
Expand Down
3 changes: 2 additions & 1 deletion app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1272,9 +1272,10 @@ def get_link(self):


SNS_PROVIDER = "sns"
PINPOINT_PROVIDER = 'pinpoint'
SES_PROVIDER = "ses"

SMS_PROVIDERS = [SNS_PROVIDER]
SMS_PROVIDERS = [SNS_PROVIDER, PINPOINT_PROVIDER]
EMAIL_PROVIDERS = [SES_PROVIDER]
PROVIDERS = SMS_PROVIDERS + EMAIL_PROVIDERS

Expand Down
Loading