Merge branch 'master' into optimise-platform-admin-live-services

app/dao/notifications_dao.py

@@ -15,7 +15,7 @@ from notifications_utils.recipients import (
 )
 from notifications_utils.statsd_decorators import statsd
 from notifications_utils.timezones import convert_utc_to_bst
-from sqlalchemy import (desc, func, or_, asc)
+from sqlalchemy import (desc, func, asc)
 from sqlalchemy.orm import joinedload
 from sqlalchemy.sql import functions
 from sqlalchemy.sql.expression import case
@@ -144,17 +144,23 @@ def _update_notification_status(notification, status):
 @statsd(namespace="dao")
 @transactional
 def update_notification_status_by_id(notification_id, status, sent_by=None):
-    notification = Notification.query.with_for_update().filter(
-        Notification.id == notification_id,
-        or_(
-            Notification.status == NOTIFICATION_CREATED,
-            Notification.status == NOTIFICATION_SENDING,
-            Notification.status == NOTIFICATION_PENDING,
-            Notification.status == NOTIFICATION_SENT,
-            Notification.status == NOTIFICATION_PENDING_VIRUS_CHECK
-        )).first()
+    notification = Notification.query.with_for_update().filter(Notification.id == notification_id).first()
 
     if not notification:
+        current_app.logger.error('notification not found for id {} (update to status {})'.format(
+            notification_id,
+            status
+        ))
+        return None
+
+    if notification.status not in {
+        NOTIFICATION_CREATED,
+        NOTIFICATION_SENDING,
+        NOTIFICATION_PENDING,
+        NOTIFICATION_SENT,
+        NOTIFICATION_PENDING_VIRUS_CHECK
+    }:
+        _duplicate_update_warning(notification, status)
         return None
 
     if notification.international and not country_records_delivery(notification.phone_prefix):
@@ -170,15 +176,18 @@ def update_notification_status_by_id(notification_id, status, sent_by=None):
 @statsd(namespace="dao")
 @transactional
 def update_notification_status_by_reference(reference, status):
-    notification = Notification.query.filter(
-        Notification.reference == reference,
-        or_(
-            Notification.status == NOTIFICATION_SENDING,
-            Notification.status == NOTIFICATION_PENDING,
-            Notification.status == NOTIFICATION_SENT
-        )).first()
+    # this is used to update letters and emails
+    notification = Notification.query.filter(Notification.reference == reference).first()
 
-    if not notification or notification.status == NOTIFICATION_SENT:
+    if not notification:
+        current_app.logger.error('notification not found for reference {} (update to {})'.format(reference, status))
+        return None
+
+    if notification.status not in {
+        NOTIFICATION_SENDING,
+        NOTIFICATION_PENDING
+    }:
+        _duplicate_update_warning(notification, status)
         return None
 
     return _update_notification_status(
@@ -693,3 +702,19 @@ def guess_notification_type(search_term):
         return EMAIL_TYPE
     else:
         return SMS_TYPE
+
+
+def _duplicate_update_warning(notification, status):
+    current_app.logger.info(
+        (
+            'Duplicate callback received. Notification id {id} received a status update to {new_status}'
+            '{time_diff} after being set to {old_status}. {type} sent by {sent_by}'
+        ).format(
+            id=notification.id,
+            old_status=notification.status,
+            new_status=status,
+            time_diff=datetime.utcnow() - (notification.updated_at or notification.created_at),
+            type=notification.notification_type,
+            sent_by=notification.sent_by
+        )
+    )

app/notifications/notifications_ses_callback.py

@@ -61,9 +61,6 @@ def process_ses_response(ses_request):
                 notification_status
             )
             if not notification:
-                warning = "SES callback failed: notification either not found or already updated " \
-                          "from sending. Status {} for notification reference {}".format(notification_status, reference)
-                current_app.logger.warning(warning)
                 return
 
             if not aws_response_dict['success']:

app/notifications/process_client_response.py

@@ -81,10 +81,6 @@ def _process_for_status(notification_status, client_name, provider_reference):
         sent_by=client_name.lower()
     )
     if not notification:
-        current_app.logger.warning("{} callback failed: notification {} either not found or already updated "
-                                   "from sending. Status {}".format(client_name,
-                                                                    provider_reference,
-                                                                    notification_status))
         return
 
     statsd_client.incr('callback.{}.{}'.format(client_name.lower(), notification_status))

app/notifications/process_notifications.py

