From 4de606069493dbd3d056ccf2ceb3ec92f294fd46 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 3 Aug 2017 14:05:13 +0100 Subject: [PATCH 01/16] Add data models, dao for inbound_numbers --- app/dao/inbound_numbers_dao.py | 28 +++++++++ app/models.py | 12 ++++ .../versions/0113_add_inbound_numbers.py | 34 +++++++++++ tests/app/dao/test_inbound_numbers_dao.py | 60 +++++++++++++++++++ tests/app/db.py | 14 +++++ 5 files changed, 148 insertions(+) create mode 100644 app/dao/inbound_numbers_dao.py create mode 100644 migrations/versions/0113_add_inbound_numbers.py create mode 100644 tests/app/dao/test_inbound_numbers_dao.py diff --git a/app/dao/inbound_numbers_dao.py b/app/dao/inbound_numbers_dao.py new file mode 100644 index 000000000..4f682710f --- /dev/null +++ b/app/dao/inbound_numbers_dao.py @@ -0,0 +1,28 @@ +from app import db +from app.dao.dao_utils import transactional +from app.models import InboundNumber + + +def dao_get_inbound_numbers(): + return InboundNumber.query.all() + + +def dao_get_available_inbound_numbers(): + return InboundNumber.query.filter(InboundNumber.active, InboundNumber.service_id.is_(None)).all() + + +def dao_get_inbound_number_for_service(service_id): + return InboundNumber.query.filter(InboundNumber.service_id == service_id).all() + + +@transactional +def dao_allocate_inbound_number_to_service(service_id): + available_numbers = InboundNumber.query.filter( + InboundNumber.active, InboundNumber.service_id.is_(None)).all() + + if len(available_numbers) > 0: + available_numbers[0].service_id = service_id + + db.session.add(available_numbers[0]) + else: + raise IndexError('No inbound numbers available') diff --git a/app/models.py b/app/models.py index 4ab486276..d81dfa4c7 100644 --- a/app/models.py +++ b/app/models.py @@ -242,6 +242,18 @@ class Service(db.Model, Versioned): return cls(**fields) +class InboundNumber(db.Model): + __tablename__ = "inbound_numbers" + + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + number = db.Column(db.String(11), unique=True, nullable=False) + provider = db.Column(db.String(), nullable=False) + service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), unique=True, index=True, nullable=True) + service = db.relationship('Service') + active = db.Column(db.Boolean, index=False, unique=False, nullable=False, default=True) + created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) + + class ServicePermission(db.Model): __tablename__ = "service_permissions" diff --git a/migrations/versions/0113_add_inbound_numbers.py b/migrations/versions/0113_add_inbound_numbers.py new file mode 100644 index 000000000..6bcf5b52d --- /dev/null +++ b/migrations/versions/0113_add_inbound_numbers.py @@ -0,0 +1,34 @@ +"""empty message + +Revision ID: 0113_add_inbound_numbers +Revises: 0112_add_start_end_dates +Create Date: 2017-08-03 11:08:00.970476 + +""" + +# revision identifiers, used by Alembic. +revision = '0113_add_inbound_numbers' +down_revision = '0112_add_start_end_dates' + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +def upgrade(): + op.create_table('inbound_numbers', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('number', sa.String(length=11), nullable=False), + sa.Column('provider', sa.String(), nullable=False), + sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.Column('active', sa.Boolean(), nullable=False), + sa.Column('created_at', sa.DateTime(), nullable=False), + sa.ForeignKeyConstraint(['service_id'], ['services.id'], ), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('number') + ) + op.create_index(op.f('ix_inbound_numbers_service_id'), 'inbound_numbers', ['service_id'], unique=True) + + +def downgrade(): + op.drop_index(op.f('ix_inbound_numbers_service_id'), table_name='inbound_numbers') + op.drop_table('inbound_numbers') diff --git a/tests/app/dao/test_inbound_numbers_dao.py b/tests/app/dao/test_inbound_numbers_dao.py new file mode 100644 index 000000000..f07b818e3 --- /dev/null +++ b/tests/app/dao/test_inbound_numbers_dao.py @@ -0,0 +1,60 @@ +import pytest + +from app.dao.inbound_numbers_dao import ( + dao_get_inbound_numbers, + dao_get_available_inbound_numbers, + dao_get_inbound_number_for_service, + dao_allocate_inbound_number_to_service +) +from app.models import InboundNumber + +from tests.app.db import create_inbound_number, create_service + + +@pytest.fixture +def service_1(notify_db, notify_db_session): + return create_service() + + +@pytest.fixture +def sample_inbound_numbers(notify_db, notify_db_session, service_1): + inbound_numbers = [] + inbound_numbers.append(create_inbound_number(number='1', provider='mmg')) + inbound_numbers.append(create_inbound_number(number='2', provider='mmg', active=False)) + inbound_numbers.append(create_inbound_number(number='3', provider='firetext', service_id=service_1.id)) + return inbound_numbers + + +def test_get_inbound_numbers(notify_db, notify_db_session, sample_inbound_numbers, service_1): + res = dao_get_inbound_numbers() + + assert len(res) == 3 + assert res == sample_inbound_numbers + + +def test_get_available_inbound_numbers(notify_db, notify_db_session, sample_inbound_numbers): + res = dao_get_available_inbound_numbers() + + assert len(res) == 1 + assert res[0] == sample_inbound_numbers[0] + + +def test_allocate_inbound_number_to_service( + notify_db, notify_db_session, sample_inbound_numbers): + service = create_service(service_name='test service') + + dao_allocate_inbound_number_to_service(service.id) + + res = InboundNumber.query.filter(InboundNumber.service_id == service.id).all() + + assert len(res) == 1 + assert res[0].service_id == service.id + + +def test_get_inbound_number_for_service(notify_db, notify_db_session, sample_inbound_numbers, service_1): + res = dao_get_inbound_number_for_service(service_1.id) + + assert len(res) == 1 + assert res[0].number == '3' + assert res[0].provider == 'firetext' + assert res[0].service_id == service_1.id diff --git a/tests/app/db.py b/tests/app/db.py index 71bf29bcb..51b06c78a 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -15,6 +15,7 @@ from app.models import ( Rate, Job, InboundSms, + InboundNumber, Organisation, EMAIL_TYPE, SMS_TYPE, @@ -276,3 +277,16 @@ def create_api_key(service, key_type=KEY_TYPE_NORMAL): db.session.add(api_key) db.session.commit() return api_key + + +def create_inbound_number(number, provider, active=True, service_id=None): + inbound_number = InboundNumber( + id=uuid.uuid4(), + number=number, + provider=provider, + active=active, + service_id=service_id + ) + db.session.add(inbound_number) + db.session.commit() + return inbound_number From 4efb3238dbf8e4c2190c1819fce1b07e6c7e2d51 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 3 Aug 2017 17:26:48 +0100 Subject: [PATCH 02/16] Added test for allocating a service an inbound number twice --- tests/app/dao/test_inbound_numbers_dao.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/app/dao/test_inbound_numbers_dao.py b/tests/app/dao/test_inbound_numbers_dao.py index f07b818e3..9299ddd52 100644 --- a/tests/app/dao/test_inbound_numbers_dao.py +++ b/tests/app/dao/test_inbound_numbers_dao.py @@ -51,10 +51,24 @@ def test_allocate_inbound_number_to_service( assert res[0].service_id == service.id +def test_allocating_a_service_twice_will_raise_an_error( + notify_db, notify_db_session, sample_inbound_numbers): + service = create_service(service_name='test service') + + dao_allocate_inbound_number_to_service(service.id) + + with pytest.raises(IndexError) as e: + dao_allocate_inbound_number_to_service(service.id) + + res = InboundNumber.query.filter(InboundNumber.service_id == service.id).all() + + assert len(res) == 1 + assert res[0].service_id == service.id + assert str(e.value) == 'No inbound numbers available' + + def test_get_inbound_number_for_service(notify_db, notify_db_session, sample_inbound_numbers, service_1): res = dao_get_inbound_number_for_service(service_1.id) assert len(res) == 1 - assert res[0].number == '3' - assert res[0].provider == 'firetext' - assert res[0].service_id == service_1.id + assert res[0] == sample_inbound_numbers[2] From 3c392596a3232f4266f9a99be401fbbc4c50183a Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 4 Aug 2017 12:13:10 +0100 Subject: [PATCH 03/16] Add backref in InboundNumber model --- app/models.py | 12 ++++++++- tests/app/conftest.py | 11 ++++++++- tests/app/dao/test_inbound_numbers_dao.py | 30 ++++++----------------- tests/app/dao/test_services_dao.py | 14 +++++++++++ 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/app/models.py b/app/models.py index d81dfa4c7..80e8ac805 100644 --- a/app/models.py +++ b/app/models.py @@ -249,10 +249,20 @@ class InboundNumber(db.Model): number = db.Column(db.String(11), unique=True, nullable=False) provider = db.Column(db.String(), nullable=False) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), unique=True, index=True, nullable=True) - service = db.relationship('Service') + service = db.relationship(Service, backref=db.backref("inbound_number", uselist=False)) active = db.Column(db.Boolean, index=False, unique=False, nullable=False, default=True) created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) + def serialize(self): + serialized = { + "id": str(self.id), + "number": self.number, + "provider": self.provider, + "service_id": self.service_id, + "active": self.active, + "created_at": self.created_at.strftime(DATETIME_FORMAT), + } + class ServicePermission(db.Model): __tablename__ = "service_permissions" diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 30a9d4210..3d6402da2 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -39,7 +39,7 @@ from app.dao.invited_user_dao import save_invited_user from app.dao.provider_rates_dao import create_provider_rates from app.clients.sms.firetext import FiretextClient from tests import create_authorization_header -from tests.app.db import create_user, create_template, create_notification, create_api_key +from tests.app.db import create_user, create_template, create_notification, create_api_key, create_inbound_number @pytest.yield_fixture @@ -983,6 +983,15 @@ def sample_provider_rate(notify_db, notify_db_session, valid_from=None, rate=Non ) +@pytest.fixture +def sample_inbound_numbers(notify_db, notify_db_session, sample_service): + inbound_numbers = [] + inbound_numbers.append(create_inbound_number(number='1', provider='mmg')) + inbound_numbers.append(create_inbound_number(number='2', provider='mmg', active=False)) + inbound_numbers.append(create_inbound_number(number='3', provider='firetext', service_id=sample_service.id)) + return inbound_numbers + + @pytest.fixture def restore_provider_details(notify_db, notify_db_session): """ diff --git a/tests/app/dao/test_inbound_numbers_dao.py b/tests/app/dao/test_inbound_numbers_dao.py index 9299ddd52..969dccceb 100644 --- a/tests/app/dao/test_inbound_numbers_dao.py +++ b/tests/app/dao/test_inbound_numbers_dao.py @@ -8,24 +8,10 @@ from app.dao.inbound_numbers_dao import ( ) from app.models import InboundNumber -from tests.app.db import create_inbound_number, create_service +from tests.app.db import create_service -@pytest.fixture -def service_1(notify_db, notify_db_session): - return create_service() - - -@pytest.fixture -def sample_inbound_numbers(notify_db, notify_db_session, service_1): - inbound_numbers = [] - inbound_numbers.append(create_inbound_number(number='1', provider='mmg')) - inbound_numbers.append(create_inbound_number(number='2', provider='mmg', active=False)) - inbound_numbers.append(create_inbound_number(number='3', provider='firetext', service_id=service_1.id)) - return inbound_numbers - - -def test_get_inbound_numbers(notify_db, notify_db_session, sample_inbound_numbers, service_1): +def test_get_inbound_numbers(notify_db, notify_db_session, sample_inbound_numbers): res = dao_get_inbound_numbers() assert len(res) == 3 @@ -39,8 +25,7 @@ def test_get_available_inbound_numbers(notify_db, notify_db_session, sample_inbo assert res[0] == sample_inbound_numbers[0] -def test_allocate_inbound_number_to_service( - notify_db, notify_db_session, sample_inbound_numbers): +def test_allocate_inbound_number_to_service(notify_db, notify_db_session, sample_inbound_numbers): service = create_service(service_name='test service') dao_allocate_inbound_number_to_service(service.id) @@ -51,8 +36,7 @@ def test_allocate_inbound_number_to_service( assert res[0].service_id == service.id -def test_allocating_a_service_twice_will_raise_an_error( - notify_db, notify_db_session, sample_inbound_numbers): +def test_allocating_a_service_twice_will_raise_an_error(notify_db, notify_db_session, sample_inbound_numbers): service = create_service(service_name='test service') dao_allocate_inbound_number_to_service(service.id) @@ -67,8 +51,8 @@ def test_allocating_a_service_twice_will_raise_an_error( assert str(e.value) == 'No inbound numbers available' -def test_get_inbound_number_for_service(notify_db, notify_db_session, sample_inbound_numbers, service_1): - res = dao_get_inbound_number_for_service(service_1.id) +def test_get_inbound_number_for_service(notify_db, notify_db_session, sample_inbound_numbers, sample_service): + res = dao_get_inbound_number_for_service(sample_service.id) assert len(res) == 1 - assert res[0] == sample_inbound_numbers[2] + assert res[0].service_id == sample_service.id diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index ee4f4f912..2047f0f54 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -7,6 +7,10 @@ from sqlalchemy.orm.exc import FlushError, NoResultFound from sqlalchemy.exc import IntegrityError from freezegun import freeze_time from app import db +from app.dao.inbound_numbers_dao import ( + dao_allocate_inbound_number_to_service, + dao_get_available_inbound_numbers +) from app.dao.services_dao import ( dao_create_service, dao_add_user_to_service, @@ -896,3 +900,13 @@ def test_dao_fetch_services_by_sms_sender(notify_db_session): services = dao_fetch_services_by_sms_sender('foo') assert {foo1.id, foo2.id} == {x.id for x in services} + + +def test_dao_allocating_inbound_number_shows_on_service(notify_db_session, sample_inbound_numbers): + inbound_numbers = dao_get_available_inbound_numbers() + + service = create_service(service_name='test service') + + dao_allocate_inbound_number_to_service(service.id) + + assert service.inbound_number.number == inbound_numbers[0].number From 61c09f142c387a0a5469f7c33da2f431b71b1919 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 4 Aug 2017 16:05:03 +0100 Subject: [PATCH 04/16] Refactored model and dao --- app/dao/inbound_numbers_dao.py | 12 +++--------- app/models.py | 4 ++-- tests/app/dao/test_inbound_numbers_dao.py | 17 +++++++++++------ tests/app/dao/test_services_dao.py | 4 ++-- tests/app/inbound_number/__init__.py | 0 5 files changed, 18 insertions(+), 19 deletions(-) create mode 100644 tests/app/inbound_number/__init__.py diff --git a/app/dao/inbound_numbers_dao.py b/app/dao/inbound_numbers_dao.py index 4f682710f..e9cdeaed2 100644 --- a/app/dao/inbound_numbers_dao.py +++ b/app/dao/inbound_numbers_dao.py @@ -16,13 +16,7 @@ def dao_get_inbound_number_for_service(service_id): @transactional -def dao_allocate_inbound_number_to_service(service_id): - available_numbers = InboundNumber.query.filter( - InboundNumber.active, InboundNumber.service_id.is_(None)).all() +def dao_set_inbound_number_to_service(service_id, inbound_number): + inbound_number.service_id = service_id - if len(available_numbers) > 0: - available_numbers[0].service_id = service_id - - db.session.add(available_numbers[0]) - else: - raise IndexError('No inbound numbers available') + db.session.add(inbound_number) diff --git a/app/models.py b/app/models.py index 80e8ac805..2c4030663 100644 --- a/app/models.py +++ b/app/models.py @@ -254,11 +254,11 @@ class InboundNumber(db.Model): created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) def serialize(self): - serialized = { + return { "id": str(self.id), "number": self.number, "provider": self.provider, - "service_id": self.service_id, + "service_id": str(self.service_id), "active": self.active, "created_at": self.created_at.strftime(DATETIME_FORMAT), } diff --git a/tests/app/dao/test_inbound_numbers_dao.py b/tests/app/dao/test_inbound_numbers_dao.py index 969dccceb..333ea5d10 100644 --- a/tests/app/dao/test_inbound_numbers_dao.py +++ b/tests/app/dao/test_inbound_numbers_dao.py @@ -1,10 +1,11 @@ import pytest +from sqlalchemy.exc import IntegrityError from app.dao.inbound_numbers_dao import ( dao_get_inbound_numbers, dao_get_available_inbound_numbers, dao_get_inbound_number_for_service, - dao_allocate_inbound_number_to_service + dao_set_inbound_number_to_service ) from app.models import InboundNumber @@ -27,8 +28,9 @@ def test_get_available_inbound_numbers(notify_db, notify_db_session, sample_inbo def test_allocate_inbound_number_to_service(notify_db, notify_db_session, sample_inbound_numbers): service = create_service(service_name='test service') + numbers = dao_get_available_inbound_numbers() - dao_allocate_inbound_number_to_service(service.id) + dao_set_inbound_number_to_service(service.id, numbers[0]) res = InboundNumber.query.filter(InboundNumber.service_id == service.id).all() @@ -37,18 +39,21 @@ def test_allocate_inbound_number_to_service(notify_db, notify_db_session, sample def test_allocating_a_service_twice_will_raise_an_error(notify_db, notify_db_session, sample_inbound_numbers): + from tests.app.db import create_inbound_number + create_inbound_number(number='4', provider='mmg') service = create_service(service_name='test service') + numbers = dao_get_available_inbound_numbers() - dao_allocate_inbound_number_to_service(service.id) + dao_set_inbound_number_to_service(service.id, numbers[0]) - with pytest.raises(IndexError) as e: - dao_allocate_inbound_number_to_service(service.id) + with pytest.raises(IntegrityError) as e: + dao_set_inbound_number_to_service(service.id, numbers[1]) res = InboundNumber.query.filter(InboundNumber.service_id == service.id).all() assert len(res) == 1 assert res[0].service_id == service.id - assert str(e.value) == 'No inbound numbers available' + assert 'duplicate key value violates unique constraint' in str(e.value) def test_get_inbound_number_for_service(notify_db, notify_db_session, sample_inbound_numbers, sample_service): diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 2047f0f54..fe03e71bc 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -8,7 +8,7 @@ from sqlalchemy.exc import IntegrityError from freezegun import freeze_time from app import db from app.dao.inbound_numbers_dao import ( - dao_allocate_inbound_number_to_service, + dao_set_inbound_number_to_service, dao_get_available_inbound_numbers ) from app.dao.services_dao import ( @@ -907,6 +907,6 @@ def test_dao_allocating_inbound_number_shows_on_service(notify_db_session, sampl service = create_service(service_name='test service') - dao_allocate_inbound_number_to_service(service.id) + dao_set_inbound_number_to_service(service.id, inbound_numbers[0]) assert service.inbound_number.number == inbound_numbers[0].number diff --git a/tests/app/inbound_number/__init__.py b/tests/app/inbound_number/__init__.py new file mode 100644 index 000000000..e69de29bb From ffc2da2369083a6defe447edd129c424e5eb5982 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 4 Aug 2017 19:06:37 +0100 Subject: [PATCH 05/16] Update serialization for services --- app/models.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/models.py b/app/models.py index 2c4030663..c9123a03a 100644 --- a/app/models.py +++ b/app/models.py @@ -254,11 +254,17 @@ class InboundNumber(db.Model): created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) def serialize(self): + def serialize_service(): + return { + "id": str(self.service_id), + "name": self.service.name + } + return { "id": str(self.id), "number": self.number, "provider": self.provider, - "service_id": str(self.service_id), + "service": serialize_service() if self.service else None, "active": self.active, "created_at": self.created_at.strftime(DATETIME_FORMAT), } From 5e2d0bf5c62d24b0ddc2014fef384a9245dc358b Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 4 Aug 2017 19:08:05 +0100 Subject: [PATCH 06/16] Update test fixture to provide default provider --- tests/app/db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/db.py b/tests/app/db.py index 51b06c78a..981b62fc9 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -279,7 +279,7 @@ def create_api_key(service, key_type=KEY_TYPE_NORMAL): return api_key -def create_inbound_number(number, provider, active=True, service_id=None): +def create_inbound_number(number, provider='mmg', active=True, service_id=None): inbound_number = InboundNumber( id=uuid.uuid4(), number=number, From a127a6e871170bd33cc5f089babff47009e22207 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 4 Aug 2017 19:09:05 +0100 Subject: [PATCH 07/16] Added active flag setting in inbound_number_dao --- app/dao/inbound_numbers_dao.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/dao/inbound_numbers_dao.py b/app/dao/inbound_numbers_dao.py index e9cdeaed2..f7b28e089 100644 --- a/app/dao/inbound_numbers_dao.py +++ b/app/dao/inbound_numbers_dao.py @@ -12,7 +12,7 @@ def dao_get_available_inbound_numbers(): def dao_get_inbound_number_for_service(service_id): - return InboundNumber.query.filter(InboundNumber.service_id == service_id).all() + return InboundNumber.query.filter(InboundNumber.service_id == service_id).first() @transactional @@ -20,3 +20,11 @@ def dao_set_inbound_number_to_service(service_id, inbound_number): inbound_number.service_id = service_id db.session.add(inbound_number) + + +@transactional +def dao_set_inbound_number_active_flag_for_service(service_id, active): + inbound_number = dao_get_inbound_number_for_service(service_id) + inbound_number.active = active + + db.session.add(inbound_number) From c9f871c0c92331d80dbe3bc85d47b2f1eee5636d Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 4 Aug 2017 19:10:22 +0100 Subject: [PATCH 08/16] Refatored tests and fixtures for inbound_number --- tests/app/conftest.py | 19 ++++++-- tests/app/dao/test_inbound_numbers_dao.py | 56 +++++++++++++++++------ 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 3d6402da2..4503d90c2 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -39,7 +39,14 @@ from app.dao.invited_user_dao import save_invited_user from app.dao.provider_rates_dao import create_provider_rates from app.clients.sms.firetext import FiretextClient from tests import create_authorization_header -from tests.app.db import create_user, create_template, create_notification, create_api_key, create_inbound_number +from tests.app.db import ( + create_user, + create_template, + create_notification, + create_service, + create_api_key, + create_inbound_number +) @pytest.yield_fixture @@ -985,9 +992,10 @@ def sample_provider_rate(notify_db, notify_db_session, valid_from=None, rate=Non @pytest.fixture def sample_inbound_numbers(notify_db, notify_db_session, sample_service): + service = create_service(service_name='sample service 2') inbound_numbers = [] inbound_numbers.append(create_inbound_number(number='1', provider='mmg')) - inbound_numbers.append(create_inbound_number(number='2', provider='mmg', active=False)) + inbound_numbers.append(create_inbound_number(number='2', provider='mmg', active=False, service_id=service.id)) inbound_numbers.append(create_inbound_number(number='3', provider='firetext', service_id=sample_service.id)) return inbound_numbers @@ -1043,8 +1051,11 @@ def admin_request(client): data=json.dumps(_data), headers=[('Content-Type', 'application/json'), create_authorization_header()] ) - json_resp = json.loads(resp.get_data(as_text=True)) - assert resp.status_code == _expected_status, json_resp + if resp.get_data(as_text=True): + json_resp = json.loads(resp.get_data(as_text=True)) + else: + json_resp = None + assert resp.status_code == _expected_status return json_resp @staticmethod diff --git a/tests/app/dao/test_inbound_numbers_dao.py b/tests/app/dao/test_inbound_numbers_dao.py index 333ea5d10..40b2db478 100644 --- a/tests/app/dao/test_inbound_numbers_dao.py +++ b/tests/app/dao/test_inbound_numbers_dao.py @@ -5,11 +5,12 @@ from app.dao.inbound_numbers_dao import ( dao_get_inbound_numbers, dao_get_available_inbound_numbers, dao_get_inbound_number_for_service, - dao_set_inbound_number_to_service + dao_set_inbound_number_to_service, + dao_set_inbound_number_active_flag_for_service ) from app.models import InboundNumber -from tests.app.db import create_service +from tests.app.db import create_service, create_inbound_number def test_get_inbound_numbers(notify_db, notify_db_session, sample_inbound_numbers): @@ -26,7 +27,7 @@ def test_get_available_inbound_numbers(notify_db, notify_db_session, sample_inbo assert res[0] == sample_inbound_numbers[0] -def test_allocate_inbound_number_to_service(notify_db, notify_db_session, sample_inbound_numbers): +def test_set_service_id_on_inbound_number(notify_db, notify_db_session, sample_inbound_numbers): service = create_service(service_name='test service') numbers = dao_get_available_inbound_numbers() @@ -38,9 +39,34 @@ def test_allocate_inbound_number_to_service(notify_db, notify_db_session, sample assert res[0].service_id == service.id -def test_allocating_a_service_twice_will_raise_an_error(notify_db, notify_db_session, sample_inbound_numbers): - from tests.app.db import create_inbound_number - create_inbound_number(number='4', provider='mmg') +def test_after_setting_service_id_that_inbound_number_is_unavailable( + notify_db, notify_db_session, sample_inbound_numbers): + service = create_service(service_name='test service') + numbers = dao_get_available_inbound_numbers() + + assert len(numbers) == 1 + + dao_set_inbound_number_to_service(service.id, numbers[0]) + + res = dao_get_available_inbound_numbers() + + assert len(res) == 0 + + +def test_get_inbound_number_for_service(notify_db, notify_db_session, sample_inbound_numbers): + service = create_service(service_name='test service') + inbound_number = create_inbound_number(number='4') + + dao_set_inbound_number_to_service(service.id, inbound_number) + + res = dao_get_inbound_number_for_service(service.id) + + assert res.service_id == service.id + + +def test_setting_a_service_twice_will_raise_an_error(notify_db, notify_db_session): + create_inbound_number(number='1') + create_inbound_number(number='2') service = create_service(service_name='test service') numbers = dao_get_available_inbound_numbers() @@ -49,15 +75,19 @@ def test_allocating_a_service_twice_will_raise_an_error(notify_db, notify_db_ses with pytest.raises(IntegrityError) as e: dao_set_inbound_number_to_service(service.id, numbers[1]) - res = InboundNumber.query.filter(InboundNumber.service_id == service.id).all() + res = dao_get_inbound_number_for_service(service.id) - assert len(res) == 1 - assert res[0].service_id == service.id + assert res.service_id == service.id assert 'duplicate key value violates unique constraint' in str(e.value) -def test_get_inbound_number_for_service(notify_db, notify_db_session, sample_inbound_numbers, sample_service): - res = dao_get_inbound_number_for_service(sample_service.id) +@pytest.mark.parametrize("active", [True, False]) +def test_set_inbound_number_active_flag(notify_db, notify_db_session, sample_service, active): + inbound_number = create_inbound_number(number='1') + dao_set_inbound_number_to_service(sample_service.id, inbound_number) - assert len(res) == 1 - assert res[0].service_id == sample_service.id + dao_set_inbound_number_active_flag_for_service(sample_service.id, active=active) + + inbound_number = dao_get_inbound_number_for_service(sample_service.id) + + assert inbound_number.active is active From 668811197c379e14e8d42479ddef9939d7be3d63 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 4 Aug 2017 19:19:43 +0100 Subject: [PATCH 09/16] Remove whitespace --- tests/app/dao/test_services_dao.py | 2 +- tests/app/db.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index fe03e71bc..b60767289 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -8,7 +8,7 @@ from sqlalchemy.exc import IntegrityError from freezegun import freeze_time from app import db from app.dao.inbound_numbers_dao import ( - dao_set_inbound_number_to_service, + dao_set_inbound_number_to_service, dao_get_available_inbound_numbers ) from app.dao.services_dao import ( diff --git a/tests/app/db.py b/tests/app/db.py index 981b62fc9..6a4f45cbf 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -281,10 +281,10 @@ def create_api_key(service, key_type=KEY_TYPE_NORMAL): def create_inbound_number(number, provider='mmg', active=True, service_id=None): inbound_number = InboundNumber( - id=uuid.uuid4(), - number=number, - provider=provider, - active=active, + id=uuid.uuid4(), + number=number, + provider=provider, + active=active, service_id=service_id ) db.session.add(inbound_number) From d101e262b5044a997316a271810f4ef43b6a6237 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 4 Aug 2017 21:04:59 +0100 Subject: [PATCH 10/16] Renamed migration scripts --- ...add_inbound_numbers.py => 0114_add_inbound_numbers.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0113_add_inbound_numbers.py => 0114_add_inbound_numbers.py} (86%) diff --git a/migrations/versions/0113_add_inbound_numbers.py b/migrations/versions/0114_add_inbound_numbers.py similarity index 86% rename from migrations/versions/0113_add_inbound_numbers.py rename to migrations/versions/0114_add_inbound_numbers.py index 6bcf5b52d..5004cdce2 100644 --- a/migrations/versions/0113_add_inbound_numbers.py +++ b/migrations/versions/0114_add_inbound_numbers.py @@ -1,14 +1,14 @@ """empty message -Revision ID: 0113_add_inbound_numbers -Revises: 0112_add_start_end_dates +Revision ID: 0114_add_inbound_numbers +Revises: 0113_job_created_by_nullable Create Date: 2017-08-03 11:08:00.970476 """ # revision identifiers, used by Alembic. -revision = '0113_add_inbound_numbers' -down_revision = '0112_add_start_end_dates' +revision = '0114_add_inbound_numbers' +down_revision = '0113_job_created_by_nullable' from alembic import op import sqlalchemy as sa From e80bc9deb294339dba6f68c521278c21cb2176f6 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 4 Aug 2017 21:26:04 +0100 Subject: [PATCH 11/16] Refactor conftest --- tests/app/conftest.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 4503d90c2..d111d8d3c 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1051,11 +1051,8 @@ def admin_request(client): data=json.dumps(_data), headers=[('Content-Type', 'application/json'), create_authorization_header()] ) - if resp.get_data(as_text=True): - json_resp = json.loads(resp.get_data(as_text=True)) - else: - json_resp = None - assert resp.status_code == _expected_status + json_resp = json.loads(resp.get_data(as_text=True)) + assert resp.status_code == _expected_status, json_resp return json_resp @staticmethod From 2cfe85a2af5f677a8ef19ae71ee8abc97f69b9d9 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 7 Aug 2017 10:16:04 +0100 Subject: [PATCH 12/16] Refactored inbound_number dao tests --- tests/app/dao/test_inbound_numbers_dao.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/app/dao/test_inbound_numbers_dao.py b/tests/app/dao/test_inbound_numbers_dao.py index 40b2db478..69d53bd7c 100644 --- a/tests/app/dao/test_inbound_numbers_dao.py +++ b/tests/app/dao/test_inbound_numbers_dao.py @@ -16,13 +16,16 @@ from tests.app.db import create_service, create_inbound_number def test_get_inbound_numbers(notify_db, notify_db_session, sample_inbound_numbers): res = dao_get_inbound_numbers() - assert len(res) == 3 + assert len(res) == len(sample_inbound_numbers) assert res == sample_inbound_numbers def test_get_available_inbound_numbers(notify_db, notify_db_session, sample_inbound_numbers): + available_numbers = [num for num in sample_inbound_numbers if num.active and num.service_id is None] + res = dao_get_available_inbound_numbers() + assert len(res) == len(available_numbers) assert len(res) == 1 assert res[0] == sample_inbound_numbers[0] From cfabab078539c30265d6cc9f9f99fe7a159c8ed7 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 10 Aug 2017 17:51:47 +0100 Subject: [PATCH 13/16] Refactor code to add updated_at --- app/dao/inbound_numbers_dao.py | 8 +++++-- app/models.py | 2 ++ .../versions/0114_add_inbound_numbers.py | 3 ++- tests/app/dao/test_inbound_numbers_dao.py | 22 +++++++------------ tests/app/test_model.py | 11 +++++++++- 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/app/dao/inbound_numbers_dao.py b/app/dao/inbound_numbers_dao.py index f7b28e089..3b8db6ff5 100644 --- a/app/dao/inbound_numbers_dao.py +++ b/app/dao/inbound_numbers_dao.py @@ -15,6 +15,10 @@ def dao_get_inbound_number_for_service(service_id): return InboundNumber.query.filter(InboundNumber.service_id == service_id).first() +def dao_get_inbound_number(inbound_number_id): + return InboundNumber.query.filter(InboundNumber.id == inbound_number_id).first() + + @transactional def dao_set_inbound_number_to_service(service_id, inbound_number): inbound_number.service_id = service_id @@ -23,8 +27,8 @@ def dao_set_inbound_number_to_service(service_id, inbound_number): @transactional -def dao_set_inbound_number_active_flag_for_service(service_id, active): - inbound_number = dao_get_inbound_number_for_service(service_id) +def dao_set_inbound_number_active_flag(inbound_number_id, active): + inbound_number = dao_get_inbound_number(inbound_number_id) inbound_number.active = active db.session.add(inbound_number) diff --git a/app/models.py b/app/models.py index c9123a03a..0f4fddf57 100644 --- a/app/models.py +++ b/app/models.py @@ -252,6 +252,7 @@ class InboundNumber(db.Model): service = db.relationship(Service, backref=db.backref("inbound_number", uselist=False)) active = db.Column(db.Boolean, index=False, unique=False, nullable=False, default=True) created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False) + updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) def serialize(self): def serialize_service(): @@ -267,6 +268,7 @@ class InboundNumber(db.Model): "service": serialize_service() if self.service else None, "active": self.active, "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/0114_add_inbound_numbers.py b/migrations/versions/0114_add_inbound_numbers.py index 5004cdce2..8dd78fa5d 100644 --- a/migrations/versions/0114_add_inbound_numbers.py +++ b/migrations/versions/0114_add_inbound_numbers.py @@ -2,7 +2,7 @@ Revision ID: 0114_add_inbound_numbers Revises: 0113_job_created_by_nullable -Create Date: 2017-08-03 11:08:00.970476 +Create Date: 2017-08-10 17:30:01.507694 """ @@ -22,6 +22,7 @@ def upgrade(): sa.Column('service_id', postgresql.UUID(as_uuid=True), nullable=True), sa.Column('active', 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'), sa.UniqueConstraint('number') diff --git a/tests/app/dao/test_inbound_numbers_dao.py b/tests/app/dao/test_inbound_numbers_dao.py index 69d53bd7c..4869ce84a 100644 --- a/tests/app/dao/test_inbound_numbers_dao.py +++ b/tests/app/dao/test_inbound_numbers_dao.py @@ -4,9 +4,9 @@ from sqlalchemy.exc import IntegrityError from app.dao.inbound_numbers_dao import ( dao_get_inbound_numbers, dao_get_available_inbound_numbers, - dao_get_inbound_number_for_service, + dao_get_inbound_number, dao_set_inbound_number_to_service, - dao_set_inbound_number_active_flag_for_service + dao_set_inbound_number_active_flag ) from app.models import InboundNumber @@ -56,15 +56,12 @@ def test_after_setting_service_id_that_inbound_number_is_unavailable( assert len(res) == 0 -def test_get_inbound_number_for_service(notify_db, notify_db_session, sample_inbound_numbers): - service = create_service(service_name='test service') - inbound_number = create_inbound_number(number='4') +def test_get_inbound_number(notify_db, notify_db_session): + inbound_number = create_inbound_number(number='1') - dao_set_inbound_number_to_service(service.id, inbound_number) + res = dao_get_inbound_number(inbound_number.id) - res = dao_get_inbound_number_for_service(service.id) - - assert res.service_id == service.id + assert res.id == inbound_number.id def test_setting_a_service_twice_will_raise_an_error(notify_db, notify_db_session): @@ -78,9 +75,6 @@ def test_setting_a_service_twice_will_raise_an_error(notify_db, notify_db_sessio with pytest.raises(IntegrityError) as e: dao_set_inbound_number_to_service(service.id, numbers[1]) - res = dao_get_inbound_number_for_service(service.id) - - assert res.service_id == service.id assert 'duplicate key value violates unique constraint' in str(e.value) @@ -89,8 +83,8 @@ def test_set_inbound_number_active_flag(notify_db, notify_db_session, sample_ser inbound_number = create_inbound_number(number='1') dao_set_inbound_number_to_service(sample_service.id, inbound_number) - dao_set_inbound_number_active_flag_for_service(sample_service.id, active=active) + dao_set_inbound_number_active_flag(inbound_number.id, active=active) - inbound_number = dao_get_inbound_number_for_service(sample_service.id) + inbound_number = dao_get_inbound_number(inbound_number.id) assert inbound_number.active is active diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 66b96ba4e..b8ffcf9b4 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -19,7 +19,7 @@ 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 +from tests.app.db import create_notification, create_service, create_inbound_number @pytest.mark.parametrize('mobile_number', [ @@ -218,3 +218,12 @@ def test_email_notification_serializes_with_subject(client, sample_email_templat def test_letter_notification_serializes_with_subject(client, sample_letter_template): res = sample_letter_template.serialize() assert res['subject'] == 'Template subject' + + +def test_inbound_number_serializes_with_service(client, notify_db_session): + service = create_service() + inbound_number = create_inbound_number(number='1', service_id=service.id) + serialized_inbound_number = inbound_number.serialize() + assert serialized_inbound_number.get('id') == str(inbound_number.id) + assert serialized_inbound_number.get('service').get('id') == str(inbound_number.service.id) + assert serialized_inbound_number.get('service').get('name') == inbound_number.service.name From 9a55e167f70300c527118aed09c3332d6dae5de1 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 11 Aug 2017 10:42:36 +0100 Subject: [PATCH 14/16] Refactored inbound_number dao --- app/dao/inbound_numbers_dao.py | 6 +----- tests/app/dao/test_inbound_numbers_dao.py | 12 ++---------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/app/dao/inbound_numbers_dao.py b/app/dao/inbound_numbers_dao.py index 3b8db6ff5..1c184b7f6 100644 --- a/app/dao/inbound_numbers_dao.py +++ b/app/dao/inbound_numbers_dao.py @@ -15,10 +15,6 @@ def dao_get_inbound_number_for_service(service_id): return InboundNumber.query.filter(InboundNumber.service_id == service_id).first() -def dao_get_inbound_number(inbound_number_id): - return InboundNumber.query.filter(InboundNumber.id == inbound_number_id).first() - - @transactional def dao_set_inbound_number_to_service(service_id, inbound_number): inbound_number.service_id = service_id @@ -28,7 +24,7 @@ def dao_set_inbound_number_to_service(service_id, inbound_number): @transactional def dao_set_inbound_number_active_flag(inbound_number_id, active): - inbound_number = dao_get_inbound_number(inbound_number_id) + inbound_number = InboundNumber.query.filter(InboundNumber.id == inbound_number_id).first() inbound_number.active = active db.session.add(inbound_number) diff --git a/tests/app/dao/test_inbound_numbers_dao.py b/tests/app/dao/test_inbound_numbers_dao.py index 4869ce84a..a37a86534 100644 --- a/tests/app/dao/test_inbound_numbers_dao.py +++ b/tests/app/dao/test_inbound_numbers_dao.py @@ -3,8 +3,8 @@ from sqlalchemy.exc import IntegrityError from app.dao.inbound_numbers_dao import ( dao_get_inbound_numbers, + dao_get_inbound_number_for_service, dao_get_available_inbound_numbers, - dao_get_inbound_number, dao_set_inbound_number_to_service, dao_set_inbound_number_active_flag ) @@ -56,14 +56,6 @@ def test_after_setting_service_id_that_inbound_number_is_unavailable( assert len(res) == 0 -def test_get_inbound_number(notify_db, notify_db_session): - inbound_number = create_inbound_number(number='1') - - res = dao_get_inbound_number(inbound_number.id) - - assert res.id == inbound_number.id - - def test_setting_a_service_twice_will_raise_an_error(notify_db, notify_db_session): create_inbound_number(number='1') create_inbound_number(number='2') @@ -85,6 +77,6 @@ def test_set_inbound_number_active_flag(notify_db, notify_db_session, sample_ser dao_set_inbound_number_active_flag(inbound_number.id, active=active) - inbound_number = dao_get_inbound_number(inbound_number.id) + inbound_number = dao_get_inbound_number_for_service(sample_service.id) assert inbound_number.active is active From 976be24cbcb3b8a06ac4c2bff13a925a72942921 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 11 Aug 2017 12:52:43 +0100 Subject: [PATCH 15/16] Refactor test --- tests/app/dao/test_inbound_numbers_dao.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/app/dao/test_inbound_numbers_dao.py b/tests/app/dao/test_inbound_numbers_dao.py index a37a86534..9d59a8110 100644 --- a/tests/app/dao/test_inbound_numbers_dao.py +++ b/tests/app/dao/test_inbound_numbers_dao.py @@ -20,14 +20,13 @@ def test_get_inbound_numbers(notify_db, notify_db_session, sample_inbound_number assert res == sample_inbound_numbers -def test_get_available_inbound_numbers(notify_db, notify_db_session, sample_inbound_numbers): - available_numbers = [num for num in sample_inbound_numbers if num.active and num.service_id is None] +def test_get_available_inbound_numbers(notify_db, notify_db_session): + inbound_number = create_inbound_number(number='1') res = dao_get_available_inbound_numbers() - assert len(res) == len(available_numbers) assert len(res) == 1 - assert res[0] == sample_inbound_numbers[0] + assert res[0] == inbound_number def test_set_service_id_on_inbound_number(notify_db, notify_db_session, sample_inbound_numbers): From 2b92c95c2293e0d53252fce83e7844a79ca71ddc Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 11 Aug 2017 13:25:09 +0100 Subject: [PATCH 16/16] Update migration script --- ...add_inbound_numbers.py => 0115_add_inbound_numbers.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0114_add_inbound_numbers.py => 0115_add_inbound_numbers.py} (86%) diff --git a/migrations/versions/0114_add_inbound_numbers.py b/migrations/versions/0115_add_inbound_numbers.py similarity index 86% rename from migrations/versions/0114_add_inbound_numbers.py rename to migrations/versions/0115_add_inbound_numbers.py index 8dd78fa5d..18644d312 100644 --- a/migrations/versions/0114_add_inbound_numbers.py +++ b/migrations/versions/0115_add_inbound_numbers.py @@ -1,14 +1,14 @@ """empty message -Revision ID: 0114_add_inbound_numbers -Revises: 0113_job_created_by_nullable +Revision ID: 0115_add_inbound_numbers +Revises: 0014_drop_monthly_billing_cols Create Date: 2017-08-10 17:30:01.507694 """ # revision identifiers, used by Alembic. -revision = '0114_add_inbound_numbers' -down_revision = '0113_job_created_by_nullable' +revision = '0115_add_inbound_numbers' +down_revision = '0014_drop_monthly_billing_cols' from alembic import op import sqlalchemy as sa