From 844677bbcec2353ff3535a4214cb6125fe1f7e3e Mon Sep 17 00:00:00 2001
From: Jared Vititoe <jjvititoe1@gmail.com>
Date: Fri, 11 Sep 2026 21:56:26 -0400
Subject: [PATCH] Fix atomicity docblock and surface per-ticket bulk-op errors
 (#33)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

processBulkOperation()'s docblock claimed the transaction "ensures
atomicity - either all tickets are updated or none are," but that's
only true when $atomic = true is passed, and the only real caller
(api/bulk_operation.php) never passes it — the actual default is
best-effort: per-ticket failures are skipped and recorded, and every
other ticket in the batch still commits. Reworded the docblock to
describe the actual default behavior and when $atomic changes it.

The model already collected per-ticket failure reasons into
$result['errors'] (dashboard.js's bulkResultMessage() already reads
data.errors to render them), but api/bulk_operation.php's success
response dropped that field entirely, so admins only ever saw a bare
"N succeeded, M failed" count with no way to see which tickets failed
or why. Added 'errors' to the response when present.

Verified against real MariaDB: a bulk_status operation against a Closed
ticket (no transition defined) and an Open ticket (Open->Pending
defined) correctly processed 1/1, and the API response now includes
errors: ["Ticket ...: transition not allowed (Closed -> Pending)"].

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
---
 api/bulk_operation.php         | 11 +++++++++--
 models/BulkOperationsModel.php | 12 +++++++++---
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/api/bulk_operation.php b/api/bulk_operation.php
index 57fa6ec..bc3a14f 100644
--- a/api/bulk_operation.php
+++ b/api/bulk_operation.php
@@ -131,12 +131,19 @@ if (isset($result['error'])) {
     if ($inaccessibleCount > 0) {
         $message .= " ($inaccessibleCount skipped - no access)";
     }
-    echo json_encode([
+    $response = [
         'success' => true,
         'operation_id' => $operationId,
         'processed' => $result['processed'],
         'failed' => $result['failed'],
         'skipped' => $inaccessibleCount,
         'message' => $message
-    ]);
+    ];
+    // Best-effort batches (the default; see processBulkOperation()'s docblock)
+    // can partially fail — surface the per-ticket reasons so the admin isn't
+    // just told a count. The dashboard's bulkResultMessage() already expects this.
+    if (!empty($result['errors'])) {
+        $response['errors'] = $result['errors'];
+    }
+    echo json_encode($response);
 }
diff --git a/models/BulkOperationsModel.php b/models/BulkOperationsModel.php
index 47dee2c..afd64be 100644
--- a/models/BulkOperationsModel.php
+++ b/models/BulkOperationsModel.php
@@ -106,12 +106,18 @@ class BulkOperationsModel
     /**
      * Process a bulk operation
      *
-     * Uses database transaction to ensure atomicity - either all tickets
-     * are updated or none are (on failure, changes are rolled back).
+     * Runs the whole batch inside one database transaction, but by default
+     * ($atomic = false, which is what api/bulk_operation.php uses) that
+     * transaction is always committed: a per-ticket failure (e.g. a
+     * disallowed workflow transition) is recorded in $failed/$errors and
+     * skipped, while every other ticket in the batch still succeeds. This
+     * is a best-effort batch, not an all-or-nothing one — set $atomic to
+     * true to roll back the entire batch when any ticket fails.
      *
      * @param int $operationId Operation ID
      * @param bool $atomic If true, rollback all changes on any failure
-     * @return array Result with processed and failed counts
+     * @return array Result with processed/failed counts and an errors[] list of
+     *               per-ticket failure reasons (surfaced to the admin by the caller)
      */
     public function processBulkOperation($operationId, bool $atomic = false)
     {