From 82d82076122025eab727e7fe6af1e0e29fbdf5cd Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 21 Nov 2018 09:53:29 +0000 Subject: [PATCH] Refactor template list logic into one place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Jinja template for the ‘choose templates’ page is now pulling in data from a lot of diparate places in order to work out what to show. As we add more logic about what to show (in order to make the live search work) it’s going to get harder to have all this logic in the Jinja template. This commit refactors it back into Python where we have more language features for managing complex logic. It’s a bit weird to call this file a model, in that it’s dealing with some presentational logic, rather than just data. Conceptually it’s more like a view model[1]. 1. https://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93viewmodel --- app/main/views/templates.py | 5 +- app/models/template_list.py | 120 ++++++++++++++++++ .../components/message-count-label.html | 23 ---- app/templates/views/templates/_move_to.html | 2 +- .../views/templates/_template_list.html | 50 +++----- app/templates/views/templates/choose.html | 24 +--- tests/app/main/views/test_template_folders.py | 15 ++- tests/app/main/views/test_templates.py | 2 +- 8 files changed, 159 insertions(+), 82 deletions(-) create mode 100644 app/models/template_list.py diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 6fc6e6dc9..fc49bbb08 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -29,6 +29,7 @@ from app.main.forms import ( ) from app.main.views.send import get_example_csv_rows, get_sender_details from app.models.service import Service +from app.models.template_list import TemplateList from app.template_previews import TemplatePreview, get_page_count_for_letter from app.utils import ( email_or_sms_not_enabled, @@ -126,9 +127,7 @@ def choose_template(service_id, template_type='all', template_folder_id=None): current_template_folder_id=template_folder_id, can_manage_folders=can_manage_folders(), template_folder_path=current_service.get_template_folder_path(template_folder_id), - template_folder_has_contents=current_service.get_template_folders_and_templates('all', template_folder_id), - template_folders=current_service.get_template_folders(template_type, template_folder_id), - templates=current_service.get_templates(template_type, template_folder_id), + template_list=TemplateList(current_service, template_type, template_folder_id), show_search_box=current_service.count_of_templates_and_folders > 7, show_template_nav=( current_service.has_multiple_template_types diff --git a/app/models/template_list.py b/app/models/template_list.py new file mode 100644 index 000000000..0b86ff1d3 --- /dev/null +++ b/app/models/template_list.py @@ -0,0 +1,120 @@ +class TemplateList(): + + def __init__( + self, + service, + template_type='all', + template_folder_id=None, + ): + self.service = service + self.template_type = template_type + self.template_folder_id = template_folder_id + + def __iter__(self): + for item in self.get_templates_and_folders( + self.template_type, self.template_folder_id, ancestors=[] + ): + yield item + + def get_templates_and_folders(self, template_type, template_folder_id, ancestors): + + for item in self.service.get_template_folders( + template_type, template_folder_id + ): + yield TemplateListFolder( + item, + folders=self.service.get_template_folders( + self.template_type, item['id'] + ), + templates=self.service.get_templates( + self.template_type, item['id'] + ), + ancestors=ancestors, + ) + for sub_item in self.get_templates_and_folders( + template_type, item['id'], ancestors + [item] + ): + yield sub_item + + for item in self.service.get_templates( + self.template_type, template_folder_id + ): + yield TemplateListTemplate( + item, + ancestors=ancestors, + ) + + @property + def templates_to_show(self): + return any(self) + + @property + def folder_is_empty(self): + return not any(self.get_templates_and_folders( + 'all', self.template_folder_id, [] + )) + + +class TemplateListItem(): + + def __init__( + self, + template_or_folder, + ancestors, + ): + self.id = template_or_folder['id'] + self.name = template_or_folder['name'] + self.ancestors = ancestors + + +class TemplateListTemplate(TemplateListItem): + + is_folder = False + + def __init__( + self, + template, + ancestors, + ): + super().__init__(template, ancestors) + self.hint = { + 'email': 'Email template', + 'sms': 'Text message template', + 'letter': 'Letter template', + }.get(template['template_type']) + + +class TemplateListFolder(TemplateListItem): + + is_folder = True + + def __init__( + self, + folder, + templates, + folders, + ancestors, + ): + super().__init__(folder, ancestors) + self.number_of_templates = len(templates) + self.number_of_folders = len(folders) + + @property + def _hint_parts(self): + + if self.number_of_folders == self.number_of_templates == 0: + yield 'Empty' + + if self.number_of_templates == 1: + yield '1 template' + elif self.number_of_templates > 1: + yield '{} templates'.format(self.number_of_templates) + + if self.number_of_folders == 1: + yield '1 folder' + elif self.number_of_folders > 1: + yield '{} folders'.format(self.number_of_folders) + + @property + def hint(self): + return ', '.join(self._hint_parts) diff --git a/app/templates/components/message-count-label.html b/app/templates/components/message-count-label.html index 0e5cfeedc..14b997665 100644 --- a/app/templates/components/message-count-label.html +++ b/app/templates/components/message-count-label.html @@ -55,26 +55,3 @@ {%- endif -%} {%- endif %} {%- endmacro %} - - -{% macro folder_contents_count(number_of_folders, number_of_templates) %} - - {% if number_of_folders == number_of_templates == 0 %} - Empty - {% endif %} - - {% if number_of_templates == 1 %} - {{ number_of_templates }} template - {%- elif number_of_templates > 1 -%} - {{ number_of_templates }} templates - {%- endif -%} - - {%- if number_of_folders and number_of_templates %}, {% endif -%} - - {%- if number_of_folders == 1 -%} - {{ number_of_folders }} folder - {%- elif number_of_folders > 1 -%} - {{ number_of_folders }} folders - {% endif %} - -{%- endmacro %} diff --git a/app/templates/views/templates/_move_to.html b/app/templates/views/templates/_move_to.html index 4b9564c3d..10ab965a2 100644 --- a/app/templates/views/templates/_move_to.html +++ b/app/templates/views/templates/_move_to.html @@ -1,7 +1,7 @@ {% from "components/radios.html" import radios %} {% from "components/page-footer.html" import page_footer %} -{% if templates_and_folders_form.move_to.choices and (templates or template_folders) %} +{% if templates_and_folders_form.move_to.choices and template_list.templates_to_show %} {{ radios(templates_and_folders_form.move_to) }} {{ page_footer('Move selected', button_name='operation', button_value='move') }} {% endif %} diff --git a/app/templates/views/templates/_template_list.html b/app/templates/views/templates/_template_list.html index 3c1cb7555..53439904a 100644 --- a/app/templates/views/templates/_template_list.html +++ b/app/templates/views/templates/_template_list.html @@ -1,52 +1,38 @@ {% from "components/checkbox.html" import unlabelled_checkbox %} {% from "components/message-count-label.html" import folder_contents_count, message_count_label %} -{% if service_has_templates_or_folders and not templates and not template_folders %} +{% if not template_list.templates_to_show %}

- {% if template_folder_has_contents %} - There are no {{ message_count_label(1, template_type, suffix='') }} templates in this folder - {% else %} + {% if template_list.folder_is_empty %} This folder is empty + {% else %} + There are no {{ message_count_label(1, template_type, suffix='') }} templates in this folder {% endif %}

{% else %}