From 7d6fb2c645c18fcbf62c055f99df07a2e04ce04f Mon Sep 17 00:00:00 2001 From: stvnrlly Date: Fri, 17 Feb 2023 14:18:52 -0500 Subject: [PATCH] remove letters from /service --- app/commands.py | 4 +- app/dao/returned_letters_dao.py | 79 ------ app/dao/service_letter_contact_dao.py | 105 -------- app/notifications/rest.py | 9 +- app/notifications/validators.py | 14 - app/service/rest.py | 129 +-------- app/service/send_notification.py | 30 +-- app/service/service_senders_schema.py | 13 - .../versions/0387_remove_letter_perms_.py | 41 +++ .../dao/test_service_letter_contact_dao.py | 248 ------------------ tests/app/notifications/test_validators.py | 43 +-- .../test_send_notification.py | 3 +- tests/app/service/test_rest.py | 187 ------------- tests/app/template/test_rest.py | 22 -- 14 files changed, 58 insertions(+), 869 deletions(-) delete mode 100644 app/dao/returned_letters_dao.py delete mode 100644 app/dao/service_letter_contact_dao.py create mode 100644 migrations/versions/0387_remove_letter_perms_.py delete mode 100644 tests/app/dao/test_service_letter_contact_dao.py diff --git a/app/commands.py b/app/commands.py index 9cda66567..5a831c741 100644 --- a/app/commands.py +++ b/app/commands.py @@ -383,12 +383,11 @@ def associate_services_to_organisations(): @notify_command(name='populate-service-volume-intentions') @click.option('-f', '--file_name', required=True, - help="Pipe delimited file containing service_id, SMS, email, letters") + help="Pipe delimited file containing service_id, SMS, email") def populate_service_volume_intentions(file_name): # [0] service_id # [1] SMS:: volume intentions for service # [2] Email:: volume intentions for service - # [3] Letters:: volume intentions for service with open(file_name, 'r') as f: for line in itertools.islice(f, 1, None): @@ -397,7 +396,6 @@ def populate_service_volume_intentions(file_name): service = dao_fetch_service_by_id(columns[0]) service.volume_sms = columns[1] service.volume_email = columns[2] - service.volume_letter = columns[3] dao_update_service(service) print("populate-service-volume-intentions complete") diff --git a/app/dao/returned_letters_dao.py b/app/dao/returned_letters_dao.py deleted file mode 100644 index e802fd297..000000000 --- a/app/dao/returned_letters_dao.py +++ /dev/null @@ -1,79 +0,0 @@ -from sqlalchemy import desc, func - -from app import db -from app.models import ( - Job, - Notification, - NotificationHistory, - ReturnedLetter, - Template, - User, -) -from app.utils import midnight_n_days_ago - - -def fetch_recent_returned_letter_count(service_id): - return db.session.query( - func.count(ReturnedLetter.notification_id).label('returned_letter_count'), - ).filter( - ReturnedLetter.service_id == service_id, - ReturnedLetter.reported_at > midnight_n_days_ago(7), - ).one() - - -def fetch_most_recent_returned_letter(service_id): - return db.session.query( - ReturnedLetter.reported_at, - ).filter( - ReturnedLetter.service_id == service_id, - ).order_by( - desc(ReturnedLetter.reported_at) - ).first() - - -def fetch_returned_letter_summary(service_id): - return db.session.query( - func.count(ReturnedLetter.notification_id).label('returned_letter_count'), - ReturnedLetter.reported_at - ).filter( - ReturnedLetter.service_id == service_id, - ).group_by( - ReturnedLetter.reported_at - ).order_by( - desc(ReturnedLetter.reported_at) - ).all() - - -def fetch_returned_letters(service_id, report_date): - results = [] - for table in [Notification, NotificationHistory]: - query = db.session.query( - ReturnedLetter.notification_id, - ReturnedLetter.reported_at, - table.client_reference, - table.created_at, - Template.name.label('template_name'), - table.template_id, - table.template_version, - Template.hidden, - table.api_key_id, - table.created_by_id, - User.name.label('user_name'), - User.email_address, - Job.original_file_name, - (table.job_row_number + 1).label('job_row_number') # row numbers start at 0 - ).outerjoin( - User, table.created_by_id == User.id - ).outerjoin( - Job, table.job_id == Job.id - ).filter( - ReturnedLetter.service_id == service_id, - ReturnedLetter.reported_at == report_date, - ReturnedLetter.notification_id == table.id, - table.template_id == Template.id - ).order_by( - desc(ReturnedLetter.reported_at), desc(table.created_at) - ) - results = results + query.all() - results = sorted(results, key=lambda i: i.created_at, reverse=True) - return results diff --git a/app/dao/service_letter_contact_dao.py b/app/dao/service_letter_contact_dao.py deleted file mode 100644 index 7ee409142..000000000 --- a/app/dao/service_letter_contact_dao.py +++ /dev/null @@ -1,105 +0,0 @@ -from sqlalchemy import desc - -from app import db -from app.dao.dao_utils import autocommit -from app.models import ServiceLetterContact, Template - - -def dao_get_letter_contacts_by_service_id(service_id): - letter_contacts = db.session.query( - ServiceLetterContact - ).filter( - ServiceLetterContact.service_id == service_id, - ServiceLetterContact.archived == False # noqa - ).order_by( - desc(ServiceLetterContact.is_default), - desc(ServiceLetterContact.created_at) - ).all() - - return letter_contacts - - -def dao_get_letter_contact_by_id(service_id, letter_contact_id): - letter_contact = db.session.query( - ServiceLetterContact - ).filter( - ServiceLetterContact.service_id == service_id, - ServiceLetterContact.id == letter_contact_id, - ServiceLetterContact.archived == False # noqa - ).one() - return letter_contact - - -@autocommit -def add_letter_contact_for_service(service_id, contact_block, is_default): - old_default = _get_existing_default(service_id) - if is_default: - _reset_old_default_to_false(old_default) - - new_letter_contact = ServiceLetterContact( - service_id=service_id, - contact_block=contact_block, - is_default=is_default - ) - db.session.add(new_letter_contact) - return new_letter_contact - - -@autocommit -def update_letter_contact(service_id, letter_contact_id, contact_block, is_default): - old_default = _get_existing_default(service_id) - # if we want to make this the default, ensure there are no other existing defaults - if is_default: - _reset_old_default_to_false(old_default) - - letter_contact_update = ServiceLetterContact.query.get(letter_contact_id) - letter_contact_update.contact_block = contact_block - letter_contact_update.is_default = is_default - db.session.add(letter_contact_update) - return letter_contact_update - - -@autocommit -def archive_letter_contact(service_id, letter_contact_id): - letter_contact_to_archive = ServiceLetterContact.query.filter_by( - id=letter_contact_id, - service_id=service_id - ).one() - - Template.query.filter_by( - service_letter_contact_id=letter_contact_id - ).update({ - 'service_letter_contact_id': None - }) - - letter_contact_to_archive.archived = True - - db.session.add(letter_contact_to_archive) - return letter_contact_to_archive - - -def _get_existing_default(service_id): - old_defaults = [ - x for x - in dao_get_letter_contacts_by_service_id(service_id=service_id) - if x.is_default - ] - - if len(old_defaults) == 0: - return None - - if len(old_defaults) == 1: - return old_defaults[0] - - raise Exception( - "There should only be one default letter contact for each service. Service {} has {}".format( - service_id, - len(old_defaults) - ) - ) - - -def _reset_old_default_to_false(old_default): - if old_default: - old_default.is_default = False - db.session.add(old_default) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 2dc42da50..59b6348da 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -5,13 +5,7 @@ from app import api_user, authenticated_service from app.config import QueueNames from app.dao import notifications_dao from app.errors import InvalidRequest, register_errors -from app.models import ( - EMAIL_TYPE, - KEY_TYPE_TEAM, - LETTER_TYPE, - PRIORITY, - SMS_TYPE, -) +from app.models import EMAIL_TYPE, KEY_TYPE_TEAM, PRIORITY, SMS_TYPE from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, @@ -81,7 +75,6 @@ def send_notification(notification_type): if notification_type not in [SMS_TYPE, EMAIL_TYPE]: msg = "{} notification type is not supported".format(notification_type) - msg = msg + ", please use the latest version of the client" if notification_type == LETTER_TYPE else msg raise InvalidRequest(msg, 400) notification_form = ( diff --git a/app/notifications/validators.py b/app/notifications/validators.py index e71584ae6..9960402a8 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -14,14 +14,12 @@ from sqlalchemy.orm.exc import NoResultFound from app import redis_store from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id -from app.dao.service_letter_contact_dao import dao_get_letter_contact_by_id from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id from app.models import ( EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, KEY_TYPE_TEAM, KEY_TYPE_TEST, - LETTER_TYPE, SMS_TYPE, ServicePermission, ) @@ -201,8 +199,6 @@ def check_reply_to(service_id, reply_to_id, type_): return check_service_email_reply_to_id(service_id, reply_to_id, type_) elif type_ == SMS_TYPE: return check_service_sms_sender_id(service_id, reply_to_id, type_) - elif type_ == LETTER_TYPE: - return check_service_letter_contact_id(service_id, reply_to_id, type_) def check_service_email_reply_to_id(service_id, reply_to_id, notification_type): @@ -223,13 +219,3 @@ def check_service_sms_sender_id(service_id, sms_sender_id, notification_type): message = 'sms_sender_id {} does not exist in database for service id {}' \ .format(sms_sender_id, service_id) raise BadRequestError(message=message) - - -def check_service_letter_contact_id(service_id, letter_contact_id, notification_type): - if letter_contact_id: - try: - return dao_get_letter_contact_by_id(service_id, letter_contact_id).contact_block - except NoResultFound: - message = 'letter_contact_id {} does not exist in database for service id {}' \ - .format(letter_contact_id, service_id) - raise BadRequestError(message=message) diff --git a/app/service/rest.py b/app/service/rest.py index 29d215678..04fcfc661 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -28,12 +28,6 @@ from app.dao.fact_notification_status_dao import ( ) from app.dao.inbound_numbers_dao import dao_allocate_number_for_service from app.dao.organisation_dao import dao_get_organisation_by_service_id -from app.dao.returned_letters_dao import ( - fetch_most_recent_returned_letter, - fetch_recent_returned_letter_count, - fetch_returned_letter_summary, - fetch_returned_letters, -) from app.dao.service_contact_list_dao import ( dao_archive_contact_list, dao_get_contact_list_by_id, @@ -59,13 +53,6 @@ from app.dao.service_guest_list_dao import ( dao_fetch_service_guest_list, dao_remove_service_guest_list, ) -from app.dao.service_letter_contact_dao import ( - add_letter_contact_for_service, - archive_letter_contact, - dao_get_letter_contact_by_id, - dao_get_letter_contacts_by_service_id, - update_letter_contact, -) from app.dao.service_sms_sender_dao import ( archive_sms_sender, dao_add_sms_sender_for_service, @@ -126,17 +113,11 @@ from app.service.service_data_retention_schema import ( ) from app.service.service_senders_schema import ( add_service_email_reply_to_request, - add_service_letter_contact_block_request, add_service_sms_sender_request, ) from app.service.utils import get_guest_list_objects from app.user.users_schema import post_set_permissions_schema -from app.utils import ( - DATE_FORMAT, - DATETIME_FORMAT_NO_TIMEZONE, - get_prev_next_pagination_links, - midnight_n_days_ago, -) +from app.utils import get_prev_next_pagination_links service_blueprint = Blueprint('service', __name__) @@ -477,6 +458,7 @@ def get_notification_for_service(service_id, notification_id): ), 200 +# TODO: possibly unnecessary after removing letters @service_blueprint.route('//notifications//cancel', methods=['POST']) def cancel_notification_for_service(service_id, notification_id): notification = notifications_dao.get_notification_by_id(notification_id, service_id) @@ -787,48 +769,6 @@ def delete_service_reply_to_email_address(service_id, reply_to_email_id): return jsonify(data=archived_reply_to.serialize()), 200 -@service_blueprint.route('//letter-contact', methods=["GET"]) -def get_letter_contacts(service_id): - result = dao_get_letter_contacts_by_service_id(service_id) - return jsonify([i.serialize() for i in result]), 200 - - -@service_blueprint.route('//letter-contact/', methods=["GET"]) -def get_letter_contact_by_id(service_id, letter_contact_id): - result = dao_get_letter_contact_by_id(service_id=service_id, letter_contact_id=letter_contact_id) - return jsonify(result.serialize()), 200 - - -@service_blueprint.route('//letter-contact', methods=['POST']) -def add_service_letter_contact(service_id): - # validate the service exists, throws ResultNotFound exception. - dao_fetch_service_by_id(service_id) - form = validate(request.get_json(), add_service_letter_contact_block_request) - new_letter_contact = add_letter_contact_for_service(service_id=service_id, - contact_block=form['contact_block'], - is_default=form.get('is_default', True)) - return jsonify(data=new_letter_contact.serialize()), 201 - - -@service_blueprint.route('//letter-contact/', methods=['POST']) -def update_service_letter_contact(service_id, letter_contact_id): - # validate the service exists, throws ResultNotFound exception. - dao_fetch_service_by_id(service_id) - form = validate(request.get_json(), add_service_letter_contact_block_request) - new_reply_to = update_letter_contact(service_id=service_id, - letter_contact_id=letter_contact_id, - contact_block=form['contact_block'], - is_default=form.get('is_default', True)) - return jsonify(data=new_reply_to.serialize()), 200 - - -@service_blueprint.route('//letter-contact//archive', methods=['POST']) -def delete_service_letter_contact(service_id, letter_contact_id): - archived_letter_contact = archive_letter_contact(service_id, letter_contact_id) - - return jsonify(data=archived_letter_contact.serialize()), 200 - - @service_blueprint.route('//sms-sender', methods=['POST']) def add_service_sms_sender(service_id): dao_fetch_service_by_id(service_id) @@ -1006,71 +946,6 @@ def check_if_reply_to_address_already_in_use(service_id, email_address): ) -@service_blueprint.route('//returned-letter-statistics', methods=['GET']) -def returned_letter_statistics(service_id): - - most_recent = fetch_most_recent_returned_letter(service_id) - - if not most_recent: - return jsonify({ - 'returned_letter_count': 0, - 'most_recent_report': None, - }) - - most_recent_reported_at = datetime.combine( - most_recent.reported_at, datetime.min.time() - ) - - if most_recent_reported_at < midnight_n_days_ago(7): - return jsonify({ - 'returned_letter_count': 0, - 'most_recent_report': most_recent.reported_at.strftime(DATETIME_FORMAT_NO_TIMEZONE), - }) - - count = fetch_recent_returned_letter_count(service_id) - - return jsonify({ - 'returned_letter_count': count.returned_letter_count, - 'most_recent_report': most_recent.reported_at.strftime(DATETIME_FORMAT_NO_TIMEZONE), - }) - - -@service_blueprint.route('//returned-letter-summary', methods=['GET']) -def returned_letter_summary(service_id): - results = fetch_returned_letter_summary(service_id) - - json_results = [{'returned_letter_count': x.returned_letter_count, - 'reported_at': x.reported_at.strftime(DATE_FORMAT) - } for x in results] - - return jsonify(json_results) - - -@service_blueprint.route('//returned-letters', methods=['GET']) -def get_returned_letters(service_id): - results = fetch_returned_letters(service_id=service_id, report_date=request.args.get('reported_at')) - - json_results = [ - {'notification_id': x.notification_id, - # client reference can only be added on API letters - 'client_reference': x.client_reference if x.api_key_id else None, - 'reported_at': x.reported_at.strftime(DATE_FORMAT), - 'created_at': x.created_at.strftime(DATETIME_FORMAT_NO_TIMEZONE), - # it doesn't make sense to show hidden templates - 'template_name': x.template_name if not x.hidden else None, - 'template_id': x.template_id if not x.hidden else None, - 'template_version': x.template_version if not x.hidden else None, - 'user_name': x.user_name or 'API', - 'email_address': x.email_address or 'API', - 'original_file_name': x.original_file_name, - 'job_row_number': x.job_row_number, - # the file name for a letter uploaded via the UI - 'uploaded_letter_file_name': x.client_reference if x.hidden and not x.api_key_id else None - } for x in results] - - return jsonify(sorted(json_results, key=lambda i: i['created_at'], reverse=True)) - - @service_blueprint.route('//contact-list', methods=['GET']) def get_contact_list(service_id): contact_lists = dao_get_contact_lists(service_id) diff --git a/app/service/send_notification.py b/app/service/send_notification.py index de9ca9645..9f7f7685b 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -1,21 +1,12 @@ from sqlalchemy.orm.exc import NoResultFound -from app import create_random_identifier from app.config import QueueNames -from app.dao.notifications_dao import _update_notification_status from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id from app.dao.services_dao import dao_fetch_service_by_id from app.dao.templates_dao import dao_get_template_by_id_and_service_id from app.dao.users_dao import get_user_by_id -from app.models import ( - EMAIL_TYPE, - KEY_TYPE_NORMAL, - LETTER_TYPE, - NOTIFICATION_DELIVERED, - PRIORITY, - SMS_TYPE, -) +from app.models import EMAIL_TYPE, KEY_TYPE_NORMAL, PRIORITY, SMS_TYPE from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, @@ -38,9 +29,8 @@ def validate_created_by(service, created_by_id): raise BadRequestError(message=message) +# TODO: possibly unnecessary after removing letters def create_one_off_reference(template_type): - if template_type == LETTER_TYPE: - return create_random_identifier() return None @@ -92,17 +82,11 @@ def send_one_off_notification(service_id, post_data): queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None - if template.template_type == LETTER_TYPE and service.research_mode: - _update_notification_status( - notification, - NOTIFICATION_DELIVERED, - ) - else: - send_notification_to_queue( - notification=notification, - research_mode=service.research_mode, - queue=queue_name, - ) + send_notification_to_queue( + notification=notification, + research_mode=service.research_mode, + queue=queue_name, + ) return {'id': str(notification.id)} diff --git a/app/service/service_senders_schema.py b/app/service/service_senders_schema.py index e39765d30..1b4ae2489 100644 --- a/app/service/service_senders_schema.py +++ b/app/service/service_senders_schema.py @@ -13,19 +13,6 @@ add_service_email_reply_to_request = { } -add_service_letter_contact_block_request = { - "$schema": "http://json-schema.org/draft-07/schema#", - "description": "POST service letter contact block", - "type": "object", - "title": "Add new letter contact block for service", - "properties": { - "contact_block": {"type": "string"}, - "is_default": {"type": "boolean"} - }, - "required": ["contact_block", "is_default"] -} - - add_service_sms_sender_request = { "$schema": "http://json-schema.org/draft-07/schema#", "description": "POST add service SMS sender", diff --git a/migrations/versions/0387_remove_letter_perms_.py b/migrations/versions/0387_remove_letter_perms_.py new file mode 100644 index 000000000..c54b30a45 --- /dev/null +++ b/migrations/versions/0387_remove_letter_perms_.py @@ -0,0 +1,41 @@ +""" + +Revision ID: 0387_remove_letter_perms_.py +Revises: 0386_remove_letter_rates_.py +Create Date: 2023-02-17 11:56:00.993409 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0387_remove_letter_perms_.py' +down_revision = '0386_remove_letter_rates_.py' + + +def upgrade(): + # this is the inverse of migration 0317 + op.execute("DELETE from service_permissions where permission = 'upload_letters'") + # ### end Alembic commands ### + + +def downgrade(): + # this is the inverse of migration 0317 + op.execute(""" + INSERT INTO + service_permissions (service_id, permission, created_at) + SELECT + id, 'upload_letters', now() + FROM + services + WHERE + NOT EXISTS ( + SELECT + FROM + service_permissions + WHERE + service_id = services.id and + permission = 'upload_letters' + ) + """) + # ### end Alembic commands ### diff --git a/tests/app/dao/test_service_letter_contact_dao.py b/tests/app/dao/test_service_letter_contact_dao.py deleted file mode 100644 index 4a996dcb4..000000000 --- a/tests/app/dao/test_service_letter_contact_dao.py +++ /dev/null @@ -1,248 +0,0 @@ -import uuid - -import pytest -from sqlalchemy.exc import SQLAlchemyError - -from app.dao.service_letter_contact_dao import ( - add_letter_contact_for_service, - archive_letter_contact, - dao_get_letter_contact_by_id, - dao_get_letter_contacts_by_service_id, - update_letter_contact, -) -from app.models import ServiceLetterContact -from tests.app.db import create_letter_contact, create_service, create_template - - -def test_dao_get_letter_contacts_by_service_id(notify_db_session): - service = create_service() - default_letter_contact = create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA') - second_letter_contact = create_letter_contact(service=service, contact_block='Cardiff, CA1 2DB', is_default=False) - third_letter_contact = create_letter_contact(service=service, contact_block='London, E1 8QS', is_default=False) - - results = dao_get_letter_contacts_by_service_id(service_id=service.id) - - assert len(results) == 3 - assert default_letter_contact == results[0] - assert third_letter_contact == results[1] - assert second_letter_contact == results[2] - - -def test_dao_get_letter_contacts_by_service_id_does_not_return_archived_contacts(notify_db_session): - service = create_service() - create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA') - create_letter_contact(service=service, contact_block='Cardiff, CA1 2DB', is_default=False) - archived_contact = create_letter_contact( - service=service, - contact_block='London, E1 8QS', - is_default=False, - archived=True - ) - - results = dao_get_letter_contacts_by_service_id(service_id=service.id) - - assert len(results) == 2 - assert archived_contact not in results - - -def test_add_letter_contact_for_service_creates_additional_letter_contact_for_service(notify_db_session): - service = create_service() - - create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA') - add_letter_contact_for_service(service_id=service.id, contact_block='Swansea, SN1 3CC', is_default=False) - - results = dao_get_letter_contacts_by_service_id(service_id=service.id) - - assert len(results) == 2 - - assert results[0].contact_block == 'Edinburgh, ED1 1AA' - assert results[0].is_default - assert not results[0].archived - - assert results[1].contact_block == 'Swansea, SN1 3CC' - assert not results[1].is_default - assert not results[1].archived - - -def test_add_another_letter_contact_as_default_overrides_existing(notify_db_session): - service = create_service() - - create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA') - add_letter_contact_for_service(service_id=service.id, contact_block='Swansea, SN1 3CC', is_default=True) - - results = dao_get_letter_contacts_by_service_id(service_id=service.id) - - assert len(results) == 2 - - assert results[0].contact_block == 'Swansea, SN1 3CC' - assert results[0].is_default - - assert results[1].contact_block == 'Edinburgh, ED1 1AA' - assert not results[1].is_default - - -def test_add_letter_contact_does_not_override_default(notify_db_session): - service = create_service() - - add_letter_contact_for_service(service_id=service.id, contact_block='Edinburgh, ED1 1AA', is_default=True) - add_letter_contact_for_service(service_id=service.id, contact_block='Swansea, SN1 3CC', is_default=False) - - results = dao_get_letter_contacts_by_service_id(service_id=service.id) - - assert len(results) == 2 - - assert results[0].contact_block == 'Edinburgh, ED1 1AA' - assert results[0].is_default - - assert results[1].contact_block == 'Swansea, SN1 3CC' - assert not results[1].is_default - - -def test_add_letter_contact_with_no_default_is_fine(notify_db_session): - service = create_service() - letter_contact = add_letter_contact_for_service( - service_id=service.id, - contact_block='Swansea, SN1 3CC', - is_default=False - ) - assert service.letter_contacts == [letter_contact] - - -def test_add_letter_contact_when_multiple_defaults_exist_raises_exception(notify_db_session): - service = create_service() - create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA') - create_letter_contact(service=service, contact_block='Aberdeen, AB12 23X') - - with pytest.raises(Exception): - add_letter_contact_for_service(service_id=service.id, contact_block='Swansea, SN1 3CC', is_default=False) - - -def test_can_update_letter_contact(notify_db_session): - service = create_service() - letter_contact = create_letter_contact(service=service, contact_block='Aberdeen, AB12 23X') - - update_letter_contact( - service_id=service.id, - letter_contact_id=letter_contact.id, - contact_block='Warwick, W14 TSR', - is_default=True - ) - - updated_letter_contact = ServiceLetterContact.query.get(letter_contact.id) - - assert updated_letter_contact.contact_block == 'Warwick, W14 TSR' - assert updated_letter_contact.updated_at - assert updated_letter_contact.is_default - - -def test_update_letter_contact_as_default_overides_existing_default(notify_db_session): - service = create_service() - - create_letter_contact(service=service, contact_block='Aberdeen, AB12 23X') - second_letter_contact = create_letter_contact(service=service, contact_block='Swansea, SN1 3CC', is_default=False) - - update_letter_contact( - service_id=service.id, - letter_contact_id=second_letter_contact.id, - contact_block='Warwick, W14 TSR', - is_default=True - ) - - results = dao_get_letter_contacts_by_service_id(service_id=service.id) - assert len(results) == 2 - - assert results[0].contact_block == 'Warwick, W14 TSR' - assert results[0].is_default - - assert results[1].contact_block == 'Aberdeen, AB12 23X' - assert not results[1].is_default - - -def test_update_letter_contact_unset_default_for_only_letter_contact_is_fine(notify_db_session): - service = create_service() - only_letter_contact = create_letter_contact(service=service, contact_block='Aberdeen, AB12 23X') - update_letter_contact( - service_id=service.id, - letter_contact_id=only_letter_contact.id, - contact_block='Warwick, W14 TSR', - is_default=False - ) - assert only_letter_contact.is_default is False - - -def test_archive_letter_contact(notify_db_session): - service = create_service() - create_letter_contact(service=service, contact_block='Aberdeen, AB12 23X') - letter_contact = create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA', is_default=False) - - archive_letter_contact(service.id, letter_contact.id) - - assert letter_contact.archived - assert letter_contact.updated_at is not None - - -def test_archive_letter_contact_does_not_archive_a_letter_contact_for_a_different_service( - notify_db_session, - sample_service, -): - service = create_service(service_name="First service") - letter_contact = create_letter_contact( - service=sample_service, - contact_block='Edinburgh, ED1 1AA', - is_default=False) - - with pytest.raises(SQLAlchemyError): - archive_letter_contact(service.id, letter_contact.id) - - assert not letter_contact.archived - - -def test_archive_letter_contact_can_archive_a_service_default_letter_contact(notify_db_session): - service = create_service() - letter_contact = create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA') - archive_letter_contact(service.id, letter_contact.id) - assert letter_contact.archived is True - - -def test_archive_letter_contact_does_dissociates_template_defaults_before_archiving(notify_db_session): - service = create_service() - create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA') - template_default = create_letter_contact(service=service, contact_block='Aberdeen, AB12 23X', is_default=False) - associated_template_1 = create_template(service=service, template_type='letter', reply_to=template_default.id) - associated_template_2 = create_template(service=service, template_type='letter', reply_to=template_default.id) - - assert associated_template_1.reply_to == template_default.id - assert associated_template_2.reply_to == template_default.id - assert template_default.archived is False - - archive_letter_contact(service.id, template_default.id) - - assert associated_template_1.reply_to is None - assert associated_template_2.reply_to is None - assert template_default.archived is True - - -def test_dao_get_letter_contact_by_id(sample_service): - letter_contact = create_letter_contact(service=sample_service, contact_block='Aberdeen, AB12 23X') - result = dao_get_letter_contact_by_id(service_id=sample_service.id, letter_contact_id=letter_contact.id) - assert result == letter_contact - - -def test_dao_get_letter_contact_by_id_raises_sqlalchemy_error_when_letter_contact_does_not_exist(sample_service): - with pytest.raises(SQLAlchemyError): - dao_get_letter_contact_by_id(service_id=sample_service.id, letter_contact_id=uuid.uuid4()) - - -def test_dao_get_letter_contact_by_id_raises_sqlalchemy_error_when_letter_contact_is_archived(sample_service): - archived_contact = create_letter_contact( - service=sample_service, - contact_block='Aberdeen, AB12 23X', - archived=True) - with pytest.raises(SQLAlchemyError): - dao_get_letter_contact_by_id(service_id=sample_service.id, letter_contact_id=archived_contact.id) - - -def test_dao_get_letter_contact_by_id_raises_sqlalchemy_error_when_service_does_not_exist(sample_service): - letter_contact = create_letter_contact(service=sample_service, contact_block='Some address') - with pytest.raises(SQLAlchemyError): - dao_get_letter_contact_by_id(service_id=uuid.uuid4(), letter_contact_id=letter_contact.id) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 9e826435c..f18c327fb 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -7,7 +7,7 @@ from notifications_utils import SMS_CHAR_COUNT_LIMIT import app from app.dao import templates_dao -from app.models import EMAIL_TYPE, LETTER_TYPE, SMS_TYPE +from app.models import EMAIL_TYPE, SMS_TYPE from app.notifications.process_notifications import ( create_content_for_notification, ) @@ -18,7 +18,6 @@ from app.notifications.validators import ( check_rate_limiting, check_reply_to, check_service_email_reply_to_id, - check_service_letter_contact_id, check_service_over_api_rate_limit, check_service_over_daily_message_limit, check_service_sms_sender_id, @@ -37,7 +36,6 @@ from app.utils import get_template_instance from app.v2.errors import BadRequestError, RateLimitError, TooManyRequestsError from tests.app.db import ( create_api_key, - create_letter_contact, create_reply_to_email, create_service, create_service_guest_list, @@ -266,7 +264,7 @@ def test_service_can_send_to_recipient_fails_when_mobile_number_is_not_on_team(s @pytest.mark.parametrize('char_count', [612, 0, 494, 200, 918]) @pytest.mark.parametrize('show_prefix', [True, False]) -@pytest.mark.parametrize('template_type', ['sms', 'email', 'letter']) +@pytest.mark.parametrize('template_type', ['sms', 'email']) def test_check_is_message_too_long_passes(notify_db_session, show_prefix, char_count, template_type): service = create_service(prefix_sms=show_prefix) t = create_template(service=service, content='a' * char_count, template_type=template_type) @@ -502,7 +500,7 @@ def test_validate_and_format_recipient_fails_when_no_recipient(): assert e.value.message == "Recipient can't be empty" -@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) +@pytest.mark.parametrize('notification_type', ['sms', 'email']) def test_check_service_email_reply_to_id_where_reply_to_id_is_none(notification_type): assert check_service_email_reply_to_id(None, None, notification_type) is None @@ -529,7 +527,7 @@ def test_check_service_email_reply_to_id_where_reply_to_id_is_not_found(sample_s .format(fake_uuid, sample_service.id) -@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) +@pytest.mark.parametrize('notification_type', ['sms', 'email']) def test_check_service_sms_sender_id_where_sms_sender_id_is_none(notification_type): assert check_service_sms_sender_id(None, None, notification_type) is None @@ -556,33 +554,7 @@ def test_check_service_sms_sender_id_where_sms_sender_is_not_found(sample_servic .format(fake_uuid, sample_service.id) -def test_check_service_letter_contact_id_where_letter_contact_id_is_none(): - assert check_service_letter_contact_id(None, None, 'letter') is None - - -def test_check_service_letter_contact_id_where_letter_contact_id_is_found(sample_service): - letter_contact = create_letter_contact(service=sample_service, contact_block='123456') - assert check_service_letter_contact_id(sample_service.id, letter_contact.id, LETTER_TYPE) == '123456' - - -def test_check_service_letter_contact_id_where_service_id_is_not_found(sample_service, fake_uuid): - letter_contact = create_letter_contact(service=sample_service, contact_block='123456') - with pytest.raises(BadRequestError) as e: - check_service_letter_contact_id(fake_uuid, letter_contact.id, LETTER_TYPE) - assert e.value.status_code == 400 - assert e.value.message == 'letter_contact_id {} does not exist in database for service id {}' \ - .format(letter_contact.id, fake_uuid) - - -def test_check_service_letter_contact_id_where_letter_contact_is_not_found(sample_service, fake_uuid): - with pytest.raises(BadRequestError) as e: - check_service_letter_contact_id(sample_service.id, fake_uuid, LETTER_TYPE) - assert e.value.status_code == 400 - assert e.value.message == 'letter_contact_id {} does not exist in database for service id {}' \ - .format(fake_uuid, sample_service.id) - - -@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) +@pytest.mark.parametrize('notification_type', ['sms', 'email']) def test_check_reply_to_with_empty_reply_to(sample_service, notification_type): assert check_reply_to(sample_service.id, None, notification_type) is None @@ -597,11 +569,6 @@ def test_check_reply_to_sms_type(sample_service): assert check_reply_to(sample_service.id, sms_sender.id, SMS_TYPE) == '123456' -def test_check_reply_to_letter_type(sample_service): - letter_contact = create_letter_contact(service=sample_service, contact_block='123456') - assert check_reply_to(sample_service.id, letter_contact.id, LETTER_TYPE) == '123456' - - @pytest.mark.skip(reason="Needs updating for TTS: Failing for unknown reason") def test_check_if_service_can_send_files_by_email_raises_if_no_contact_link_set(sample_service): with pytest.raises(BadRequestError) as e: diff --git a/tests/app/service/send_notification/test_send_notification.py b/tests/app/service/send_notification/test_send_notification.py index 83ee417c9..83122dcbc 100644 --- a/tests/app/service/send_notification/test_send_notification.py +++ b/tests/app/service/send_notification/test_send_notification.py @@ -1167,8 +1167,7 @@ def test_should_not_allow_email_notifications_if_service_permission_not_set( @pytest.mark.parametrize( "notification_type, err_msg", - [("letter", "letter notification type is not supported, please use the latest version of the client"), - ("apple", "apple notification type is not supported")]) + [("apple", "apple notification type is not supported")]) def test_should_throw_exception_if_notification_type_is_invalid(client, sample_service, notification_type, err_msg): auth_header = create_service_authorization_header(service_id=sample_service.id) response = client.post( diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index ede410310..05d043661 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -47,7 +47,6 @@ from tests.app.db import ( create_ft_billing, create_ft_notification_status, create_inbound_number, - create_letter_contact, create_notification, create_organisation, create_reply_to_email, @@ -2771,192 +2770,6 @@ def test_get_email_reply_to_address(client, notify_db_session): assert json.loads(response.get_data(as_text=True)) == reply_to.serialize() -def test_get_letter_contacts_when_there_are_no_letter_contacts(client, sample_service): - response = client.get('/service/{}/letter-contact'.format(sample_service.id), - headers=[create_admin_authorization_header()]) - - assert json.loads(response.get_data(as_text=True)) == [] - assert response.status_code == 200 - - -def test_get_letter_contacts_with_one_letter_contact(client, notify_db_session): - service = create_service() - create_letter_contact(service, 'Aberdeen, AB23 1XH') - - response = client.get('/service/{}/letter-contact'.format(service.id), - headers=[create_admin_authorization_header()]) - json_response = json.loads(response.get_data(as_text=True)) - - assert len(json_response) == 1 - assert json_response[0]['contact_block'] == 'Aberdeen, AB23 1XH' - assert json_response[0]['is_default'] - assert json_response[0]['created_at'] - assert not json_response[0]['updated_at'] - assert response.status_code == 200 - - -def test_get_letter_contacts_with_multiple_letter_contacts(client, notify_db_session): - service = create_service() - letter_contact_a = create_letter_contact(service, 'Aberdeen, AB23 1XH') - letter_contact_b = create_letter_contact(service, 'London, E1 8QS', False) - - response = client.get('/service/{}/letter-contact'.format(service.id), - headers=[create_admin_authorization_header()]) - json_response = json.loads(response.get_data(as_text=True)) - - assert len(json_response) == 2 - assert response.status_code == 200 - - assert json_response[0]['id'] == str(letter_contact_a.id) - assert json_response[0]['service_id'] == str(letter_contact_a.service_id) - assert json_response[0]['contact_block'] == 'Aberdeen, AB23 1XH' - assert json_response[0]['is_default'] - assert json_response[0]['created_at'] - assert not json_response[0]['updated_at'] - - assert json_response[1]['id'] == str(letter_contact_b.id) - assert json_response[1]['service_id'] == str(letter_contact_b.service_id) - assert json_response[1]['contact_block'] == 'London, E1 8QS' - assert not json_response[1]['is_default'] - assert json_response[1]['created_at'] - assert not json_response[1]['updated_at'] - - -def test_get_letter_contact_by_id(client, notify_db_session): - service = create_service() - letter_contact = create_letter_contact(service, 'London, E1 8QS') - - response = client.get('/service/{}/letter-contact/{}'.format(service.id, letter_contact.id), - headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) - - assert response.status_code == 200 - assert json.loads(response.get_data(as_text=True)) == letter_contact.serialize() - - -def test_get_letter_contact_return_404_when_invalid_contact_id(client, notify_db_session): - service = create_service() - - response = client.get('/service/{}/letter-contact/{}'.format(service.id, '93d59f88-4aa1-453c-9900-f61e2fc8a2de'), - headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) - - assert response.status_code == 404 - - -def test_add_service_contact_block(client, sample_service): - data = json.dumps({"contact_block": "London, E1 8QS", "is_default": True}) - response = client.post('/service/{}/letter-contact'.format(sample_service.id), - data=data, - headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) - - assert response.status_code == 201 - json_resp = json.loads(response.get_data(as_text=True)) - results = ServiceLetterContact.query.all() - assert len(results) == 1 - assert json_resp['data'] == results[0].serialize() - - -def test_add_service_letter_contact_can_add_multiple_addresses(client, sample_service): - first = json.dumps({"contact_block": "London, E1 8QS", "is_default": True}) - client.post('/service/{}/letter-contact'.format(sample_service.id), - data=first, - headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) - - second = json.dumps({"contact_block": "Aberdeen, AB23 1XH", "is_default": True}) - response = client.post('/service/{}/letter-contact'.format(sample_service.id), - data=second, - headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) - assert response.status_code == 201 - json_resp = json.loads(response.get_data(as_text=True)) - results = ServiceLetterContact.query.all() - assert len(results) == 2 - default = [x for x in results if x.is_default] - assert json_resp['data'] == default[0].serialize() - first_letter_contact_not_default = [x for x in results if not x.is_default] - assert first_letter_contact_not_default[0].contact_block == 'London, E1 8QS' - - -def test_add_service_letter_contact_block_fine_if_no_default(client, sample_service): - data = json.dumps({"contact_block": "London, E1 8QS", "is_default": False}) - response = client.post('/service/{}/letter-contact'.format(sample_service.id), - data=data, - headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) - assert response.status_code == 201 - - -def test_add_service_letter_contact_block_404s_when_invalid_service_id(client, notify_db_session): - response = client.post('/service/{}/letter-contact'.format(uuid.uuid4()), - data={}, - headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) - - assert response.status_code == 404 - result = json.loads(response.get_data(as_text=True)) - assert result['result'] == 'error' - assert result['message'] == 'No result found' - - -def test_update_service_letter_contact(client, sample_service): - original_letter_contact = create_letter_contact(service=sample_service, contact_block="Aberdeen, AB23 1XH") - data = json.dumps({"contact_block": "London, E1 8QS", "is_default": True}) - response = client.post('/service/{}/letter-contact/{}'.format(sample_service.id, original_letter_contact.id), - data=data, - headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) - - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - results = ServiceLetterContact.query.all() - assert len(results) == 1 - assert json_resp['data'] == results[0].serialize() - - -def test_update_service_letter_contact_returns_200_when_no_default(client, sample_service): - original_reply_to = create_letter_contact(service=sample_service, contact_block="Aberdeen, AB23 1XH") - data = json.dumps({"contact_block": "London, E1 8QS", "is_default": False}) - response = client.post('/service/{}/letter-contact/{}'.format(sample_service.id, original_reply_to.id), - data=data, - headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) - assert response.status_code == 200 - - -def test_update_service_letter_contact_returns_404_when_invalid_service_id(client, notify_db_session): - response = client.post('/service/{}/letter-contact/{}'.format(uuid.uuid4(), uuid.uuid4()), - data={}, - headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) - - assert response.status_code == 404 - result = json.loads(response.get_data(as_text=True)) - assert result['result'] == 'error' - assert result['message'] == 'No result found' - - -def test_delete_service_letter_contact_can_archive_letter_contact(admin_request, notify_db_session): - service = create_service() - create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA') - letter_contact = create_letter_contact(service=service, contact_block='Swansea, SN1 3CC', is_default=False) - - admin_request.post( - 'service.delete_service_letter_contact', - service_id=service.id, - letter_contact_id=letter_contact.id, - ) - - assert letter_contact.archived is True - - -def test_delete_service_letter_contact_returns_200_if_archiving_template_default(admin_request, notify_db_session): - service = create_service() - create_letter_contact(service=service, contact_block='Edinburgh, ED1 1AA') - letter_contact = create_letter_contact(service=service, contact_block='Swansea, SN1 3CC', is_default=False) - create_template(service=service, template_type='letter', reply_to=letter_contact.id) - - response = admin_request.post( - 'service.delete_service_letter_contact', - service_id=service.id, - letter_contact_id=letter_contact.id, - _expected_status=200 - ) - assert response['data']['archived'] is True - - def test_add_service_sms_sender_can_add_multiple_senders(client, notify_db_session): service = create_service() data = { diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 434ed5e60..a30756e35 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -785,28 +785,6 @@ def test_create_a_template_with_reply_to(admin_request, sample_user): assert th.service_letter_contact_id == letter_contact.id -def test_create_a_template_with_foreign_service_reply_to(admin_request, sample_user): - service = create_service(service_permissions=['letter']) - service2 = create_service(service_name='test service', email_from='test@example.com', - service_permissions=['letter']) - letter_contact = create_letter_contact(service2, "Edinburgh, ED1 1AA") - data = { - 'name': 'my template', - 'subject': 'subject', - 'template_type': 'letter', - 'content': 'template content', - 'service': str(service.id), - 'created_by': str(sample_user.id), - 'reply_to': str(letter_contact.id), - } - - json_resp = admin_request.post('template.create_template', service_id=service.id, _data=data, _expected_status=400) - - assert json_resp['message'] == "letter_contact_id {} does not exist in database for service id {}".format( - str(letter_contact.id), str(service.id) - ) - - @pytest.mark.parametrize('post_data, expected_errors', [ ( {},