From 02e5ff61a42459f4d80df22fec40769f0144cebe Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 29 Jun 2020 11:16:29 +0100 Subject: [PATCH 1/3] Test that template history updates created by --- tests/app/template/test_rest.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 5b49a8cd7..59f9fbff3 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -340,8 +340,13 @@ def test_must_have_a_subject_on_an_email_or_letter_template(client, sample_user, def test_update_should_update_a_template(client, sample_user): + service = create_service(service_permissions=[LETTER_TYPE]) template = create_template(service, template_type="letter", postage="second") + + assert template.created_by == service.created_by + assert template.created_by != sample_user + data = { 'content': 'my template has new content, swell!', 'created_by': str(sample_user.id), @@ -366,6 +371,14 @@ def test_update_should_update_a_template(client, sample_user): assert update_json_resp['data']['template_type'] == template.template_type assert update_json_resp['data']['version'] == 2 + assert update_json_resp['data']['created_by'] == str(sample_user.id) + assert [ + template.created_by_id for template in TemplateHistory.query.all() + ] == [ + service.created_by.id, + sample_user.id, + ] + def test_should_be_able_to_archive_template(client, sample_template): data = { From f541ad42d67b8e222997debdbfe647a0bf8c501a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 29 Jun 2020 11:16:37 +0100 Subject: [PATCH 2/3] Revert "Avoid extra query when serialising Template created_by" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 58a9862cd1392c0c3384ce37d4b8eeb51dd7da32. That commit tried to optimize the fetch template query by However it had the side effect of making Marshmallow ignore `created_by` when loading the JSON in the post request. So the field on the model was never being updated, just copied from the original template. The quick way of getting things to work again is to revert this optimisation. There’s probably some Marshmallow magic we could use to avoid the extra query and still have created_by not be ignored. It does mean we’re introducing an extra query, but I’m not too fussed about that since the caching seems to be working well. --- app/schemas.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 6199c14bd..3258c1c45 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -356,9 +356,7 @@ class BaseTemplateSchema(BaseSchema): class TemplateSchema(BaseTemplateSchema, UUIDsAsStringsMixin): - created_by_id = field_for( - models.Template, 'created_by_id', dump_to='created_by', dump_only=True - ) + created_by = field_for(models.Template, 'created_by', required=True) process_type = field_for(models.Template, 'process_type') redact_personalisation = fields.Method("redact") @@ -372,9 +370,6 @@ class TemplateSchema(BaseTemplateSchema, UUIDsAsStringsMixin): if not subject or subject.strip() == '': raise ValidationError('Invalid template subject', 'subject') - class Meta(BaseTemplateSchema.Meta): - exclude = BaseTemplateSchema.Meta.exclude + ('created_by',) - class TemplateSchemaNoDetail(TemplateSchema): class Meta(TemplateSchema.Meta): From bf6e468a7c9b0d79f2b8b14092ced5562f0161cb Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 29 Jun 2020 12:48:24 +0100 Subject: [PATCH 3/3] Keep excluding created_by from TemplateSchemaNoDetail --- app/schemas.py | 1 + 1 file changed, 1 insertion(+) diff --git a/app/schemas.py b/app/schemas.py index 3258c1c45..a2601ed75 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -377,6 +377,7 @@ class TemplateSchemaNoDetail(TemplateSchema): 'archived', 'content', 'created_at', + 'created_by', 'created_by_id', 'hidden', 'postage',