From 0b3136397bfcd7a65c8fe3a74754077dfdc5fdb1 Mon Sep 17 00:00:00 2001 From: Athanasios Voutsadakis Date: Thu, 12 Oct 2017 14:51:04 +0100 Subject: [PATCH 1/6] Drop api-paas.* route This was introduced during the migration and is no longer needed nor should it be used. --- manifest-api-preview.yml | 1 - manifest-api-production.yml | 1 - manifest-api-staging.yml | 1 - 3 files changed, 3 deletions(-) diff --git a/manifest-api-preview.yml b/manifest-api-preview.yml index eccae21b1..1eb88ef10 100644 --- a/manifest-api-preview.yml +++ b/manifest-api-preview.yml @@ -4,7 +4,6 @@ inherit: manifest-api-base.yml routes: - route: notify-api-preview.cloudapps.digital - - route: api-paas.notify.works - route: api.notify.works instances: 1 diff --git a/manifest-api-production.yml b/manifest-api-production.yml index dca5332fd..6dfdbdaf7 100644 --- a/manifest-api-production.yml +++ b/manifest-api-production.yml @@ -4,7 +4,6 @@ inherit: manifest-api-base.yml routes: - route: notify-api-production.cloudapps.digital - - route: api-paas.notifications.service.gov.uk - route: api.notifications.service.gov.uk instances: 2 memory: 1G diff --git a/manifest-api-staging.yml b/manifest-api-staging.yml index 2e0adbabb..ec6602930 100644 --- a/manifest-api-staging.yml +++ b/manifest-api-staging.yml @@ -4,7 +4,6 @@ inherit: manifest-api-base.yml routes: - route: notify-api-staging.cloudapps.digital - - route: api-paas.staging-notify.works - route: api.staging-notify.works instances: 2 memory: 1G From 616a6f8ef8aa4a273cd7dfa83219783dc1260e44 Mon Sep 17 00:00:00 2001 From: Venus Bailey Date: Mon, 16 Oct 2017 12:43:05 +0100 Subject: [PATCH 2/6] Revert "[#151837054] Add new column free_sms_fragment_limit in the Services table" --- app/commands.py | 24 ------ app/models.py | 5 +- app/schemas.py | 12 ++- app/service/rest.py | 4 - application.py | 2 - .../0124_add_free_sms_fragment_limit.py | 23 ------ tests/app/conftest.py | 4 +- tests/app/service/test_rest.py | 78 +------------------ 8 files changed, 17 insertions(+), 135 deletions(-) delete mode 100644 migrations/versions/0124_add_free_sms_fragment_limit.py diff --git a/app/commands.py b/app/commands.py index 8686eeb0e..b9825d8d6 100644 --- a/app/commands.py +++ b/app/commands.py @@ -305,27 +305,3 @@ class PopulateServiceLetterContact(Command): db.session.commit() print("Populated letter contacts for {} services".format(result.rowcount)) - - -class PopulateServiceAndServiceHistoryFreeSmsFragmentLimit(Command): - - def run(self): - 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)) diff --git a/app/models.py b/app/models.py index 69bce7fd5..a5da33951 100644 --- a/app/models.py +++ b/app/models.py @@ -211,7 +211,6 @@ class Service(db.Model, Versioned): _letter_contact_block = db.Column('letter_contact_block', db.Text, index=False, unique=False, nullable=True) 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) - free_sms_fragment_limit = db.Column(db.BigInteger, index=False, unique=False, nullable=True) organisation = db.relationship('Organisation') dvla_organisation_id = db.Column( db.String, @@ -231,6 +230,10 @@ class Service(db.Model, Versioned): association_proxy('permissions', 'service_permission_types') + @staticmethod + def free_sms_fragment_limit(): + return current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] + @classmethod def from_json(cls, data): """ diff --git a/app/schemas.py b/app/schemas.py index e84eca3ce..8ce2e5ed5 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -175,17 +175,20 @@ class ProviderDetailsHistorySchema(BaseSchema): class ServiceSchema(BaseSchema): + free_sms_fragment_limit = fields.Method(method_name='get_free_sms_fragment_limit') created_by = field_for(models.Service, 'created_by', required=True) organisation = field_for(models.Service, 'organisation') branding = field_for(models.Service, 'branding') dvla_organisation = field_for(models.Service, 'dvla_organisation') - free_sms_fragment_limit = field_for(models.Service, 'free_sms_fragment_limit') permissions = fields.Method("service_permissions") override_flag = False reply_to_email_address = fields.Method(method_name="get_reply_to_email_address") sms_sender = fields.Method(method_name="get_sms_sender") letter_contact_block = fields.Method(method_name="get_letter_contact") + def get_free_sms_fragment_limit(selfs, service): + return service.free_sms_fragment_limit() + def service_permissions(self, service): return [p.permission for p in service.permissions] @@ -200,7 +203,7 @@ class ServiceSchema(BaseSchema): class Meta: model = models.Service - dump_only = ['reply_to_email_address', 'letter_contact_block'] + dump_only = ['free_sms_fragment_limit', 'reply_to_email_address', 'letter_contact_block'] exclude = ( 'updated_at', 'created_at', @@ -249,6 +252,11 @@ class ServiceSchema(BaseSchema): class DetailedServiceSchema(BaseSchema): statistics = fields.Dict() + free_sms_fragment_limit = fields.Method(method_name='get_free_sms_fragment_limit') + + def get_free_sms_fragment_limit(selfs, service): + return service.free_sms_fragment_limit() + class Meta: model = models.Service exclude = ( diff --git a/app/service/rest.py b/app/service/rest.py index 66cd90317..e06d66dc9 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -138,10 +138,6 @@ def create_service(): errors = {'user_id': ['Missing data for required field.']} raise InvalidRequest(errors, status_code=400) - # TODO: to be removed when front-end is updated - if 'free_sms_fragment_limit' not in data: - data['free_sms_fragment_limit'] = current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] - # validate json with marshmallow service_schema.load(request.get_json()) diff --git a/application.py b/application.py index 07fe4f0de..0cee490a5 100644 --- a/application.py +++ b/application.py @@ -21,8 +21,6 @@ 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.command diff --git a/migrations/versions/0124_add_free_sms_fragment_limit.py b/migrations/versions/0124_add_free_sms_fragment_limit.py deleted file mode 100644 index 83647fcfe..000000000 --- a/migrations/versions/0124_add_free_sms_fragment_limit.py +++ /dev/null @@ -1,23 +0,0 @@ -""" - -Revision ID: 0124_add_free_sms_fragment_limit -Revises: 0123_add_noti_to_email_reply -Create Date: 2017-10-10 11:30:16.225980 - -""" -from alembic import op -import sqlalchemy as sa - - -revision = '0124_add_free_sms_fragment_limit' -down_revision = '0123_add_noti_to_email_reply' - - -def upgrade(): - op.add_column('services_history', sa.Column('free_sms_fragment_limit', sa.BigInteger(), nullable=True)) - op.add_column('services', sa.Column('free_sms_fragment_limit', sa.BigInteger(), nullable=True)) - - -def downgrade(): - op.drop_column('services_history', 'free_sms_fragment_limit') - op.drop_column('services', 'free_sms_fragment_limit') diff --git a/tests/app/conftest.py b/tests/app/conftest.py index d904c8ecf..a9dfa0a1b 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -139,7 +139,6 @@ def sample_service( email_from=None, permissions=[SMS_TYPE, EMAIL_TYPE], research_mode=None, - free_sms_fragment_limit=250000 ): if user is None: user = create_user() @@ -151,8 +150,7 @@ def sample_service( 'message_limit': limit, 'restricted': restricted, 'email_from': email_from, - 'created_by': user, - 'free_sms_fragment_limit': free_sms_fragment_limit + 'created_by': user } service = Service.query.filter_by(name=service_name).first() if not service: diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index c131abad3..ba0465958 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -162,7 +162,7 @@ def test_get_service_by_id_returns_free_sms_limit(client, sample_service): ) assert resp.status_code == 200 json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] + assert json_resp['data']['free_sms_fragment_limit'] == 250000 def test_get_detailed_service_by_id_returns_free_sms_limit(client, sample_service): @@ -174,7 +174,7 @@ def test_get_detailed_service_by_id_returns_free_sms_limit(client, sample_servic ) assert resp.status_code == 200 json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] + assert json_resp['data']['free_sms_fragment_limit'] == 250000 def test_get_service_list_has_default_permissions(client, service_factory): @@ -272,7 +272,6 @@ def test_create_service(client, sample_user): assert not json_resp['data']['research_mode'] assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] - assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] service_db = Service.query.get(json_resp['data']['id']) assert service_db.name == 'created service' @@ -317,49 +316,6 @@ def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_u assert 'Missing data for required field.' in json_resp['message']['user_id'] -def test_create_service_free_sms_fragment_limit_is_optional(client, sample_user): - data1 = { - 'name': 'service 1', - 'user_id': str(sample_user.id), - 'message_limit': 1000, - 'restricted': False, - 'active': False, - 'email_from': 'sample_user.email1', - 'created_by': str(sample_user.id), - 'free_sms_fragment_limit': 9999 - } - - auth_header = create_authorization_header() - headers = [('Content-Type', 'application/json'), auth_header] - resp = client.post( - '/service', - data=json.dumps(data1), - headers=headers) - json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 201 - assert json_resp['data']['free_sms_fragment_limit'] == 9999 - - data2 = { - 'name': 'service 2', - 'user_id': str(sample_user.id), - 'message_limit': 1000, - 'restricted': False, - 'active': False, - 'email_from': 'sample_user.email2', - 'created_by': str(sample_user.id), - } - - auth_header = create_authorization_header() - headers = [('Content-Type', 'application/json'), auth_header] - resp = client.post( - '/service', - data=json.dumps(data2), - headers=headers) - json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == 201 - assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] - - def test_should_error_if_created_by_missing(notify_api, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -594,36 +550,6 @@ def test_update_service_flags_will_remove_service_permissions(client, notify_db, assert set([p.permission for p in permissions]) == set([SMS_TYPE, EMAIL_TYPE]) -def test_update_service_free_sms_fragment_limit(client, notify_db, sample_service): - org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') - notify_db.session.add(org) - notify_db.session.commit() - - 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']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] - - data = { - 'free_sms_fragment_limit': 9999 - } - - 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']['free_sms_fragment_limit'] == 9999 - - def test_update_permissions_will_override_permission_flags(client, service_with_no_permissions): auth_header = create_authorization_header() From 63d9fb46e70e8033a9eeb81076ebe6e9757fb53f Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 16 Oct 2017 13:23:19 +0100 Subject: [PATCH 3/6] remove ip restrictions from sms delivery receipts for now our research mode tasks were hitting these endpoints, and getting 403s, causing lots of unexpected exceptions. --- app/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/__init__.py b/app/__init__.py index 1e0ce73ab..31a601f3c 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -117,9 +117,13 @@ def register_blueprint(application): ses_callback_blueprint.before_request(requires_no_auth) application.register_blueprint(ses_callback_blueprint) - sms_callback_blueprint.before_request(restrict_ip_sms) + # delivery receipts + # TODO: make sure research mode can still trigger sms callbacks, then re-enable this + # sms_callback_blueprint.before_request(restrict_ip_sms) + sms_callback_blueprint.before_request(requires_no_auth) application.register_blueprint(sms_callback_blueprint) + # inbound sms receive_notifications_blueprint.before_request(restrict_ip_sms) application.register_blueprint(receive_notifications_blueprint) From d33c25bdac7eb33890e887e6a717aedbfad541c1 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 16 Oct 2017 13:24:06 +0100 Subject: [PATCH 4/6] update research mode notifications after callback has been made if the callback fails, they should still tech fail --- app/delivery/send_to_providers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index e0202c897..38c1e0015 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -54,8 +54,8 @@ def send_sms_to_provider(notification): if service.research_mode or notification.key_type == KEY_TYPE_TEST: notification.billable_units = 0 - update_notification(notification, provider) send_sms_response(provider.get_name(), str(notification.id), notification.to) + update_notification(notification, provider) else: try: provider.send_sms( From 19bb0d157dd1cb206dd3f3415b2bf918fdbe3d16 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 16 Oct 2017 15:33:21 +0100 Subject: [PATCH 5/6] reset notification to created if fake callback throws exception else we ignore it when retrying --- app/delivery/send_to_providers.py | 12 +++++++++++- tests/app/delivery/test_send_to_providers.py | 15 +++++++++++++++ .../v2/notifications/test_post_notifications.py | 2 +- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 38c1e0015..4dd2892cb 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -25,6 +25,7 @@ from app.models import ( BRANDING_ORG_BANNER, BRANDING_GOVUK, EMAIL_TYPE, + NOTIFICATION_CREATED, NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_SENT, NOTIFICATION_SENDING @@ -54,8 +55,17 @@ def send_sms_to_provider(notification): if service.research_mode or notification.key_type == KEY_TYPE_TEST: notification.billable_units = 0 - send_sms_response(provider.get_name(), str(notification.id), notification.to) update_notification(notification, provider) + try: + send_sms_response(provider.get_name(), str(notification.id), notification.to) + except: + # when we retry, we only do anything if the notification is in created - it's currently in sending, + # so set it back so that we actually attempt the callback again + notification.sent_at = None + notification.sent_by = None + notification.status = NOTIFICATION_CREATED + dao_update_notification(notification) + raise else: try: provider.send_sms( diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 99fffc002..00b75f873 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -6,6 +6,7 @@ from unittest.mock import ANY, call import pytest from notifications_utils.recipients import validate_and_format_phone_number from flask import current_app +from requests import HTTPError import app from app import mmg_client, firetext_client @@ -235,6 +236,20 @@ def test_should_call_send_sms_response_task_if_research_mode( assert not persisted_notification.personalisation +def test_should_leave_as_created_if_fake_callback_function_fails(sample_notification, mocker): + mocker.patch('app.delivery.send_to_providers.send_sms_response', side_effect=HTTPError) + + sample_notification.key_type = KEY_TYPE_TEST + + with pytest.raises(HTTPError): + send_to_providers.send_sms_to_provider( + sample_notification + ) + assert sample_notification.status == 'created' + assert sample_notification.sent_at is None + assert sample_notification.sent_by is None + + @pytest.mark.parametrize('research_mode,key_type', [ (True, KEY_TYPE_NORMAL), (False, KEY_TYPE_TEST) diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 9e9847dc1..390ccac08 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -181,7 +181,7 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_ assert response.status_code == 201 resp_json = json.loads(response.get_data(as_text=True)) assert validate(resp_json, post_email_response) == resp_json - notification = Notification.query.first() + notification = Notification.query.one() assert resp_json['id'] == str(notification.id) assert resp_json['reference'] == reference assert notification.reference is None From 9b60d699319a5c8ad8fa04fd2c2dceff04dd7213 Mon Sep 17 00:00:00 2001 From: Venus Bailey Date: Mon, 16 Oct 2017 16:24:34 +0100 Subject: [PATCH 6/6] Revert "Revert "[#151837054] Add new column free_sms_fragment_limit in the Services table"" --- app/commands.py | 24 ++++++ app/models.py | 5 +- app/schemas.py | 12 +-- app/service/rest.py | 4 + application.py | 2 + .../0124_add_free_sms_fragment_limit.py | 23 ++++++ tests/app/conftest.py | 4 +- tests/app/service/test_rest.py | 78 ++++++++++++++++++- 8 files changed, 135 insertions(+), 17 deletions(-) create mode 100644 migrations/versions/0124_add_free_sms_fragment_limit.py diff --git a/app/commands.py b/app/commands.py index b9825d8d6..8686eeb0e 100644 --- a/app/commands.py +++ b/app/commands.py @@ -305,3 +305,27 @@ class PopulateServiceLetterContact(Command): db.session.commit() print("Populated letter contacts for {} services".format(result.rowcount)) + + +class PopulateServiceAndServiceHistoryFreeSmsFragmentLimit(Command): + + def run(self): + 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)) diff --git a/app/models.py b/app/models.py index a5da33951..69bce7fd5 100644 --- a/app/models.py +++ b/app/models.py @@ -211,6 +211,7 @@ class Service(db.Model, Versioned): _letter_contact_block = db.Column('letter_contact_block', db.Text, index=False, unique=False, nullable=True) 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) + free_sms_fragment_limit = db.Column(db.BigInteger, index=False, unique=False, nullable=True) organisation = db.relationship('Organisation') dvla_organisation_id = db.Column( db.String, @@ -230,10 +231,6 @@ class Service(db.Model, Versioned): association_proxy('permissions', 'service_permission_types') - @staticmethod - def free_sms_fragment_limit(): - return current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] - @classmethod def from_json(cls, data): """ diff --git a/app/schemas.py b/app/schemas.py index 8ce2e5ed5..e84eca3ce 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -175,20 +175,17 @@ class ProviderDetailsHistorySchema(BaseSchema): class ServiceSchema(BaseSchema): - free_sms_fragment_limit = fields.Method(method_name='get_free_sms_fragment_limit') created_by = field_for(models.Service, 'created_by', required=True) organisation = field_for(models.Service, 'organisation') branding = field_for(models.Service, 'branding') dvla_organisation = field_for(models.Service, 'dvla_organisation') + free_sms_fragment_limit = field_for(models.Service, 'free_sms_fragment_limit') permissions = fields.Method("service_permissions") override_flag = False reply_to_email_address = fields.Method(method_name="get_reply_to_email_address") sms_sender = fields.Method(method_name="get_sms_sender") letter_contact_block = fields.Method(method_name="get_letter_contact") - def get_free_sms_fragment_limit(selfs, service): - return service.free_sms_fragment_limit() - def service_permissions(self, service): return [p.permission for p in service.permissions] @@ -203,7 +200,7 @@ class ServiceSchema(BaseSchema): class Meta: model = models.Service - dump_only = ['free_sms_fragment_limit', 'reply_to_email_address', 'letter_contact_block'] + dump_only = ['reply_to_email_address', 'letter_contact_block'] exclude = ( 'updated_at', 'created_at', @@ -252,11 +249,6 @@ class ServiceSchema(BaseSchema): class DetailedServiceSchema(BaseSchema): statistics = fields.Dict() - free_sms_fragment_limit = fields.Method(method_name='get_free_sms_fragment_limit') - - def get_free_sms_fragment_limit(selfs, service): - return service.free_sms_fragment_limit() - class Meta: model = models.Service exclude = ( diff --git a/app/service/rest.py b/app/service/rest.py index e06d66dc9..66cd90317 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -138,6 +138,10 @@ def create_service(): errors = {'user_id': ['Missing data for required field.']} raise InvalidRequest(errors, status_code=400) + # TODO: to be removed when front-end is updated + if 'free_sms_fragment_limit' not in data: + data['free_sms_fragment_limit'] = current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] + # validate json with marshmallow service_schema.load(request.get_json()) diff --git a/application.py b/application.py index 0cee490a5..07fe4f0de 100644 --- a/application.py +++ b/application.py @@ -21,6 +21,8 @@ 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.command diff --git a/migrations/versions/0124_add_free_sms_fragment_limit.py b/migrations/versions/0124_add_free_sms_fragment_limit.py new file mode 100644 index 000000000..83647fcfe --- /dev/null +++ b/migrations/versions/0124_add_free_sms_fragment_limit.py @@ -0,0 +1,23 @@ +""" + +Revision ID: 0124_add_free_sms_fragment_limit +Revises: 0123_add_noti_to_email_reply +Create Date: 2017-10-10 11:30:16.225980 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0124_add_free_sms_fragment_limit' +down_revision = '0123_add_noti_to_email_reply' + + +def upgrade(): + op.add_column('services_history', sa.Column('free_sms_fragment_limit', sa.BigInteger(), nullable=True)) + op.add_column('services', sa.Column('free_sms_fragment_limit', sa.BigInteger(), nullable=True)) + + +def downgrade(): + op.drop_column('services_history', 'free_sms_fragment_limit') + op.drop_column('services', 'free_sms_fragment_limit') diff --git a/tests/app/conftest.py b/tests/app/conftest.py index a9dfa0a1b..d904c8ecf 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -139,6 +139,7 @@ def sample_service( email_from=None, permissions=[SMS_TYPE, EMAIL_TYPE], research_mode=None, + free_sms_fragment_limit=250000 ): if user is None: user = create_user() @@ -150,7 +151,8 @@ def sample_service( 'message_limit': limit, 'restricted': restricted, 'email_from': email_from, - 'created_by': user + 'created_by': user, + 'free_sms_fragment_limit': free_sms_fragment_limit } service = Service.query.filter_by(name=service_name).first() if not service: diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index ba0465958..c131abad3 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -162,7 +162,7 @@ def test_get_service_by_id_returns_free_sms_limit(client, sample_service): ) assert resp.status_code == 200 json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['data']['free_sms_fragment_limit'] == 250000 + assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] def test_get_detailed_service_by_id_returns_free_sms_limit(client, sample_service): @@ -174,7 +174,7 @@ def test_get_detailed_service_by_id_returns_free_sms_limit(client, sample_servic ) assert resp.status_code == 200 json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['data']['free_sms_fragment_limit'] == 250000 + assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] def test_get_service_list_has_default_permissions(client, service_factory): @@ -272,6 +272,7 @@ def test_create_service(client, sample_user): assert not json_resp['data']['research_mode'] assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] + assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] service_db = Service.query.get(json_resp['data']['id']) assert service_db.name == 'created service' @@ -316,6 +317,49 @@ def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_u assert 'Missing data for required field.' in json_resp['message']['user_id'] +def test_create_service_free_sms_fragment_limit_is_optional(client, sample_user): + data1 = { + 'name': 'service 1', + 'user_id': str(sample_user.id), + 'message_limit': 1000, + 'restricted': False, + 'active': False, + 'email_from': 'sample_user.email1', + 'created_by': str(sample_user.id), + 'free_sms_fragment_limit': 9999 + } + + auth_header = create_authorization_header() + headers = [('Content-Type', 'application/json'), auth_header] + resp = client.post( + '/service', + data=json.dumps(data1), + headers=headers) + json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 201 + assert json_resp['data']['free_sms_fragment_limit'] == 9999 + + data2 = { + 'name': 'service 2', + 'user_id': str(sample_user.id), + 'message_limit': 1000, + 'restricted': False, + 'active': False, + 'email_from': 'sample_user.email2', + 'created_by': str(sample_user.id), + } + + auth_header = create_authorization_header() + headers = [('Content-Type', 'application/json'), auth_header] + resp = client.post( + '/service', + data=json.dumps(data2), + headers=headers) + json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == 201 + assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] + + def test_should_error_if_created_by_missing(notify_api, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -550,6 +594,36 @@ def test_update_service_flags_will_remove_service_permissions(client, notify_db, assert set([p.permission for p in permissions]) == set([SMS_TYPE, EMAIL_TYPE]) +def test_update_service_free_sms_fragment_limit(client, notify_db, sample_service): + org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') + notify_db.session.add(org) + notify_db.session.commit() + + 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']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] + + data = { + 'free_sms_fragment_limit': 9999 + } + + 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']['free_sms_fragment_limit'] == 9999 + + def test_update_permissions_will_override_permission_flags(client, service_with_no_permissions): auth_header = create_authorization_header()