add actual_template relationship to notification

also renamed the function to make it apparent that it'll join and grab personalisation

app/dao/notifications_dao.py

@@ -8,6 +8,7 @@ from datetime import (
 from flask import current_app
 from werkzeug.datastructures import MultiDict
 from sqlalchemy import (desc, func, Integer, or_, and_, asc)
+from sqlalchemy.orm import joinedload
 from sqlalchemy.sql.expression import cast
 from notifications_utils.template import get_sms_fragment_count
 
@@ -351,12 +352,12 @@ def get_notifications_for_job(service_id, job_id, filter_dict=None, page=1, page
 
 
 @statsd(namespace="dao")
-def get_notification(service_id, notification_id, key_type=None):
+def get_notification_with_personalisation(service_id, notification_id, key_type):
     filter_dict = {'service_id': service_id, 'id': notification_id}
     if key_type:
         filter_dict['key_type'] = key_type
 
-    return Notification.query.filter_by(**filter_dict).one()
+    return Notification.query.filter_by(**filter_dict).options(joinedload('actual_template')).one()
 
 
 @statsd(namespace="dao")
@@ -374,7 +375,8 @@ def get_notifications_for_service(service_id,
                                   page=1,
                                   page_size=None,
                                   limit_days=None,
-                                  key_type=None):
+                                  key_type=None,
+                                  personalisation=False):
     if page_size is None:
         page_size = current_app.config['PAGE_SIZE']
     filters = [Notification.service_id == service_id]
@@ -388,6 +390,10 @@ def get_notifications_for_service(service_id,
 
     query = Notification.query.filter(*filters)
     query = _filter_query(query, filter_dict)
+    if personalisation:
+        query.options(
+            joinedload('actual_template')
+        )
     return query.order_by(desc(Notification.created_at)).paginate(
         page=page,
         per_page=page_size

app/models.py

@@ -5,7 +5,8 @@ from sqlalchemy.dialects.postgresql import (
     UUID,
     JSON
 )
-from sqlalchemy import UniqueConstraint, text
+from sqlalchemy import UniqueConstraint, text, ForeignKeyConstraint, and_
+from sqlalchemy.orm import foreign, remote
 
 from app.encryption import (
     hashpw,
@@ -447,6 +448,14 @@ class Notification(db.Model):
     reference = db.Column(db.String, nullable=True, index=True)
     _personalisation = db.Column(db.String, nullable=True)
 
+    # __table_args__ = (
+    #     ForeignKeyConstraint(['template_id', 'template_version'], ['template_history.id', 'template_history.version']),
+    # )
+    actual_template = db.relationship('TemplateHistory', primaryjoin=and_(
+        foreign(template_id) == remote(TemplateHistory.id),
+        foreign(template_version) == remote(TemplateHistory.version)
+    ))
+
     @property
     def personalisation(self):
         if self._personalisation:

app/notifications/rest.py

@@ -1,5 +1,6 @@
 from datetime import datetime
 import itertools
+
 from flask import (
     Blueprint,
     jsonify,
@@ -7,6 +8,7 @@ from flask import (
     current_app,
     json
 )
+
 from notifications_utils.recipients import allowed_to_send_to, first_column_heading
 from notifications_utils.template import Template
 from notifications_utils.renderers import PassThrough
@@ -170,10 +172,10 @@ def process_firetext_response():
 
 
 @notifications.route('/notifications/<uuid:notification_id>', methods=['GET'])
-def get_notifications(notification_id):
+def get_notification_by_id(notification_id):
-    notification = notifications_dao.get_notification(str(api_user.service_id),
+    notification = notifications_dao.get_notification_with_personalisation(str(api_user.service_id),
-                                                      notification_id,
+                                                                           notification_id,
-                                                      key_type=api_user.key_type)
+                                                                           key_type=api_user.key_type)
     return jsonify(data={"notification": notification_with_personalisation_schema.dump(notification).data}), 200
 
 
@@ -186,6 +188,7 @@ def get_all_notifications():
 
     pagination = notifications_dao.get_notifications_for_service(
         str(api_user.service_id),
+        personalisation=True,
         filter_dict=data,
         page=page,
         page_size=page_size,

tests/app/celery/test_provider_tasks.py

@@ -188,7 +188,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest(
         sender=None
     )
 
-    persisted_notification = notifications_dao.get_notification(sample_template.service_id, db_notification.id)
+    persisted_notification = notifications_dao.get_notification_by_id(db_notification.id)
     assert persisted_notification.to == db_notification.to
     assert persisted_notification.template_id == sample_template.id
     assert persisted_notification.template_version == version_on_notification
@@ -223,7 +223,7 @@ def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_s
         ('mmg', str(sample_notification.id), sample_notification.to), queue='research-mode'
     )
 
-    persisted_notification = notifications_dao.get_notification(sample_service.id, sample_notification.id)
+    persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id)
     assert persisted_notification.to == sample_notification.to
     assert persisted_notification.template_id == sample_notification.template_id
     assert persisted_notification.status == 'sending'
@@ -499,7 +499,7 @@ def test_should_not_set_billable_units_if_research_mode(notify_db, sample_servic
         sample_notification.id
     )
 
-    persisted_notification = notifications_dao.get_notification(sample_service.id, sample_notification.id)
+    persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id)
     assert persisted_notification.billable_units == 0
 
 

tests/app/dao/test_notification_dao.py

@@ -23,7 +23,7 @@ from app.models import (
 from app.dao.notifications_dao import (
     dao_create_notification,
     dao_update_notification,
-    get_notification,
+    get_notification_with_personalisation,
     get_notification_for_job,
     get_notifications_for_job,
     dao_get_notification_statistics_for_service,
@@ -57,7 +57,7 @@ def test_should_have_decorated_notifications_dao_functions():
     assert update_notification_status_by_reference.__wrapped__.__name__ == 'update_notification_status_by_reference'  # noqa
     assert get_notification_for_job.__wrapped__.__name__ == 'get_notification_for_job'  # noqa
     assert get_notifications_for_job.__wrapped__.__name__ == 'get_notifications_for_job'  # noqa
-    assert get_notification.__wrapped__.__name__ == 'get_notification'  # noqa
+    assert get_notification_with_personalisation.__wrapped__.__name__ == 'get_notification_with_personalisation'  # noqa
     assert get_notifications_for_service.__wrapped__.__name__ == 'get_notifications_for_service'  # noqa
     assert get_notification_by_id.__wrapped__.__name__ == 'get_notification_by_id'  # noqa
     assert delete_notifications_created_more_than_a_week_ago.__wrapped__.__name__ == 'delete_notifications_created_more_than_a_week_ago'  # noqa
@@ -637,9 +637,11 @@ def test_save_notification_with_no_job(sample_template, mmg_provider):
 
 
 def test_get_notification(sample_notification):
-    notification_from_db = get_notification(
+    notification_from_db = get_notification_with_personalisation(
         sample_notification.service.id,
-        sample_notification.id)
+        sample_notification.id,
+        key_type=None
+    )
     assert sample_notification == notification_from_db
 
 

tests/app/notifications/test_rest.py

@@ -640,14 +640,15 @@ def test_get_notification_selects_correct_template_for_personalisation(notify_ap
         auth_header = create_authorization_header(service_id=sample_template.service_id)
 
         response = client.get(path='/notifications', headers=[auth_header])
-        assert response.status_code == 200
 
-        resp = json.loads(response.get_data(as_text=True))
+    assert response.status_code == 200
-        assert len(resp['notifications']) == 2
+
-        assert resp['notifications'][0]['template_version'] == 1
+    resp = json.loads(response.get_data(as_text=True))
-        assert resp['notifications'][0]['body'] == 'This is a template'
+    assert len(resp['notifications']) == 2
-        assert resp['notifications'][1]['template_version'] == 2
+    assert resp['notifications'][0]['template_version'] == 1
-        assert resp['notifications'][1]['body'] == 'foo'
+    assert resp['notifications'][0]['body'] == 'This is a template'
+    assert resp['notifications'][1]['template_version'] == 2
+    assert resp['notifications'][1]['body'] == 'foo'
 
 
 def _create_auth_header_from_key(api_key):