From f73319f5ef886e66969a08c42bc338d74516f713 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 4 Dec 2017 13:24:53 +0000 Subject: [PATCH 1/4] Add and populate crown column to services and services_history - Added the boolean 'crown' column to services and services_history tables - We populate this column in the same migration script by checking the 'organisation_type' of a service --- app/dao/services_dao.py | 1 + app/models.py | 1 + .../versions/0149_add_crown_to_services.py | 42 +++++++++++++++++++ tests/app/dao/test_services_dao.py | 2 + 4 files changed, 46 insertions(+) create mode 100644 migrations/versions/0149_add_crown_to_services.py diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 8753b2dc4..0b5f3e245 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -172,6 +172,7 @@ def dao_create_service(service, user, service_id=None, service_permissions=None) service.id = service_id or uuid.uuid4() # must be set now so version history model can use same id service.active = True service.research_mode = False + service.crown = service.organisation_type == 'central' for permission in service_permissions: service_permission = ServicePermission(service_id=service.id, permission=permission) diff --git a/app/models.py b/app/models.py index 9d81073c7..fdd00fa9a 100644 --- a/app/models.py +++ b/app/models.py @@ -249,6 +249,7 @@ class Service(db.Model, Versioned): db.String(255), nullable=True, ) + crown = db.Column(db.Boolean, index=False, nullable=False, default=True) association_proxy('permissions', 'service_permission_types') diff --git a/migrations/versions/0149_add_crown_to_services.py b/migrations/versions/0149_add_crown_to_services.py new file mode 100644 index 000000000..1b34f9e2a --- /dev/null +++ b/migrations/versions/0149_add_crown_to_services.py @@ -0,0 +1,42 @@ +""" + +Revision ID: 0149_add_crown_column_to_services +Revises: 0148_add_letters_as_pdf_svc_perm +Create Date: 2017-12-04 12:13:35.268712 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0149_add_crown_to_services' +down_revision = '0148_add_letters_as_pdf_svc_perm' + + +def upgrade(): + op.add_column('services', sa.Column('crown', sa.Boolean(), nullable=True)) + op.execute(""" + update services set crown = True + where organisation_type = 'central' + """) + op.execute(""" + update services set crown = False + where crown is null + """) + op.alter_column('services', 'crown', nullable=False) + + op.add_column('services_history', sa.Column('crown', sa.Boolean(), nullable=True)) + op.execute(""" + update services_history set crown = True + where organisation_type = 'central' + """) + op.execute(""" + update services_history set crown = False + where crown is null + """) + op.alter_column('services_history', 'crown', nullable=False) + + +def downgrade(): + op.drop_column('services', 'crown') + op.drop_column('services_history', 'crown') diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index d601bd59d..21a88eecf 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -87,6 +87,7 @@ def test_create_service(sample_user): email_from="email_from", message_limit=1000, restricted=False, + organisation_type='central', created_by=sample_user) dao_create_service(service, sample_user) assert Service.query.count() == 1 @@ -100,6 +101,7 @@ def test_create_service(sample_user): assert service_db.prefix_sms is True assert service.active is True assert sample_user in service_db.users + assert service_db.crown is True def test_cannot_create_two_services_with_same_name(sample_user): From 7fa4e7ffc706d1399a4a5b5912f5a41caffc46bf Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 4 Dec 2017 16:07:26 +0000 Subject: [PATCH 2/4] Set crown if organisation_type is updated on service. Add some tests. Update the initial values of crown. --- app/service/rest.py | 4 ++++ .../versions/0149_add_crown_to_services.py | 8 ++++++++ tests/app/dao/test_services_dao.py | 3 +++ tests/app/service/test_rest.py | 20 +++++++++++++++++++ 4 files changed, 35 insertions(+) diff --git a/app/service/rest.py b/app/service/rest.py index efb7044a3..c4d0e2bb9 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -178,7 +178,11 @@ def update_service(service_id): service_going_live = fetched_service.restricted and not req_json.get('restricted', True) current_data = dict(service_schema.dump(fetched_service).data.items()) current_data.update(request.get_json()) + update_dict = service_schema.load(current_data).data + org_type = req_json.get('organisation_type', None) + if org_type: + update_dict.crown = org_type == 'central' dao_update_service(update_dict) # bridging code between frontend is deployed and data has not been migrated yet. Can only update current year diff --git a/migrations/versions/0149_add_crown_to_services.py b/migrations/versions/0149_add_crown_to_services.py index 1b34f9e2a..bfbbf976c 100644 --- a/migrations/versions/0149_add_crown_to_services.py +++ b/migrations/versions/0149_add_crown_to_services.py @@ -19,6 +19,10 @@ def upgrade(): update services set crown = True where organisation_type = 'central' """) + op.execute(""" + update services set crown = True + where organisation_type is null + """) op.execute(""" update services set crown = False where crown is null @@ -30,6 +34,10 @@ def upgrade(): update services_history set crown = True where organisation_type = 'central' """) + op.execute(""" + update services_history set crown = True + where organisation_type is null + """) op.execute(""" update services_history set crown = False where crown is null diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 21a88eecf..2caf092fe 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -97,10 +97,13 @@ def test_create_service(sample_user): assert service_db.id == service.id assert service_db.branding == BRANDING_GOVUK assert service_db.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT + assert service_db.email_from == 'email_from' assert service_db.research_mode is False assert service_db.prefix_sms is True assert service.active is True assert sample_user in service_db.users + assert service_db.free_sms_fragment_limit == 250000 + assert service_db.organisation_type == 'central' assert service_db.crown is True diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 73679001f..553742a26 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -536,6 +536,26 @@ def test_update_service_flags(client, sample_service): assert set(result['data']['permissions']) == set([LETTER_TYPE, INTERNATIONAL_SMS_TYPE]) +@pytest.mark.parametrize("org_type, expected", + [("central", True), + ('local', False), + ("nhs", False)]) +def test_update_service_sets_crown(client, sample_service, org_type, expected): + data = { + 'organisation_type': org_type, + } + 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']['crown'] is expected + + @pytest.fixture(scope='function') def service_with_no_permissions(notify_db, notify_db_session): return create_service(service_permissions=[]) From b4a1d635f75029022d949c88f896ddf7afd1e1c8 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 5 Dec 2017 10:27:29 +0000 Subject: [PATCH 3/4] Rebuild the letter_rates table to include everything it needs. The current letter_rates table is not used so it is ok to drop and re-create it --- app/models.py | 15 ++---- .../versions/0150_refactor_letter_rates.py | 47 +++++++++++++++++++ 2 files changed, 52 insertions(+), 10 deletions(-) create mode 100644 migrations/versions/0150_refactor_letter_rates.py diff --git a/app/models.py b/app/models.py index 9d81073c7..0a65ae0a7 100644 --- a/app/models.py +++ b/app/models.py @@ -1484,17 +1484,12 @@ class LetterRate(db.Model): __tablename__ = 'letter_rates' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - valid_from = valid_from = db.Column(db.DateTime, nullable=False) - - -class LetterRateDetail(db.Model): - __tablename__ = 'letter_rate_details' - - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - letter_rate_id = db.Column(UUID(as_uuid=True), db.ForeignKey('letter_rates.id'), index=True, nullable=False) - letter_rate = db.relationship('LetterRate', backref='letter_rates') - page_total = db.Column(db.Integer, nullable=False) + start_date = db.Column(db.DateTime, nullable=False) + end_date = db.Column(db.DateTime, nullable=True) + sheet_total = db.Column(db.Integer, nullable=False) # double sided sheet rate = db.Column(db.Numeric(), nullable=False) + crown = db.Column(db.Boolean, nullable=False) + post_class = db.Column(db.String, nullable=False) class MonthlyBilling(db.Model): diff --git a/migrations/versions/0150_refactor_letter_rates.py b/migrations/versions/0150_refactor_letter_rates.py new file mode 100644 index 000000000..2429e5222 --- /dev/null +++ b/migrations/versions/0150_refactor_letter_rates.py @@ -0,0 +1,47 @@ +""" + +Revision ID: 0150_refactor_letter_rates +Revises: 0148_add_letters_as_pdf_svc_perm +Create Date: 2017-12-05 10:24:41.232128 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0150_refactor_letter_rates' +down_revision = '0148_add_letters_as_pdf_svc_perm' + + +def upgrade(): + op.drop_table('letter_rate_details') + op.drop_table('letter_rates') + op.create_table('letter_rates', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('start_date', sa.DateTime(), nullable=False), + sa.Column('end_date', sa.DateTime(), nullable=True), + sa.Column('sheet_total', sa.Integer(), nullable=False), + sa.Column('rate', sa.Numeric(), nullable=False), + sa.Column('crown', sa.Boolean(), nullable=False), + sa.Column('post_class', sa.String(), nullable=False), + sa.PrimaryKeyConstraint('id') + ) + + +def downgrade(): + op.drop_table('letter_rates') + op.create_table('letter_rates', + sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('valid_from', postgresql.TIMESTAMP(), autoincrement=False, nullable=False), + sa.PrimaryKeyConstraint('id', name='letter_rates_pkey'), + postgresql_ignore_search_path=False + ) + op.create_table('letter_rate_details', + sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('letter_rate_id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('page_total', sa.INTEGER(), autoincrement=False, nullable=False), + sa.Column('rate', sa.NUMERIC(), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint(['letter_rate_id'], ['letter_rates.id'], + name='letter_rate_details_letter_rate_id_fkey'), + sa.PrimaryKeyConstraint('id', name='letter_rate_details_pkey') + ) From 956907c170428d3e0846e1bc97bc11be31a5ff39 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 5 Dec 2017 10:54:55 +0000 Subject: [PATCH 4/4] Revert the last commit. It was intended for another branch. --- app/models.py | 15 ++++-- .../versions/0150_refactor_letter_rates.py | 47 ------------------- 2 files changed, 10 insertions(+), 52 deletions(-) delete mode 100644 migrations/versions/0150_refactor_letter_rates.py diff --git a/app/models.py b/app/models.py index d5c4ea5af..fdd00fa9a 100644 --- a/app/models.py +++ b/app/models.py @@ -1485,12 +1485,17 @@ class LetterRate(db.Model): __tablename__ = 'letter_rates' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - start_date = db.Column(db.DateTime, nullable=False) - end_date = db.Column(db.DateTime, nullable=True) - sheet_total = db.Column(db.Integer, nullable=False) # double sided sheet + valid_from = valid_from = db.Column(db.DateTime, nullable=False) + + +class LetterRateDetail(db.Model): + __tablename__ = 'letter_rate_details' + + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + letter_rate_id = db.Column(UUID(as_uuid=True), db.ForeignKey('letter_rates.id'), index=True, nullable=False) + letter_rate = db.relationship('LetterRate', backref='letter_rates') + page_total = db.Column(db.Integer, nullable=False) rate = db.Column(db.Numeric(), nullable=False) - crown = db.Column(db.Boolean, nullable=False) - post_class = db.Column(db.String, nullable=False) class MonthlyBilling(db.Model): diff --git a/migrations/versions/0150_refactor_letter_rates.py b/migrations/versions/0150_refactor_letter_rates.py deleted file mode 100644 index 2429e5222..000000000 --- a/migrations/versions/0150_refactor_letter_rates.py +++ /dev/null @@ -1,47 +0,0 @@ -""" - -Revision ID: 0150_refactor_letter_rates -Revises: 0148_add_letters_as_pdf_svc_perm -Create Date: 2017-12-05 10:24:41.232128 - -""" -from alembic import op -import sqlalchemy as sa -from sqlalchemy.dialects import postgresql - -revision = '0150_refactor_letter_rates' -down_revision = '0148_add_letters_as_pdf_svc_perm' - - -def upgrade(): - op.drop_table('letter_rate_details') - op.drop_table('letter_rates') - op.create_table('letter_rates', - sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), - sa.Column('start_date', sa.DateTime(), nullable=False), - sa.Column('end_date', sa.DateTime(), nullable=True), - sa.Column('sheet_total', sa.Integer(), nullable=False), - sa.Column('rate', sa.Numeric(), nullable=False), - sa.Column('crown', sa.Boolean(), nullable=False), - sa.Column('post_class', sa.String(), nullable=False), - sa.PrimaryKeyConstraint('id') - ) - - -def downgrade(): - op.drop_table('letter_rates') - op.create_table('letter_rates', - sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), - sa.Column('valid_from', postgresql.TIMESTAMP(), autoincrement=False, nullable=False), - sa.PrimaryKeyConstraint('id', name='letter_rates_pkey'), - postgresql_ignore_search_path=False - ) - op.create_table('letter_rate_details', - sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), - sa.Column('letter_rate_id', postgresql.UUID(), autoincrement=False, nullable=False), - sa.Column('page_total', sa.INTEGER(), autoincrement=False, nullable=False), - sa.Column('rate', sa.NUMERIC(), autoincrement=False, nullable=False), - sa.ForeignKeyConstraint(['letter_rate_id'], ['letter_rates.id'], - name='letter_rate_details_letter_rate_id_fkey'), - sa.PrimaryKeyConstraint('id', name='letter_rate_details_pkey') - )