Merge branch 'master' into async-job-stats

Conflicts:
	app/dao/services_dao.py

tests/app/dao/test_service_permissions_dao.py

@@ -1,36 +1,44 @@
+import pytest
+
 from app.dao.service_permissions_dao import dao_fetch_service_permissions, dao_remove_service_permission
 from app.models import EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INCOMING_SMS_TYPE
 
-from tests.app.db import create_service_permission
+from tests.app.db import create_service_permission, create_service
 
 
-def test_create_service_permission(sample_service):
-    service_permissions = create_service_permission(service_id=sample_service.id, permission=SMS_TYPE)
+@pytest.fixture(scope='function')
+def service_without_permissions(notify_db, notify_db_session):
+    return create_service(service_permissions=[])
+
+
+def test_create_service_permission(service_without_permissions):
+    service_permissions = create_service_permission(
+        service_id=service_without_permissions.id, permission=SMS_TYPE)
 
     assert len(service_permissions) == 1
-    assert service_permissions[0].service_id == sample_service.id
+    assert service_permissions[0].service_id == service_without_permissions.id
     assert service_permissions[0].permission == SMS_TYPE
 
 
-def test_fetch_service_permissions_gets_service_permissions(sample_service):
-    create_service_permission(service_id=sample_service.id, permission=LETTER_TYPE)
-    create_service_permission(service_id=sample_service.id, permission=INTERNATIONAL_SMS_TYPE)
-    create_service_permission(service_id=sample_service.id, permission=SMS_TYPE)
+def test_fetch_service_permissions_gets_service_permissions(service_without_permissions):
+    create_service_permission(service_id=service_without_permissions.id, permission=LETTER_TYPE)
+    create_service_permission(service_id=service_without_permissions.id, permission=INTERNATIONAL_SMS_TYPE)
+    create_service_permission(service_id=service_without_permissions.id, permission=SMS_TYPE)
 
-    service_permissions = dao_fetch_service_permissions(sample_service.id)
+    service_permissions = dao_fetch_service_permissions(service_without_permissions.id)
 
     assert len(service_permissions) == 3
-    assert all(sp.service_id == sample_service.id for sp in service_permissions)
+    assert all(sp.service_id == service_without_permissions.id for sp in service_permissions)
     assert all(sp.permission in [LETTER_TYPE, INTERNATIONAL_SMS_TYPE, SMS_TYPE] for sp in service_permissions)
 
 
-def test_remove_service_permission(sample_service):
-    create_service_permission(service_id=sample_service.id, permission=EMAIL_TYPE)
-    create_service_permission(service_id=sample_service.id, permission=INCOMING_SMS_TYPE)
+def test_remove_service_permission(service_without_permissions):
+    create_service_permission(service_id=service_without_permissions.id, permission=EMAIL_TYPE)
+    create_service_permission(service_id=service_without_permissions.id, permission=INCOMING_SMS_TYPE)
 
-    dao_remove_service_permission(sample_service.id, EMAIL_TYPE)
+    dao_remove_service_permission(service_without_permissions.id, EMAIL_TYPE)
 
-    permissions = dao_fetch_service_permissions(sample_service.id)
+    permissions = dao_fetch_service_permissions(service_without_permissions.id)
     assert len(permissions) == 1
     assert permissions[0].permission == INCOMING_SMS_TYPE
-    assert permissions[0].service_id == sample_service.id
+    assert permissions[0].service_id == service_without_permissions.id

tests/app/dao/test_services_dao.py

@@ -27,6 +27,7 @@ from app.dao.services_dao import (
     dao_resume_service,
     dao_fetch_active_users_for_service
 )
