diff --git a/airlock/templates/file_browser/_includes/directory_table/request.html b/airlock/templates/file_browser/_includes/directory_table/request.html index f9ffca63..38a20f31 100644 --- a/airlock/templates/file_browser/_includes/directory_table/request.html +++ b/airlock/templates/file_browser/_includes/directory_table/request.html @@ -15,6 +15,11 @@
Modified{% datatable_sort_icon %}
+ {% if multiselect_withdraw %} + + + + {% endif %} {% /table_head %} @@ -24,6 +29,13 @@ {{ path.request_filetype.name|title }} {{ path.size_mb }} {{ path.modified_at|date:"Y-m-d H:i" }} + {% if multiselect_withdraw %} + + {% if not path.is_directory %} + {% form_checkbox name="selected" value=path.relpath custom_field=True %} + {% endif %} + + {% endif %} {% endfor %} diff --git a/airlock/templates/file_browser/directory.html b/airlock/templates/file_browser/directory.html index d4f8ec2e..d6257fa3 100644 --- a/airlock/templates/file_browser/directory.html +++ b/airlock/templates/file_browser/directory.html @@ -40,6 +40,13 @@ {% /button %} {% endif %} {% endif %} + {% elif context == "request" and multiselect_withdraw %} +
+
+ {% #button type="submit" name="action" value="withdraw_files" variant="warning" form="multiselect_form"%}Withdraw Files from Request{% /button %} +
+
+
{% endif %} {% endfragment %} diff --git a/airlock/urls.py b/airlock/urls.py index 80cc78c7..48bfbb89 100644 --- a/airlock/urls.py +++ b/airlock/urls.py @@ -103,6 +103,11 @@ airlock.views.file_withdraw, name="file_withdraw", ), + path( + "requests/multiselect/", + airlock.views.request_multiselect, + name="request_multiselect", + ), path( "requests/release/", airlock.views.request_release_files, diff --git a/airlock/views/__init__.py b/airlock/views/__init__.py index d7716b62..b867a33a 100644 --- a/airlock/views/__init__.py +++ b/airlock/views/__init__.py @@ -10,6 +10,7 @@ group_edit, request_contents, request_index, + request_multiselect, request_reject, request_release_files, request_return, @@ -37,6 +38,7 @@ "file_withdraw", "request_contents", "request_index", + "request_multiselect", "request_reject", "file_reset_review", "request_release_files", diff --git a/airlock/views/request.py b/airlock/views/request.py index d9dacb75..3f827d04 100644 --- a/airlock/views/request.py +++ b/airlock/views/request.py @@ -18,12 +18,18 @@ from airlock.business_logic import ROOT_PATH, bll from airlock.enums import RequestFileType, RequestFileVote, RequestStatus, Visibility from airlock.file_browser_api import get_request_tree -from airlock.forms import GroupCommentDeleteForm, GroupCommentForm, GroupEditForm +from airlock.forms import ( + GroupCommentDeleteForm, + GroupCommentForm, + GroupEditForm, + MultiselectForm, +) from airlock.types import UrlPath from services.tracing import instrument from .helpers import ( display_form_errors, + display_multiple_messages, download_file, get_path_item_from_tree_or_404, get_release_request_or_raise, @@ -335,8 +341,10 @@ def request_view(request, request_id: str, path: str = ""): "group_activity": group_activity, "show_c3": settings.SHOW_C3, "request_action_required": request_action_required, - # TODO, but for now stops template variable errors - "multiselect_url": "", + "multiselect_url": reverse( + "request_multiselect", kwargs={"request_id": request_id} + ), + "multiselect_withdraw": release_request.is_editing(), "code_url": code_url, "return_url": "", "group_comment_delete_url": group_comment_delete_url, @@ -475,6 +483,39 @@ def file_withdraw(request, request_id, path: str): return redirect(redirect_url) +@instrument(func_attributes={"release_request": "request_id"}) +@require_http_methods(["POST"]) +def request_multiselect(request, request_id: str): + release_request = get_release_request_or_raise(request.user, request_id) + + multiform = MultiselectForm(request.POST) + + if not multiform.is_valid(): + display_form_errors(request, multiform.errors) + else: + action = multiform.cleaned_data["action"] + if action != "withdraw_files": + raise Http404(f"Invalid action {action}") + + errors = [] + successes = [] + for path_str in multiform.cleaned_data["selected"]: + path = UrlPath(path_str) + try: + bll.withdraw_file_from_request(release_request, path, request.user) + successes.append(f"The file {path} has been withdrawn from the request") + except exceptions.RequestPermissionDenied as exc: + errors.append(str(exc)) + + if errors: + display_multiple_messages(request, errors, "error") + if successes: + display_multiple_messages(request, successes, "success") + + url = multiform.cleaned_data["next_url"] + return HttpResponse(headers={"HX-Redirect": url}) + + @instrument def requests_for_workspace(request, workspace_name: str): requests_for_workspace = bll.get_requests_for_workspace( diff --git a/tests/functional/test_e2e.py b/tests/functional/test_e2e.py index 7d7879a2..b08c27c8 100644 --- a/tests/functional/test_e2e.py +++ b/tests/functional/test_e2e.py @@ -509,11 +509,19 @@ def test_e2e_withdraw_and_readd_file(page, live_server, dev_users): login_as(live_server, page, "researcher") - # Withdraw files after they've been reviewd + # Withdraw file1 from the file page page.goto(live_server.url + release_request.get_url("default/subdir/file1.txt")) find_and_click(page.locator("#withdraw-file-button")) - page.goto(live_server.url + release_request.get_url("default/subdir/file2.txt")) - find_and_click(page.locator("#withdraw-file-button")) + expect(page.locator("body")).to_contain_text("has been withdrawn from the request") + + # Withdraw file2 from the directory page + page.goto(live_server.url + release_request.get_url("default/subdir")) + expect(page.locator("#customTable.datatable-table")).to_be_visible() + find_and_click( + page.locator('input[name="selected"][value="default/subdir/file2.txt"]') + ) + find_and_click(page.locator("button[value=withdraw_files]")) + expect(page.locator("body")).to_contain_text("has been withdrawn from the request") # Change our mind on file 2: go to file page and re-add it page.goto(live_server.url + workspace.get_url(path2)) diff --git a/tests/integration/views/test_request.py b/tests/integration/views/test_request.py index 7ed45cd1..d239342e 100644 --- a/tests/integration/views/test_request.py +++ b/tests/integration/views/test_request.py @@ -2,6 +2,7 @@ import pytest import requests +from django.contrib.messages import get_messages from airlock import exceptions from airlock.business_logic import bll @@ -21,6 +22,10 @@ pytestmark = pytest.mark.django_db +def get_messages_text(response): + return "\n".join(m.message for m in get_messages(response.wsgi_request)) + + def test_request_index_no_user(airlock_client): release_request = factories.create_release_request("workspace") response = airlock_client.get(f"/requests/view/{release_request.id}/") @@ -1321,6 +1326,118 @@ def test_file_withdraw_file_bad_request(airlock_client): assert response.status_code == 404 +def test_request_multiselect_withdraw_files(airlock_client): + user = factories.create_user(workspaces=["workspace"]) + release_request = factories.create_request_at_status( + list(user.workspaces)[0], + author=user, + status=RequestStatus.RETURNED, + files=[ + factories.request_file(group="group", path="file1.txt", approved=True), + factories.request_file(group="group", path="file2.txt", approved=True), + ], + ) + + airlock_client.login_with_user(user) + response = airlock_client.post( + f"/requests/multiselect/{release_request.id}", + data={ + "action": "withdraw_files", + "selected": [ + "group/file1.txt", + "group/file2.txt", + ], + "next_url": release_request.get_url(), + }, + ) + persisted_request = factories.refresh_release_request(release_request) + messages = get_messages_text(response) + + for path in ["group/file1.txt", "group/file2.txt"]: + request_file = persisted_request.get_request_file_from_urlpath(path) + assert request_file.filetype == RequestFileType.WITHDRAWN + assert f"{path} has been withdrawn from the request" in messages + + +def test_request_multiselect_withdraw_files_not_permitted(airlock_client): + user = factories.create_user(workspaces=["workspace"]) + release_request = factories.create_request_at_status( + list(user.workspaces)[0], + author=user, + status=RequestStatus.SUBMITTED, + files=[ + factories.request_file(group="group", path="file1.txt", approved=True), + factories.request_file(group="group", path="file2.txt", approved=True), + ], + ) + + airlock_client.login_with_user(user) + response = airlock_client.post( + f"/requests/multiselect/{release_request.id}", + data={ + "action": "withdraw_files", + "selected": [ + "group/file1.txt", + "group/file2.txt", + ], + "next_url": release_request.get_url(), + }, + ) + persisted_request = factories.refresh_release_request(release_request) + messages = get_messages_text(response) + assert "Cannot withdraw file" in messages + + for path in ["group/file1.txt", "group/file2.txt"]: + request_file = persisted_request.get_request_file_from_urlpath(path) + assert request_file.filetype != RequestFileType.WITHDRAWN + + +def test_request_multiselect_withdraw_files_none_selected(airlock_client): + user = factories.create_user(workspaces=["workspace"]) + release_request = factories.create_request_at_status( + list(user.workspaces)[0], + author=user, + status=RequestStatus.RETURNED, + files=[ + factories.request_file(group="group", path="file1.txt", approved=True), + ], + ) + + airlock_client.login_with_user(user) + response = airlock_client.post( + f"/requests/multiselect/{release_request.id}", + data={ + "action": "withdraw_files", + "selected": [], + "next_url": release_request.get_url(), + }, + ) + assert "You must select at least one file" in get_messages_text(response) + + +def test_request_multiselect_withdraw_files_invalid_action(airlock_client): + user = factories.create_user(workspaces=["workspace"]) + release_request = factories.create_request_at_status( + list(user.workspaces)[0], + author=user, + status=RequestStatus.RETURNED, + files=[ + factories.request_file(group="group", path="file1.txt", approved=True), + ], + ) + + airlock_client.login_with_user(user) + response = airlock_client.post( + f"/requests/multiselect/{release_request.id}", + data={ + "action": "prorogate_files", + "selected": ["group/file1.txt"], + "next_url": release_request.get_url(), + }, + ) + assert response.status_code == 404 + + @pytest.mark.parametrize("status", [RequestStatus.REVIEWED, RequestStatus.APPROVED]) def test_request_release_files_success(airlock_client, release_files_stubber, status): airlock_client.login(username="checker", output_checker=True)