fix: Use explicit transBegin/transCommit/transRollback for atomicity

- Replace transStart/transComplete with transBegin/transCommit/transRollback
- Check all success conditions before committing
- Explicit rollback on failure

Address CodeRabbit review feedback

app/Controllers/Items.php

@@ -663,10 +663,13 @@ class Items extends Secure_Controller
 
         $employee_id = $this->employee->get_logged_in_employee_info()->person_id;
 
-        if ($this->item->save_value($item_data, $item_id)) {
-            $success = true;
-            $new_item = false;
+        // Wrap the entire save sequence in a single transaction for atomicity
+        $this->db->transBegin();
 
+        $success = $this->item->save_value($item_data, $item_id);
+        $new_item = false;
+
+        if ($success) {
             if ($item_id === NEW_ENTRY) {
                 $item_id = $item_data['item_id'];
                 $new_item = true;
@@ -690,7 +693,7 @@ class Items extends Secure_Controller
 
                     $tax_name_index++;
                 }
-                $success &= $this->item_taxes->save_value($items_taxes_data, $item_id);
+                $success = $success && $this->item_taxes->save_value($items_taxes_data, $item_id);
             }
 
             // Save item quantity
@@ -711,7 +714,7 @@ class Items extends Secure_Controller
                 $item_quantity = $this->item_quantity->get_item_quantity($item_id, $location['location_id']);
 
                 if ($item_quantity->quantity != $updated_quantity || $new_item) {
-                    $success = $success &&  $this->item_quantity->save_value($location_detail, $item_id, $location['location_id']);
+                    $success = $success && $this->item_quantity->save_value($location_detail, $item_id, $location['location_id']);
 
                     $inv_data = [
                         'trans_date'      => date('Y-m-d H:i:s'),
@@ -726,21 +729,21 @@ class Items extends Secure_Controller
                 }
             }
             $success = $success && $this->saveItemAttributes($item_id);
-
-            if ($success && $upload_success) {
-                $message = lang('Items.successful_' . ($new_item ? 'adding' : 'updating')) . ' ' . $item_data['name'];
-
-                return $this->response->setJSON(['success' => true, 'message' => $message, 'id' => $item_id]);
-            } else {
-                $message = $upload_success ? lang('Items.error_adding_updating') . ' ' . $item_data['name'] : strip_tags($upload_data['error']);
-
-                return $this->response->setJSON(['success' => false, 'message' => $message, 'id' => $item_id]);
-            }
-        } else {
-            $message = lang('Items.error_adding_updating') . ' ' . $item_data['name'];
-
-            return $this->response->setJSON(['success' => false, 'message' => $message, 'id' => NEW_ENTRY]);
         }
+
+        // Check all success conditions before committing
+        if ($success && $upload_success) {
+            $this->db->transCommit();
+            $message = lang('Items.successful_' . ($new_item ? 'adding' : 'updating')) . ' ' . $item_data['name'];
+
+            return $this->response->setJSON(['success' => true, 'message' => $message, 'id' => $item_id]);
+        }
+
+        // Rollback on failure
+        $this->db->transRollback();
+        $message = $upload_success ? lang('Items.error_adding_updating') . ' ' . $item_data['name'] : strip_tags($upload_data['error']);
+
+        return $this->response->setJSON(['success' => false, 'message' => $message, 'id' => $item_id]);
     }
 
     /**

app/Models/Item_taxes.php

@@ -37,9 +37,6 @@ class Item_taxes extends Model
     {
         $success = true;
 
-        // Run these queries as a transaction, we want to make sure we do all or nothing
-        $this->db->transStart();
-
         $this->delete($item_id);
 
         $builder = $this->db->table('items_taxes');
@@ -49,10 +46,6 @@ class Item_taxes extends Model
             $success &= $builder->insert($row);
         }
 
-        $this->db->transComplete();
-
-        $success &= $this->db->transStatus();
-
         return $success;
     }