From 7ce3bfb2328629d7d24d76b44540d59e10f6bd49 Mon Sep 17 00:00:00 2001
From: Konrad du Plessis <konradzar@gmail.com>
Date: Fri, 12 Jun 2026 17:42:39 +0200
Subject: [PATCH] =?UTF-8?q?fix:=20Batch=20Pay=20modal=20=E2=80=94=20filter?=
 =?UTF-8?q?s=20no=20longer=20silently=20re-tick=20unticked=20workers;=20su?=
 =?UTF-8?q?rface=20server=20errors?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two payment-safety fixes in the Batch Pay modal JS:

1. Changing the team/loan filter force-checked every visible row,
   silently re-selecting workers the admin had deliberately unticked
   (untick a disputed worker -> change filter -> Confirm & Pay All pays
   them anyway). Filters now only EXCLUDE (hidden rows untick); visible
   rows keep the admin's manual choice, and Select All reflects the
   real state instead of being forced on.

2. The batch-pay fetch() redirected to the dashboard on ANY HTTP
   response — fetch only rejects on network failure, so a 500 (batch
   died partway; each worker pays in its own transaction) looked like
   success. Now checks resp.ok and tells the admin to verify the
   History tab before retrying.

JS-only change; needs manual verification in the browser (no JS test
harness in this project).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 core/templates/core/payroll_dashboard.html | 30 ++++++++++++++++++----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/core/templates/core/payroll_dashboard.html b/core/templates/core/payroll_dashboard.html
index f1fb24a..7400177 100644
--- a/core/templates/core/payroll_dashboard.html
+++ b/core/templates/core/payroll_dashboard.html
@@ -3266,10 +3266,16 @@ document.addEventListener('DOMContentLoaded', function() {
                 });
 
                 // --- Shared filter function (team + loan filters combined) ---
+                // SAFETY RULE: a filter change may EXCLUDE workers from the
+                // batch (hidden rows are unticked) but must NEVER silently
+                // re-tick a worker the admin deliberately unticked — that
+                // would pay someone the admin chose to skip. Use Select All
+                // to re-include everyone after changing filters.
                 function applyBatchFilters() {
                     var selectedTeam = filterSelect.value;
                     var loanMode = batchLoanFilter ? batchLoanFilter.value : '';
                     var rows = tbody.querySelectorAll('tr');
+                    var allChecked = true;
                     for (var i = 0; i < rows.length; i++) {
                         var row = rows[i];
                         var teamMatch = !selectedTeam || row.dataset.team === selectedTeam;
@@ -3278,13 +3284,17 @@ document.addEventListener('DOMContentLoaded', function() {
                             || (loanMode === 'without' && row.dataset.hasLoan !== 'true');
                         if (teamMatch && loanMatch) {
                             row.style.display = '';
-                            row.querySelector('.batch-worker-cb').checked = true;
+                            // keep the admin's manual tick/untick choice
+                            if (!row.querySelector('.batch-worker-cb').checked) {
+                                allChecked = false;
+                            }
                         } else {
                             row.style.display = 'none';
                             row.querySelector('.batch-worker-cb').checked = false;
                         }
                     }
-                    selectAllCb.checked = true;
+                    // Reflect the real state of the visible rows (not forced on)
+                    selectAllCb.checked = allChecked;
                     updateBatchSummary(data, summary);
                 }
 
@@ -3374,17 +3384,27 @@ document.addEventListener('DOMContentLoaded', function() {
                     'X-CSRFToken': '{{ csrf_token }}',
                 },
                 body: JSON.stringify({ workers: workers }),
-            }).then(function() {
+            }).then(function(resp) {
+                // fetch() does NOT reject on HTTP errors (4xx/5xx) — only on
+                // network failure. A 500 here can mean the batch died partway
+                // (each worker pays in its own transaction), so surface it
+                // instead of redirecting as if everything succeeded.
+                if (!resp.ok) {
+                    throw new Error('server returned ' + resp.status);
+                }
                 // Redirect to refresh page and show Django success messages
                 window.location.href = '/payroll/';
-            }).catch(function() {
+            }).catch(function(err) {
                 btn.disabled = false;
                 while (btn.firstChild) btn.removeChild(btn.firstChild);
                 var retryIcon = document.createElement('i');
                 retryIcon.className = 'fas fa-money-bill-wave me-1';
                 btn.appendChild(retryIcon);
                 btn.appendChild(document.createTextNode('Confirm & Pay All'));
-                alert('Batch payment failed. Please try again.');
+                alert('Batch payment may have partially failed ('
+                    + (err && err.message ? err.message : 'network error')
+                    + '). Check the History tab to see which workers were paid '
+                    + 'before retrying — do not blindly re-pay everyone.');
             });
         });
     }