Skip to content

Commit

Permalink
Add confirmation before deletion (#289)
Browse files Browse the repository at this point in the history
Prompt user for confirmation before deleting storage service
or fetch job.
  • Loading branch information
mcantelon committed Nov 22, 2024
1 parent 911875f commit 0290b7a
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 3 deletions.
13 changes: 13 additions & 0 deletions AIPscan/Aggregator/templates/confirm.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{% extends "base.html" %}

{% block content %}
<div class="text-center align-middle" style="margin-top: 15%">
<p>{{ prompt }}</p>

<form method="GET" action="{{ request.url }}">
<input type="hidden" name="confirm" value="1" />
<input type="submit" value="{{ action }}" class='btn btn-danger' />
<a href="{{ url_for(cancel_route) }}" class='btn btn-default'>Cancel</a>
</form>
</div>
{% endblock %}
15 changes: 13 additions & 2 deletions AIPscan/Aggregator/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,32 @@ def test_edit_storage_service(app_with_populated_files):

def test_delete_storage_service(app_with_populated_files, mocker):
with current_app.test_client() as test_client:
# The delete confirmation page should not show up for a non-existent storage service
response = test_client.get("/aggregator/delete_storage_service/0")
assert response.status_code == 404

# Deletion should not be attempted for a non-existent storage service
response = test_client.get("/aggregator/delete_storage_service/0?confirm=1")
assert response.status_code == 404

# Don't actually instantiate Celery job
mocker.patch("AIPscan.Aggregator.tasks.delete_storage_service.delay")

# The delete confirmation page should show up for an existing storage service
response = test_client.get("/aggregator/delete_storage_service/1")
assert response.status_code == 200

# Deletion should be attempted if the storage service exists
response = test_client.get("/aggregator/delete_storage_service/1?confirm=1")
assert response.status_code == 302


def test_delete_fetch_job(app_with_populated_files, mocker):
with current_app.test_client() as test_client:
response = test_client.get("/aggregator/delete_fetch_job/0")
response = test_client.get("/aggregator/delete_fetch_job/0?confirm=1")
assert response.status_code == 404

mocker.patch("AIPscan.Aggregator.tasks.delete_fetch_job.delay")

response = test_client.get("/aggregator/delete_fetch_job/1")
response = test_client.get("/aggregator/delete_fetch_job/1?confirm=1")
assert response.status_code == 302
16 changes: 15 additions & 1 deletion AIPscan/Aggregator/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
url_for,
)

from AIPscan import db, typesense_helpers
from AIPscan import db, decorators, typesense_helpers
from AIPscan.Aggregator import database_helpers, tasks
from AIPscan.Aggregator.forms import StorageServiceForm
from AIPscan.Aggregator.task_helpers import (
Expand Down Expand Up @@ -171,6 +171,13 @@ def new_storage_service():


@aggregator.route("/delete_storage_service/<storage_service_id>", methods=["GET"])
@decorators.confirm_required(
StorageService,
"storage_service_id",
"Are you sure you'd like to delete this storage service?",
"Delete",
"aggregator.ss_default",
)
def delete_storage_service(storage_service_id):
storage_service = StorageService.query.get(storage_service_id)

Expand Down Expand Up @@ -241,6 +248,13 @@ def new_fetch_job(fetch_job_id):


@aggregator.route("/delete_fetch_job/<fetch_job_id>", methods=["GET"])
@decorators.confirm_required(
FetchJob,
"fetch_job_id",
"Are you sure you'd like to delete this fetch job?",
"Delete",
"aggregator.ss_default",
)
def delete_fetch_job(fetch_job_id):
fetch_job = FetchJob.query.get(fetch_job_id)

Expand Down
27 changes: 27 additions & 0 deletions AIPscan/decorators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from functools import wraps

from flask import abort, render_template, request


def confirm_required(model, id_argument, prompt, action, cancel_route):
def decorator(f):
@wraps(f)
def decorated_function(*args, **kwargs):
instance = model.query.get(kwargs[id_argument])

if not instance:
abort(404)

if request.args.get("confirm"):
return f(*args, **kwargs)

return render_template(
"confirm.html",
prompt=prompt,
action=action,
cancel_route=cancel_route,
)

return decorated_function

return decorator

0 comments on commit 0290b7a

Please sign in to comment.