From ee9b6f1fe05ece770301a5402229afe8dead7e7e Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 2 Mar 2018 13:43:27 +0000 Subject: [PATCH 1/8] Return hidden field as part of json for notification by id - required by Admin app so that it can distinguish between created and precompiled letters --- app/schemas.py | 2 +- tests/app/db.py | 14 ++++++------- tests/app/service/test_rest.py | 38 ++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 5592026b1..b36de4871 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -454,7 +454,7 @@ class NotificationWithTemplateSchema(BaseSchema): template = fields.Nested( TemplateSchema, - only=['id', 'version', 'name', 'template_type', 'content', 'subject', 'redact_personalisation'], + only=['id', 'version', 'name', 'template_type', 'content', 'subject', 'redact_personalisation', 'hidden'], dump_only=True ) job = fields.Nested(JobSchema, only=["id", "original_file_name"], dump_only=True) diff --git a/tests/app/db.py b/tests/app/db.py index 6a09413a8..20e1c7997 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -125,13 +125,13 @@ def create_service_with_defined_sms_sender( def create_template( - service, - template_type=SMS_TYPE, - template_name=None, - subject='Template subject', - content='Dear Sir/Madam, Hello. Yours Truly, The Government.', - reply_to=None, - hidden=False + service, + template_type=SMS_TYPE, + template_name=None, + subject='Template subject', + content='Dear Sir/Madam, Hello. Yours Truly, The Government.', + reply_to=None, + hidden=False ): data = { 'name': template_name or '{} Template Name'.format(template_type), diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 7998b6b91..92a74c233 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2139,6 +2139,17 @@ def test_get_notification_for_service_includes_template_redacted(admin_request, assert resp['template']['redact_personalisation'] is False +def test_get_notification_for_service_includes_template_hidden(admin_request, sample_notification): + resp = admin_request.get( + 'service.get_notification_for_service', + service_id=sample_notification.service_id, + notification_id=sample_notification.id + ) + + assert resp['id'] == str(sample_notification.id) + assert resp['template']['hidden'] is False + + def test_get_all_notifications_for_service_includes_template_redacted(admin_request, sample_service): normal_template = create_template(sample_service) @@ -2162,6 +2173,33 @@ def test_get_all_notifications_for_service_includes_template_redacted(admin_requ assert resp['notifications'][1]['template']['redact_personalisation'] is True +def test_get_all_notifications_for_service_includes_template_hidden(admin_request, sample_service): + letter_template = create_template(sample_service, template_type=LETTER_TYPE) + precompiled_template = create_template( + sample_service, + template_type=LETTER_TYPE, + template_name='Pre-compiled PDF', + subject='Pre-compiled PDF', + hidden=True + ) + + with freeze_time('2000-01-01'): + letter_noti = create_notification(letter_template) + with freeze_time('2000-01-02'): + precompiled_noti = create_notification(precompiled_template) + + resp = admin_request.get( + 'service.get_all_notifications_for_service', + service_id=sample_service.id + ) + + assert resp['notifications'][0]['id'] == str(precompiled_noti.id) + assert resp['notifications'][0]['template']['hidden'] is True + + assert resp['notifications'][1]['id'] == str(letter_noti.id) + assert resp['notifications'][1]['template']['hidden'] is False + + def test_search_for_notification_by_to_field_returns_personlisation( client, notify_db, From 564504bf97a785589f1ffe7ba2551f890f76f68c Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 5 Mar 2018 13:38:41 +0000 Subject: [PATCH 2/8] Add template hidden field in response --- app/dao/notifications_dao.py | 1 + app/dao/services_dao.py | 4 ++++ app/dao/stats_template_usage_by_month_dao.py | 1 + app/dao/templates_dao.py | 1 + app/service/rest.py | 3 ++- app/template_statistics/rest.py | 3 ++- tests/app/service/test_rest.py | 5 ++++- 7 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 155616801..efd13426e 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -83,6 +83,7 @@ def dao_get_template_usage(service_id, limit_days=None): Template.id.label('template_id'), Template.name, Template.template_type, + Template.hidden, notifications_aggregate_query.c.count ).join( notifications_aggregate_query, diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 57d5aa866..6ad83d214 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -522,6 +522,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) stat.month = result.month stat.year = result.year stat.count = result.count + stat.hidden = result.hidden stats.append(stat) month = get_london_month_from_utc_column(Notification.created_at) @@ -533,6 +534,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) if fy_start < datetime.now() < fy_end: today_results = db.session.query( Notification.template_id, + Template.hidden, Template.name, Template.template_type, extract('month', month).label('month'), @@ -547,6 +549,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) Notification.key_type != KEY_TYPE_TEST ).group_by( Notification.template_id, + Template.hidden, Template.name, Template.template_type, month, @@ -571,6 +574,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) new_stat.month = int(today_result.month) new_stat.year = int(today_result.year) new_stat.count = today_result.count + new_stat.hidden = today_result.hidden stats.append(new_stat) return stats diff --git a/app/dao/stats_template_usage_by_month_dao.py b/app/dao/stats_template_usage_by_month_dao.py index 29aba1cdc..49e95cfe1 100644 --- a/app/dao/stats_template_usage_by_month_dao.py +++ b/app/dao/stats_template_usage_by_month_dao.py @@ -35,6 +35,7 @@ def insert_or_update_stats_for_template(template_id, month, year, count): def dao_get_template_usage_stats_by_service(service_id, year): return db.session.query( StatsTemplateUsageByMonth.template_id, + Template.hidden, Template.name, Template.template_type, StatsTemplateUsageByMonth.month, diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index fb7dc7602..c664c4346 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -135,6 +135,7 @@ def dao_get_templates_for_cache(cache): query = db.session.query(Template.id.label('template_id'), Template.template_type, Template.name, + Template.hidden, cache_subq.c.count.label('count') ).join(cache_subq, Template.id == cache_subq.c.template_id diff --git a/app/service/rest.py b/app/service/rest.py index 99f03cee9..32305e301 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -536,7 +536,8 @@ def get_monthly_template_usage(service_id): 'type': i.template_type, 'month': i.month, 'year': i.year, - 'count': i.count + 'count': i.count, + 'hidden': i.hidden } ) diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index 86548dc00..f71a40492 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -47,7 +47,8 @@ def get_template_statistics_for_service_by_day(service_id): 'count': data.count, 'template_id': str(data.template_id), 'template_name': data.name, - 'template_type': data.template_type + 'template_type': data.template_type, + 'template_hidden': data.hidden } return jsonify(data=[serialize(row) for row in stats]) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 92a74c233..86f7641de 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1831,7 +1831,7 @@ def test_get_template_usage_by_month_returns_two_templates( sample_service ): - template_one = create_template(sample_service) + template_one = create_template(sample_service, hidden=True) # add a historical notification for template not1 = create_notification_history( @@ -1889,6 +1889,7 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[0]["month"] == 4 assert resp_json[0]["year"] == 2017 assert resp_json[0]["count"] == 1 + assert resp_json[0]["hidden"] is True assert resp_json[1]["template_id"] == str(sample_template.id) assert resp_json[1]["name"] == sample_template.name @@ -1896,6 +1897,7 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[1]["month"] == 4 assert resp_json[1]["year"] == 2017 assert resp_json[1]["count"] == 3 + assert resp_json[1]["hidden"] is False assert resp_json[2]["template_id"] == str(sample_template.id) assert resp_json[2]["name"] == sample_template.name @@ -1903,6 +1905,7 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[2]["month"] == 11 assert resp_json[2]["year"] == 2017 assert resp_json[2]["count"] == 1 + assert resp_json[2]["hidden"] is False def test_search_for_notification_by_to_field(client, notify_db, notify_db_session): From 5dd3c62b5c2bf1ff034acf7f9b991a67888aa2aa Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 5 Mar 2018 16:56:14 +0000 Subject: [PATCH 3/8] Fix template caching tests --- tests/app/dao/test_templates_dao.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index 8891d2e26..e9daa86ae 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -489,7 +489,8 @@ def test_get_templates_by_ids_successful(notify_db, notify_db_session): notify_db_session, template_name='Sample Template 2', template_type="sms", - content="Template content" + content="Template content", + hidden=True ) create_sample_template( notify_db, @@ -503,8 +504,8 @@ def test_get_templates_by_ids_successful(notify_db, notify_db_session): cache = [[k, v] for k, v in sample_cache_dict.items()] templates = dao_get_templates_for_cache(cache) assert len(templates) == 2 - assert [(template_1.id, template_1.template_type, template_1.name, 2), - (template_2.id, template_2.template_type, template_2.name, 3)] == templates + assert [(template_1.id, template_1.template_type, template_1.name, False, 2), + (template_2.id, template_2.template_type, template_2.name, True, 3)] == templates def test_get_templates_by_ids_successful_for_one_cache_item(notify_db, notify_db_session): @@ -519,7 +520,7 @@ def test_get_templates_by_ids_successful_for_one_cache_item(notify_db, notify_db cache = [[k, v] for k, v in sample_cache_dict.items()] templates = dao_get_templates_for_cache(cache) assert len(templates) == 1 - assert [(template_1.id, template_1.template_type, template_1.name, 2)] == templates + assert [(template_1.id, template_1.template_type, template_1.name, False, 2)] == templates def test_get_templates_by_ids_returns_empty_list(): From b5a2dbb24e8c18d1aad641e4c3469ae90d706f4c Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 5 Mar 2018 18:47:45 +0000 Subject: [PATCH 4/8] Update schema to dump precompiled_letter - to be a precompiled letter, the template must be a letter, be hidden, and have a matching template name to the one expected in config['PRECOMPILED_TEMPLATE_NAME'] --- app/schemas.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/app/schemas.py b/app/schemas.py index b36de4871..9b4ae055c 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -2,6 +2,7 @@ from datetime import ( datetime, date, timedelta) +from flask import current_app from flask_marshmallow.fields import fields from marshmallow import ( post_load, @@ -305,6 +306,7 @@ class NotificationModelSchema(BaseSchema): class BaseTemplateSchema(BaseSchema): reply_to = fields.Method("get_reply_to", allow_none=True) reply_to_text = fields.Method("get_reply_to_text", allow_none=True) + precompiled_letter = fields.Method("get_precompiled_letter") def get_reply_to(self, template): return template.reply_to @@ -312,6 +314,13 @@ class BaseTemplateSchema(BaseSchema): def get_reply_to_text(self, template): return template.get_reply_to_text() + def get_precompiled_letter(self, template): + return ( + template.template_type == 'letter' and + template.hidden and + template.name == current_app.config['PRECOMPILED_TEMPLATE_NAME'] + ) + class Meta: model = models.Template exclude = ("service_id", "jobs", "service_letter_contact_id") @@ -454,7 +463,16 @@ class NotificationWithTemplateSchema(BaseSchema): template = fields.Nested( TemplateSchema, - only=['id', 'version', 'name', 'template_type', 'content', 'subject', 'redact_personalisation', 'hidden'], + only=[ + 'id', + 'version', + 'name', + 'template_type', + 'content', + 'subject', + 'redact_personalisation', + 'precompiled_letter' + ], dump_only=True ) job = fields.Nested(JobSchema, only=["id", "original_file_name"], dump_only=True) From bca858f4a87473b3d44eb134e4a1f86e2ecf912c Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 5 Mar 2018 18:51:04 +0000 Subject: [PATCH 5/8] Return precompiled_letter flag rather than hidden --- app/service/rest.py | 6 +++++- app/template_statistics/rest.py | 6 +++++- tests/app/service/test_rest.py | 21 +++++++++++++-------- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 32305e301..33c647131 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -537,7 +537,11 @@ def get_monthly_template_usage(service_id): 'month': i.month, 'year': i.year, 'count': i.count, - 'hidden': i.hidden + 'precompiled_letter': ( + i.template_type == 'letter' and + i.hidden and + i.name == current_app.config['PRECOMPILED_TEMPLATE_NAME'] + ) } ) diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index f71a40492..6dae8bec5 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -48,7 +48,11 @@ def get_template_statistics_for_service_by_day(service_id): 'template_id': str(data.template_id), 'template_name': data.name, 'template_type': data.template_type, - 'template_hidden': data.hidden + 'precompiled_letter': ( + data.template_type == 'letter' and + data.hidden and + data.name == current_app.config['PRECOMPILED_TEMPLATE_NAME'] + ) } return jsonify(data=[serialize(row) for row in stats]) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 86f7641de..b600b239b 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1831,7 +1831,12 @@ def test_get_template_usage_by_month_returns_two_templates( sample_service ): - template_one = create_template(sample_service, hidden=True) + template_one = create_template( + sample_service, + template_type=LETTER_TYPE, + template_name=current_app.config['PRECOMPILED_TEMPLATE_NAME'], + hidden=True + ) # add a historical notification for template not1 = create_notification_history( @@ -1889,7 +1894,7 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[0]["month"] == 4 assert resp_json[0]["year"] == 2017 assert resp_json[0]["count"] == 1 - assert resp_json[0]["hidden"] is True + assert resp_json[0]["precompiled_letter"] is True assert resp_json[1]["template_id"] == str(sample_template.id) assert resp_json[1]["name"] == sample_template.name @@ -1897,7 +1902,7 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[1]["month"] == 4 assert resp_json[1]["year"] == 2017 assert resp_json[1]["count"] == 3 - assert resp_json[1]["hidden"] is False + assert resp_json[1]["precompiled_letter"] is False assert resp_json[2]["template_id"] == str(sample_template.id) assert resp_json[2]["name"] == sample_template.name @@ -1905,7 +1910,7 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[2]["month"] == 11 assert resp_json[2]["year"] == 2017 assert resp_json[2]["count"] == 1 - assert resp_json[2]["hidden"] is False + assert resp_json[2]["precompiled_letter"] is False def test_search_for_notification_by_to_field(client, notify_db, notify_db_session): @@ -2142,7 +2147,7 @@ def test_get_notification_for_service_includes_template_redacted(admin_request, assert resp['template']['redact_personalisation'] is False -def test_get_notification_for_service_includes_template_hidden(admin_request, sample_notification): +def test_get_notification_for_service_includes_precompiled_letter(admin_request, sample_notification): resp = admin_request.get( 'service.get_notification_for_service', service_id=sample_notification.service_id, @@ -2150,7 +2155,7 @@ def test_get_notification_for_service_includes_template_hidden(admin_request, sa ) assert resp['id'] == str(sample_notification.id) - assert resp['template']['hidden'] is False + assert resp['template']['precompiled_letter'] is False def test_get_all_notifications_for_service_includes_template_redacted(admin_request, sample_service): @@ -2197,10 +2202,10 @@ def test_get_all_notifications_for_service_includes_template_hidden(admin_reques ) assert resp['notifications'][0]['id'] == str(precompiled_noti.id) - assert resp['notifications'][0]['template']['hidden'] is True + assert resp['notifications'][0]['template']['precompiled_letter'] is True assert resp['notifications'][1]['id'] == str(letter_noti.id) - assert resp['notifications'][1]['template']['hidden'] is False + assert resp['notifications'][1]['template']['precompiled_letter'] is False def test_search_for_notification_by_to_field_returns_personlisation( From 28136734e4cb9a58d5587a9b8df844cadf96be62 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 6 Mar 2018 13:04:57 +0000 Subject: [PATCH 6/8] Refactor to use is_precompiled_letter in letters/utils.py --- app/letters/utils.py | 7 ++++++- app/schemas.py | 8 ++------ app/template_statistics/rest.py | 7 ++----- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/app/letters/utils.py b/app/letters/utils.py index 5e7730882..a5f67c11b 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -5,6 +5,7 @@ from flask import current_app from notifications_utils.s3 import s3upload +from app.models import LETTER_TYPE from app.variables import Retention @@ -79,4 +80,8 @@ def get_letter_pdf(notification): def is_precompiled_letter(template): - return template.hidden and template.name == current_app.config['PRECOMPILED_TEMPLATE_NAME'] + return ( + template.template_type == LETTER_TYPE and + template.hidden and + template.name == current_app.config['PRECOMPILED_TEMPLATE_NAME'] + ) diff --git a/app/schemas.py b/app/schemas.py index 9b4ae055c..f2282ac05 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -2,7 +2,6 @@ from datetime import ( datetime, date, timedelta) -from flask import current_app from flask_marshmallow.fields import fields from marshmallow import ( post_load, @@ -25,6 +24,7 @@ from notifications_utils.recipients import ( from app import ma from app import models +from app.letters.utils import is_precompiled_letter from app.models import ServicePermission from app.dao.permissions_dao import permission_dao from app.utils import get_template_instance @@ -315,11 +315,7 @@ class BaseTemplateSchema(BaseSchema): return template.get_reply_to_text() def get_precompiled_letter(self, template): - return ( - template.template_type == 'letter' and - template.hidden and - template.name == current_app.config['PRECOMPILED_TEMPLATE_NAME'] - ) + return is_precompiled_letter(template) class Meta: model = models.Template diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index 6dae8bec5..f00741a7b 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -14,6 +14,7 @@ from app.dao.templates_dao import ( dao_get_template_by_id_and_service_id ) +from app.letters.utils import is_precompiled_letter from app.schemas import notification_with_template_schema from app.utils import cache_key_for_service_template_counter from app.errors import register_errors, InvalidRequest @@ -48,11 +49,7 @@ def get_template_statistics_for_service_by_day(service_id): 'template_id': str(data.template_id), 'template_name': data.name, 'template_type': data.template_type, - 'precompiled_letter': ( - data.template_type == 'letter' and - data.hidden and - data.name == current_app.config['PRECOMPILED_TEMPLATE_NAME'] - ) + 'precompiled_letter': is_precompiled_letter(data) } return jsonify(data=[serialize(row) for row in stats]) From 7011b90bd4efdf33f85e8bfe1b9024e7747db9af Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Wed, 7 Mar 2018 15:42:59 +0000 Subject: [PATCH 7/8] Refactor is_precompiled_letter to model --- app/config.py | 1 - app/dao/notifications_dao.py | 2 +- app/dao/services_dao.py | 6 +-- app/dao/stats_template_usage_by_month_dao.py | 2 +- app/dao/templates_dao.py | 8 +++- app/letters/utils.py | 9 ----- app/models.py | 12 ++++++ app/schemas.py | 3 +- app/service/rest.py | 6 +-- app/template/rest.py | 4 +- app/template_statistics/rest.py | 3 +- .../test_stats_template_usage_by_month_dao.py | 32 +++++++++++++++- tests/app/dao/test_templates_dao.py | 37 +++++++++++++++++-- tests/app/letters/test_letter_utils.py | 23 +----------- tests/app/service/test_rest.py | 17 +++++---- tests/app/test_model.py | 23 +++++++++++- 16 files changed, 126 insertions(+), 62 deletions(-) diff --git a/app/config.py b/app/config.py index 951f5ece3..f9b125be1 100644 --- a/app/config.py +++ b/app/config.py @@ -149,7 +149,6 @@ class Config(object): CHANGE_EMAIL_CONFIRMATION_TEMPLATE_ID = 'eb4d9930-87ab-4aef-9bce-786762687884' SERVICE_NOW_LIVE_TEMPLATE_ID = '618185c6-3636-49cd-b7d2-6f6f5eb3bdde' ORGANISATION_INVITATION_EMAIL_TEMPLATE_ID = '203566f0-d835-47c5-aa06-932439c86573' - PRECOMPILED_TEMPLATE_NAME = 'Pre-compiled PDF' BROKER_URL = 'sqs://' BROKER_TRANSPORT_OPTIONS = { diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index efd13426e..7c25f8d21 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -83,7 +83,7 @@ def dao_get_template_usage(service_id, limit_days=None): Template.id.label('template_id'), Template.name, Template.template_type, - Template.hidden, + Template.is_precompiled_letter, notifications_aggregate_query.c.count ).join( notifications_aggregate_query, diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 6ad83d214..838a6336e 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -522,7 +522,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) stat.month = result.month stat.year = result.year stat.count = result.count - stat.hidden = result.hidden + stat.is_precompiled_letter = result.is_precompiled_letter stats.append(stat) month = get_london_month_from_utc_column(Notification.created_at) @@ -534,7 +534,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) if fy_start < datetime.now() < fy_end: today_results = db.session.query( Notification.template_id, - Template.hidden, + Template.is_precompiled_letter, Template.name, Template.template_type, extract('month', month).label('month'), @@ -574,7 +574,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) new_stat.month = int(today_result.month) new_stat.year = int(today_result.year) new_stat.count = today_result.count - new_stat.hidden = today_result.hidden + new_stat.is_precompiled_letter = today_result.is_precompiled_letter stats.append(new_stat) return stats diff --git a/app/dao/stats_template_usage_by_month_dao.py b/app/dao/stats_template_usage_by_month_dao.py index 49e95cfe1..541ab7193 100644 --- a/app/dao/stats_template_usage_by_month_dao.py +++ b/app/dao/stats_template_usage_by_month_dao.py @@ -35,9 +35,9 @@ def insert_or_update_stats_for_template(template_id, month, year, count): def dao_get_template_usage_stats_by_service(service_id, year): return db.session.query( StatsTemplateUsageByMonth.template_id, - Template.hidden, Template.name, Template.template_type, + Template.is_precompiled_letter, StatsTemplateUsageByMonth.month, StatsTemplateUsageByMonth.year, StatsTemplateUsageByMonth.count diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index c664c4346..9dd397ef0 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -5,7 +5,11 @@ from sqlalchemy import asc, desc from sqlalchemy.sql.expression import bindparam from app import db -from app.models import (Template, TemplateHistory, TemplateRedacted) +from app.models import ( + Template, + TemplateHistory, + TemplateRedacted +) from app.dao.dao_utils import ( transactional, version_class @@ -135,7 +139,7 @@ def dao_get_templates_for_cache(cache): query = db.session.query(Template.id.label('template_id'), Template.template_type, Template.name, - Template.hidden, + Template.is_precompiled_letter, cache_subq.c.count.label('count') ).join(cache_subq, Template.id == cache_subq.c.template_id diff --git a/app/letters/utils.py b/app/letters/utils.py index a5f67c11b..a7f05c00f 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -5,7 +5,6 @@ from flask import current_app from notifications_utils.s3 import s3upload -from app.models import LETTER_TYPE from app.variables import Retention @@ -77,11 +76,3 @@ def get_letter_pdf(notification): file_content = obj.get()["Body"].read() return file_content - - -def is_precompiled_letter(template): - return ( - template.template_type == LETTER_TYPE and - template.hidden and - template.name == current_app.config['PRECOMPILED_TEMPLATE_NAME'] - ) diff --git a/app/models.py b/app/models.py index 6678d91e6..4f54f04fc 100644 --- a/app/models.py +++ b/app/models.py @@ -6,6 +6,7 @@ from flask import url_for, current_app from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.ext.associationproxy import association_proxy +from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.dialects.postgresql import ( UUID, JSON @@ -641,6 +642,9 @@ class TemplateProcessTypes(db.Model): name = db.Column(db.String(255), primary_key=True) +PRECOMPILED_TEMPLATE_NAME = 'Pre-compiled PDF' + + class TemplateBase(db.Model): __abstract__ = True @@ -718,6 +722,14 @@ class TemplateBase(db.Model): else: return None + @hybrid_property + def is_precompiled_letter(self): + return self.hidden and self.name == PRECOMPILED_TEMPLATE_NAME and self.template_type == LETTER_TYPE + + @is_precompiled_letter.setter + def is_precompiled_letter(self, value): + pass + def _as_utils_template(self): if self.template_type == EMAIL_TYPE: return PlainTextEmailTemplate( diff --git a/app/schemas.py b/app/schemas.py index f2282ac05..c76c2c922 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -24,7 +24,6 @@ from notifications_utils.recipients import ( from app import ma from app import models -from app.letters.utils import is_precompiled_letter from app.models import ServicePermission from app.dao.permissions_dao import permission_dao from app.utils import get_template_instance @@ -315,7 +314,7 @@ class BaseTemplateSchema(BaseSchema): return template.get_reply_to_text() def get_precompiled_letter(self, template): - return is_precompiled_letter(template) + return template.is_precompiled_letter class Meta: model = models.Template diff --git a/app/service/rest.py b/app/service/rest.py index 33c647131..2ccf51635 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -68,7 +68,7 @@ from app.errors import ( InvalidRequest, register_errors ) -from app.models import Service, EmailBranding +from app.models import Service, EmailBranding, LETTER_TYPE, PRECOMPILED_TEMPLATE_NAME from app.schema_validation import validate from app.service import statistics from app.service.service_senders_schema import ( @@ -538,9 +538,9 @@ def get_monthly_template_usage(service_id): 'year': i.year, 'count': i.count, 'precompiled_letter': ( - i.template_type == 'letter' and + i.template_type == LETTER_TYPE and i.hidden and - i.name == current_app.config['PRECOMPILED_TEMPLATE_NAME'] + i.name == PRECOMPILED_TEMPLATE_NAME ) } ) diff --git a/app/template/rest.py b/app/template/rest.py index e49eea762..b068d268c 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -21,7 +21,7 @@ from app.dao.templates_dao import ( dao_get_template_by_id) from notifications_utils.template import SMSMessageTemplate from app.dao.services_dao import dao_fetch_service_by_id -from app.letters.utils import get_letter_pdf, is_precompiled_letter +from app.letters.utils import get_letter_pdf from app.models import SMS_TYPE from app.notifications.validators import service_has_permission, check_reply_to from app.schemas import (template_schema, template_history_schema) @@ -199,7 +199,7 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file template = dao_get_template_by_id(notification.template_id) - if is_precompiled_letter(template): + if template.is_precompiled_letter: try: diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index f00741a7b..2af5efdae 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -14,7 +14,6 @@ from app.dao.templates_dao import ( dao_get_template_by_id_and_service_id ) -from app.letters.utils import is_precompiled_letter from app.schemas import notification_with_template_schema from app.utils import cache_key_for_service_template_counter from app.errors import register_errors, InvalidRequest @@ -49,7 +48,7 @@ def get_template_statistics_for_service_by_day(service_id): 'template_id': str(data.template_id), 'template_name': data.name, 'template_type': data.template_type, - 'precompiled_letter': is_precompiled_letter(data) + 'precompiled_letter': data.is_precompiled_letter } return jsonify(data=[serialize(row) for row in stats]) diff --git a/tests/app/dao/test_stats_template_usage_by_month_dao.py b/tests/app/dao/test_stats_template_usage_by_month_dao.py index 7d21696b5..676e00952 100644 --- a/tests/app/dao/test_stats_template_usage_by_month_dao.py +++ b/tests/app/dao/test_stats_template_usage_by_month_dao.py @@ -3,7 +3,7 @@ from app.dao.stats_template_usage_by_month_dao import ( insert_or_update_stats_for_template, dao_get_template_usage_stats_by_service ) -from app.models import StatsTemplateUsageByMonth +from app.models import StatsTemplateUsageByMonth, LETTER_TYPE, PRECOMPILED_TEMPLATE_NAME from tests.app.db import create_service, create_template @@ -74,6 +74,36 @@ def test_dao_get_template_usage_stats_by_service(sample_service): assert len(result) == 1 +def test_dao_get_template_usage_stats_by_service_for_precompiled_letters(sample_service): + + letter_template = create_template(service=sample_service, template_type=LETTER_TYPE) + + precompiled_letter_template = create_template( + service=sample_service, template_name=PRECOMPILED_TEMPLATE_NAME, hidden=True, template_type=LETTER_TYPE) + + db.session.add(StatsTemplateUsageByMonth( + template_id=letter_template.id, + month=5, + year=2017, + count=10 + )) + + db.session.add(StatsTemplateUsageByMonth( + template_id=precompiled_letter_template.id, + month=4, + year=2017, + count=20 + )) + + result = dao_get_template_usage_stats_by_service(sample_service.id, 2017) + + assert len(result) == 2 + assert [ + (letter_template.id, 'letter Template Name', 'letter', False, 5, 2017, 10), + (precompiled_letter_template.id, PRECOMPILED_TEMPLATE_NAME, 'letter', True, 4, 2017, 20) + ] == result + + def test_dao_get_template_usage_stats_by_service_specific_year(sample_service): email_template = create_template(service=sample_service, template_type="email") diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index e9daa86ae..d4768f598 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -13,7 +13,12 @@ from app.dao.templates_dao import ( dao_get_templates_for_cache, dao_redact_template, dao_update_template_reply_to ) -from app.models import Template, TemplateHistory, TemplateRedacted +from app.models import ( + Template, + TemplateHistory, + TemplateRedacted, + PRECOMPILED_TEMPLATE_NAME +) from tests.app.conftest import sample_template as create_sample_template from tests.app.db import create_template, create_letter_contact @@ -489,8 +494,7 @@ def test_get_templates_by_ids_successful(notify_db, notify_db_session): notify_db_session, template_name='Sample Template 2', template_type="sms", - content="Template content", - hidden=True + content="Template content" ) create_sample_template( notify_db, @@ -505,7 +509,32 @@ def test_get_templates_by_ids_successful(notify_db, notify_db_session): templates = dao_get_templates_for_cache(cache) assert len(templates) == 2 assert [(template_1.id, template_1.template_type, template_1.name, False, 2), - (template_2.id, template_2.template_type, template_2.name, True, 3)] == templates + (template_2.id, template_2.template_type, template_2.name, False, 3)] == templates + + +def test_get_letter_templates_by_ids_successful(notify_db, notify_db_session): + template_1 = create_sample_template( + notify_db, + notify_db_session, + template_name=PRECOMPILED_TEMPLATE_NAME, + template_type="letter", + content="Template content", + hidden=True + ) + template_2 = create_sample_template( + notify_db, + notify_db_session, + template_name='Sample Template 2', + template_type="letter", + content="Template content" + ) + sample_cache_dict = {str.encode(str(template_1.id)): str.encode('2'), + str.encode(str(template_2.id)): str.encode('3')} + cache = [[k, v] for k, v in sample_cache_dict.items()] + templates = dao_get_templates_for_cache(cache) + assert len(templates) == 2 + assert [(template_1.id, template_1.template_type, template_1.name, True, 2), + (template_2.id, template_2.template_type, template_2.name, False, 3)] == templates def test_get_templates_by_ids_successful_for_one_cache_item(notify_db, notify_db_session): diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index efa55bc0b..244c21f6a 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -1,7 +1,6 @@ import pytest -from flask import current_app -from app.letters.utils import get_bucket_prefix_for_notification, is_precompiled_letter +from app.letters.utils import get_bucket_prefix_for_notification def test_get_bucket_prefix_for_notification_valid_notification(sample_notification): @@ -17,23 +16,3 @@ def test_get_bucket_prefix_for_notification_valid_notification(sample_notificati def test_get_bucket_prefix_for_notification_invalid_notification(): with pytest.raises(AttributeError): get_bucket_prefix_for_notification(None) - - -def test_is_precompiled_letter_false(sample_letter_template): - assert not is_precompiled_letter(sample_letter_template) - - -def test_is_precompiled_letter_true(sample_letter_template): - sample_letter_template.hidden = True - sample_letter_template.name = current_app.config['PRECOMPILED_TEMPLATE_NAME'] - assert is_precompiled_letter(sample_letter_template) - - -def test_is_precompiled_letter_hidden_true_not_name(sample_letter_template): - sample_letter_template.hidden = True - assert not is_precompiled_letter(sample_letter_template) - - -def test_is_precompiled_letter_name_correct_not_hidden(sample_letter_template): - sample_letter_template.name = current_app.config['PRECOMPILED_TEMPLATE_NAME'] - assert not is_precompiled_letter(sample_letter_template) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index b600b239b..7fb44db12 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -25,7 +25,8 @@ from app.models import ( User, DVLA_ORG_LAND_REGISTRY, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, - EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE + EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE, + PRECOMPILED_TEMPLATE_NAME ) from tests import create_authorization_header from tests.app.conftest import ( @@ -1834,7 +1835,7 @@ def test_get_template_usage_by_month_returns_two_templates( template_one = create_template( sample_service, template_type=LETTER_TYPE, - template_name=current_app.config['PRECOMPILED_TEMPLATE_NAME'], + template_name=PRECOMPILED_TEMPLATE_NAME, hidden=True ) @@ -1894,7 +1895,7 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[0]["month"] == 4 assert resp_json[0]["year"] == 2017 assert resp_json[0]["count"] == 1 - assert resp_json[0]["precompiled_letter"] is True + assert resp_json[0]["is_precompiled_letter"] is True assert resp_json[1]["template_id"] == str(sample_template.id) assert resp_json[1]["name"] == sample_template.name @@ -1902,7 +1903,7 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[1]["month"] == 4 assert resp_json[1]["year"] == 2017 assert resp_json[1]["count"] == 3 - assert resp_json[1]["precompiled_letter"] is False + assert resp_json[1]["is_precompiled_letter"] is False assert resp_json[2]["template_id"] == str(sample_template.id) assert resp_json[2]["name"] == sample_template.name @@ -1910,7 +1911,7 @@ def test_get_template_usage_by_month_returns_two_templates( assert resp_json[2]["month"] == 11 assert resp_json[2]["year"] == 2017 assert resp_json[2]["count"] == 1 - assert resp_json[2]["precompiled_letter"] is False + assert resp_json[2]["is_precompiled_letter"] is False def test_search_for_notification_by_to_field(client, notify_db, notify_db_session): @@ -2155,7 +2156,7 @@ def test_get_notification_for_service_includes_precompiled_letter(admin_request, ) assert resp['id'] == str(sample_notification.id) - assert resp['template']['precompiled_letter'] is False + assert resp['template']['is_precompiled_letter'] is False def test_get_all_notifications_for_service_includes_template_redacted(admin_request, sample_service): @@ -2202,10 +2203,10 @@ def test_get_all_notifications_for_service_includes_template_hidden(admin_reques ) assert resp['notifications'][0]['id'] == str(precompiled_noti.id) - assert resp['notifications'][0]['template']['precompiled_letter'] is True + assert resp['notifications'][0]['template']['is_precompiled_letter'] is True assert resp['notifications'][1]['id'] == str(letter_noti.id) - assert resp['notifications'][1]['template']['precompiled_letter'] is False + assert resp['notifications'][1]['template']['is_precompiled_letter'] is False def test_search_for_notification_by_to_field_returns_personlisation( diff --git a/tests/app/test_model.py b/tests/app/test_model.py index a0b8d4804..c7e253470 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -18,7 +18,8 @@ from app.models import ( NOTIFICATION_STATUS_LETTER_ACCEPTED, NOTIFICATION_STATUS_LETTER_RECEIVED, NOTIFICATION_STATUS_TYPES_FAILED, - NOTIFICATION_TECHNICAL_FAILURE + NOTIFICATION_TECHNICAL_FAILURE, + PRECOMPILED_TEMPLATE_NAME ) from tests.app.conftest import ( sample_template as create_sample_template, @@ -319,3 +320,23 @@ def test_letter_notification_postcode_can_be_null_for_precompiled_letters(client assert json['line_1'] == 'test' assert json['line_2'] == 'London' assert json['postcode'] is None + + +def test_is_precompiled_letter_false(sample_letter_template): + assert not sample_letter_template.is_precompiled_letter + + +def test_is_precompiled_letter_true(sample_letter_template): + sample_letter_template.hidden = True + sample_letter_template.name = PRECOMPILED_TEMPLATE_NAME + assert sample_letter_template.is_precompiled_letter + + +def test_is_precompiled_letter_hidden_true_not_name(sample_letter_template): + sample_letter_template.hidden = True + assert not sample_letter_template.is_precompiled_letter + + +def test_is_precompiled_letter_name_correct_not_hidden(sample_letter_template): + sample_letter_template.name = PRECOMPILED_TEMPLATE_NAME + assert not sample_letter_template.is_precompiled_letter From 23ce36dc484f37079afcc780120e5ae2b2dc28c3 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Wed, 7 Mar 2018 23:02:38 +0000 Subject: [PATCH 8/8] Update response to return is_precompiled_letter --- app/schemas.py | 6 +----- app/service/rest.py | 8 ++------ app/template_statistics/rest.py | 2 +- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index c76c2c922..2131bb9b7 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -305,7 +305,6 @@ class NotificationModelSchema(BaseSchema): class BaseTemplateSchema(BaseSchema): reply_to = fields.Method("get_reply_to", allow_none=True) reply_to_text = fields.Method("get_reply_to_text", allow_none=True) - precompiled_letter = fields.Method("get_precompiled_letter") def get_reply_to(self, template): return template.reply_to @@ -313,9 +312,6 @@ class BaseTemplateSchema(BaseSchema): def get_reply_to_text(self, template): return template.get_reply_to_text() - def get_precompiled_letter(self, template): - return template.is_precompiled_letter - class Meta: model = models.Template exclude = ("service_id", "jobs", "service_letter_contact_id") @@ -466,7 +462,7 @@ class NotificationWithTemplateSchema(BaseSchema): 'content', 'subject', 'redact_personalisation', - 'precompiled_letter' + 'is_precompiled_letter' ], dump_only=True ) diff --git a/app/service/rest.py b/app/service/rest.py index 2ccf51635..f6c7b8650 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -68,7 +68,7 @@ from app.errors import ( InvalidRequest, register_errors ) -from app.models import Service, EmailBranding, LETTER_TYPE, PRECOMPILED_TEMPLATE_NAME +from app.models import Service, EmailBranding from app.schema_validation import validate from app.service import statistics from app.service.service_senders_schema import ( @@ -537,11 +537,7 @@ def get_monthly_template_usage(service_id): 'month': i.month, 'year': i.year, 'count': i.count, - 'precompiled_letter': ( - i.template_type == LETTER_TYPE and - i.hidden and - i.name == PRECOMPILED_TEMPLATE_NAME - ) + 'is_precompiled_letter': i.is_precompiled_letter } ) diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index 2af5efdae..b7198409e 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -48,7 +48,7 @@ def get_template_statistics_for_service_by_day(service_id): 'template_id': str(data.template_id), 'template_name': data.name, 'template_type': data.template_type, - 'precompiled_letter': data.is_precompiled_letter + 'is_precompiled_letter': data.is_precompiled_letter } return jsonify(data=[serialize(row) for row in stats])