Merge branch 'master' into add-reply-to-notifications

Makefile

@@ -277,7 +277,6 @@ cf-deploy: ## Deploys the app to Cloud Foundry
 	cf rename ${CF_APP} ${CF_APP}-rollback
 	cf push ${CF_APP} -f ${CF_MANIFEST_FILE}
 	cf scale -i $$(cf curl /v2/apps/$$(cf app --guid ${CF_APP}-rollback) | jq -r ".entity.instances" 2>/dev/null || echo "1") ${CF_APP}
-	cf stop ${CF_APP}-rollback
 	cf delete -f ${CF_APP}-rollback
 
 .PHONY: cf-deploy-api-db-migration
@@ -287,7 +286,7 @@ cf-deploy-api-db-migration:
 	cf unbind-service notify-api-db-migration notify-config
 	cf unbind-service notify-api-db-migration notify-aws
 	cf push notify-api-db-migration -f manifest-api-${CF_SPACE}.yml
-	cf run-task notify-api-db-migration "python db.py db upgrade" --name api_db_migration
+	cf run-task notify-api-db-migration "flask db upgrade" --name api_db_migration
 
 .PHONY: cf-check-api-db-migration-task
 cf-check-api-db-migration-task: ## Get the status for the last notify-api-db-migration task
@@ -310,4 +309,3 @@ cf-push:
 .PHONY: check-if-migrations-to-run
 check-if-migrations-to-run:
 	@echo $(shell python3 scripts/check_if_new_migration.py)
-

README.md

@@ -44,6 +44,9 @@ export FIRETEXT_API_KEY='FIRETEXT_ACTUAL_KEY'
 export STATSD_PREFIX='YOU_OWN_PREFIX'
 export NOTIFICATION_QUEUE_PREFIX='YOUR_OWN_PREFIX'
 export REDIS_URL="redis://localhost:6379/0"
+export FLASK_APP=application.py
+export FLASK_DEBUG=1
+export WERKZEUG_DEBUG_PIN=off
 "> environment.sh
 ```
 
@@ -102,17 +105,20 @@ That will run pycodestyle for code analysis and our unit test suite. If you wish
 
 
 
-## To remove functional test data
+## To run one off tasks
 
-NOTE: There is assumption that both the server name prefix and user name prefix are followed by a uuid.
+Tasks are run through the `flask` command - run `flask --help` for more information. There are two sections we need to
-The script will search for all services/users with that prefix and only remove it if the prefix is followed by a uuid otherwise it will be skipped.
+care about: `flask db` contains alembic migration commands, and `flask command` contains all of our custom commands. For
+example, to purge all dynamically generated functional test data, do the following:
 
 Locally
 ```
-python application.py purge_functional_test_data -u <functional tests user name prefix> # Remove the user and associated services.
+flask command purge_functional_test_data -u <functional tests user name prefix>
 ```
 
 On the server
 ```
