mirror of
https://github.com/GSA/notifications-api.git
synced 2025-12-21 07:51:13 -05:00
Simplify getting name of email / sms providers
Previously we used a combination of "provider.name" and "get_name()" which was confusing. Using a non-property function also gave me the impression that the name was more dynamic than it actually is.
This commit is contained in:
@@ -28,5 +28,6 @@ class EmailClient(Client):
|
|||||||
def send_email(self, *args, **kwargs):
|
def send_email(self, *args, **kwargs):
|
||||||
raise NotImplementedError('TODO Need to implement.')
|
raise NotImplementedError('TODO Need to implement.')
|
||||||
|
|
||||||
def get_name(self):
|
@property
|
||||||
|
def name(self):
|
||||||
raise NotImplementedError('TODO Need to implement.')
|
raise NotImplementedError('TODO Need to implement.')
|
||||||
|
|||||||
@@ -59,11 +59,11 @@ class AwsSesClient(EmailClient):
|
|||||||
def init_app(self, region, statsd_client, *args, **kwargs):
|
def init_app(self, region, statsd_client, *args, **kwargs):
|
||||||
self._client = boto3.client('ses', region_name=region)
|
self._client = boto3.client('ses', region_name=region)
|
||||||
super(AwsSesClient, self).__init__(*args, **kwargs)
|
super(AwsSesClient, self).__init__(*args, **kwargs)
|
||||||
self.name = 'ses'
|
|
||||||
self.statsd_client = statsd_client
|
self.statsd_client = statsd_client
|
||||||
|
|
||||||
def get_name(self):
|
@property
|
||||||
return self.name
|
def name(self):
|
||||||
|
return 'ses'
|
||||||
|
|
||||||
def send_email(self,
|
def send_email(self,
|
||||||
source,
|
source,
|
||||||
|
|||||||
@@ -13,12 +13,12 @@ class AwsSesStubClientException(EmailClientException):
|
|||||||
|
|
||||||
class AwsSesStubClient(EmailClient):
|
class AwsSesStubClient(EmailClient):
|
||||||
def init_app(self, region, statsd_client, stub_url):
|
def init_app(self, region, statsd_client, stub_url):
|
||||||
self.name = 'ses'
|
|
||||||
self.statsd_client = statsd_client
|
self.statsd_client = statsd_client
|
||||||
self.url = stub_url
|
self.url = stub_url
|
||||||
|
|
||||||
def get_name(self):
|
@property
|
||||||
return self.name
|
def name(self):
|
||||||
|
return 'ses'
|
||||||
|
|
||||||
def send_email(self,
|
def send_email(self,
|
||||||
source,
|
source,
|
||||||
|
|||||||
@@ -21,5 +21,6 @@ class SmsClient(Client):
|
|||||||
def send_sms(self, *args, **kwargs):
|
def send_sms(self, *args, **kwargs):
|
||||||
raise NotImplementedError('TODO Need to implement.')
|
raise NotImplementedError('TODO Need to implement.')
|
||||||
|
|
||||||
def get_name(self):
|
@property
|
||||||
|
def name(self):
|
||||||
raise NotImplementedError('TODO Need to implement.')
|
raise NotImplementedError('TODO Need to implement.')
|
||||||
|
|||||||
@@ -68,12 +68,12 @@ class FiretextClient(SmsClient):
|
|||||||
self.api_key = current_app.config.get('FIRETEXT_API_KEY')
|
self.api_key = current_app.config.get('FIRETEXT_API_KEY')
|
||||||
self.international_api_key = current_app.config.get('FIRETEXT_INTERNATIONAL_API_KEY')
|
self.international_api_key = current_app.config.get('FIRETEXT_INTERNATIONAL_API_KEY')
|
||||||
self.from_number = current_app.config.get('FROM_NUMBER')
|
self.from_number = current_app.config.get('FROM_NUMBER')
|
||||||
self.name = 'firetext'
|
|
||||||
self.url = current_app.config.get('FIRETEXT_URL')
|
self.url = current_app.config.get('FIRETEXT_URL')
|
||||||
self.statsd_client = statsd_client
|
self.statsd_client = statsd_client
|
||||||
|
|
||||||
def get_name(self):
|
@property
|
||||||
return self.name
|
def name(self):
|
||||||
|
return 'firetext'
|
||||||
|
|
||||||
def record_outcome(self, success, response):
|
def record_outcome(self, success, response):
|
||||||
status_code = response.status_code if response else 503
|
status_code = response.status_code if response else 503
|
||||||
|
|||||||
@@ -74,7 +74,6 @@ class MMGClient(SmsClient):
|
|||||||
self.current_app = current_app
|
self.current_app = current_app
|
||||||
self.api_key = current_app.config.get('MMG_API_KEY')
|
self.api_key = current_app.config.get('MMG_API_KEY')
|
||||||
self.from_number = current_app.config.get('FROM_NUMBER')
|
self.from_number = current_app.config.get('FROM_NUMBER')
|
||||||
self.name = 'mmg'
|
|
||||||
self.statsd_client = statsd_client
|
self.statsd_client = statsd_client
|
||||||
self.mmg_url = current_app.config.get('MMG_URL')
|
self.mmg_url = current_app.config.get('MMG_URL')
|
||||||
|
|
||||||
@@ -94,8 +93,9 @@ class MMGClient(SmsClient):
|
|||||||
self.statsd_client.incr("clients.mmg.error")
|
self.statsd_client.incr("clients.mmg.error")
|
||||||
self.current_app.logger.warning(log_message)
|
self.current_app.logger.warning(log_message)
|
||||||
|
|
||||||
def get_name(self):
|
@property
|
||||||
return self.name
|
def name(self):
|
||||||
|
return 'mmg'
|
||||||
|
|
||||||
def send_sms(self, to, content, reference, international, multi=True, sender=None):
|
def send_sms(self, to, content, reference, international, multi=True, sender=None):
|
||||||
data = {
|
data = {
|
||||||
|
|||||||
@@ -60,7 +60,7 @@ def send_sms_to_provider(notification):
|
|||||||
key_type = notification.key_type
|
key_type = notification.key_type
|
||||||
if service.research_mode or notification.key_type == KEY_TYPE_TEST:
|
if service.research_mode or notification.key_type == KEY_TYPE_TEST:
|
||||||
update_notification_to_sending(notification, provider)
|
update_notification_to_sending(notification, provider)
|
||||||
send_sms_response(provider.get_name(), str(notification.id), notification.to)
|
send_sms_response(provider.name, str(notification.id), notification.to)
|
||||||
|
|
||||||
else:
|
else:
|
||||||
try:
|
try:
|
||||||
@@ -82,7 +82,7 @@ def send_sms_to_provider(notification):
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
notification.billable_units = template.fragment_count
|
notification.billable_units = template.fragment_count
|
||||||
dao_update_notification(notification)
|
dao_update_notification(notification)
|
||||||
dao_reduce_sms_provider_priority(provider.get_name(), time_threshold=timedelta(minutes=1))
|
dao_reduce_sms_provider_priority(provider.name, time_threshold=timedelta(minutes=1))
|
||||||
raise e
|
raise e
|
||||||
else:
|
else:
|
||||||
notification.billable_units = template.fragment_count
|
notification.billable_units = template.fragment_count
|
||||||
@@ -158,7 +158,7 @@ def send_email_to_provider(notification):
|
|||||||
|
|
||||||
def update_notification_to_sending(notification, provider):
|
def update_notification_to_sending(notification, provider):
|
||||||
notification.sent_at = datetime.utcnow()
|
notification.sent_at = datetime.utcnow()
|
||||||
notification.sent_by = provider.get_name()
|
notification.sent_by = provider.name
|
||||||
if notification.status not in NOTIFICATION_STATUS_TYPES_COMPLETED:
|
if notification.status not in NOTIFICATION_STATUS_TYPES_COMPLETED:
|
||||||
notification.status = NOTIFICATION_SENT if notification.international else NOTIFICATION_SENDING
|
notification.status = NOTIFICATION_SENT if notification.international else NOTIFICATION_SENDING
|
||||||
dao_update_notification(notification)
|
dao_update_notification(notification)
|
||||||
|
|||||||
@@ -54,7 +54,7 @@ def test_provider_to_use_should_return_random_provider(mocker, notify_db_session
|
|||||||
ret = send_to_providers.provider_to_use('sms', international=False)
|
ret = send_to_providers.provider_to_use('sms', international=False)
|
||||||
|
|
||||||
mock_choices.assert_called_once_with([mmg, firetext], weights=[25, 75])
|
mock_choices.assert_called_once_with([mmg, firetext], weights=[25, 75])
|
||||||
assert ret.get_name() == 'mmg'
|
assert ret.name == 'mmg'
|
||||||
|
|
||||||
|
|
||||||
def test_provider_to_use_should_cache_repeated_calls(mocker, notify_db_session):
|
def test_provider_to_use_should_cache_repeated_calls(mocker, notify_db_session):
|
||||||
@@ -89,7 +89,7 @@ def test_provider_to_use_should_only_return_mmg_for_international(
|
|||||||
ret = send_to_providers.provider_to_use('sms', international=True)
|
ret = send_to_providers.provider_to_use('sms', international=True)
|
||||||
|
|
||||||
mock_choices.assert_called_once_with([mmg], weights=[100])
|
mock_choices.assert_called_once_with([mmg], weights=[100])
|
||||||
assert ret.get_name() == 'mmg'
|
assert ret.name == 'mmg'
|
||||||
|
|
||||||
|
|
||||||
def test_provider_to_use_should_only_return_active_providers(mocker, restore_provider_details):
|
def test_provider_to_use_should_only_return_active_providers(mocker, restore_provider_details):
|
||||||
@@ -101,7 +101,7 @@ def test_provider_to_use_should_only_return_active_providers(mocker, restore_pro
|
|||||||
ret = send_to_providers.provider_to_use('sms')
|
ret = send_to_providers.provider_to_use('sms')
|
||||||
|
|
||||||
mock_choices.assert_called_once_with([firetext], weights=[100])
|
mock_choices.assert_called_once_with([firetext], weights=[100])
|
||||||
assert ret.get_name() == 'firetext'
|
assert ret.name == 'firetext'
|
||||||
|
|
||||||
|
|
||||||
def test_provider_to_use_raises_if_no_active_providers(mocker, restore_provider_details):
|
def test_provider_to_use_raises_if_no_active_providers(mocker, restore_provider_details):
|
||||||
|
|||||||
Reference in New Issue
Block a user