From 718f8ccad07713ccd06389240f29d4bd05d1d0a1 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 16 Jun 2020 17:10:25 +0100
Subject: [PATCH] Remove content and subject from get all templates
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Content and subject are user-submitted so are effectively unbounded in
size. And we’re serialising them for every template when sending the
list of templates to the admin app.

For the service with the most templates this results in a 1.3Mb blob of
JSON going over the wire, and being cached in Redis.

And then the admin app completely ignores these fields, because it does
show template content until you’ve clicked into a single template.

This commit adds a new query parameter, `detailed`, that the admin app
can set to `False`. When it does only the fields needed to render the
`/templates` page are returned.

This is done with a new parameter so as not to break the V1 API.
Although I looked in Kibana and it doesn’t seem like anyone external is
using this endpoint we’ve come this far without breaking the API so…
---
 app/schemas.py                  | 23 +++++++++++++
 app/template/rest.py            |  7 ++--
 tests/app/template/test_rest.py | 61 +++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/app/schemas.py b/app/schemas.py
index c985d1812..66736aaed 100644
--- a/app/schemas.py
+++ b/app/schemas.py
@@ -358,6 +358,28 @@ class TemplateSchema(BaseTemplateSchema):
                 raise ValidationError('Invalid template subject', 'subject')
 
 
+class TemplateSchemaNoDetail(TemplateSchema):
+    class Meta(TemplateSchema.Meta):
+        exclude = TemplateSchema.Meta.exclude + (
+            'archived',
+            'content',
+            'created_at',
+            'created_by',
+            'hidden',
+            'postage',
+            'process_type',
+            'redact_personalisation',
+            'reply_to',
+            'reply_to_text',
+            'service',
+            'service_letter_contact',
+            'subject',
+            'template_redacted',
+            'updated_at',
+            'version',
+        )
+
+
 class TemplateHistorySchema(BaseSchema):
 
     reply_to = fields.Method("get_reply_to", allow_none=True)
@@ -669,6 +691,7 @@ user_update_password_schema_load_json = UserUpdatePasswordSchema(load_json=True,
 service_schema = ServiceSchema()
 detailed_service_schema = DetailedServiceSchema()
 template_schema = TemplateSchema()
+template_schema_no_detail = TemplateSchemaNoDetail()
 api_key_schema = ApiKeySchema()
 job_schema = JobSchema()
 sms_template_notification_schema = SmsTemplateNotificationSchema()
diff --git a/app/template/rest.py b/app/template/rest.py
index 10a9653d0..da401094b 100644
--- a/app/template/rest.py
+++ b/app/template/rest.py
@@ -36,7 +36,7 @@ from app.letters.utils import get_letter_pdf_and_metadata
 from app.models import SMS_TYPE, Template, SECOND_CLASS, LETTER_TYPE
 from app.notifications.validators import service_has_permission, check_reply_to
 from app.schema_validation import validate
-from app.schemas import (template_schema, template_history_schema)
+from app.schemas import (template_schema, template_schema_no_detail, template_history_schema)
 from app.template.template_schemas import post_create_template_schema, post_update_template_schema
 from app.utils import get_public_notify_type_text
 
@@ -152,7 +152,10 @@ def get_precompiled_template_for_service(service_id):
 @template_blueprint.route('', methods=['GET'])
 def get_all_templates_for_service(service_id):
     templates = dao_get_all_templates_for_service(service_id=service_id)
-    data = template_schema.dump(templates, many=True).data
+    if str(request.args.get('detailed', True)) == 'True':
+        data = template_schema.dump(templates, many=True).data
+    else:
+        data = template_schema_no_detail.dump(templates, many=True).data
     return jsonify(data=data)
 
 
diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py
index 17e97d037..5b49a8cd7 100644
--- a/tests/app/template/test_rest.py
+++ b/tests/app/template/test_rest.py
@@ -499,6 +499,67 @@ def test_should_get_only_templates_for_that_service(admin_request, notify_db_ses
     assert {template['id'] for template in json_resp_2['data']} == {str(id_3)}
 
 
+@pytest.mark.parametrize('extra_args', (
+    {},
+    {'detailed': True},
+    {'detailed': 'True'},
+))
+def test_should_get_return_all_fields_by_default(
+    admin_request,
+    sample_email_template,
+    extra_args,
+):
+    json_response = admin_request.get(
+        'template.get_all_templates_for_service',
+        service_id=sample_email_template.service.id,
+        **extra_args
+    )
+    assert json_response['data'][0].keys() == {
+        'archived',
+        'content',
+        'created_at',
+        'created_by',
+        'folder',
+        'hidden',
+        'id',
+        'name',
+        'postage',
+        'process_type',
+        'redact_personalisation',
+        'reply_to',
+        'reply_to_text',
+        'service',
+        'service_letter_contact',
+        'subject',
+        'template_redacted',
+        'template_type',
+        'updated_at',
+        'version',
+    }
+
+
+@pytest.mark.parametrize('extra_args', (
+    {'detailed': False},
+    {'detailed': 'False'},
+))
+def test_should_not_return_content_and_subject_if_requested(
+    admin_request,
+    sample_email_template,
+    extra_args,
+):
+    json_response = admin_request.get(
+        'template.get_all_templates_for_service',
+        service_id=sample_email_template.service.id,
+        **extra_args
+    )
+    assert json_response['data'][0].keys() == {
+        'folder',
+        'id',
+        'name',
+        'template_type',
+    }
+
+
 @pytest.mark.parametrize(
     "subject, content, template_type", [
         (