@@ -23,7 +23,8 @@ from app.models import (
     LETTER_TYPE,
     NOTIFICATION_CREATED,
     Notification,
-    ScheduledNotification
+    ScheduledNotification,
+    CHOOSE_POSTAGE
 )
 from app.dao.notifications_dao import (
     dao_create_notification,
@@ -31,6 +32,8 @@ from app.dao.notifications_dao import (
     dao_created_scheduled_notification
 )
 
+from app.dao.templates_dao import dao_get_template_by_id
+
 from app.v2.errors import BadRequestError
 from app.utils import (
     cache_key_for_service_template_counter,
@@ -109,7 +112,11 @@ def persist_notification(
     elif notification_type == EMAIL_TYPE:
         notification.normalised_to = format_email_address(notification.to)
     elif notification_type == LETTER_TYPE:
-        notification.postage = service.postage
+        template = dao_get_template_by_id(template_id, template_version)
+        if service.has_permission(CHOOSE_POSTAGE) and template.postage:
+            notification.postage = template.postage
+        else:
+            notification.postage = service.postage
 
     # if simulated create a Notification model to return but do not persist the Notification to the dB
     if not simulated:

app/template/rest.py

@@ -31,7 +31,7 @@ from app.errors import (
     InvalidRequest
 )
 from app.letters.utils import get_letter_pdf
-from app.models import SMS_TYPE, Template
+from app.models import SMS_TYPE, Template, CHOOSE_POSTAGE
 from app.notifications.validators import service_has_permission, check_reply_to
 from app.schema_validation import validate
 from app.schemas import (template_schema, template_history_schema)
@@ -78,6 +78,12 @@ def create_template(service_id):
         errors = {'template_type': [message]}
         raise InvalidRequest(errors, 403)
 
+    if new_template.postage:
+        if not service_has_permission(CHOOSE_POSTAGE, fetched_service.permissions):
+            message = "Setting postage on templates is not enabled for this service."
+            errors = {'template_postage': [message]}
+            raise InvalidRequest(errors, 403)
+
     new_template.service = fetched_service
 
     over_limit = _content_count_greater_than_limit(new_template.content, new_template.template_type)
@@ -110,6 +116,12 @@ def update_template(service_id, template_id):
     if data.get('redact_personalisation') is True:
         return redact_template(fetched_template, data)
 
+    if data.get('postage'):
+        if not service_has_permission(CHOOSE_POSTAGE, fetched_template.service.permissions):
+            message = "Setting postage on templates is not enabled for this service."
+            errors = {'template_postage': [message]}
+            raise InvalidRequest(errors, 403)
+
     if "reply_to" in data:
         check_reply_to(service_id, data.get("reply_to"), fetched_template.template_type)
         updated = dao_update_template_reply_to(template_id=template_id, reply_to=data.get("reply_to"))
@@ -191,7 +203,7 @@ def get_template_versions(service_id, template_id):
 def _template_has_not_changed(current_data, updated_template):
     return all(
         current_data[key] == updated_template[key]
-        for key in ('name', 'content', 'subject', 'archived', 'process_type')
+        for key in ('name', 'content', 'subject', 'archived', 'process_type', 'postage')
     )
 
 

app/template/template_schemas.py

@@ -17,7 +17,8 @@ post_create_template_schema = {
         "content": {"type": "string"},
         "subject": {"type": "string"},
         "created_by": uuid,
-        "parent_folder_id": uuid
+        "parent_folder_id": uuid,
+        "postage": {"type": "string"},
     },
     "if": {
         "properties": {

app/v2/template/template_schemas.py

@@ -37,6 +37,7 @@ get_template_by_id_response = {
         "body": {"type": "string"},
         "subject": {"type": ["string", "null"]},
         "name": {"type": "string"},
+        "postage": {"type": "string"}
     },
     "required": ["id", "type", "created_at", "updated_at", "version", "created_by", "body", "name"],
 }
@@ -63,7 +64,8 @@ post_template_preview_response = {
         "type": {"enum": TEMPLATE_TYPES},
         "version": {"type": "integer"},
         "body": {"type": "string"},
-        "subject": {"type": ["string", "null"]}
+        "subject": {"type": ["string", "null"]},
+        "postage": {"type": "string"}
     },
     "required": ["id", "type", "version", "body"]
 }
@@ -77,5 +79,6 @@ def create_post_template_preview_response(template, template_object):
         "type": template.template_type,
         "version": template.version,
         "body": str(template_object),
-        "subject": subject
+        "subject": subject,
+        "postage": template.postage
     }