From d6633aea5c3bcc82b5cae72d8b5af5d98bd41c3e Mon Sep 17 00:00:00 2001 From: stvnrlly Date: Tue, 28 Feb 2023 14:33:01 -0500 Subject: [PATCH] remove letters from models --- app/models.py | 79 +------------------ app/schemas.py | 4 +- tests/app/conftest.py | 25 ------ tests/app/dao/test_service_permissions_dao.py | 4 +- tests/app/db.py | 34 -------- tests/app/service/test_rest.py | 2 - tests/app/service/test_statistics.py | 14 ++-- tests/app/test_model.py | 6 +- .../notifications/test_get_notifications.py | 2 +- .../test_notification_schemas.py | 4 +- .../notifications/test_post_notifications.py | 1 - 11 files changed, 17 insertions(+), 158 deletions(-) diff --git a/app/models.py b/app/models.py index 3cb8d4363..04718ba06 100644 --- a/app/models.py +++ b/app/models.py @@ -14,7 +14,6 @@ from notifications_utils.recipients import ( validate_phone_number, ) from notifications_utils.template import ( - LetterPrintTemplate, PlainTextEmailTemplate, SMSMessageTemplate, ) @@ -463,7 +462,6 @@ class Service(db.Model, Versioned): contact_link = db.Column(db.String(255), nullable=True, unique=False) volume_sms = db.Column(db.Integer(), nullable=True, unique=False) volume_email = db.Column(db.Integer(), nullable=True, unique=False) - volume_letter = db.Column(db.Integer(), nullable=True, unique=False) consent_to_research = db.Column(db.Boolean, nullable=True) count_as_live = db.Column(db.Boolean, nullable=False, default=True) go_live_user_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), nullable=True) @@ -937,11 +935,6 @@ class TemplateBase(db.Model): return PlainTextEmailTemplate(self.__dict__) if self.template_type == SMS_TYPE: return SMSMessageTemplate(self.__dict__) - if self.template_type == LETTER_TYPE: - return LetterPrintTemplate( - self.__dict__, - contact_block=self.get_reply_to_text(), - ) def _as_utils_template_with_personalisation(self, values): template = self._as_utils_template() @@ -957,7 +950,7 @@ class TemplateBase(db.Model): "created_by": self.created_by.email_address, "version": self.version, "body": self.content, - "subject": self.subject if self.template_type in {EMAIL_TYPE, LETTER_TYPE} else None, + "subject": self.subject if self.template_type == EMAIL_TYPE else None, "name": self.name, "personalisation": { key: { @@ -1048,7 +1041,7 @@ SMS_PROVIDERS = [SNS_PROVIDER] EMAIL_PROVIDERS = [SES_PROVIDER] PROVIDERS = SMS_PROVIDERS + EMAIL_PROVIDERS -NOTIFICATION_TYPE = [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE] +NOTIFICATION_TYPE = [EMAIL_TYPE, SMS_TYPE] notification_types = db.Enum(*NOTIFICATION_TYPE, name='notification_type') @@ -1209,7 +1202,6 @@ NOTIFICATION_PERMANENT_FAILURE = 'permanent-failure' NOTIFICATION_PENDING_VIRUS_CHECK = 'pending-virus-check' NOTIFICATION_VALIDATION_FAILED = 'validation-failed' NOTIFICATION_VIRUS_SCAN_FAILED = 'virus-scan-failed' -NOTIFICATION_RETURNED_LETTER = 'returned-letter' NOTIFICATION_STATUS_TYPES_FAILED = [ NOTIFICATION_TECHNICAL_FAILURE, @@ -1217,7 +1209,6 @@ NOTIFICATION_STATUS_TYPES_FAILED = [ NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_VALIDATION_FAILED, NOTIFICATION_VIRUS_SCAN_FAILED, - NOTIFICATION_RETURNED_LETTER, ] NOTIFICATION_STATUS_TYPES_COMPLETED = [ @@ -1227,7 +1218,6 @@ NOTIFICATION_STATUS_TYPES_COMPLETED = [ NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE, - NOTIFICATION_RETURNED_LETTER, NOTIFICATION_CANCELLED, ] @@ -1244,7 +1234,6 @@ NOTIFICATION_STATUS_TYPES_BILLABLE = [ NOTIFICATION_FAILED, NOTIFICATION_TEMPORARY_FAILURE, NOTIFICATION_PERMANENT_FAILURE, - NOTIFICATION_RETURNED_LETTER, ] NOTIFICATION_STATUS_TYPES_BILLABLE_SMS = [ @@ -1279,16 +1268,12 @@ NOTIFICATION_STATUS_TYPES = [ NOTIFICATION_PENDING_VIRUS_CHECK, NOTIFICATION_VALIDATION_FAILED, NOTIFICATION_VIRUS_SCAN_FAILED, - NOTIFICATION_RETURNED_LETTER, ] NOTIFICATION_STATUS_TYPES_NON_BILLABLE = list(set(NOTIFICATION_STATUS_TYPES) - set(NOTIFICATION_STATUS_TYPES_BILLABLE)) NOTIFICATION_STATUS_TYPES_ENUM = db.Enum(*NOTIFICATION_STATUS_TYPES, name='notify_status_type') -NOTIFICATION_STATUS_LETTER_ACCEPTED = 'accepted' -NOTIFICATION_STATUS_LETTER_RECEIVED = 'received' - class NotificationStatusTypes(db.Model): __tablename__ = 'notification_status_types' @@ -1470,10 +1455,7 @@ class Notification(db.Model): def _substitute_status_str(_status): return ( - NOTIFICATION_STATUS_TYPES_FAILED if _status == NOTIFICATION_FAILED else - [NOTIFICATION_CREATED, NOTIFICATION_SENDING] if _status == NOTIFICATION_STATUS_LETTER_ACCEPTED else - NOTIFICATION_DELIVERED if _status == NOTIFICATION_STATUS_LETTER_RECEIVED else - [_status] + NOTIFICATION_STATUS_TYPES_FAILED if _status == NOTIFICATION_FAILED else [_status] ) def _substitute_status_seq(_statuses): @@ -1518,36 +1500,9 @@ class Notification(db.Model): 'sending': 'Sending', 'created': 'Sending', 'sent': 'Sent internationally' - }, - 'letter': { - 'technical-failure': 'Technical failure', - 'permanent-failure': 'Permanent failure', - 'sending': 'Accepted', - 'created': 'Accepted', - 'delivered': 'Received', - 'returned-letter': 'Returned', } }[self.template.template_type].get(self.status, self.status) - def get_letter_status(self): - """ - Return the notification_status, as we should present for letters. The distinction between created and sending is - a bit more confusing for letters, not to mention that there's no concept of temporary or permanent failure yet. - - - """ - # this should only ever be called for letter notifications - it makes no sense otherwise and I'd rather not - # get the two code flows mixed up at all - assert self.notification_type == LETTER_TYPE # nosec B101 - current calling code validates correct type - - if self.status in [NOTIFICATION_CREATED, NOTIFICATION_SENDING]: - return NOTIFICATION_STATUS_LETTER_ACCEPTED - elif self.status in [NOTIFICATION_DELIVERED, NOTIFICATION_RETURNED_LETTER]: - return NOTIFICATION_STATUS_LETTER_RECEIVED - else: - # Currently can only be technical-failure OR pending-virus-check OR validation-failed - return self.status - def get_created_by_name(self): if self.created_by: return self.created_by.name @@ -1597,7 +1552,7 @@ class Notification(db.Model): "line_6": None, "postcode": None, "type": self.notification_type, - "status": self.get_letter_status() if self.notification_type == LETTER_TYPE else self.status, + "status": self.status, "provider_response": self.provider_response, "template": template_dict, "body": self.content, @@ -1906,20 +1861,6 @@ class AuthType(db.Model): name = db.Column(db.String, primary_key=True) -class DailySortedLetter(db.Model): - __tablename__ = "daily_sorted_letter" - - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - billing_day = db.Column(db.Date, nullable=False, index=True) - file_name = db.Column(db.String, nullable=True, index=True) - unsorted_count = db.Column(db.Integer, nullable=False, default=0) - sorted_count = db.Column(db.Integer, nullable=False, default=0) - updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) - - __table_args__ = (UniqueConstraint('file_name', 'billing_day', name='uix_file_name_billing_day'), - ) - - class FactBilling(db.Model): __tablename__ = "ft_billing" @@ -2020,18 +1961,6 @@ class ServiceDataRetention(db.Model): } -class ReturnedLetter(db.Model): - __tablename__ = 'returned_letters' - - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - reported_at = db.Column(db.Date, nullable=False) - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), unique=False, index=True, nullable=False) - service = db.relationship(Service, backref=db.backref('returned_letters')) - notification_id = db.Column(UUID(as_uuid=True), unique=True, nullable=False) - created_at = db.Column(db.DateTime, nullable=False) - updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) - - class ServiceContactList(db.Model): __tablename__ = 'service_contact_list' diff --git a/app/schemas.py b/app/schemas.py index ed80daf69..26335ef9f 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -265,7 +265,6 @@ class ServiceSchema(BaseSchema, UUIDsAsStringsMixin): 'inbound_sms', 'jobs', 'reply_to_email_addresses', - 'returned_letters', 'service_sms_senders', 'templates', 'updated_at', @@ -325,7 +324,6 @@ class DetailedServiceSchema(BaseSchema): 'permissions', 'rate_limit', 'reply_to_email_addresses', - 'returned_letters', 'service_sms_senders', 'templates', 'users', @@ -372,7 +370,7 @@ class TemplateSchema(BaseTemplateSchema, UUIDsAsStringsMixin): @validates_schema def validate_type(self, data, **kwargs): - if data.get('template_type') in {models.EMAIL_TYPE, models.LETTER_TYPE}: + if data.get('template_type') == models.EMAIL_TYPE: subject = data.get('subject') if not subject or subject.strip() == '': raise ValidationError('Invalid template subject', 'subject') diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 4524b6b48..b3a2ff1c1 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -763,31 +763,6 @@ def create_custom_template(service, user, template_config_name, template_type, c return template -@pytest.fixture(scope='function') -def letter_volumes_email_template(notify_service): - email_template_content = '\n'.join([ - "((total_volume)) letters (((total_sheets)) sheets) sent via Notify are coming in today''s batch. These include: ", # noqa - "", - "((first_class_volume)) first class letters (((first_class_sheets)) sheets).", - "((second_class_volume)) second class letters (((second_class_sheets)) sheets).", - "((international_volume)) international letters (((international_sheets)) sheets).", - "", - "Thanks", - "", - "GOV.​UK Notify team", - "https://www.gov.uk/notify" - ]) - - return create_custom_template( - service=notify_service, - user=notify_service.users[0], - template_config_name='LETTERS_VOLUME_EMAIL_TEMPLATE_ID', - content=email_template_content, - subject="Notify letter volume for ((date)): ((total_volume)) letters, ((total_sheets)) sheets", - template_type='email' - ) - - @pytest.fixture def notify_service(notify_db_session, sample_user): service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID']) diff --git a/tests/app/dao/test_service_permissions_dao.py b/tests/app/dao/test_service_permissions_dao.py index f1cc125a0..6d83dd126 100644 --- a/tests/app/dao/test_service_permissions_dao.py +++ b/tests/app/dao/test_service_permissions_dao.py @@ -8,7 +8,6 @@ from app.models import ( EMAIL_TYPE, INBOUND_SMS_TYPE, INTERNATIONAL_SMS_TYPE, - LETTER_TYPE, SMS_TYPE, ) from tests.app.db import create_service, create_service_permission @@ -29,7 +28,6 @@ def test_create_service_permission(service_without_permissions): def test_fetch_service_permissions_gets_service_permissions(service_without_permissions): - create_service_permission(service_id=service_without_permissions.id, permission=LETTER_TYPE) create_service_permission(service_id=service_without_permissions.id, permission=INTERNATIONAL_SMS_TYPE) create_service_permission(service_id=service_without_permissions.id, permission=SMS_TYPE) @@ -37,7 +35,7 @@ def test_fetch_service_permissions_gets_service_permissions(service_without_perm assert len(service_permissions) == 3 assert all(sp.service_id == service_without_permissions.id for sp in service_permissions) - assert all(sp.permission in [LETTER_TYPE, INTERNATIONAL_SMS_TYPE, SMS_TYPE] for sp in service_permissions) + assert all(sp.permission in [INTERNATIONAL_SMS_TYPE, SMS_TYPE] for sp in service_permissions) def test_remove_service_permission(service_without_permissions): diff --git a/tests/app/db.py b/tests/app/db.py index d2ec3ae51..39a829e9f 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -34,7 +34,6 @@ from app.models import ( AnnualBilling, ApiKey, Complaint, - DailySortedLetter, Domain, EmailBranding, FactBilling, @@ -50,7 +49,6 @@ from app.models import ( Organisation, Permission, Rate, - ReturnedLetter, Service, ServiceCallbackApi, ServiceContactList, @@ -643,23 +641,6 @@ def create_invited_org_user(organisation, invited_by, email_address='invite@exam return invited_org_user -def create_daily_sorted_letter(billing_day=None, - file_name="Notify-20180118123.rs.txt", - unsorted_count=0, - sorted_count=0): - daily_sorted_letter = DailySortedLetter( - billing_day=billing_day or date(2018, 1, 18), - file_name=file_name, - unsorted_count=unsorted_count, - sorted_count=sorted_count - ) - - db.session.add(daily_sorted_letter) - db.session.commit() - - return daily_sorted_letter - - def create_ft_billing(local_date, template, *, @@ -970,21 +951,6 @@ def set_up_usage_data(start_date): } -def create_returned_letter(service=None, reported_at=None, notification_id=None): - if not service: - service = create_service(service_name='a - with sms and letter') - returned_letter = ReturnedLetter( - service_id=service.id, - reported_at=reported_at or datetime.utcnow(), - notification_id=notification_id or uuid.uuid4(), - created_at=datetime.utcnow(), - ) - - db.session.add(returned_letter) - db.session.commit() - return returned_letter - - def create_service_contact_list( service=None, original_file_name='EmergencyContactList.xls', diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 04203ac53..25c05e3df 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -261,7 +261,6 @@ def test_get_service_by_id(admin_request, sample_service): 'restricted', 'service_callback_api', 'volume_email', - 'volume_letter', 'volume_sms', } @@ -762,7 +761,6 @@ def test_update_service_flags(client, sample_service): @pytest.mark.parametrize('field', ( 'volume_email', 'volume_sms', - 'volume_letter', )) @pytest.mark.parametrize('value, expected_status, expected_persisted', ( (1234, 200, 1234), diff --git a/tests/app/service/test_statistics.py b/tests/app/service/test_statistics.py index 8ef41ed43..09edfee0d 100644 --- a/tests/app/service/test_statistics.py +++ b/tests/app/service/test_statistics.py @@ -19,33 +19,33 @@ NewStatsRow = collections.namedtuple('row', ('notification_type', 'status', 'key # email_counts and sms_counts are 3-tuple of requested, delivered, failed -@pytest.mark.idparametrize('stats, email_counts, sms_counts, letter_counts', { +@pytest.mark.idparametrize('stats, email_counts, sms_counts', { 'empty': ([], [0, 0, 0], [0, 0, 0], [0, 0, 0]), 'always_increment_requested': ([ StatsRow('email', 'delivered', 1), StatsRow('email', 'failed', 1) - ], [2, 1, 1], [0, 0, 0], [0, 0, 0]), + ], [2, 1, 1], [0, 0, 0]), 'dont_mix_template_types': ([ StatsRow('email', 'delivered', 1), StatsRow('sms', 'delivered', 1), - ], [1, 1, 0], [1, 1, 0], [1, 1, 0]), + ], [1, 1, 0], [1, 1, 0]), 'convert_fail_statuses_to_failed': ([ StatsRow('email', 'failed', 1), StatsRow('email', 'technical-failure', 1), StatsRow('email', 'temporary-failure', 1), StatsRow('email', 'permanent-failure', 1), - ], [4, 0, 4], [0, 0, 0], [3, 0, 3]), + ], [4, 0, 4], [0, 0, 0]), 'convert_sent_to_delivered': ([ StatsRow('sms', 'sending', 1), StatsRow('sms', 'delivered', 1), StatsRow('sms', 'sent', 1), - ], [0, 0, 0], [3, 2, 0], [0, 0, 0]), + ], [0, 0, 0], [3, 2, 0]), 'handles_none_rows': ([ StatsRow('sms', 'sending', 1), StatsRow(None, None, None) - ], [0, 0, 0], [1, 0, 0], [0, 0, 0]) + ], [0, 0, 0], [1, 0, 0]) }) -def test_format_statistics(stats, email_counts, sms_counts, letter_counts): +def test_format_statistics(stats, email_counts, sms_counts): ret = format_statistics(stats) diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 5c9824992..a8cb0e5b5 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -11,8 +11,6 @@ from app.models import ( NOTIFICATION_FAILED, NOTIFICATION_PENDING, NOTIFICATION_SENDING, - NOTIFICATION_STATUS_LETTER_ACCEPTED, - NOTIFICATION_STATUS_LETTER_RECEIVED, NOTIFICATION_STATUS_TYPES_FAILED, NOTIFICATION_TECHNICAL_FAILURE, SMS_TYPE, @@ -61,20 +59,18 @@ def test_should_not_build_service_guest_list_from_invalid_contact(recipient_type @pytest.mark.parametrize('initial_statuses, expected_statuses', [ # passing in single statuses as strings (NOTIFICATION_FAILED, NOTIFICATION_STATUS_TYPES_FAILED), - (NOTIFICATION_STATUS_LETTER_ACCEPTED, [NOTIFICATION_SENDING, NOTIFICATION_CREATED]), (NOTIFICATION_CREATED, [NOTIFICATION_CREATED]), (NOTIFICATION_TECHNICAL_FAILURE, [NOTIFICATION_TECHNICAL_FAILURE]), # passing in lists containing single statuses ([NOTIFICATION_FAILED], NOTIFICATION_STATUS_TYPES_FAILED), ([NOTIFICATION_CREATED], [NOTIFICATION_CREATED]), ([NOTIFICATION_TECHNICAL_FAILURE], [NOTIFICATION_TECHNICAL_FAILURE]), - (NOTIFICATION_STATUS_LETTER_RECEIVED, NOTIFICATION_DELIVERED), # passing in lists containing multiple statuses ([NOTIFICATION_FAILED, NOTIFICATION_CREATED], NOTIFICATION_STATUS_TYPES_FAILED + [NOTIFICATION_CREATED]), ([NOTIFICATION_CREATED, NOTIFICATION_PENDING], [NOTIFICATION_CREATED, NOTIFICATION_PENDING]), ([NOTIFICATION_CREATED, NOTIFICATION_TECHNICAL_FAILURE], [NOTIFICATION_CREATED, NOTIFICATION_TECHNICAL_FAILURE]), ( - [NOTIFICATION_FAILED, NOTIFICATION_STATUS_LETTER_ACCEPTED], + [NOTIFICATION_FAILED, NOTIFICATION_SENDING], NOTIFICATION_STATUS_TYPES_FAILED + [NOTIFICATION_SENDING, NOTIFICATION_CREATED] ), # checking we don't end up with duplicates diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index d48524e8a..1a17b5eb0 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -382,7 +382,7 @@ def test_get_all_notifications_filter_by_status_invalid_status(client, sample_no assert len(json_response['errors']) == 1 assert json_response['errors'][0]['message'] == "status elephant is not one of [cancelled, created, sending, " \ "sent, delivered, pending, failed, technical-failure, temporary-failure, permanent-failure, " \ - "pending-virus-check, validation-failed, virus-scan-failed, returned-letter]" + "pending-virus-check, validation-failed, virus-scan-failed]" def test_get_all_notifications_filter_by_multiple_statuses(client, sample_template): diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index e78000e73..dd95aaf61 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -45,7 +45,7 @@ def test_get_notifications_request_invalid_statuses( partial_error_status = "is not one of " \ "[cancelled, created, sending, sent, delivered, pending, failed, " \ "technical-failure, temporary-failure, permanent-failure, pending-virus-check, " \ - "validation-failed, virus-scan-failed, returned-letter]" + "validation-failed, virus-scan-failed]" with pytest.raises(ValidationError) as e: validate({'status': invalid_statuses + valid_statuses}, get_notifications_request) @@ -93,7 +93,7 @@ def test_get_notifications_request_invalid_statuses_and_template_types(): for invalid_status in ["elephant", "giraffe"]: assert "status {} is not one of [cancelled, created, sending, sent, delivered, " \ "pending, failed, technical-failure, temporary-failure, permanent-failure, " \ - "pending-virus-check, validation-failed, virus-scan-failed, returned-letter]".format( + "pending-virus-check, validation-failed, virus-scan-failed]".format( invalid_status ) in error_messages diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 7d7be8450..91189a3a1 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -357,7 +357,6 @@ def test_post_notification_returns_400_and_missing_template(client, sample_servi @pytest.mark.parametrize("notification_type, key_send_to, send_to", [ ("sms", "phone_number", "+447700900855"), ("email", "email_address", "sample@email.com"), - ("letter", "personalisation", {"address_line_1": "The queen", "postcode": "SW1 1AA"}) ]) def test_post_notification_returns_401_and_well_formed_auth_error(client, sample_template, notification_type, key_send_to, send_to):