diff --git a/app/dao/service_sms_sender_dao.py b/app/dao/service_sms_sender_dao.py
index 3cba5cc12b..6054d93e31 100644
--- a/app/dao/service_sms_sender_dao.py
+++ b/app/dao/service_sms_sender_dao.py
@@ -1,14 +1,18 @@
+from typing import Optional
+from uuid import UUID
+
+from sqlalchemy import desc, select, update
+
from app import db
from app.dao.dao_utils import transactional
from app.exceptions import ArchiveValidationError
-from app.models import ServiceSmsSender, InboundNumber
+from app.models import ProviderDetails, ServiceSmsSender, InboundNumber
from app.service.exceptions import (
SmsSenderDefaultValidationException,
+ SmsSenderProviderValidationException,
SmsSenderInboundNumberIntegrityException,
SmsSenderRateLimitIntegrityException,
)
-from sqlalchemy import desc, select, update
-from typing import Optional
def insert_service_sms_sender(
@@ -66,11 +70,13 @@ def dao_add_sms_sender_for_service(
service_id,
sms_sender,
is_default,
+ provider_id,
+ description,
inbound_number_id=None,
rate_limit=None,
rate_limit_interval=None,
sms_sender_specifics={},
-):
+) -> ServiceSmsSender:
default_sms_sender = _get_default_sms_sender_for_service(service_id=service_id)
if not default_sms_sender and not is_default:
@@ -79,17 +85,7 @@ def dao_add_sms_sender_for_service(
if is_default:
_set_default_sms_sender_to_not_default(default_sms_sender)
- if rate_limit is not None and rate_limit < 1:
- raise SmsSenderRateLimitIntegrityException('rate_limit cannot be less than 1.')
-
- if rate_limit_interval is not None and rate_limit_interval < 1:
- raise SmsSenderRateLimitIntegrityException('rate_limit_interval cannot be less than 1.')
-
- # TODO - Refactor validation after merging inbound number & sms_sender
- if (rate_limit is not None and rate_limit_interval is None) or (
- rate_limit_interval is not None and rate_limit is None
- ):
- raise SmsSenderRateLimitIntegrityException('Provide both rate_limit and rate_limit_interval.')
+ _validate_rate_limit(None, rate_limit, rate_limit_interval)
if inbound_number_id is not None:
inbound_number = _allocate_inbound_number_for_service(service_id, inbound_number_id)
@@ -100,13 +96,18 @@ def dao_add_sms_sender_for_service(
f"and the Inbound Number '{inbound_number.id}' ('{inbound_number.number}')."
)
+ provider_details = _validate_provider(provider_id)
+
new_sms_sender = ServiceSmsSender(
- service_id=service_id,
- sms_sender=sms_sender,
+ description=description,
is_default=is_default,
inbound_number_id=inbound_number_id,
+ provider=provider_details,
+ provider_id=provider_id,
rate_limit=rate_limit,
rate_limit_interval=rate_limit_interval,
+ service_id=service_id,
+ sms_sender=sms_sender,
sms_sender_specifics=sms_sender_specifics,
)
@@ -114,57 +115,54 @@ def dao_add_sms_sender_for_service(
return new_sms_sender
+def _validate_provider(provider_id: UUID) -> ProviderDetails:
+ """Validate the provider_details. This is a helper function when adding or updating an SMS sender.
+ It checks the provider exists and raises an Exception if it doesn't.
+
+ Args:
+ provider_id (UUID): The ID of the provider to validate.
+
+ Returns:
+ ProviderDetails: The provider details.
+
+ Raises:
+ SmsSenderProviderValidationException: If the provider doesn't exist.
+ """
+ # this causes a circular import, so it's being imported here...
+ from app.dao.provider_details_dao import get_provider_details_by_id
+
+ provider_details = get_provider_details_by_id(provider_id)
+
+ if provider_details is None:
+ raise SmsSenderProviderValidationException(f'No provider details found for id {provider_id}')
+
+ return provider_details
+
+
@transactional
-def dao_update_service_sms_sender( # noqa: C901
+def dao_update_service_sms_sender(
service_id,
service_sms_sender_id,
**kwargs,
-):
+) -> ServiceSmsSender:
if 'is_default' in kwargs:
- default_sms_sender = _get_default_sms_sender_for_service(service_id)
- is_default = kwargs['is_default']
-
- if service_sms_sender_id == default_sms_sender.id and not is_default:
- raise SmsSenderDefaultValidationException('You must have at least one SMS sender as the default.')
-
- if is_default:
- _set_default_sms_sender_to_not_default(default_sms_sender)
+ _handle_default_sms_sender(service_id, service_sms_sender_id, kwargs['is_default'])
if 'inbound_number_id' in kwargs:
_allocate_inbound_number_for_service(service_id, kwargs['inbound_number_id'])
- sms_sender_to_update = db.session.get(ServiceSmsSender, service_sms_sender_id)
-
- if 'rate_limit' in kwargs and kwargs['rate_limit'] is not None and kwargs['rate_limit'] < 1:
- raise SmsSenderRateLimitIntegrityException('rate_limit cannot be less than 1.')
- if 'rate_limit' in kwargs and kwargs['rate_limit_interval'] is not None and kwargs['rate_limit_interval'] < 1:
- raise SmsSenderRateLimitIntegrityException('rate_limit_interval cannot be less than 1.')
-
- if (
- 'rate_limit' in kwargs
- and kwargs['rate_limit']
- and ('rate_limit_interval' not in kwargs or not kwargs['rate_limit_interval'])
- ):
- if not sms_sender_to_update.rate_limit_interval:
- raise SmsSenderRateLimitIntegrityException(
- 'Cannot update sender to have only one of rate limit value and interval.'
- )
+ sms_sender_to_update: ServiceSmsSender = db.session.get(ServiceSmsSender, service_sms_sender_id)
- if (
- 'rate_limit_interval' in kwargs
- and kwargs['rate_limit_interval']
- and ('rate_limit' not in kwargs or not kwargs['rate_limit'])
- ):
- if not sms_sender_to_update.rate_limit:
- raise SmsSenderRateLimitIntegrityException(
- 'Cannot update sender to have only one of rate limit value and interval.'
- )
+ _validate_rate_limit(sms_sender_to_update, kwargs.get('rate_limit'), kwargs.get('rate_limit_interval'))
if 'sms_sender' in kwargs and sms_sender_to_update.inbound_number_id:
raise SmsSenderInboundNumberIntegrityException(
'You cannot update the number for this SMS sender because it has an associated Inbound Number.'
)
+ if 'provider_id' in kwargs:
+ _validate_provider(kwargs['provider_id'])
+
for key, value in kwargs.items():
setattr(sms_sender_to_update, key, value)
@@ -172,6 +170,76 @@ def dao_update_service_sms_sender( # noqa: C901
return sms_sender_to_update
+def _handle_default_sms_sender(service_id: UUID, service_sms_sender_id: UUID, is_default: bool) -> None:
+ """Check the default SMS sender.
+ This is a helper function when updating an SMS sender. It ensures there is a default SMS sender for the service and
+ raises an exception if there won't be a default sender after the update.
+
+ Args:
+ service_id (UUID): The ID of the service.
+ service_sms_sender_id (UUID): The ID of the SMS sender.
+ is_default (bool): Whether the SMS sender should be updated to be the default.
+
+ Raises:
+ SmsSenderDefaultValidationException: If there is no default SMS sender for the service.
+
+ """
+ default_sms_sender = _get_default_sms_sender_for_service(service_id)
+
+ # ensure there will still be a default sender on the service, else raise an exception
+ if service_sms_sender_id == default_sms_sender.id and not is_default:
+ raise SmsSenderDefaultValidationException('You must have at least one SMS sender as the default.')
+
+ if is_default:
+ _set_default_sms_sender_to_not_default(default_sms_sender)
+
+
+def _validate_rate_limit(
+ sms_sender_to_update: ServiceSmsSender | None,
+ rate_limit: int | None,
+ rate_limit_interval: int | None,
+) -> None:
+ """Validate the rate limit and rate limit interval.
+ This is a helper function when adding or updating a SMS sender.
+ It ensures the rate limit and rate limit interval are valid.
+
+ Args:
+ sms_sender_to_update (ServiceSmsSender | None): The SMS sender to update, or None if adding a new SMS sender.
+ rate_limit (int | None): The sms sender's rate limit.
+ rate_limit_interval (int | None): The sms sender's rate limit interval.
+
+ Raises:
+ SmsSenderRateLimitIntegrityException: If the rate limit or rate limit interval is invalid.
+ """
+ # ensure rate_limit is a positive integer, when included in kwargs
+ if rate_limit is not None and rate_limit < 1:
+ raise SmsSenderRateLimitIntegrityException('rate_limit cannot be less than 1.')
+
+ # ensure rate_limit_interval is a positive integer, when included in kwargs
+ if rate_limit_interval is not None and rate_limit_interval < 1:
+ raise SmsSenderRateLimitIntegrityException('rate_limit_interval cannot be less than 1.')
+
+ # only run these checks when updating an existing SMS sender
+ if sms_sender_to_update:
+ # if rate limit is being updated, ensure rate limit interval is also being updated, or is already valid
+ if rate_limit and not rate_limit_interval:
+ if not sms_sender_to_update.rate_limit_interval:
+ raise SmsSenderRateLimitIntegrityException(
+ 'Cannot update sender to have only one of rate limit value and interval.'
+ )
+
+ # if rate limit interval is being updated, ensure rate limit is also being updated, or is already valid
+ if not rate_limit and rate_limit_interval:
+ if not sms_sender_to_update.rate_limit:
+ raise SmsSenderRateLimitIntegrityException(
+ 'Cannot update sender to have only one of rate limit value and interval.'
+ )
+ else:
+ # when adding a new sender ensure both rate limit and rate limit interval are provided, or neither
+ if (rate_limit is None) != (rate_limit_interval is None):
+ raise SmsSenderRateLimitIntegrityException('Provide both rate_limit and rate_limit_interval, or neither.')
+
+
@transactional
def archive_sms_sender(
service_id,
diff --git a/app/models.py b/app/models.py
index 121481fef5..0ee53de0ec 100644
--- a/app/models.py
+++ b/app/models.py
@@ -549,7 +549,7 @@ class ServiceSmsSender(db.Model):
def get_reply_to_text(self):
return try_validate_and_format_phone_number(self.sms_sender)
- def serialize(self):
+ def serialize(self) -> dict[str, bool | int | str | None]:
return {
'archived': self.archived,
'created_at': self.created_at.strftime(DATETIME_FORMAT),
@@ -557,7 +557,8 @@ def serialize(self):
'id': str(self.id),
'inbound_number_id': str(self.inbound_number_id) if self.inbound_number_id else None,
'is_default': self.is_default,
- 'provider_id': str(self.provider_id),
+ 'provider_id': str(self.provider_id) if self.provider_id else None,
+ 'provider_name': self.provider.display_name if self.provider else None,
'rate_limit': self.rate_limit if self.rate_limit else None,
'rate_limit_interval': self.rate_limit_interval if self.rate_limit_interval else None,
'service_id': str(self.service_id),
diff --git a/app/service/exceptions.py b/app/service/exceptions.py
index 163b714528..cd099491e0 100644
--- a/app/service/exceptions.py
+++ b/app/service/exceptions.py
@@ -6,5 +6,9 @@ class SmsSenderInboundNumberIntegrityException(Exception):
pass
+class SmsSenderProviderValidationException(Exception):
+ pass
+
+
class SmsSenderRateLimitIntegrityException(Exception):
pass
diff --git a/app/service/service_senders_schema.py b/app/service/service_senders_schema.py
index 0a641ee6a5..01aaafe5d5 100644
--- a/app/service/service_senders_schema.py
+++ b/app/service/service_senders_schema.py
@@ -19,36 +19,31 @@
'required': ['contact_block', 'is_default'],
}
+service_sms_sender_request_properties = {
+ 'description': {'type': 'string'},
+ 'inbound_number_id': uuid,
+ 'is_default': {'type': 'boolean'},
+ 'provider_id': uuid,
+ 'rate_limit': {'type': ['integer', 'null'], 'minimum': 1},
+ 'rate_limit_interval': {'type': ['integer', 'null'], 'minimum': 1},
+ 'sms_sender': {'type': 'string'},
+ 'sms_sender_specifics': {'type': ['object', 'null']},
+}
add_service_sms_sender_request = {
'$schema': 'http://json-schema.org/draft-04/schema#',
'description': 'POST add service SMS sender',
'type': 'object',
'title': 'Add new SMS sender for service',
- 'properties': {
- 'inbound_number_id': uuid,
- 'is_default': {'type': 'boolean'},
- 'rate_limit': {'type': ['integer', 'null'], 'minimum': 1},
- 'rate_limit_interval': {'type': ['integer', 'null'], 'minimum': 1},
- 'sms_sender': {'type': 'string'},
- 'sms_sender_specifics': {'type': ['object', 'null']},
- },
- 'required': ['is_default', 'sms_sender'],
+ 'properties': service_sms_sender_request_properties,
+ 'required': ['description', 'is_default', 'provider_id', 'sms_sender'],
}
-
update_service_sms_sender_request = {
'$schema': 'http://json-schema.org/draft-04/schema#',
'description': 'POST update service SMS sender',
'type': 'object',
'title': 'Update SMS sender for service',
- 'properties': {
- 'inbound_number_id': uuid,
- 'is_default': {'type': 'boolean'},
- 'rate_limit': {'type': ['integer', 'null'], 'minimum': 1},
- 'rate_limit_interval': {'type': ['integer', 'null'], 'minimum': 1},
- 'sms_sender': {'type': 'string'},
- 'sms_sender_specifics': {'type': ['object', 'null']},
- },
+ 'properties': service_sms_sender_request_properties,
'minProperties': 1,
}
diff --git a/app/service/sms_sender_rest.py b/app/service/sms_sender_rest.py
index f4f919e74f..20949777ca 100644
--- a/app/service/sms_sender_rest.py
+++ b/app/service/sms_sender_rest.py
@@ -12,6 +12,7 @@
from app.service.exceptions import (
SmsSenderDefaultValidationException,
SmsSenderInboundNumberIntegrityException,
+ SmsSenderProviderValidationException,
SmsSenderRateLimitIntegrityException,
)
from app.service.service_senders_schema import (
@@ -35,6 +36,7 @@ def _validate_service_exists():
@service_sms_sender_blueprint.errorhandler(SmsSenderRateLimitIntegrityException)
@service_sms_sender_blueprint.errorhandler(SmsSenderDefaultValidationException)
@service_sms_sender_blueprint.errorhandler(SmsSenderInboundNumberIntegrityException)
+@service_sms_sender_blueprint.errorhandler(SmsSenderProviderValidationException)
def handle_errors(error):
current_app.logger.info(error)
return jsonify(result='error', message=str(error)), 400
diff --git a/documents/openapi/openapi.yaml b/documents/openapi/openapi.yaml
index 12d8f7915c..82cb3a330a 100644
--- a/documents/openapi/openapi.yaml
+++ b/documents/openapi/openapi.yaml
@@ -1,7 +1,7 @@
openapi: 3.0.0
info:
title: VA Notify API Documentation
- version: 1.0.8
+ version: 1.0.9
description: |
This documents the API schemas for consumption by internal VA developers.
Authorization header
@@ -252,7 +252,7 @@ paths:
type: string
mobile_number:
type: string
- example: "+19876543210"
+ example: '+12025551212'
auth_type:
type: string
enum:
@@ -421,11 +421,15 @@ paths:
schema:
type: object
properties:
+ description:
+ type: string
inbound_number_id:
$ref: '#/components/schemas/Id'
is_default:
type: boolean
example: false
+ provider_id:
+ $ref: '#/components/schemas/Id'
rate_limit:
type: integer
minimum: 1
@@ -434,13 +438,16 @@ paths:
minimum: 1
sms_sender:
type: string
+ example: '+12025551212'
sms_sender_specifics:
description: Arbitrary JSON relevant to a specific messaging service provider
type: object
example: '{"messaging_service_sid": "MG0000000000000000000000"}'
required:
- - sms_sender
+ - description
- is_default
+ - provider_id
+ - sms_sender
responses:
'201':
description: OK
@@ -491,11 +498,15 @@ paths:
schema:
type: object
properties:
+ description:
+ type: string
inbound_number_id:
$ref: '#/components/schemas/Id'
is_default:
type: boolean
example: false
+ provider_id:
+ $ref: '#/components/schemas/Id'
rate_limit:
type: integer
minimum: 1
@@ -1248,7 +1259,7 @@ paths:
type: string
from_number:
type: string
- example: "+19876543210"
+ example: '+12025551212'
'400':
$ref: '#/components/responses/BadRequest'
'401':
@@ -1496,7 +1507,7 @@ paths:
scheduled_for: "2023-09-06T19:55:23.592973+00:00"
billing_code: string
sms_sender_id: 3fa85f64-5717-4562-b3fc-2c963f66afa6
- phone_number: "+19876543210"
+ phone_number: '+12025551212'
recipient_identifier:
value:
reference: string
@@ -1522,7 +1533,7 @@ paths:
scheduled_for: "2023-09-06T19:55:23.592973+00:00"
billing_code: string
sms_sender_id: 3fa85f64-5717-4562-b3fc-2c963f66afa6
- phone_number: "+19876543210"
+ phone_number: '+12025551212'
recipient_identifier:
id_type: ICN
id_value: 1000000000V000000
@@ -1578,7 +1589,7 @@ paths:
value:
active: true
auth_parameter: "auth-param-name"
- number: "+19876543210"
+ number: '+12025551212'
provider: "ExampleProvider"
self_managed: true
url_endpoint: "http://example.com/api"
@@ -2053,7 +2064,7 @@ components:
mobile_number:
type: string
description: Required if auth_type is sms_auth
- example: "+19876543210"
+ example: '+12025551212'
state:
type: string
user_permissions:
@@ -2197,7 +2208,7 @@ components:
type: string
mobile_number:
type: string
- example: "+19876543210"
+ example: '+12025551212'
name:
type: string
organisations:
@@ -2569,7 +2580,7 @@ components:
- properties:
phone_number:
type: string
- example: "+19876543210"
+ example: '+12025551212'
required: [phone_number]
- properties:
recipient_identifier:
@@ -2993,6 +3004,8 @@ components:
type: boolean
provider_id:
$ref: '#/components/schemas/Id'
+ provider_name:
+ type: string
rate_limit:
type: integer
rate_limit_interval:
diff --git a/documents/postman/internal_api_developers/notification-api.postman_collection.json b/documents/postman/internal_api_developers/notification-api.postman_collection.json
index a1de968670..5a9dc75842 100644
--- a/documents/postman/internal_api_developers/notification-api.postman_collection.json
+++ b/documents/postman/internal_api_developers/notification-api.postman_collection.json
@@ -3070,7 +3070,7 @@
],
"body": {
"mode": "raw",
- "raw": "{\r\n \"sms_sender\": \"+18334981539\",\r\n \"is_default\": false,\r\n \"rate_limit\": null\r\n}"
+ "raw": "{\r\n \"description\": \"used for xyz\",\r\n \"provider_id\": \"uuid_of_provider\",\r\n \"sms_sender\": \"+12025551212\",\r\n \"is_default\": false\r\n}"
},
"url": {
"raw": "{{notification-api-url}}/service/{{service-id}}/sms-sender",
@@ -3272,7 +3272,7 @@
],
"body": {
"mode": "raw",
- "raw": "{\r\n \"sms_sender\": \"+18334981539\",\r\n \"is_default\": false,\r\n \"rate_limit\": 3000\r\n}"
+ "raw": "{\r\n \"description\": \"used for xyz\",\r\n \"provider_id\": \"uuid_of_provider\",\r\n \"sms_sender\": \"+12025551212\",\r\n \"is_default\": false,\r\n \"rate_limit\": 3000\r\n}"
},
"url": {
"raw": "{{notification-api-url}}/service/{{service-id}}/sms-sender/{{sms-sender-id}}",
@@ -3376,7 +3376,7 @@
],
"body": {
"mode": "raw",
- "raw": "{\r\n \"sms_sender\": \"+18334981539\",\r\n \"is_default\": true\r\n}"
+ "raw": "{\r\n \"sms_sender\": \"+12025551212\",\r\n \"is_default\": true\r\n}"
},
"url": {
"raw": "{{notification-api-url}}/service/{{service-id}}/sms-sender/{{sms-sender-id}}/archive",
@@ -6843,4 +6843,4 @@
]
}
]
-}
+}
\ No newline at end of file
diff --git a/tests/app/conftest.py b/tests/app/conftest.py
index b5e7dd4917..832468f1fa 100644
--- a/tests/app/conftest.py
+++ b/tests/app/conftest.py
@@ -2133,9 +2133,11 @@ def _wrapper(
rate_limit_interval=None,
sms_sender_specifics=None,
archived=None,
+ provider_id=None,
):
data = {
'service_id': service_id,
+ 'provider_id': provider_id,
'sms_sender': sms_sender or current_app.config['FROM_NUMBER'],
'is_default': is_default,
'inbound_number_id': inbound_number_id,
diff --git a/tests/app/dao/test_service_sms_sender_dao.py b/tests/app/dao/test_service_sms_sender_dao.py
index d266ebb39c..994efed9f5 100644
--- a/tests/app/dao/test_service_sms_sender_dao.py
+++ b/tests/app/dao/test_service_sms_sender_dao.py
@@ -17,21 +17,29 @@
dao_get_service_sms_sender_by_service_id_and_number,
dao_get_sms_senders_by_service_id,
dao_update_service_sms_sender,
+ _validate_rate_limit,
)
from app.exceptions import ArchiveValidationError
from app.models import InboundNumber, ServiceSmsSender
from app.service.exceptions import (
SmsSenderDefaultValidationException,
SmsSenderInboundNumberIntegrityException,
+ SmsSenderProviderValidationException,
SmsSenderRateLimitIntegrityException,
)
from tests.app.db import create_service_sms_sender
-def test_dao_get_service_sms_sender_by_id(sample_service):
+def test_dao_get_service_sms_sender_by_id(sample_provider, sample_service):
+ provider = sample_provider()
service = sample_service()
second_sender = dao_add_sms_sender_for_service(
- service_id=service.id, sms_sender='second', is_default=False, inbound_number_id=None
+ service_id=service.id,
+ sms_sender='second',
+ is_default=False,
+ inbound_number_id=None,
+ provider_id=provider.id,
+ description='test',
)
service_sms_sender = dao_get_service_sms_sender_by_id(service_id=service.id, service_sms_sender_id=second_sender.id)
@@ -43,12 +51,13 @@ def test_dao_get_service_sms_sender_by_id(sample_service):
), 'This should be an empty dictionary by default.'
-def test_dao_get_service_sms_sender_by_id_with_sender_specifics(sample_service):
+def test_dao_get_service_sms_sender_by_id_with_sender_specifics(sample_provider, sample_service):
sender_specifics = {
'provider': 'Twilio',
'misc': 'This is some text.',
'some_value': 42,
}
+ provider = sample_provider()
service = sample_service()
second_sender = dao_add_sms_sender_for_service(
@@ -57,6 +66,8 @@ def test_dao_get_service_sms_sender_by_id_with_sender_specifics(sample_service):
is_default=False,
inbound_number_id=None,
sms_sender_specifics=sender_specifics,
+ provider_id=provider.id,
+ description='test',
)
service_sms_sender = dao_get_service_sms_sender_by_id(service_id=service.id, service_sms_sender_id=second_sender.id)
@@ -81,13 +92,17 @@ def test_dao_get_service_sms_senders_id_raises_exception_with_archived_sms_sende
dao_get_service_sms_sender_by_id(service_id=service.id, service_sms_sender_id=archived_sms_sender.id)
-def test_dao_get_sms_senders_by_service_id(
- sample_service,
-):
+def test_dao_get_sms_senders_by_service_id(sample_provider, sample_service):
+ provider = sample_provider()
service = sample_service()
second_sender = dao_add_sms_sender_for_service(
- service_id=service.id, sms_sender='second', is_default=False, inbound_number_id=None
+ service_id=service.id,
+ sms_sender='second',
+ is_default=False,
+ inbound_number_id=None,
+ provider_id=provider.id,
+ description='test',
)
sms_senders = dao_get_sms_senders_by_service_id(service_id=service.id)
@@ -116,7 +131,8 @@ def test_dao_get_sms_senders_by_service_id_does_not_return_archived_senders(
class TestDaoAddSmsSenderForService:
- def test_dao_add_sms_sender_for_service(self, notify_db_session, sample_service):
+ def test_dao_add_sms_sender_for_service(self, notify_db_session, sample_provider, sample_service) -> None:
+ provider = sample_provider()
service = sample_service()
stmt = select(ServiceSmsSender).where(ServiceSmsSender.service_id == service.id)
@@ -125,7 +141,12 @@ def test_dao_add_sms_sender_for_service(self, notify_db_session, sample_service)
assert len(service_sms_senders) == 1
new_sms_sender = dao_add_sms_sender_for_service(
- service_id=service.id, sms_sender='new_sms', is_default=False, inbound_number_id=None
+ service_id=service.id,
+ sms_sender='new_sms',
+ is_default=False,
+ inbound_number_id=None,
+ provider_id=provider.id,
+ description='test',
)
service_sms_senders_after_updates = notify_db_session.session.scalars(stmt).all()
@@ -134,14 +155,20 @@ def test_dao_add_sms_sender_for_service(self, notify_db_session, sample_service)
assert new_sms_sender in service_sms_senders_after_updates
- def test_dao_switches_default(self, notify_db_session, sample_service):
+ def test_dao_switches_default(self, notify_db_session, sample_provider, sample_service) -> None:
+ provider = sample_provider()
service = sample_service()
stmt = select(ServiceSmsSender).where(ServiceSmsSender.service_id == service.id)
existing_sms_sender = notify_db_session.session.scalars(stmt).one()
new_sms_sender = dao_add_sms_sender_for_service(
- service_id=service.id, sms_sender='new_sms', is_default=True, inbound_number_id=None
+ service_id=service.id,
+ sms_sender='new_sms',
+ is_default=True,
+ inbound_number_id=None,
+ provider_id=provider.id,
+ description='test',
)
stmt = select(ServiceSmsSender).where(ServiceSmsSender.id == existing_sms_sender.id)
@@ -156,8 +183,12 @@ def test_dao_switches_default(self, notify_db_session, sample_service):
@pytest.mark.parametrize('rate_limit, rate_limit_interval', ([1, None], [None, 1]))
def test_raises_exception_if_only_one_of_rate_limit_value_and_interval_provided(
- self, notify_db_session, sample_service, rate_limit, rate_limit_interval
- ):
+ self,
+ notify_db_session,
+ sample_service,
+ rate_limit,
+ rate_limit_interval,
+ ) -> None:
service = sample_service()
stmt = select(ServiceSmsSender).where(ServiceSmsSender.service_id == service.id)
@@ -173,16 +204,18 @@ def test_raises_exception_if_only_one_of_rate_limit_value_and_interval_provided(
inbound_number_id=None,
rate_limit=rate_limit,
rate_limit_interval=rate_limit_interval,
+ provider_id=None,
+ description='test',
)
- assert 'Provide both rate_limit and rate_limit_interval.' in str(e.value)
+ assert 'Provide both rate_limit and rate_limit_interval' in str(e.value)
def test_raises_exception_if_adding_number_to_use_already_allocated_inbound_number(
self,
notify_db_session,
sample_service,
sample_service_with_inbound_number,
- ):
+ ) -> None:
service_with_inbound_number = sample_service_with_inbound_number()
stmt = select(InboundNumber).where(InboundNumber.service_id == service_with_inbound_number.id)
@@ -196,6 +229,8 @@ def test_raises_exception_if_adding_number_to_use_already_allocated_inbound_numb
sms_sender='new-number',
is_default=False,
inbound_number_id=inbound_number.id,
+ provider_id=None,
+ description='test',
)
expected_msg = f'Inbound number: {inbound_number.id} is not available'
@@ -216,6 +251,8 @@ def test_raises_exception_if_adding_number_different_to_inbound_number(
sms_sender=wrong_number.number,
is_default=False,
inbound_number_id=inbound_number.id,
+ provider_id=None,
+ description='test',
)
def test_raises_exception_for_zero_rate_limit(self, notify_db_session, sample_service):
@@ -234,6 +271,8 @@ def test_raises_exception_for_zero_rate_limit(self, notify_db_session, sample_se
inbound_number_id=None,
rate_limit=0,
rate_limit_interval=1,
+ provider_id=None,
+ description='test',
)
assert 'rate_limit cannot be less than 1.' in str(e.value)
@@ -258,10 +297,27 @@ def test_raises_exception_for_zero_rate_limit_interval(
inbound_number_id=None,
rate_limit=1,
rate_limit_interval=0,
+ provider_id=None,
+ description='test',
)
assert 'rate_limit_interval cannot be less than 1.' in str(e.value)
+ def test_raises_exception_attempting_to_add_invalid_provider(self, sample_service) -> None:
+ service = sample_service()
+
+ with pytest.raises(SmsSenderProviderValidationException) as e:
+ dao_add_sms_sender_for_service(
+ service_id=service.id,
+ sms_sender='new_sms',
+ is_default=False,
+ inbound_number_id=None,
+ provider_id=uuid.uuid4(),
+ description='test',
+ )
+
+ assert 'No provider details found for id' in str(e.value)
+
class TestDaoUpdateServiceUpdateSmsSender:
def test_dao_update_service_sms_sender(
@@ -293,14 +349,20 @@ def test_dao_update_service_sms_sender(
assert existing_sms_sender_after_updates.inbound_number_id == inbound_number.id
assert existing_sms_sender_after_updates.sms_sender_specifics == sender_specifics
- def test_switches_default(self, notify_db_session, sample_service):
+ def test_switches_default(self, notify_db_session, sample_provider, sample_service) -> None:
+ provider = sample_provider()
service = sample_service()
stmt = select(ServiceSmsSender).where(ServiceSmsSender.service_id == service.id)
existing_sms_sender = notify_db_session.session.scalars(stmt).one()
new_sms_sender = dao_add_sms_sender_for_service(
- service_id=service.id, sms_sender='new_sms', is_default=False, inbound_number_id=None
+ service_id=service.id,
+ sms_sender='new_sms',
+ is_default=False,
+ inbound_number_id=None,
+ provider_id=provider.id,
+ description='test',
)
dao_update_service_sms_sender(service_id=service.id, service_sms_sender_id=new_sms_sender.id, is_default=True)
@@ -427,9 +489,16 @@ def test_raises_exception_if_updating_number_to_use_already_allocated_inbound_nu
assert expected_msg in str(e.value)
-def test_archive_sms_sender(sample_service):
+def test_archive_sms_sender(sample_provider, sample_service) -> None:
+ provider = sample_provider()
service = sample_service()
- second_sms_sender = dao_add_sms_sender_for_service(service_id=service.id, sms_sender='second', is_default=False)
+ second_sms_sender = dao_add_sms_sender_for_service(
+ service_id=service.id,
+ sms_sender='second',
+ is_default=False,
+ provider_id=provider.id,
+ description='test',
+ )
archive_sms_sender(service_id=service.id, sms_sender_id=second_sms_sender.id)
@@ -438,10 +507,18 @@ def test_archive_sms_sender(sample_service):
def test_archive_sms_sender_does_not_archive_a_sender_for_a_different_service(
+ sample_provider,
sample_service,
-):
+) -> None:
+ provider = sample_provider()
service = sample_service(service_name=f'{str(uuid.uuid4())}First service')
- sms_sender = dao_add_sms_sender_for_service(service_id=sample_service().id, sms_sender='second', is_default=False)
+ sms_sender = dao_add_sms_sender_for_service(
+ service_id=sample_service().id,
+ sms_sender='second',
+ is_default=False,
+ provider_id=provider.id,
+ description='test',
+ )
with pytest.raises(SQLAlchemyError):
archive_sms_sender(service.id, sms_sender.id)
@@ -462,11 +539,13 @@ def test_archive_sms_sender_raises_an_error_if_attempting_to_archive_a_default(s
@pytest.mark.parametrize('is_default', [True, False])
def test_archive_sms_sender_raises_an_error_if_attempting_to_archive_an_inbound_number(
notify_db_session,
+ sample_provider,
sample_service_with_inbound_number,
is_default,
-):
+) -> None:
+ provider = sample_provider()
service = sample_service_with_inbound_number()
- dao_add_sms_sender_for_service(service.id, 'second', is_default=True)
+ dao_add_sms_sender_for_service(service.id, 'second', is_default=True, provider_id=provider.id, description='test')
inbound_number = next(x for x in service.service_sms_senders if x.inbound_number_id)
@@ -517,3 +596,25 @@ def test_returns_sms_sender_if_matching_service_and_number(self, notify_db_sessi
)
assert found_sms_sender is sms_sender
+
+
+@pytest.mark.parametrize(
+ 'rate_limit, rate_limit_interval, raises_exception',
+ (
+ [1, 1, False],
+ [1, None, True],
+ [None, 1, True],
+ [None, None, False],
+ [-1, 1, True],
+ [1, -1, True],
+ ),
+)
+def test_validate_rate_limit(rate_limit, rate_limit_interval, raises_exception) -> None:
+ """Test that rate_limit and rate_limit_interval are validated correctly, raising an exception when necessary."""
+ sms_sender = ServiceSmsSender(service_id=uuid.uuid4())
+
+ if raises_exception:
+ with pytest.raises(SmsSenderRateLimitIntegrityException):
+ _validate_rate_limit(sms_sender, rate_limit, rate_limit_interval)
+ else:
+ assert _validate_rate_limit(sms_sender, rate_limit, rate_limit_interval) is None
diff --git a/tests/app/service/test_sms_sender_rest.py b/tests/app/service/test_sms_sender_rest.py
index 278530324e..18cb490020 100644
--- a/tests/app/service/test_sms_sender_rest.py
+++ b/tests/app/service/test_sms_sender_rest.py
@@ -1,7 +1,9 @@
from datetime import datetime
+from unittest.mock import ANY
import uuid
from flask import current_app
+import pytest
from sqlalchemy import select
from sqlalchemy.orm.exc import NoResultFound
@@ -10,6 +12,7 @@
def test_add_service_sms_sender_calls_dao_method(admin_request, mocker):
service_id = uuid.uuid4()
+ provider_id = str(uuid.uuid4())
added_service_sms_sender = ServiceSmsSender(created_at=datetime.utcnow())
dao_add_sms_sender_for_service = mocker.patch(
@@ -22,16 +25,111 @@ def test_add_service_sms_sender_calls_dao_method(admin_request, mocker):
'service_sms_sender.add_service_sms_sender',
service_id=service_id,
_data={
- 'sms_sender': 'second',
+ 'description': 'test description',
'is_default': False,
+ 'provider_id': provider_id,
+ 'sms_sender': 'second',
},
_expected_status=201,
)
- dao_add_sms_sender_for_service.assert_called_with(service_id=service_id, sms_sender='second', is_default=False)
+ dao_add_sms_sender_for_service.assert_called_with(
+ service_id=service_id,
+ description='test description',
+ is_default=False,
+ provider_id=provider_id,
+ sms_sender='second',
+ )
assert response_json == added_service_sms_sender.serialize()
+def test_add_service_sms_sender_returns_201_with_proper_data(admin_request, sample_provider, sample_service) -> None:
+ service = sample_service()
+ provider = sample_provider(display_name='test_provider_name')
+
+ test_sms_sender = '+1234567890'
+
+ expected_data = {
+ 'archived': False,
+ 'created_at': ANY,
+ 'description': 'test description',
+ 'id': ANY,
+ 'inbound_number_id': None,
+ 'is_default': True,
+ 'provider_id': str(provider.id),
+ 'provider_name': 'test_provider_name',
+ 'rate_limit': None,
+ 'rate_limit_interval': None,
+ 'service_id': str(service.id),
+ 'sms_sender': test_sms_sender,
+ 'sms_sender_specifics': {},
+ 'updated_at': ANY,
+ }
+
+ response_json = admin_request.post(
+ 'service_sms_sender.add_service_sms_sender',
+ service_id=service.id,
+ _data={
+ 'description': 'test description',
+ 'is_default': True,
+ 'provider_id': str(provider.id),
+ 'sms_sender': test_sms_sender,
+ },
+ _expected_status=201,
+ )
+
+ assert response_json == expected_data
+
+
+@pytest.mark.parametrize(
+ 'include_provider, include_description',
+ [(True, False), (False, True), (False, False)],
+ ids=['no_provider', 'no_description', 'no_provider_nor_description'],
+)
+def test_add_service_sms_sender_returns_400_error_when_missing_expected_data(
+ admin_request,
+ sample_provider,
+ sample_service,
+ include_provider,
+ include_description,
+) -> None:
+ service = sample_service()
+ provider = sample_provider(display_name='test_provider_name')
+
+ resp_json = admin_request.post(
+ 'service_sms_sender.add_service_sms_sender',
+ service_id=service.id,
+ _data={
+ 'description': 'test description' if include_description else None,
+ 'is_default': True,
+ 'provider_id': str(provider.id) if include_provider else None,
+ 'sms_sender': '+1234567890',
+ },
+ _expected_status=400,
+ )
+
+ assert 'ValidationError' in resp_json['errors'][0]['error']
+
+
+def test_add_service_sms_sender_returns_400_error_when_provider_does_not_exist(admin_request, sample_service) -> None:
+ service = sample_service()
+ missing_provider_id = str(uuid.uuid4())
+
+ resp_json = admin_request.post(
+ 'service_sms_sender.add_service_sms_sender',
+ service_id=service.id,
+ _data={
+ 'description': 'test description',
+ 'is_default': True,
+ 'provider_id': missing_provider_id,
+ 'sms_sender': '+1234567890',
+ },
+ _expected_status=400,
+ )
+
+ assert 'No provider details found' in resp_json['message']
+
+
def test_add_service_sms_sender_return_404_when_service_does_not_exist(admin_request, mocker):
mocker.patch('app.service.sms_sender_rest.dao_fetch_service_by_id', side_effect=NoResultFound())
@@ -66,6 +164,7 @@ def test_add_service_sms_sender_return_404_when_rate_limit_too_small(admin_reque
def test_add_service_sms_sender_new_sender_to_default(
notify_db_session,
admin_request,
+ sample_provider,
sample_service,
):
"""
@@ -76,6 +175,8 @@ def test_add_service_sms_sender_new_sender_to_default(
# This fixture also should create a default ServiceSmsSender instance.
service = sample_service()
+ provider = sample_provider()
+
stmt = select(ServiceSmsSender.id).where(
ServiceSmsSender.service_id == service.id, ServiceSmsSender.is_default.is_(True)
)
@@ -86,6 +187,8 @@ def test_add_service_sms_sender_new_sender_to_default(
'service_sms_sender.add_service_sms_sender',
service_id=service.id,
_data={
+ 'description': 'test description',
+ 'provider_id': str(provider.id),
'sms_sender': '54321',
'is_default': True,
},
@@ -135,6 +238,85 @@ def test_update_service_sms_sender_sender_specifics(
assert response_json['sms_sender_specifics'] == sender_specifics
+def test_update_service_sms_sender_provider(
+ admin_request,
+ sample_provider,
+ sample_service,
+ sample_sms_sender,
+) -> None:
+ """
+ Test updating the provider_id field of a given service_sms_sender row. This test does
+ not attempt to affect the default sender.
+ """
+
+ service = sample_service()
+ provider = sample_provider()
+
+ service_sms_sender = sample_sms_sender(
+ service_id=service.id,
+ sms_sender='1235',
+ is_default=False,
+ )
+
+ assert service_sms_sender.provider_id is None
+
+ response_json = admin_request.post(
+ 'service_sms_sender.update_service_sms_sender',
+ service_id=service.id,
+ sms_sender_id=service_sms_sender.id,
+ _data={
+ 'sms_sender': 'second',
+ 'is_default': False,
+ 'provider_id': str(provider.id),
+ },
+ _expected_status=200,
+ )
+
+ assert response_json['sms_sender'] == 'second'
+ assert not response_json['inbound_number_id']
+ assert not response_json['is_default']
+ assert response_json['provider_id'] == str(provider.id)
+
+
+def test_update_service_sms_sender_checks_for_invalid_provider(
+ admin_request,
+ sample_provider,
+ sample_service,
+ sample_sms_sender,
+) -> None:
+ """
+ Test updating the provider_id field of a given service_sms_sender row. This test does
+ not attempt to affect the default sender.
+ """
+
+ service = sample_service()
+ provider = sample_provider()
+
+ service_sms_sender = sample_sms_sender(
+ service_id=service.id,
+ sms_sender='1235',
+ is_default=False,
+ provider_id=provider.id,
+ )
+
+ assert service_sms_sender.provider_id == provider.id
+
+ response_json = admin_request.post(
+ 'service_sms_sender.update_service_sms_sender',
+ service_id=service.id,
+ sms_sender_id=service_sms_sender.id,
+ _data={
+ 'sms_sender': '54321',
+ 'is_default': False,
+ 'provider_id': str(uuid.uuid4()),
+ },
+ _expected_status=400,
+ )
+
+ assert response_json['result'] == 'error'
+ assert 'No provider details found' in response_json['message']
+
+
def test_update_service_sms_sender_does_not_allow_sender_update_for_inbound_number(
admin_request,
sample_inbound_number,