From f500db44f1faec291521ead03db81c1429aa4a4f Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Thu, 26 May 2022 13:53:12 +0100 Subject: [PATCH] Reuse TemplateList class when deleting a folder Part of moving "get_template_folders" et al. into TemplateList so we can cache it more effectively. This is slightly less efficient as iterating a TemplateList will instantiate an object for each item in the folder; but the difference is minimal. Note that: - The default template_type for TemplateList is "all". - We need to pass realistic template "JSON" in the test now. --- app/main/views/templates.py | 5 ++--- app/models/service.py | 6 ------ tests/app/main/views/test_template_folders.py | 4 ++-- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index f3939a481..670f167a2 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -470,10 +470,9 @@ def manage_template_folder(service_id, template_folder_id): @user_has_permissions('manage_templates') def delete_template_folder(service_id, template_folder_id): template_folder = current_service.get_template_folder_with_user_permission_or_403(template_folder_id, current_user) + template_list = TemplateList(service=current_service, template_folder_id=template_folder_id) - if len(current_service.get_template_folders_and_templates( - template_type="all", template_folder_id=template_folder_id - )) > 0: + if not template_list.folder_is_empty: flash("You must empty this folder before you can delete it", 'info') return redirect( url_for( diff --git a/app/models/service.py b/app/models/service.py index 3694d8487..161cd2f60 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -650,12 +650,6 @@ class Service(JSONModel, SortByNameMixin): template, ] - def get_template_folders_and_templates(self, template_type, template_folder_id): - return ( - self.get_templates(template_type, template_folder_id) - + self.get_template_folders(template_type, template_folder_id) - ) - @property def count_of_templates_and_folders(self): return len(self.all_templates + self.all_template_folders) diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 1b67611f4..77279e190 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -5,7 +5,7 @@ from flask import abort, url_for from notifications_python_client.errors import HTTPError from app.models.user import User -from tests import sample_uuid +from tests import sample_uuid, template_json from tests.conftest import ( SERVICE_ONE_ID, TEMPLATE_ONE_ID, @@ -968,7 +968,7 @@ def test_delete_template_folder_should_detect_non_empty_folder_on_get( ] mocker.patch( 'app.models.service.Service.get_templates', - return_value=[{'id': template_id, 'name': 'template'}], + return_value=[template_json(service_one['id'], template_id)], ) client_request.get( 'main.delete_template_folder', service_id=service_one['id'],