Merge pull request #2714 from alphagov/docs-in-the-db

Docs in the db

app/models.py

@@ -1407,6 +1407,8 @@ class Notification(db.Model):
 
     reply_to_text = db.Column(db.String, nullable=True)
 
+    document_download_count = db.Column(db.Integer, nullable=True)
+
     postage = db.Column(db.String, nullable=True)
     CheckConstraint("""
         CASE WHEN notification_type = 'letter' THEN
@@ -1683,6 +1685,8 @@ class NotificationHistory(db.Model, HistoryModel):
         END
     """)
 
+    document_download_count = db.Column(db.Integer, nullable=True)
+
     __table_args__ = (
         db.ForeignKeyConstraint(
             ['template_id', 'template_version'],

app/notifications/process_notifications.py

@@ -70,7 +70,8 @@ def persist_notification(
     reply_to_text=None,
     billable_units=None,
     postage=None,
-    template_postage=None
+    template_postage=None,
+    document_download_count=None,
 ):
     notification_created_at = created_at or datetime.utcnow()
     if not notification_id:
@@ -94,7 +95,8 @@ def persist_notification(
         created_by_id=created_by_id,
         status=status,
         reply_to_text=reply_to_text,
-        billable_units=billable_units
+        billable_units=billable_units,
+        document_download_count=document_download_count,
     )
 
     if notification_type == SMS_TYPE:

app/v2/notifications/post_notifications.py

@@ -186,7 +186,11 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
     # Do not persist or send notification to the queue if it is a simulated recipient
     simulated = simulated_recipient(send_to, notification_type)
 
-    personalisation = process_document_uploads(form.get('personalisation'), service, simulated=simulated)
+    personalisation, document_download_count = process_document_uploads(
+        form.get('personalisation'),
+        service,
+        simulated=simulated
+    )
 
     notification = persist_notification(
         template_id=template.id,
@@ -199,7 +203,8 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
         key_type=api_key.key_type,
         client_reference=form.get('reference', None),
         simulated=simulated,
-        reply_to_text=reply_to_text
+        reply_to_text=reply_to_text,
+        document_download_count=document_download_count
     )
 
     scheduled_for = form.get("scheduled_for", None)
@@ -220,9 +225,13 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
 
 
 def process_document_uploads(personalisation_data, service, simulated=False):
+    """
+    Returns modified personalisation dict and a count of document uploads. If there are no document uploads, returns
+    a count of `None` rather than `0`.
+    """
     file_keys = [k for k, v in (personalisation_data or {}).items() if isinstance(v, dict) and 'file' in v]
     if not file_keys:
-        return personalisation_data
+        return personalisation_data, None
 
     personalisation_data = personalisation_data.copy()
 
@@ -239,7 +248,7 @@ def process_document_uploads(personalisation_data, service, simulated=False):
             except DocumentDownloadError as e:
                 raise BadRequestError(message=e.message, status_code=e.status_code)
 
-    return personalisation_data
+    return personalisation_data, len(file_keys)
 
 
 def process_letter_notification(*, letter_data, api_key, template, reply_to_text, precompiled=False):

migrations/versions/0315_document_download_count.py

@@ -0,0 +1,23 @@
+"""
+
+Revision ID: 0315_document_download_count
+Revises: 0314_populate_email_access
+Create Date: 2020-02-12 14:19:18.066425
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+revision = '0315_document_download_count'
+down_revision = '0314_populate_email_access'
+
+
+def upgrade():
+    op.add_column('notifications', sa.Column('document_download_count', sa.Integer(), nullable=True))
+    op.add_column('notification_history', sa.Column('document_download_count', sa.Integer(), nullable=True))
+
+
+def downgrade():
+    op.drop_column('notifications', 'document_download_count')
+    op.drop_column('notification_history', 'document_download_count')

tests/app/db.py

@@ -233,7 +233,8 @@ def create_notification(
         one_off=False,
         reply_to_text=None,
         created_by_id=None,
-        postage=None
+        postage=None,
+        document_download_count=None,
 ):
     assert job or template
     if job:
@@ -287,7 +288,8 @@ def create_notification(
         'normalised_to': normalised_to,
         'reply_to_text': reply_to_text,
         'created_by_id': created_by_id,
-        'postage': postage
+        'postage': postage,
+        'document_download_count': document_download_count,
     }
     notification = Notification(**data)
     dao_create_notification(notification)

tests/app/v2/notifications/test_post_notifications.py

@@ -1,4 +1,5 @@
 import uuid
+from unittest.mock import call
 
 import pytest
 from freezegun import freeze_time
@@ -55,6 +56,7 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol
     assert notifications[0].status == NOTIFICATION_CREATED
     notification_id = notifications[0].id
     assert notifications[0].postage is None
+    assert notifications[0].document_download_count is None
     assert resp_json['id'] == str(notification_id)
     assert resp_json['reference'] == reference
     assert resp_json['content']['body'] == sample_template_with_placeholders.content.replace("(( Name))", "Jo")
@@ -311,6 +313,7 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_
     assert resp_json['reference'] == reference
     assert notification.reference is None
     assert notification.reply_to_text is None
+    assert notification.document_download_count is None
     assert resp_json['content']['body'] == sample_email_template_with_placeholders.content \
         .replace('((name))', 'Bob')
     assert resp_json['content']['subject'] == sample_email_template_with_placeholders.subject \
@@ -776,17 +779,20 @@ def test_post_notification_with_document_upload(client, notify_db_session, mocke
     template = create_template(
         service=service,
         template_type='email',
-        content="Document: ((document))"
+        content="Document 1: ((first_link)). Document 2: ((second_link))"
     )
 
     mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
     document_download_mock = mocker.patch('app.v2.notifications.post_notifications.document_download_client')
-    document_download_mock.upload_document.return_value = 'https://document-url/'
+    document_download_mock.upload_document.side_effect = lambda service_id, content: f'{content}-link'
 
     data = {
         "email_address": service.users[0].email_address,
         "template_id": template.id,
-        "personalisation": {"document": {"file": "abababab"}}
+        "personalisation": {
+            "first_link": {"file": "abababab"},
+            "second_link": {"file": "cdcdcdcd"}
+        }
     }
 
     auth_header = create_authorization_header(service_id=service.id)
@@ -799,11 +805,20 @@ def test_post_notification_with_document_upload(client, notify_db_session, mocke
     resp_json = json.loads(response.get_data(as_text=True))
     assert validate(resp_json, post_email_response) == resp_json
 
+    assert document_download_mock.upload_document.call_args_list == [
+        call(service.id, 'abababab'),
+        call(service.id, 'cdcdcdcd')
+    ]
+
     notification = Notification.query.one()
     assert notification.status == NOTIFICATION_CREATED
-    assert notification.personalisation == {'document': 'https://document-url/'}
+    assert notification.personalisation == {
+        'first_link': 'abababab-link',
+        'second_link': 'cdcdcdcd-link'
+    }
+    assert notification.document_download_count == 2
 
-    assert resp_json['content']['body'] == 'Document: https://document-url/'
+    assert resp_json['content']['body'] == 'Document 1: abababab-link. Document 2: cdcdcdcd-link'
 
 
 def test_post_notification_with_document_upload_simulated(client, notify_db_session, mocker):