Merge pull request #3648 from Uninett/feat/improve-dashboard-delete-warning

Show confirmation modal when deleting dashboards

Author: Simrayz

changelog.d/3648.added.md

@@ -0,0 +1 @@
+Add confirmation modal when deleting dashboards

python/nav/web/static/js/src/webfront.js

@@ -288,24 +288,6 @@ require([
     }
 
 
-    /** Functions for deleting a dashboard */
-    function addDeleteDashboardListener(feedback) {
-        $('#form-delete-dashboard').submit(function (event) {
-            event.preventDefault();
-            var $this = $(this);
-            var doDelete = confirm('Really delete dashboard and all widgets on it?');
-            if (doDelete) {
-                var request = $.post(this.getAttribute('action'), $this.serialize());
-                request.done(function (response) {
-                    window.location = '/';
-                });
-                request.fail(function (response) {
-                    feedback.addFeedback(response.responseText, 'error');
-                });
-            }
-        });
-    }
-
     /**
      * Load runner - runs on page load
      */
@@ -363,7 +345,6 @@ require([
         addDefaultDashboardListener(feedback);
         addCreateDashboardListener(feedback);
         addRenameDashboardListener(feedback);
-        addDeleteDashboardListener(feedback);
     });
 
 });

python/nav/web/templates/webfront/_dashboard_settings_delete_confirmation.html

@@ -0,0 +1,28 @@
+{% if not error_message %}
+<h4>Delete Dashboard?</h4>
+<p>
+    Are you sure you want to permanently delete the dashboard <em>{{ dashboard.name }}</em> and all its associated navlets?
+</p>
+{% if subscribers_count > 0 %}
+<p data-test-id="subscriber-warning">
+    This dashboard has <strong>{{ subscribers_count }}</strong>
+    {{ subscribers_count|pluralize:"subscriber,subscribers" }}.
+    They will lose access if you proceed.
+</p>
+{% endif %}
+<form hx-post="{% url 'delete-dashboard' dashboard.pk %}">
+    {% csrf_token %}
+    <input type="hidden" name="confirm_delete" value="true">
+    <button class="small alert" type="submit">Yes, delete this dashboard</button>
+    <button class="small secondary" type="button" hx-on:click="htmx.remove(this.closest('.nav-modal'))">
+        Cancel
+    </button>
+</form>
+{% else %}
+<div class="alert-box error">
+    {{ error_message }}
+</div>
+<button class="small secondary" type="button" hx-on:click="htmx.remove(this.closest('.nav-modal'))">
+    Close modal
+</button>
+{% endif %}

python/nav/web/templates/webfront/_dashboard_settings_form.html

@@ -46,10 +46,20 @@
 
             <div class="column medium-4">
                 {% if request.account.account_dashboards.count > 1 and not dashboard.is_default %}
-                <form id="form-delete-dashboard" method="post" action="{% url 'delete-dashboard' dashboard.pk %}">
-                    {% csrf_token %}
-                    <input type="submit" class="small button alert expand" value="Delete dashboard">
-                </form>
+                    <form
+                        hx-post="{% url 'delete-dashboard' dashboard.pk %}"
+                            hx-target="body"
+                            hx-swap="beforeend"
+                    >
+                        {% csrf_token %}
+                        <button
+                                class="small alert expand"
+                                type="submit"
+                                close-popover
+                        >
+                            Delete dashboard
+                        </button>
+                    </form>
                 {% endif %}
 
             </div>

python/nav/web/webfront/views.py

