From 366d07dbbed9d0f753479abfb15da54eb16c5133 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 21 Sep 2017 16:08:49 +0100 Subject: [PATCH 1/4] Add ServiceLetterContact data model and script --- app/models.py | 30 +++++++++++++++++ .../0122_add_service_letter_contact.py | 32 +++++++++++++++++++ tests/app/db.py | 21 +++++++++++- tests/app/test_model.py | 19 ++++++++++- 4 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 migrations/versions/0122_add_service_letter_contact.py diff --git a/app/models.py b/app/models.py index 4b3c24b94..5739f1659 100644 --- a/app/models.py +++ b/app/models.py @@ -257,6 +257,14 @@ class Service(db.Model, Versioned): else: return default_reply_to[0].email_address if default_reply_to else None + def get_default_letter_contact(self): + default_letter_contact = [x for x in self.letter_contacts if x.is_default] + if len(default_letter_contact) > 1: + raise Exception("There should only ever be one default") + else: + return default_letter_contact[0].contact_block if default_letter_contact else \ + self.letter_contact_block # need to update this to None after dropping the letter_contact_block column + class InboundNumber(db.Model): __tablename__ = "inbound_numbers" @@ -1381,3 +1389,25 @@ class ServiceEmailReplyTo(db.Model): 'created_at': self.created_at.strftime(DATETIME_FORMAT), 'updated_at': self.updated_at.strftime(DATETIME_FORMAT) if self.updated_at else None } + + +class ServiceLetterContact(db.Model): + __tablename__ = "service_letter_contacts" + + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + + 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("letter_contacts")) + + contact_block = db.Column(db.Text, nullable=False, index=False, unique=False) + is_default = db.Column(db.Boolean, nullable=False, default=True) + created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) + updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) + + def serialize(self): + return { + 'contact_block': self.contact_block, + 'is_default': self.is_default, + 'created_at': self.created_at.strftime(DATETIME_FORMAT), + 'updated_at': self.updated_at.strftime(DATETIME_FORMAT) if self.updated_at else None + } diff --git a/migrations/versions/0122_add_service_letter_contact.py b/migrations/versions/0122_add_service_letter_contact.py new file mode 100644 index 000000000..2fbe904c2 --- /dev/null +++ b/migrations/versions/0122_add_service_letter_contact.py @@ -0,0 +1,32 @@ +""" + +Revision ID: 0122_add_service_letter_contact +Revises: 0121_nullable_logos +Create Date: 2017-09-21 12:16:02.975120 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0122_add_service_letter_contact' +down_revision = '0121_nullable_logos' + + +def upgrade(): + op.create_table('service_letter_contacts', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('contact_block', sa.Text(), nullable=False), + sa.Column('is_default', sa.Boolean(), nullable=False), + sa.Column('created_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['service_id'], ['services.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_service_letter_contact_service_id'), 'service_letter_contacts', ['service_id'], unique=False) + + +def downgrade(): + op.drop_index(op.f('ix_service_letter_contact_service_id'), table_name='service_letter_contacts') + op.drop_table('service_letter_contacts') diff --git a/tests/app/db.py b/tests/app/db.py index 50c04bbde..f271d70e9 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -26,7 +26,8 @@ from app.models import ( INBOUND_SMS_TYPE, KEY_TYPE_NORMAL, ServiceInboundApi, - ServiceEmailReplyTo + ServiceEmailReplyTo, + ServiceLetterContact ) from app.dao.users_dao import save_model_user from app.dao.notifications_dao import dao_create_notification, dao_created_scheduled_notification @@ -347,3 +348,21 @@ def create_reply_to_email( db.session.commit() return reply_to + + +def create_letter_contact( + service, + contact_block, + is_default=True +): + data = { + 'service': service, + 'contact_block': contact_block, + 'is_default': is_default, + } + letter_content = ServiceLetterContact(**data) + + db.session.add(letter_content) + db.session.commit() + + return letter_content diff --git a/tests/app/test_model.py b/tests/app/test_model.py index b0bf01a4e..0303782ff 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -21,7 +21,13 @@ from tests.app.conftest import ( sample_template as create_sample_template, sample_notification_with_job as create_sample_notification_with_job ) -from tests.app.db import create_notification, create_service, create_inbound_number, create_reply_to_email +from tests.app.db import ( + create_notification, + create_service, + create_inbound_number, + create_reply_to_email, + create_letter_contact +) from tests.conftest import set_config @@ -262,3 +268,14 @@ def test_service_get_default_reply_to_email_address(sample_service): create_reply_to_email(service=sample_service, email_address="default@email.com") assert sample_service.get_default_reply_to_email_address() == 'default@email.com' + + +def test_service_get_default_contact_letter(sample_service): + create_letter_contact(service=sample_service, contact_block='London,\nNW1A 1AA') + + assert sample_service.get_default_letter_contact() == 'London,\nNW1A 1AA' + + +# this test will need to be removed after letter_contact_block is dropped +def test_service_get_default_letter_contact_block_from_service(sample_service): + assert sample_service.get_default_letter_contact() == sample_service.letter_contact_block From 91a618531dd1cb0dd069f96666792b63b0db8afa Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 21 Sep 2017 16:38:24 +0100 Subject: [PATCH 2/4] Update serialization and service schema - added id and service_id in serialization - added 'letter_contacts' to the exluded list for marshmallow service schema --- app/models.py | 2 ++ app/schemas.py | 1 + 2 files changed, 3 insertions(+) diff --git a/app/models.py b/app/models.py index 5739f1659..639d619d3 100644 --- a/app/models.py +++ b/app/models.py @@ -1406,6 +1406,8 @@ class ServiceLetterContact(db.Model): def serialize(self): return { + 'id': str(self.id), + 'service_id': str(self.service_id), 'contact_block': self.contact_block, 'is_default': self.is_default, 'created_at': self.created_at.strftime(DATETIME_FORMAT), diff --git a/app/schemas.py b/app/schemas.py index f2fe4ab82..96f64a24c 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -209,6 +209,7 @@ class ServiceSchema(BaseSchema): 'service_sms_senders', 'monthly_billing', 'reply_to_email_addresses', + 'letter_contacts', ) strict = True From 795bd4271ce66d1cdd9e38979f7ba506a23135cb Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 21 Sep 2017 17:02:58 +0100 Subject: [PATCH 3/4] New endpoint to fetch a single reply-to email address by id --- app/dao/service_email_reply_to_dao.py | 10 +++++++++ app/service/rest.py | 8 ++++++- .../dao/test_service_email_reply_to_dao.py | 22 ++++++++++++++++++- tests/app/service/test_rest.py | 11 ++++++++++ 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/app/dao/service_email_reply_to_dao.py b/app/dao/service_email_reply_to_dao.py index ef94a7130..d63395ab8 100644 --- a/app/dao/service_email_reply_to_dao.py +++ b/app/dao/service_email_reply_to_dao.py @@ -13,6 +13,16 @@ def dao_get_reply_to_by_service_id(service_id): return reply_to +def dao_get_reply_to_by_id(service_id, reply_to_id): + reply_to = db.session.query( + ServiceEmailReplyTo + ).filter( + ServiceEmailReplyTo.service_id == service_id, + ServiceEmailReplyTo.id == reply_to_id + ).order_by(ServiceEmailReplyTo.created_at).one() + return reply_to + + def create_or_update_email_reply_to(service_id, email_address): reply_to = dao_get_reply_to_by_service_id(service_id) if len(reply_to) == 0: diff --git a/app/service/rest.py b/app/service/rest.py index 61527ce1c..48f1e6f44 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -47,7 +47,7 @@ from app.dao.service_whitelist_dao import ( dao_remove_service_whitelist ) from app.dao.service_email_reply_to_dao import create_or_update_email_reply_to, dao_get_reply_to_by_service_id, \ - add_reply_to_email_address_for_service, update_reply_to_email_address + add_reply_to_email_address_for_service, update_reply_to_email_address, dao_get_reply_to_by_id from app.dao.provider_statistics_dao import get_fragment_count from app.dao.users_dao import get_user_by_id from app.errors import ( @@ -531,6 +531,12 @@ def get_email_reply_to_addresses(service_id): return jsonify([i.serialize() for i in result]), 200 +@service_blueprint.route('//email-reply-to/', methods=["GET"]) +def get_email_reply_to_address(service_id, reply_to_id): + result = dao_get_reply_to_by_id(service_id=service_id, reply_to_id=reply_to_id) + return jsonify(result.serialize()), 200 + + @service_blueprint.route('//email-reply-to', methods=['POST']) def add_service_reply_to_email_address(service_id): # validate the service exists, throws ResultNotFound exception. diff --git a/tests/app/dao/test_service_email_reply_to_dao.py b/tests/app/dao/test_service_email_reply_to_dao.py index 1761b44eb..f95badf80 100644 --- a/tests/app/dao/test_service_email_reply_to_dao.py +++ b/tests/app/dao/test_service_email_reply_to_dao.py @@ -1,9 +1,12 @@ +import uuid + import pytest +from sqlalchemy.exc import SQLAlchemyError from app.dao.service_email_reply_to_dao import ( create_or_update_email_reply_to, dao_get_reply_to_by_service_id, - add_reply_to_email_address_for_service, update_reply_to_email_address) + add_reply_to_email_address_for_service, update_reply_to_email_address, dao_get_reply_to_by_id) from app.errors import InvalidRequest from app.models import ServiceEmailReplyTo from tests.app.db import create_reply_to_email, create_service @@ -191,3 +194,20 @@ def test_update_reply_to_email_address_raises_exception_if_single_reply_to_and_s reply_to_id=first_reply_to.id, email_address='should@fail.com', is_default=False) + + +def test_dao_get_reply_to_by_id(sample_service): + reply_to = create_reply_to_email(service=sample_service, email_address='email@address.com') + result = dao_get_reply_to_by_id(service_id=sample_service.id, reply_to_id=reply_to.id) + assert result == reply_to + + +def test_dao_get_reply_to_by_id_raises_sqlalchemy_error_when_reply_to_does_not_exist(sample_service): + with pytest.raises(SQLAlchemyError): + dao_get_reply_to_by_id(service_id=sample_service.id, reply_to_id=uuid.uuid4()) + + +def test_dao_get_reply_to_by_id_raises_sqlalchemy_error_when_service_does_not_exist(sample_service): + reply_to = create_reply_to_email(service=sample_service, email_address='email@address.com') + with pytest.raises(SQLAlchemyError): + dao_get_reply_to_by_id(service_id=uuid.uuid4(), reply_to_id=reply_to.id) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 5b267be2c..0551ec7c5 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2290,3 +2290,14 @@ def test_update_service_reply_to_email_address_404s_when_invalid_service_id(clie result = json.loads(response.get_data(as_text=True)) assert result['result'] == 'error' assert result['message'] == 'No result found' + + +def test_get_email_reply_to_address(client, notify_db, notify_db_session): + service = create_service(notify_db=notify_db, notify_db_session=notify_db_session) + reply_to = create_reply_to_email(service, 'test_a@mail.com') + + response = client.get('/service/{}/email-reply-to/{}'.format(service.id, reply_to.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == reply_to.serialize() From 03ea09fd6ae5938cd516440da9089fa5af155679 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 22 Sep 2017 10:02:59 +0100 Subject: [PATCH 4/4] Add order by in the dao_get_reply_to_by_service_id() --- app/dao/service_email_reply_to_dao.py | 4 +++- tests/app/dao/test_service_email_reply_to_dao.py | 8 +++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/dao/service_email_reply_to_dao.py b/app/dao/service_email_reply_to_dao.py index d63395ab8..a3364ce4d 100644 --- a/app/dao/service_email_reply_to_dao.py +++ b/app/dao/service_email_reply_to_dao.py @@ -1,3 +1,5 @@ +from sqlalchemy import desc + from app import db from app.dao.dao_utils import transactional from app.errors import InvalidRequest @@ -9,7 +11,7 @@ def dao_get_reply_to_by_service_id(service_id): ServiceEmailReplyTo ).filter( ServiceEmailReplyTo.service_id == service_id - ).order_by(ServiceEmailReplyTo.created_at).all() + ).order_by(desc(ServiceEmailReplyTo.is_default), desc(ServiceEmailReplyTo.created_at)).all() return reply_to diff --git a/tests/app/dao/test_service_email_reply_to_dao.py b/tests/app/dao/test_service_email_reply_to_dao.py index f95badf80..66e5c261c 100644 --- a/tests/app/dao/test_service_email_reply_to_dao.py +++ b/tests/app/dao/test_service_email_reply_to_dao.py @@ -61,13 +61,15 @@ def test_create_or_update_email_reply_to_raises_exception_if_multilple_email_add def test_dao_get_reply_to_by_service_id(notify_db_session): service = create_service() default_reply_to = create_reply_to_email(service=service, email_address='something@email.com') + second_reply_to = create_reply_to_email(service=service, email_address='second@email.com', is_default=False) another_reply_to = create_reply_to_email(service=service, email_address='another@email.com', is_default=False) results = dao_get_reply_to_by_service_id(service_id=service.id) - assert len(results) == 2 - assert default_reply_to in results - assert another_reply_to in results + assert len(results) == 3 + assert default_reply_to == results[0] + assert another_reply_to == results[1] + assert second_reply_to == results[2] def test_add_reply_to_email_address_for_service_creates_first_email_for_service(notify_db_session):