From 55dc7184cc935260e9ebb1590fcf497bd75ba385 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 13 Aug 2019 17:25:30 +0100 Subject: [PATCH 1/3] Add organisation_id to Service. This is the second commit in the series to add organisation_id to Service. - Data migration to update services.organisation_id from data in organisation_to_service (The rollback will lose any updates to organisation unless the script is updated to set organistion_to_service from service.organisation_id ) - Update Service.organisation relationship to a ForeignKey relationship to Organisation. - Update Organisation.services to a backref relationship to Service. --- app/dao/organisation_dao.py | 1 - app/dao/services_dao.py | 2 +- app/models.py | 10 ++----- .../versions/0303_populate_services_org_id.py | 30 +++++++++++++++++++ tests/app/dao/test_organisation_dao.py | 11 ------- tests/app/dao/test_services_dao.py | 2 ++ 6 files changed, 36 insertions(+), 20 deletions(-) create mode 100644 migrations/versions/0303_populate_services_org_id.py diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py index d1354a4d9..9cb5a03e0 100644 --- a/app/dao/organisation_dao.py +++ b/app/dao/organisation_dao.py @@ -99,7 +99,6 @@ def dao_add_service_to_organisation(service, organisation_id): id=organisation_id ).one() - organisation.services.append(service) service.organisation_id = organisation_id service.organisation_type = organisation.organisation_type service.crown = organisation.crown diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 1e1a9b9df..85d2a6e17 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -293,7 +293,7 @@ def dao_create_service( insert_service_sms_sender(service, current_app.config['FROM_NUMBER']) if organisation: - service.organisation = organisation + # service.organisation = organisation service.organisation_id = organisation.id service.organisation_type = organisation.organisation_type if organisation.email_branding: diff --git a/app/models.py b/app/models.py index f41b4e50b..0c454fb85 100644 --- a/app/models.py +++ b/app/models.py @@ -352,8 +352,8 @@ class Organisation(db.Model): services = db.relationship( 'Service', - secondary='organisation_to_service', - uselist=True) + uselist=True, + backref=db.backref('services')) agreement_signed = db.Column(db.Boolean, nullable=True) agreement_signed_at = db.Column(db.DateTime, nullable=True) @@ -480,11 +480,7 @@ class Service(db.Model, Versioned): go_live_at = db.Column(db.DateTime, nullable=True) organisation_id = db.Column(UUID(as_uuid=True), db.ForeignKey('organisation.id'), index=True, nullable=True) - organisation = db.relationship( - 'Organisation', - secondary=organisation_to_service, - uselist=False, - single_parent=True) + organisation = db.relationship('Organisation', foreign_keys=[organisation_id]) email_branding = db.relationship( 'EmailBranding', diff --git a/migrations/versions/0303_populate_services_org_id.py b/migrations/versions/0303_populate_services_org_id.py new file mode 100644 index 000000000..eb2963cd0 --- /dev/null +++ b/migrations/versions/0303_populate_services_org_id.py @@ -0,0 +1,30 @@ +""" + +Revision ID: 0303_populate_services_org_id +Revises: 0302_add_org_id_to_services +Create Date: 2019-08-06 09:43:57.993510 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0303_populate_services_org_id' +down_revision = '0302_add_org_id_to_services' + + +def upgrade(): + sql = """ + UPDATE services + SET organisation_id = (SELECT organisation_id from organisation_to_service + where organisation_to_service.service_id = services.id) + """ + op.execute(sql) + + +def downgrade(): + sql = """ + UPDATE services + SET organisation_id = null + """ + op.execute(sql) diff --git a/tests/app/dao/test_organisation_dao.py b/tests/app/dao/test_organisation_dao.py index 39fbde4b5..aae0b3e61 100644 --- a/tests/app/dao/test_organisation_dao.py +++ b/tests/app/dao/test_organisation_dao.py @@ -178,17 +178,6 @@ def test_add_service_to_organisation(sample_service, sample_organisation): assert sample_service.organisation_id == sample_organisation.id -def test_add_service_to_multiple_organisation_raises_error(sample_service, sample_organisation): - another_org = create_organisation() - dao_add_service_to_organisation(sample_service, sample_organisation.id) - - with pytest.raises(IntegrityError): - dao_add_service_to_organisation(sample_service, another_org.id) - - assert len(sample_organisation.services) == 1 - assert sample_organisation.services[0] == sample_service - - def test_get_organisation_services(sample_service, sample_organisation): another_service = create_service(service_name='service 2') another_org = create_organisation() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index eebdc6590..384b1fa0e 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -58,6 +58,7 @@ from app.models import ( INTERNATIONAL_SMS_TYPE, LETTER_TYPE, user_folder_permissions, + Organisation ) from tests.app.db import ( create_ft_billing, @@ -124,6 +125,7 @@ def test_create_service_with_organisation(notify_db_session): dao_create_service(service, user) assert Service.query.count() == 1 service_db = Service.query.one() + organisation = Organisation.query.get(organisation.id) assert service_db.name == "service_name" assert service_db.id == service.id assert service_db.email_from == 'email_from' From c4127d0508803c856c60216d1984ee77648b4e10 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 14 Aug 2019 11:01:22 +0100 Subject: [PATCH 2/3] Improve the data migration and downgrade script. --- .../versions/0303_populate_services_org_id.py | 53 ++++++++++++++----- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/migrations/versions/0303_populate_services_org_id.py b/migrations/versions/0303_populate_services_org_id.py index eb2963cd0..0413b0bec 100644 --- a/migrations/versions/0303_populate_services_org_id.py +++ b/migrations/versions/0303_populate_services_org_id.py @@ -7,24 +7,53 @@ Create Date: 2019-08-06 09:43:57.993510 """ from alembic import op import sqlalchemy as sa -from sqlalchemy.dialects import postgresql +from sqlalchemy.sql import text revision = '0303_populate_services_org_id' down_revision = '0302_add_org_id_to_services' def upgrade(): - sql = """ - UPDATE services - SET organisation_id = (SELECT organisation_id from organisation_to_service - where organisation_to_service.service_id = services.id) - """ - op.execute(sql) + conn = op.get_bind() + results = conn.execute("select service_id, organisation_id from organisation_to_service") + org_to_service = results.fetchall() + for x in org_to_service: + sql = """ + UPDATE services + SET organisation_id = :organisation_id + WHERE id = :service_id + """ + conn.execute(text(sql), service_id=str(x.service_id), organisation_id=str(x.organisation_id)) + history_sql = """ + UPDATE services_history + SET organisation_id = :organisation_id + WHERE id = :service_id + AND version = (select max(version) from services_history sh2 where id = services_history.id); + """ + conn.execute(text(history_sql), service_id=str(x.service_id), organisation_id=str(x.organisation_id)) def downgrade(): - sql = """ - UPDATE services - SET organisation_id = null - """ - op.execute(sql) + conn = op.get_bind() + + results = conn.execute("select id, organisation_id from services where organisation_id is not null") + services = results.fetchall() + results_2 = conn.execute("select service_id, organisation_id from organisation_to_service") + org_to_service = results_2.fetchall() + + for x in services: + os = [y for y in org_to_service if y.service_id == x.id] + if len(os) == 1: + update_sql = """ + UPDATE organisation_to_service + SET organisation_id = :organisation_id + WHERE service_id = :service_id + """ + conn.execute(text(update_sql), service_id=str(x.id), organisation_id=str(x.organisation_id)) + elif len(os) == 0: + insert_sql = """ + INSERT INTO organisation_to_service(service_id, organisation_id) VALUES(:service_id, :organisation_id) + """ + conn.execute(text(insert_sql), service_id=str(x.id), organisation_id=str(x.organisation_id)) + else: + raise Exception("should only have 1 row. Service_id {}, orgid: {}".format(x.id, x.organisation_id)) From fdee6cbfb2675e5868e52d033ebf24c48b15ed85 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 14 Aug 2019 13:23:20 +0100 Subject: [PATCH 3/3] As per PR comments: - Remove Organisation.services - added backref='services' to Services.organisation relationship and removed redundant foreign_keys argument. --- app/dao/services_dao.py | 1 - app/models.py | 8 +------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 85d2a6e17..199c1a5c7 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -293,7 +293,6 @@ def dao_create_service( insert_service_sms_sender(service, current_app.config['FROM_NUMBER']) if organisation: - # service.organisation = organisation service.organisation_id = organisation.id service.organisation_type = organisation.organisation_type if organisation.email_branding: diff --git a/app/models.py b/app/models.py index 0c454fb85..1879d5827 100644 --- a/app/models.py +++ b/app/models.py @@ -349,12 +349,6 @@ class Organisation(db.Model): active = db.Column(db.Boolean, nullable=False, default=True) created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) - - services = db.relationship( - 'Service', - uselist=True, - backref=db.backref('services')) - agreement_signed = db.Column(db.Boolean, nullable=True) agreement_signed_at = db.Column(db.DateTime, nullable=True) agreement_signed_by_id = db.Column( @@ -480,7 +474,7 @@ class Service(db.Model, Versioned): go_live_at = db.Column(db.DateTime, nullable=True) organisation_id = db.Column(UUID(as_uuid=True), db.ForeignKey('organisation.id'), index=True, nullable=True) - organisation = db.relationship('Organisation', foreign_keys=[organisation_id]) + organisation = db.relationship('Organisation', backref='services') email_branding = db.relationship( 'EmailBranding',