From ee991d01424e22d9cb959be79e78be9400be3495 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 23 Nov 2018 16:29:21 +0000 Subject: [PATCH] Allow any sub-items to be moved from a folder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since you can now see them when searching you should also be able to select and move them. Which means that they needed to be included in the `Form`’s list of possible choices of things to move. --- app/main/forms.py | 22 ++++-------- app/main/views/templates.py | 7 ++-- app/models/template_list.py | 4 +++ tests/app/main/views/test_template_folders.py | 4 +++ tests/app/main/views/test_templates.py | 34 +++++++++++++++++++ 5 files changed, 54 insertions(+), 17 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index b5299c0cb..b4ddb4b6a 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1143,8 +1143,8 @@ class TemplateAndFoldersSelectionForm(Form): def __init__( self, - service, - template_type, + all_template_folders, + template_list, current_folder_id, *args, **kwargs @@ -1152,20 +1152,12 @@ class TemplateAndFoldersSelectionForm(Form): super().__init__(*args, **kwargs) - self.templates_and_folders.choices = self.ids_and_names( - service.get_template_folders_and_templates(template_type, current_folder_id) - ) + self.templates_and_folders.choices = template_list.as_id_and_name - self.move_to.choices = self.ids_and_names( - [self.ALL_TEMPLATES_FOLDER] + service.all_template_folders, - exclude=current_folder_id, - ) - - @staticmethod - def ids_and_names(items, exclude=None): - return [ - (item['id'], item['name']) for item in items - if item['id'] != str(exclude) + self.move_to.choices = [ + (item['id'], item['name']) + for item in ([self.ALL_TEMPLATES_FOLDER] + all_template_folders) + if item['id'] != str(current_folder_id) ] templates_and_folders = MultiCheckboxField('Choose templates or folders') diff --git a/app/main/views/templates.py b/app/main/views/templates.py index fc49bbb08..4b5c43f85 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -109,8 +109,11 @@ def start_tour(service_id, template_id): @user_has_permissions() def choose_template(service_id, template_type='all', template_folder_id=None): + template_list = TemplateList(current_service, template_type, template_folder_id) + templates_and_folders_form = TemplateAndFoldersSelectionForm( - service=current_service, + all_template_folders=current_service.all_template_folders, + template_list=template_list, template_type=template_type, current_folder_id=template_folder_id, ) @@ -127,7 +130,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_list=TemplateList(current_service, template_type, template_folder_id), + template_list=template_list, 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 index 0b86ff1d3..329ceab1f 100644 --- a/app/models/template_list.py +++ b/app/models/template_list.py @@ -44,6 +44,10 @@ class TemplateList(): ancestors=ancestors, ) + @property + def as_id_and_name(self): + return [(item.id, item.name) for item in self] + @property def templates_to_show(self): return any(self) diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 84ec435ba..2e951a070 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -378,7 +378,11 @@ def test_should_show_templates_folder_page( assert links_in_page[index].text.strip() == expected_link all_page_items = page.select('.template-list-item') + checkboxes = page.select('input[name=templates_and_folders]') + unique_checkbox_values = set(item['value'] for item in checkboxes) assert len(all_page_items) == len(expected_items) + assert len(checkboxes) == len(expected_items) + assert len(unique_checkbox_values) == len(expected_items) for index, expected_item in enumerate(expected_items): assert normalize_spaces(all_page_items[index].text) == expected_item diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index e9632f38c..28f623382 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -365,6 +365,40 @@ def test_should_post_move_to_api( ) +def test_should_be_able_to_move_a_sub_item( + client_request, + service_one, + fake_uuid, + mock_get_service_templates, + mock_get_template_folders, + mock_move_to_template_folder, +): + service_one['permissions'] += ['edit_folders'] + GRANDCHILD_FOLDER_ID = str(uuid.uuid4()) + mock_get_template_folders.return_value = [ + {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None}, + {'id': CHILD_FOLDER_ID, 'name': 'folder_one_one', 'parent_id': PARENT_FOLDER_ID}, + {'id': GRANDCHILD_FOLDER_ID, 'name': 'folder_one_one_one', 'parent_id': CHILD_FOLDER_ID}, + ] + client_request.post( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + template_folder_id=PARENT_FOLDER_ID, + _data={ + 'operation': 'move', + 'move_to': 'None', + 'templates_and_folders': [GRANDCHILD_FOLDER_ID], + }, + _expected_status=302, + ) + mock_move_to_template_folder.assert_called_once_with( + service_id=SERVICE_ONE_ID, + folder_id=None, + folder_ids={GRANDCHILD_FOLDER_ID}, + template_ids=set(), + ) + + @pytest.mark.parametrize('thing_to_move', [ PARENT_FOLDER_ID, # Can’t move a folder inside itself CHILD_FOLDER_ID, # Can’t move a folder which doesn’t belong to the service