From 86ef91abcbbadf0b4ae780d50cfddfb8578a1347 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 11 Sep 2026 14:28:21 -0400 Subject: [PATCH] Reject duplicate workflow transitions with a clear error (#62) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit status_transitions already has a DB-level UNIQUE KEY on (from_status, to_status), so a genuine duplicate pair was never actually possible to insert — but hitting that constraint raw surfaced as an opaque "An internal error occurred" to the admin instead of a clear message, since manage_workflows.php only validated from_status !== to_status before attempting the insert/update. Added an explicit existence check before insert/update in both the POST and PUT handlers (excluding the row's own ID on update), so the common case — an admin re-adding or renaming into a pair that already exists — gets a specific 409 with the conflicting pair named, instead of a generic 500. Also added ORDER BY transition_id to WorkflowModel::getAllTransitions() as a defense-in-depth backstop: since it collapses rows into a PHP array keyed by [from_status][to_status] with no defined winner otherwise, if the DB constraint were ever weakened or bypassed, this at least makes which row wins deterministic (most recently created). Verified against a real running server + real MariaDB: creating a duplicate active pair, a duplicate inactive pair, and updating a different row into an existing pair are all correctly rejected with the friendly message; updating a row to keep its own existing pair succeeds; and a genuinely different pair still creates normally. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv --- api/manage_workflows.php | 51 ++++++++++++++++++++++++++++++++++++++-- models/WorkflowModel.php | 8 ++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/api/manage_workflows.php b/api/manage_workflows.php index 90ba6dd..9ce5c6e 100644 --- a/api/manage_workflows.php +++ b/api/manage_workflows.php @@ -97,13 +97,39 @@ try { exit; } + $wf_active = (int)($data['is_active'] ?? 1); + + // status_transitions already has a DB-level UNIQUE KEY on + // (from_status, to_status) (regardless of is_active), so a + // duplicate pair can't actually be inserted — but hitting that + // constraint raw surfaces as an opaque "internal error occurred" + // to the admin instead of a clear message. Check first so the + // common case (an admin re-adding a pair that already exists) + // gets a friendly, specific error. + $dupCheck = $conn->prepare( + "SELECT transition_id FROM status_transitions WHERE from_status = ? AND to_status = ?" + ); + $dupCheck->bind_param('ss', $data['from_status'], $data['to_status']); + $dupCheck->execute(); + if ($dupCheck->get_result()->fetch_assoc()) { + $dupCheck->close(); + http_response_code(409); + echo json_encode([ + 'success' => false, + 'error' => 'A transition already exists for ' + . $data['from_status'] . ' → ' . $data['to_status'] + . ' — edit that row instead of creating a duplicate.', + ]); + exit; + } + $dupCheck->close(); + $stmt = $conn->prepare("INSERT INTO status_transitions (from_status, to_status, requires_comment, requires_admin, is_active) VALUES (?, ?, ?, ?, ?)"); $wf_from = $data['from_status']; $wf_to = $data['to_status']; $wf_comment = (int)($data['requires_comment'] ?? 0); $wf_admin = (int)($data['requires_admin'] ?? 0); - $wf_active = (int)($data['is_active'] ?? 1); $stmt->bind_param('ssiii', $wf_from, $wf_to, $wf_comment, $wf_admin, $wf_active); if ($stmt->execute()) { @@ -149,6 +175,28 @@ try { exit; } + $wf_active = (int)($data['is_active'] ?? 1); + + // Same duplicate-pair guard as create, excluding this row itself. + $dupCheck = $conn->prepare( + "SELECT transition_id FROM status_transitions + WHERE from_status = ? AND to_status = ? AND transition_id != ?" + ); + $dupCheck->bind_param('ssi', $data['from_status'], $data['to_status'], $id); + $dupCheck->execute(); + if ($dupCheck->get_result()->fetch_assoc()) { + $dupCheck->close(); + http_response_code(409); + echo json_encode([ + 'success' => false, + 'error' => 'A transition already exists for ' + . $data['from_status'] . ' → ' . $data['to_status'] + . ' — edit that row instead of creating a duplicate.', + ]); + exit; + } + $dupCheck->close(); + $stmt = $conn->prepare("UPDATE status_transitions SET from_status = ?, to_status = ?, requires_comment = ?, requires_admin = ?, is_active = ? WHERE transition_id = ?"); @@ -156,7 +204,6 @@ try { $wf_to = $data['to_status']; $wf_comment = (int)($data['requires_comment'] ?? 0); $wf_admin = (int)($data['requires_admin'] ?? 0); - $wf_active = (int)($data['is_active'] ?? 1); $stmt->bind_param('ssiiii', $wf_from, $wf_to, $wf_comment, $wf_admin, $wf_active, $id); $success = $stmt->execute(); diff --git a/models/WorkflowModel.php b/models/WorkflowModel.php index 473bfed..7928871 100644 --- a/models/WorkflowModel.php +++ b/models/WorkflowModel.php @@ -31,9 +31,15 @@ class WorkflowModel return $cached; } + // ORDER BY makes which row wins deterministic (most recently created, + // by transition_id) in the pathological case where two active rows + // exist for the same (from_status, to_status) pair — manage_workflows.php + // now rejects creating that duplicate going forward, but this is a + // defense-in-depth backstop against any duplicate already in the DB. $sql = "SELECT from_status, to_status, requires_comment, requires_admin FROM status_transitions - WHERE is_active = TRUE"; + WHERE is_active = TRUE + ORDER BY transition_id ASC"; $result = $this->conn->query($sql); if (!$result) {