Template and personalisation content is now merged and returned with

notifications, when retrieved by notification id, or service id (i.e. all notifications for service). There is a new element returned at top level of notification json called body, which is the template content merged with personalisation. This is consistent with api to endpoint to create notification which returns what was sent as 'body' in json response. Merging of template with personalisation is done in the NotificationStatusSchema. Personalisation data in encrypted before storing in db.
2026-07-16 03:10:48 -04:00 · 2016-06-20 16:23:56 +01:00
parent 561b02208d
commit 731bb19a9c
8 changed files with 210 additions and 24 deletions
--- a/app/celery/tasks.py
+++ b/app/celery/tasks.py
@@ -18,7 +18,6 @@ from notifications_utils.template import Template

 from notifications_utils.recipients import (
    RecipientCSV,
-    validate_and_format_phone_number,
    allowed_to_send_to
 )

@@ -233,7 +232,8 @@ def send_sms(self, service_id, notification_id, encrypted_notification, created_
            job_id=notification.get('job', None),
            job_row_number=notification.get('row_number', None),
            status='sending',
-            created_at=datetime.strptime(created_at, DATETIME_FORMAT)
+            created_at=datetime.strptime(created_at, DATETIME_FORMAT),
+            personalisation=notification.get('personalisation')
        )
        dao_create_notification(notification_db_object, TEMPLATE_TYPE_SMS)

@@ -277,7 +277,8 @@ def send_email(service_id, notification_id, encrypted_notification, created_at,
            status='sending',
            created_at=datetime.strptime(created_at, DATETIME_FORMAT),
            sent_at=sent_at,
-            sent_by=provider.get_name()
+            sent_by=provider.get_name(),
+            personalisation=notification.get('personalisation')
        )

        dao_create_notification(notification_db_object, TEMPLATE_TYPE_EMAIL)
--- a/app/models.py
+++ b/app/models.py
@@ -13,7 +13,10 @@ from app.encryption import (
    check_hash
 )

-from app import db
+from app import (
+    db,
+    encryption
+)

 from app.history_meta import Versioned

@@ -347,6 +350,18 @@ class Notification(db.Model):
    status = db.Column(
        db.Enum(*NOTIFICATION_STATUS_TYPES, name='notify_status_types'), nullable=False, default='sending')
    reference = db.Column(db.String, nullable=True, index=True)
+    _personalisation = db.Column(db.String, nullable=True)
+
+    @property
+    def personalisation(self):
+        if self._personalisation:
+            return encryption.decrypt(self._personalisation)
+        return None
+
+    @personalisation.setter
+    def personalisation(self, personalisation):
+        if personalisation:
+            self._personalisation = encryption.encrypt(personalisation)


 INVITED_USER_STATUS_TYPES = ['pending', 'accepted', 'cancelled']
--- a/app/notifications/rest.py
+++ b/app/notifications/rest.py
@@ -277,7 +277,7 @@ def send_notification(notification_type):

    service_stats = notifications_dao.dao_get_notification_statistics_for_service_and_day(
        service_id,
-        datetime.utcnow().strftime(DATE_FORMAT)
+        datetime.today().strftime(DATE_FORMAT)
    )

    if service_stats:
--- a/app/schemas.py
+++ b/app/schemas.py
@@ -10,7 +10,8 @@ from marshmallow import (
    validates,
    validates_schema,
    pre_load,
-    pre_dump
+    pre_dump,
+    post_dump
 )
 from marshmallow_sqlalchemy import field_for

@@ -110,6 +111,7 @@ class NotificationModelSchema(BaseSchema):
    class Meta:
        model = models.Notification
        strict = True
+        exclude = ("_personalisation",)


 class BaseTemplateSchema(BaseSchema):
@@ -246,12 +248,30 @@ class SmsAdminNotificationSchema(SmsNotificationSchema):

 class NotificationStatusSchema(BaseSchema):

-    template = fields.Nested(TemplateSchema, only=["id", "name", "template_type"], dump_only=True)
+    template = fields.Nested(TemplateSchema, only=["id", "name", "template_type", "content"], dump_only=True)
    job = fields.Nested(JobSchema, only=["id", "original_file_name"], dump_only=True)
+    personalisation = fields.Dict(required=False)

    class Meta:
        model = models.Notification
        strict = True
+        exclude = ('_personalisation',)
+
+    @pre_dump
+    def handle_personalisation_property(self, in_data):
+        if in_data.personalisation:
+            self.personalisation = in_data.personalisation
+        return in_data
+
+    @post_dump
+    def handle_template_merge(self, in_data):
+        if in_data.get('personalisation'):
+            from notifications_utils.template import Template
+            merged = Template(in_data['template'], in_data['personalisation']).replaced
+            in_data['body'] = merged
+            in_data.pop('personalisation', None)
+        in_data['template'].pop('content', None)
+        return in_data


 class InvitedUserSchema(BaseSchema):
