From c4bfd5479ef6f7ac3a5d992d4bcfcf69a2ca8b1b Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 25 Jul 2018 16:06:08 +0100 Subject: [PATCH 1/6] Update EmailBranding model so it has separate name and text fields --- app/models.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models.py b/app/models.py index 61f8b79c9..ae720af72 100644 --- a/app/models.py +++ b/app/models.py @@ -205,6 +205,7 @@ class EmailBranding(db.Model): colour = db.Column(db.String(7), nullable=True) logo = db.Column(db.String(255), nullable=True) name = db.Column(db.String(255), nullable=True) + text = db.Column(db.String(255), nullable=True) def serialize(self): serialized = { @@ -212,6 +213,7 @@ class EmailBranding(db.Model): "colour": self.colour, "logo": self.logo, "name": self.name, + "text": self.text, } return serialized From c5792f4d79c8a65cc9dba8894d9245692140a343 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 25 Jul 2018 16:20:40 +0100 Subject: [PATCH 2/6] Migration file for email_branding fields update --- .../versions/0209_email_branding_update.py | 27 +++++++++++++++++++ tests/app/email_branding/test_rest.py | 7 ++--- 2 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 migrations/versions/0209_email_branding_update.py diff --git a/migrations/versions/0209_email_branding_update.py b/migrations/versions/0209_email_branding_update.py new file mode 100644 index 000000000..121989139 --- /dev/null +++ b/migrations/versions/0209_email_branding_update.py @@ -0,0 +1,27 @@ +""" + +Revision ID: 0209_email_branding_update +Revises: 84c3b6eb16b3 +Create Date: 2018-07-25 16:08:15.713656 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0209_email_branding_update' +down_revision = '84c3b6eb16b3' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('email_branding', sa.Column('text', sa.String(length=255), nullable=True)) + op.execute('UPDATE email_branding SET text = name') + op.execute('UPDATE email_branding SET name = NULL') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('email_branding', 'text') + # ### end Alembic commands ### diff --git a/tests/app/email_branding/test_rest.py b/tests/app/email_branding/test_rest.py index 19294e5ab..8df02e317 100644 --- a/tests/app/email_branding/test_rest.py +++ b/tests/app/email_branding/test_rest.py @@ -22,7 +22,7 @@ def test_get_email_branding_options(admin_request, notify_db, notify_db_session) def test_get_email_branding_by_id(admin_request, notify_db, notify_db_session): - email_branding = EmailBranding(colour='#FFFFFF', logo='/path/image.png', name='My Org') + email_branding = EmailBranding(colour='#FFFFFF', logo='/path/image.png', name='Some Org', text='My Org') notify_db.session.add(email_branding) notify_db.session.commit() @@ -32,10 +32,11 @@ def test_get_email_branding_by_id(admin_request, notify_db, notify_db_session): email_branding_id=email_branding.id ) - assert set(response['email_branding'].keys()) == {'colour', 'logo', 'name', 'id'} + assert set(response['email_branding'].keys()) == {'colour', 'logo', 'name', 'id', 'text'} assert response['email_branding']['colour'] == '#FFFFFF' assert response['email_branding']['logo'] == '/path/image.png' - assert response['email_branding']['name'] == 'My Org' + assert response['email_branding']['name'] == 'Some Org' + assert response['email_branding']['text'] == 'My Org' assert response['email_branding']['id'] == str(email_branding.id) From f69bc50985a644f90c3f59d06cb7b99a6aeb3b53 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 27 Jul 2018 10:29:58 +0100 Subject: [PATCH 3/6] Move data back before dropping the column for downgrade --- migrations/versions/0209_email_branding_update.py | 1 + 1 file changed, 1 insertion(+) diff --git a/migrations/versions/0209_email_branding_update.py b/migrations/versions/0209_email_branding_update.py index 121989139..a28f70f63 100644 --- a/migrations/versions/0209_email_branding_update.py +++ b/migrations/versions/0209_email_branding_update.py @@ -23,5 +23,6 @@ def upgrade(): def downgrade(): # ### commands auto generated by Alembic - please adjust! ### + op.execute('UPDATE email_branding SET name = text') op.drop_column('email_branding', 'text') # ### end Alembic commands ### From d8d94b431fce6de82e03872f01ed63da7a4866be Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 30 Jul 2018 13:27:49 +0100 Subject: [PATCH 4/6] Find email_branding by name --- app/dao/email_branding_dao.py | 4 ++++ tests/app/dao/test_email_branding_dao.py | 9 +++++++++ tests/app/db.py | 5 +++-- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/dao/email_branding_dao.py b/app/dao/email_branding_dao.py index 94c5cbaad..bcf4a93c8 100644 --- a/app/dao/email_branding_dao.py +++ b/app/dao/email_branding_dao.py @@ -11,6 +11,10 @@ def dao_get_email_branding_by_id(email_branding_id): return EmailBranding.query.filter_by(id=email_branding_id).one() +def dao_get_email_branding_by_name(email_branding_name): + return EmailBranding.query.filter_by(name=email_branding_name).one() + + @transactional def dao_create_email_branding(email_branding): db.session.add(email_branding) diff --git a/tests/app/dao/test_email_branding_dao.py b/tests/app/dao/test_email_branding_dao.py index 1e7553523..fc150beca 100644 --- a/tests/app/dao/test_email_branding_dao.py +++ b/tests/app/dao/test_email_branding_dao.py @@ -1,6 +1,7 @@ from app.dao.email_branding_dao import ( dao_get_email_branding_options, dao_get_email_branding_by_id, + dao_get_email_branding_by_name, dao_update_email_branding, ) from app.models import EmailBranding @@ -27,6 +28,14 @@ def test_get_email_branding_by_id_gets_correct_email_branding(notify_db, notify_ assert email_branding_from_db == email_branding +def test_get_email_branding_by_name_gets_correct_email_branding(notify_db, notify_db_session): + email_branding = create_email_branding(name="Crystal Gems") + + email_branding_from_db = dao_get_email_branding_by_name("Crystal Gems") + + assert email_branding_from_db == email_branding + + def test_update_email_branding(notify_db, notify_db_session): updated_name = 'new name' create_email_branding() diff --git a/tests/app/db.py b/tests/app/db.py index 4e275155d..c97e908c0 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -329,11 +329,12 @@ def create_service_callback_api( return service_callback_api -def create_email_branding(colour='blue', logo='test_x2.png', name='test_org_1'): +def create_email_branding(colour='blue', logo='test_x2.png', name='test_org_1', text='DisplayName'): data = { 'colour': colour, 'logo': logo, - 'name': name + 'name': name, + 'text': text, } email_branding = EmailBranding(**data) dao_create_email_branding(email_branding) From 43457715479a1bbf526ef1608864a25bf56fc633 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 31 Jul 2018 18:01:58 +0100 Subject: [PATCH 5/6] Update migration file version number for email_branding update --- ...nding_update.py => 0211_email_branding_update_.py} | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) rename migrations/versions/{0209_email_branding_update.py => 0211_email_branding_update_.py} (74%) diff --git a/migrations/versions/0209_email_branding_update.py b/migrations/versions/0211_email_branding_update_.py similarity index 74% rename from migrations/versions/0209_email_branding_update.py rename to migrations/versions/0211_email_branding_update_.py index a28f70f63..705fc8470 100644 --- a/migrations/versions/0209_email_branding_update.py +++ b/migrations/versions/0211_email_branding_update_.py @@ -1,16 +1,16 @@ """ -Revision ID: 0209_email_branding_update -Revises: 84c3b6eb16b3 -Create Date: 2018-07-25 16:08:15.713656 +Revision ID: 0211_email_branding_update +Revises: 0210_remove_monthly_billing +Create Date: 2018-07-31 18:00:20.457755 """ from alembic import op import sqlalchemy as sa -revision = '0209_email_branding_update' -down_revision = '84c3b6eb16b3' +revision = '0211_email_branding_update' +down_revision = '0210_remove_monthly_billing' def upgrade(): @@ -20,7 +20,6 @@ def upgrade(): op.execute('UPDATE email_branding SET name = NULL') # ### end Alembic commands ### - def downgrade(): # ### commands auto generated by Alembic - please adjust! ### op.execute('UPDATE email_branding SET name = text') From c30e58eaad3e4dd47fc1d3fd526be2bc79601585 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 2 Aug 2018 15:55:45 +0100 Subject: [PATCH 6/6] Instead of deleting the data in the name field just copy it to the text field. The admin app still needs to use the name column. Add the text field to the post data schemas. If the text is not in the post data, then populate it with the data in the name field. This should make the migration to text easier, and will work until we are able to update the admin app. --- app/email_branding/email_branding_schema.py | 2 + app/email_branding/rest.py | 4 ++ .../versions/0211_email_branding_update_.py | 7 +- tests/app/email_branding/test_rest.py | 69 +++++++++++++++++++ 4 files changed, 76 insertions(+), 6 deletions(-) diff --git a/app/email_branding/email_branding_schema.py b/app/email_branding/email_branding_schema.py index a200b9dc4..3da94c985 100644 --- a/app/email_branding/email_branding_schema.py +++ b/app/email_branding/email_branding_schema.py @@ -5,6 +5,7 @@ post_create_email_branding_schema = { "properties": { "colour": {"type": ["string", "null"]}, "name": {"type": ["string", "null"]}, + "text": {"type": ["string", "null"]}, "logo": {"type": ["string", "null"]} }, "required": [] @@ -17,6 +18,7 @@ post_update_email_branding_schema = { "properties": { "colour": {"type": ["string", "null"]}, "name": {"type": ["string", "null"]}, + "text": {"type": ["string", "null"]}, "logo": {"type": ["string", "null"]} }, "required": [] diff --git a/app/email_branding/rest.py b/app/email_branding/rest.py index 277d27714..356e19bac 100644 --- a/app/email_branding/rest.py +++ b/app/email_branding/rest.py @@ -37,6 +37,8 @@ def create_email_branding(): validate(data, post_create_email_branding_schema) email_branding = EmailBranding(**data) + if not data.get('text'): + email_branding.text = email_branding.name dao_create_email_branding(email_branding) return jsonify(data=email_branding.serialize()), 201 @@ -49,6 +51,8 @@ def update_email_branding(email_branding_id): validate(data, post_update_email_branding_schema) fetched_email_branding = dao_get_email_branding_by_id(email_branding_id) + if not data.get('text') and data.get('name'): + data['text'] = data['name'] dao_update_email_branding(fetched_email_branding, **data) return jsonify(data=fetched_email_branding.serialize()), 200 diff --git a/migrations/versions/0211_email_branding_update_.py b/migrations/versions/0211_email_branding_update_.py index 705fc8470..f3e210ffe 100644 --- a/migrations/versions/0211_email_branding_update_.py +++ b/migrations/versions/0211_email_branding_update_.py @@ -14,14 +14,9 @@ down_revision = '0210_remove_monthly_billing' def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### op.add_column('email_branding', sa.Column('text', sa.String(length=255), nullable=True)) op.execute('UPDATE email_branding SET text = name') - op.execute('UPDATE email_branding SET name = NULL') - # ### end Alembic commands ### + def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.execute('UPDATE email_branding SET name = text') op.drop_column('email_branding', 'text') - # ### end Alembic commands ### diff --git a/tests/app/email_branding/test_rest.py b/tests/app/email_branding/test_rest.py index 8df02e317..3c460b137 100644 --- a/tests/app/email_branding/test_rest.py +++ b/tests/app/email_branding/test_rest.py @@ -54,6 +54,7 @@ def test_post_create_email_branding(admin_request, notify_db_session): assert data['name'] == response['data']['name'] assert data['colour'] == response['data']['colour'] assert data['logo'] == response['data']['logo'] + assert data['name'] == response['data']['text'] def test_post_create_email_branding_without_logo_is_ok(admin_request, notify_db_session): @@ -81,6 +82,42 @@ def test_post_create_email_branding_without_name_or_colour_is_valid(admin_reques assert response['data']['logo'] == data['logo'] assert response['data']['name'] is None assert response['data']['colour'] is None + assert response['data']['text'] is None + + +def test_post_create_email_branding_with_text(admin_request, notify_db_session): + data = { + 'text': 'text for brand', + 'logo': 'images/text_x2.png' + } + response = admin_request.post( + 'email_branding.create_email_branding', + _data=data, + _expected_status=201 + ) + + assert response['data']['logo'] == data['logo'] + assert response['data']['name'] is None + assert response['data']['colour'] is None + assert response['data']['text'] == 'text for brand' + + +def test_post_create_email_branding_with_text_and_name(admin_request, notify_db_session): + data = { + 'name': 'name for brand', + 'text': 'text for brand', + 'logo': 'images/text_x2.png' + } + response = admin_request.post( + 'email_branding.create_email_branding', + _data=data, + _expected_status=201 + ) + + assert response['data']['logo'] == data['logo'] + assert response['data']['name'] == 'name for brand' + assert response['data']['colour'] is None + assert response['data']['text'] == 'text for brand' @pytest.mark.parametrize('data_update', [ @@ -108,6 +145,38 @@ def test_post_update_email_branding_updates_field(admin_request, notify_db_sessi email_branding = EmailBranding.query.all() + assert len(email_branding) == 1 + assert str(email_branding[0].id) == email_branding_id + for key in data_update.keys(): + assert getattr(email_branding[0], key) == data_update[key] + assert email_branding[0].text == email_branding[0].name + + +@pytest.mark.parametrize('data_update', [ + ({'text': 'text email branding'}), + ({'text': 'new text', 'name': 'new name'}), +]) +def test_post_update_email_branding_updates_field_with_text(admin_request, notify_db_session, data_update): + data = { + 'name': 'test email_branding', + 'logo': 'images/text_x2.png' + } + response = admin_request.post( + 'email_branding.create_email_branding', + _data=data, + _expected_status=201 + ) + + email_branding_id = response['data']['id'] + + response = admin_request.post( + 'email_branding.update_email_branding', + _data=data_update, + email_branding_id=email_branding_id + ) + + email_branding = EmailBranding.query.all() + assert len(email_branding) == 1 assert str(email_branding[0].id) == email_branding_id for key in data_update.keys():