From 171dd056b3791b59d712f759ebd09a1981a56169 Mon Sep 17 00:00:00 2001
From: objecttothis <17935339+objecttothis@users.noreply.github.com>
Date: Thu, 17 Sep 2026 16:22:55 +0400
Subject: [PATCH] fix(jobs): validate all throttles before saving any
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Bug: saveThrottles validated and saved throttles in same loop.
Invalid entry mid-loop stopped processing but left already-saved
throttles committed, and omitted throttles could still get deleted
via notToDelete tracking — partial/inconsistent state on failure.

- app/Controllers/Jobs.php: split into two passes — validate all
  throttleData entries first; on any invalid entry, roll back
  transaction and return failure immediately before touching DB.
  Only save/delete once full payload validated. Drop now-redundant
  $success flag folded into invalid-entry early return.
- tests/Controllers/JobsControllerTest.php: add test asserting
  existing throttles stay unchanged in DB when payload contains
  invalid throttle data.

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
---
 app/Controllers/Jobs.php                 | 15 ++++++++-------
 tests/Controllers/JobsControllerTest.php | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/app/Controllers/Jobs.php b/app/Controllers/Jobs.php
index d5d15ab7a..5029635ed 100644
--- a/app/Controllers/Jobs.php
+++ b/app/Controllers/Jobs.php
@@ -98,14 +98,15 @@ class Jobs extends Secure_Controller
             }
         }
 
-        $success = true;
+        foreach ($arraySave as $throttleData) {
+            if (!ctype_digit((string)$throttleData['max_count']) || !in_array($throttleData['period'], $allowedPeriods, true)) {
+                $this->db->transRollback();
+
+                return $this->response->setJSON(['success' => false, 'message' => lang('Jobs.saved_unsuccessfully')]);
+            }
+        }
 
         foreach ($arraySave as $throttleId => $throttleData) {
-            if (!ctype_digit((string)$throttleData['max_count']) || !in_array($throttleData['period'], $allowedPeriods, true)) {
-                $success = false;
-                continue;
-            }
-
             $savedThrottleId = $this->jobThrottle->saveValue($throttleData, $throttleId);
             $notToDelete[] = (string)$savedThrottleId;
         }
@@ -121,7 +122,7 @@ class Jobs extends Secure_Controller
 
         $this->db->transComplete();
 
-        $success = $success && $this->db->transStatus();
+        $success = $this->db->transStatus();
 
         return $this->response->setJSON(['success' => $success, 'message' => lang('Jobs.saved_' . ($success ? '' : 'un') . 'successfully')]);
     }
diff --git a/tests/Controllers/JobsControllerTest.php b/tests/Controllers/JobsControllerTest.php
index 6ce929ebb..c49ce2eda 100644
--- a/tests/Controllers/JobsControllerTest.php
+++ b/tests/Controllers/JobsControllerTest.php
@@ -135,6 +135,26 @@ class JobsControllerTest extends CIUnitTestCase
         $this->assertFalse($result['success']);
     }
 
+    public function testPostSaveThrottlesLeavesExistingThrottlesUnchangedWhenPayloadInvalid(): void
+    {
+        $this->loginAsAdmin();
+        $this->jobThrottle->saveValue(['max_count' => 5, 'period' => 'minute'], 1);
+        $this->jobThrottle->saveValue(['max_count' => 20, 'period' => 'day'], 2);
+
+        // Throttle 2 is invalid and throttle 1 is omitted; neither should be touched.
+        $response = $this->post('/jobs/saveThrottles', [
+            'throttle_count_2'  => 15,
+            'throttle_period_2' => 'fortnight',
+        ]);
+
+        $response->assertStatus(200);
+        $result = json_decode($response->getJSON(), true);
+        $this->assertFalse($result['success']);
+
+        $this->seeInDatabase('job_throttles', ['throttle_id' => 1, 'max_count' => 5, 'period' => 'minute', 'deleted' => 0]);
+        $this->seeInDatabase('job_throttles', ['throttle_id' => 2, 'max_count' => 20, 'period' => 'day', 'deleted' => 0]);
+    }
+
     public function testGetThrottlesRendersPartial(): void
     {
         $this->loginAsAdmin();