--- a/migrations/versions/0031_store_personalisation.py
+++ b/migrations/versions/0031_store_personalisation.py
@@ -0,0 +1,26 @@
+"""empty message
+
+Revision ID: 0031_store_personalisation
+Revises: 0030_service_id_not_null
+Create Date: 2016-06-20 10:39:50.892847
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '0031_store_personalisation'
+down_revision = '0030_service_id_not_null'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    ### commands auto generated by Alembic - please adjust! ###
+    op.add_column('notifications', sa.Column('_personalisation', sa.String(), nullable=True))
+    ### end Alembic commands ###
+
+
+def downgrade():
+    ### commands auto generated by Alembic - please adjust! ###
+    op.drop_column('notifications', '_personalisation')
+    ### end Alembic commands ###
--- a/tests/app/celery/test_tasks.py
+++ b/tests/app/celery/test_tasks.py
@@ -45,6 +45,21 @@ class AnyStringWith(str):
 mmg_error = {'Error': '40', 'Description': 'error'}


+def _notification_json(template, to, personalisation=None, job_id=None, row_number=None):
+    notification = {
+        "template": str(template.id),
+        "template_version": template.version,
+        "to": to,
+    }
+    if personalisation:
+        notification.update({"personalisation": personalisation})
+    if job_id:
+        notification.update({"job": str(job_id)})
+    if row_number:
+        notification['row_number'] = row_number
+    return notification
+
+
 # TODO moved to test_provider_tasks once send-email migrated
 def test_should_return_highest_priority_active_provider(notify_db, notify_db_session):
    providers = provider_details_dao.get_provider_details_by_notification_type('sms')
@@ -894,21 +909,6 @@ def test_should_call_send_not_update_provider_email_stats_if_research_mode(
        providers=[ses_provider.identifier]).first()


-def _notification_json(template, to, personalisation=None, job_id=None, row_number=None):
-    notification = {
-        "template": template.id,
-        "template_version": template.version,
-        "to": to,
-    }
-    if personalisation:
-        notification.update({"personalisation": personalisation})
-    if job_id:
-        notification.update({"job": job_id})
-    if row_number:
-        notification['row_number'] = row_number
-    return notification
-
-
 def test_update_status_of_notifications_after_timeout(notify_api,
                                                      notify_db,
                                                      notify_db_session,
@@ -945,3 +945,69 @@ def test_not_update_status_of_notification_before_timeout(notify_api,
                seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') - 10))
        timeout_notifications()
        assert not1.status == 'sending'
+
+
+def test_email_template_personalisation_persisted(sample_email_template_with_placeholders, mocker):
+    encrypted_notification = encryption.encrypt(_notification_json(
+        sample_email_template_with_placeholders,
+        "my_email@my_email.com",
+        {"name": "Jo"},
+        row_number=1))
+    mocker.patch('app.statsd_client.incr')
+    mocker.patch('app.statsd_client.timing_with_dates')
+    mocker.patch('app.statsd_client.timing')
+    mocker.patch('app.aws_ses_client.get_name', return_value='ses')
+    mocker.patch('app.aws_ses_client.send_email', return_value='ses')
+
+    notification_id = uuid.uuid4()
+
+    send_email(
+        sample_email_template_with_placeholders.service_id,
+        notification_id,
+        encrypted_notification,
+        datetime.utcnow().strftime(DATETIME_FORMAT)
+    )
+
+    persisted_notification = notifications_dao.get_notification(
+        sample_email_template_with_placeholders.service_id, notification_id
+    )
+
+    assert persisted_notification.id == notification_id
+    assert persisted_notification.personalisation == {"name": "Jo"}
+    # personalisation data is encrypted in db
+    assert persisted_notification._personalisation == encryption.encrypt({"name": "Jo"})
+
+
+def test_sms_template_personalisation_persisted(sample_template_with_placeholders, mocker):
+
+    encrypted_notification = encryption.encrypt(_notification_json(sample_template_with_placeholders,
+                                                to="+447234123123", personalisation={"name": "Jo"}))
+    mocker.patch('app.statsd_client.incr')
+    mocker.patch('app.statsd_client.timing_with_dates')
+    mocker.patch('app.statsd_client.timing')
+    mocker.patch('app.celery.provider_tasks.send_sms_to_provider.apply_async')
+
+    notification_id = uuid.uuid4()
+
+    send_sms(
+        sample_template_with_placeholders.service_id,
+        notification_id,
+        encrypted_notification,
+        datetime.utcnow().strftime(DATETIME_FORMAT)
+    )
+
+    provider_tasks.send_sms_to_provider.apply_async.assert_called_once_with(
+        (sample_template_with_placeholders.service.id,
+         notification_id,
+         encrypted_notification),
+        queue="sms"
+    )
+    persisted_notification = notifications_dao.get_notification(
+        sample_template_with_placeholders.service_id, notification_id
+    )
+
+    assert persisted_notification.id == notification_id
+    assert persisted_notification.to == '+447234123123'
+    assert persisted_notification.personalisation == {"name": "Jo"}
+    # personalisation data is encrypted in db
+    assert persisted_notification._personalisation == encryption.encrypt({"name": "Jo"})
--- a/tests/app/conftest.py
+++ b/tests/app/conftest.py
@@ -320,7 +320,8 @@ def sample_notification(notify_db,
                        reference=None,
                        created_at=datetime.utcnow(),
                        content_char_count=160,
-                        create=True):
+                        create=True,
+                        personalisation=None):
    if service is None:
        service = sample_service(notify_db, notify_db_session)
    if template is None:
@@ -347,7 +348,8 @@ def sample_notification(notify_db,
        'status': status,
        'reference': reference,
        'created_at': created_at,
-        'content_char_count': content_char_count
+        'content_char_count': content_char_count,
+        'personalisation': personalisation
    }
    if job_row_number:
        data['job_row_number'] = job_row_number
--- a/tests/app/notifications/test_rest.py
+++ b/tests/app/notifications/test_rest.py
@@ -72,6 +72,7 @@ def test_get_all_notifications(notify_api, sample_notification):
                'id': str(sample_notification.job.id),
                'original_file_name': sample_notification.job.original_file_name
            }
+
            assert notifications['notifications'][0]['to'] == '+447700900855'
            assert notifications['notifications'][0]['service'] == str(sample_notification.service_id)
            assert response.status_code == 200
@@ -1227,6 +1228,61 @@ def test_firetext_callback_should_record_statsd(notify_api, notify_db, notify_db
            app.statsd_client.incr.assert_any_call("notifications.callback.firetext.delivered")


+def test_get_notification_by_id_returns_merged_template_content(notify_db,
+                                                                notify_db_session,
+                                                                notify_api,
+                                                                sample_template_with_placeholders):
+
+    sample_notification = create_sample_notification(notify_db,
+                                                     notify_db_session,
+                                                     template=sample_template_with_placeholders,
+                                                     personalisation={"name": "world"})
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            auth_header = create_authorization_header(service_id=sample_notification.service_id)
+
+            response = client.get(
+                '/notifications/{}'.format(sample_notification.id),
+                headers=[auth_header])
+
+            notification = json.loads(response.get_data(as_text=True))['data']['notification']
+            assert response.status_code == 200
+            assert notification['body'] == 'Hello world'
+
+
+def test_get_notifications_for_service_returns_merged_template_content(notify_api,
+                                                                       notify_db,
+                                                                       notify_db_session,
+                                                                       sample_template_with_placeholders):
+
+    create_sample_notification(notify_db,
+                               notify_db_session,
+                               service=sample_template_with_placeholders.service,
+                               template=sample_template_with_placeholders,
+                               personalisation={"name": "merged with first"})
+
+    create_sample_notification(notify_db,
+                               notify_db_session,
+                               service=sample_template_with_placeholders.service,
+                               template=sample_template_with_placeholders,
+                               personalisation={"name": "merged with second"})
+
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+
+            auth_header = create_authorization_header()
+
+            response = client.get(
+                path='/service/{}/notifications'.format(sample_template_with_placeholders.service.id),
+                headers=[auth_header])
+            assert response.status_code == 200
+
+            resp = json.loads(response.get_data(as_text=True))
+            assert len(resp['notifications']) == 2
+            assert resp['notifications'][0]['body'] == 'Hello merged with first'
+            assert resp['notifications'][1]['body'] == 'Hello merged with second'
+
+
 def ses_validation_code_callback():
    return b'{\n  "Type" : "Notification",\n  "MessageId" : "ref",\n  "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",\n  "Message" : "{\\"notificationType\\":\\"Delivery\\",\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",\\"source\\":\\"valid-code@test.com\\",\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",\\"sendingAccountId\\":\\"123456789012\\",\\"messageId\\":\\"ref\\",\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",\\"processingTimeMillis\\":658,\\"recipients\\":[\\"testing@digital.cabinet-office.gov.u\\"],\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",\n  "Timestamp" : "2016-03-14T12:35:26.665Z",\n  "SignatureVersion" : "1",\n  "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUtOowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYLVSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMAPmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",\n  "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750dd426d95ee9390147a5624348ee.pem",\n  "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&SubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}'  # noqa