Remove scheduled_for from schema

Years ago we started to implement a way to schedule a notification. We hit a problem but we never came up with a good solution and the feature never made it back to the top of the prioriry list.

This PR removes the code for scheduled_for. There will be another PR to drop the scheduled_notifications table.

Unforetunately, I don't think we can remove the `scheduled_for` attribute from the notification.serialized method because out clients might fail if something is missing. For now I have left it in but defaulted the value to None.

app/dao/notifications_dao.py

@@ -677,12 +677,6 @@ def dao_get_notifications_by_references(references):
     ).all()
 
 
-@statsd(namespace="dao")
-def dao_created_scheduled_notification(scheduled_notification):
-    db.session.add(scheduled_notification)
-    db.session.commit()
-
-
 def dao_get_total_notifications_sent_per_day_for_performance_platform(start_date, end_date):
     """
     SELECT

app/models.py

@@ -27,7 +27,7 @@ from notifications_utils.template import (
     SMSMessageTemplate,
     LetterPrintTemplate,
 )
-from notifications_utils.timezones import convert_bst_to_utc, convert_utc_to_bst
+from notifications_utils.timezones import convert_utc_to_bst
 
 from app.hashing import (
     hashpw,
@@ -291,7 +291,6 @@ service_letter_branding = db.Table(
 
 INTERNATIONAL_SMS_TYPE = 'international_sms'
 INBOUND_SMS_TYPE = 'inbound_sms'
-SCHEDULE_NOTIFICATIONS = 'schedule_notifications'
 EMAIL_AUTH = 'email_auth'
 LETTERS_AS_PDF = 'letters_as_pdf'
 PRECOMPILED_LETTER = 'precompiled_letter'
@@ -306,7 +305,6 @@ SERVICE_PERMISSION_TYPES = [
     LETTER_TYPE,
     INTERNATIONAL_SMS_TYPE,
     INBOUND_SMS_TYPE,
-    SCHEDULE_NOTIFICATIONS,
     EMAIL_AUTH,
     LETTERS_AS_PDF,
     UPLOAD_DOCUMENT,
@@ -1415,8 +1413,6 @@ class Notification(db.Model):
     client_reference = db.Column(db.String, index=True, nullable=True)
     _personalisation = db.Column(db.String, nullable=True)
 
-    scheduled_notification = db.relationship('ScheduledNotification', uselist=False)
-
     client_reference = db.Column(db.String, index=True, nullable=True)
 
     international = db.Column(db.Boolean, nullable=False, default=False)
@@ -1632,13 +1628,7 @@ class Notification(db.Model):
             "created_by_name": self.get_created_by_name(),
             "sent_at": self.sent_at.strftime(DATETIME_FORMAT) if self.sent_at else None,
             "completed_at": self.completed_at(),
-            "scheduled_for": (
-                convert_bst_to_utc(
-                    self.scheduled_notification.scheduled_for
-                ).strftime(DATETIME_FORMAT)
-                if self.scheduled_notification
-                else None
-            ),
+            "scheduled_for": None,
             "postage": self.postage
         }
 

app/notifications/process_notifications.py

@@ -9,7 +9,6 @@ from notifications_utils.recipients import (
     validate_and_format_phone_number,
     format_email_address
 )
-from notifications_utils.timezones import convert_bst_to_utc
 
 from app import redis_store
 from app.celery import provider_tasks
@@ -22,13 +21,11 @@ from app.models import (
     SMS_TYPE,
     LETTER_TYPE,
     NOTIFICATION_CREATED,
-    Notification,
-    ScheduledNotification
+    Notification
 )
 from app.dao.notifications_dao import (
     dao_create_notification,
-    dao_delete_notifications_by_id,
-    dao_created_scheduled_notification
+    dao_delete_notifications_by_id
 )
 
 from app.v2.errors import BadRequestError
@@ -161,10 +158,3 @@ 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_datetime = convert_bst_to_utc(datetime.strptime(scheduled_for, "%Y-%m-%d %H:%M"))
-    scheduled_notification = ScheduledNotification(notification_id=notification_id,
-                                                   scheduled_for=scheduled_datetime)
-    dao_created_scheduled_notification(scheduled_notification)

app/notifications/validators.py

@@ -12,7 +12,7 @@ from app.dao import services_dao, templates_dao
 from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id
 from app.models import (
     INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE, LETTER_TYPE,
-    KEY_TYPE_TEST, KEY_TYPE_TEAM, SCHEDULE_NOTIFICATIONS
+    KEY_TYPE_TEST, KEY_TYPE_TEAM
 )
 from app.service.utils import service_allowed_to_send_to
 from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError
@@ -98,12 +98,6 @@ def check_if_service_can_send_files_by_email(service_contact_link, service_id):
         )
 
 
-def check_service_can_schedule_notification(permissions, scheduled_for):
-    if scheduled_for:
-        if not service_has_permission(SCHEDULE_NOTIFICATIONS, permissions):
-            raise BadRequestError(message="Cannot schedule notifications (this feature is invite-only)")
-
-
 def validate_and_format_recipient(send_to, key_type, service, notification_type, allow_whitelisted_recipients=True):
     if send_to is None:
         raise BadRequestError(message="Recipient can't be empty")

app/v2/notifications/notification_schemas.py

@@ -136,7 +136,6 @@ post_sms_request = {
         "phone_number": {"type": "string", "format": "phone_number"},
         "template_id": uuid,
         "personalisation": personalisation,
-        "scheduled_for": {"type": ["string", "null"], "format": "datetime_within_next_day"},
         "sms_sender_id": uuid
     },
     "required": ["phone_number", "template_id"],
@@ -182,7 +181,6 @@ post_email_request = {
         "email_address": {"type": "string", "format": "email_address"},
         "template_id": uuid,
         "personalisation": personalisation,
-        "scheduled_for": {"type": ["string", "null"], "format": "datetime_within_next_day"},
         "email_reply_to_id": uuid
     },
     "required": ["email_address", "template_id"],

app/v2/notifications/post_notifications.py

@@ -43,14 +43,12 @@ from app.notifications.process_letter_notifications import (
 )
 from app.notifications.process_notifications import (
     persist_notification,
-    persist_scheduled_notification,
     send_notification_to_queue,
     simulated_recipient
 )
 from app.notifications.validators import (
     check_if_service_can_send_files_by_email,
     check_rate_limiting,
-    check_service_can_schedule_notification,
     check_service_email_reply_to_id,
     check_service_has_permission,
     check_service_sms_sender_id,
@@ -129,10 +127,6 @@ def post_notification(notification_type):
 
     check_service_has_permission(notification_type, authenticated_service.permissions)
 
-    scheduled_for = form.get("scheduled_for", None)
-
-    check_service_can_schedule_notification(authenticated_service.permissions, scheduled_for)
-
     check_rate_limiting(authenticated_service, api_user)
 
     template, template_with_content = validate_template(
@@ -183,7 +177,7 @@ def post_notification(notification_type):
     resp = create_resp_partial(
         notification=notification,
         url_root=request.url_root,
-        scheduled_for=scheduled_for,
+        scheduled_for=None,
         content=template_with_content.content_with_placeholders_filled_in,
     )
     return jsonify(resp), 201
@@ -250,19 +244,15 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
         document_download_count=document_download_count
     )
 
-    scheduled_for = form.get("scheduled_for", None)
-    if scheduled_for:
-        persist_scheduled_notification(notification.id, form["scheduled_for"])
+    if not simulated:
+        queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None
+        send_notification_to_queue(
+            notification=notification,
+            research_mode=service.research_mode,
+            queue=queue_name
+        )
     else:
-        if not simulated:
-            queue_name = QueueNames.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.debug("POST simulated notification for id: {}".format(notification.id))
+        current_app.logger.debug("POST simulated notification for id: {}".format(notification.id))
 
     return notification
 

tests/app/dao/notification_dao/test_notification_dao.py

@@ -10,7 +10,6 @@ from sqlalchemy.orm.exc import NoResultFound
 
 from app.dao.notifications_dao import (
     dao_create_notification,
-    dao_created_scheduled_notification,
     dao_delete_notifications_by_id,
     dao_get_last_notification_added_for_job_id,
     dao_get_notifications_by_recipient_or_reference,
@@ -36,7 +35,6 @@ from app.models import (
     Job,
     Notification,
     NotificationHistory,
-    ScheduledNotification,
     NOTIFICATION_STATUS_TYPES,
     NOTIFICATION_STATUS_TYPES_FAILED,
     NOTIFICATION_TEMPORARY_FAILURE,
@@ -449,7 +447,6 @@ def test_save_notification_with_no_job(sample_template, mmg_provider):
 
 def test_get_notification_with_personalisation_by_id(sample_template):
     notification = create_notification(template=sample_template,
-                                       scheduled_for='2017-05-05 14:15',
                                        status='created')
     notification_from_db = get_notification_with_personalisation(
         sample_template.service.id,
@@ -457,7 +454,6 @@ def test_get_notification_with_personalisation_by_id(sample_template):
         key_type=None
     )
     assert notification == notification_from_db
-    assert notification_from_db.scheduled_notification.scheduled_for == datetime(2017, 5, 5, 14, 15)
 
 
 def test_get_notification_by_id_when_notification_exists(sample_notification):
@@ -1392,18 +1388,6 @@ def test_dao_get_notifications_by_reference(
     assert results.items[0].id == letter.id
 
 
-def test_dao_created_scheduled_notification(sample_notification):
-
-    scheduled_notification = ScheduledNotification(notification_id=sample_notification.id,
-                                                   scheduled_for=datetime.strptime("2017-01-05 14:15",
-                                                                                   "%Y-%m-%d %H:%M"))
-    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, 15)
-
-
 def test_dao_get_notifications_by_to_field_filters_status(sample_template):
     notification = create_notification(
         template=sample_template, to_field='+447700900855',

tests/app/db.py

@@ -9,8 +9,7 @@ from app.dao.invited_org_user_dao import save_invited_org_user
 from app.dao.invited_user_dao import save_invited_user
 from app.dao.jobs_dao import dao_create_job
 from app.dao.notifications_dao import (
-    dao_create_notification,
-    dao_created_scheduled_notification
+    dao_create_notification
 )
 from app.dao.organisation_dao import dao_create_organisation, dao_add_service_to_organisation
 from app.dao.permissions_dao import permission_dao
@@ -39,7 +38,6 @@ from app.models import (
     ServiceInboundApi,
     ServiceCallbackApi,
     ServiceLetterContact,
-    ScheduledNotification,
     ServicePermission,
     ServiceSmsSender,
     ServiceWhitelist,
@@ -294,14 +292,6 @@ def create_notification(
     }
     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"))
-        if status != 'created':
-            scheduled_notification.pending = False
-        dao_created_scheduled_notification(scheduled_notification)
 
     return notification
 

tests/app/notifications/test_process_notification.py

@@ -10,14 +10,12 @@ from collections import namedtuple
 from app.models import (
     Notification,
     NotificationHistory,
-    ScheduledNotification,
     Template,
     LETTER_TYPE
 )
 from app.notifications.process_notifications import (
     create_content_for_notification,
     persist_notification,
-    persist_scheduled_notification,
     send_notification_to_queue,
     simulated_recipient
 )
@@ -385,14 +383,6 @@ def test_persist_notification_with_international_info_does_not_store_for_email(
     assert persisted_notification.rate_multiplier is None
 
 
-def test_persist_scheduled_notification(sample_notification):
-    persist_scheduled_notification(sample_notification.id, '2017-05-12 14:15')
-    scheduled_notification = ScheduledNotification.query.all()
-    assert len(scheduled_notification) == 1
-    assert scheduled_notification[0].notification_id == sample_notification.id
-    assert scheduled_notification[0].scheduled_for == datetime.datetime(2017, 5, 12, 13, 15)
-
-
 @pytest.mark.parametrize('recipient, expected_recipient_normalised', [
     ('7900900123', '447900900123'),
     ('+447900   900 123', '447900900123'),

tests/app/v2/notifications/test_get_notifications.py

@@ -21,16 +21,14 @@ def test_get_notification_by_id_returns_200(
     sample_notification = create_notification(
         template=sample_template,
         billable_units=billable_units,
-        sent_by=provider,
-        scheduled_for="2017-05-12 15:15"
+        sent_by=provider
     )
 
     # another
     create_notification(
         template=sample_template,
         billable_units=billable_units,
-        sent_by=provider,
-        scheduled_for="2017-06-12 15:15"
+        sent_by=provider
     )
 
     auth_header = create_authorization_header(service_id=sample_notification.service_id)
@@ -70,7 +68,7 @@ def test_get_notification_by_id_returns_200(
         "subject": None,
         'sent_at': sample_notification.sent_at,
         'completed_at': sample_notification.completed_at(),
-        'scheduled_for': '2017-05-12T14:15:00.000000Z',
+        'scheduled_for': None,
         'postage': None,
     }
 
@@ -166,7 +164,7 @@ def test_get_notification_by_id_returns_created_by_name_if_notification_created_
     assert json_response['created_by_name'] == 'Test User'
 
 
-def test_get_notifications_returns_scheduled_for(client, sample_template):
+def test_get_notifications_returns_none_for_scheduled_for(client, sample_template):
     sample_notification_with_reference = create_notification(template=sample_template,
                                                              client_reference='some-client-reference',
                                                              scheduled_for='2017-05-23 17:15')
@@ -183,7 +181,7 @@ def test_get_notifications_returns_scheduled_for(client, sample_template):
     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:15:00.000000Z"
+    assert not json_response['notifications'][0]['scheduled_for']
 
 
 def test_get_notification_by_reference_nonexistent_reference_returns_no_notifications(client, sample_service):

tests/app/v2/notifications/test_notification_schemas.py

@@ -2,7 +2,6 @@ import uuid
 
 import pytest
 from flask import json
-from freezegun import freeze_time
 from jsonschema import ValidationError
 
 from app.models import NOTIFICATION_CREATED, EMAIL_TYPE
@@ -257,66 +256,3 @@ def valid_email_response():
         },
         "scheduled_for": ""
     }
-
-
-@pytest.mark.parametrize("schema",
-                         [post_email_request_schema, post_sms_request_schema])
-@freeze_time("2017-05-12 13:00:00")
-def test_post_schema_valid_scheduled_for(schema):
-    j = {"template_id": str(uuid.uuid4()),
-         "scheduled_for": "2017-05-12 13:15"}
-    if schema == post_email_request_schema:
-        j.update({"email_address": "joe@gmail.com"})
-    else:
-        j.update({"phone_number": "07515111111"})
-    assert validate(j, schema) == j
-
-
-@pytest.mark.parametrize("invalid_datetime",
-                         ["13:00:00 2017-01-01",
-                          "2017-31-12 13:00:00",
-                          "01-01-2017T14:00:00.0000Z"
-                          ])
-@pytest.mark.parametrize("schema",
-                         [post_email_request_schema, post_sms_request_schema])
-def test_post_email_schema_invalid_scheduled_for(invalid_datetime, schema):
-    j = {"template_id": str(uuid.uuid4()),
-         "scheduled_for": invalid_datetime}
-    if schema == post_email_request_schema:
-        j.update({"email_address": "joe@gmail.com"})
-    else:
-        j.update({"phone_number": "07515111111"})
-    with pytest.raises(ValidationError) as e:
-        validate(j, schema)
-    error = json.loads(str(e.value))
-    assert error['status_code'] == 400
-    assert error['errors'] == [{'error': 'ValidationError',
-                                'message': "scheduled_for datetime format is invalid. "
-                                           "It must be a valid ISO8601 date time format, "
-                                           "https://en.wikipedia.org/wiki/ISO_8601"}]
-
-
-@freeze_time("2017-05-12 13:00:00")
-def test_scheduled_for_raises_validation_error_when_in_the_past():
-    j = {"phone_number": "07515111111",
-         "template_id": str(uuid.uuid4()),
-         "scheduled_for": "2017-05-12 10:00"}
-    with pytest.raises(ValidationError) as e:
-        validate(j, post_sms_request_schema)
-    error = json.loads(str(e.value))
-    assert error['status_code'] == 400
-    assert error['errors'] == [{'error': 'ValidationError',
-                                'message': "scheduled_for datetime can not be in the past"}]
-
-
-@freeze_time("2017-05-12 13:00:00")
-def test_scheduled_for_raises_validation_error_when_more_than_24_hours_in_the_future():
-    j = {"phone_number": "07515111111",
-         "template_id": str(uuid.uuid4()),
-         "scheduled_for": "2017-05-13 14:00"}
-    with pytest.raises(ValidationError) as e:
-        validate(j, post_sms_request_schema)
-    error = json.loads(str(e.value))
-    assert error['status_code'] == 400
-    assert error['errors'] == [{'error': 'ValidationError',
-                                'message': "scheduled_for datetime can only be 24 hours in the future"}]

tests/app/v2/notifications/test_post_notifications.py

@@ -3,15 +3,12 @@ from unittest import mock
 from unittest.mock import call
 
 import pytest
-from freezegun import freeze_time
 from boto.exception import SQSError
 
 from app.dao.service_sms_sender_dao import dao_update_service_sms_sender
 from app.models import (
-    ScheduledNotification,
     EMAIL_TYPE,
     NOTIFICATION_CREATED,
-    SCHEDULE_NOTIFICATIONS,
     SMS_TYPE,
     INTERNATIONAL_SMS_TYPE
 )
@@ -609,56 +606,6 @@ def test_post_sms_should_persist_supplied_sms_number(client, sample_template_wit
     assert mocked.called
 
 
-@pytest.mark.parametrize("notification_type, key_send_to, send_to",
-                         [("sms", "phone_number", "07700 900 855"),
-                          ("email", "email_address", "sample@email.com")])
-@freeze_time("2017-05-14 14:00:00")
-def test_post_notification_with_scheduled_for(
-        client, notify_db_session, notification_type, key_send_to, send_to
-):
-    service = create_service(service_name=str(uuid.uuid4()),
-                             service_permissions=[EMAIL_TYPE, SMS_TYPE, SCHEDULE_NOTIFICATIONS])
-    template = create_template(service=service, template_type=notification_type)
-    data = {
-        key_send_to: send_to,
-        'template_id': str(template.id) if notification_type == EMAIL_TYPE else str(template.id),
-        'scheduled_for': '2017-05-14 14:15'
-    }
-    auth_header = create_authorization_header(service_id=service.id)
-
-    response = client.post('/v2/notifications/{}'.format(notification_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.filter_by(notification_id=resp_json["id"]).all()
-    assert len(scheduled_notification) == 1
-    assert resp_json["id"] == str(scheduled_notification[0].notification_id)
-    assert resp_json["scheduled_for"] == '2017-05-14 14:15'
-
-
-@pytest.mark.parametrize("notification_type, key_send_to, send_to",
-                         [("sms", "phone_number", "07700 900 855"),
-                          ("email", "email_address", "sample@email.com")])
-@freeze_time("2017-05-14 14:00:00")
-def test_post_notification_raises_bad_request_if_service_not_invited_to_schedule(
-        client, sample_template, sample_email_template, notification_type, key_send_to, send_to):
-    data = {
-        key_send_to: send_to,
-        'template_id': str(sample_email_template.id) if notification_type == EMAIL_TYPE else str(sample_template.id),
-        'scheduled_for': '2017-05-14 14:15'
-    }
-    auth_header = create_authorization_header(service_id=sample_template.service_id)
-
-    response = client.post('/v2/notifications/{}'.format(notification_type),
-                           data=json.dumps(data),
-                           headers=[('Content-Type', 'application/json'), auth_header])
-    assert response.status_code == 400
-    error_json = json.loads(response.get_data(as_text=True))
-    assert error_json['errors'] == [
-        {"error": "BadRequestError", "message": 'Cannot schedule notifications (this feature is invite-only)'}]
-
-
 def test_post_notification_raises_bad_request_if_not_valid_notification_type(client, sample_service):
     auth_header = create_authorization_header(service_id=sample_service.id)
     response = client.post(