diff --git a/app/main/views/conversation.py b/app/main/views/conversation.py index 65016bca3..2916c66de 100644 --- a/app/main/views/conversation.py +++ b/app/main/views/conversation.py @@ -1,5 +1,5 @@ from flask import jsonify, redirect, render_template, session, url_for -from flask_login import login_required +from flask_login import current_user, login_required from notifications_python_client.errors import HTTPError from notifications_utils.recipients import format_phone_number_human_readable from notifications_utils.template import SMSPreviewTemplate @@ -44,7 +44,7 @@ def conversation_reply( service_id, notification_id, ): - templates = current_service.get_templates('sms') + templates = current_service.get_user_templates_across_folders(current_user.id, template_type='sms') return render_template( 'views/templates/choose-reply.html', templates=templates, diff --git a/app/models/service.py b/app/models/service.py index 844bcaaae..f37dfea8d 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -154,9 +154,11 @@ class Service(): return {template['id'] for template in self.all_templates} def get_templates_for_folder(self, template_type, all_templates, folder_id): + if isinstance(template_type, str): + template_type = [template_type] return [ template for template in all_templates - if (set([template_type]) & {'all', template['template_type']}) + if (set(template_type) & {'all', template['template_type']}) and (template.get('folder') == folder_id) ] @@ -176,10 +178,10 @@ class Service(): folders = self.all_template_folders all_templates = self.all_templates user_templates = [] + user_templates += self.get_templates_for_folder(template_type, all_templates, None) for folder in folders: if user_id in folder.get("users_with_permission", []): user_templates += self.get_templates_for_folder(template_type, all_templates, folder["id"]) - user_templates += self.get_templates_for_folder(template_type, all_templates, None) return user_templates @property diff --git a/tests/app/main/views/test_conversation.py b/tests/app/main/views/test_conversation.py index adbbcae45..59b1be442 100644 --- a/tests/app/main/views/test_conversation.py +++ b/tests/app/main/views/test_conversation.py @@ -10,10 +10,14 @@ from notifications_python_client.errors import HTTPError from app.main.views.conversation import get_user_number from tests.conftest import ( SERVICE_ONE_ID, + _template, mock_get_notifications, normalize_spaces, ) +INV_PARENT_FOLDER_ID = '7e979e79-d970-43a5-ac69-b625a8d147b0' +VIS_PARENT_FOLDER_ID = 'bbbb222b-2b22-2b22-222b-b222b22b2222' + def test_get_user_phone_number_when_only_inbound_exists(mocker): @@ -255,6 +259,7 @@ def test_conversation_reply_shows_link_to_add_templates_if_service_has_no_templa client_request, fake_uuid, mock_get_service_templates_when_no_templates_exist, + mock_get_template_folders, ): page = client_request.get( 'main.conversation_reply', @@ -275,8 +280,32 @@ def test_conversation_reply_shows_link_to_add_templates_if_service_has_no_templa def test_conversation_reply_shows_templates( client_request, fake_uuid, - mock_get_service_templates, + mocker, + mock_get_template_folders, + active_user_with_permissions ): + + all_templates = {'data': [ + _template('sms', 'sms_template_one', parent=INV_PARENT_FOLDER_ID), + _template('sms', 'sms_template_two'), + _template('sms', 'sms_template_three', parent=VIS_PARENT_FOLDER_ID), + _template('letter', 'letter_template_one') + ]} + mocker.patch('app.service_api_client.get_service_templates', return_value=all_templates) + mock_get_template_folders.return_value = [ + { + 'name': "Parent 1 - invisible", + 'id': INV_PARENT_FOLDER_ID, + 'parent_id': None, + 'users_with_permission': [] + }, + { + 'name': "Parent 2 - visible", + 'id': VIS_PARENT_FOLDER_ID, + 'parent_id': None, + 'users_with_permission': [active_user_with_permissions.id] + }, + ] page = client_request.get( 'main.conversation_reply', service_id=SERVICE_ONE_ID, @@ -284,8 +313,8 @@ def test_conversation_reply_shows_templates( ) for index, expected in enumerate([ - 'sms_template_one', 'sms_template_two', + 'sms_template_three' ]): link = page.select('.message-name')[index] assert normalize_spaces(link.text) == expected @@ -303,6 +332,7 @@ def test_conversation_reply_shows_live_search_if_list_of_templates_taller_than_s client_request, fake_uuid, mock_get_more_service_templates_than_can_fit_onscreen, + mock_get_template_folders, ): page = client_request.get( 'main.conversation_reply', @@ -317,6 +347,7 @@ def test_conversation_reply_shows_live_search_if_list_of_templates_fits_onscreen client_request, fake_uuid, mock_get_service_templates, + mock_get_template_folders, ): page = client_request.get( 'main.conversation_reply', diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 1b0db751a..df15bef50 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -7,6 +7,7 @@ from notifications_python_client.errors import HTTPError from tests.conftest import ( SERVICE_ONE_ID, TEMPLATE_ONE_ID, + _template, active_caseworking_user, active_user_view_permissions, active_user_with_permissions, @@ -31,15 +32,6 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None): } -def _template(template_type, name, parent=None, template_id=None): - return { - 'id': template_id or str(uuid.uuid4()), - 'name': name, - 'template_type': template_type, - 'folder': parent, - } - - @pytest.mark.parametrize( ( 'expected_title_tag,' diff --git a/tests/app/models/test_service.py b/tests/app/models/test_service.py index bd7ce2d1f..8abdd21c3 100644 --- a/tests/app/models/test_service.py +++ b/tests/app/models/test_service.py @@ -1,6 +1,7 @@ import uuid from app.models.service import Service +from tests.conftest import _template INV_PARENT_FOLDER_ID = '7e979e79-d970-43a5-ac69-b625a8d147b0' INV_CHILD_1_FOLDER_ID = '92ee1ee0-e4ee-4dcc-b1a7-a5da9ebcfa2b' @@ -67,15 +68,6 @@ def _get_all_folders(active_user_with_permissions): ] -def _template(template_type, name, parent=None, template_id=None): - return { - 'id': template_id or str(uuid.uuid4()), - 'name': name, - 'template_type': template_type, - 'folder': parent, - } - - def test_get_user_template_folders_only_returns_folders_visible_to_user( mock_get_template_folders, service_one, @@ -201,12 +193,13 @@ def test_get_user_templates_across_folders( service = Service(service_one) result = service.get_user_templates_across_folders(active_user_with_permissions.id) assert result == [ - {'folder': 'bbbb222b-2b22-2b22-222b-b222b22b2222', 'id': mocker.ANY, - 'name': 'email_template_one', 'template_type': 'email'}, {'folder': None, 'id': mocker.ANY, 'name': 'sms_template_two', 'template_type': 'sms'}, {'folder': None, 'id': mocker.ANY, - 'name': 'letter_template_one', 'template_type': 'letter'}] + 'name': 'letter_template_one', 'template_type': 'letter'}, + {'folder': 'bbbb222b-2b22-2b22-222b-b222b22b2222', 'id': mocker.ANY, + 'name': 'email_template_one', 'template_type': 'email'} + ] def test_get_user_templates_across_folders_sms_only( diff --git a/tests/conftest.py b/tests/conftest.py index de4566c59..314f89a81 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,7 +3,7 @@ import os from contextlib import contextmanager from datetime import date, datetime, timedelta from unittest.mock import Mock -from uuid import UUID +from uuid import UUID, uuid4 import pytest from bs4 import BeautifulSoup @@ -986,6 +986,15 @@ def create_service_templates(service_id, number_of_templates=6): return {'data': service_templates} +def _template(template_type, name, parent=None, template_id=None): + return { + 'id': template_id or str(uuid4()), + 'name': name, + 'template_type': template_type, + 'folder': parent, + } + + @pytest.fixture(scope='function') def mock_get_service_templates(mocker): def _create(service_id):