From d33698216cb7849a665de069cafaa8bad01a4bdb Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 1 Jun 2017 11:00:26 +0100 Subject: [PATCH 1/2] Revert "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, 70 insertions(+), 66 deletions(-) create mode 100644 migrations/versions/0088_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/0088_govuk_sms_sender.py b/migrations/versions/0088_govuk_sms_sender.py new file mode 100644 index 000000000..3d580d6ad --- /dev/null +++ b/migrations/versions/0088_govuk_sms_sender.py @@ -0,0 +1,25 @@ +"""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 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 46adbba1b..18c384722 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1315,61 +1315,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', [ @@ -1929,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 ab50a3557ea36153a9f418f4590b7df4de471788 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 1 Jun 2017 13:18:56 +0100 Subject: [PATCH 2/2] fix versions --- ...{0088_govuk_sms_sender.py => 0089_govuk_sms_sender.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0088_govuk_sms_sender.py => 0089_govuk_sms_sender.py} (79%) diff --git a/migrations/versions/0088_govuk_sms_sender.py b/migrations/versions/0089_govuk_sms_sender.py similarity index 79% rename from migrations/versions/0088_govuk_sms_sender.py rename to migrations/versions/0089_govuk_sms_sender.py index 3d580d6ad..b69701abd 100644 --- a/migrations/versions/0088_govuk_sms_sender.py +++ b/migrations/versions/0089_govuk_sms_sender.py @@ -1,14 +1,14 @@ """empty message -Revision ID: 0088_govuk_sms_sender -Revises: 0087_scheduled_notifications +Revision ID: 0089_govuk_sms_sender +Revises: 0088_add_schedule_serv_perm Create Date: 2017-05-22 13:46:09.584801 """ # revision identifiers, used by Alembic. -revision = '0088_govuk_sms_sender' -down_revision = '0087_scheduled_notifications' +revision = '0089_govuk_sms_sender' +down_revision = '0088_add_schedule_serv_perm' from alembic import op