Widen bulk_operations.status so partial bulk results can be recorded (#21)
Lint / PHP (phpcs PSR-12) (push) Successful in 26s
Lint / JS (eslint) (push) Successful in 9s
Lint / PHP requirements (version + extensions) (push) Successful in 22s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m8s
Lint / Deploy (push) Successful in 3s
Lint / PHP (phpcs PSR-12) (push) Successful in 26s
Lint / JS (eslint) (push) Successful in 9s
Lint / PHP requirements (version + extensions) (push) Successful in 22s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m8s
Lint / Deploy (push) Successful in 3s
Found while verifying #21 against the live schema: the model writes 'completed_with_errors' (21 chars) when a bulk operation finishes with per-ticket failures, but bulk_operations.status was varchar(20), so the write failed with "Data too long for column 'status'". This was latent — bulk status changes previously forced every transition through, so failed was always 0. Now that they honour the Workflow Designer, a partially-skipped batch is a normal outcome and hits it. - migrations/001 widens the column to varchar(32) (idempotent). - The baseline is updated to match, for fresh installs. - The bookkeeping UPDATE is wrapped in a try/catch: it runs after the ticket changes are committed, so an instance deployed ahead of its migrations must not turn a completed operation into an error response. Verified against the live database with a disposable-ticket harness: comment-required rejection changes nothing, undefined transitions are refused per ticket with a reason, allowed transitions still work, mixed batches apply the valid half, and an already-Closed ticket is a no-op.
This commit is contained in:
@@ -59,7 +59,8 @@ CREATE TABLE IF NOT EXISTS `bulk_operations` (
|
||||
`ticket_ids` text NOT NULL,
|
||||
`performed_by` int(11) NOT NULL,
|
||||
`parameters` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL CHECK (json_valid(`parameters`)),
|
||||
`status` varchar(20) DEFAULT 'pending',
|
||||
-- 32, not 20: 'completed_with_errors' is 21 chars (see 001_widen_bulk_operations_status.sql)
|
||||
`status` varchar(32) DEFAULT 'pending',
|
||||
`total_tickets` int(11) DEFAULT NULL,
|
||||
`processed_tickets` int(11) DEFAULT 0,
|
||||
`failed_tickets` int(11) DEFAULT 0,
|
||||
|
||||
@@ -0,0 +1,12 @@
|
||||
-- Widen bulk_operations.status
|
||||
--
|
||||
-- The code writes 'completed_with_errors' (21 chars) when a bulk operation
|
||||
-- finishes with per-ticket failures, but the column was varchar(20), so the
|
||||
-- write failed with "Data too long for column 'status'". This was unreachable
|
||||
-- while bulk status changes forced every transition through; now that they
|
||||
-- honour the Workflow Designer, partial failures are a normal outcome.
|
||||
--
|
||||
-- Safe to re-run.
|
||||
|
||||
ALTER TABLE `bulk_operations`
|
||||
MODIFY COLUMN `status` varchar(32) DEFAULT 'pending';
|
||||
@@ -377,14 +377,22 @@ class BulkOperationsModel
|
||||
];
|
||||
}
|
||||
|
||||
// Update operation status
|
||||
$status = $failed > 0 ? 'completed_with_errors' : 'completed';
|
||||
$sql = "UPDATE bulk_operations SET status = ?, processed_tickets = ?, failed_tickets = ?,
|
||||
completed_at = NOW() WHERE operation_id = ?";
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
$stmt->bind_param("siii", $status, $processed, $failed, $operationId);
|
||||
$stmt->execute();
|
||||
$stmt->close();
|
||||
// Update operation status. This is bookkeeping only and runs after the
|
||||
// ticket changes are committed, so a failure here (e.g. the status column
|
||||
// not yet widened by 001_widen_bulk_operations_status.sql on an instance
|
||||
// deployed ahead of its migrations) must not turn a completed operation
|
||||
// into an error response.
|
||||
try {
|
||||
$status = $failed > 0 ? 'completed_with_errors' : 'completed';
|
||||
$sql = "UPDATE bulk_operations SET status = ?, processed_tickets = ?, failed_tickets = ?,
|
||||
completed_at = NOW() WHERE operation_id = ?";
|
||||
$stmt = $this->conn->prepare($sql);
|
||||
$stmt->bind_param("siii", $status, $processed, $failed, $operationId);
|
||||
$stmt->execute();
|
||||
$stmt->close();
|
||||
} catch (Throwable $e) {
|
||||
error_log("Bulk operation $operationId completed but status bookkeeping failed: " . $e->getMessage());
|
||||
}
|
||||
|
||||
$result = ['processed' => $processed, 'failed' => $failed];
|
||||
if (!empty($errors)) {
|
||||
|
||||
Reference in New Issue
Block a user