From 3171074695c3b53a43e03745385ca4453dd7b52a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 17 Feb 2025 16:07:49 +0000 Subject: [PATCH 01/22] make everything buttons --- app/assets/stylesheets/views/template.scss | 5 +++++ .../templates/_email_or_sms_template.html | 20 ++++++++++++------- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/views/template.scss b/app/assets/stylesheets/views/template.scss index aa4cb84cde..c23edd32b3 100644 --- a/app/assets/stylesheets/views/template.scss +++ b/app/assets/stylesheets/views/template.scss @@ -161,3 +161,8 @@ } } + +.template-action-button { + display: block; + text-align: center; +} \ No newline at end of file diff --git a/app/templates/views/templates/_email_or_sms_template.html b/app/templates/views/templates/_email_or_sms_template.html index f011856ab8..b378398e5d 100644 --- a/app/templates/views/templates/_email_or_sms_template.html +++ b/app/templates/views/templates/_email_or_sms_template.html @@ -1,18 +1,24 @@
{% if current_user.has_permissions('send_messages', restrict_admin_usage=True) %}
- - Get ready to send a message using this template - + {{ govukButton({ + "element": "a", + "text": "Get ready to send", + "href": url_for(".set_sender", service_id=current_service.id, template_id=template.id), + "classes": "govuk-button--secondary template-action-button" + }) }}
{% endif %} {% if current_user.has_permissions('manage_templates') %}
- - Edit this template - + {{ govukButton({ + "element": "a", + "text": "Edit", + "href": url_for(".edit_service_template", service_id=current_service.id, template_id=template.id), + "classes": "govuk-button--secondary template-action-button" + }) }}
{% endif %}
-{{ template|string }} +{{ template|string }} \ No newline at end of file From 7325b6f5800a637d9aa57a52e90ddbd0ea90835a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 17 Feb 2025 16:14:15 +0000 Subject: [PATCH 02/22] Add page to manage attachments --- app/main/views/templates.py | 11 ++++++++ .../templates/_email_or_sms_template.html | 25 ++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index fcb2fc74ae..d0fc3c8f5f 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -1392,6 +1392,17 @@ def letter_template_change_language(template_id, service_id): ) +@main.route("/services//templates//attachments", methods=["GET", "POST"]) +@user_has_permissions("manage_templates") +def email_template_manage_attachments(template_id, service_id): + template = current_service.get_template(template_id) + + return render_template( + "views/templates/manage-email-attachments.html", + template=template, + ) + + @main.route("/services//templates//change-language/confirm", methods=["GET", "POST"]) @user_has_permissions("manage_templates") def letter_template_confirm_remove_welsh(template_id, service_id): diff --git a/app/templates/views/templates/_email_or_sms_template.html b/app/templates/views/templates/_email_or_sms_template.html index b378398e5d..04b21da983 100644 --- a/app/templates/views/templates/_email_or_sms_template.html +++ b/app/templates/views/templates/_email_or_sms_template.html @@ -21,4 +21,27 @@ {% endif %} -{{ template|string }} \ No newline at end of file +{{ template|string }} + +{% if template.template_type == "email" %} + +
+
+ + No files attached + + + {{ govukButton({ + "element": "a", + "text": "Manage attachments", + "href": url_for( + '.email_template_manage_attachments', + service_id=current_service.id, + template_id=template.id + ), + "classes": "govuk-button--secondary change-language" + }) }} +
+
+ + {% endif %} \ No newline at end of file From cd40c9a220c7e66969a4975bcf0188e158cc0277 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 17 Feb 2025 16:49:13 +0000 Subject: [PATCH 03/22] Add page to manage attachments --- app/main/forms.py | 11 +++ app/main/views/templates.py | 45 ++++++++++ .../templates/manage-email-attachment.html | 87 +++++++++++++++++++ .../templates/manage-email-attachments.html | 26 ++++++ 4 files changed, 169 insertions(+) create mode 100644 app/templates/views/templates/manage-email-attachment.html create mode 100644 app/templates/views/templates/manage-email-attachments.html diff --git a/app/main/forms.py b/app/main/forms.py index 85398eeb80..56edf1af4e 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2977,3 +2977,14 @@ def validate_report_has_been_processed(self, field): if field.data and self.report_completed: raise ValidationError("There is a problem. You have already marked the report as Completed") + + +class EmailAttachmentForm(StripWhitespaceForm): + file = FileField( + "Add recipients", + validators=[ + DataRequired(message="You need to chose a file to upload"), + CsvFileValidator(), + FileSize(max_size=10 * 1024 * 1024, message="The file must be smaller than 10MB"), + ], + ) \ No newline at end of file diff --git a/app/main/views/templates.py b/app/main/views/templates.py index d0fc3c8f5f..16caa8cc11 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -54,6 +54,7 @@ TemplateAndFoldersSelectionForm, TemplateFolderForm, WelshLetterTemplateForm, + EmailAttachmentForm, ) from app.main.views.send import get_sender_details from app.models.service import Service @@ -1397,9 +1398,53 @@ def letter_template_change_language(template_id, service_id): def email_template_manage_attachments(template_id, service_id): template = current_service.get_template(template_id) + rows = [ + { + "key": { + "classes": "notify-summary-list__key notify-summary-list__key--35-100", + "text": placeholder + }, + "value": { + "text": "No file attached", + "classes": "govuk-summary-list__value--truncate govuk-hint" + }, + "actions": { + "items": [ + { + "href": url_for( + 'main.email_template_manage_attachment', + service_id=service_id, + template_id=template_id, + placeholder=placeholder.strip(), + ), + "text": "Change", + "visuallyHiddenText": "service name", + "classes": "govuk-link--no-visited-state" + } + ] + } + } + for placeholder in template.placeholders + ] + return render_template( "views/templates/manage-email-attachments.html", template=template, + rows=rows, + ) + + +@main.route("/services//templates//attachment", methods=["GET", "POST"]) +@user_has_permissions("manage_templates") +def email_template_manage_attachment(template_id, service_id): + template = current_service.get_template(template_id) + placeholder = request.args.get("placeholder") + form = EmailAttachmentForm() + return render_template( + "views/templates/manage-email-attachment.html", + template=template, + placeholder=placeholder, + form=form, ) diff --git a/app/templates/views/templates/manage-email-attachment.html b/app/templates/views/templates/manage-email-attachment.html new file mode 100644 index 0000000000..b929f5f704 --- /dev/null +++ b/app/templates/views/templates/manage-email-attachment.html @@ -0,0 +1,87 @@ +{% extends "withnav_template.html" %} +{% from "components/page-header.html" import page_header %} +{% from "govuk_frontend_jinja/components/back-link/macro.html" import govukBackLink %} +{% from "govuk_frontend_jinja/components/summary-list/macro.html" import govukSummaryList %} +{% from "components/file-upload.html" import file_upload %} +{% set title = 'Attach ‘' + placeholder + '’'%} +{% block service_page_title %} + {{ title }} +{% endblock %} + +{% block backLink %} + {{ govukBackLink({ "href": url_for('main.email_template_manage_attachments', service_id=current_service.id, template_id=template.id) }) }} +{% endblock %} + +{% block maincolumn_content %} + {{ page_header(title) }} + {{ govukSummaryList({ + "classes": "notify-summary-list service-base-settings", + "rows": [ + { + "key": { + "classes": "notify-summary-list__key notify-summary-list__key--35-100", + "text": "File" + }, + "value": { + "html": file_upload(form.file), + "classes": "govuk-summary-list__value--truncate govuk-hint" + }, + "actions": { + "items": [ + + ] + } + }, + { + "key": { + "classes": "notify-summary-list__key notify-summary-list__key--35-100", + "text": "Available for" + }, + "value": { + "text": "90 days after sending", + "classes": "govuk-summary-list__value--truncate" + }, + "actions": { + "items": [ + { + "href": url_for( + 'main.email_template_manage_attachment', + service_id=service_id, + template_id=template_id, + placeholder=placeholder.strip(), + ), + "text": "Change", + "visuallyHiddenText": "service name", + "classes": "govuk-link--no-visited-state" + } + ] + } + }, + { + "key": { + "classes": "notify-summary-list__key notify-summary-list__key--35-100", + "text": "Confirm email address" + }, + "value": { + "text": "Yes", + "classes": "govuk-summary-list__value--truncate" + }, + "actions": { + "items": [ + { + "href": url_for( + 'main.email_template_manage_attachment', + service_id=service_id, + template_id=template_id, + placeholder=placeholder.strip(), + ), + "text": "Change", + "visuallyHiddenText": "service name", + "classes": "govuk-link--no-visited-state" + } + ] + } + } + ] + }) }} +{% endblock %} diff --git a/app/templates/views/templates/manage-email-attachments.html b/app/templates/views/templates/manage-email-attachments.html new file mode 100644 index 0000000000..7a62efb64e --- /dev/null +++ b/app/templates/views/templates/manage-email-attachments.html @@ -0,0 +1,26 @@ +{% extends "withnav_template.html" %} +{% from "components/page-header.html" import page_header %} +{% from "govuk_frontend_jinja/components/back-link/macro.html" import govukBackLink %} +{% from "govuk_frontend_jinja/components/summary-list/macro.html" import govukSummaryList %} +{% set title = 'Manage attachments' %} +{% block service_page_title %} + {{ title }} +{% endblock %} + +{% block backLink %} + {{ govukBackLink({ "href": url_for('main.view_template', service_id=current_service.id, template_id=template.id) }) }} +{% endblock %} + +{% block maincolumn_content %} + {{ page_header(title) }} + {% if not template.placeholders %} +

