Merge branch 'master' into improve-get-services

app/delivery/send_to_providers.py

@@ -73,7 +73,7 @@ def send_sms_to_provider(notification):
                     to=validate_and_format_phone_number(notification.to, international=notification.international),
                     content=str(template),
                     reference=str(notification.id),
-                    sender=service.get_inbound_number()
+                    sender=service.get_default_sms_sender()
                 )
             except Exception as e:
                 dao_toggle_sms_provider(provider.name)

app/models.py

@@ -275,8 +275,6 @@ class Service(db.Model, Versioned):
     def get_inbound_number(self):
         if self.inbound_number and self.inbound_number.active:
             return self.inbound_number.number
-        else:
-            return self.get_default_sms_sender()
 
     def get_default_sms_sender(self):
         default_sms_sender = [x for x in self.service_sms_senders if x.is_default]

app/v2/notifications/post_notifications.py

@@ -95,7 +95,7 @@ def post_notification(notification_type):
     if notification_type == SMS_TYPE:
         create_resp_partial = functools.partial(
             create_post_sms_response_from_notification,
-            from_number=authenticated_service.get_inbound_number()
+            from_number=authenticated_service.get_default_sms_sender()
         )
     elif notification_type == EMAIL_TYPE:
         create_resp_partial = functools.partial(

tests/app/dao/test_services_dao.py

@@ -772,6 +772,7 @@ def test_dao_suspend_service_marks_service_as_inactive_and_expires_api_keys(samp
                          [("5", "1", "4"),  # a date range less than 7 days ago returns test and normal notifications
                           ("9", "8", "1"),  # a date range older than 9 days does not return test notifications.
                           ("8", "4", "2")])  # a date range that starts more than 7 days ago
+@freeze_time('2017-10-23T00:00:00')
 def test_fetch_stats_by_date_range_for_all_services_returns_test_notifications(notify_db,
                                                                                notify_db_session,
                                                                                start_delta,
@@ -795,6 +796,33 @@ def test_fetch_stats_by_date_range_for_all_services_returns_test_notifications(n
     assert results[0] == ('sms', 'created', result_one.service_id, int(expected))
 
 
+@pytest.mark.parametrize("start_delta, end_delta, expected",
+                         [("5", "1", "4"),  # a date range less than 7 days ago returns test and normal notifications
+                          ("9", "8", "1"),  # a date range older than 9 days does not return test notifications.
+                          ("8", "4", "2")])  # a date range that starts more than 7 days ago
+@freeze_time('2017-10-23T23:00:00')
+def test_fetch_stats_by_date_range_during_bst_hour_for_all_services_returns_test_notifications(
+    notify_db, notify_db_session, start_delta, end_delta, expected
+):
+    create_noti = functools.partial(create_notification, notify_db, notify_db_session)
+    result_one = create_noti(created_at=datetime.now(), key_type='test')
+    create_noti(created_at=datetime.now() - timedelta(days=2), key_type='test')
+    create_noti(created_at=datetime.now() - timedelta(days=3), key_type='test')
+    create_noti(created_at=datetime.now() - timedelta(days=4), key_type='normal')
+    create_noti(created_at=datetime.now() - timedelta(days=4), key_type='test')
+    create_noti(created_at=datetime.now() - timedelta(days=8), key_type='normal')
+    create_noti(created_at=datetime.now() - timedelta(days=9), key_type='normal')
+    create_noti(created_at=datetime.now() - timedelta(days=9), key_type='test')
+
+    start_date = (datetime.utcnow() - timedelta(days=int(start_delta))).date()
+    end_date = (datetime.utcnow() - timedelta(days=int(end_delta))).date()
+
+    results = fetch_stats_by_date_range_for_all_services(start_date, end_date, include_from_test_key=True)
+
+    assert len(results) == 1
+    assert results[0] == ('sms', 'created', result_one.service_id, int(expected))
+
+
 @freeze_time('2001-01-01T23:59:00')
 def test_dao_resume_service_marks_service_as_active_and_api_keys_are_still_revoked(sample_service, sample_api_key):
     dao_suspend_service(sample_service.id)

tests/app/delivery/test_send_to_providers.py

@@ -12,6 +12,7 @@ import app
 from app import mmg_client, firetext_client
 from app.dao import (provider_details_dao, notifications_dao)
 from app.dao.provider_details_dao import dao_switch_sms_provider_to_provider_with_identifier
+from app.dao.service_sms_sender_dao import dao_add_sms_sender_for_service
 from app.delivery import send_to_providers
 from app.models import (
     Notification,
@@ -699,14 +700,18 @@ def test_should_handle_sms_sender_and_prefix_message(
     )
 
 
-def test_should_use_inbound_number_as_sender_if_set(
+def test_should_use_inbound_number_as_sender_if_default_sms_sender(
-    sample_service,
+        notify_db_session,
-    mocker
+        mocker
 ):
-    sample_service.sms_sender = 'test sender'
+    service = create_service(sms_sender='test sender')
-    template = create_template(sample_service, content='bar')
+    inbound_number = create_inbound_number('1')
+    dao_add_sms_sender_for_service(service_id=service.id,
+                                   sms_sender=inbound_number.number,
+                                   is_default=True,
+                                   inbound_number_id=inbound_number.id)
+    template = create_template(service, content='bar')
     notification = create_notification(template)
-    inbound_number = create_inbound_number('1', service_id=sample_service.id)
 
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
@@ -721,6 +726,32 @@ def test_should_use_inbound_number_as_sender_if_set(
     )
 
 
+def test_should_use_default_sms_sender(
+        notify_db_session,
+        mocker
+):
+    service = create_service(sms_sender='test sender')
+    inbound_number = create_inbound_number('1')
+    dao_add_sms_sender_for_service(service_id=service.id,
+                                   sms_sender=inbound_number.number,
+                                   is_default=False,
+                                   inbound_number_id=inbound_number.id)
+    template = create_template(service, content='bar')
+    notification = create_notification(template)
+
+    mocker.patch('app.mmg_client.send_sms')
+    mocker.patch('app.delivery.send_to_providers.create_initial_notification_statistic_tasks')
+
+    send_to_providers.send_sms_to_provider(notification)
+
+    mmg_client.send_sms.assert_called_once_with(
+        to=ANY,
+        content=ANY,
+        reference=str(notification.id),
+        sender='test sender'
+    )
+
+
 def test_send_email_to_provider_get_linked_email_reply_to_default_is_false(
         sample_service,
         sample_email_template,

tests/app/test_model.py

@@ -252,17 +252,11 @@ def test_inbound_number_returns_inbound_number(client, notify_db_session):
     assert service.get_inbound_number() == inbound_number.number
 
 
-def test_inbound_number_returns_sms_sender(client, notify_db_session):
+def test_inbound_number_returns_none_when_no_inbound_number(client, notify_db_session):
-    service = create_service(sms_sender='testing')
-
-    assert service.get_inbound_number() == service.sms_sender
-
-
-def test_inbound_number_returns_from_number_config(client, notify_db_session):
     with set_config(client.application, 'FROM_NUMBER', 'test'):
         service = create_service(sms_sender=None)
 
-    assert service.get_inbound_number() == 'test'
+    assert not service.get_inbound_number()
 
 
 def test_service_get_default_reply_to_email_address(sample_service):

tests/app/v2/notifications/test_post_notifications.py

@@ -8,7 +8,6 @@ from app.models import (
     ScheduledNotification,
     SCHEDULE_NOTIFICATIONS,
     EMAIL_TYPE,
-    INTERNATIONAL_SMS_TYPE,
     SMS_TYPE
 )
 from flask import json, current_app
@@ -25,7 +24,11 @@ from tests.app.conftest import (
     sample_template_without_sms_permission
 )
 
-from tests.app.db import create_inbound_number, create_service, create_template, create_reply_to_email
+from tests.app.db import (
+    create_service,
+    create_template,
+    create_reply_to_email
+)
 
 
 @pytest.mark.parametrize("reference", [None, "reference_from_client"])
@@ -64,15 +67,16 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol
     assert mocked.called
 
 
-def test_post_sms_notification_uses_inbound_number_as_sender(client, sample_template_with_placeholders, mocker):
+def test_post_sms_notification_uses_inbound_number_as_sender(client, notify_db_session, mocker):
+    service = create_service(sms_sender='1', do_create_inbound_number=True)
+    template = create_template(service=service, content="Hello (( Name))\nYour thing is due soon")
     mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
     data = {
         'phone_number': '+447700900855',
-        'template_id': str(sample_template_with_placeholders.id),
+        'template_id': str(template.id),
         'personalisation': {' Name': 'Jo'}
     }
-    inbound_number = create_inbound_number('1', service_id=sample_template_with_placeholders.service_id)
+    auth_header = create_authorization_header(service_id=service.id)
-    auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id)
 
     response = client.post(
         path='/v2/notifications/sms',
@@ -85,7 +89,8 @@ def test_post_sms_notification_uses_inbound_number_as_sender(client, sample_temp
     assert len(notifications) == 1
     notification_id = notifications[0].id
     assert resp_json['id'] == str(notification_id)
-    assert resp_json['content']['from_number'] == inbound_number.number
+    assert resp_json['content']['from_number'] == '1'
+    mocked.assert_called_once_with([str(notification_id)], queue='send-sms-tasks')
 
 
 @pytest.mark.parametrize("notification_type, key_send_to, send_to",