From f0e2713bef87403899b4eef98c92a0d287281708 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 15 May 2017 17:27:38 +0100 Subject: [PATCH] Add scheduled_for in the post notification request form. Return scheduled for in get_notification requests. --- app/dao/notifications_dao.py | 6 ++ app/models.py | 6 +- app/notifications/process_notifications.py | 12 ++- app/v2/notifications/notification_schemas.py | 13 +++- app/v2/notifications/post_notifications.py | 23 ++++-- tests/app/conftest.py | 13 +++- tests/app/dao/test_notification_dao.py | 15 +++- .../notifications/test_get_notifications.py | 35 ++++++++- .../notifications/test_post_notifications.py | 74 +++++++++++++------ 9 files changed, 151 insertions(+), 46 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index da81c348b..1a97418d3 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -463,3 +463,9 @@ def dao_get_notifications_by_to_field(service_id, search_term): return Notification.query.filter( Notification.service_id == service_id, func.replace(func.lower(Notification.to), " ", "") == search_term.lower().replace(" ", "")).all() + + +@statsd(namespace="dao") +def dao_created_scheduled_notification(scheduled_notification): + db.session.add(scheduled_notification) + db.session.commit() diff --git a/app/models.py b/app/models.py index 1fe07460e..f15507e01 100644 --- a/app/models.py +++ b/app/models.py @@ -686,6 +686,8 @@ class Notification(db.Model): foreign(template_version) == remote(TemplateHistory.version) )) + scheduled_for = db.relationship('ScheduledNotification') + client_reference = db.Column(db.String, index=True, nullable=True) international = db.Column(db.Boolean, nullable=False, default=False) @@ -846,7 +848,9 @@ class Notification(db.Model): "subject": self.subject, "created_at": self.created_at.strftime(DATETIME_FORMAT), "sent_at": self.sent_at.strftime(DATETIME_FORMAT) if self.sent_at else None, - "completed_at": self.completed_at() + "completed_at": self.completed_at(), + "scheduled_for": self.scheduled_for[0].scheduled_for.strftime( + DATETIME_FORMAT) if self.scheduled_for else None } return serialized diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index e8510ba63..bb88f1265 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -10,8 +10,10 @@ from notifications_utils.recipients import ( from app import redis_store from app.celery import provider_tasks from notifications_utils.clients import redis -from app.dao.notifications_dao import dao_create_notification, dao_delete_notifications_and_history_by_id -from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE +from app.dao.notifications_dao import (dao_create_notification, + dao_delete_notifications_and_history_by_id, + dao_created_scheduled_notification) +from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE, ScheduledNotification from app.v2.errors import BadRequestError, SendNotificationToQueueError from app.utils import get_template_instance, cache_key_for_service_template_counter @@ -120,3 +122,9 @@ def simulated_recipient(to_address, notification_type): return to_address in formatted_simulated_numbers else: return to_address in current_app.config['SIMULATED_EMAIL_ADDRESSES'] + + +def persist_scheduled_notification(notification_id, scheduled_for): + scheduled_notification = ScheduledNotification(notification_id=notification_id, + scheduled_for=scheduled_for) + dao_created_scheduled_notification(scheduled_notification) diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 8777b49ba..c4752ef7b 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -1,3 +1,5 @@ +from datetime import datetime + from app.models import NOTIFICATION_STATUS_TYPES, TEMPLATE_TYPES from app.schema_validation.definitions import (uuid, personalisation) @@ -192,7 +194,7 @@ post_email_response = { } -def create_post_sms_response_from_notification(notification, body, from_number, url_root, service_id): +def create_post_sms_response_from_notification(notification, body, from_number, url_root, service_id, scheduled_for): return {"id": notification.id, "reference": notification.client_reference, "content": {'body': body, @@ -200,11 +202,13 @@ def create_post_sms_response_from_notification(notification, body, from_number, "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)), "template": __create_template_from_notification(notification=notification, url_root=url_root, - service_id=service_id) + service_id=service_id), + "scheduled_for": scheduled_for } -def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, service_id): +def create_post_email_response_from_notification(notification, content, subject, email_from, url_root, service_id, + scheduled_for): return { "id": notification.id, "reference": notification.client_reference, @@ -216,7 +220,8 @@ def create_post_email_response_from_notification(notification, content, subject, "uri": "{}v2/notifications/{}".format(url_root, str(notification.id)), "template": __create_template_from_notification(notification=notification, url_root=url_root, - service_id=service_id) + service_id=service_id), + "scheduled_for": scheduled_for } diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 347884ed8..a7f666a99 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -8,7 +8,8 @@ from app.notifications.process_notifications import ( create_content_for_notification, persist_notification, send_notification_to_queue, - simulated_recipient) + simulated_recipient, + persist_scheduled_notification) from app.notifications.validators import ( check_template_is_for_notification_type, check_template_is_active, @@ -57,26 +58,32 @@ def post_notification(notification_type): key_type=api_user.key_type, client_reference=form.get('reference', None), simulated=simulated) - - if not simulated: - queue_name = 'priority' if template.process_type == PRIORITY else None - send_notification_to_queue(notification=notification, research_mode=service.research_mode, queue=queue_name) + scheduled_for = form.get("scheduled_for", None) + if scheduled_for: + persist_scheduled_notification(notification.id, form["scheduled_for"]) else: - current_app.logger.info("POST simulated notification for id: {}".format(notification.id)) + if not simulated: + queue_name = 'priority' if template.process_type == PRIORITY else None + send_notification_to_queue(notification=notification, research_mode=service.research_mode, queue=queue_name) + else: + current_app.logger.info("POST simulated notification for id: {}".format(notification.id)) + if notification_type == SMS_TYPE: sms_sender = service.sms_sender if service.sms_sender else current_app.config.get('FROM_NUMBER') resp = create_post_sms_response_from_notification(notification=notification, body=str(template_with_content), from_number=sms_sender, url_root=request.url_root, - service_id=service.id) + service_id=service.id, + scheduled_for=scheduled_for) else: resp = create_post_email_response_from_notification(notification=notification, content=str(template_with_content), subject=template_with_content.subject, email_from=service.email_from, url_root=request.url_root, - service_id=service.id) + service_id=service.id, + scheduled_for=scheduled_for) return jsonify(resp), 201 diff --git a/tests/app/conftest.py b/tests/app/conftest.py index fc9748870..7e859324f 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -24,7 +24,7 @@ from app.models import ( NotificationStatistics, ServiceWhitelist, KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM, - MOBILE_TYPE, EMAIL_TYPE, LETTER_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED) + MOBILE_TYPE, EMAIL_TYPE, LETTER_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED, ScheduledNotification) from app.dao.users_dao import (create_user_code, create_secret_code) from app.dao.services_dao import (dao_create_service, dao_add_user_to_service) from app.dao.templates_dao import dao_create_template @@ -445,7 +445,8 @@ def sample_notification( key_type=KEY_TYPE_NORMAL, sent_by=None, client_reference=None, - rate_multiplier=1.0 + rate_multiplier=1.0, + scheduled_for=None ): if created_at is None: created_at = datetime.utcnow() @@ -489,6 +490,14 @@ def sample_notification( data['job_row_number'] = job_row_number notification = Notification(**data) dao_create_notification(notification) + if scheduled_for: + scheduled_notification = ScheduledNotification(id=uuid.uuid4(), + notification_id=notification.id, + scheduled_for=datetime.strptime(scheduled_for, + "%Y-%m-%d %H:%M:%S")) + db.session.add(scheduled_notification) + db.session.commit() + return notification diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 95dc3e09d..0f1f3721c 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -12,6 +12,7 @@ from app.models import ( Job, NotificationStatistics, TemplateStatistics, + ScheduledNotification, NOTIFICATION_STATUS_TYPES, NOTIFICATION_STATUS_TYPES_FAILED, NOTIFICATION_SENT, @@ -39,7 +40,9 @@ from app.dao.notifications_dao import ( dao_delete_notifications_and_history_by_id, dao_timeout_notifications, is_delivery_slow_for_provider, - dao_update_notifications_sent_to_dvla, dao_get_notifications_by_to_field) + dao_update_notifications_sent_to_dvla, + dao_get_notifications_by_to_field, + dao_created_scheduled_notification) from app.dao.services_dao import dao_update_service from tests.app.db import create_notification @@ -1691,3 +1694,13 @@ def test_dao_get_notifications_by_to_field_search_ignores_spaces(sample_template assert notification1.id in [r.id for r in results] assert notification2.id in [r.id for r in results] assert notification3.id in [r.id for r in results] + + +def test_dao_created_scheduled_notification(sample_notification): + scheduled_notification = ScheduledNotification(notification_id=sample_notification.id, + scheduled_for="2017-01-05 14:00:00") + dao_created_scheduled_notification(scheduled_notification) + saved_notification = ScheduledNotification.query.all() + assert len(saved_notification) == 1 + assert saved_notification[0].notification_id == sample_notification.id + assert saved_notification[0].scheduled_for == datetime(2017, 1, 5, 14) diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 6e3b72ed2..2b8fd0832 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -16,12 +16,17 @@ from tests.app.conftest import ( (1, None) ]) def test_get_notification_by_id_returns_200( - client, notify_db, notify_db_session, sample_provider_rate, billable_units, provider + client, notify_db, notify_db_session, billable_units, provider ): sample_notification = create_sample_notification( - notify_db, notify_db_session, billable_units=billable_units, sent_by=provider + notify_db, notify_db_session, billable_units=billable_units, sent_by=provider, + scheduled_for="2017-05-12 14:00:00" ) + another = create_sample_notification( + notify_db, notify_db_session, billable_units=billable_units, sent_by=provider, + scheduled_for="2017-06-12 14:00:00" + ) auth_header = create_authorization_header(service_id=sample_notification.service_id) response = client.get( path='/v2/notifications/{}'.format(sample_notification.id), @@ -57,7 +62,8 @@ def test_get_notification_by_id_returns_200( 'body': sample_notification.template.content, "subject": None, 'sent_at': sample_notification.sent_at, - 'completed_at': sample_notification.completed_at() + 'completed_at': sample_notification.completed_at(), + 'scheduled_for': '2017-05-12T14:00:00.000000Z' } assert json_response == expected_response @@ -105,7 +111,8 @@ def test_get_notification_by_id_with_placeholders_returns_200( 'body': "Hello Bob\nThis is an email from GOV.\u200bUK", "subject": "Bob", 'sent_at': sample_notification.sent_at, - 'completed_at': sample_notification.completed_at() + 'completed_at': sample_notification.completed_at(), + 'scheduled_for': None } assert json_response == expected_response @@ -130,6 +137,25 @@ def test_get_notification_by_reference_returns_200(client, notify_db, notify_db_ assert json_response['notifications'][0]['reference'] == "some-client-reference" +def test_get_notifications_returns_scheduled_for(client, notify_db, notify_db_session): + sample_notification_with_reference = create_sample_notification( + notify_db, notify_db_session, client_reference='some-client-reference', scheduled_for='2017-05-23 16:00:00') + + auth_header = create_authorization_header(service_id=sample_notification_with_reference.service_id) + response = client.get( + path='/v2/notifications?reference={}'.format(sample_notification_with_reference.client_reference), + headers=[('Content-Type', 'application/json'), auth_header]) + + assert response.status_code == 200 + assert response.headers['Content-type'] == 'application/json' + + json_response = json.loads(response.get_data(as_text=True)) + assert len(json_response['notifications']) == 1 + + assert json_response['notifications'][0]['id'] == str(sample_notification_with_reference.id) + assert json_response['notifications'][0]['scheduled_for'] == "2017-05-23T16:00:00.000000Z" + + def test_get_notification_by_reference_nonexistent_reference_returns_no_notifications(client, sample_service): auth_header = create_authorization_header(service_id=sample_service.id) response = client.get( @@ -208,6 +234,7 @@ def test_get_all_notifications_returns_200(client, notify_db, notify_db_session) } assert json_response['notifications'][0]['phone_number'] == "+447700900855" assert json_response['notifications'][0]['type'] == "sms" + assert not json_response['notifications'][0]['scheduled_for'] def test_get_all_notifications_no_notifications_if_no_notifications(client, sample_service): diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 14f8d6d2e..6ead0aeb5 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -1,7 +1,7 @@ import uuid import pytest from flask import json -from app.models import Notification +from app.models import Notification, ScheduledNotification from app.v2.errors import RateLimitError from tests import create_authorization_header from tests.app.conftest import sample_template as create_sample_template, sample_service @@ -41,6 +41,7 @@ def test_post_sms_notification_returns_201(notify_api, sample_template_with_plac assert 'services/{}/templates/{}'.format(sample_template_with_placeholders.service_id, sample_template_with_placeholders.id) \ in resp_json['template']['uri'] + assert not resp_json["scheduled_for"] assert mocked.called @@ -149,6 +150,7 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_ assert 'services/{}/templates/{}'.format(str(sample_email_template_with_placeholders.service_id), str(sample_email_template_with_placeholders.id)) \ in resp_json['template']['uri'] + assert not resp_json["scheduled_for"] assert mocked.called @@ -318,32 +320,56 @@ def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms(notify_db, data=json.dumps(data), headers=[('Content-Type', 'application/json'), auth_header]) - print(json.loads(response.get_data(as_text=True))) assert response.status_code == 201 assert response.headers['Content-type'] == 'application/json' -def test_post_sms_should_persist_supplied_sms_number(notify_api, sample_template_with_placeholders, mocker): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') - data = { - 'phone_number': '+(44) 77009-00855', - 'template_id': str(sample_template_with_placeholders.id), - 'personalisation': {' Name': 'Jo'} - } +def test_post_sms_should_persist_supplied_sms_number(client, sample_template_with_placeholders, mocker): + mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') + data = { + 'phone_number': '+(44) 77009-00855', + 'template_id': str(sample_template_with_placeholders.id), + 'personalisation': {' Name': 'Jo'} + } - auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) + auth_header = create_authorization_header(service_id=sample_template_with_placeholders.service_id) - response = client.post( - path='/v2/notifications/sms', - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header]) - assert response.status_code == 201 - resp_json = json.loads(response.get_data(as_text=True)) - notifications = Notification.query.all() - assert len(notifications) == 1 - notification_id = notifications[0].id - assert '+(44) 77009-00855' == notifications[0].to - assert resp_json['id'] == str(notification_id) - assert mocked.called + response = client.post( + path='/v2/notifications/sms', + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + notifications = Notification.query.all() + assert len(notifications) == 1 + notification_id = notifications[0].id + assert '+(44) 77009-00855' == notifications[0].to + assert resp_json['id'] == str(notification_id) + assert mocked.called + + +@pytest.mark.parametrize("type", ["sms", "email"]) +def test_post_notification_with_scheduled_for(client, sample_template, sample_email_template, type): + if type == 'sms': + data = { + 'phone_number': '+(44) 77009-00855', + 'template_id': str(sample_template.id), + 'scheduled_for': '2017-05-14 15:00:00' + } + else: + data = { + 'email_address': 'jack@blah.com', + 'template_id': str(sample_email_template.id), + 'scheduled_for': '2017-05-14 15:00:00' + } + auth_header = create_authorization_header(service_id=sample_template.service_id) + + response = client.post('/v2/notifications/{}'.format(type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header]) + assert response.status_code == 201 + resp_json = json.loads(response.get_data(as_text=True)) + scheduled_notification = ScheduledNotification.query.all() + assert len(scheduled_notification) == 1 + assert resp_json["id"] == str(scheduled_notification[0].notification_id) + assert resp_json["scheduled_for"] == str(scheduled_notification[0].scheduled_for)