From f95282a84d03af92d2fc0f915c4e0b9f24150fe2 Mon Sep 17 00:00:00 2001 From: venusbb Date: Tue, 10 Oct 2017 15:33:31 +0100 Subject: [PATCH 1/3] Add column, free_sms_fragment_limit, to services & services_history - Created new column in both tables - Modified model and Service schema - Modifed existing test --- app/models.py | 5 +--- app/schemas.py | 12 ++-------- .../0124_add_free_sms_fragment_limit.py | 23 +++++++++++++++++++ tests/app/conftest.py | 4 +++- 4 files changed, 29 insertions(+), 15 deletions(-) create mode 100644 migrations/versions/0124_add_free_sms_fragment_limit.py 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/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: From 2001bfca36d2646f9bac343efb0a8edb37572f05 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 12 Oct 2017 12:07:52 +0100 Subject: [PATCH 2/3] Set free_sms_fragment_limit when creating a service The free_sms_fragment_limit of a service is set when the service is created. If a value for the free_sms_fragment_limit is not given, the default value is used. --- app/service/rest.py | 4 ++ tests/app/service/test_rest.py | 78 +++++++++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) 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/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() From 9322ed0ca34469f08cf23204343a84786d63b5af Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 11 Oct 2017 14:00:53 +0100 Subject: [PATCH 3/3] Add command to populate the free_sms_fragment_limit Added a one-off command to set values of the free_sms_fragment_limit in the services table and the services_history table to the default of 250000. --- app/commands.py | 24 ++++++++++++++++++++++++ application.py | 2 ++ 2 files changed, 26 insertions(+) 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/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