Merge branch 'master' into split-sms-and-retry

README.md

@@ -29,7 +29,6 @@ export ADMIN_CLIENT_USER_NAME='dev-notify-admin'
 export AWS_REGION='eu-west-1'
 export DANGEROUS_SALT='dev-notify-salt'
 export FIRETEXT_API_KEY=[contact team member for api key]
-export FIRETEXT_NUMBER="Firetext"
 export INVITATION_EMAIL_FROM='invites@notifications.service.gov.uk'
 export INVITATION_EXPIRATION_DAYS=2
 export NOTIFY_EMAIL_DOMAIN='notify.works'
@@ -41,11 +40,11 @@ export TWILIO_ACCOUNT_SID=[contact team member for account sid]
 export TWILIO_AUTH_TOKEN=[contact team member for auth token]
 export VERIFY_CODE_FROM_EMAIL_ADDRESS='no-reply@notify.works'
 export MMG_API_KEY=mmg=secret-key
-export MMG_FROM_NUMBER="MMG
 export STATSD_ENABLED=True
 export STATSD_HOST="localhost"
 export STATSD_PORT=1000
 export STATSD_PREFIX="stats-prefix"
+export FROM_NUMBER='from_number'
 "> environment.sh
 ```
 

app/celery/research_mode_tasks.py

@@ -19,7 +19,7 @@ def send_sms_response(provider, reference, to):
         body = mmg_callback(reference, to)
         headers = {"Content-type": "application/json"}
     else:
-        headers = {"Content-type": "text/plain"}
+        headers = {"Content-type": "application/x-www-form-urlencoded"}
         body = firetext_callback(reference, to)
     make_request('sms', provider, body, headers)
 
@@ -92,7 +92,12 @@ def firetext_callback(notification_id, to):
         status = "1"
     else:
         status = "0"
-    return 'mobile={}&status={}&time=2016-03-10 14:17:00&reference={}'.format(to, status, notification_id)
+    return {
+        'mobile': to,
+        'status': status,
+        'time': '2016-03-10 14:17:00',
+        'reference': notification_id
+    }
 
 
 def ses_notification_callback(reference):

app/celery/tasks.py

@@ -1,11 +1,12 @@
 import itertools
-from datetime import datetime
+from datetime import (datetime, timedelta)
 
 from flask import current_app
 from monotonic import monotonic
 from sqlalchemy.exc import SQLAlchemyError
 
 from app import clients, statsd_client
+from app.clients import STATISTICS_FAILURE
 from app.clients.email import EmailClientException
 from app.clients.sms import SmsClientException
 from app.dao.services_dao import dao_fetch_service_by_id
@@ -40,7 +41,9 @@ from app.dao.notifications_dao import (
     dao_update_notification,
     delete_notifications_created_more_than_a_week_ago,
     dao_get_notification_statistics_for_service_and_day,
-    update_provider_stats
+    update_provider_stats,
+    get_notifications,
+    update_notification_status_by_id
 )
 
 from app.dao.jobs_dao import (
@@ -337,20 +340,6 @@ def send_email(service_id, notification_id, from_address, encrypted_notification
         current_app.logger.exception(e)
 
 
-@notify_celery.task(name='send-sms-code')
-def send_sms_code(encrypted_verification):
-    provider = provider_to_use('sms', 'send-sms-code')
-
-    verification_message = encryption.decrypt(encrypted_verification)
-    try:
-        provider.send_sms(validate_and_format_phone_number(verification_message['to']),
-                          "{} is your Notify authentication code".format(
-                              verification_message['secret_code']),
-                          'send-sms-code')
-    except SmsClientException as e:
-        current_app.logger.exception(e)
-
-
 # TODO: when placeholders in templates work, this will be a real template
 def invitation_template(user_name, service_name, url, expiry_date):
     from string import Template
@@ -489,3 +478,23 @@ def provider_to_use(notification_type, notification_id):
         raise Exception("No active {} providers".format(notification_type))
 
     return clients.get_client_by_name_and_type(active_providers_in_order[0].identifier, notification_type)
+
+
+@notify_celery.task(name='timeout-sending-notifications')
+def timeout_notifications():
+    notifications = get_notifications(filter_dict={'status': 'sending'})
+    now = datetime.utcnow()
+    for noti in notifications:
+        try:
+            if (now - noti.created_at) > timedelta(
+                seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD')
+            ):
+                update_notification_status_by_id(noti.id, 'temporary-failure', STATISTICS_FAILURE)
+                current_app.logger.info((
+                    "Timeout period reached for notification ({})"
+                    ", status has been updated.").format(noti.id))
+        except Exception as e:
+            current_app.logger.exception(e)
+            current_app.logger.error((
+                "Exception raised trying to timeout notification ({})"
+                ", skipping notification update.").format(noti.id))

app/clients/sms/firetext.py

@@ -60,7 +60,7 @@ class FiretextClient(SmsClient):
         super(SmsClient, self).__init__(*args, **kwargs)
         self.current_app = current_app
         self.api_key = current_app.config.get('FIRETEXT_API_KEY')
-        self.from_number = current_app.config.get('FIRETEXT_NUMBER')
+        self.from_number = current_app.config.get('FROM_NUMBER')
         self.name = 'firetext'
         self.statsd_client = statsd_client
 
@@ -93,7 +93,7 @@ class FiretextClient(SmsClient):
                     "POST",
                     "https://www.firetext.co.uk/api/sendsms",
                     response.status_code,
-                    firetext_response
+                    firetext_response.items()
                 )
             )
         except RequestException as e:

app/clients/sms/mmg.py

@@ -60,7 +60,7 @@ class MMGClient(SmsClient):
         super(SmsClient, self).__init__(*args, **kwargs)
         self.current_app = current_app
         self.api_key = current_app.config.get('MMG_API_KEY')
-        self.from_number = current_app.config.get('MMG_FROM_NUMBER')
+        self.from_number = current_app.config.get('FROM_NUMBER')
         self.name = 'mmg'
         self.statsd_client = statsd_client
 
@@ -91,7 +91,7 @@ class MMGClient(SmsClient):
                     "POST",
                     "https://www.mmgrp.co.uk/API/json/api.php",
                     response.status_code,
-                    response.json()
+                    response.json().items()
                 )
             )
         except RequestException as e:

app/dao/notifications_dao.py

@@ -139,6 +139,14 @@ def dao_get_template_statistics_for_service(service_id, limit_days=None):
         desc(TemplateStatistics.updated_at)).all()
 
 
+def dao_get_template_statistics_for_template(template_id):
+    return TemplateStatistics.query.filter(
+        TemplateStatistics.template_id == template_id
+    ).order_by(
+        desc(TemplateStatistics.updated_at)
+    ).all()
+
+
 @transactional
 def dao_create_notification(notification, notification_type):
     if notification.job_id:
@@ -332,6 +340,10 @@ def get_notification_by_id(notification_id):
     return Notification.query.filter_by(id=notification_id).first()
 
 
+def get_notifications(filter_dict=None):
+    return _filter_query(Notification.query, filter_dict=filter_dict)
+
+
 def get_notifications_for_service(service_id,
                                   filter_dict=None,
                                   page=1,

app/dao/permissions_dao.py

@@ -32,6 +32,8 @@ class PermissionDAO(DAOClass):
     class Meta:
         model = Permission
 
+    # TODO rework this as last filter wins, whereas what is needed is
+    # append to filter so that semantics are 'and'
     def get_query(self, filter_by_dict=None):
         if filter_by_dict is None:
             filter_by_dict = MultiDict()
@@ -60,13 +62,14 @@ class PermissionDAO(DAOClass):
             self.create_instance(permission, _commit=False)
 
     def remove_user_service_permissions(self, user, service):
-        query = self.get_query(filter_by_dict={'user': user.id, 'service': service.id})
+        query = self.Meta.model.query.filter_by(user=user, service=service)
         query.delete()
 
-    def set_user_service_permission(self, user, service, permissions, _commit=False):
+    def set_user_service_permission(self, user, service, permissions, _commit=False, replace=False):
         try:
-            query = self.get_query(filter_by_dict={'user': user.id, 'service': service.id})
+            if replace:
-            query.delete()
+                query = self.Meta.model.query.filter_by(user=user, service=service)
+                query.delete()
             for p in permissions:
                 p.user = user
                 p.service = service

app/notifications/rest.py

@@ -176,9 +176,12 @@ def process_firetext_response():
         current_app.logger.info(validation_errors)
         return jsonify(result='error', message=validation_errors), 400
 
-    statsd_client.incr('notifications.callback.firetext.code.{}'.format(request.form.get('code')))
+    response_code = request.form.get('code')
+    status = request.form.get('status')
+    statsd_client.incr('notifications.callback.firetext.code.{}'.format(response_code))
+    current_app.logger.info('Firetext status: {}, extended error code: {}'.format(status, response_code))
 
-    success, errors = process_sms_client_response(status=request.form.get('status'),
+    success, errors = process_sms_client_response(status=status,
                                                   reference=request.form.get('reference'),
                                                   client_name=client_name)
     if errors:

app/schemas.py

@@ -160,6 +160,7 @@ class JobSchema(BaseSchema):
 
     class Meta:
         model = models.Job
+        exclude = ('notifications',)
 
 
 class RequestVerifyCodeSchema(ma.Schema):

app/service/rest.py

@@ -1,9 +1,13 @@
-from datetime import (datetime, date)
+from datetime import (
+    datetime,
+    date
+)
 
-from flask import Blueprint
 from flask import (
     jsonify,
-    request
+    request,
+    abort,
+    Blueprint
 )
 from sqlalchemy.orm.exc import NoResultFound
 
@@ -152,6 +156,8 @@ def add_user_to_service(service_id, user_id):
                        message='User id: {} already part of service id: {}'.format(user_id, service_id)), 400
 
     permissions, errors = permission_schema.load(request.get_json(), many=True)
+    if errors:
+        abort(400, errors)
 
     dao_add_user_to_service(service, user, permissions)
     data, errors = service_schema.dump(service)
@@ -174,15 +180,6 @@ def remove_user_from_service(service_id, user_id):
     return jsonify({}), 204
 
 
-def _process_permissions(user, service, permission_groups):
-    from app.permissions_utils import get_permissions_by_group
-    permissions = get_permissions_by_group(permission_groups)
-    for permission in permissions:
-        permission.user = user
-        permission.service = service
-    return permissions
-
-
 @service.route('/<uuid:service_id>/fragment/aggregate_statistics')
 def get_service_provider_aggregate_statistics(service_id):
     service = dao_fetch_service_by_id(service_id)

app/template_statistics/rest.py

@@ -5,7 +5,10 @@ from flask import (
     current_app
 )
 
-from app.dao.notifications_dao import dao_get_template_statistics_for_service
+from app.dao.notifications_dao import (
+    dao_get_template_statistics_for_service,
+    dao_get_template_statistics_for_template
+)
 
 from app.schemas import template_statistics_schema
 
@@ -34,3 +37,12 @@ def get_template_statistics_for_service(service_id):
     if errors:
         return jsonify(result="error", message=errors), 400
     return jsonify(data=data)
+
+
+@template_statistics.route('/<template_id>')
+def get_template_statistics_for_template_id(service_id, template_id):
+    stats = dao_get_template_statistics_for_template(template_id)
+    data, errors = template_statistics_schema.dump(stats, many=True)
+    if errors:
+        return jsonify(result="error", message=errors), 400
+    return jsonify(data=data)

app/user/rest.py

@@ -188,7 +188,7 @@ def set_permissions(user_id, service_id):
     for p in permissions:
         p.user = user
         p.service = service
-    permission_dao.set_user_service_permission(user, service, permissions, _commit=True)
+    permission_dao.set_user_service_permission(user, service, permissions, _commit=True, replace=True)
     return jsonify({}), 204
 
 

config.py

@@ -18,7 +18,6 @@ class Config(object):
     NOTIFY_JOB_QUEUE = os.environ['NOTIFY_JOB_QUEUE']
     # Notification Queue names are a combination of a prefix plus a name
     NOTIFICATION_QUEUE_PREFIX = os.environ['NOTIFICATION_QUEUE_PREFIX']
-    MMG_FROM_NUMBER = os.environ['MMG_FROM_NUMBER']
     SECRET_KEY = os.environ['SECRET_KEY']
     SQLALCHEMY_COMMIT_ON_TEARDOWN = False
     SQLALCHEMY_DATABASE_URI = os.environ['SQLALCHEMY_DATABASE_URI']
@@ -67,6 +66,11 @@ class Config(object):
             'task': 'delete-successful-notifications',
             'schedule': crontab(minute=0, hour='0,1,2'),
             'options': {'queue': 'periodic'}
+        },
+        'timeout-sending-notifications': {
+            'task': 'timeout-sending-notifications',
+            'schedule': crontab(minute=0, hour='0,1,2'),
+            'options': {'queue': 'periodic'}
         }
     }
     CELERY_QUEUES = [
@@ -87,19 +91,20 @@ class Config(object):
     ]
     TWILIO_ACCOUNT_SID = os.getenv('TWILIO_ACCOUNT_SID')
     TWILIO_AUTH_TOKEN = os.getenv('TWILIO_AUTH_TOKEN')
-    TWILIO_NUMBER = os.getenv('TWILIO_NUMBER')
-    FIRETEXT_NUMBER = os.getenv('FIRETEXT_NUMBER')
     FIRETEXT_API_KEY = os.getenv("FIRETEXT_API_KEY")
     LOADTESTING_NUMBER = os.getenv('LOADTESTING_NUMBER')
     LOADTESTING_API_KEY = os.getenv("LOADTESTING_API_KEY")
     CSV_UPLOAD_BUCKET_NAME = 'local-notifications-csv-upload'
     NOTIFICATIONS_ALERT = 5  # five mins
+    FROM_NUMBER = os.getenv('FROM_NUMBER')
 
     STATSD_ENABLED = False
     STATSD_HOST = "localhost"
     STATSD_PORT = None
     STATSD_PREFIX = None
 
+    SENDING_NOTIFICATIONS_TIMEOUT_PERIOD = 259200
+
 
 class Development(Config):
     DEBUG = True

config_live.py

@@ -14,7 +14,6 @@ class Live(Config):
     VERIFY_CODE_FROM_EMAIL_ADDRESS = os.environ['LIVE_VERIFY_CODE_FROM_EMAIL_ADDRESS']
     NOTIFY_EMAIL_DOMAIN = os.environ['LIVE_NOTIFY_EMAIL_DOMAIN']
     FIRETEXT_API_KEY = os.getenv("LIVE_FIRETEXT_API_KEY")
-    FIRETEXT_NUMBER = os.getenv("LIVE_FIRETEXT_NUMBER")
     TWILIO_AUTH_TOKEN = os.getenv('LIVE_TWILIO_AUTH_TOKEN')
     MMG_API_KEY = os.environ['LIVE_MMG_API_KEY']
     CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload'
@@ -22,6 +21,7 @@ class Live(Config):
     STATSD_HOST = os.getenv('LIVE_STATSD_HOST')
     STATSD_PORT = os.getenv('LIVE_STATSD_PORT')
     STATSD_PREFIX = os.getenv('LIVE_STATSD_PREFIX')
+    FROM_NUMBER = os.getenv('LIVE_FROM_NUMBER')
 
     BROKER_TRANSPORT_OPTIONS = {
         'region': 'eu-west-1',

config_staging.py

@@ -14,10 +14,10 @@ class Staging(Config):
     VERIFY_CODE_FROM_EMAIL_ADDRESS = os.environ['STAGING_VERIFY_CODE_FROM_EMAIL_ADDRESS']
     NOTIFY_EMAIL_DOMAIN = os.environ['STAGING_NOTIFY_EMAIL_DOMAIN']
     FIRETEXT_API_KEY = os.getenv("STAGING_FIRETEXT_API_KEY")
-    FIRETEXT_NUMBER = os.getenv("STAGING_FIRETEXT_NUMBER")
     TWILIO_AUTH_TOKEN = os.getenv('STAGING_TWILIO_AUTH_TOKEN')
     MMG_API_KEY = os.environ['STAGING_MMG_API_KEY']
     CSV_UPLOAD_BUCKET_NAME = 'staging-notifications-csv-upload'
+    FROM_NUMBER = os.getenv('STAGING_FROM_NUMBER')
 
     BROKER_TRANSPORT_OPTIONS = {
         'region': 'eu-west-1',

environment_test.sh

@@ -14,12 +14,9 @@ export SQLALCHEMY_DATABASE_URI='postgresql://localhost/test_notification_api'
 export VERIFY_CODE_FROM_EMAIL_ADDRESS='no-reply@notify.works'
 export TWILIO_ACCOUNT_SID="test"
 export TWILIO_AUTH_TOKEN="test"
-export TWILIO_NUMBER="test"
 export FIRETEXT_API_KEY="Firetext"
-export FIRETEXT_NUMBER="Firetext"
 export NOTIFY_EMAIL_DOMAIN="test.notify.com"
 export MMG_API_KEY='mmg-secret-key'
-export MMG_FROM_NUMBER='test'
 export LOADTESTING_API_KEY="loadtesting"
 export LOADTESTING_NUMBER="loadtesting"
 export STATSD_ENABLED=True
@@ -27,3 +24,4 @@ export STATSD_HOST="localhost"
 export STATSD_PORT=1000
 export STATSD_PREFIX="stats-prefix"
 export API_HOST_NAME="http://localhost:6011"
+export FROM_NUMBER='from_number'

migrations/versions/0027_update_provider_rates.py

@@ -0,0 +1,35 @@
+"""empty message
+
+Revision ID: 0027_update_provider_rates
+Revises: 0026_rename_notify_service
+Create Date: 2016-06-08 01:00:00.000000
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '0027_update_provider_rates'
+down_revision = '0026_rename_notify_service'
+
+from alembic import op
+import sqlalchemy as sa
+from datetime import datetime
+import uuid
+
+
+def upgrade():
+    ### commands auto generated by Alembic - please adjust! ###
+    op.get_bind()
+    op.execute((
+        "INSERT INTO provider_rates (id, valid_from, rate, provider_id) VALUES ('{}', '{}', 1.8, "
+        "(SELECT id FROM provider_details WHERE identifier = 'mmg'))").format(uuid.uuid4(), datetime.utcnow()))
+    op.execute((
+        "INSERT INTO provider_rates (id, valid_from, rate, provider_id) VALUES ('{}', '{}', 2.5, "
+        "(SELECT id FROM provider_details WHERE identifier = 'firetext'))").format(uuid.uuid4(), datetime.utcnow()))
+    ### end Alembic commands ###
+
+
+def downgrade():
+    ### commands auto generated by Alembic - please adjust! ###
+    op.get_bind()
+    op.execute("DELETE FROM provider_rates")
+    ### end Alembic commands ###

requirements.txt

@@ -22,4 +22,4 @@ statsd==3.2.1
 git+https://github.com/alphagov/notifications-python-client.git@1.0.0#egg=notifications-python-client==1.0.0
 
 
-git+https://github.com/alphagov/notifications-utils.git@6.2.0#egg=notifications-utils==6.2.0
+git+https://github.com/alphagov/notifications-utils.git@6.3.1#egg=notifications-utils==6.3.1

tests/app/celery/test_research_mode_tasks.py

@@ -20,6 +20,8 @@ def test_make_mmg_callback(notify_api, rmock):
     send_sms_response("mmg", "1234", "07811111111")
 
     assert rmock.called
+    assert rmock.request_history[0].url == endpoint
+    assert json.loads(rmock.request_history[0].text)['MSISDN'] == '07811111111'
 
 
 def test_make_firetext_callback(notify_api, rmock):
@@ -32,6 +34,8 @@ def test_make_firetext_callback(notify_api, rmock):
     send_sms_response("firetext", "1234", "07811111111")
 
     assert rmock.called
+    assert rmock.request_history[0].url == endpoint
+    assert 'mobile=07811111111' in rmock.request_history[0].text
 
 
 def test_make_ses_callback(notify_api, rmock):
@@ -71,11 +75,21 @@ def test_temp_failure_mmg_callback():
 
 
 def test_delivered_firetext_callback():
-    assert firetext_callback("1234", "07811111111") == "mobile=07811111111&status=0&time=2016-03-10 14:17:00&reference=1234"  # noqa
+    assert firetext_callback('1234', '07811111111') == {
+        'mobile': '07811111111',
+        'status': '0',
+        'time': '2016-03-10 14:17:00',
+        'reference': '1234'
+    }
 
 
 def test_failure_firetext_callback():
-    assert firetext_callback("1234", "07822222222") == "mobile=07822222222&status=1&time=2016-03-10 14:17:00&reference=1234"  # noqa
+    assert firetext_callback('1234', '07822222222') == {
+        'mobile': '07822222222',
+        'status': '1',
+        'time': '2016-03-10 14:17:00',
+        'reference': '1234'
+    }
 
 
 def test_delivered_ses_callback():

tests/app/celery/test_tasks.py

@@ -7,7 +7,6 @@ from notifications_utils.recipients import validate_phone_number, format_phone_n
 from app.celery import provider_tasks
 from app.celery.tasks import (
     send_sms,
-    send_sms_code,
     send_email,
     process_job,
     email_invited_user,
@@ -16,7 +15,8 @@ from app.celery.tasks import (
     delete_invitations,
     delete_failed_notifications,
     delete_successful_notifications,
-    provider_to_use
+    provider_to_use,
+    timeout_notifications
 )
 from app.celery.research_mode_tasks import (
     send_email_response,
@@ -807,33 +807,6 @@ def test_should_not_send_email_if_db_peristance_failed(sample_email_template, mo
     assert 'No row was found for one' in str(e.value)
 
 
-def test_should_send_sms_code(mocker):
-    notification = {'to': '+447234123123',
-                    'secret_code': '12345'}
-
-    encrypted_notification = encryption.encrypt(notification)
-
-    mocker.patch('app.mmg_client.send_sms')
-    send_sms_code(encrypted_notification)
-    mmg_client.send_sms.assert_called_once_with(
-        format_phone_number(validate_phone_number(notification['to'])),
-        "{} is your Notify authentication code".format(notification['secret_code']),
-        'send-sms-code')
-
-
-def test_should_throw_mmg_client_exception(mocker):
-    notification = {'to': '+447234123123',
-                    'secret_code': '12345'}
-
-    encrypted_notification = encryption.encrypt(notification)
-    mocker.patch('app.mmg_client.send_sms', side_effect=MMGClientException(mmg_error))
-    send_sms_code(encrypted_notification)
-    mmg_client.send_sms.assert_called_once_with(
-        format_phone_number(validate_phone_number(notification['to'])),
-        "{} is your Notify authentication code".format(notification['secret_code']),
-        'send-sms-code')
-
-
 def test_email_invited_user_should_send_email(notify_api, mocker):
     with notify_api.test_request_context():
         invitation = {'to': 'new_person@it.gov.uk',
@@ -1016,3 +989,41 @@ def _notification_json(template, to, personalisation=None, job_id=None, row_numb
     if row_number:
         notification['row_number'] = row_number
     return notification
+
+
+def test_update_status_of_notifications_after_timeout(notify_api,
+                                                      notify_db,
+                                                      notify_db_session,
+                                                      sample_service,
+                                                      sample_template,
+                                                      mmg_provider):
+    with notify_api.test_request_context():
+        not1 = sample_notification(
+            notify_db,
+            notify_db_session,
+            service=sample_service,
+            template=sample_template,
+            status='sending',
+            created_at=datetime.utcnow() - timedelta(
+                seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10))
+        timeout_notifications()
+        assert not1.status == 'temporary-failure'
+
+
+def test_not_update_status_of_notification_before_timeout(notify_api,
+                                                          notify_db,
+                                                          notify_db_session,
+                                                          sample_service,
+                                                          sample_template,
+                                                          mmg_provider):
+    with notify_api.test_request_context():
+        not1 = sample_notification(
+            notify_db,
+            notify_db_session,
+            service=sample_service,
+            template=sample_template,
+            status='sending',
+            created_at=datetime.utcnow() - timedelta(
+                seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') - 10))
+        timeout_notifications()
+        assert not1.status == 'sending'

tests/app/conftest.py

@@ -536,7 +536,7 @@ def mock_firetext_client(mocker, statsd_client=None):
     statsd_client = statsd_client or mocker.Mock()
     current_app = mocker.Mock(config={
         'FIRETEXT_API_KEY': 'foo',
-        'FIRETEXT_NUMBER': 'bar'
+        'FROM_NUMBER': 'bar'
     })
     client.init_app(current_app, statsd_client)
     return client
@@ -548,7 +548,7 @@ def mock_mmg_client(mocker, statsd_client=None):
     statsd_client = statsd_client or mocker.Mock()()
     current_app = mocker.Mock(config={
         'MMG_API_KEY': 'foo',
-        'MMG_FROM_NUMBER': 'bar'
+        'FROM_NUMBER': 'bar'
     })
     client.init_app(current_app, statsd_client)
     return client

tests/app/dao/test_services_dao.py

@@ -334,3 +334,53 @@ def test_delete_service_and_associated_objects(notify_db,
     assert InvitedUser.query.count() == 0
     assert Service.query.count() == 0
     assert Service.get_history_model().query.count() == 0
+
+
+def test_add_existing_user_to_another_service_doesnot_change_old_permissions(sample_user):
+
+    service_one = Service(name="service_one",
+                          email_from="service_one",
+                          message_limit=1000,
+                          active=True,
+                          restricted=False,
+                          created_by=sample_user)
+
+    dao_create_service(service_one, sample_user)
+    assert sample_user.id == service_one.users[0].id
+    test_user_permissions = Permission.query.filter_by(service=service_one, user=sample_user).all()
+    assert len(test_user_permissions) == 8
+
+    other_user = User(
+        name='Other Test User',
+        email_address='other_user@digital.cabinet-office.gov.uk',
+        password='password',
+        mobile_number='+447700900987'
+    )
+    save_model_user(other_user)
+    service_two = Service(name="service_two",
+                          email_from="service_two",
+                          message_limit=1000,
+                          active=True,
+                          restricted=False,
+                          created_by=other_user)
+    dao_create_service(service_two, other_user)
+
+    assert other_user.id == service_two.users[0].id
+    other_user_permissions = Permission.query.filter_by(service=service_two, user=other_user).all()
+    assert len(other_user_permissions) == 8
+
+    other_user_service_one_permissions = Permission.query.filter_by(service=service_one, user=other_user).all()
+    assert len(other_user_service_one_permissions) == 0
+
+    # adding the other_user to service_one should leave all other_user permissions on service_two intact
+    permissions = []
+    for p in ['send_emails', 'send_texts', 'send_letters']:
+        permissions.append(Permission(permission=p))
+
+    dao_add_user_to_service(service_one, other_user, permissions=permissions)
+
+    other_user_service_one_permissions = Permission.query.filter_by(service=service_one, user=other_user).all()
+    assert len(other_user_service_one_permissions) == 3
+
+    other_user_service_two_permissions = Permission.query.filter_by(service=service_two, user=other_user).all()
+    assert len(other_user_service_two_permissions) == 8

tests/app/template_statistics/test_rest.py

@@ -1,10 +1,12 @@
+from datetime import datetime
 import json
+
 from freezegun import freeze_time
 
 from app import db
 from app.models import TemplateStatistics
-
 from tests import create_authorization_header
+from tests.app.conftest import sample_template as create_sample_template
 
 
 @freeze_time('2016-04-09')
@@ -130,3 +132,102 @@ def test_get_all_template_statistics_with_bad_limit_arg_returns_400(notify_api,
             json_resp = json.loads(response.get_data(as_text=True))
             assert json_resp['result'] == 'error'
             assert json_resp['message'] == {'limit_days': ['blurk is not an integer']}
+
+
+def test_get_template_statistics_for_template_only_returns_for_provided_template(
+        notify_db,
+        notify_db_session,
+        notify_api,
+        sample_service
+):
+    template_1 = create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_name='Sample Template 1',
+        service=sample_service
+    )
+    template_2 = create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_name='Sample Template 2',
+        service=sample_service
+    )
+
+    template_1_stats_1 = TemplateStatistics(
+        template_id=template_1.id,
+        service_id=sample_service.id,
+        day=datetime(2001, 1, 1)
+    )
+    template_1_stats_2 = TemplateStatistics(
+        template_id=template_1.id,
+        service_id=sample_service.id,
+        day=datetime(2001, 1, 2)
+    )
+    template_2_stats = TemplateStatistics(
+        template_id=template_2.id,
+        service_id=sample_service.id,
+        day=datetime(2001, 1, 1)
+    )
+
+    # separate commit to ensure stats_1 has earlier updated_at time
+    db.session.add(template_1_stats_1)
+    db.session.commit()
+
+    db.session.add_all([template_1_stats_2, template_2_stats])
+    db.session.commit()
+
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            auth_header = create_authorization_header()
+
+            response = client.get(
+                '/service/{}/template-statistics/{}'.format(sample_service.id, template_1.id),
+                headers=[('Content-Type', 'application/json'), auth_header],
+            )
+
+            assert response.status_code == 200
+            json_resp = json.loads(response.get_data(as_text=True))
+            assert len(json_resp['data']) == 2
+            assert json_resp['data'][0]['id'] == str(template_1_stats_2.id)
+            assert json_resp['data'][1]['id'] == str(template_1_stats_1.id)
+
+
+def test_get_template_statistics_for_template_returns_empty_if_no_statistics(
+        notify_db,
+        notify_db_session,
+        notify_api,
+        sample_service
+):
+    template_1 = create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_name='Sample Template 1',
+        service=sample_service
+    )
+    template_2 = create_sample_template(
+        notify_db,
+        notify_db_session,
+        template_name='Sample Template 2',
+        service=sample_service
+    )
+
+    template_1_stats = TemplateStatistics(
+        template_id=template_1.id,
+        service_id=sample_service.id,
+        day=datetime(2001, 1, 1)
+    )
+    db.session.add(template_1_stats)
+    db.session.commit()
+
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            auth_header = create_authorization_header()
+
+            response = client.get(
+                '/service/{}/template-statistics/{}'.format(sample_service.id, template_2.id),
+                headers=[('Content-Type', 'application/json'), auth_header],
+            )
+
+            assert response.status_code == 200
+            json_resp = json.loads(response.get_data(as_text=True))
+            assert json_resp['data'] == []

tests/app/test_schemas.py

@@ -0,0 +1,10 @@
+def test_job_schema_doesnt_return_notifications(sample_notification):
+    from app.schemas import job_schema
+
+    job = sample_notification.job
+    assert job.notifications.count() == 1
+
+    data, errors = job_schema.dump(job)
+
+    assert not errors
+    assert 'notifications' not in data