Add confirmation before deletion (#289)

AIPscan/Aggregator/templates/confirm.html

@@ -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 %}

AIPscan/Aggregator/tests/test_views.py

@@ -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

AIPscan/Aggregator/views.py

@@ -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 (
@@ -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)
 
@@ -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)
 

AIPscan/decorators.py

@@ -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