From 0b28766442007ec18f15e6d96e43931fcdc4d7f4 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 20 May 2020 18:28:35 +0100 Subject: [PATCH] Reverts the new postage constraints Reverts https://github.com/alphagov/notifications-api/pull/2843 and https://github.com/alphagov/notifications-api/pull/2848 --- app/models.py | 18 ++-- app/schema_validation/__init__.py | 4 +- .../0321_update_postage_constraint_1.py | 84 ------------------- .../0322_update_postage_constraint_2.py | 45 ---------- .../0323_update_postage_constraint_3.py | 22 ----- tests/app/service/test_rest.py | 3 +- .../test_post_letter_notifications.py | 2 +- 7 files changed, 15 insertions(+), 163 deletions(-) delete mode 100644 migrations/versions/0321_update_postage_constraint_1.py delete mode 100644 migrations/versions/0322_update_postage_constraint_2.py delete mode 100644 migrations/versions/0323_update_postage_constraint_3.py diff --git a/app/models.py b/app/models.py index 447ba22d9..0e0afd521 100644 --- a/app/models.py +++ b/app/models.py @@ -903,7 +903,7 @@ class TemplateBase(db.Model): postage = db.Column(db.String, nullable=True) CheckConstraint(""" CASE WHEN template_type = 'letter' THEN - postage is not null and postage in ('first', 'second', 'europe', 'rest-of-world') + postage is not null and postage in ('first', 'second') ELSE postage is null END @@ -1353,13 +1353,10 @@ DVLA_RESPONSE_STATUS_SENT = 'Sent' FIRST_CLASS = 'first' SECOND_CLASS = 'second' -EUROPE = 'europe' -REST_OF_WORLD = 'rest-of-world' +POSTAGE_TYPES = [FIRST_CLASS, SECOND_CLASS] RESOLVE_POSTAGE_FOR_FILE_NAME = { FIRST_CLASS: 1, - SECOND_CLASS: 2, - EUROPE: 'E', - REST_OF_WORLD: 'N', + SECOND_CLASS: 2 } @@ -1436,7 +1433,7 @@ class Notification(db.Model): postage = db.Column(db.String, nullable=True) CheckConstraint(""" CASE WHEN notification_type = 'letter' THEN - postage is not null and postage in ('first', 'second', 'europe', 'rest-of-world') + postage is not null and postage in ('first', 'second') ELSE postage is null END @@ -1701,6 +1698,13 @@ class NotificationHistory(db.Model, HistoryModel): created_by_id = db.Column(UUID(as_uuid=True), nullable=True) postage = db.Column(db.String, nullable=True) + CheckConstraint(""" + CASE WHEN notification_type = 'letter' THEN + postage is not null and postage in ('first', 'second') + ELSE + postage is null + END + """) document_download_count = db.Column(db.Integer, nullable=True) diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 0ddd51f16..e1995f528 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -34,8 +34,8 @@ def validate_schema_email_address(instance): @format_checker.checks('postage', raises=ValidationError) def validate_schema_postage(instance): if isinstance(instance, str): - if instance not in ["first", "second", "europe", "rest-of-world"]: - raise ValidationError("invalid. It must be first, second, europe or rest-of-world.") + if instance not in ["first", "second"]: + raise ValidationError("invalid. It must be either first or second.") return True diff --git a/migrations/versions/0321_update_postage_constraint_1.py b/migrations/versions/0321_update_postage_constraint_1.py deleted file mode 100644 index 53f9d2ea2..000000000 --- a/migrations/versions/0321_update_postage_constraint_1.py +++ /dev/null @@ -1,84 +0,0 @@ -""" - -Revision ID: 0321_update_postage_constraint_1 -Revises: 0320_optimise_notifications -Create Date: 2020-05-12 16:17:21.874281 - -""" -import os - -from alembic import op - - -revision = '0321_update_postage_constraint_1' -down_revision = '0320_optimise_notifications' -environment = os.environ['NOTIFY_ENVIRONMENT'] - - -def upgrade(): - op.drop_constraint('chk_notifications_postage_null', 'notifications') - op.execute(""" - ALTER TABLE notifications ADD CONSTRAINT "chk_notifications_postage_null" - CHECK ( - CASE WHEN notification_type = 'letter' THEN - postage is not null and postage in ('first', 'second', 'europe', 'rest-of-world') - ELSE - postage is null - END - ) - NOT VALID - """) - if environment not in ["live", "production"]: - op.execute('ALTER TABLE notification_history DROP CONSTRAINT IF EXISTS chk_notification_history_postage_null') - op.execute('COMMIT') - - -def downgrade(): - pass - # To downgrade this migration and migrations 0322 and 0323 * LOCALLY ONLY * use the following code. - # This should not be used in production - it will lock the tables for a long time - # - # op.drop_constraint('chk_notifications_postage_null', 'notifications') - # op.drop_constraint('chk_templates_postage', 'templates') - # op.drop_constraint('chk_templates_history_postage', 'templates_history') - # - # op.execute(""" - # ALTER TABLE notifications ADD CONSTRAINT "chk_notifications_postage_null" - # CHECK ( - # CASE WHEN notification_type = 'letter' THEN - # postage is not null and postage in ('first', 'second') - # ELSE - # postage is null - # END - # ) - # """) - # op.execute(""" - # ALTER TABLE notification_history ADD CONSTRAINT "chk_notification_history_postage_null" - # CHECK ( - # CASE WHEN notification_type = 'letter' THEN - # postage is not null and postage in ('first', 'second') - # ELSE - # postage is null - # END - # ) - # """) - # op.execute(""" - # ALTER TABLE templates ADD CONSTRAINT "chk_templates_postage" - # CHECK ( - # CASE WHEN template_type = 'letter' THEN - # postage is not null and postage in ('first', 'second') - # ELSE - # postage is null - # END - # ) - # """) - # op.execute(""" - # ALTER TABLE templates_history ADD CONSTRAINT "chk_templates_history_postage" - # CHECK ( - # CASE WHEN template_type = 'letter' THEN - # postage is not null and postage in ('first', 'second') - # ELSE - # postage is null - # END - # ) - # """) diff --git a/migrations/versions/0322_update_postage_constraint_2.py b/migrations/versions/0322_update_postage_constraint_2.py deleted file mode 100644 index c5bd1b630..000000000 --- a/migrations/versions/0322_update_postage_constraint_2.py +++ /dev/null @@ -1,45 +0,0 @@ -""" - -Revision ID: 0322_update_postage_constraint_2 -Revises: 0321_update_postage_constraint_1 -Create Date: 2020-05-12 16:20:16.548347 - -""" -from alembic import op - - -revision = '0322_update_postage_constraint_2' -down_revision = '0321_update_postage_constraint_1' - - -def upgrade(): - op.drop_constraint('chk_templates_postage', 'templates') - op.drop_constraint('chk_templates_history_postage', 'templates_history') - - op.execute(""" - ALTER TABLE templates ADD CONSTRAINT "chk_templates_postage" - CHECK ( - CASE WHEN template_type = 'letter' THEN - postage is not null and postage in ('first', 'second', 'europe', 'rest-of-world') - ELSE - postage is null - END - ) - NOT VALID - """) - op.execute(""" - ALTER TABLE templates_history ADD CONSTRAINT "chk_templates_history_postage" - CHECK ( - CASE WHEN template_type = 'letter' THEN - postage is not null and postage in ('first', 'second', 'europe', 'rest-of-world') - ELSE - postage is null - END - ) - NOT VALID - """) - op.execute('COMMIT') - - -def downgrade(): - pass diff --git a/migrations/versions/0323_update_postage_constraint_3.py b/migrations/versions/0323_update_postage_constraint_3.py deleted file mode 100644 index 33d76c127..000000000 --- a/migrations/versions/0323_update_postage_constraint_3.py +++ /dev/null @@ -1,22 +0,0 @@ -""" - -Revision ID: 0323_update_postage_constraint_3 -Revises: 0322_update_postage_constraint_2 -Create Date: 2020-05-12 16:21:56.210025 - -""" -from alembic import op - - -revision = '0323_update_postage_constraint_3' -down_revision = '0322_update_postage_constraint_2' - - -def upgrade(): - op.execute('ALTER TABLE notifications VALIDATE CONSTRAINT "chk_notifications_postage_null"') - op.execute('ALTER TABLE templates VALIDATE CONSTRAINT "chk_templates_postage"') - op.execute('ALTER TABLE templates_history VALIDATE CONSTRAINT "chk_templates_history_postage"') - - -def downgrade(): - pass diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 4e55127e0..31e27460c 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2343,8 +2343,7 @@ def test_create_pdf_letter(mocker, sample_service_full_permissions, client, fake {"postage": "third", "filename": "string", "created_by": "string", "file_id": "string", "recipient_address": "Some Address"}, [ - {'error': 'ValidationError', - 'message': 'postage invalid. It must be first, second, europe or rest-of-world.'} + {'error': 'ValidationError', 'message': 'postage invalid. It must be either first or second.'} ] ) ]) diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 6d17c3d38..39b4232e4 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -636,7 +636,7 @@ def test_post_letter_notification_throws_error_for_invalid_postage(client, notif assert response.status_code == 400, response.get_data(as_text=True) resp_json = json.loads(response.get_data(as_text=True)) - assert resp_json['errors'][0]['message'] == "postage invalid. It must be first, second, europe or rest-of-world." + assert resp_json['errors'][0]['message'] == "postage invalid. It must be either first or second." assert not Notification.query.first()