Merge pull request #2518 from alphagov/delete-folder-refinements

Make some refinements to the delete folder journey

app/main/views/templates.py

@@ -419,16 +419,18 @@ def manage_template_folder(service_id, template_folder_id):
 @login_required
 @user_has_permissions('manage_templates')
 def delete_template_folder(service_id, template_folder_id):
+
     if not current_service.has_permission('edit_folders'):
         abort(403)
-    form = TemplateFolderForm()
+
-    template_folder_path = current_service.get_template_folder_path(template_folder_id)
+    template_folder = current_service.get_template_folder(template_folder_id)
-    template_folder_name = template_folder_path[-1]["name"]
+
+    form = TemplateFolderForm(name=template_folder['name'])
 
     if len(current_service.get_template_folders_and_templates(
         template_type="all", template_folder_id=template_folder_id
     )) > 0:
-        flash("You must empty this folder before you can delete it".format(template_folder_name), 'info')
+        flash("You must empty this folder before you can delete it".format(template_folder['name']), 'info')
         return redirect(
             url_for(
                 '.choose_template', service_id=service_id, template_type="all", template_folder_id=template_folder_id
@@ -440,12 +442,12 @@ def delete_template_folder(service_id, template_folder_id):
             template_folder_api_client.delete_template_folder(current_service.id, template_folder_id)
 
             return redirect(
-                url_for('.choose_template', service_id=service_id)
+                url_for('.choose_template', service_id=service_id, template_folder_id=template_folder['parent_id'])
             )
         except HTTPError as e:
             msg = "Folder is not empty"
             if e.status_code == 400 and msg in e.message:
-                flash("You must empty this folder before you can delete it".format(template_folder_name), 'info')
+                flash("You must empty this folder before you can delete it", 'info')
                 return redirect(
                     url_for(
                         '.choose_template',
@@ -457,15 +459,14 @@ def delete_template_folder(service_id, template_folder_id):
             else:
                 abort(500, e)
 
-    flash("Are you sure you want to delete the ‘{}’ folder?".format(template_folder_name), 'delete')
+    flash("Are you sure you want to delete the ‘{}’ folder?".format(template_folder['name']), 'delete')
     return render_template(
         'views/templates/manage-template-folder.html',
         form=form,
-        template_folder_path=template_folder_path,
+        template_folder_path=current_service.get_template_folder_path(template_folder_id),
         current_service_id=current_service.id,
         template_folder_id=template_folder_id,
         template_type="all",
-        delete_folder=True
     )
 
 

app/templates/views/templates/manage-template-folder.html

@@ -21,20 +21,16 @@
     </div>
   </div>
 
-  {% if not delete_folder %}
+  {% call form_wrapper(action=url_for('main.manage_template_folder', service_id=current_service.id, template_folder_id=template_folder_id)) %}
-    {% call form_wrapper() %}
+    {{ textbox(form.name) }}
-      {{ textbox(form.name) }}
+    {{ page_footer(
-      {{ page_footer(
+      'Save',
-        'Save',
+      delete_link=url_for(
-        delete_link=url_for(
+        '.delete_template_folder',
-          '.delete_template_folder',
+        service_id=current_service_id,
-          service_id=current_service_id,
+        template_folder_id=template_folder_id
-          template_folder_id=template_folder_id
+      ),
-        ),
+      delete_link_text="Delete this folder") }}
-        delete_link_text="Delete this folder") }}
+  {% endcall %}
-    {% endcall %}
-  {% else %}
-    <a href="{{url_for('.manage_template_folder', service_id=current_service.id, template_folder_id=template_folder_id)}}">Back to manage folder page</a>
-  {% endif %}
 
 {% endblock %}

tests/app/main/views/test_template_folders.py

@@ -402,7 +402,7 @@ def test_get_manage_folder_page(
     assert normalize_spaces(page.select_one('title').text) == (
         'folder_two – Templates – service one – GOV.UK Notify'
     )
-    assert page.select_one('input[name=name]') is not None
+    assert page.select_one('input[name=name]')['value'] == 'folder_two'
     delete_link = page.find('a', string="Delete this folder")
     expected_delete_url = "/services/{}/templates/folders/{}/delete".format(service_one['id'], folder_id)
 
@@ -478,9 +478,20 @@ def test_delete_template_folder_should_request_confirmation(
         'Yes, delete'
     )
 
-    assert len(page.select('label')) == 0
+    assert page.select_one('input[name=name]')['value'] == 'sacrifice'
-    assert len(page.select('button')) == 1
+
-    assert "Back to manage folder page" in page.text
+    assert len(page.select('form')) == 2
+    assert len(page.select('button')) == 2
+
+    assert 'action' not in page.select('form')[0]
+    assert page.select('form button')[0].text == 'Yes, delete'
+
+    assert page.select('form')[1]['action'] == url_for(
+        'main.manage_template_folder',
+        service_id=service_one['id'],
+        template_folder_id=folder_id,
+    )
+    assert page.select('form button')[1].text == 'Save'
 
 
 def test_delete_template_folder_should_detect_non_empty_folder_on_get(
@@ -511,11 +522,15 @@ def test_delete_template_folder_should_detect_non_empty_folder_on_get(
     )
 
 
-def test_delete_folder(client_request, service_one, mock_get_template_folders, mocker):
+@pytest.mark.parametrize('parent_folder_id', (
+    None,
+    PARENT_FOLDER_ID,
+))
+def test_delete_folder(client_request, service_one, mock_get_template_folders, mocker, parent_folder_id):
     mock_delete = mocker.patch('app.template_folder_api_client.delete_template_folder')
     folder_id = str(uuid.uuid4())
     mock_get_template_folders.side_effect = [[
-        {'id': folder_id, 'name': 'sacrifice', 'parent_id': None},
+        {'id': folder_id, 'name': 'sacrifice', 'parent_id': parent_folder_id},
     ], []]
     mocker.patch(
         'app.models.service.Service.get_templates',
@@ -527,9 +542,12 @@ def test_delete_folder(client_request, service_one, mock_get_template_folders, m
         'main.delete_template_folder',
         service_id=service_one['id'],
         template_folder_id=folder_id,
-        _expected_redirect=url_for("main.choose_template",
+        _expected_redirect=url_for(
-                                   service_id=service_one['id'],
+            "main.choose_template",
-                                   _external=True)
+            service_id=service_one['id'],
+            template_folder_id=parent_folder_id,
+            _external=True,
+        )
     )
 
     mock_delete.assert_called_once_with(service_one['id'], folder_id)