From 79815b4f8a904ba784cb11a78479e0c724fb1a95 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 13 Sep 2018 17:06:36 +0100 Subject: [PATCH] In order to support sending letters by first class we need a way to know how the service wants us to send the letter. To start with this will be an attribute on the service, at the time the notification is created it will look at Service.letter_class to decide what class to use for the letter. This PR adds Service.letter class as a nullable column. Updated the create_service and update_service method to default the value to second. Subsequent PRs will add the check constraint to ensure we only get first or second in the letter_class column and make that column nullable. This can't be done all at once because it will cause an error if someone inserts or updates a service during the deploy. --- app/dao/services_dao.py | 2 ++ app/models.py | 1 + migrations/versions/0225_letter_class.py | 23 +++++++++++++++++++++++ tests/app/dao/test_services_dao.py | 22 ++++++++++++++++++++++ tests/app/service/test_rest.py | 1 + 5 files changed, 49 insertions(+) create mode 100644 migrations/versions/0225_letter_class.py diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 40918f7e7..fc3881d25 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -167,6 +167,7 @@ def dao_create_service(service, user, service_id=None, service_permissions=None) service.active = True service.research_mode = False service.crown = service.organisation_type == 'central' + service.letter_class = 'second' for permission in service_permissions: service_permission = ServicePermission(service_id=service.id, permission=permission) @@ -180,6 +181,7 @@ def dao_create_service(service, user, service_id=None, service_permissions=None) @transactional @version_class(Service) def dao_update_service(service): + service.letter_class = service.letter_class or 'second' db.session.add(service) diff --git a/app/models.py b/app/models.py index 86e2ee595..a518ca451 100644 --- a/app/models.py +++ b/app/models.py @@ -353,6 +353,7 @@ class Service(db.Model, Versioned): crown = db.Column(db.Boolean, index=False, nullable=False, default=True) rate_limit = db.Column(db.Integer, index=False, nullable=False, default=3000) contact_link = db.Column(db.String(255), nullable=True, unique=False) + letter_class = db.Column(db.String(255), index=False, nullable=True) organisation = db.relationship( 'Organisation', diff --git a/migrations/versions/0225_letter_class.py b/migrations/versions/0225_letter_class.py new file mode 100644 index 000000000..225426b57 --- /dev/null +++ b/migrations/versions/0225_letter_class.py @@ -0,0 +1,23 @@ +""" + +Revision ID: 0225_letter_class +Revises: 0224_returned_letter_status +Create Date: 2018-09-13 16:23:59.168877 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0225_letter_class' +down_revision = '0224_returned_letter_status' + + +def upgrade(): + op.add_column('services', sa.Column('letter_class', sa.String(length=255), nullable=True)) + op.add_column('services_history', sa.Column('letter_class', sa.String(length=255), nullable=True)) + + +def downgrade(): + op.drop_column('services_history', 'letter_class') + op.drop_column('services', 'letter_class') diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index a4045e96d..eb2f1ff18 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -100,6 +100,7 @@ def test_create_service(sample_user): assert service_db.email_from == 'email_from' assert service_db.research_mode is False assert service_db.prefix_sms is True + assert service_db.letter_class == 'second' assert service.active is True assert sample_user in service_db.users assert service_db.organisation_type == 'central' @@ -345,10 +346,12 @@ def test_create_service_creates_a_history_record_with_current_data(sample_user): assert service_from_db.name == service_history.name assert service_from_db.version == 1 assert service_from_db.version == service_history.version + assert service_from_db.letter_class == 'second' assert sample_user.id == service_history.created_by_id assert service_from_db.created_by.id == service_history.created_by_id assert service_from_db.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT assert service_history.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT + assert service_history.letter_class == 'second' def test_update_service_creates_a_history_record_with_current_data(sample_user): @@ -424,6 +427,25 @@ def test_update_service_permission_creates_a_history_record_with_current_data(sa assert Service.get_history_model().query.filter_by(name='service_name').all()[2].version == 3 +def test_update_service_set_letter_class_to_default(sample_user): + assert Service.query.count() == 0 + assert Service.get_history_model().query.count() == 0 + service = Service(name="service_name", + email_from="email_from", + message_limit=1000, + restricted=False, + created_by=sample_user) + dao_create_service(service, sample_user) + + service_from_db = Service.query.first() + service_from_db.letter_class = None + dao_update_service(service_from_db) + service_with_update = Service.query.first() + assert service_with_update.letter_class == 'second' + service_history_with_update = Service.get_history_model().query.filter_by(version=2) + assert service_history_with_update.letter_class == 'second' + + def test_create_service_and_history_is_transactional(sample_user): assert Service.query.count() == 0 assert Service.get_history_model().query.count() == 0 diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index e6d4942d6..2640bf478 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -138,6 +138,7 @@ def test_get_service_by_id(admin_request, sample_service): assert 'branding' not in json_resp['data'] assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['prefix_sms'] is True + assert json_resp['data']['letter_class'] == 'second' @pytest.mark.parametrize('detailed', [True, False])