+ You need to personalise message first +

+ {% else %} + {{ govukSummaryList({ + "classes": "notify-summary-list service-base-settings", + "rows": rows + }) }} + {% endif %} +{% endblock %} From ff8db69c6d553832c0316c286d86f78eeeba97a1 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 19 Feb 2025 14:01:37 +0000 Subject: [PATCH 04/22] Add pages to change settings --- app/main/forms.py | 12 +++++++ app/main/views/templates.py | 34 +++++++++++++++++++ ...e-email-attachment-email-confirmation.html | 29 ++++++++++++++++ .../manage-email-attachment-retention.html | 21 ++++++++++++ .../templates/manage-email-attachment.html | 14 ++++---- 5 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 app/templates/views/templates/manage-email-attachment-email-confirmation.html create mode 100644 app/templates/views/templates/manage-email-attachment-retention.html diff --git a/app/main/forms.py b/app/main/forms.py index 56edf1af4e..014dc722ff 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2514,6 +2514,18 @@ class SetServiceDataRetentionForm(StripWhitespaceForm): ) +class SetServiceAttachmentDataRetentionForm(StripWhitespaceForm): + weeks_of_retention = GovukIntegerField( + label="Number of weeks", + things="the number of weeks", + validators=[ + NotifyDataRequired(thing="a number of weeks"), + validators.NumberRange(min=3, max=90, message="The number of weeks must be between 1 and 78"), + ], + param_extensions={"hint": {"text": "Must be between 1 week and 78 weeks (18 months)"}}, + ) + + class AdminServiceAddDataRetentionForm(StripWhitespaceForm): notification_type = GovukRadiosField( "What notification type?", diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 16caa8cc11..fdba93f36c 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -55,6 +55,8 @@ TemplateFolderForm, WelshLetterTemplateForm, EmailAttachmentForm, + SetServiceAttachmentDataRetentionForm, + OnOffSettingForm, ) from app.main.views.send import get_sender_details from app.models.service import Service @@ -1448,6 +1450,38 @@ def email_template_manage_attachment(template_id, service_id): ) +@main.route("/services//templates//attachment/retention", methods=["GET", "POST"]) +@user_has_permissions("manage_templates") +def email_template_manage_attachment_retention(template_id, service_id): + template = current_service.get_template(template_id) + placeholder = request.args.get("placeholder") + form = SetServiceAttachmentDataRetentionForm() + return render_template( + "views/templates/manage-email-attachment-retention.html", + template=template, + placeholder=placeholder, + form=form, + ) + + +@main.route("/services//templates//attachment/email-confirmation", methods=["GET", "POST"]) +@user_has_permissions("manage_templates") +def email_template_manage_attachment_email_confirmation(template_id, service_id): + template = current_service.get_template(template_id) + placeholder = request.args.get("placeholder") + form = OnOffSettingForm( + "Require recipient to confirm email address", + truthy="Yes", + falsey="No", + ) + return render_template( + "views/templates/manage-email-attachment-email-confirmation.html", + template=template, + placeholder=placeholder, + form=form, + ) + + @main.route("/services//templates//change-language/confirm", methods=["GET", "POST"]) @user_has_permissions("manage_templates") def letter_template_confirm_remove_welsh(template_id, service_id): diff --git a/app/templates/views/templates/manage-email-attachment-email-confirmation.html b/app/templates/views/templates/manage-email-attachment-email-confirmation.html new file mode 100644 index 0000000000..6eb3c353d2 --- /dev/null +++ b/app/templates/views/templates/manage-email-attachment-email-confirmation.html @@ -0,0 +1,29 @@ +{% extends "withnav_template.html" %} +{% from "components/page-header.html" import page_header %} +{% from "govuk_frontend_jinja/components/back-link/macro.html" import govukBackLink %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} +{% block service_page_title %} + Require recipient to confirm email address +{% endblock %} + +{% block backLink %} + {{ govukBackLink({ "href": url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder) }) }} +{% endblock %} + +{% block maincolumn_content %} + {% call form_wrapper() %} + {{ form.enabled( + param_extensions={ + 'fieldset': { + 'legend': { + 'isPageHeading': True, + 'classes': 'govuk-fieldset__legend--l' + } + } + } + ) }} + {{ page_footer('Continue') }} + {% endcall %} + +{% endblock %} diff --git a/app/templates/views/templates/manage-email-attachment-retention.html b/app/templates/views/templates/manage-email-attachment-retention.html new file mode 100644 index 0000000000..b5c9f4177c --- /dev/null +++ b/app/templates/views/templates/manage-email-attachment-retention.html @@ -0,0 +1,21 @@ +{% extends "withnav_template.html" %} +{% from "components/page-header.html" import page_header %} +{% from "govuk_frontend_jinja/components/back-link/macro.html" import govukBackLink %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} +{% set title = 'How long should the file be available for?'%} +{% block service_page_title %} + {{ title }} +{% endblock %} + +{% block backLink %} + {{ govukBackLink({ "href": url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder) }) }} +{% endblock %} + +{% block maincolumn_content %} + {{ page_header(title) }} + {% call form_wrapper(class='top-gutter') %} + {{ form.weeks_of_retention() }} + {{ page_footer('Continue') }} + {% endcall %} +{% endblock %} diff --git a/app/templates/views/templates/manage-email-attachment.html b/app/templates/views/templates/manage-email-attachment.html index b929f5f704..e15b6e3a1a 100644 --- a/app/templates/views/templates/manage-email-attachment.html +++ b/app/templates/views/templates/manage-email-attachment.html @@ -38,16 +38,16 @@ "text": "Available for" }, "value": { - "text": "90 days after sending", + "text": "26 weeks after sending", "classes": "govuk-summary-list__value--truncate" }, "actions": { "items": [ { "href": url_for( - 'main.email_template_manage_attachment', - service_id=service_id, - template_id=template_id, + 'main.email_template_manage_attachment_retention', + service_id=current_service.id, + template_id=template.id, placeholder=placeholder.strip(), ), "text": "Change", @@ -70,9 +70,9 @@ "items": [ { "href": url_for( - 'main.email_template_manage_attachment', - service_id=service_id, - template_id=template_id, + 'main.email_template_manage_attachment_email_confirmation', + service_id=current_service.id, + template_id=template.id, placeholder=placeholder.strip(), ), "text": "Change", From adefa95c68385f61dd8206cbe6ccc4dd0f228904 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 19 Feb 2025 14:03:47 +0000 Subject: [PATCH 05/22] Make form submissions redirect --- app/main/forms.py | 2 +- app/main/views/templates.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/main/forms.py b/app/main/forms.py index 014dc722ff..6e9bb3ff0c 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2520,7 +2520,7 @@ class SetServiceAttachmentDataRetentionForm(StripWhitespaceForm): things="the number of weeks", validators=[ NotifyDataRequired(thing="a number of weeks"), - validators.NumberRange(min=3, max=90, message="The number of weeks must be between 1 and 78"), + validators.NumberRange(min=1, max=78, message="The number of weeks must be between 1 and 78"), ], param_extensions={"hint": {"text": "Must be between 1 week and 78 weeks (18 months)"}}, ) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index fdba93f36c..02f444fbc3 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -1456,6 +1456,8 @@ def email_template_manage_attachment_retention(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") form = SetServiceAttachmentDataRetentionForm() + if form.validate_on_submit(): + return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) return render_template( "views/templates/manage-email-attachment-retention.html", template=template, @@ -1474,6 +1476,8 @@ def email_template_manage_attachment_email_confirmation(template_id, service_id) truthy="Yes", falsey="No", ) + if form.validate_on_submit(): + return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) return render_template( "views/templates/manage-email-attachment-email-confirmation.html", template=template, From 1add4d3e03092927ace20744a1c5ee69c0430c3d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 19 Feb 2025 14:37:58 +0000 Subject: [PATCH 06/22] Add a data model --- app/main/views/templates.py | 14 ++++++++--- app/models/template_attachment.py | 25 +++++++++++++++++++ .../templates/manage-email-attachment.html | 4 +-- 3 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 app/models/template_attachment.py diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 02f444fbc3..aa8ccbf67c 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -61,6 +61,7 @@ from app.main.views.send import get_sender_details from app.models.service import Service from app.models.template_list import TemplateList, UserTemplateList, UserTemplateLists +from app.models.template_attachment import TemplateAttachments from app.s3_client.s3_letter_upload_client import ( backup_original_letter_to_s3, get_attachment_pdf_and_metadata, @@ -1399,7 +1400,7 @@ def letter_template_change_language(template_id, service_id): @user_has_permissions("manage_templates") def email_template_manage_attachments(template_id, service_id): template = current_service.get_template(template_id) - + attachments = TemplateAttachments(template.id) rows = [ { "key": { @@ -1407,7 +1408,7 @@ def email_template_manage_attachments(template_id, service_id): "text": placeholder }, "value": { - "text": "No file attached", + "text": attachments[placeholder].file_name or "No file attached", "classes": "govuk-summary-list__value--truncate govuk-hint" }, "actions": { @@ -1441,12 +1442,14 @@ def email_template_manage_attachments(template_id, service_id): def email_template_manage_attachment(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") + attachment = TemplateAttachments(template.id)["placeholder"] form = EmailAttachmentForm() return render_template( "views/templates/manage-email-attachment.html", template=template, placeholder=placeholder, form=form, + attachment=attachment, ) @@ -1455,7 +1458,10 @@ def email_template_manage_attachment(template_id, service_id): def email_template_manage_attachment_retention(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") - form = SetServiceAttachmentDataRetentionForm() + attachment = TemplateAttachments(template.id)["placeholder"] + form = SetServiceAttachmentDataRetentionForm( + weeks_of_retention=attachment.weeks_of_retention + ) if form.validate_on_submit(): return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) return render_template( @@ -1471,10 +1477,12 @@ def email_template_manage_attachment_retention(template_id, service_id): def email_template_manage_attachment_email_confirmation(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") + attachment = TemplateAttachments(template.id)["placeholder"] form = OnOffSettingForm( "Require recipient to confirm email address", truthy="Yes", falsey="No", + enabled=attachment.email_confirmation, ) if form.validate_on_submit(): return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py new file mode 100644 index 0000000000..d3d1c635ee --- /dev/null +++ b/app/models/template_attachment.py @@ -0,0 +1,25 @@ +from app.extensions import redis_client +from app.models import JSONModel + + +class TemplateAttachment(JSONModel): + file_name: str + weeks_of_retention: int + email_confirmation: bool + + __sort_attribute__ = "file_name" + + +class TemplateAttachments(): + def __init__(self, template_id): + self._dict = redis_client.get(f"template-{id}-attachments") or {} + + def __getitem__(self, placeholder_name): + try: + return TemplateAttachment(self._dict[placeholder_name]) + except KeyError: + return TemplateAttachment({ + "file_name": None, + "weeks_of_retention": 26, + "email_confirmation": True, + }) diff --git a/app/templates/views/templates/manage-email-attachment.html b/app/templates/views/templates/manage-email-attachment.html index e15b6e3a1a..e2ae0c6166 100644 --- a/app/templates/views/templates/manage-email-attachment.html +++ b/app/templates/views/templates/manage-email-attachment.html @@ -38,7 +38,7 @@ "text": "Available for" }, "value": { - "text": "26 weeks after sending", + "text": attachment.weeks_of_retention|string + " weeks after sending", "classes": "govuk-summary-list__value--truncate" }, "actions": { @@ -63,7 +63,7 @@ "text": "Confirm email address" }, "value": { - "text": "Yes", + "text": "Yes" if attachment.email_confirmation else "No", "classes": "govuk-summary-list__value--truncate" }, "actions": { From 470920cdea5de3f2b0af77312400888826b5d943 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 19 Feb 2025 15:35:51 +0000 Subject: [PATCH 07/22] Persist data --- app/main/forms.py | 1 - app/main/views/templates.py | 14 +++++-- app/models/template_attachment.py | 41 ++++++++++++++++--- .../templates/manage-email-attachment.html | 4 +- .../templates/manage-email-attachments.html | 1 + 5 files changed, 48 insertions(+), 13 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 6e9bb3ff0c..22cb3d0110 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2996,7 +2996,6 @@ class EmailAttachmentForm(StripWhitespaceForm): "Add recipients", validators=[ DataRequired(message="You need to chose a file to upload"), - CsvFileValidator(), FileSize(max_size=10 * 1024 * 1024, message="The file must be smaller than 10MB"), ], ) \ No newline at end of file diff --git a/app/main/views/templates.py b/app/main/views/templates.py index aa8ccbf67c..78a8bf151f 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -1409,7 +1409,7 @@ def email_template_manage_attachments(template_id, service_id): }, "value": { "text": attachments[placeholder].file_name or "No file attached", - "classes": "govuk-summary-list__value--truncate govuk-hint" + "classes": "govuk-summary-list__value--truncate" if attachments[placeholder] else "govuk-summary-list__value--truncate govuk-hint", }, "actions": { "items": [ @@ -1434,6 +1434,7 @@ def email_template_manage_attachments(template_id, service_id): "views/templates/manage-email-attachments.html", template=template, rows=rows, + attachments=attachments, ) @@ -1442,8 +1443,11 @@ def email_template_manage_attachments(template_id, service_id): def email_template_manage_attachment(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") - attachment = TemplateAttachments(template.id)["placeholder"] + attachment = TemplateAttachments(template.id)[placeholder] form = EmailAttachmentForm() + if form.validate_on_submit(): + attachment.file_name = form.file.data.filename + return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) return render_template( "views/templates/manage-email-attachment.html", template=template, @@ -1458,11 +1462,12 @@ def email_template_manage_attachment(template_id, service_id): def email_template_manage_attachment_retention(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") - attachment = TemplateAttachments(template.id)["placeholder"] + attachment = TemplateAttachments(template.id)[placeholder] form = SetServiceAttachmentDataRetentionForm( weeks_of_retention=attachment.weeks_of_retention ) if form.validate_on_submit(): + attachment.weeks_of_retention = form.weeks_of_retention.data return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) return render_template( "views/templates/manage-email-attachment-retention.html", @@ -1477,7 +1482,7 @@ def email_template_manage_attachment_retention(template_id, service_id): def email_template_manage_attachment_email_confirmation(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") - attachment = TemplateAttachments(template.id)["placeholder"] + attachment = TemplateAttachments(template.id)[placeholder] form = OnOffSettingForm( "Require recipient to confirm email address", truthy="Yes", @@ -1485,6 +1490,7 @@ def email_template_manage_attachment_email_confirmation(template_id, service_id) enabled=attachment.email_confirmation, ) if form.validate_on_submit(): + attachment.email_confirmation = form.enabled.data return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) return render_template( "views/templates/manage-email-attachment-email-confirmation.html", diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py index d3d1c635ee..748bf61138 100644 --- a/app/models/template_attachment.py +++ b/app/models/template_attachment.py @@ -1,3 +1,7 @@ +import json + +from notifications_utils.insensitive_dict import InsensitiveDict + from app.extensions import redis_client from app.models import JSONModel @@ -9,17 +13,42 @@ class TemplateAttachment(JSONModel): __sort_attribute__ = "file_name" + def __init__(self, *args, parent, placeholder_name, **kwargs): + super().__init__(*args, **kwargs) + self._parent = parent + self._placeholder_name = placeholder_name + + def __repr__(self): + return f"{self.__class__.__name__}(<{self._dict}>)" + + def __setattr__(self, name, value): + if name not in self.__annotations__.keys() or not hasattr(self, name): + return super().__setattr__(name, value) + self._dict[name] = value + self._parent[self._placeholder_name] = self._dict + + def __bool__(self): + return bool(self.file_name) + class TemplateAttachments(): def __init__(self, template_id): - self._dict = redis_client.get(f"template-{id}-attachments") or {} + self._template_id = template_id + self._dict = InsensitiveDict(json.loads(redis_client.get(self.cache_key) or "{}")) + + @property + def cache_key(self): + return f"template-{self._template_id}-attachments" def __getitem__(self, placeholder_name): - try: - return TemplateAttachment(self._dict[placeholder_name]) - except KeyError: - return TemplateAttachment({ + if placeholder_name not in self._dict: + self._dict[placeholder_name] = { "file_name": None, "weeks_of_retention": 26, "email_confirmation": True, - }) + } + return TemplateAttachment(self._dict[placeholder_name], parent=self, placeholder_name=placeholder_name) + + def __setitem__(self, placeholder_name, value): + self._dict[InsensitiveDict.make_key(placeholder_name)] = value + redis_client.set(self.cache_key, json.dumps(self._dict)) diff --git a/app/templates/views/templates/manage-email-attachment.html b/app/templates/views/templates/manage-email-attachment.html index e2ae0c6166..ced4a42e77 100644 --- a/app/templates/views/templates/manage-email-attachment.html +++ b/app/templates/views/templates/manage-email-attachment.html @@ -23,8 +23,8 @@ "text": "File" }, "value": { - "html": file_upload(form.file), - "classes": "govuk-summary-list__value--truncate govuk-hint" + "html": attachment.file_name or file_upload(form.file), + "classes": "" }, "actions": { "items": [ diff --git a/app/templates/views/templates/manage-email-attachments.html b/app/templates/views/templates/manage-email-attachments.html index 7a62efb64e..6ebc7fd1a5 100644 --- a/app/templates/views/templates/manage-email-attachments.html +++ b/app/templates/views/templates/manage-email-attachments.html @@ -13,6 +13,7 @@ {% block maincolumn_content %} {{ page_header(title) }} + {% if not template.placeholders %}

You need to personalise message first From 8ad87048f07d9610497e02b968fd9f4e81bf7be1 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 19 Feb 2025 15:51:34 +0000 Subject: [PATCH 08/22] Add count --- app/main/views/templates.py | 9 +++++---- app/models/template_attachment.py | 15 +++++++++++---- .../views/templates/_email_or_sms_template.html | 9 ++++++++- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 78a8bf151f..d45a4225a2 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -134,6 +134,7 @@ def view_template(service_id, template_id): template=template, user_has_template_permission=user_has_template_permission, content_count_message=content_count_message, + attachments=TemplateAttachments(template), ) @@ -1400,7 +1401,7 @@ def letter_template_change_language(template_id, service_id): @user_has_permissions("manage_templates") def email_template_manage_attachments(template_id, service_id): template = current_service.get_template(template_id) - attachments = TemplateAttachments(template.id) + attachments = TemplateAttachments(template) rows = [ { "key": { @@ -1443,7 +1444,7 @@ def email_template_manage_attachments(template_id, service_id): def email_template_manage_attachment(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") - attachment = TemplateAttachments(template.id)[placeholder] + attachment = TemplateAttachments(template)[placeholder] form = EmailAttachmentForm() if form.validate_on_submit(): attachment.file_name = form.file.data.filename @@ -1462,7 +1463,7 @@ def email_template_manage_attachment(template_id, service_id): def email_template_manage_attachment_retention(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") - attachment = TemplateAttachments(template.id)[placeholder] + attachment = TemplateAttachments(template)[placeholder] form = SetServiceAttachmentDataRetentionForm( weeks_of_retention=attachment.weeks_of_retention ) @@ -1482,7 +1483,7 @@ def email_template_manage_attachment_retention(template_id, service_id): def email_template_manage_attachment_email_confirmation(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") - attachment = TemplateAttachments(template.id)[placeholder] + attachment = TemplateAttachments(template)[placeholder] form = OnOffSettingForm( "Require recipient to confirm email address", truthy="Yes", diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py index 748bf61138..c97103e2e1 100644 --- a/app/models/template_attachment.py +++ b/app/models/template_attachment.py @@ -1,6 +1,6 @@ import json -from notifications_utils.insensitive_dict import InsensitiveDict +from notifications_utils.insensitive_dict import InsensitiveDict, InsensitiveSet from app.extensions import redis_client from app.models import JSONModel @@ -32,13 +32,13 @@ def __bool__(self): class TemplateAttachments(): - def __init__(self, template_id): - self._template_id = template_id + def __init__(self, template): + self._template = template self._dict = InsensitiveDict(json.loads(redis_client.get(self.cache_key) or "{}")) @property def cache_key(self): - return f"template-{self._template_id}-attachments" + return f"template-{self._template.id}-attachments" def __getitem__(self, placeholder_name): if placeholder_name not in self._dict: @@ -52,3 +52,10 @@ def __getitem__(self, placeholder_name): def __setitem__(self, placeholder_name, value): self._dict[InsensitiveDict.make_key(placeholder_name)] = value redis_client.set(self.cache_key, json.dumps(self._dict)) + + @property + def count(self): + return sum( + bool(self[key]) for key in self._dict + if key in InsensitiveDict.from_keys(self._template.placeholders) + ) diff --git a/app/templates/views/templates/_email_or_sms_template.html b/app/templates/views/templates/_email_or_sms_template.html index 04b21da983..bd0341ad4d 100644 --- a/app/templates/views/templates/_email_or_sms_template.html +++ b/app/templates/views/templates/_email_or_sms_template.html @@ -28,7 +28,14 @@

- No files attached + {% if not attachments.count %} + No files + {% elif attachments.count == 1 %} + 1 file + {% else %} + {{ attachments.count }} files + {% endif %} + attached {{ govukButton({ From 65d65145f91208777eb9d73e61292cd7668dcd29 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 19 Feb 2025 16:07:57 +0000 Subject: [PATCH 09/22] implement delete --- app/main/views/templates.py | 7 ++++++- app/models/template_attachment.py | 4 ++++ .../templates/manage-email-attachment.html | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index d45a4225a2..155db44703 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -1443,9 +1443,13 @@ def email_template_manage_attachments(template_id, service_id): @user_has_permissions("manage_templates") def email_template_manage_attachment(template_id, service_id): template = current_service.get_template(template_id) - placeholder = request.args.get("placeholder") + placeholder = request.args.get("placeholder", "") attachment = TemplateAttachments(template)[placeholder] + delete = bool(request.args.get("delete")) form = EmailAttachmentForm() + if delete and request.method == "POST": + del TemplateAttachments(template)[placeholder] + return redirect(url_for('main.email_template_manage_attachments', service_id=current_service.id, template_id=template.id)) if form.validate_on_submit(): attachment.file_name = form.file.data.filename return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) @@ -1455,6 +1459,7 @@ def email_template_manage_attachment(template_id, service_id): placeholder=placeholder, form=form, attachment=attachment, + delete=delete, ) diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py index c97103e2e1..e1087591ce 100644 --- a/app/models/template_attachment.py +++ b/app/models/template_attachment.py @@ -53,6 +53,10 @@ def __setitem__(self, placeholder_name, value): self._dict[InsensitiveDict.make_key(placeholder_name)] = value redis_client.set(self.cache_key, json.dumps(self._dict)) + def __delitem__(self, placeholder_name): + self._dict.pop(InsensitiveDict.make_key(placeholder_name)) + redis_client.set(self.cache_key, json.dumps(self._dict)) + @property def count(self): return sum( diff --git a/app/templates/views/templates/manage-email-attachment.html b/app/templates/views/templates/manage-email-attachment.html index ced4a42e77..9e0a11d3ff 100644 --- a/app/templates/views/templates/manage-email-attachment.html +++ b/app/templates/views/templates/manage-email-attachment.html @@ -3,6 +3,8 @@ {% from "govuk_frontend_jinja/components/back-link/macro.html" import govukBackLink %} {% from "govuk_frontend_jinja/components/summary-list/macro.html" import govukSummaryList %} {% from "components/file-upload.html" import file_upload %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/banner.html" import banner %} {% set title = 'Attach ‘' + placeholder + '’'%} {% block service_page_title %} {{ title }} @@ -13,6 +15,15 @@ {% endblock %} {% block maincolumn_content %} + + {% if delete %} + {{ banner( + 'Are you sure you want to remove this attachment', + type='dangerous', + delete_button='Yes, remove', + action=url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder, delete="true") + ) }} + {% endif %} {{ page_header(title) }} {{ govukSummaryList({ "classes": "notify-summary-list service-base-settings", @@ -84,4 +95,11 @@ } ] }) }} + + {% if attachment.file_name %} + {{ page_footer( + delete_link=url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder, delete=true), + delete_link_text="Remove this attachment", + ) }} + {% endif %} {% endblock %} From 11cba49859b985b51e102f7c78b3f481063844a9 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 19 Feb 2025 16:29:31 +0000 Subject: [PATCH 10/22] Show in preview of template --- app/main/views/templates.py | 4 +++- app/models/template_attachment.py | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 155db44703..7839f9b857 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -117,6 +117,8 @@ def view_template(service_id, template_id): show_recipient=True, include_letter_edit_ui_overlay=True, ) + attachments = TemplateAttachments(template) + template.values = attachments.as_personalisation if template._template["archived"]: template.include_letter_edit_ui_overlay = False @@ -134,7 +136,7 @@ def view_template(service_id, template_id): template=template, user_has_template_permission=user_has_template_permission, content_count_message=content_count_message, - attachments=TemplateAttachments(template), + attachments=attachments, ) diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py index e1087591ce..e5f00b8ad3 100644 --- a/app/models/template_attachment.py +++ b/app/models/template_attachment.py @@ -1,3 +1,4 @@ +import base64 import json from notifications_utils.insensitive_dict import InsensitiveDict, InsensitiveSet @@ -32,6 +33,9 @@ def __bool__(self): class TemplateAttachments(): + + BASE_URL = "https://www.download.example.gov.uk/f/" + def __init__(self, template): self._template = template self._dict = InsensitiveDict(json.loads(redis_client.get(self.cache_key) or "{}")) @@ -63,3 +67,10 @@ def count(self): bool(self[key]) for key in self._dict if key in InsensitiveDict.from_keys(self._template.placeholders) ) + + @property + def as_personalisation(self): + return { + placeholder: f"{self.BASE_URL}{base64.b64encode(attachment['file_name'].encode()).decode()}" if attachment['file_name'] else None + for placeholder, attachment in self._dict.items() + } From 2dedaad8dd81e51cedff9717da5e93d34d149af2 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 19 Feb 2025 16:41:30 +0000 Subject: [PATCH 11/22] Render fields --- app/main/views/templates.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 7839f9b857..e8b941952a 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -19,6 +19,7 @@ from flask_login import current_user from notifications_python_client.errors import HTTPError from notifications_utils import SMS_CHAR_COUNT_LIMIT +from notifications_utils.field import Field from notifications_utils.pdf import pdf_page_count from notifications_utils.s3 import s3download from notifications_utils.template import Template @@ -1408,7 +1409,7 @@ def email_template_manage_attachments(template_id, service_id): { "key": { "classes": "notify-summary-list__key notify-summary-list__key--35-100", - "text": placeholder + "html": Field(f"(({placeholder}))") }, "value": { "text": attachments[placeholder].file_name or "No file attached", From 87c752b260322ae784b01027cdfb20b457b0e1b4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 20 Feb 2025 14:22:09 +0000 Subject: [PATCH 12/22] Refactor into template model --- app/main/views/templates.py | 18 +++++++----------- .../templates/_email_or_sms_template.html | 6 +++--- app/utils/templates.py | 6 ++++++ 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index e8b941952a..bfd316cecd 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -118,8 +118,7 @@ def view_template(service_id, template_id): show_recipient=True, include_letter_edit_ui_overlay=True, ) - attachments = TemplateAttachments(template) - template.values = attachments.as_personalisation + template.values = template.attachments.as_personalisation if template._template["archived"]: template.include_letter_edit_ui_overlay = False @@ -137,7 +136,6 @@ def view_template(service_id, template_id): template=template, user_has_template_permission=user_has_template_permission, content_count_message=content_count_message, - attachments=attachments, ) @@ -1404,7 +1402,6 @@ def letter_template_change_language(template_id, service_id): @user_has_permissions("manage_templates") def email_template_manage_attachments(template_id, service_id): template = current_service.get_template(template_id) - attachments = TemplateAttachments(template) rows = [ { "key": { @@ -1412,8 +1409,8 @@ def email_template_manage_attachments(template_id, service_id): "html": Field(f"(({placeholder}))") }, "value": { - "text": attachments[placeholder].file_name or "No file attached", - "classes": "govuk-summary-list__value--truncate" if attachments[placeholder] else "govuk-summary-list__value--truncate govuk-hint", + "text": template.attachments[placeholder].file_name or "No file attached", + "classes": "govuk-summary-list__value--truncate" if template.attachments[placeholder] else "govuk-summary-list__value--truncate govuk-hint", }, "actions": { "items": [ @@ -1438,7 +1435,6 @@ def email_template_manage_attachments(template_id, service_id): "views/templates/manage-email-attachments.html", template=template, rows=rows, - attachments=attachments, ) @@ -1447,11 +1443,11 @@ def email_template_manage_attachments(template_id, service_id): def email_template_manage_attachment(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder", "") - attachment = TemplateAttachments(template)[placeholder] + attachment = template.attachments[placeholder] delete = bool(request.args.get("delete")) form = EmailAttachmentForm() if delete and request.method == "POST": - del TemplateAttachments(template)[placeholder] + del template.attachments[placeholder] return redirect(url_for('main.email_template_manage_attachments', service_id=current_service.id, template_id=template.id)) if form.validate_on_submit(): attachment.file_name = form.file.data.filename @@ -1471,7 +1467,7 @@ def email_template_manage_attachment(template_id, service_id): def email_template_manage_attachment_retention(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") - attachment = TemplateAttachments(template)[placeholder] + attachment = template.attachments[placeholder] form = SetServiceAttachmentDataRetentionForm( weeks_of_retention=attachment.weeks_of_retention ) @@ -1491,7 +1487,7 @@ def email_template_manage_attachment_retention(template_id, service_id): def email_template_manage_attachment_email_confirmation(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") - attachment = TemplateAttachments(template)[placeholder] + attachment = template.attachments[placeholder] form = OnOffSettingForm( "Require recipient to confirm email address", truthy="Yes", diff --git a/app/templates/views/templates/_email_or_sms_template.html b/app/templates/views/templates/_email_or_sms_template.html index bd0341ad4d..0dbb4d9773 100644 --- a/app/templates/views/templates/_email_or_sms_template.html +++ b/app/templates/views/templates/_email_or_sms_template.html @@ -28,12 +28,12 @@
- {% if not attachments.count %} + {% if not template.attachments.count %} No files - {% elif attachments.count == 1 %} + {% elif template.attachments.count == 1 %} 1 file {% else %} - {{ attachments.count }} files + {{ template.attachments.count }} files {% endif %} attached diff --git a/app/utils/templates.py b/app/utils/templates.py index 16c4b553da..0027817119 100644 --- a/app/utils/templates.py +++ b/app/utils/templates.py @@ -13,9 +13,11 @@ SMSPreviewTemplate, do_nice_typography, ) +from werkzeug.utils import cached_property from app.extensions import redis_client from app.models import JSONModel +from app.models.template_attachment import TemplateAttachments from app.notify_client import cache @@ -274,6 +276,10 @@ def subject(self): .then(normalise_whitespace) ) + @cached_property + def attachments(self): + return TemplateAttachments(self) + class LetterAttachment(JSONModel): id: Any From 6b4830e56a8b7d2cd35cc7cdf286809e0b2b1bf8 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 20 Feb 2025 15:04:33 +0000 Subject: [PATCH 13/22] Populate attachment links everywhere --- app/main/views/templates.py | 3 +-- app/models/template_attachment.py | 2 +- .../templates/manage-email-attachments.html | 2 +- app/utils/templates.py | 26 +++++++++++++++++++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index bfd316cecd..7c5cf10850 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -118,7 +118,6 @@ def view_template(service_id, template_id): show_recipient=True, include_letter_edit_ui_overlay=True, ) - template.values = template.attachments.as_personalisation if template._template["archived"]: template.include_letter_edit_ui_overlay = False @@ -1428,7 +1427,7 @@ def email_template_manage_attachments(template_id, service_id): ] } } - for placeholder in template.placeholders + for placeholder in template.all_placeholders ] return render_template( diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py index e5f00b8ad3..fd5d1abd7d 100644 --- a/app/models/template_attachment.py +++ b/app/models/template_attachment.py @@ -65,7 +65,7 @@ def __delitem__(self, placeholder_name): def count(self): return sum( bool(self[key]) for key in self._dict - if key in InsensitiveDict.from_keys(self._template.placeholders) + if key in InsensitiveDict.from_keys(self._template.all_placeholders) ) @property diff --git a/app/templates/views/templates/manage-email-attachments.html b/app/templates/views/templates/manage-email-attachments.html index 6ebc7fd1a5..b8dfbf1126 100644 --- a/app/templates/views/templates/manage-email-attachments.html +++ b/app/templates/views/templates/manage-email-attachments.html @@ -14,7 +14,7 @@ {% block maincolumn_content %} {{ page_header(title) }} - {% if not template.placeholders %} + {% if not rows %}

You need to personalise message first

diff --git a/app/utils/templates.py b/app/utils/templates.py index 0027817119..3d013a4ed2 100644 --- a/app/utils/templates.py +++ b/app/utils/templates.py @@ -3,7 +3,9 @@ from flask import current_app, render_template, url_for from markupsafe import Markup +from ordered_set import OrderedSet from notifications_utils.countries import Postage +from notifications_utils.insensitive_dict import InsensitiveDict from notifications_utils.field import Field from notifications_utils.formatters import escape_html, formatted_list, normalise_whitespace from notifications_utils.take import Take @@ -280,6 +282,30 @@ def subject(self): def attachments(self): return TemplateAttachments(self) + @property + def values(self): + return super().values | self.attachments.as_personalisation + + @values.setter + def values(self, value): + # Assigning to super().values doesn’t work here. We need to get + # the property object instead, which has the special method + # fset, which invokes the setter it as if we were + # assigning to it outside this class. + super(EmailPreviewTemplate, type(self)).values.fset(self, value) + + @property + def all_placeholders(self): + # Includes those pre-populated with an attachment + return super().placeholders + + @property + def placeholders(self): + return OrderedSet(( + placeholder for placeholder in self.all_placeholders + if InsensitiveDict.make_key(placeholder) not in self.attachments.as_personalisation.keys() + )) + class LetterAttachment(JSONModel): id: Any From c723e6faeef5cec871eee0388d509976e7d64be7 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 21 Feb 2025 10:29:18 +0000 Subject: [PATCH 14/22] Improve empty state message --- .../templates/manage-email-attachments.html | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/app/templates/views/templates/manage-email-attachments.html b/app/templates/views/templates/manage-email-attachments.html index b8dfbf1126..5a4246dcbe 100644 --- a/app/templates/views/templates/manage-email-attachments.html +++ b/app/templates/views/templates/manage-email-attachments.html @@ -15,9 +15,20 @@ {{ page_header(title) }} {% if not rows %} -

- You need to personalise message first -

+
+
+

+ You need to personalise your template before you can upload an attachment, for example: +

+
+ Download file here: ((application form)) +
+

+ When you upload an attachment, Notify will replace the placeholder with a link + to download the file. +

+
+
{% else %} {{ govukSummaryList({ "classes": "notify-summary-list service-base-settings", From 983996c67611af9e1db4bab42b54c578a6ce5d96 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 21 Feb 2025 10:48:07 +0000 Subject: [PATCH 15/22] Run ruff format --- app/main/forms.py | 2 +- app/main/views/templates.py | 73 +++++++++++++++++++++---------- app/models/template_attachment.py | 10 ++--- app/utils/templates.py | 11 +++-- 4 files changed, 62 insertions(+), 34 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 22cb3d0110..29ae203783 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2998,4 +2998,4 @@ class EmailAttachmentForm(StripWhitespaceForm): DataRequired(message="You need to chose a file to upload"), FileSize(max_size=10 * 1024 * 1024, message="The file must be smaller than 10MB"), ], - ) \ No newline at end of file + ) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 7c5cf10850..46517025aa 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -1405,27 +1405,29 @@ def email_template_manage_attachments(template_id, service_id): { "key": { "classes": "notify-summary-list__key notify-summary-list__key--35-100", - "html": Field(f"(({placeholder}))") + "html": Field(f"(({placeholder}))"), }, "value": { "text": template.attachments[placeholder].file_name or "No file attached", - "classes": "govuk-summary-list__value--truncate" if template.attachments[placeholder] else "govuk-summary-list__value--truncate govuk-hint", + "classes": "govuk-summary-list__value--truncate" + if template.attachments[placeholder] + else "govuk-summary-list__value--truncate govuk-hint", }, "actions": { - "items": [ - { - "href": url_for( - 'main.email_template_manage_attachment', - service_id=service_id, - template_id=template_id, - placeholder=placeholder.strip(), - ), - "text": "Change", - "visuallyHiddenText": "service name", - "classes": "govuk-link--no-visited-state" - } - ] - } + "items": [ + { + "href": url_for( + "main.email_template_manage_attachment", + service_id=service_id, + template_id=template_id, + placeholder=placeholder.strip(), + ), + "text": "Change", + "visuallyHiddenText": "service name", + "classes": "govuk-link--no-visited-state", + } + ] + }, } for placeholder in template.all_placeholders ] @@ -1447,10 +1449,19 @@ def email_template_manage_attachment(template_id, service_id): form = EmailAttachmentForm() if delete and request.method == "POST": del template.attachments[placeholder] - return redirect(url_for('main.email_template_manage_attachments', service_id=current_service.id, template_id=template.id)) + return redirect( + url_for("main.email_template_manage_attachments", service_id=current_service.id, template_id=template.id) + ) if form.validate_on_submit(): attachment.file_name = form.file.data.filename - return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) + return redirect( + url_for( + "main.email_template_manage_attachment", + service_id=current_service.id, + template_id=template.id, + placeholder=placeholder, + ) + ) return render_template( "views/templates/manage-email-attachment.html", template=template, @@ -1467,12 +1478,17 @@ def email_template_manage_attachment_retention(template_id, service_id): template = current_service.get_template(template_id) placeholder = request.args.get("placeholder") attachment = template.attachments[placeholder] - form = SetServiceAttachmentDataRetentionForm( - weeks_of_retention=attachment.weeks_of_retention - ) + form = SetServiceAttachmentDataRetentionForm(weeks_of_retention=attachment.weeks_of_retention) if form.validate_on_submit(): attachment.weeks_of_retention = form.weeks_of_retention.data - return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) + return redirect( + url_for( + "main.email_template_manage_attachment", + service_id=current_service.id, + template_id=template.id, + placeholder=placeholder, + ) + ) return render_template( "views/templates/manage-email-attachment-retention.html", template=template, @@ -1481,7 +1497,9 @@ def email_template_manage_attachment_retention(template_id, service_id): ) -@main.route("/services//templates//attachment/email-confirmation", methods=["GET", "POST"]) +@main.route( + "/services//templates//attachment/email-confirmation", methods=["GET", "POST"] +) @user_has_permissions("manage_templates") def email_template_manage_attachment_email_confirmation(template_id, service_id): template = current_service.get_template(template_id) @@ -1495,7 +1513,14 @@ def email_template_manage_attachment_email_confirmation(template_id, service_id) ) if form.validate_on_submit(): attachment.email_confirmation = form.enabled.data - return redirect(url_for('main.email_template_manage_attachment', service_id=current_service.id, template_id=template.id, placeholder=placeholder)) + return redirect( + url_for( + "main.email_template_manage_attachment", + service_id=current_service.id, + template_id=template.id, + placeholder=placeholder, + ) + ) return render_template( "views/templates/manage-email-attachment-email-confirmation.html", template=template, diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py index fd5d1abd7d..1770132302 100644 --- a/app/models/template_attachment.py +++ b/app/models/template_attachment.py @@ -32,8 +32,7 @@ def __bool__(self): return bool(self.file_name) -class TemplateAttachments(): - +class TemplateAttachments: BASE_URL = "https://www.download.example.gov.uk/f/" def __init__(self, template): @@ -64,13 +63,14 @@ def __delitem__(self, placeholder_name): @property def count(self): return sum( - bool(self[key]) for key in self._dict - if key in InsensitiveDict.from_keys(self._template.all_placeholders) + bool(self[key]) for key in self._dict if key in InsensitiveDict.from_keys(self._template.all_placeholders) ) @property def as_personalisation(self): return { - placeholder: f"{self.BASE_URL}{base64.b64encode(attachment['file_name'].encode()).decode()}" if attachment['file_name'] else None + placeholder: f"{self.BASE_URL}{base64.b64encode(attachment['file_name'].encode()).decode()}" + if attachment["file_name"] + else None for placeholder, attachment in self._dict.items() } diff --git a/app/utils/templates.py b/app/utils/templates.py index 3d013a4ed2..8b7aad7bf1 100644 --- a/app/utils/templates.py +++ b/app/utils/templates.py @@ -301,10 +301,13 @@ def all_placeholders(self): @property def placeholders(self): - return OrderedSet(( - placeholder for placeholder in self.all_placeholders - if InsensitiveDict.make_key(placeholder) not in self.attachments.as_personalisation.keys() - )) + return OrderedSet( + ( + placeholder + for placeholder in self.all_placeholders + if InsensitiveDict.make_key(placeholder) not in self.attachments.as_personalisation.keys() + ) + ) class LetterAttachment(JSONModel): From a398b4cfa81ddce9bc300d6b083f9f21e83e4f17 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 21 Feb 2025 10:48:42 +0000 Subject: [PATCH 16/22] Run ruff check . --fix --- app/main/views/templates.py | 7 +++---- app/models/template_attachment.py | 2 +- app/utils/templates.py | 8 ++++---- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 46517025aa..a4eb7472d0 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -42,27 +42,26 @@ from app.main import main, no_cookie from app.main.forms import ( CopyTemplateForm, + EmailAttachmentForm, EmailTemplateForm, FieldWithNoneOption, LetterTemplateForm, LetterTemplateLanguagesForm, LetterTemplatePostageForm, + OnOffSettingForm, PDFUploadForm, RenameTemplateForm, SearchTemplatesForm, + SetServiceAttachmentDataRetentionForm, SetTemplateSenderForm, SMSTemplateForm, TemplateAndFoldersSelectionForm, TemplateFolderForm, WelshLetterTemplateForm, - EmailAttachmentForm, - SetServiceAttachmentDataRetentionForm, - OnOffSettingForm, ) from app.main.views.send import get_sender_details from app.models.service import Service from app.models.template_list import TemplateList, UserTemplateList, UserTemplateLists -from app.models.template_attachment import TemplateAttachments from app.s3_client.s3_letter_upload_client import ( backup_original_letter_to_s3, get_attachment_pdf_and_metadata, diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py index 1770132302..a60114b38b 100644 --- a/app/models/template_attachment.py +++ b/app/models/template_attachment.py @@ -1,7 +1,7 @@ import base64 import json -from notifications_utils.insensitive_dict import InsensitiveDict, InsensitiveSet +from notifications_utils.insensitive_dict import InsensitiveDict from app.extensions import redis_client from app.models import JSONModel diff --git a/app/utils/templates.py b/app/utils/templates.py index 8b7aad7bf1..f50541355a 100644 --- a/app/utils/templates.py +++ b/app/utils/templates.py @@ -3,11 +3,10 @@ from flask import current_app, render_template, url_for from markupsafe import Markup -from ordered_set import OrderedSet from notifications_utils.countries import Postage -from notifications_utils.insensitive_dict import InsensitiveDict from notifications_utils.field import Field from notifications_utils.formatters import escape_html, formatted_list, normalise_whitespace +from notifications_utils.insensitive_dict import InsensitiveDict from notifications_utils.take import Take from notifications_utils.template import ( BaseEmailTemplate, @@ -15,6 +14,7 @@ SMSPreviewTemplate, do_nice_typography, ) +from ordered_set import OrderedSet from werkzeug.utils import cached_property from app.extensions import redis_client @@ -302,11 +302,11 @@ def all_placeholders(self): @property def placeholders(self): return OrderedSet( - ( + placeholder for placeholder in self.all_placeholders if InsensitiveDict.make_key(placeholder) not in self.attachments.as_personalisation.keys() - ) + ) From bd880b65c97c5ec46a4521eb37953a2b23039a97 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 21 Feb 2025 14:05:49 +0000 Subject: [PATCH 17/22] Remove attachment when placeholder removed --- app/main/views/templates.py | 2 ++ app/models/template_attachment.py | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index a4eb7472d0..1fb05d27da 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -771,6 +771,8 @@ def edit_service_template(service_id, template_id, language=None): else: raise e else: + if new_template.template_type == "email": + new_template.attachments.prune_orphans() editing_english_content_in_bilingual_letter = ( template.template_type == "letter" and template.welsh_page_count and language != "welsh" ) diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py index a60114b38b..bbda21036f 100644 --- a/app/models/template_attachment.py +++ b/app/models/template_attachment.py @@ -74,3 +74,8 @@ def as_personalisation(self): else None for placeholder, attachment in self._dict.items() } + + def prune_orphans(self): + for placeholder in self._dict.keys(): + if placeholder not in self._template.all_placeholders: + del self[placeholder] From b02ba87f5812e84d6bae7cbd9196319740edee61 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 24 Feb 2025 17:24:48 +0000 Subject: [PATCH 18/22] Remove truncation --- app/main/views/templates.py | 4 +--- app/utils/templates.py | 8 +++----- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 1fb05d27da..268f669d84 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -1410,9 +1410,7 @@ def email_template_manage_attachments(template_id, service_id): }, "value": { "text": template.attachments[placeholder].file_name or "No file attached", - "classes": "govuk-summary-list__value--truncate" - if template.attachments[placeholder] - else "govuk-summary-list__value--truncate govuk-hint", + "classes": "" if template.attachments[placeholder] else "govuk-hint", }, "actions": { "items": [ diff --git a/app/utils/templates.py b/app/utils/templates.py index f50541355a..1ecdd6d3b7 100644 --- a/app/utils/templates.py +++ b/app/utils/templates.py @@ -302,11 +302,9 @@ def all_placeholders(self): @property def placeholders(self): return OrderedSet( - - placeholder - for placeholder in self.all_placeholders - if InsensitiveDict.make_key(placeholder) not in self.attachments.as_personalisation.keys() - + placeholder + for placeholder in self.all_placeholders + if InsensitiveDict.make_key(placeholder) not in self.attachments.as_personalisation.keys() ) From dc9ffb31f1fdeb5f2ff5b6102602c6445836ce41 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 7 Jan 2025 14:47:08 +0000 Subject: [PATCH 19/22] Stop logging out platform admin users after 30 mins This is purely to help when running user research sessions, and not a change we want to make in other environments. --- app/notify_session.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/app/notify_session.py b/app/notify_session.py index ee47f0073d..9400861891 100644 --- a/app/notify_session.py +++ b/app/notify_session.py @@ -1,4 +1,4 @@ -from datetime import UTC, datetime, timedelta +from datetime import UTC, datetime from flask import Flask, Request, Response, request from flask.sessions import SecureCookieSession, SecureCookieSessionInterface @@ -17,10 +17,7 @@ def _get_inactive_session_expiry(self, app, session_start: datetime): """ absolute_expiration = session_start + app.permanent_session_lifetime - if current_user and current_user.platform_admin: - refresh_duration = timedelta(seconds=app.config["PLATFORM_ADMIN_INACTIVE_SESSION_TIMEOUT"]) - else: - refresh_duration = app.permanent_session_lifetime + refresh_duration = app.permanent_session_lifetime return min(datetime.now(UTC) + refresh_duration, absolute_expiration) From c92e71469a052191f55c314878ffbb90c01bd3c0 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 29 Nov 2024 15:11:24 +0000 Subject: [PATCH 20/22] Use production header colour Changes the header colour to the blue we use in production to avoid confusion for any participants who already use Notify. --- app/config.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/config.py b/app/config.py index f062c12aa4..923a9f7af3 100644 --- a/app/config.py +++ b/app/config.py @@ -37,8 +37,7 @@ class Config: INVITATION_EXPIRY_SECONDS = 3600 * 24 * 2 # 2 days - also set on api EMAIL_2FA_EXPIRY_SECONDS = 1800 # 30 Minutes - # mix(govuk-colour("dark-grey"), govuk-colour("mid-grey")) - HEADER_COLOUR = os.environ.get("HEADER_COLOUR", "#81878b") + HEADER_COLOUR = "#1d70b8" HTTP_PROTOCOL = os.environ.get("HTTP_PROTOCOL", "http") NOTIFY_APP_NAME = "admin" NOTIFY_LOG_LEVEL = "DEBUG" From bfa924f29b997b227b94b49b4736479f401ee629 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 25 Feb 2025 16:47:57 +0000 Subject: [PATCH 21/22] Refactor to inherit from InsensitiveDict Means less manual munging of keys --- app/models/template_attachment.py | 39 +++++++++++++++++++------------ app/utils/templates.py | 6 +---- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py index bbda21036f..4835b0fff5 100644 --- a/app/models/template_attachment.py +++ b/app/models/template_attachment.py @@ -2,11 +2,18 @@ import json from notifications_utils.insensitive_dict import InsensitiveDict +from notifications_utils.insensitive_dict import InsensitiveSet as UtilsInsensitiveSet from app.extensions import redis_client from app.models import JSONModel +# Implements https://github.com/alphagov/notifications-utils/pull/1197/files +class InsensitiveSet(UtilsInsensitiveSet): + def __contains__(self, key): + return key in InsensitiveDict.from_keys(self) + + class TemplateAttachment(JSONModel): file_name: str weeks_of_retention: int @@ -32,39 +39,41 @@ def __bool__(self): return bool(self.file_name) -class TemplateAttachments: +class TemplateAttachments(InsensitiveDict): BASE_URL = "https://www.download.example.gov.uk/f/" def __init__(self, template): self._template = template - self._dict = InsensitiveDict(json.loads(redis_client.get(self.cache_key) or "{}")) + super().__init__(json.loads(redis_client.get(self.cache_key) or "{}")) @property def cache_key(self): return f"template-{self._template.id}-attachments" def __getitem__(self, placeholder_name): - if placeholder_name not in self._dict: - self._dict[placeholder_name] = { + if placeholder_name not in self: + self[placeholder_name] = { "file_name": None, "weeks_of_retention": 26, "email_confirmation": True, } - return TemplateAttachment(self._dict[placeholder_name], parent=self, placeholder_name=placeholder_name) + return TemplateAttachment( + super().__getitem__(placeholder_name), + parent=self, + placeholder_name=placeholder_name, + ) def __setitem__(self, placeholder_name, value): - self._dict[InsensitiveDict.make_key(placeholder_name)] = value - redis_client.set(self.cache_key, json.dumps(self._dict)) + super().__setitem__(placeholder_name, value) + redis_client.set(self.cache_key, json.dumps(self)) def __delitem__(self, placeholder_name): - self._dict.pop(InsensitiveDict.make_key(placeholder_name)) - redis_client.set(self.cache_key, json.dumps(self._dict)) + super().__delitem__(InsensitiveDict.make_key(placeholder_name)) + redis_client.set(self.cache_key, json.dumps(self)) @property def count(self): - return sum( - bool(self[key]) for key in self._dict if key in InsensitiveDict.from_keys(self._template.all_placeholders) - ) + return sum(bool(self[key]) for key in self if key in InsensitiveSet(self._template.all_placeholders)) @property def as_personalisation(self): @@ -72,10 +81,10 @@ def as_personalisation(self): placeholder: f"{self.BASE_URL}{base64.b64encode(attachment['file_name'].encode()).decode()}" if attachment["file_name"] else None - for placeholder, attachment in self._dict.items() + for placeholder, attachment in self.items() } def prune_orphans(self): - for placeholder in self._dict.keys(): - if placeholder not in self._template.all_placeholders: + for placeholder in self.keys(): + if placeholder not in InsensitiveSet(self._template.all_placeholders): del self[placeholder] diff --git a/app/utils/templates.py b/app/utils/templates.py index 1ecdd6d3b7..e8a8005031 100644 --- a/app/utils/templates.py +++ b/app/utils/templates.py @@ -301,11 +301,7 @@ def all_placeholders(self): @property def placeholders(self): - return OrderedSet( - placeholder - for placeholder in self.all_placeholders - if InsensitiveDict.make_key(placeholder) not in self.attachments.as_personalisation.keys() - ) + return OrderedSet(placeholder for placeholder in self.all_placeholders if placeholder not in self.attachments) class LetterAttachment(JSONModel): From 8e0ca0731005b72dcee4f2a6c3b5dcdff42399f4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 26 Feb 2025 14:33:56 +0000 Subject: [PATCH 22/22] Encapsulate URL generation in single item --- app/models/template_attachment.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/app/models/template_attachment.py b/app/models/template_attachment.py index 4835b0fff5..98025a306a 100644 --- a/app/models/template_attachment.py +++ b/app/models/template_attachment.py @@ -15,6 +15,8 @@ def __contains__(self, key): class TemplateAttachment(JSONModel): + BASE_URL = "https://www.download.example.gov.uk/f/" + file_name: str weeks_of_retention: int email_confirmation: bool @@ -38,10 +40,14 @@ def __setattr__(self, name, value): def __bool__(self): return bool(self.file_name) + @property + def url(self): + if not self.file_name: + return + return f"{self.BASE_URL}{base64.b64encode(self.file_name.encode()).decode()}" + class TemplateAttachments(InsensitiveDict): - BASE_URL = "https://www.download.example.gov.uk/f/" - def __init__(self, template): self._template = template super().__init__(json.loads(redis_client.get(self.cache_key) or "{}")) @@ -77,12 +83,7 @@ def count(self): @property def as_personalisation(self): - return { - placeholder: f"{self.BASE_URL}{base64.b64encode(attachment['file_name'].encode()).decode()}" - if attachment["file_name"] - else None - for placeholder, attachment in self.items() - } + return {placeholder: self[placeholder].url for placeholder in self} def prune_orphans(self): for placeholder in self.keys():