From af0dbd14be880bea4caefd520a9e57e0ee91e895 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Thu, 22 Sep 2016 17:17:34 +0100
Subject: [PATCH] correct service_whitelist dao/rest functionality

additionally added dao tests
---
 app/dao/service_whitelist_dao.py            | 14 +++++++-----
 app/service/rest.py                         | 19 +++++++---------
 tests/app/conftest.py                       | 16 +++++++++++++
 tests/app/dao/test_service_whitelist_dao.py | 25 +++++++++++++++++++++
 4 files changed, 58 insertions(+), 16 deletions(-)
 create mode 100644 tests/app/dao/test_service_whitelist_dao.py

diff --git a/app/dao/service_whitelist_dao.py b/app/dao/service_whitelist_dao.py
index 359ad0bdc..4407cbe08 100644
--- a/app/dao/service_whitelist_dao.py
+++ b/app/dao/service_whitelist_dao.py
@@ -1,11 +1,15 @@
-from sqlalchemy import or_
-
 from app import db
 from app.models import ServiceWhitelist
 
+
 def dao_fetch_service_whitelist(service_id):
-    return ServiceWhitelist.query().filter(ServiceWhitelist.service_id == service_id).all()
+    return ServiceWhitelist.query.filter(ServiceWhitelist.service_id == service_id).all()
 
 
-def dao_add_whitelisted_contact(obj):
-    db.session.add(obj)
+def dao_add_and_commit_whitelisted_contacts(objs):
+    db.session.add_all(objs)
+    db.session.commit()
+
+
+def dao_remove_service_whitelist(service_id):
+    return ServiceWhitelist.query.filter(ServiceWhitelist.service_id == service_id).delete()
diff --git a/app/service/rest.py b/app/service/rest.py
index 7dce1e19a..e475839a2 100644
--- a/app/service/rest.py
+++ b/app/service/rest.py
@@ -28,7 +28,8 @@ from app.dao.services_dao import (
 )
 from app.dao.service_whitelist_dao import (
     dao_fetch_service_whitelist,
-    dao_add_whitelisted_contact
+    dao_add_and_commit_whitelisted_contacts,
+    dao_remove_service_whitelist
 )
 from app.dao import notifications_dao
 from app.dao.provider_statistics_dao import get_fragment_count
@@ -278,14 +279,8 @@ def get_whitelist(service_id):
     whitelist = dao_fetch_service_whitelist(service_id)
 
     return {
-        'emails': [
-            {'id': item.id, 'email_address': item.email_address}
-            for item in whitelist if item.email_address is not None
-        ],
-        'mobile_numbers': [
-            {'id': item.id, 'mobile_number': item.mobile_number}
-            for item in whitelist if item.mobile_number is not None
-        ]
+        'emails': [item.email_address for item in whitelist if item.email_address is not None],
+        'mobile_numbers': [item.mobile_number for item in whitelist if item.mobile_number is not None]
     }
 
 
@@ -293,5 +288,7 @@ def get_whitelist(service_id):
 def update_whitelist(service_id):
     # todo: make this transactional
     dao_remove_service_whitelist(service_id)
-    for contact in request.get_json():
-        dao_add_whitelisted_contact(ServiceWhitelist.from_string(contact))
+
+    whitelist_objs = [ServiceWhitelist.from_string(service_id, contact) for contact in request.get_json()]
+
+    dao_add_and_commit_whitelisted_contacts(whitelist_objs)
diff --git a/tests/app/conftest.py b/tests/app/conftest.py
index d16adcc87..0f9c66af7 100644
--- a/tests/app/conftest.py
+++ b/tests/app/conftest.py
@@ -19,6 +19,7 @@ from app.models import (
     ProviderStatistics,
     ProviderDetails,
     NotificationStatistics,
+    ServiceWhitelist,
     KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM)
 from app.dao.users_dao import (save_model_user, create_user_code, create_secret_code)
 from app.dao.services_dao import (dao_create_service, dao_add_user_to_service)
@@ -862,3 +863,18 @@ def already_registered_template(notify_db,
         template = Template(**data)
         db.session.add(template)
     return template
+
+
+@pytest.fixture(scope='function')
+def sample_service_whitelist(notify_db, notify_db_session, service=None, email_address=None, mobile_number=None):
+    if service is None:
+        service = sample_service(notify_db, notify_db_session)
+
+    if not email_address and not mobile_number:
+        email_address = 'whitelisted_user@digital.gov.uk'
+
+    whitelisted_user = ServiceWhitelist.from_string(service.id, email_address or mobile_number)
+
+    notify_db.session.add(whitelisted_user)
+    notify_db.session.commit()
+    return whitelisted_user
diff --git a/tests/app/dao/test_service_whitelist_dao.py b/tests/app/dao/test_service_whitelist_dao.py
new file mode 100644
index 000000000..d399a9d4f
--- /dev/null
+++ b/tests/app/dao/test_service_whitelist_dao.py
@@ -0,0 +1,25 @@
+import uuid
+
+from app.models import ServiceWhitelist
+from app.dao.service_whitelist_dao import (#
+    dao_fetch_service_whitelist,
+    dao_add_and_commit_whitelisted_contacts,
+    dao_remove_service_whitelist
+)
+
+
+def test_fetch_service_whitelist_gets_whitelists(sample_service_whitelist):
+    whitelist = dao_fetch_service_whitelist(sample_service_whitelist.service_id)
+    assert len(whitelist) == 1
+    assert whitelist[0].id == sample_service_whitelist.id
+
+def test_fetch_service_whitelist_ignores_other_service(sample_service_whitelist):
+    assert len(dao_fetch_service_whitelist(uuid.uuid4())) == 0
+
+def test_add_and_commit_whitelisted_contacts_saves_data(sample_service):
+    whitelist = ServiceWhitelist.from_string(sample_service.id, 'foo@example.com')
+    dao_add_and_commit_whitelisted_contacts([whitelist])
+
+    db_contents = ServiceWhitelist.query.all()
+    assert len(db_contents) == 1
+    assert db_contents[0].id == whitelist.id