+from app.dao.service_permissions_dao import dao_add_service_permission, dao_remove_service_permission
 from app.dao.users_dao import save_model_user
 from app.models import (
     NotificationStatistics,
@@ -47,7 +48,11 @@ from app.models import (
     DVLA_ORG_HM_GOVERNMENT,
     KEY_TYPE_NORMAL,
     KEY_TYPE_TEAM,
-    KEY_TYPE_TEST
+    KEY_TYPE_TEST,
+    EMAIL_TYPE,
+    SMS_TYPE,
+    LETTER_TYPE,
+    INTERNATIONAL_SMS_TYPE
 )
 
 from tests.app.db import create_user, create_service
@@ -245,6 +250,62 @@ def test_get_service_by_id_returns_service(service_factory):
     assert dao_fetch_service_by_id(service.id).name == 'testing'
 
 
+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 all(p.permission in [SMS_TYPE, EMAIL_TYPE] for p in service.permissions)
+
+
+# This test is only for backward compatibility and will be removed
+# when the 'can_use' columns are dropped from the Service data model
+@pytest.mark.parametrize("permission_to_add, can_send_letters, can_send_international_sms",
+                         [(LETTER_TYPE, True, False),
+                          (INTERNATIONAL_SMS_TYPE, False, True)])
+def test_create_service_by_id_adding_service_permission_returns_service_with_permissions_set(
+        service_factory, permission_to_add, can_send_letters, can_send_international_sms):
+    service = service_factory.get('testing', email_from='testing')
+
+    dao_add_service_permission(service_id=service.id, permission=permission_to_add)
+    service.set_permissions()
+
+    service = dao_fetch_service_by_id(service.id)
+    assert len(service.permissions) == 3
+    assert all(p.permission in [SMS_TYPE, EMAIL_TYPE, permission_to_add] for p in service.permissions)
+    assert service.can_send_letters == can_send_letters
+    assert service.can_send_international_sms == can_send_international_sms
+
+
+def test_remove_permission_from_service_by_id_returns_service_with_correct_permissions(service_factory):
+    service = service_factory.get('testing', email_from='testing')
+    dao_remove_service_permission(service_id=service.id, permission=SMS_TYPE)
+
+    service = dao_fetch_service_by_id(service.id)
+    assert len(service.permissions) == 1
+    assert service.permissions[0].permission == EMAIL_TYPE
+
+
+def test_create_service_by_id_adding_and_removing_letter_returns_service_without_letter(service_factory):
+    service = service_factory.get('testing', email_from='testing')
+
+    dao_add_service_permission(service_id=service.id, permission=LETTER_TYPE)
+    service.set_permissions()
+
+    service = dao_fetch_service_by_id(service.id)
+    assert len(service.permissions) == 3
+    assert all(p.permission in [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE] for p in service.permissions)
+    assert service.can_send_letters
+
+    dao_remove_service_permission(service_id=service.id, permission=LETTER_TYPE)
+    service.set_permissions()
+    service = dao_fetch_service_by_id(service.id)
+
+    assert len(service.permissions) == 2
+    assert all(p.permission in [SMS_TYPE, EMAIL_TYPE] for p in service.permissions)
+    assert not service.can_send_letters
+
+
 def test_create_service_creates_a_history_record_with_current_data(sample_user):
     assert Service.query.count() == 0
     assert Service.get_history_model().query.count() == 0

tests/app/db.py

@@ -8,7 +8,7 @@ from app.dao.users_dao import save_model_user
 from app.dao.notifications_dao import dao_create_notification
 from app.dao.templates_dao import dao_create_template
 from app.dao.services_dao import dao_create_service
-from app.dao.service_permissions_dao import dao_create_service_permission
+from app.dao.service_permissions_dao import dao_add_service_permission
 
 
 def create_user(mobile_number="+447700900986", email="notify@digital.cabinet-office.gov.uk", state='active'):
@@ -27,7 +27,9 @@ def create_user(mobile_number="+447700900986", email="notify@digital.cabinet-off
     return user
 
 
-def create_service(user=None, service_name="Sample service", service_id=None, restricted=False):
+def create_service(
+        user=None, service_name="Sample service", service_id=None, restricted=False,
+        service_permissions=[EMAIL_TYPE, SMS_TYPE]):
     service = Service(
         name=service_name,
         message_limit=1000,
@@ -35,7 +37,7 @@ def create_service(user=None, service_name="Sample service", service_id=None, re
         email_from=service_name.lower().replace(' ', '.'),
         created_by=user or create_user()
     )
-    dao_create_service(service, service.created_by, service_id)
+    dao_create_service(service, service.created_by, service_id, service_permissions=service_permissions)
     return service
 
 
@@ -147,7 +149,7 @@ def create_job(template,
 
 
 def create_service_permission(service_id, permission=EMAIL_TYPE):
-    dao_create_service_permission(
+    dao_add_service_permission(
         service_id if service_id else create_service().id, permission)
 
     service_permissions = ServicePermission.query.all()

tests/app/notifications/test_process_notification.py

@@ -97,25 +97,6 @@ def test_persist_notification_throws_exception_when_missing_template(sample_api_
     assert NotificationHistory.query.count() == 0
 
 
-def test_exception_thown_by_redis_store_get_should_not_be_fatal(sample_template, sample_api_key, mocker):
-    mocker.patch(
-        'app.notifications.process_notifications.redis_store.redis_store.incr',
-        side_effect=Exception("broken redis"))
-
-    notification = persist_notification(
-        sample_template.id,
-        sample_template.version,
-        '+447111111111',
-        sample_template.service,
-        {},
-        'sms',
-        sample_api_key.id,
-        sample_api_key.key_type)
-    assert Notification.query.count() == 1
-    assert Notification.query.get(notification.id) is not None
-    assert NotificationHistory.query.count() == 1
-
-
 def test_cache_is_not_incremented_on_failure_to_persist_notification(sample_api_key, mocker):
     mocked_redis = mocker.patch('app.redis_store.get')
     mock_service_template_cache = mocker.patch('app.redis_store.get_all_from_hash')

tests/app/notifications/test_validators.py

@@ -23,42 +23,6 @@ from tests.app.conftest import (
     sample_api_key)
 
 
-@pytest.mark.parametrize('key_type', ['team', 'normal'])
-def test_exception_thrown_by_redis_store_get_should_not_be_fatal(
-        notify_db,
-        notify_db_session,
-        notify_api,
-        key_type,
-        mocker):
-    with freeze_time("2016-01-01 12:00:00.000000"):
-
-        mocker.patch('app.notifications.validators.redis_store.redis_store.get', side_effect=Exception("broken redis"))
-        mocker.patch('app.notifications.validators.redis_store.redis_store.set')
-
-        service = create_service(notify_db, notify_db_session, restricted=True, limit=4)
-        for x in range(5):
-            create_notification(notify_db, notify_db_session, service=service)
-
-        with pytest.raises(TooManyRequestsError) as e:
-            check_service_over_daily_message_limit(key_type, service)
-        assert e.value.status_code == 429
-        assert e.value.message == 'Exceeded send limits (4) for today'
-        assert e.value.fields == []
-        app.notifications.validators.redis_store.redis_store.set.assert_called_with(
-            "{}-2016-01-01-count".format(str(service.id)), 5, 3600, None, False, False
-        )
-
-
-@pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
-def test_exception_thown_by_redis_store_set_should_not_be_fatal(
-        key_type,
-        sample_service,
-        mocker):
-    mocker.patch('app.notifications.validators.redis_store.redis_store.set', side_effect=Exception("broken redis"))
-    mocker.patch('app.notifications.validators.redis_store.get', return_value=None)
-    assert not check_service_over_daily_message_limit(key_type, sample_service)
-
-
 @pytest.mark.parametrize('key_type', ['test', 'team', 'normal'])
 def test_check_service_message_limit_in_cache_with_unrestricted_service_is_allowed(
         key_type,