From 58503c855a2c934bde16ecc42966d0234504c2ac Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 22 May 2017 14:43:46 +0100 Subject: [PATCH 01/12] set sms_sender to be 'GOVUK' if not otherwise specified this is a precursor to making the column non-nullable --- tests/app/service/test_rest.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 46adbba1b..2912060af 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -240,7 +240,11 @@ def test_create_service(client, sample_user): assert json_resp['data']['email_from'] == 'created.service' assert not json_resp['data']['research_mode'] assert json_resp['data']['dvla_organisation'] == '001' +<<<<<<< HEAD assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] +======= + assert json_resp['data']['sms_sender'] == 'GOVUK' +>>>>>>> set sms_sender to be 'GOVUK' if not otherwise specified service_db = Service.query.get(json_resp['data']['id']) assert service_db.name == 'created service' From eb6edf06a3404f1f69c46575f02c07e952aab064 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 22 May 2017 14:18:12 +0100 Subject: [PATCH 02/12] add upgrade script to remove non-null values from the sender column --- app/models.py | 2 +- migrations/versions/0085_govuk_sms_sender.py | 25 ++++++ tests/app/service/test_rest.py | 92 ++++++++++---------- 3 files changed, 70 insertions(+), 49 deletions(-) create mode 100644 migrations/versions/0085_govuk_sms_sender.py diff --git a/app/models.py b/app/models.py index 8c1e22e79..85bbcfbd1 100644 --- a/app/models.py +++ b/app/models.py @@ -188,7 +188,7 @@ class Service(db.Model, Versioned): created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) reply_to_email_address = db.Column(db.Text, index=False, unique=False, nullable=True) letter_contact_block = db.Column(db.Text, index=False, unique=False, nullable=True) - sms_sender = db.Column(db.String(11), nullable=True, default=lambda: current_app.config['FROM_NUMBER']) + sms_sender = db.Column(db.String(11), nullable=False, default=lambda: current_app.config['FROM_NUMBER']) organisation_id = db.Column(UUID(as_uuid=True), db.ForeignKey('organisation.id'), index=True, nullable=True) organisation = db.relationship('Organisation') dvla_organisation_id = db.Column( diff --git a/migrations/versions/0085_govuk_sms_sender.py b/migrations/versions/0085_govuk_sms_sender.py new file mode 100644 index 000000000..34c0fa835 --- /dev/null +++ b/migrations/versions/0085_govuk_sms_sender.py @@ -0,0 +1,25 @@ +"""empty message + +Revision ID: 0085_govuk_sms_sender +Revises: 0084_add_job_stats +Create Date: 2017-05-22 13:46:09.584801 + +""" + +# revision identifiers, used by Alembic. +revision = '0085_govuk_sms_sender' +down_revision = '0084_add_job_stats' + +from alembic import op + + +def upgrade(): + op.execute("UPDATE services SET sms_sender = 'GOVUK' where sms_sender is null") + op.execute("UPDATE services_history SET sms_sender = 'GOVUK' where sms_sender is null") + op.alter_column('services', 'sms_sender', nullable=False) + op.alter_column('services_history', 'sms_sender', nullable=False) + + +def downgrade(): + op.alter_column('services_history', 'sms_sender', nullable=True) + op.alter_column('services', 'sms_sender', nullable=True) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 2912060af..f61fa3e6b 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1319,61 +1319,57 @@ def test_get_only_api_created_notifications_for_service( assert response.status_code == 200 -def test_set_sms_sender_for_service(notify_api, sample_service): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - auth_header = create_authorization_header() - resp = client.get( - '/service/{}'.format(sample_service.id), - headers=[auth_header] - ) - json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 200 - assert json_resp['data']['name'] == sample_service.name +def test_set_sms_sender_for_service(client, sample_service): + data = { + 'sms_sender': 'elevenchars', + } - data = { - 'sms_sender': 'elevenchars', - } + auth_header = create_authorization_header() - auth_header = create_authorization_header() - - resp = client.post( - '/service/{}'.format(sample_service.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header] - ) - result = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 200 - assert result['data']['sms_sender'] == 'elevenchars' + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 200 + assert result['data']['sms_sender'] == 'elevenchars' -def test_set_sms_sender_for_service_rejects_invalid_characters(notify_api, sample_service): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - auth_header = create_authorization_header() - resp = client.get( - '/service/{}'.format(sample_service.id), - headers=[auth_header] - ) - json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 200 - assert json_resp['data']['name'] == sample_service.name +def test_set_sms_sender_for_service_rejects_invalid_characters(client, sample_service): + data = { + 'sms_sender': 'invalid####', + } - data = { - 'sms_sender': 'invalid####', - } + auth_header = create_authorization_header() - auth_header = create_authorization_header() + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 400 + assert result['result'] == 'error' + assert result['message'] == {'sms_sender': ['Only alphanumeric characters allowed']} - resp = client.post( - '/service/{}'.format(sample_service.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header] - ) - result = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 400 - assert result['result'] == 'error' - assert result['message'] == {'sms_sender': ['Only alphanumeric characters allowed']} + +def test_set_sms_sender_for_service_rejects_null(client, sample_service): + data = { + 'sms_sender': None, + } + + auth_header = create_authorization_header() + + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 400 + assert result['result'] == 'error' + assert result['message'] == {'sms_sender': 'Field may not be null.'} @pytest.mark.parametrize('today_only,stats', [ From 25011f09ef4af7c48dbf7b86ff60ec3f245671e9 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 22 May 2017 17:24:31 +0100 Subject: [PATCH 03/12] test no longer applicable as null sms_sender is an error now --- ...{0085_govuk_sms_sender.py => 0086_govuk_sms_sender.py} | 8 ++++---- tests/app/delivery/test_send_to_providers.py | 1 - tests/app/service/test_rest.py | 6 +----- 3 files changed, 5 insertions(+), 10 deletions(-) rename migrations/versions/{0085_govuk_sms_sender.py => 0086_govuk_sms_sender.py} (78%) diff --git a/migrations/versions/0085_govuk_sms_sender.py b/migrations/versions/0086_govuk_sms_sender.py similarity index 78% rename from migrations/versions/0085_govuk_sms_sender.py rename to migrations/versions/0086_govuk_sms_sender.py index 34c0fa835..6e0fbf7d6 100644 --- a/migrations/versions/0085_govuk_sms_sender.py +++ b/migrations/versions/0086_govuk_sms_sender.py @@ -1,14 +1,14 @@ """empty message -Revision ID: 0085_govuk_sms_sender -Revises: 0084_add_job_stats +Revision ID: 0086_govuk_sms_sender +Revises: 0085_update_incoming_to_inbound Create Date: 2017-05-22 13:46:09.584801 """ # revision identifiers, used by Alembic. -revision = '0085_govuk_sms_sender' -down_revision = '0084_add_job_stats' +revision = '0086_govuk_sms_sender' +down_revision = '0085_update_incoming_to_inbound' from alembic import op diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 5cb467ac3..e1a4ec4a2 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -627,7 +627,6 @@ def test_should_set_international_phone_number_to_sent_status( # if 40604 is actually in DB then treat that as if entered manually ('40604', '40604', 'bar'), # 'testing' is the FROM_NUMBER during unit tests - (None, 'testing', 'Sample service: bar'), ('testing', 'testing', 'Sample service: bar'), ]) def test_should_handle_sms_sender_and_prefix_message( diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index f61fa3e6b..e9e1913a0 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -240,11 +240,7 @@ def test_create_service(client, sample_user): assert json_resp['data']['email_from'] == 'created.service' assert not json_resp['data']['research_mode'] assert json_resp['data']['dvla_organisation'] == '001' -<<<<<<< HEAD assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] -======= - assert json_resp['data']['sms_sender'] == 'GOVUK' ->>>>>>> set sms_sender to be 'GOVUK' if not otherwise specified service_db = Service.query.get(json_resp['data']['id']) assert service_db.name == 'created service' @@ -1369,7 +1365,7 @@ def test_set_sms_sender_for_service_rejects_null(client, sample_service): result = json.loads(resp.get_data(as_text=True)) assert resp.status_code == 400 assert result['result'] == 'error' - assert result['message'] == {'sms_sender': 'Field may not be null.'} + assert result['message'] == {'sms_sender': ['Field may not be null.']} @pytest.mark.parametrize('today_only,stats', [ From 012b2bf36c7c56b31184fabc6744b15f2cf192d2 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 25 May 2017 10:35:10 +0100 Subject: [PATCH 04/12] version number bump --- ...{0086_govuk_sms_sender.py => 0087_govuk_sms_sender.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0086_govuk_sms_sender.py => 0087_govuk_sms_sender.py} (78%) diff --git a/migrations/versions/0086_govuk_sms_sender.py b/migrations/versions/0087_govuk_sms_sender.py similarity index 78% rename from migrations/versions/0086_govuk_sms_sender.py rename to migrations/versions/0087_govuk_sms_sender.py index 6e0fbf7d6..ecd969f8d 100644 --- a/migrations/versions/0086_govuk_sms_sender.py +++ b/migrations/versions/0087_govuk_sms_sender.py @@ -1,14 +1,14 @@ """empty message -Revision ID: 0086_govuk_sms_sender -Revises: 0085_update_incoming_to_inbound +Revision ID: 0087_govuk_sms_sender +Revises: 0086_add_norm_to_notification Create Date: 2017-05-22 13:46:09.584801 """ # revision identifiers, used by Alembic. -revision = '0086_govuk_sms_sender' -down_revision = '0085_update_incoming_to_inbound' +revision = '0087_govuk_sms_sender' +down_revision = '0086_add_norm_to_notification' from alembic import op From db4b3e371a8f20911fdd1c041a873774844b9f68 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 25 May 2017 12:10:11 +0100 Subject: [PATCH 05/12] remove null sms sender test it's no longer possible for an sms_sender to be null --- tests/app/service/test_rest.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index e9e1913a0..18c384722 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1925,22 +1925,6 @@ def test_update_service_does_not_call_send_notification_when_restricted_not_chan assert not send_notification_mock.called -def test_update_service_works_when_sms_sender_is_null(sample_service, client, mocker): - sample_service.sms_sender = None - data = {'name': 'new name'} - - resp = client.post( - 'service/{}'.format(sample_service.id), - data=json.dumps(data), - headers=[create_authorization_header()], - content_type='application/json' - ) - - assert resp.status_code == 200 - # make sure it wasn't changed to not-null under the hood - assert sample_service.sms_sender is None - - def test_search_for_notification_by_to_field_filters_by_status(client, notify_db, notify_db_session): create_notification = partial( create_sample_notification, From de3b5a13a95250130ba125dcba60ef901b719caa Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 30 May 2017 12:49:30 +0100 Subject: [PATCH 06/12] version number bump --- ...{0087_govuk_sms_sender.py => 0088_govuk_sms_sender.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0087_govuk_sms_sender.py => 0088_govuk_sms_sender.py} (79%) diff --git a/migrations/versions/0087_govuk_sms_sender.py b/migrations/versions/0088_govuk_sms_sender.py similarity index 79% rename from migrations/versions/0087_govuk_sms_sender.py rename to migrations/versions/0088_govuk_sms_sender.py index ecd969f8d..3d580d6ad 100644 --- a/migrations/versions/0087_govuk_sms_sender.py +++ b/migrations/versions/0088_govuk_sms_sender.py @@ -1,14 +1,14 @@ """empty message -Revision ID: 0087_govuk_sms_sender -Revises: 0086_add_norm_to_notification +Revision ID: 0088_govuk_sms_sender +Revises: 0087_scheduled_notifications Create Date: 2017-05-22 13:46:09.584801 """ # revision identifiers, used by Alembic. -revision = '0087_govuk_sms_sender' -down_revision = '0086_add_norm_to_notification' +revision = '0088_govuk_sms_sender' +down_revision = '0087_scheduled_notifications' from alembic import op From 9ada8b27538aa30c516e4f1ccd8b3331f0f72ddc Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 30 May 2017 14:40:27 +0100 Subject: [PATCH 07/12] =?UTF-8?q?Don=E2=80=99t=20500=20when=20searching=20?= =?UTF-8?q?with=20bad=20email=20address?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the future we might want to validate email addresses before attempting to search by them. But for a first pass we can just return no results when a user types in something that isn’t an email address or phone number. It definitely better than returning a 500. --- app/dao/notifications_dao.py | 8 ++++++-- tests/app/dao/test_notification_dao.py | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 7ebf9170c..f4ff2a24f 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -9,7 +9,8 @@ from flask import current_app from notifications_utils.recipients import ( validate_and_format_phone_number, validate_and_format_email_address, - InvalidPhoneError + InvalidPhoneError, + InvalidEmailError, ) from werkzeug.datastructures import MultiDict from sqlalchemy import (desc, func, or_, and_, asc) @@ -477,7 +478,10 @@ def dao_get_notifications_by_to_field(service_id, search_term, statuses=None): try: normalised = validate_and_format_phone_number(search_term) except InvalidPhoneError: - normalised = validate_and_format_email_address(search_term) + try: + normalised = validate_and_format_email_address(search_term) + except InvalidEmailError: + normalised = search_term filters = [ Notification.service_id == service_id, diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index a6b69cd24..dc0f3fc34 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -1772,6 +1772,20 @@ def test_dao_get_notifications_by_to_field_search_is_not_case_sensitive(sample_t assert notification.id in notification_ids +@pytest.mark.parametrize('to', [ + 'not@email', '123' +]) +def test_dao_get_notifications_by_to_field_accepts_invalid_phone_numbers_and_email_addresses( + sample_template, + to, +): + notification = create_notification( + template=sample_template, to_field='test@example.com', normalised_to='test@example.com' + ) + results = dao_get_notifications_by_to_field(notification.service_id, to) + assert len(results) == 0 + + def test_dao_get_notifications_by_to_field_search_ignores_spaces(sample_template): notification1 = create_notification( template=sample_template, to_field='+447700900855', normalised_to='447700900855' From 8f7afcdb16720faf59995dac15edbdc3902b4687 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 30 May 2017 17:07:43 +0100 Subject: [PATCH 08/12] Did some work around the delete queues script --- .gitignore | 2 + scripts/delete_sqs_queues.py | 72 +++++++++++++++++++++++++++--------- 2 files changed, 56 insertions(+), 18 deletions(-) mode change 100644 => 100755 scripts/delete_sqs_queues.py diff --git a/.gitignore b/.gitignore index d19df2e94..12f77b15c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ +queues.csv + # Byte-compiled / optimized / DLL files __pycache__/ *.py[cod] diff --git a/scripts/delete_sqs_queues.py b/scripts/delete_sqs_queues.py old mode 100644 new mode 100755 index bdbf6ff65..1e274b4ed --- a/scripts/delete_sqs_queues.py +++ b/scripts/delete_sqs_queues.py @@ -1,10 +1,33 @@ +""" + +Script to manage SQS queues. Can list or delete queues. + +Uses boto, so relies on correctly set up AWS access keys and tokens. + +In principle use this script to dump details of all queues in a gievn environment, and then +manipulate the resultant CSV file so that it contains the queues you want to delete. + +Very hands on. Starter for a more automagic process. + +Usage: + scripts/delete_sqs_queues.py + + options are: + - list: dumps queue details to local file queues.csv in current directory. + - delete: delete queues from local file queues.csv in current directory. + +Example: + scripts/delete_sqs_queues.py list delete +""" + +from docopt import docopt import boto3 import csv from datetime import datetime -from pprint import pprint -import os -client = boto3.client('sqs', region_name=os.getenv('AWS_REGION')) +FILE_NAME = "/tmp/queues.csv" + +client = boto3.client('sqs', region_name='eu-west-1') def _formatted_date_from_timestamp(timestamp): @@ -27,15 +50,19 @@ def get_queue_attributes(queue_name): ] ) queue_attributes = response['Attributes'] + queue_attributes.update({ + 'QueueUrl': queue_name + }) return queue_attributes -def delete_queue(queue_name): +def delete_queue(queue_url): + print("DELETEING {}".format(queue_url)) response = client.delete_queue( - QueueUrl=queue_name + QueueUrl=queue_url ) if response['ResponseMetadata']['HTTPStatusCode'] == 200: - print('Deleted queue successfully') + print('Deleted queue successfully {}'.format(response['ResponseMetadata'])) else: print('Error occured when attempting to delete queue') pprint(response) @@ -43,10 +70,10 @@ def delete_queue(queue_name): def output_to_csv(queue_attributes): - csv_name = 'queues.csv' - with open(csv_name, 'w') as csvfile: + with open(FILE_NAME, 'w') as csvfile: fieldnames = [ 'Queue Name', + 'Queue URL', 'Number of Messages', 'Number of Messages Delayed', 'Number of Messages Not Visible', @@ -55,23 +82,19 @@ def output_to_csv(queue_attributes): writer = csv.DictWriter(csvfile, fieldnames=fieldnames) writer.writeheader() for queue_attr in queue_attributes: - queue_url = client.get_queue_url( - QueueName=queue_attr['QueueArn'] - )['QueueUrl'] writer.writerow({ 'Queue Name': queue_attr['QueueArn'], - 'Queue URL': queue_url, + 'Queue URL': queue_attr['QueueUrl'], 'Number of Messages': queue_attr['ApproximateNumberOfMessages'], 'Number of Messages Delayed': queue_attr['ApproximateNumberOfMessagesDelayed'], 'Number of Messages Not Visible': queue_attr['ApproximateNumberOfMessagesNotVisible'], 'Created': _formatted_date_from_timestamp(queue_attr['CreatedTimestamp']) }) - return csv_name -def read_from_csv(csv_name): +def read_from_csv(): queue_urls = [] - with open(csv_name, 'r') as csvfile: + with open(FILE_NAME, 'r') as csvfile: next(csvfile) rows = csv.reader(csvfile, delimiter=',') for row in rows: @@ -79,6 +102,19 @@ def read_from_csv(csv_name): return queue_urls -queues = get_queues() -for queue in queues: - delete_queue(queue) +if __name__ == "__main__": + arguments = docopt(__doc__) + + if arguments[''] == 'list': + queues = get_queues() + queue_attributes = [] + for queue in queues: + queue_attributes.append(get_queue_attributes(queue)) + output_to_csv(queue_attributes) + elif arguments[''] == 'delete': + queues_to_delete = read_from_csv() + for queue in queues_to_delete: + delete_queue(queue) + else: + print("UNKNOWN COMMAND") + exit(1) From 68e15b57f591bda31654ae93c130591c1e967ac3 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 30 May 2017 17:29:14 +0100 Subject: [PATCH 09/12] Fixed pep8 --- scripts/delete_sqs_queues.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/scripts/delete_sqs_queues.py b/scripts/delete_sqs_queues.py index 1e274b4ed..1ab641f76 100755 --- a/scripts/delete_sqs_queues.py +++ b/scripts/delete_sqs_queues.py @@ -4,20 +4,20 @@ Script to manage SQS queues. Can list or delete queues. Uses boto, so relies on correctly set up AWS access keys and tokens. -In principle use this script to dump details of all queues in a gievn environment, and then +In principle use this script to dump details of all queues in a gievn environment, and then manipulate the resultant CSV file so that it contains the queues you want to delete. Very hands on. Starter for a more automagic process. Usage: scripts/delete_sqs_queues.py - + options are: - list: dumps queue details to local file queues.csv in current directory. - delete: delete queues from local file queues.csv in current directory. Example: - scripts/delete_sqs_queues.py list delete + scripts/delete_sqs_queues.py list delete """ from docopt import docopt @@ -50,9 +50,7 @@ def get_queue_attributes(queue_name): ] ) queue_attributes = response['Attributes'] - queue_attributes.update({ - 'QueueUrl': queue_name - }) + queue_attributes.update({'QueueUrl': queue_name}) return queue_attributes From 25c8f71f2cb3802efd326da4e0bea5f5cc34dfd2 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 31 May 2017 11:47:52 +0100 Subject: [PATCH 10/12] Reduced memory footprint of the API apps. Staging and prod now default to 768M of RAM, down from a 1G saves 512M per instance type Preview down to 256M per app --- manifest-api-preview.yml | 3 +++ manifest-delivery-base.yml | 3 ++- manifest-delivery-preview.yml | 1 + manifest-delivery-production.yml | 2 +- manifest-delivery-staging.yml | 2 +- 5 files changed, 8 insertions(+), 3 deletions(-) diff --git a/manifest-api-preview.yml b/manifest-api-preview.yml index 04b396388..a40990194 100644 --- a/manifest-api-preview.yml +++ b/manifest-api-preview.yml @@ -6,3 +6,6 @@ routes: - route: notify-api-preview.cloudapps.digital - route: api-paas.notify.works - route: api.notify.works + +instances: 1 +memory: 256M diff --git a/manifest-delivery-base.yml b/manifest-delivery-base.yml index c68d36965..2eaf380a6 100644 --- a/manifest-delivery-base.yml +++ b/manifest-delivery-base.yml @@ -24,6 +24,7 @@ applications: - name: notify-delivery-worker-database command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q db-sms,db-email,db-letter,database-tasks + memory: 1G env: NOTIFY_APP_NAME: delivery-worker-database @@ -34,13 +35,13 @@ applications: - name: notify-delivery-worker-sender command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q send-sms,send-email,send-tasks + memory: 1G env: NOTIFY_APP_NAME: delivery-worker-sender - name: notify-delivery-worker-periodic command: scripts/run_app_paas.sh celery -A aws_run_celery.notify_celery worker --loglevel=INFO --concurrency=2 -Q periodic,statistics,periodic-tasks,statistics-tasks instances: 1 - memory: 2G env: NOTIFY_APP_NAME: delivery-worker-periodic diff --git a/manifest-delivery-preview.yml b/manifest-delivery-preview.yml index d628e5fc9..2bbb3c0dc 100644 --- a/manifest-delivery-preview.yml +++ b/manifest-delivery-preview.yml @@ -1,3 +1,4 @@ --- inherit: manifest-delivery-base.yml +memory: 256M diff --git a/manifest-delivery-production.yml b/manifest-delivery-production.yml index 53c8d2f12..d2c2ba647 100644 --- a/manifest-delivery-production.yml +++ b/manifest-delivery-production.yml @@ -3,4 +3,4 @@ inherit: manifest-delivery-base.yml instances: 2 -memory: 1G +memory: 768M diff --git a/manifest-delivery-staging.yml b/manifest-delivery-staging.yml index 53c8d2f12..d2c2ba647 100644 --- a/manifest-delivery-staging.yml +++ b/manifest-delivery-staging.yml @@ -3,4 +3,4 @@ inherit: manifest-delivery-base.yml instances: 2 -memory: 1G +memory: 768M From ea0ba8d87ab4aa92eb794c40a32e4da6429236c8 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 31 May 2017 14:52:48 +0100 Subject: [PATCH 11/12] Revert "Remove nulls from sms_sender" --- app/models.py | 2 +- migrations/versions/0088_govuk_sms_sender.py | 25 ----- tests/app/delivery/test_send_to_providers.py | 1 + tests/app/service/test_rest.py | 108 +++++++++++-------- 4 files changed, 66 insertions(+), 70 deletions(-) delete mode 100644 migrations/versions/0088_govuk_sms_sender.py diff --git a/app/models.py b/app/models.py index 85bbcfbd1..8c1e22e79 100644 --- a/app/models.py +++ b/app/models.py @@ -188,7 +188,7 @@ class Service(db.Model, Versioned): created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) reply_to_email_address = db.Column(db.Text, index=False, unique=False, nullable=True) letter_contact_block = db.Column(db.Text, index=False, unique=False, nullable=True) - sms_sender = db.Column(db.String(11), nullable=False, default=lambda: current_app.config['FROM_NUMBER']) + sms_sender = db.Column(db.String(11), nullable=True, default=lambda: current_app.config['FROM_NUMBER']) organisation_id = db.Column(UUID(as_uuid=True), db.ForeignKey('organisation.id'), index=True, nullable=True) organisation = db.relationship('Organisation') dvla_organisation_id = db.Column( diff --git a/migrations/versions/0088_govuk_sms_sender.py b/migrations/versions/0088_govuk_sms_sender.py deleted file mode 100644 index 3d580d6ad..000000000 --- a/migrations/versions/0088_govuk_sms_sender.py +++ /dev/null @@ -1,25 +0,0 @@ -"""empty message - -Revision ID: 0088_govuk_sms_sender -Revises: 0087_scheduled_notifications -Create Date: 2017-05-22 13:46:09.584801 - -""" - -# revision identifiers, used by Alembic. -revision = '0088_govuk_sms_sender' -down_revision = '0087_scheduled_notifications' - -from alembic import op - - -def upgrade(): - op.execute("UPDATE services SET sms_sender = 'GOVUK' where sms_sender is null") - op.execute("UPDATE services_history SET sms_sender = 'GOVUK' where sms_sender is null") - op.alter_column('services', 'sms_sender', nullable=False) - op.alter_column('services_history', 'sms_sender', nullable=False) - - -def downgrade(): - op.alter_column('services_history', 'sms_sender', nullable=True) - op.alter_column('services', 'sms_sender', nullable=True) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index e1a4ec4a2..5cb467ac3 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -627,6 +627,7 @@ def test_should_set_international_phone_number_to_sent_status( # if 40604 is actually in DB then treat that as if entered manually ('40604', '40604', 'bar'), # 'testing' is the FROM_NUMBER during unit tests + (None, 'testing', 'Sample service: bar'), ('testing', 'testing', 'Sample service: bar'), ]) def test_should_handle_sms_sender_and_prefix_message( diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 18c384722..46adbba1b 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1315,57 +1315,61 @@ def test_get_only_api_created_notifications_for_service( assert response.status_code == 200 -def test_set_sms_sender_for_service(client, sample_service): - data = { - 'sms_sender': 'elevenchars', - } +def test_set_sms_sender_for_service(notify_api, sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth_header = create_authorization_header() + resp = client.get( + '/service/{}'.format(sample_service.id), + headers=[auth_header] + ) + json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 200 + assert json_resp['data']['name'] == sample_service.name - auth_header = create_authorization_header() + data = { + 'sms_sender': 'elevenchars', + } - resp = client.post( - '/service/{}'.format(sample_service.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header] - ) - result = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 200 - assert result['data']['sms_sender'] == 'elevenchars' + auth_header = create_authorization_header() + + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 200 + assert result['data']['sms_sender'] == 'elevenchars' -def test_set_sms_sender_for_service_rejects_invalid_characters(client, sample_service): - data = { - 'sms_sender': 'invalid####', - } +def test_set_sms_sender_for_service_rejects_invalid_characters(notify_api, sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth_header = create_authorization_header() + resp = client.get( + '/service/{}'.format(sample_service.id), + headers=[auth_header] + ) + json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 200 + assert json_resp['data']['name'] == sample_service.name - auth_header = create_authorization_header() + data = { + 'sms_sender': 'invalid####', + } - resp = client.post( - '/service/{}'.format(sample_service.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header] - ) - result = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 400 - assert result['result'] == 'error' - assert result['message'] == {'sms_sender': ['Only alphanumeric characters allowed']} + auth_header = create_authorization_header() - -def test_set_sms_sender_for_service_rejects_null(client, sample_service): - data = { - 'sms_sender': None, - } - - auth_header = create_authorization_header() - - resp = client.post( - '/service/{}'.format(sample_service.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header] - ) - result = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 400 - assert result['result'] == 'error' - assert result['message'] == {'sms_sender': ['Field may not be null.']} + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 400 + assert result['result'] == 'error' + assert result['message'] == {'sms_sender': ['Only alphanumeric characters allowed']} @pytest.mark.parametrize('today_only,stats', [ @@ -1925,6 +1929,22 @@ def test_update_service_does_not_call_send_notification_when_restricted_not_chan assert not send_notification_mock.called +def test_update_service_works_when_sms_sender_is_null(sample_service, client, mocker): + sample_service.sms_sender = None + data = {'name': 'new name'} + + resp = client.post( + 'service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[create_authorization_header()], + content_type='application/json' + ) + + assert resp.status_code == 200 + # make sure it wasn't changed to not-null under the hood + assert sample_service.sms_sender is None + + def test_search_for_notification_by_to_field_filters_by_status(client, notify_db, notify_db_session): create_notification = partial( create_sample_notification, From b98b97c4a21c87f92dbc63784483417c2ec66daf Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Wed, 31 May 2017 15:06:21 +0100 Subject: [PATCH 12/12] Added a comment about delete queues --- scripts/delete_sqs_queues.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/delete_sqs_queues.py b/scripts/delete_sqs_queues.py index 1ab641f76..b167ce392 100755 --- a/scripts/delete_sqs_queues.py +++ b/scripts/delete_sqs_queues.py @@ -55,6 +55,7 @@ def get_queue_attributes(queue_name): def delete_queue(queue_url): + # Note that deleting a queue returns 200 OK if it doesn't exist print("DELETEING {}".format(queue_url)) response = client.delete_queue( QueueUrl=queue_url