@@ -25,7 +25,6 @@
 from django.db import models
 from django.db.models import Q
 from django.http import (
-    HttpResponseBadRequest,
     HttpResponseForbidden,
     HttpResponseRedirect,
     HttpResponse,
@@ -561,20 +560,39 @@ def add_dashboard(request):
 
 @require_POST
 def delete_dashboard(request, did):
-    """Delete this dashboard and all widgets on it"""
+    """Delete this dashboard and all widgets on it, with confirmation modal"""
     account = get_account(request)
-    is_last = AccountDashboard.objects.filter(account=account).count() == 1
-    if is_last:
-        return HttpResponseBadRequest('Cannot delete last dashboard')
+    dashboard = get_object_or_404(AccountDashboard, pk=did, account=account)
 
-    dash = get_object_or_404(AccountDashboard, pk=did, account=account)
+    is_last = AccountDashboard.objects.filter(account=account).count() == 1
+    if is_last or dashboard.is_default:
+        error_message = (
+            "Cannot delete last dashboard"
+            if is_last
+            else "Cannot delete default dashboard"
+        )
+        return render_modal(
+            request,
+            'webfront/_dashboard_settings_delete_confirmation.html',
+            context={'error_message': error_message},
+            modal_id='delete-dashboard-confirmation',
+            size='small',
+        )
 
-    if dash.is_default:
-        return HttpResponseBadRequest('Cannot delete default dashboard')
+    confirm_delete = request.POST.get("confirm_delete", None) == "true"
+    if confirm_delete:
+        dashboard.delete()
+        return HttpResponseClientRedirect(reverse('webfront-index'))
 
-    dash.delete()
+    subscribers_count = dashboard.subscribers.count()
 
-    return HttpResponse('Dashboard deleted')
+    return render_modal(
+        request,
+        'webfront/_dashboard_settings_delete_confirmation.html',
+        context={'dashboard': dashboard, 'subscribers_count': subscribers_count},
+        modal_id='delete-dashboard-confirmation',
+        size='small',
+    )
 
 
 @require_POST

tests/integration/web/webfront_test.py

@@ -81,39 +81,121 @@ def test_set_default_dashboard_with_multiple_previous_defaults_should_succeed(
     )
 
 
-def test_delete_last_dashboard_should_fail(db, client, admin_account):
-    """Tests that the last dashboard cannot be deleted"""
-    dashboard = AccountDashboard.objects.get(
-        is_default=True,
-        account=admin_account,
-    )
-    url = reverse("delete-dashboard", args=(dashboard.pk,))
-    response = client.post(url, follow=True)
+class TestDeleteDashboardView:
+    """Tests for the delete_dashboard view which allows deleting dashboards"""
 
-    assert response.status_code == 400
-    assert "Cannot delete last dashboard" in smart_str(response.content)
-    assert AccountDashboard.objects.filter(id=dashboard.id).exists()
+    def test_given_dashboard_when_delete_unconfirmed_then_do_not_delete_dashboard(
+        self, db, client, admin_account
+    ):
+        """
+        Tests that the dashboard is not deleted when deletion is not yet confirmed
+        """
+        dashboard = self._create_dashboard(
+            admin_account,
+            name="to_be_deleted",
+            is_default=False,
+        )
+        url = reverse("delete-dashboard", args=(dashboard.pk,))
+        response = client.post(url)
 
+        assert response.status_code == 200
+        assert AccountDashboard.objects.filter(id=dashboard.id).exists()
 
-def test_delete_default_dashboard_should_fail(db, client, admin_account):
-    """Tests that the default dashboard cannot be deleted"""
-    # Creating another dashboard, so that default is not the last one
-    AccountDashboard.objects.create(
-        name="non_default",
-        is_default=False,
-        account=admin_account,
-    )
+    def test_given_dashboard_id_when_delete_unconfirmed_then_render_confirmation(
+        self, db, client, admin_account
+    ):
+        """
+        Tests that the deletion confirmation is rendered when deletion is unconfirmed
+        """
+        dashboard = self._create_dashboard(
+            admin_account,
+            name="to_be_deleted",
+            is_default=False,
+        )
+        url = reverse("delete-dashboard", args=(dashboard.pk,))
+        response = client.post(url)
 
-    default_dashboard = AccountDashboard.objects.get(
-        is_default=True,
-        account=admin_account,
-    )
-    url = reverse("delete-dashboard", args=(default_dashboard.pk,))
-    response = client.post(url, follow=True)
+        assert response.status_code == 200
+        assert 'id="delete-dashboard-confirmation"' in smart_str(response.content)
+
+    def test_given_dashboard_with_subscribers_then_render_subscriber_warning(
+        self, db, client, admin_account, non_admin_account
+    ):
+        """Tests that the deletion confirmation shows subscriber info when applicable"""
+        dashboard = self._create_dashboard(
+            admin_account,
+            is_shared=True,
+        )
+        AccountDashboardSubscription.objects.create(
+            account=non_admin_account,
+            dashboard=dashboard,
+        )
+        url = reverse("delete-dashboard", args=(dashboard.pk,))
+        response = client.post(url)
+        response_content = smart_str(response.content)
+
+        assert response.status_code == 200
+        assert 'data-test-id="subscriber-warning"' in response_content
+
+    def test_given_dashboard_when_delete_confirmed_then_delete_dashboard(
+        self, db, client, admin_account
+    ):
+        """Tests that an existing dashboard can be deleted"""
+        dashboard = self._create_dashboard(admin_account)
+        url = reverse("delete-dashboard", args=(dashboard.pk,))
+        response = client.post(
+            url,
+            data={"confirm_delete": "true"},
+            follow=True,
+        )
+
+        assert response.status_code == 200
+        assert not AccountDashboard.objects.filter(id=dashboard.id).exists()
+
+    def test_given_last_dashboard_of_account_then_render_error_message(
+        self, db, client, admin_account
+    ):
+        """Tests that the last dashboard cannot be deleted"""
 
-    assert response.status_code == 400
-    assert "Cannot delete default dashboard" in smart_str(response.content)
-    assert AccountDashboard.objects.filter(id=default_dashboard.id).exists()
+        # Ensure only one dashboard exists for the account
+        AccountDashboard.objects.filter(account=admin_account).delete()
+        dashboard = self._create_dashboard(admin_account, name="last_dashboard")
+
+        url = reverse("delete-dashboard", args=(dashboard.pk,))
+        response = client.post(
+            url,
+        )
+
+        assert response.status_code == 200
+        assert "Cannot delete last dashboard" in smart_str(response.content)
+        assert AccountDashboard.objects.filter(id=dashboard.id).exists()
+
+    def test_given_default_dashboard_of_account_then_render_error_message(
+        self, db, client, admin_account
+    ):
+        """Tests that the default dashboard cannot be deleted"""
+        # Ensure at least one non-default dashboard exists
+        self._create_dashboard(admin_account, name="non_default_dashboard")
+        default_dashboard = AccountDashboard.objects.get(
+            is_default=True, account=admin_account
+        )
+        url = reverse("delete-dashboard", args=(default_dashboard.pk,))
+        response = client.post(url)
+
+        assert response.status_code == 200
+        assert "Cannot delete default dashboard" in smart_str(response.content)
+        assert AccountDashboard.objects.filter(id=default_dashboard.id).exists()
+
+    @staticmethod
+    def _create_dashboard(
+        account, name="to_be_deleted", is_default=False, is_shared=False
+    ):
+        return AccountDashboard.objects.create(
+            name=name,
+            is_default=is_default,
+            account=account,
+            is_shared=is_shared,
+        )
 
 
 def test_when_logging_in_it_should_change_the_session_id(