-python server_commands.py purge_functional_test_data -u <functional tests user name prefix> # Remove the user and associated services.
+cf run-task notify-api "flask command purge_functional_test_data -u <functional tests user name prefix>"
 ```
+
+All commands and command options have a --help command if you need more information.

app/__init__.py

@@ -6,6 +6,7 @@ import uuid
 from flask import Flask, _request_ctx_stack, request, g, jsonify
 from flask_sqlalchemy import SQLAlchemy
 from flask_marshmallow import Marshmallow
+from flask_migrate import Migrate
 from monotonic import monotonic
 from notifications_utils.clients.statsd.statsd_client import StatsdClient
 from notifications_utils.clients.redis.redis_client import RedisClient
@@ -25,6 +26,7 @@ DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%S.%fZ"
 DATE_FORMAT = "%Y-%m-%d"
 
 db = SQLAlchemy()
+migrate = Migrate()
 ma = Marshmallow()
 notify_celery = NotifyCelery()
 firetext_client = FiretextClient()
@@ -42,21 +44,19 @@ api_user = LocalProxy(lambda: _request_ctx_stack.top.api_user)
 authenticated_service = LocalProxy(lambda: _request_ctx_stack.top.authenticated_service)
 
 
-def create_app(app_name=None):
+def create_app(application):
-    application = Flask(__name__)
-
     from app.config import configs
 
     notify_environment = os.environ['NOTIFY_ENVIRONMENT']
 
     application.config.from_object(configs[notify_environment])
 
-    if app_name:
+    application.config['NOTIFY_APP_NAME'] = application.name
-        application.config['NOTIFY_APP_NAME'] = app_name
 
     init_app(application)
     request_helper.init_app(application)
     db.init_app(application)
+    migrate.init_app(application, db=db)
     ma.init_app(application)
     statsd_client.init_app(application)
     logging.init_app(application, statsd_client)
@@ -73,6 +73,10 @@ def create_app(app_name=None):
     register_blueprint(application)
     register_v2_blueprints(application)
 
+    # avoid circular imports by importing this file later 😬
+    from app.commands import setup_commands
+    setup_commands(application)
+
     return application
 
 

app/celery/tasks.py

@@ -488,6 +488,7 @@ def send_inbound_sms_to_service(self, inbound_sms_id, service_id):
                                             inbound_id=inbound_sms_id)
     data = {
         "id": str(inbound_sms.id),
+        # TODO: should we be validating and formatting the phone number here?
         "source_number": inbound_sms.user_number,
         "destination_number": inbound_sms.notify_number,
         "message": inbound_sms.content,

app/commands.py

@@ -1,7 +1,11 @@
 import uuid
 from datetime import datetime, timedelta
 from decimal import Decimal
-from flask_script import Command, Option
+import functools
+
+import flask
+from flask import current_app
+import click
 
 from app import db
 from app.dao.monthly_billing_dao import (
@@ -14,181 +18,197 @@ from app.dao.services_dao import (
     delete_service_and_all_associated_db_objects,
     dao_fetch_all_services_by_user
 )
-from app.dao.provider_rates_dao import create_provider_rates
+from app.dao.provider_rates_dao import create_provider_rates as dao_create_provider_rates
 from app.dao.users_dao import (delete_model_user, delete_user_verify_codes)
 from app.utils import get_midnight_for_day_before, get_london_midnight_in_utc
 from app.performance_platform.processing_time import send_processing_time_for_start_and_end
 
 
-class CreateProviderRateCommand(Command):
+@click.group(name='command', help='Additional commands')
+def command_group():
+    pass
 
-    option_list = (
-        Option('-p', '--provider_name', dest="provider_name", help='Provider name'),
-        Option('-c', '--cost', dest="cost", help='Cost (pence) per message including decimals'),
-        Option('-d', '--valid_from', dest="valid_from", help="Date (%Y-%m-%dT%H:%M:%S) valid from")
-    )
 
-    def run(self, provider_name, cost, valid_from):
+class notify_command:
-        if provider_name not in PROVIDERS:
+    def __init__(self, name=None):
-            raise Exception("Invalid provider name, must be one of ({})".format(', '.join(PROVIDERS)))
+        self.name = name
 
+    def __call__(self, func):
+        # we need to call the flask with_appcontext decorator to ensure the config is loaded, db connected etc etc.
+        # we also need to use functools.wraps to carry through the names and docstrings etc of the functions.
+        # Then we need to turn it into a click.Command - that's what command_group.add_command expects.
+        @click.command(name=self.name)
+        @functools.wraps(func)
+        @flask.cli.with_appcontext
+        def wrapper(*args, **kwargs):
+            return func(*args, **kwargs)
+
+        command_group.add_command(wrapper)
+
+        return wrapper
+
+
+@notify_command()
+@click.option('-p', '--provider_name', required=True, help='Provider name')
+@click.option('-c', '--cost', required=True, help='Cost (pence) per message including decimals')
+@click.option('-d', '--valid_from', required=True, help="Date (%Y-%m-%dT%H:%M:%S) valid from")
+def create_provider_rates(provider_name, cost, valid_from):
+    """
+    Backfill rates for a given provider
+    """
+    if provider_name not in PROVIDERS:
+        raise Exception("Invalid provider name, must be one of ({})".format(', '.join(PROVIDERS)))
+
+    try:
+        cost = Decimal(cost)
+    except:
+        raise Exception("Invalid cost value.")
+
+    try:
+        valid_from = datetime.strptime('%Y-%m-%dT%H:%M:%S', valid_from)
+    except:
+        raise Exception("Invalid valid_from date. Use the format %Y-%m-%dT%H:%M:%S")
+
+    dao_create_provider_rates(provider_name, valid_from, cost)
+
+
+@notify_command()
+@click.option('-u', '--user_email_prefix', required=True, help="""
+    Functional test user email prefix. eg "notify-test-preview"
+""")  # noqa
+def purge_functional_test_data(user_email_prefix):
+    """
+    Remove non-seeded functional test data
+
+    users, services, etc. Give an email prefix. Probably "notify-test-preview".
+    """
+    users = User.query.filter(User.email_address.like("{}%".format(user_email_prefix))).all()
+    for usr in users:
+        # Make sure the full email includes a uuid in it
+        # Just in case someone decides to use a similar email address.
         try:
-            cost = Decimal(cost)
+            uuid.UUID(usr.email_address.split("@")[0].split('+')[1])
-        except:
+        except ValueError:
-            raise Exception("Invalid cost value.")
+            print("Skipping {} as the user email doesn't contain a UUID.".format(usr.email_address))
-
-        try:
-            valid_from = datetime.strptime('%Y-%m-%dT%H:%M:%S', valid_from)
-        except:
-            raise Exception("Invalid valid_from date. Use the format %Y-%m-%dT%H:%M:%S")
-
-        create_provider_rates(provider_name, valid_from, cost)
-
-
-class PurgeFunctionalTestDataCommand(Command):
-
-    option_list = (
-        Option('-u', '-user-email-prefix', dest='user_email_prefix', help="Functional test user email prefix."),
-    )
-
-    def run(self, user_email_prefix=None):
-        if user_email_prefix:
-            users = User.query.filter(User.email_address.like("{}%".format(user_email_prefix))).all()
-            for usr in users:
-                # Make sure the full email includes a uuid in it
-                # Just in case someone decides to use a similar email address.
-                try:
-                    uuid.UUID(usr.email_address.split("@")[0].split('+')[1])
-                except ValueError:
-                    print("Skipping {} as the user email doesn't contain a UUID.".format(usr.email_address))
-                else:
-                    services = dao_fetch_all_services_by_user(usr.id)
-                    if services:
-                        for service in services:
-                            delete_service_and_all_associated_db_objects(service)
-                    else:
-                        delete_user_verify_codes(usr)
-                        delete_model_user(usr)
-
-
-class CustomDbScript(Command):
-
-    option_list = (
-        Option('-n', '-name-of-db-function', dest='name_of_db_function', help="Function name of the DB script to run"),
-    )
-
-    def run(self, name_of_db_function):
-        db_function = getattr(self, name_of_db_function, None)
-        if callable(db_function):
-            db_function()
         else:
-            print('The specified function does not exist.')
+            services = dao_fetch_all_services_by_user(usr.id)
+            if services:
+                for service in services:
+                    delete_service_and_all_associated_db_objects(service)
+            else:
+                delete_user_verify_codes(usr)
+                delete_model_user(usr)
 
-    def backfill_notification_statuses(self):
+
-        """
+@notify_command()
-        This will be used to populate the new `Notification._status_fkey` with the old
+def backfill_notification_statuses():
-        `Notification._status_enum`
+    """
-        """
+    DEPRECATED. Populates notification_status.
-        LIMIT = 250000
+
-        subq = "SELECT id FROM notification_history WHERE notification_status is NULL LIMIT {}".format(LIMIT)
+    This will be used to populate the new `Notification._status_fkey` with the old
-        update = "UPDATE notification_history SET notification_status = status WHERE id in ({})".format(subq)
+    `Notification._status_enum`
+    """
+    LIMIT = 250000
+    subq = "SELECT id FROM notification_history WHERE notification_status is NULL LIMIT {}".format(LIMIT)
+    update = "UPDATE notification_history SET notification_status = status WHERE id in ({})".format(subq)
+    result = db.session.execute(subq).fetchall()
+
+    while len(result) > 0:
+        db.session.execute(update)
+        print('commit {} updates at {}'.format(LIMIT, datetime.utcnow()))
+        db.session.commit()
         result = db.session.execute(subq).fetchall()
 
-        while len(result) > 0:
-            db.session.execute(update)
-            print('commit {} updates at {}'.format(LIMIT, datetime.utcnow()))
-            db.session.commit()
-            result = db.session.execute(subq).fetchall()
 
-    def update_notification_international_flag(self):
+@notify_command()
-        # 250,000 rows takes 30 seconds to update.
+def update_notification_international_flag():
-        subq = "select id from notifications where international is null limit 250000"
+    """
-        update = "update notifications set international = False where id in ({})".format(subq)
+    DEPRECATED. Set notifications.international=false.
+    """
+    # 250,000 rows takes 30 seconds to update.
+    subq = "select id from notifications where international is null limit 250000"
+    update = "update notifications set international = False where id in ({})".format(subq)
+    result = db.session.execute(subq).fetchall()
+
+    while len(result) > 0:
+        db.session.execute(update)
+        print('commit 250000 updates at {}'.format(datetime.utcnow()))
+        db.session.commit()
         result = db.session.execute(subq).fetchall()
 
-        while len(result) > 0:
+    # Now update notification_history
-            db.session.execute(update)
+    subq_history = "select id from notification_history where international is null limit 250000"
-            print('commit 250000 updates at {}'.format(datetime.utcnow()))
+    update_history = "update notification_history set international = False where id in ({})".format(subq_history)
-            db.session.commit()
+    result_history = db.session.execute(subq_history).fetchall()
-            result = db.session.execute(subq).fetchall()
+    while len(result_history) > 0:
-
+        db.session.execute(update_history)
-        # Now update notification_history
+        print('commit 250000 updates at {}'.format(datetime.utcnow()))
-        subq_history = "select id from notification_history where international is null limit 250000"
+        db.session.commit()
-        update_history = "update notification_history set international = False where id in ({})".format(subq_history)
         result_history = db.session.execute(subq_history).fetchall()
-        while len(result_history) > 0:
-            db.session.execute(update_history)
-            print('commit 250000 updates at {}'.format(datetime.utcnow()))
-            db.session.commit()
-            result_history = db.session.execute(subq_history).fetchall()
 
-    def fix_notification_statuses_not_in_sync(self):
-        """
-        This will be used to correct an issue where Notification._status_enum and NotificationHistory._status_fkey
-        became out of sync. See 979e90a.
 
-        Notification._status_enum is the source of truth so NotificationHistory._status_fkey will be updated with
+@notify_command()
-        these values.
+def fix_notification_statuses_not_in_sync():
-        """
+    """
-        MAX = 10000
+    DEPRECATED.
+    This will be used to correct an issue where Notification._status_enum and NotificationHistory._status_fkey
+    became out of sync. See 979e90a.
 
-        subq = "SELECT id FROM notifications WHERE cast (status as text) != notification_status LIMIT {}".format(MAX)
+    Notification._status_enum is the source of truth so NotificationHistory._status_fkey will be updated with
-        update = "UPDATE notifications SET notification_status = status WHERE id in ({})".format(subq)
+    these values.
+    """
+    MAX = 10000
+
+    subq = "SELECT id FROM notifications WHERE cast (status as text) != notification_status LIMIT {}".format(MAX)
+    update = "UPDATE notifications SET notification_status = status WHERE id in ({})".format(subq)
+    result = db.session.execute(subq).fetchall()
+
+    while len(result) > 0:
+        db.session.execute(update)
+        print('Committed {} updates at {}'.format(len(result), datetime.utcnow()))
+        db.session.commit()
         result = db.session.execute(subq).fetchall()
 
-        while len(result) > 0:
+    subq_hist = "SELECT id FROM notification_history WHERE cast (status as text) != notification_status LIMIT {}" \
-            db.session.execute(update)
+        .format(MAX)
-            print('Committed {} updates at {}'.format(len(result), datetime.utcnow()))
+    update = "UPDATE notification_history SET notification_status = status WHERE id in ({})".format(subq_hist)
-            db.session.commit()
+    result = db.session.execute(subq_hist).fetchall()
-            result = db.session.execute(subq).fetchall()
 
-        subq_hist = "SELECT id FROM notification_history WHERE cast (status as text) != notification_status LIMIT {}" \
+    while len(result) > 0:
-            .format(MAX)
+        db.session.execute(update)
-        update = "UPDATE notification_history SET notification_status = status WHERE id in ({})".format(subq_hist)
+        print('Committed {} updates at {}'.format(len(result), datetime.utcnow()))
+        db.session.commit()
         result = db.session.execute(subq_hist).fetchall()
 
-        while len(result) > 0:
-            db.session.execute(update)
-            print('Committed {} updates at {}'.format(len(result), datetime.utcnow()))
-            db.session.commit()
-            result = db.session.execute(subq_hist).fetchall()
 
-    def link_inbound_numbers_to_service(self):
+@notify_command()
-        update = """
+def link_inbound_numbers_to_service():
-        UPDATE inbound_numbers SET
+    """
-        service_id = services.id,
+    DEPRECATED.
-        updated_at = now()
-        FROM services
-        WHERE services.sms_sender = inbound_numbers.number AND
-        inbound_numbers.service_id is null
-        """
-        result = db.session.execute(update)
-        db.session.commit()
 
-        print("Linked {} inbound numbers to service".format(result.rowcount))
+    Matches inbound numbers and service ids based on services.sms_sender
+    """
+    update = """
+    UPDATE inbound_numbers SET
+    service_id = services.id,
+    updated_at = now()
+    FROM services
+    WHERE services.sms_sender = inbound_numbers.number AND
+    inbound_numbers.service_id is null
+    """
+    result = db.session.execute(update)
+    db.session.commit()
+
+    print("Linked {} inbound numbers to service".format(result.rowcount))
 
 
-class PopulateMonthlyBilling(Command):
+@notify_command()
-    option_list = (
+@click.option('-y', '--year', required=True, help="Use for integer value for year, e.g. 2017")
-        Option('-y', '-year', dest="year", help="Use for integer value for year, e.g. 2017"),
+def populate_monthly_billing(year):
-    )
+    """
-
+    Populate monthly billing table for all services for a given year.
-    def run(self, year):
+    """
-        service_ids = get_service_ids_that_need_billing_populated(
+    def populate(service_id, year, month):
-            start_date=datetime(2016, 5, 1), end_date=datetime(2017, 8, 16)
-        )
-        start, end = 1, 13
-        if year == '2016':
-            start = 4
-
-        for service_id in service_ids:
-            print('Starting to populate data for service {}'.format(str(service_id)))
-            print('Starting populating monthly billing for {}'.format(year))
-            for i in range(start, end):
-                print('Population for {}-{}'.format(i, year))
-                self.populate(service_id, year, i)
-
-    def populate(self, service_id, year, month):
         create_or_update_monthly_billing(service_id, datetime(int(year), int(month), 1))
         sms_res = get_monthly_billing_by_notification_type(
             service_id, datetime(int(year), int(month), 1), SMS_TYPE
@@ -200,165 +220,202 @@ class PopulateMonthlyBilling(Command):
         print('SMS: {}'.format(sms_res.monthly_totals))
         print('Email: {}'.format(email_res.monthly_totals))
 
-
+    service_ids = get_service_ids_that_need_billing_populated(
-class BackfillProcessingTime(Command):
+        start_date=datetime(2016, 5, 1), end_date=datetime(2017, 8, 16)
-    option_list = (
-        Option('-s', '--start_date', dest='start_date', help="Date (%Y-%m-%d) start date inclusive"),
-        Option('-e', '--end_date', dest='end_date', help="Date (%Y-%m-%d) end date inclusive"),
     )
+    start, end = 1, 13
 
-    def run(self, start_date, end_date):
+    if year == '2016':
-        start_date = datetime.strptime(start_date, '%Y-%m-%d')
+        start = 4
-        end_date = datetime.strptime(end_date, '%Y-%m-%d')
 
-        delta = end_date - start_date
+    for service_id in service_ids:
-
+        print('Starting to populate data for service {}'.format(str(service_id)))
-        print('Sending notification processing-time data for all days between {} and {}'.format(start_date, end_date))
+        print('Starting populating monthly billing for {}'.format(year))
-
+        for i in range(start, end):
-        for i in range(delta.days + 1):
+            print('Population for {}-{}'.format(i, year))
-            # because the tz conversion funcs talk about midnight, and the midnight before last,
+            populate(service_id, year, i)
-            # we want to pretend we're running this from the next morning, so add one.
-            process_date = start_date + timedelta(days=i + 1)
-
-            process_start_date = get_midnight_for_day_before(process_date)
-            process_end_date = get_london_midnight_in_utc(process_date)
-
-            print('Sending notification processing-time for {} - {}'.format(
-                process_start_date.isoformat(),
-                process_end_date.isoformat()
-            ))
-            send_processing_time_for_start_and_end(process_start_date, process_end_date)
 
 
-class PopulateServiceEmailReplyTo(Command):
+@notify_command()
+@click.option('-s', '--start_date', required=True, help="Date (%Y-%m-%d) start date inclusive")
+@click.option('-e', '--end_date', required=True, help="Date (%Y-%m-%d) end date inclusive")
+def backfill_processing_time(start_date, end_date):
+    """
+    Send historical performance platform stats.
+    """
+    start_date = datetime.strptime(start_date, '%Y-%m-%d')
+    end_date = datetime.strptime(end_date, '%Y-%m-%d')
 
-    def run(self):
+    delta = end_date - start_date
-        services_to_update = """
-            INSERT INTO service_email_reply_to(id, service_id, email_address, is_default, created_at)
-            SELECT uuid_in(md5(random()::text || now()::text)::cstring), id, reply_to_email_address, true, '{}'
-            FROM services
-            WHERE reply_to_email_address IS NOT NULL
-            AND id NOT IN(
-                SELECT service_id
-                FROM service_email_reply_to
-            )
-        """.format(datetime.utcnow())
 
-        result = db.session.execute(services_to_update)
+    print('Sending notification processing-time data for all days between {} and {}'.format(start_date, end_date))
-        db.session.commit()
 
-        print("Populated email reply to addresses for {}".format(result.rowcount))
+    for i in range(delta.days + 1):
+        # because the tz conversion funcs talk about midnight, and the midnight before last,
+        # we want to pretend we're running this from the next morning, so add one.
+        process_date = start_date + timedelta(days=i + 1)
+
+        process_start_date = get_midnight_for_day_before(process_date)
+        process_end_date = get_london_midnight_in_utc(process_date)
+
+        print('Sending notification processing-time for {} - {}'.format(
+            process_start_date.isoformat(),
+            process_end_date.isoformat()
+        ))
+        send_processing_time_for_start_and_end(process_start_date, process_end_date)
 
 
-class PopulateServiceSmsSender(Command):
+@notify_command()
+def populate_service_email_reply_to():
+    """
+    Migrate reply to emails.
+    """
+    services_to_update = """
+        INSERT INTO service_email_reply_to(id, service_id, email_address, is_default, created_at)
+        SELECT uuid_in(md5(random()::text || now()::text)::cstring), id, reply_to_email_address, true, '{}'
+        FROM services
+        WHERE reply_to_email_address IS NOT NULL
+        AND id NOT IN(
+            SELECT service_id
+            FROM service_email_reply_to
+        )
+    """.format(datetime.utcnow())
 
-    def run(self):
+    result = db.session.execute(services_to_update)
-        services_to_update = """
+    db.session.commit()
-            INSERT INTO service_sms_senders(id, service_id, sms_sender, inbound_number_id, is_default, created_at)
-            SELECT uuid_in(md5(random()::text || now()::text)::cstring), service_id, number, id, true, '{}'
-            FROM inbound_numbers
-            WHERE service_id NOT IN(
-                SELECT service_id
-                FROM service_sms_senders
-            )
-        """.format(datetime.utcnow())
 
-        services_to_update_from_services = """
+    print("Populated email reply to addresses for {}".format(result.rowcount))
-            INSERT INTO service_sms_senders(id, service_id, sms_sender, inbound_number_id, is_default, created_at)
+
-            SELECT uuid_in(md5(random()::text || now()::text)::cstring), id, sms_sender, null, true, '{}'
+
+@notify_command()
+def populate_service_sms_sender():
+    """
+    Migrate sms senders. Must be called when working on a fresh db!
+    """
+    services_to_update = """
+        INSERT INTO service_sms_senders(id, service_id, sms_sender, inbound_number_id, is_default, created_at)
+        SELECT uuid_in(md5(random()::text || now()::text)::cstring), service_id, number, id, true, '{}'
+        FROM inbound_numbers
+        WHERE service_id NOT IN(
+            SELECT service_id
+            FROM service_sms_senders
+        )
+    """.format(datetime.utcnow())
+
+    services_to_update_from_services = """
+        INSERT INTO service_sms_senders(id, service_id, sms_sender, inbound_number_id, is_default, created_at)
+        SELECT uuid_in(md5(random()::text || now()::text)::cstring), id, sms_sender, null, true, '{}'
+        FROM services
+        WHERE id NOT IN(
+            SELECT service_id
+            FROM service_sms_senders
+        )
+    """.format(datetime.utcnow())
+
+    result = db.session.execute(services_to_update)
+    second_result = db.session.execute(services_to_update_from_services)
+    db.session.commit()
+
+    services_count_query = db.session.execute("Select count(*) from services").fetchall()[0][0]
+
+    service_sms_sender_count_query = db.session.execute("Select count(*) from service_sms_senders").fetchall()[0][0]
+
+    print("Populated sms sender {} services from inbound_numbers".format(result.rowcount))
+    print("Populated sms sender {} services from services".format(second_result.rowcount))
+    print("{} services in table".format(services_count_query))
+    print("{} service_sms_senders".format(service_sms_sender_count_query))
+
+
+@notify_command()
+def populate_service_letter_contact():
+    """
+    Migrates letter contact blocks.
+    """
+    services_to_update = """
+        INSERT INTO service_letter_contacts(id, service_id, contact_block, is_default, created_at)
+        SELECT uuid_in(md5(random()::text || now()::text)::cstring), id, letter_contact_block, true, '{}'
+        FROM services
+        WHERE letter_contact_block IS NOT NULL
+        AND id NOT IN(
+            SELECT service_id
+            FROM service_letter_contacts
+        )
+    """.format(datetime.utcnow())
+
+    result = db.session.execute(services_to_update)
+    db.session.commit()
+
+    print("Populated letter contacts for {} services".format(result.rowcount))
+
+
+@notify_command()
+def populate_service_and_service_history_free_sms_fragment_limit():
+    """
+    DEPRECATED. Set services to have 250k sms limit.
+    """
+    services_to_update = """
+        UPDATE services
+        SET free_sms_fragment_limit = 250000
+        WHERE free_sms_fragment_limit IS NULL
+    """
+
+    services_history_to_update = """
+        UPDATE services_history
+        SET free_sms_fragment_limit = 250000
+        WHERE free_sms_fragment_limit IS NULL
+    """
+
+    services_result = db.session.execute(services_to_update)
+    services_history_result = db.session.execute(services_history_to_update)
+
+    db.session.commit()
+
+    print("Populated free sms fragment limits for {} services".format(services_result.rowcount))
+    print("Populated free sms fragment limits for {} services history".format(services_history_result.rowcount))
+
+
+@notify_command()
+def populate_annual_billing():
+    """
+    add annual_billing for 2016, 2017 and 2018.
+    """
+    financial_year = [2016, 2017, 2018]
+
+    for fy in financial_year:
+        populate_data = """
+        INSERT INTO annual_billing(id, service_id, free_sms_fragment_limit, financial_year_start,
+                created_at, updated_at)
+            SELECT uuid_in(md5(random()::text || now()::text)::cstring), id, 250000, {}, '{}', '{}'
             FROM services
             WHERE id NOT IN(
                 SELECT service_id
-                FROM service_sms_senders
+                FROM annual_billing
-            )
+                WHERE financial_year_start={})
-        """.format(datetime.utcnow())
+        """.format(fy, datetime.utcnow(), datetime.utcnow(), fy)
 
-        result = db.session.execute(services_to_update)
+        services_result1 = db.session.execute(populate_data)
-        second_result = db.session.execute(services_to_update_from_services)
         db.session.commit()
 
-        services_count_query = db.session.execute("Select count(*) from services").fetchall()[0][0]
+        print("Populated annual billing {} for {} services".format(fy, services_result1.rowcount))
-
-        service_sms_sender_count_query = db.session.execute("Select count(*) from service_sms_senders").fetchall()[0][0]
-
-        print("Populated sms sender {} services from inbound_numbers".format(result.rowcount))
-        print("Populated sms sender {} services from services".format(second_result.rowcount))
-        print("{} services in table".format(services_count_query))
-        print("{} service_sms_senders".format(service_sms_sender_count_query))
 
 
-class PopulateServiceLetterContact(Command):
+@notify_command()
-
+@click.option('-j', '--job_id', required=True, help="Enter the job id to rebuild the dvla file for")
-    def run(self):
+def re_run_build_dvla_file_for_job(job_id):
-        services_to_update = """
+    """
-            INSERT INTO service_letter_contacts(id, service_id, contact_block, is_default, created_at)
+    Rebuild dvla file for a job.
-            SELECT uuid_in(md5(random()::text || now()::text)::cstring), id, letter_contact_block, true, '{}'
+    """
-            FROM services
+    from app.celery.tasks import build_dvla_file
-            WHERE letter_contact_block IS NOT NULL
+    from app.config import QueueNames
-            AND id NOT IN(
+    build_dvla_file.apply_async([job_id], queue=QueueNames.JOBS)
-                SELECT service_id
-                FROM service_letter_contacts
-            )
-        """.format(datetime.utcnow())
-
-        result = db.session.execute(services_to_update)
-        db.session.commit()
-
-        print("Populated letter contacts for {} services".format(result.rowcount))
 
 
-class PopulateServiceAndServiceHistoryFreeSmsFragmentLimit(Command):
+@notify_command(name='list-routes')
-
+def list_routes():
-    def run(self):
+    """List URLs of all application routes."""
-        services_to_update = """
+    for rule in sorted(current_app.url_map.iter_rules(), key=lambda r: r.rule):
-            UPDATE services
+        print("{:10} {}".format(", ".join(rule.methods - set(['OPTIONS', 'HEAD'])), rule.rule))
-            SET free_sms_fragment_limit = 250000
-            WHERE free_sms_fragment_limit IS NULL
-        """
-
-        services_history_to_update = """
-            UPDATE services_history
-            SET free_sms_fragment_limit = 250000
-            WHERE free_sms_fragment_limit IS NULL
-        """
-
-        services_result = db.session.execute(services_to_update)
-        services_history_result = db.session.execute(services_history_to_update)
-
-        db.session.commit()
-
-        print("Populated free sms fragment limits for {} services".format(services_result.rowcount))
-        print("Populated free sms fragment limits for {} services history".format(services_history_result.rowcount))
 
 
-class PopulateAnnualBilling(Command):
+def setup_commands(application):
-    def run(self):
+    application.cli.add_command(command_group)
-        financial_year = [2016, 2017, 2018]
-
-        for fy in financial_year:
-            populate_data = """
-            INSERT INTO annual_billing(id, service_id, free_sms_fragment_limit, financial_year_start,
-                    created_at, updated_at)
-                SELECT uuid_in(md5(random()::text || now()::text)::cstring), id, 250000, {}, '{}', '{}'
-                FROM services
-                WHERE id NOT IN(
-                    SELECT service_id
-                    FROM annual_billing
-                    WHERE financial_year_start={})
-            """.format(fy, datetime.utcnow(), datetime.utcnow(), fy)
-
-            services_result1 = db.session.execute(populate_data)
-            db.session.commit()
-
-            print("Populated annual billing {} for {} services".format(fy, services_result1.rowcount))
-
-
-class ReRunBuildDvlaFileForJob(Command):
-    option_list = (
-        Option('-j', '--job_id', dest='job_id', help="Enter the job id to rebuild the dvla file for"),
-    )
-
-    def run(self, job_id):
-        from app.celery.tasks import build_dvla_file
-        from app.config import QueueNames
-        build_dvla_file.apply_async([job_id], queue=QueueNames.JOBS)

app/config.py

@@ -389,6 +389,21 @@ class Staging(Config):
     API_RATE_LIMIT_ENABLED = True
     CHECK_PROXY_HEADER = True
 
+    API_KEY_LIMITS = {
+        KEY_TYPE_TEAM: {
+            "limit": 21000,
+            "interval": 60
+        },
+        KEY_TYPE_NORMAL: {
+            "limit": 21000,
+            "interval": 60
+        },
+        KEY_TYPE_TEST: {
+            "limit": 21000,
+            "interval": 60
+        }
+    }
+
 
 class Live(Config):
     NOTIFY_EMAIL_DOMAIN = 'notifications.service.gov.uk'

app/dao/templates_dao.py

@@ -1,7 +1,7 @@
 from datetime import datetime
 import uuid
 
-from sqlalchemy import desc
+from sqlalchemy import asc, desc
 from sqlalchemy.sql.expression import bindparam
 
 from app import db
@@ -65,14 +65,16 @@ def dao_get_all_templates_for_service(service_id, template_type=None):
             template_type=template_type,
             archived=False
         ).order_by(
-            desc(Template.created_at)
+            asc(Template.name),
+            asc(Template.template_type),
         ).all()
 
     return Template.query.filter_by(
         service_id=service_id,
         archived=False
     ).order_by(
-        desc(Template.created_at)
+        asc(Template.name),
+        asc(Template.template_type),
     ).all()
 
 

app/inbound_sms/inbound_sms_schemas.py

@@ -3,7 +3,7 @@ get_inbound_sms_for_service_schema = {
     "description": "schema for parameters allowed when searching for to field=",
     "type": "object",
     "properties": {
-        "phone_number": {"type": "string", "format": "phone_number"},
+        "phone_number": {"type": "string"},
         "limit": {"type": ["integer", "null"], "minimum": 1}
     }
 }

app/inbound_sms/rest.py

@@ -1,11 +1,10 @@
 from flask import (
     Blueprint,
     jsonify,
-    request,
+    request
-    current_app, json)
+)
-from jsonschema import ValidationError
 
-from notifications_utils.recipients import validate_and_format_phone_number
+from notifications_utils.recipients import try_validate_and_format_phone_number
 
 from app.dao.inbound_sms_dao import (
     dao_get_inbound_sms_for_service,
@@ -26,25 +25,31 @@ inbound_sms = Blueprint(
 register_errors(inbound_sms)
 
 
-@inbound_sms.route('', methods=['POST', 'GET'])
+@inbound_sms.route('', methods=['POST'])
-def get_inbound_sms_for_service(service_id):
+def post_query_inbound_sms_for_service(service_id):
-
+    form = validate(request.get_json(), get_inbound_sms_for_service_schema)
-    if request.method == 'GET':
+    if 'phone_number' in form:
-        limit = request.args.get('limit')
+        # we use this to normalise to an international phone number - but this may fail if it's an alphanumeric
-        user_number = request.args.get('user_number')
+        user_number = try_validate_and_format_phone_number(form['phone_number'], international=True)
-
-        if user_number:
-            # we use this to normalise to an international phone number
-            user_number = validate_and_format_phone_number(user_number, international=True)
-
-        results = dao_get_inbound_sms_for_service(service_id, limit, user_number)
-
-        return jsonify(data=[row.serialize() for row in results])
     else:
-        form = validate(request.get_json(), get_inbound_sms_for_service_schema)
+        user_number = None
-        results = dao_get_inbound_sms_for_service(service_id, form.get('limit'), form.get('phone_number'))
+    results = dao_get_inbound_sms_for_service(service_id, form.get('limit'), user_number)
 
-        return jsonify(data=[row.serialize() for row in results])
+    return jsonify(data=[row.serialize() for row in results])
+
+
+@inbound_sms.route('', methods=['GET'])
+def get_inbound_sms_for_service(service_id):
+    limit = request.args.get('limit')
+    user_number = request.args.get('user_number')
+
+    if user_number:
+        # we use this to normalise to an international phone number - but this may fail if it's an alphanumeric
+        user_number = try_validate_and_format_phone_number(user_number, international=True)
+
+    results = dao_get_inbound_sms_for_service(service_id, limit, user_number)
+
+    return jsonify(data=[row.serialize() for row in results])
 
 
 @inbound_sms.route('/summary')

app/notifications/receive_notifications.py

@@ -2,7 +2,7 @@ from urllib.parse import unquote
 
 import iso8601
 from flask import jsonify, Blueprint, current_app, request, abort
-from notifications_utils.recipients import validate_and_format_phone_number
+from notifications_utils.recipients import try_validate_and_format_phone_number
 
 from app import statsd_client, firetext_client, mmg_client
 from app.celery import tasks
@@ -109,7 +109,11 @@ def format_mmg_datetime(date):
 
 
 def create_inbound_sms_object(service, content, from_number, provider_ref, date_received, provider_name):
-    user_number = validate_and_format_phone_number(from_number, international=True)
+    user_number = try_validate_and_format_phone_number(
+        from_number,
+        international=True,
+        log_msg='Invalid from_number received'
+    )
 
     provider_date = date_received
     if provider_date:

application.py

@@ -1,38 +1,10 @@
-#!/usr/bin/env python
+##!/usr/bin/env python
-
 from __future__ import print_function
-import os
-from flask_script import Manager, Server
-from flask_migrate import Migrate, MigrateCommand
-from app import (create_app, db, commands)
 
-application = create_app()
+from flask import Flask
-manager = Manager(application)
-port = int(os.environ.get('PORT', 6011))
-manager.add_command("runserver", Server(host='0.0.0.0', port=port))
 
-migrate = Migrate(application, db)
+from app import create_app
-manager.add_command('db', MigrateCommand)
-manager.add_command('create_provider_rate', commands.CreateProviderRateCommand)
-manager.add_command('purge_functional_test_data', commands.PurgeFunctionalTestDataCommand)
-manager.add_command('custom_db_script', commands.CustomDbScript)
-manager.add_command('populate_monthly_billing', commands.PopulateMonthlyBilling)
-manager.add_command('backfill_processing_time', commands.BackfillProcessingTime)
-manager.add_command('populate_service_email_reply_to', commands.PopulateServiceEmailReplyTo)
-manager.add_command('populate_service_sms_sender', commands.PopulateServiceSmsSender)
-manager.add_command('populate_service_letter_contact', commands.PopulateServiceLetterContact)
-manager.add_command('populate_service_and_service_history_free_sms_fragment_limit',
-                    commands.PopulateServiceAndServiceHistoryFreeSmsFragmentLimit)
-manager.add_command('populate_annual_billing', commands.PopulateAnnualBilling)
-manager.add_command('rerun_build_dvla_file', commands.ReRunBuildDvlaFileForJob)
 
+application = Flask('app')
 
-@manager.command
+create_app(application)
-def list_routes():
-    """List URLs of all application routes."""
-    for rule in sorted(application.url_map.iter_rules(), key=lambda r: r.rule):
-        print("{:10} {}".format(", ".join(rule.methods - set(['OPTIONS', 'HEAD'])), rule.rule))
-
-
-if __name__ == '__main__':
-    manager.run()

manifest-api-base.yml

@@ -1,7 +1,7 @@
 ---
 
 buildpack: python_buildpack
-command: scripts/run_app_paas.sh gunicorn -c /home/vcap/app/gunicorn_config.py --error-logfile /home/vcap/logs/gunicorn_error.log -w 5 -b 0.0.0.0:$PORT wsgi
+command: scripts/run_app_paas.sh gunicorn -c /home/vcap/app/gunicorn_config.py --error-logfile /home/vcap/logs/gunicorn_error.log -w 5 -b 0.0.0.0:$PORT application
 services:
   - notify-aws
   - notify-config
@@ -14,6 +14,8 @@ services:
 env:
   NOTIFY_APP_NAME: public-api
   CW_APP_NAME: api
+  # required by cf run-task
+  FLASK_APP: application.py
 instances: 1
 memory: 1G
 

migrations/README

@@ -1,9 +0,0 @@
-Generic single-database configuration.
-
-python application.py db migrate to generate migration script.
-
-python application.py db upgrade to upgrade db with script.
-
-python application.py db downgrade to rollback db changes.
-
-python application.py db current to show current script.

migrations/README.md

@@ -0,0 +1,9 @@
+Generic single-database configuration.
+
+flask db migrate to generate migration script.
+
+flask db upgrade to upgrade db with script.
+
+flask db downgrade to rollback db changes.
+
+flask db current to show current script.

requirements.txt

@@ -5,7 +5,6 @@ docopt==0.6.2
 Flask-Bcrypt==0.7.1
 Flask-Marshmallow==0.8.0
 Flask-Migrate==2.1.1
-Flask-Script==2.0.5
 Flask-SQLAlchemy==2.3.2
 Flask==0.12.2
 gunicorn==19.7.1
@@ -26,6 +25,6 @@ notifications-python-client==4.6.0
 awscli>=1.11,<1.12
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@23.0.1#egg=notifications-utils==23.0.1
+git+https://github.com/alphagov/notifications-utils.git@23.1.0#egg=notifications-utils==23.1.0
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3

run_celery.py

@@ -1,6 +1,10 @@
 #!/usr/bin/env python
 # notify_celery is referenced from manifest_delivery_base.yml, and cannot be removed
+from flask import Flask
+
 from app import notify_celery, create_app
 
-application = create_app('delivery')
+
+application = Flask('delivery')
+create_app(application)
 application.app_context().push()

scripts/bootstrap.sh

@@ -36,4 +36,4 @@ createdb notification_api
 
 # Upgrade databases
 source environment.sh
-python application.py db upgrade
+flask db upgrade

scripts/check_if_new_migration.py

@@ -8,7 +8,7 @@ def get_latest_db_migration_to_apply():
     project_dir = dirname(dirname(abspath(__file__)))  # Get the main project directory
     migrations_dir = '{}/migrations/versions/'.format(project_dir)
     migration_files = [migration_file for migration_file in os.listdir(migrations_dir) if migration_file.endswith('py')]
-    # sometimes there's a trailing underscore, if script was created with `python app.py db migrate --rev-id=...`
+    # sometimes there's a trailing underscore, if script was created with `flask db migrate --rev-id=...`
     latest_file = sorted(migration_files, reverse=True)[0].replace('_.py', '').replace('.py', '')
     return latest_file
 

scripts/fix_migrations.py

@@ -0,0 +1,87 @@
+#!/usr/bin/env python
+# encoding: utf-8
+
+import os
+import sys
+
+from alembic.script import ScriptDirectory
+
+sys.path.append('.')
+
+
+def get_branch_points(migrations):
+    return [m for m in migrations.walk_revisions() if m.is_branch_point]
+
+
+def get_branches(migrations, branch_point, heads):
+    return [list(migrations.iterate_revisions(m, branch_point.revision))[::-1]
+            for m in heads]
+
+
+def choice(prompt, options, option_fmt=lambda x: x):
+    print("{}:\n".format(prompt))
+    for i, option in enumerate(options):
+        print("{}. {}".format(i + 1, option_fmt(option)))
+
+    print()
+    choice = input("Option> ")
+
+    return options[int(choice) - 1]
+
+
+def rename_revision(current_revision, new_base):
+    new_id = int(new_base[:4]) + 1
+    return "{:04d}{}".format(new_id, current_revision[4:])
+
+
+def reorder_revisions(revisions, old_base, new_base):
+    if not revisions:
+        return
+
+    head, *tail = revisions
+    new_revision_id = rename_revision(head.revision, new_base)
+
+    print("Moving {} to {}".format(head.revision, new_revision_id))
+    with open(head.path, 'r') as rev_file:
+        file_data = rev_file.read()
+
+    file_data = file_data.replace(head.revision, new_revision_id).replace(old_base, new_base)
+
+    with open(head.path.replace(head.revision, new_revision_id), 'w') as rev_file:
+        rev_file.write(file_data)
+
+    print("Removing {}".format(head.path))
+    os.remove(head.path)
+
+    reorder_revisions(tail, head.revision, new_revision_id)
+
+
+def fix_branch_point(migrations, branch_point, heads):
+    print("Migrations directory has a branch point at {}".format(branch_point.revision))
+
+    branches = get_branches(migrations, branch_point, heads)
+    move_branch = choice("Select migrations to move", branches,
+                         lambda x: " -> ".join(m.revision for m in x))
+    branches.remove(move_branch)
+
+    reorder_revisions(move_branch, branch_point.revision, branches[0][-1].revision)
+
+
+def main(migrations_path):
+    migrations = ScriptDirectory(migrations_path)
+
+    branch_points = get_branch_points(migrations)
+    heads = migrations.get_heads()
+
+    if not branch_points:
+        print("Migrations are ordered")
+    elif len(branch_points) == 1 and len(heads) == 2:
+        fix_branch_point(migrations, branch_points[0], heads)
+    else:
+        print("Found {} branch points and {} heads, can't fix automatically".format(
+            [bp.revision for bp in branch_points], heads))
+        sys.exit(1)
+
+
+if __name__ == '__main__':
+    main('migrations/')

scripts/run_app.sh

@@ -3,4 +3,4 @@
 set -e
 
 source environment.sh
-python3 application.py runserver
+flask run -p 6011

server_commands.py

@@ -1,26 +0,0 @@
-from flask_script import Manager, Server
-from flask_migrate import Migrate, MigrateCommand
-from app import (create_app, db, commands)
-import os
-
-default_env_file = '/home/ubuntu/environment'
-environment = 'live'
-
-if os.path.isfile(default_env_file):
-    with open(default_env_file, 'r') as environment_file:
-        environment = environment_file.readline().strip()
-
-from app.config import configs
-
-os.environ['NOTIFY_API_ENVIRONMENT'] = configs[environment]
-
-application = create_app()
-
-manager = Manager(application)
-migrate = Migrate(application, db)
-manager.add_command('db', MigrateCommand)
-manager.add_command('purge_functional_test_data', commands.PurgeFunctionalTestDataCommand)
-manager.add_command('custom_db_script', commands.CustomDbScript)
-
-if __name__ == '__main__':
-    manager.run()

tests/app/dao/test_templates_dao.py

@@ -163,7 +163,7 @@ def test_get_all_templates_for_service(notify_db, notify_db_session, service_fac
     assert len(dao_get_all_templates_for_service(service_2.id)) == 2
 
 
-def test_get_all_templates_for_service_shows_newest_created_first(notify_db, notify_db_session, sample_service):
+def test_get_all_templates_for_service_is_alphabetised(notify_db, notify_db_session, sample_service):
     template_1 = create_sample_template(
         notify_db,
         notify_db_session,
@@ -190,14 +190,14 @@ def test_get_all_templates_for_service_shows_newest_created_first(notify_db, not
     )
 
     assert Template.query.count() == 3
-    assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template 3'
+    assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template 1'
     assert dao_get_all_templates_for_service(sample_service.id)[1].name == 'Sample Template 2'
-    assert dao_get_all_templates_for_service(sample_service.id)[2].name == 'Sample Template 1'
+    assert dao_get_all_templates_for_service(sample_service.id)[2].name == 'Sample Template 3'
 
-    template_2.name = 'Sample Template 2 (updated)'
+    template_2.name = 'AAAAA Sample Template 2'
     dao_update_template(template_2)
-    assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'Sample Template 3'
+    assert dao_get_all_templates_for_service(sample_service.id)[0].name == 'AAAAA Sample Template 2'
-    assert dao_get_all_templates_for_service(sample_service.id)[1].name == 'Sample Template 2 (updated)'
+    assert dao_get_all_templates_for_service(sample_service.id)[1].name == 'Sample Template 1'
 
 
 def test_get_all_returns_empty_list_if_no_templates(sample_service):

tests/app/db.py

@@ -121,12 +121,13 @@ def create_service_with_defined_sms_sender(
 def create_template(
     service,
     template_type=SMS_TYPE,
+    template_name=None,
     subject='Template subject',
     content='Dear Sir/Madam, Hello. Yours Truly, The Government.',
     template_id=None
 ):
     data = {
-        'name': '{} Template Name'.format(template_type),
+        'name': template_name or '{} Template Name'.format(template_type),
         'template_type': template_type,
         'content': content,
         'service': service,

tests/app/inbound_sms/test_rest.py

@@ -1,28 +1,20 @@
 from datetime import datetime
 
 import pytest
-from flask import json
 from freezegun import freeze_time
 
-from tests import create_authorization_header
 from tests.app.db import create_inbound_sms, create_service, create_service_with_inbound_number
 
 
-def test_get_inbound_sms_with_no_params(client, sample_service):
+def test_post_to_get_inbound_sms_with_no_params(admin_request, sample_service):
     one = create_inbound_sms(sample_service)
     two = create_inbound_sms(sample_service)
 
-    auth_header = create_authorization_header()
+    sms = admin_request.post(
-
+        'inbound_sms.post_query_inbound_sms_for_service',
-    data = {}
+        service_id=sample_service.id,
-
+        _data={}
-    response = client.post(
+    )['data']
-        path='/service/{}/inbound-sms'.format(sample_service.id),
-        data=json.dumps(data),
-        headers=[('Content-Type', 'application/json'), auth_header])
-
-    json_resp = json.loads(response.get_data(as_text=True))
-    sms = json_resp['data']
 
     assert len(sms) == 2
     assert {inbound['id'] for inbound in sms} == {str(one.id), str(two.id)}
@@ -37,40 +29,34 @@ def test_get_inbound_sms_with_no_params(client, sample_service):
     }
 
 
-def test_get_inbound_sms_with_limit(client, sample_service):
+def test_post_to_get_inbound_sms_with_limit(admin_request, sample_service):
     with freeze_time('2017-01-01'):
         one = create_inbound_sms(sample_service)
     with freeze_time('2017-01-02'):
         two = create_inbound_sms(sample_service)
 
-    auth_header = create_authorization_header()
-
     data = {'limit': 1}
 
-    response = client.post(
+    sms = admin_request.post(
-        path='/service/{}/inbound-sms'.format(sample_service.id),
+        'inbound_sms.post_query_inbound_sms_for_service',
-        data=json.dumps(data),
+        service_id=sample_service.id,
-        headers=[('Content-Type', 'application/json'), auth_header])
+        _data=data
-
+    )['data']
-    json_resp = json.loads(response.get_data(as_text=True))
-    sms = json_resp['data']
 
     assert len(sms) == 1
     assert sms[0]['id'] == str(two.id)
 
 
-def test_get_inbound_sms_should_error_with_invalid_limit(client, sample_service):
+def test_post_to_get_inbound_sms_should_error_with_invalid_limit(admin_request, sample_service):
-
-    auth_header = create_authorization_header()
-
     data = {'limit': 'limit'}
 
-    response = client.post(
+    error_resp = admin_request.post(
-        path='/service/{}/inbound-sms'.format(sample_service.id),
+        'inbound_sms.post_query_inbound_sms_for_service',
-        data=json.dumps(data),
+        service_id=sample_service.id,
-        headers=[('Content-Type', 'application/json'), auth_header])
+        _data=data,
+        _expected_status=400
+    )
 
-    error_resp = json.loads(response.get_data(as_text=True))
     assert error_resp['status_code'] == 400
     assert error_resp['errors'] == [{
         'error': 'ValidationError',
@@ -78,73 +64,61 @@ def test_get_inbound_sms_should_error_with_invalid_limit(client, sample_service)
     }]
 
 
-def test_get_inbound_sms_should_error_with_invalid_phone_number(client, sample_service):
-
-    auth_header = create_authorization_header()
-
-    data = {'phone_number': 'invalid phone number'}
-
-    response = client.post(
-        path='/service/{}/inbound-sms'.format(sample_service.id),
-        data=json.dumps(data),
-        headers=[('Content-Type', 'application/json'), auth_header])
-
-    error_resp = json.loads(response.get_data(as_text=True))
-    assert error_resp['status_code'] == 400
-    assert error_resp['errors'] == [{
-        'error': 'ValidationError',
-        'message': "phone_number Must not contain letters or symbols"
-    }]
-
-
 @pytest.mark.parametrize('user_number', [
     '(07700) 900-001',
     '+4407700900001',
     '447700900001',
 ])
-def test_get_inbound_sms_filters_user_number(client, sample_service, user_number):
+def test_post_to_get_inbound_sms_filters_user_number(admin_request, sample_service, user_number):
     # user_number in the db is international and normalised
     one = create_inbound_sms(sample_service, user_number='447700900001')
     two = create_inbound_sms(sample_service, user_number='447700900002')
 
-    auth_header = create_authorization_header()
-
     data = {
         'limit': 1,
         'phone_number': user_number
     }
 
-    response = client.post(
+    sms = admin_request.post(
-        path='/service/{}/inbound-sms'.format(sample_service.id),
+        'inbound_sms.post_query_inbound_sms_for_service',
-        data=json.dumps(data),
+        service_id=sample_service.id,
-        headers=[('Content-Type', 'application/json'), auth_header])
+        _data=data
+    )['data']
 
-    json_resp = json.loads(response.get_data(as_text=True))
-    sms = json_resp['data']
     assert len(sms) == 1
     assert sms[0]['id'] == str(one.id)
     assert sms[0]['user_number'] == str(one.user_number)
 
 
-def test_get_inbound_sms_filters_international_user_number(admin_request, sample_service):
+def test_post_to_get_inbound_sms_filters_international_user_number(admin_request, sample_service):
     # user_number in the db is international and normalised
     one = create_inbound_sms(sample_service, user_number='12025550104')
     two = create_inbound_sms(sample_service)
 
-    auth_header = create_authorization_header()
-
     data = {
         'limit': 1,
         'phone_number': '+1 (202) 555-0104'
     }
 
-    response = client.post(
+    sms = admin_request.post(
-        path='/service/{}/inbound-sms'.format(sample_service.id),
+        'inbound_sms.post_query_inbound_sms_for_service',
-        data=json.dumps(data),
+        service_id=sample_service.id,
-        headers=[('Content-Type', 'application/json'), auth_header])
+        _data=data
+    )['data']
 
-    json_resp = json.loads(response.get_data(as_text=True))
+    assert len(sms) == 1
-    sms = json_resp['data']
+    assert sms[0]['id'] == str(one.id)
+    assert sms[0]['user_number'] == str(one.user_number)
+
+
+def test_post_to_get_inbound_sms_allows_badly_formatted_number(admin_request, sample_service):
+    one = create_inbound_sms(sample_service, user_number='ALPHANUM3R1C')
+
+    sms = admin_request.post(
+        'inbound_sms.get_inbound_sms_for_service',
+        service_id=sample_service.id,
+        _data={'phone_number': 'ALPHANUM3R1C'}
+    )['data']
 
     assert len(sms) == 1
     assert sms[0]['id'] == str(one.id)
@@ -156,7 +130,7 @@ def test_get_inbound_sms_filters_international_user_number(admin_request, sample
 ##############################################################
 
 
-def test_get_inbound_sms(admin_request, sample_service):
+def test_old_get_inbound_sms(admin_request, sample_service):
     one = create_inbound_sms(sample_service)
     two = create_inbound_sms(sample_service)
 
@@ -180,7 +154,7 @@ def test_get_inbound_sms(admin_request, sample_service):
     }
 
 
-def test_get_inbound_sms_limits(admin_request, sample_service):
+def test_old_get_inbound_sms_limits(admin_request, sample_service):
     with freeze_time('2017-01-01'):
         one = create_inbound_sms(sample_service)
     with freeze_time('2017-01-02'):
@@ -201,7 +175,7 @@ def test_get_inbound_sms_limits(admin_request, sample_service):
     '+4407700900001',
     '447700900001',
 ])
-def test_get_inbound_sms_filters_user_number(admin_request, sample_service, user_number):
+def test_old_get_inbound_sms_filters_user_number(admin_request, sample_service, user_number):
     # user_number in the db is international and normalised
     one = create_inbound_sms(sample_service, user_number='447700900001')
     two = create_inbound_sms(sample_service, user_number='447700900002')
@@ -217,7 +191,7 @@ def test_get_inbound_sms_filters_user_number(admin_request, sample_service, user
     assert sms['data'][0]['user_number'] == str(one.user_number)
 
 
-def test_get_inbound_sms_filters_international_user_number(admin_request, sample_service):
+def test_old_get_inbound_sms_filters_international_user_number(admin_request, sample_service):
     # user_number in the db is international and normalised
     one = create_inbound_sms(sample_service, user_number='12025550104')
     two = create_inbound_sms(sample_service)
@@ -233,6 +207,20 @@ def test_get_inbound_sms_filters_international_user_number(admin_request, sample
     assert sms['data'][0]['user_number'] == str(one.user_number)
 
 
+def test_old_get_inbound_sms_allows_badly_formatted_number(admin_request, sample_service):
+    one = create_inbound_sms(sample_service, user_number='ALPHANUM3R1C')
+
+    sms = admin_request.get(
+        'inbound_sms.get_inbound_sms_for_service',
+        service_id=sample_service.id,
+        user_number='ALPHANUM3R1C',
+    )
+
+    assert len(sms['data']) == 1
+    assert sms['data'][0]['id'] == str(one.id)
+    assert sms['data'][0]['user_number'] == str(one.user_number)
+
+
 ##############################
 # End delete section
 ##############################

tests/app/notifications/test_receive_notification.py

@@ -413,3 +413,24 @@ def test_firetext_inbound_sms_auth(notify_db_session, notify_api, client, mocker
     with set_config(notify_api, 'FIRETEXT_INBOUND_SMS_AUTH', keys):
         response = firetext_post(client, data, auth=bool(auth), password=auth)
         assert response.status_code == status_code
+
+
+def test_create_inbound_sms_object_works_with_alphanumeric_sender(sample_service_full_permissions):
+    data = {
+        'Message': 'hello',
+        'Number': sample_service_full_permissions.get_inbound_number(),
+        'MSISDN': 'ALPHANUM3R1C',
+        'DateRecieved': '2017-01-02+03%3A04%3A05',
+        'ID': 'bar',
+    }
+
+    inbound_sms = create_inbound_sms_object(
+        service=sample_service_full_permissions,
+        content=format_mmg_message(data["Message"]),
+        from_number='ALPHANUM3R1C',
+        provider_ref='foo',
+        date_received=None,
+        provider_name="mmg"
+    )
+
+    assert inbound_sms.user_number == 'ALPHANUM3R1C'

tests/app/template/test_rest.py

@@ -301,10 +301,10 @@ def test_should_be_able_to_get_all_templates_for_a_service(client, sample_user,
 
     assert response.status_code == 200
     update_json_resp = json.loads(response.get_data(as_text=True))
-    assert update_json_resp['data'][0]['name'] == 'my template 2'
+    assert update_json_resp['data'][0]['name'] == 'my template 1'
     assert update_json_resp['data'][0]['version'] == 1
     assert update_json_resp['data'][0]['created_at']
-    assert update_json_resp['data'][1]['name'] == 'my template 1'
+    assert update_json_resp['data'][1]['name'] == 'my template 2'
     assert update_json_resp['data'][1]['version'] == 1
     assert update_json_resp['data'][1]['created_at']
 

tests/app/test_commands.py

@@ -1,12 +1,14 @@
 from datetime import datetime
 
-from app.commands import BackfillProcessingTime
+from app.commands import backfill_processing_time
 
 
-def test_backfill_processing_time_works_for_correct_dates(mocker):
+def test_backfill_processing_time_works_for_correct_dates(mocker, notify_api):
     send_mock = mocker.patch('app.commands.send_processing_time_for_start_and_end')
 
-    BackfillProcessingTime().run('2017-08-01', '2017-08-03')
+    # backfill_processing_time is a click.Command object - if you try invoking the callback on its own, it
+    # throws a `RuntimeError: There is no active click context.` - so get at the original function using __wrapped__
+    backfill_processing_time.callback.__wrapped__('2017-08-01', '2017-08-03')
 
     assert send_mock.call_count == 3
     send_mock.assert_any_call(datetime(2017, 7, 31, 23, 0), datetime(2017, 8, 1, 23, 0))

tests/app/v2/templates/test_get_templates.py

@@ -1,6 +1,7 @@
 import pytest
 
 from flask import json
+from itertools import product
 
 from app.models import TEMPLATE_TYPES, EMAIL_TYPE
 from tests import create_authorization_header
@@ -8,12 +9,15 @@ from tests.app.db import create_template
 
 
 def test_get_all_templates_returns_200(client, sample_service):
-    num_templates = 3
+    templates = [
-    templates = []
+        create_template(
-    for i in range(num_templates):
+            sample_service,
-        for tmp_type in TEMPLATE_TYPES:
+            template_type=tmp_type,
-            subject = 'subject_{}'.format(i) if tmp_type == EMAIL_TYPE else ''
+            subject='subject_{}'.format(name) if tmp_type == EMAIL_TYPE else '',
-            templates.append(create_template(sample_service, template_type=tmp_type, subject=subject))
+            template_name=name,
+        )
+        for name, tmp_type in product(('A', 'B', 'C'), TEMPLATE_TYPES)
+    ]
 
     auth_header = create_authorization_header(service_id=sample_service.id)
 
@@ -25,25 +29,27 @@ def test_get_all_templates_returns_200(client, sample_service):
 
     json_response = json.loads(response.get_data(as_text=True))
 
-    assert len(json_response['templates']) == num_templates * len(TEMPLATE_TYPES)
+    assert len(json_response['templates']) == len(templates)
 
-    # need to reverse index as get all templates returns list sorted by descending date
+    for index, template in enumerate(json_response['templates']):
-    for i in range(len(json_response['templates'])):
+        assert template['id'] == str(templates[index].id)
-        reverse_index = len(json_response['templates']) - 1 - i
+        assert template['body'] == templates[index].content
-        assert json_response['templates'][reverse_index]['id'] == str(templates[i].id)
+        assert template['type'] == templates[index].template_type
-        assert json_response['templates'][reverse_index]['body'] == templates[i].content
+        if templates[index].template_type == EMAIL_TYPE:
-        assert json_response['templates'][reverse_index]['type'] == templates[i].template_type
+            assert template['subject'] == templates[index].subject
-        if templates[i].template_type == EMAIL_TYPE:
-            assert json_response['templates'][reverse_index]['subject'] == templates[i].subject
 
 
 @pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES)
 def test_get_all_templates_for_valid_type_returns_200(client, sample_service, tmp_type):
-    num_templates = 3
+    templates = [
-    templates = []
+        create_template(
-    for i in range(num_templates):
+            sample_service,
-        subject = 'subject_{}'.format(i) if tmp_type == EMAIL_TYPE else ''
+            template_type=tmp_type,
-        templates.append(create_template(sample_service, template_type=tmp_type, subject=subject))
+            template_name='Template {}'.format(i),
+            subject='subject_{}'.format(i) if tmp_type == EMAIL_TYPE else ''
+        )
+        for i in range(3)
+    ]
 
     auth_header = create_authorization_header(service_id=sample_service.id)
 
@@ -55,16 +61,14 @@ def test_get_all_templates_for_valid_type_returns_200(client, sample_service, tm
 
     json_response = json.loads(response.get_data(as_text=True))
 
-    assert len(json_response['templates']) == num_templates
+    assert len(json_response['templates']) == len(templates)
 
-    # need to reverse index as get all templates returns list sorted by descending date
+    for index, template in enumerate(json_response['templates']):
-    for i in range(len(json_response['templates'])):
+        assert template['id'] == str(templates[index].id)
-        reverse_index = len(json_response['templates']) - 1 - i
+        assert template['body'] == templates[index].content
-        assert json_response['templates'][reverse_index]['id'] == str(templates[i].id)
+        assert template['type'] == tmp_type
-        assert json_response['templates'][reverse_index]['body'] == templates[i].content
+        if templates[index].template_type == EMAIL_TYPE:
-        assert json_response['templates'][reverse_index]['type'] == tmp_type
+            assert template['subject'] == templates[index].subject
-        if templates[i].template_type == EMAIL_TYPE:
-            assert json_response['templates'][reverse_index]['subject'] == templates[i].subject
 
 
 @pytest.mark.parametrize("tmp_type", TEMPLATE_TYPES)

tests/conftest.py

@@ -1,10 +1,9 @@
 from contextlib import contextmanager
 import os
 
+from flask import Flask
 from alembic.command import upgrade
 from alembic.config import Config
-from flask_migrate import Migrate, MigrateCommand
-from flask_script import Manager
 import boto3
 import pytest
 import sqlalchemy
@@ -14,7 +13,8 @@ from app import create_app, db
 
 @pytest.fixture(scope='session')
 def notify_api():
-    app = create_app()
+    app = Flask('test')
+    create_app(app)
 
     # deattach server-error error handlers - error_handler_spec looks like:
     #   {'blueprint_name': {
@@ -76,8 +76,6 @@ def notify_db(notify_api, worker_id):
     current_app.config['SQLALCHEMY_DATABASE_URI'] += '_{}'.format(worker_id)
     create_test_db(current_app.config['SQLALCHEMY_DATABASE_URI'])
 
-    Migrate(notify_api, db)
-    Manager(db, MigrateCommand)
     BASE_DIR = os.path.dirname(os.path.dirname(__file__))
     ALEMBIC_CONFIG = os.path.join(BASE_DIR, 'migrations')
     config = Config(ALEMBIC_CONFIG + '/alembic.ini')

wsgi.py

@@ -1,7 +0,0 @@
-from app import create_app
-
-
-application = create_app()
-
-if __name__ == "__main__":
-    application.run()