From 46c0728b12dd65a0617712d317f09c11e723d809 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 9 Aug 2016 13:07:48 +0100 Subject: [PATCH] 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 | 12 +++++++++--- app/models.py | 11 ++++++++++- app/notifications/rest.py | 11 +++++++---- tests/app/celery/test_provider_tasks.py | 6 +++--- tests/app/dao/test_notification_dao.py | 10 ++++++---- tests/app/notifications/test_rest.py | 15 ++++++++------- 6 files changed, 43 insertions(+), 22 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index d25adac3a..b70d1fad7 100644 --- a/app/dao/notifications_dao.py +++ b/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 diff --git a/app/models.py b/app/models.py index cc68e1843..3d9e12cfa 100644 --- a/app/models.py +++ b/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: diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 5461c198e..b54831105 100644 --- a/app/notifications/rest.py +++ b/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/', methods=['GET']) -def get_notifications(notification_id): - notification = notifications_dao.get_notification(str(api_user.service_id), - notification_id, - key_type=api_user.key_type) +def get_notification_by_id(notification_id): + notification = notifications_dao.get_notification_with_personalisation(str(api_user.service_id), + notification_id, + 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, diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 648c4a33d..9ef5681b4 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/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 diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 40c5228a2..cbed327cb 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/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 diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 49813da1b..c5774c6a9 100644 --- a/tests/app/notifications/test_rest.py +++ b/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 len(resp['notifications']) == 2 - assert resp['notifications'][0]['template_version'] == 1 - assert resp['notifications'][0]['body'] == 'This is a template' - assert resp['notifications'][1]['template_version'] == 2 - assert resp['notifications'][1]['body'] == 'foo' + assert response.status_code == 200 + + resp = json.loads(response.get_data(as_text=True)) + assert len(resp['notifications']) == 2 + assert resp['notifications'][0]['template_version'] == 1 + 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):