Merge pull request #1328 from alphagov/international-default

Make international text messaging on by default for new services

tests/app/conftest.py

@@ -137,7 +137,7 @@ def sample_service(
     restricted=False,
     limit=1000,
     email_from=None,
-    permissions=[SMS_TYPE, EMAIL_TYPE],
+    permissions=None,
     research_mode=None,
     free_sms_fragment_limit=250000
 ):
@@ -166,7 +166,7 @@ def sample_service(
         if user not in service.users:
             dao_add_user_to_service(service, user)
 
-    if INBOUND_SMS_TYPE in permissions:
+    if permissions and INBOUND_SMS_TYPE in permissions:
         create_inbound_number('12345', service_id=service.id)
 
     return service

tests/app/dao/test_services_dao.py

@@ -57,6 +57,7 @@ from app.models import (
     KEY_TYPE_TEST,
     EMAIL_TYPE,
     SMS_TYPE,
+    INTERNATIONAL_SMS_TYPE,
     LETTER_TYPE,
     SERVICE_PERMISSION_TYPES
 )
@@ -260,25 +261,30 @@ def test_create_service_returns_service_with_default_permissions(service_factory
     service = service_factory.get('testing', email_from='testing')
 
     service = dao_fetch_service_by_id(service.id)
-    assert len(service.permissions) == 2
-    assert set([SMS_TYPE, EMAIL_TYPE]) == set(p.permission for p in service.permissions)
+    _assert_service_permissions(service.permissions, (
+        SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE,
+    ))
 
 
-@pytest.mark.parametrize("permission_to_remove, permission_remaining",
-                         [(SMS_TYPE, EMAIL_TYPE),
-                          (EMAIL_TYPE, SMS_TYPE)])
+@pytest.mark.parametrize("permission_to_remove, permission_remaining", [
+    (SMS_TYPE, EMAIL_TYPE),
+    (EMAIL_TYPE, SMS_TYPE),
+])
 def test_remove_permission_from_service_by_id_returns_service_with_correct_permissions(
-        sample_service, permission_to_remove, permission_remaining):
+    sample_service, permission_to_remove, permission_remaining
+):
     dao_remove_service_permission(service_id=sample_service.id, permission=permission_to_remove)
 
     service = dao_fetch_service_by_id(sample_service.id)
-    assert len(service.permissions) == 1
-    assert service.permissions[0].permission == permission_remaining
+    _assert_service_permissions(service.permissions, (
+        permission_remaining, INTERNATIONAL_SMS_TYPE,
+    ))
 
 
 def test_removing_all_permission_returns_service_with_no_permissions(sample_service):
     dao_remove_service_permission(service_id=sample_service.id, permission=SMS_TYPE)
     dao_remove_service_permission(service_id=sample_service.id, permission=EMAIL_TYPE)
+    dao_remove_service_permission(service_id=sample_service.id, permission=INTERNATIONAL_SMS_TYPE)
 
     service = dao_fetch_service_by_id(sample_service.id)
     assert len(service.permissions) == 0
@@ -298,14 +304,16 @@ def test_create_service_by_id_adding_and_removing_letter_returns_service_without
     dao_add_service_permission(service_id=service.id, permission=LETTER_TYPE)
 
     service = dao_fetch_service_by_id(service.id)
-    assert len(service.permissions) == 3
-    assert set([SMS_TYPE, EMAIL_TYPE, LETTER_TYPE]) == set([p.permission for p in service.permissions])
+    _assert_service_permissions(service.permissions, (
+        SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE,
+    ))
 
     dao_remove_service_permission(service_id=service.id, permission=LETTER_TYPE)
     service = dao_fetch_service_by_id(service.id)
 
-    assert len(service.permissions) == 2
-    assert set([SMS_TYPE, EMAIL_TYPE]) == set([p.permission for p in service.permissions])
+    _assert_service_permissions(service.permissions, (
+        SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE,
+    ))
 
 
 def test_create_service_creates_a_history_record_with_current_data(sample_user):
@@ -382,7 +390,10 @@ def test_update_service_permission_creates_a_history_record_with_current_data(sa
     service_from_db = Service.query.first()
 
     assert service_from_db.version == 2
-    assert LETTER_TYPE in [p.permission for p in service_from_db.permissions]
+
+    _assert_service_permissions(service.permissions, (
+        SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, LETTER_TYPE,
+    ))
 
     permission = [p for p in service.permissions if p.permission == 'sms'][0]
     service.permissions.remove(permission)
@@ -393,7 +404,9 @@ def test_update_service_permission_creates_a_history_record_with_current_data(sa
 
     service_from_db = Service.query.first()
     assert service_from_db.version == 3
-    assert SMS_TYPE not in [p.permission for p in service_from_db.permissions]
+    _assert_service_permissions(service.permissions, (
+        EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, LETTER_TYPE,
+    ))
 
     assert len(Service.get_history_model().query.filter_by(name='service_name').all()) == 3
     assert Service.get_history_model().query.filter_by(name='service_name').all()[2].version == 3
@@ -431,7 +444,7 @@ def test_delete_service_and_associated_objects(notify_db,
                                                sample_permission,
                                                sample_provider_statistics):
     # Default service permissions of Email and SMS
-    assert ServicePermission.query.count() == 2
+    assert ServicePermission.query.count() == 3
 
     delete_service_and_all_associated_db_objects(sample_service)
     assert NotificationStatistics.query.count() == 0
@@ -949,3 +962,9 @@ def test_dao_allocating_inbound_number_shows_on_service(notify_db_session, sampl
     dao_set_inbound_number_to_service(service.id, inbound_numbers[0])
 
     assert service.inbound_number.number == inbound_numbers[0].number
+
+
+def _assert_service_permissions(service_permissions, expected):
+
+    assert len(service_permissions) == len(expected)
+    assert set(expected) == set(p.permission for p in service_permissions)

tests/app/notifications/test_validators.py

@@ -310,9 +310,14 @@ def test_should_not_rate_limit_if_limiting_is_disabled(
 
 
 @pytest.mark.parametrize('key_type', ['test', 'normal'])
-def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_int_sms(sample_service, key_type):
+def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_int_sms(
+    key_type,
+    notify_db,
+    notify_db_session,
+):
+    service = create_service(notify_db, notify_db_session, permissions=[SMS_TYPE])
     with pytest.raises(BadRequestError) as e:
-        validate_and_format_recipient('20-12-1234-1234', key_type, sample_service, SMS_TYPE)
+        validate_and_format_recipient('20-12-1234-1234', key_type, service, SMS_TYPE)
     assert e.value.status_code == 400
     assert e.value.message == 'Cannot send to international mobile numbers'
     assert e.value.fields == []

tests/app/service/test_rest.py

@@ -207,7 +207,14 @@ def test_get_service_list_has_default_permissions(client, service_factory):
     assert response.status_code == 200
     json_resp = json.loads(response.get_data(as_text=True))
     assert len(json_resp['data']) == 3
-    assert all([set(json['permissions']) == set([EMAIL_TYPE, SMS_TYPE]) for json in json_resp['data']])
+    assert all(
+        set(
+            json['permissions']
+        ) == set([
+            EMAIL_TYPE, SMS_TYPE, INTERNATIONAL_SMS_TYPE,
+        ])
+        for json in json_resp['data']
+    )
 
 
 def test_get_service_by_id_has_default_service_permissions(client, sample_service):
@@ -218,7 +225,11 @@ def test_get_service_by_id_has_default_service_permissions(client, sample_servic
     )
     json_resp = json.loads(resp.get_data(as_text=True))
 
-    assert set(json_resp['data']['permissions']) == set([EMAIL_TYPE, SMS_TYPE])
+    assert set(
+        json_resp['data']['permissions']
+    ) == set([
+        EMAIL_TYPE, SMS_TYPE, INTERNATIONAL_SMS_TYPE,
+    ])
 
 
 def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db):

tests/app/v2/notifications/test_post_notifications.py

@@ -328,17 +328,26 @@ def test_returns_a_429_limit_exceeded_if_rate_limit_exceeded(
     assert not deliver_mock.called
 
 
-def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(client, sample_service, sample_template):
+def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(
+    client,
+    notify_db,
+    notify_db_session,
+):
+
+    service = sample_service(notify_db, notify_db_session, permissions=[SMS_TYPE])
+    template = create_sample_template(notify_db, notify_db_session, service=service)
+
     data = {
         'phone_number': '20-12-1234-1234',
-        'template_id': sample_template.id
+        'template_id': template.id
     }
-    auth_header = create_authorization_header(service_id=sample_service.id)
+    auth_header = create_authorization_header(service_id=service.id)
 
     response = client.post(
         path='/v2/notifications/sms',
         data=json.dumps(data),
-        headers=[('Content-Type', 'application/json'), auth_header])
+        headers=[('Content-Type', 'application/json'), auth_header]
+    )
 
     assert response.status_code == 400
     assert response.headers['Content-type'] == 'application/json'
@@ -378,18 +387,20 @@ def test_post_sms_notification_returns_400_if_not_allowed_to_send_notification(
     ]
 
 
-def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms(notify_db, notify_db_session, client, mocker):
-
-    service = sample_service(notify_db, notify_db_session, permissions=[SMS_TYPE, INTERNATIONAL_SMS_TYPE])
-    template = create_sample_template(notify_db, notify_db_session, service=service)
+def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms(
+    sample_service,
+    sample_template,
+    client,
+    mocker,
+):
 
     mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
 
     data = {
         'phone_number': '20-12-1234-1234',
-        'template_id': template.id
+        'template_id': sample_template.id
     }
-    auth_header = create_authorization_header(service_id=service.id)
+    auth_header = create_authorization_header(service_id=sample_service.id)
 
     response = client.post(
         path='/v2/notifications/sms',