Merge pull request 'Polish forms and action flows' (#35) from issue-25-forms-action-flows into master

Reviewed-on: #35

src/pobsync_backend/templates/pobsync_backend/base.html

@@ -280,6 +280,17 @@
 	    }
     button.secondary:hover,
     .button-link.secondary:hover { background: #eef3f8; }
+    button.danger,
+    .button-link.danger {
+      background: var(--failed);
+      border-color: var(--failed);
+      color: #fff;
+    }
+    button.danger:hover,
+    .button-link.danger:hover {
+      background: #842828;
+      border-color: #842828;
+    }
     button.compact,
     .button-link.compact {
       font-size: 12px;
@@ -679,6 +690,30 @@
 	    .message.error { border-color: #e8b4b4; background: #fff0f0; color: var(--failed); }
 	    .message.warning { border-color: #e7cf8a; background: #fff8df; color: var(--running); }
 	    .form-grid { display: grid; gap: 15px; max-width: 720px; }
+	    .filter-form {
+	      align-items: end;
+	      display: grid;
+	      gap: 15px;
+	      grid-template-columns: repeat(auto-fit, minmax(180px, 1fr));
+	      max-width: none;
+	    }
+	    .form-actions {
+	      align-items: center;
+	      border-top: 1px solid var(--border);
+	      display: flex;
+	      flex-wrap: wrap;
+	      gap: 10px;
+	      margin-top: 4px;
+	      padding-top: 15px;
+	    }
+	    .form-actions .button-link.secondary { margin-left: auto; }
+	    .filter-form .form-actions {
+	      border-top: 0;
+	      justify-content: flex-end;
+	      margin-top: 0;
+	      padding-top: 0;
+	    }
+	    .filter-form .form-actions .button-link.secondary { margin-left: 0; }
 	    .field { display: grid; gap: 6px; }
 	    .field label { font-weight: 700; }
 	    .field input[type="text"], .field input[type="number"], .field select, .field textarea {
@@ -736,6 +771,7 @@
       .host-control-grid { grid-template-columns: 1fr; }
       .schedule-row { grid-template-columns: 1fr; }
       .schedule-time { justify-items: start; text-align: left; }
+	      .form-actions .button-link.secondary { margin-left: 0; }
 	      .host-card-header { display: grid; }
       .host-card-status { justify-content: flex-start; max-width: none; }
       .host-card-layout { grid-template-columns: 1fr; }

src/pobsync_backend/templates/pobsync_backend/global_form.html

@@ -33,8 +33,9 @@
         </div>
       {% endfor %}
 
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit">Save global config</button>
+        <a class="button-link secondary" href="{% url 'dashboard' %}">Cancel</a>
       </div>
     </form>
   </section>

src/pobsync_backend/templates/pobsync_backend/host_detail.html

@@ -433,7 +433,7 @@
         </div>
       {% endfor %}
 
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit" {% if not can_queue_dry_run and not can_queue_real_backup %}disabled{% endif %}>Queue with options</button>
       </div>
     </form>

src/pobsync_backend/templates/pobsync_backend/host_form.html

@@ -33,8 +33,13 @@
         </div>
       {% endfor %}
 
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit">{% if host %}Save config{% else %}Create host{% endif %}</button>
+        {% if host %}
+          <a class="button-link secondary" href="{% url 'host_detail' host.host %}">Cancel</a>
+        {% else %}
+          <a class="button-link secondary" href="{% url 'dashboard' %}">Cancel</a>
+        {% endif %}
       </div>
     </form>
   </section>

src/pobsync_backend/templates/pobsync_backend/logs.html

@@ -16,7 +16,7 @@
 
   <section class="panel">
     <h2>Filter</h2>
-    <form method="get" class="form-grid">
+    <form method="get" class="filter-form">
       <div class="field">
         <label for="unit">Unit</label>
         <select id="unit" name="unit">
@@ -54,8 +54,9 @@
         <label for="q">Message contains</label>
         <input id="q" name="q" value="{{ query }}">
       </div>
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit">Filter logs</button>
+        <a class="button-link secondary" href="{% url 'logs' %}">Clear</a>
       </div>
     </form>
   </section>

src/pobsync_backend/templates/pobsync_backend/purged_snapshots.html

@@ -16,7 +16,7 @@
 
   <section class="panel">
     <h2>Filters</h2>
-    <form method="get" class="form-grid">
+    <form method="get" class="filter-form">
       <div class="field">
         <label for="host">Host</label>
         <select id="host" name="host">
@@ -35,7 +35,7 @@
           {% endfor %}
         </select>
       </div>
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit">Apply filters</button>
         <a class="button-link secondary" href="{% url 'purged_snapshots' %}">Clear</a>
       </div>

src/pobsync_backend/templates/pobsync_backend/retention_plan.html

@@ -104,8 +104,12 @@
   </section>
 
   {% if plan.delete %}
-    <section class="panel">
+    <section class="panel highlight warning">
       <h2>Apply Retention</h2>
+      <p class="muted">
+        This permanently deletes the snapshot directories listed in Would Delete. Confirm the host and delete count
+        before applying the plan.
+      </p>
       <form method="post" action="{% url 'apply_host_retention' host.host %}" class="form-grid">
         {% csrf_token %}
         {{ apply_form.non_field_errors }}
@@ -138,8 +142,9 @@
           <div class="helptext">{{ apply_form.confirm_delete_count.help_text }}</div>
         </div>
 
-        <div class="actions">
-          <button type="submit">Apply retention</button>
+        <div class="form-actions">
+          <button type="submit" class="danger">Apply retention</button>
+          <a class="button-link secondary" href="{% url 'host_retention_plan' host.host %}?kind={{ kind }}">Cancel</a>
         </div>
       </form>
     </section>
@@ -200,6 +205,10 @@
       </table>
 
       <h3>Cleanup Incomplete Snapshots</h3>
+      <p class="muted">
+        This deletes only incomplete snapshot directories and their tracking records. Successful manual and scheduled
+        snapshots are not touched.
+      </p>
       <form method="post" action="{% url 'cleanup_host_incomplete_snapshots' host.host %}" class="form-grid">
         {% csrf_token %}
         {{ incomplete_cleanup_form.non_field_errors }}
@@ -225,8 +234,9 @@
           <div class="helptext">{{ incomplete_cleanup_form.confirm_delete_count.help_text }}</div>
         </div>
 
-        <div class="actions">
-          <button type="submit">Delete incomplete snapshots</button>
+        <div class="form-actions">
+          <button type="submit" class="danger">Delete incomplete snapshots</button>
+          <a class="button-link secondary" href="{% url 'host_retention_plan' host.host %}?kind={{ kind }}">Cancel</a>
         </div>
       </form>
     </section>

src/pobsync_backend/templates/pobsync_backend/run_detail.html

@@ -11,20 +11,6 @@
     </div>
     <section class="actions" aria-label="Run actions">
       <a class="button-link" href="{% url 'host_detail' run.host.host %}">Back to host</a>
-      {% if can_cancel %}
-        <form method="post" action="{% url 'cancel_run' run.id %}">
-          {% csrf_token %}
-          <button type="submit" class="secondary">Cancel run</button>
-        </form>
-      {% endif %}
-      {% if run.status == "failed" or run.status == "warning" %}
-        {% if not run.reviewed_at %}
-          <form method="post" action="{% url 'resolve_run_review' run.id %}">
-            {% csrf_token %}
-            <button type="submit" class="secondary">Mark reviewed</button>
-          </form>
-        {% endif %}
-      {% endif %}
     </section>
   </header>
 
@@ -35,6 +21,22 @@
     <div class="metric"><div class="label">Rsync</div><div class="value">{{ run.rsync_exit_code|default:"" }}</div></div>
   </section>
 
+  {% if can_cancel %}
+    <section class="panel highlight warning">
+      <h2>Run Control</h2>
+      <p>
+        Cancelling a queued run stops it immediately. Cancelling a running run asks the worker to stop
+        and records the cancellation request on this run.
+      </p>
+      <form method="post" action="{% url 'cancel_run' run.id %}">
+        {% csrf_token %}
+        <div class="form-actions">
+          <button type="submit" class="danger">Cancel run</button>
+        </div>
+      </form>
+    </section>
+  {% endif %}
+
   {% if failure %}
     <section class="panel highlight failed">
       <h2>Failure</h2>
@@ -46,6 +48,21 @@
     </section>
   {% endif %}
 
+  {% if run.status == "failed" or run.status == "warning" %}
+    {% if not run.reviewed_at %}
+      <section class="panel highlight warning">
+        <h2>Review Required</h2>
+        <p>Mark this run as reviewed after you have checked the failure or warning and no longer need it in the action queue.</p>
+        <form method="post" action="{% url 'resolve_run_review' run.id %}">
+          {% csrf_token %}
+          <div class="form-actions">
+            <button type="submit" class="secondary">Mark reviewed</button>
+          </div>
+        </form>
+      </section>
+    {% endif %}
+  {% endif %}
+
   {% if run.reviewed_at %}
     <section class="panel highlight success">
       <h2>Review</h2>

src/pobsync_backend/templates/pobsync_backend/runs_list.html

@@ -16,7 +16,7 @@
 
   <section class="panel">
     <h2>Filters</h2>
-    <form method="get" class="form-grid">
+    <form method="get" class="filter-form">
       <div class="field">
         <label for="status">Status</label>
         <select id="status" name="status">
@@ -52,7 +52,7 @@
           <option value="reviewed" {% if selected_review == "reviewed" %}selected{% endif %}>Reviewed</option>
         </select>
       </div>
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit">Apply filters</button>
         <a class="button-link secondary" href="{% url 'runs_list' %}">Clear</a>
       </div>

src/pobsync_backend/templates/pobsync_backend/schedule_form.html

@@ -30,8 +30,9 @@
         </div>
       {% endfor %}
 
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit">Save schedule</button>
+        <a class="button-link secondary" href="{% url 'host_detail' host.host %}">Cancel</a>
       </div>
     </form>
   </section>

src/pobsync_backend/templates/pobsync_backend/schedules_list.html

@@ -16,7 +16,7 @@
 
   <section class="panel">
     <h2>Filters</h2>
-    <form method="get" class="form-grid">
+    <form method="get" class="filter-form">
       <div class="field">
         <label for="host">Host</label>
         <select id="host" name="host">
@@ -42,7 +42,7 @@
           <option value="no" {% if selected_prune == "no" %}selected{% endif %}>Prune disabled</option>
         </select>
       </div>
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit">Apply filters</button>
         <a class="button-link secondary" href="{% url 'schedules_list' %}">Clear</a>
       </div>

src/pobsync_backend/templates/pobsync_backend/snapshots_list.html

@@ -16,7 +16,7 @@
 
   <section class="panel">
     <h2>Filters</h2>
-    <form method="get" class="form-grid">
+    <form method="get" class="filter-form">
       <div class="field">
         <label for="host">Host</label>
         <select id="host" name="host">
@@ -44,7 +44,7 @@
           {% endfor %}
         </select>
       </div>
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit">Apply filters</button>
         <a class="button-link secondary" href="{% url 'snapshots_list' %}">Clear</a>
       </div>

src/pobsync_backend/templates/pobsync_backend/ssh_credential_form.html

@@ -41,8 +41,9 @@
         </div>
       {% endfor %}
 
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit">{% if credential %}Save SSH key{% else %}Create SSH key{% endif %}</button>
+        <a class="button-link secondary" href="{% url 'ssh_credentials' %}">Cancel</a>
       </div>
     </form>
   </section>
@@ -64,7 +65,10 @@
           <label for="confirm_name">Confirm key name</label>
           <input id="confirm_name" name="confirm_name" type="text" {% if credential.hosts.exists or credential.global_configs.exists %}disabled{% endif %}>
         </div>
-        <button type="submit" class="danger" {% if credential.hosts.exists or credential.global_configs.exists %}disabled{% endif %}>Delete SSH key</button>
+        <div class="form-actions">
+          <button type="submit" class="danger" {% if credential.hosts.exists or credential.global_configs.exists %}disabled{% endif %}>Delete SSH key</button>
+          <a class="button-link secondary" href="{% url 'ssh_credentials' %}">Cancel</a>
+        </div>
       </form>
     </section>
   {% endif %}

src/pobsync_backend/templates/pobsync_backend/ssh_credential_generate.html

@@ -29,8 +29,9 @@
         </div>
       {% endfor %}
 
-      <div class="actions">
+      <div class="form-actions">
         <button type="submit">Generate SSH key</button>
+        <a class="button-link secondary" href="{% url 'ssh_credentials' %}">Cancel</a>
       </div>
     </form>
   </section>

src/pobsync_backend/tests/test_views.py

@@ -233,6 +233,9 @@ class ViewTests(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "Runs")
         self.assertContains(response, "Review queued, running, completed")
+        self.assertContains(response, "Apply filters")
+        self.assertContains(response, reverse("runs_list"))
+        self.assertContains(response, "Clear")
         self.assertContains(response, f"Run {failed.id}")
         self.assertContains(response, "web-01")
         self.assertContains(response, "needed")
@@ -275,6 +278,9 @@ class ViewTests(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "Snapshots")
         self.assertContains(response, "Browse discovered scheduled, manual, and incomplete snapshots")
+        self.assertContains(response, "Apply filters")
+        self.assertContains(response, reverse("snapshots_list"))
+        self.assertContains(response, "Clear")
         self.assertContains(response, manual.dirname)
         self.assertContains(response, "web-01")
         self.assertNotContains(response, scheduled.dirname)
@@ -291,6 +297,9 @@ class ViewTests(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "Schedules")
         self.assertContains(response, "Review configured backup schedules")
+        self.assertContains(response, "Apply filters")
+        self.assertContains(response, reverse("schedules_list"))
+        self.assertContains(response, "Clear")
         self.assertContains(response, "web-01")
         self.assertContains(response, "15 2 * * *")
         self.assertContains(response, "success")
@@ -428,6 +437,9 @@ class ViewTests(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "Logs")
         self.assertContains(response, "Filter pobsync service logs")
+        self.assertContains(response, "Filter logs")
+        self.assertContains(response, reverse("logs"))
+        self.assertContains(response, "Clear")
         self.assertContains(response, "web-01 failed backup run 12")
         self.assertNotContains(response, "web-02 failed backup run 12")
         self.assertNotContains(response, "started")
@@ -458,6 +470,9 @@ class ViewTests(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "Purged Snapshots")
         self.assertContains(response, "Audit trail for snapshots removed")
+        self.assertContains(response, "Apply filters")
+        self.assertContains(response, reverse("purged_snapshots"))
+        self.assertContains(response, "Clear")
         self.assertContains(response, "20260518-021500Z__OLDSNAP")
         self.assertContains(response, "outside retention policy")
         self.assertContains(response, "Scheduled")
@@ -542,6 +557,21 @@ class ViewTests(TestCase):
         self.assertEqual(credential.private_key, "UPLOADED PRIVATE KEY\n")
         self.assertEqual(credential.public_key, "DERIVED PUBLIC KEY")
 
+    def test_ssh_credential_forms_render_cancel_actions(self) -> None:
+        self.client.force_login(self.staff_user)
+        credential = SshCredential.objects.create(name="backup-key")
+
+        create_response = self.client.get(reverse("create_ssh_credential"))
+        edit_response = self.client.get(reverse("edit_ssh_credential", args=[credential.id]))
+        generate_response = self.client.get(reverse("generate_ssh_credential"))
+
+        for response in (create_response, edit_response, generate_response):
+            self.assertEqual(response.status_code, 200)
+            self.assertContains(response, "Cancel")
+            self.assertContains(response, reverse("ssh_credentials"))
+        self.assertContains(edit_response, "Delete SSH key")
+        self.assertContains(edit_response, 'class="danger"', html=False)
+
     def test_ssh_credentials_view_generates_filesystem_key(self) -> None:
         self.client.force_login(self.staff_user)
         with TemporaryDirectory() as tmp, override_settings(POBSYNC_HOME=str(Path(tmp) / "home")):
@@ -746,6 +776,8 @@ class ViewTests(TestCase):
         self.assertContains(response, f'value="{credential.id}" selected')
         self.assertContains(response, "--archive")
         self.assertContains(response, "/proc/***")
+        self.assertContains(response, "Cancel")
+        self.assertContains(response, reverse("dashboard"))
 
     def test_global_config_form_renders_static_container_backup_root_on_edit(self) -> None:
         self.client.force_login(self.staff_user)
@@ -1635,8 +1667,11 @@ class ViewTests(TestCase):
         response = self.client.get(reverse("run_detail", args=[run.id]))
 
         self.assertEqual(response.status_code, 200)
+        self.assertContains(response, "Run Control")
+        self.assertContains(response, "Cancelling a queued run stops it immediately")
         self.assertContains(response, "Cancel run")
         self.assertContains(response, reverse("cancel_run", args=[run.id]))
+        self.assertContains(response, 'class="danger"', html=False)
 
     def test_run_detail_renders_worker_execution_metadata(self) -> None:
         self.client.force_login(self.staff_user)
@@ -1820,6 +1855,9 @@ class ViewTests(TestCase):
         self.assertNotContains(response, "<div class=\"label\">Source</div>", html=True)
         self.assertContains(response, "Confirm delete count")
         self.assertContains(response, "Type 1 to confirm the current number of planned deletions.")
+        self.assertContains(response, "This permanently deletes the snapshot directories listed in Would Delete.")
+        self.assertContains(response, 'class="danger"', html=False)
+        self.assertContains(response, "Cancel")
 
     def test_retention_plan_warns_when_scheduled_prune_limit_is_exceeded(self) -> None:
         self.client.force_login(self.staff_user)
@@ -1895,6 +1933,8 @@ class ViewTests(TestCase):
         self.assertContains(response, "excluded from retention cleanup")
         self.assertContains(response, "Delete incomplete snapshots")
         self.assertContains(response, "Type 1 to confirm the current number of incomplete snapshots.")
+        self.assertContains(response, "This deletes only incomplete snapshot directories")
+        self.assertContains(response, 'class="danger"', html=False)
 
     def test_incomplete_cleanup_deletes_incomplete_snapshot_after_confirmation(self) -> None:
         self.client.force_login(self.staff_user)
@@ -2142,6 +2182,8 @@ class ViewTests(TestCase):
         self.assertContains(response, "evaluated by the pobsync scheduler service")
         self.assertContains(response, "15 2 * * *")
         self.assertContains(response, "Save schedule")
+        self.assertContains(response, "Cancel")
+        self.assertContains(response, reverse("host_detail", args=[host.host]))
 
     def test_schedule_form_creates_schedule(self) -> None:
         self.client.force_login(self.staff_user)
@@ -2243,6 +2285,8 @@ class ViewTests(TestCase):
         self.assertContains(response, "/srv")
         self.assertContains(response, "*.tmp")
         self.assertContains(response, "--numeric-ids")
+        self.assertContains(response, "Cancel")
+        self.assertContains(response, reverse("host_detail", args=[host.host]))
 
     def test_host_config_form_renders_effective_config_check(self) -> None:
         self.client.force_login(self.staff_user)