From 64428368d1160770e33de0fc648466f67da9bddf Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 9 Apr 2019 14:33:38 +0100 Subject: [PATCH] Update EmailBranding.name to be not null and unique. Needed to update old migration scripts so that the email_branding name is not null when creating the test dbs. This should no affect the migrations elsewhere. --- app/email_branding/email_branding_schema.py | 4 +-- app/models.py | 2 +- migrations/versions/0093_data_gov_uk.py | 2 +- migrations/versions/0099_tfl_dar.py | 2 +- migrations/versions/0101_een_logo.py | 2 +- .../versions/0286_add_unique_email_name.py | 26 ++++++++++++++ tests/app/email_branding/test_rest.py | 36 +++++++++++++------ 7 files changed, 58 insertions(+), 16 deletions(-) create mode 100644 migrations/versions/0286_add_unique_email_name.py diff --git a/app/email_branding/email_branding_schema.py b/app/email_branding/email_branding_schema.py index 6dbfaf761..2f562d2ae 100644 --- a/app/email_branding/email_branding_schema.py +++ b/app/email_branding/email_branding_schema.py @@ -6,13 +6,13 @@ post_create_email_branding_schema = { "type": "object", "properties": { "colour": {"type": ["string", "null"]}, - "name": {"type": ["string", "null"]}, + "name": {"type": "string"}, "text": {"type": ["string", "null"]}, "logo": {"type": ["string", "null"]}, "domain": {"type": ["string", "null"]}, "brand_type": {"enum": BRANDING_TYPES}, }, - "required": [] + "required": ["name"] } post_update_email_branding_schema = { diff --git a/app/models.py b/app/models.py index a381881ba..41ba0398f 100644 --- a/app/models.py +++ b/app/models.py @@ -219,7 +219,7 @@ class EmailBranding(db.Model): id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) colour = db.Column(db.String(7), nullable=True) logo = db.Column(db.String(255), nullable=True) - name = db.Column(db.String(255), nullable=True) + name = db.Column(db.String(255), unique=True, nullable=False) text = db.Column(db.String(255), nullable=True) domain = db.Column(db.Text, unique=True, nullable=True) brand_type = db.Column( diff --git a/migrations/versions/0093_data_gov_uk.py b/migrations/versions/0093_data_gov_uk.py index fbe22d38a..6053ad487 100644 --- a/migrations/versions/0093_data_gov_uk.py +++ b/migrations/versions/0093_data_gov_uk.py @@ -22,7 +22,7 @@ def upgrade(): '{}', '', 'data_gov_uk_x2.png', - '' + 'data gov.uk' )""".format(DATA_GOV_UK_ID)) diff --git a/migrations/versions/0099_tfl_dar.py b/migrations/versions/0099_tfl_dar.py index 0f52ec9e4..63016dbde 100644 --- a/migrations/versions/0099_tfl_dar.py +++ b/migrations/versions/0099_tfl_dar.py @@ -22,7 +22,7 @@ def upgrade(): '{}', '', 'tfl_dar_x2.png', - '' + 'tfl' )""".format(TFL_DAR_ID)) diff --git a/migrations/versions/0101_een_logo.py b/migrations/versions/0101_een_logo.py index 38494ad23..97c0b1fe0 100644 --- a/migrations/versions/0101_een_logo.py +++ b/migrations/versions/0101_een_logo.py @@ -20,7 +20,7 @@ def upgrade(): '{}', '', 'een_x2.png', - '' + 'een' )""".format(ENTERPRISE_EUROPE_NETWORK_ID)) diff --git a/migrations/versions/0286_add_unique_email_name.py b/migrations/versions/0286_add_unique_email_name.py new file mode 100644 index 000000000..1209902fe --- /dev/null +++ b/migrations/versions/0286_add_unique_email_name.py @@ -0,0 +1,26 @@ +""" + +Revision ID: 0286_add_unique_email_name +Revises: 0285_default_org_branding +Create Date: 2019-04-09 13:01:13.892249 + +""" +from alembic import op +import sqlalchemy as sa + +revision = '0286_add_unique_email_name' +down_revision = '0285_default_org_branding' + + +def upgrade(): + op.alter_column('email_branding', 'name', + existing_type=sa.VARCHAR(length=255), + nullable=False) + op.create_unique_constraint('uq_email_branding_name', 'email_branding', ['name']) + + +def downgrade(): + op.drop_constraint('uq_email_branding_name', 'email_branding', type_='unique') + op.alter_column('email_branding', 'name', + existing_type=sa.VARCHAR(length=255), + nullable=True) diff --git a/tests/app/email_branding/test_rest.py b/tests/app/email_branding/test_rest.py index 7c7954887..0a2655eee 100644 --- a/tests/app/email_branding/test_rest.py +++ b/tests/app/email_branding/test_rest.py @@ -92,9 +92,10 @@ def test_post_create_email_branding_without_logo_is_ok(admin_request, notify_db_ assert not response['data']['logo'] -def test_post_create_email_branding_without_name_or_colour_is_valid(admin_request, notify_db_session): +def test_post_create_email_branding_colour_is_valid(admin_request, notify_db_session): data = { - 'logo': 'images/text_x2.png' + 'logo': 'images/text_x2.png', + 'name': 'test branding' } response = admin_request.post( 'email_branding.create_email_branding', @@ -103,15 +104,16 @@ 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']['name'] == 'test branding' assert response['data']['colour'] is None - assert response['data']['text'] is None + assert response['data']['text'] == 'test branding' def test_post_create_email_branding_with_text(admin_request, notify_db_session): data = { 'text': 'text for brand', - 'logo': 'images/text_x2.png' + 'logo': 'images/text_x2.png', + 'name': 'test branding' } response = admin_request.post( 'email_branding.create_email_branding', @@ -120,7 +122,7 @@ def test_post_create_email_branding_with_text(admin_request, notify_db_session): ) assert response['data']['logo'] == data['logo'] - assert response['data']['name'] is None + assert response['data']['name'] == 'test branding' assert response['data']['colour'] is None assert response['data']['text'] == 'text for brand' @@ -161,6 +163,20 @@ def test_post_create_email_branding_with_text_as_none_and_name(admin_request, no assert response['data']['text'] is None +def test_post_create_email_branding_returns_400_when_name_is_missing(admin_request, notify_db_session): + data = { + 'text': 'some text', + 'logo': 'images/text_x2.png' + } + response = admin_request.post( + 'email_branding.create_email_branding', + _data=data, + _expected_status=400 + ) + + assert response['errors'][0]['message'] == 'name is a required property' + + @pytest.mark.parametrize('data_update', [ ({'name': 'test email_branding 1'}), ({'logo': 'images/text_x3.png', 'colour': '#ffffff'}), @@ -260,11 +276,11 @@ def test_update_email_branding_reject_invalid_brand_type(admin_request, notify_d def test_400_for_duplicate_domain(admin_request, notify_db_session): - branding_1 = create_email_branding() - branding_2 = create_email_branding() + branding_1 = create_email_branding(name='first brand') + branding_2 = create_email_branding(name='second brand') admin_request.post( 'email_branding.update_email_branding', - _data={'domain': 'example.com'}, + _data={'domain': 'example.com', }, email_branding_id=branding_1.id, ) @@ -279,7 +295,7 @@ def test_400_for_duplicate_domain(admin_request, notify_db_session): response = admin_request.post( 'email_branding.create_email_branding', - _data={'domain': 'example.com'}, + _data={'domain': 'example.com', 'name': 'another brand'}, _expected_status=400, ) assert response['result'] == 'error'