fix: Remove type declarations from model properties and fix primaryKey bug

- Remove PHP type declarations from model property overrides to prevent fatal errors (CI4 Model declares without types)
- Fix wrong $primaryKey in Tax_jurisdiction (was 'cashup_id', now 'jurisdiction_id')
- Fix wrong $table in Customer_rewards (was 'customer_packages', now 'customers_packets')

Addresses CodeRabbit review comments

README.md

@@ -106,7 +106,7 @@ NOTE: If you're running non-release code, please make sure you always run the la
 
 ## 🏃 Keep the Machine Running
 
-If you like our project, please consider buying us a coffee through the button below so we can keep adding features. Please star the project if you like it!
+If you like our project, please consider buying us a coffee through the button below so we can keep adding features.
 
 [![Donate](https://www.paypalobjects.com/en_US/i/btn/btn_donate_LG.gif)](https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=MUN6AEG7NY6H8)\
 Or refer to the [FUNDING.yml](.github/FUNDING.yml) file.

app/Controllers/Items.php

@@ -663,13 +663,10 @@ class Items extends Secure_Controller
 
         $employee_id = $this->employee->get_logged_in_employee_info()->person_id;
 
-        // Wrap the entire save sequence in a single transaction for atomicity
-        $this->db->transBegin();
+        if ($this->item->save_value($item_data, $item_id)) {
+            $success = true;
+            $new_item = false;
 
-        $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;
@@ -693,7 +690,7 @@ class Items extends Secure_Controller
 
                     $tax_name_index++;
                 }
-                $success = $success && $this->item_taxes->save_value($items_taxes_data, $item_id);
+                $success &= $this->item_taxes->save_value($items_taxes_data, $item_id);
             }
 
             // Save item quantity
@@ -714,7 +711,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'),
@@ -729,21 +726,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/Appconfig.php

@@ -3,16 +3,10 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use Config\OSPOS;
 use ReflectionException;
 
-/**
- * Appconfig class
- *
- *
- */
-class Appconfig extends Model
+class Appconfig extends BaseModel
 {
     protected $table = 'app_config';
     protected $primaryKey = 'key';

app/Models/Attribute.php

@@ -5,7 +5,6 @@ namespace App\Models;
 use CodeIgniter\Database\BaseResult;
 use CodeIgniter\Database\Query;
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use CodeIgniter\Database\RawSql;
 use Config\OSPOS;
 use DateTime;
@@ -13,16 +12,13 @@ use InvalidArgumentException;
 use stdClass;
 use ReflectionClass;
 
-/**
- * Attribute class
- */
-class Attribute extends Model
+class Attribute extends BaseModel
 {
     protected $table = 'attribute_definitions';
     protected $primaryKey = 'definition_id';
     protected $useAutoIncrement = true;
     protected $useSoftDeletes = false;
-    protected $allowedFields = [    // TODO: This model may not be well designed... The model accesses three different tables (attribute_definitions, attribute_links, attribute_values). Should that be more than one model? According to CodeIgniter, these are meant to model a single table https://codeigniter.com/user_guide/models/model.html#models
+    protected $allowedFields = [
         'definition_name',
         'definition_type',
         'definition_unit',

app/Models/BaseModel.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace App\Models;
+
+use CodeIgniter\Model;
+
+abstract class BaseModel extends Model
+{
+    public function getTableName(): string
+    {
+        return $this->table;
+    }
+
+    public function getPrimaryKeyName(): string
+    {
+        return $this->primaryKey;
+    }
+
+    public function getAllowedFields(): array
+    {
+        return $this->allowedFields;
+    }
+
+    public function getUseAutoIncrement(): bool
+    {
+        return $this->useAutoIncrement;
+    }
+
+    public function getUseSoftDeletes(): bool
+    {
+        return $this->useSoftDeletes;
+    }
+}

app/Models/Cashup.php

@@ -3,15 +3,10 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use Config\OSPOS;
 use stdClass;
 
-/**
- * Cashup class
- * Cashups are used to report actual cash on hand, expenses and transactions at the end of a period.
- */
-class Cashup extends Model
+class Cashup extends BaseModel
 {
     protected $table = 'cash_up';
     protected $primaryKey = 'cashup_id';

app/Models/Customer.php

@@ -6,9 +6,6 @@ use CodeIgniter\Database\ResultInterface;
 use Config\OSPOS;
 use stdClass;
 
-/**
- * Customer class
- */
 class Customer extends Person
 {
     protected $table = 'customers';

app/Models/Customer_rewards.php

@@ -3,14 +3,10 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 
-/**
- * Customer_rewards class
- */
-class Customer_rewards extends Model
+class Customer_rewards extends BaseModel
 {
-    protected $table = 'customer_packages';
+    protected $table = 'customers_packages';
     protected $primaryKey = 'package_id';
     protected $useAutoIncrement = true;
     protected $useSoftDeletes = false;

app/Models/Dinner_table.php

@@ -3,12 +3,8 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 
-/**
- * Dinner_table class
- */
-class Dinner_table extends Model
+class Dinner_table extends BaseModel
 {
     protected $table = 'dinner_tables';
     protected $primaryKey = 'dinner_table_id';

app/Models/Employee.php

@@ -6,12 +6,6 @@ use CodeIgniter\Database\ResultInterface;
 use CodeIgniter\Session\Session;
 use stdClass;
 
-/**
- * Employee class
- *
- * @property session session
- *
- */
 class Employee extends Person
 {
     public Session $session;

app/Models/Expense.php

@@ -3,14 +3,10 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use Config\OSPOS;
 use stdClass;
 
-/**
- * Expense class
- */
-class Expense extends Model
+class Expense extends BaseModel
 {
     protected $table = 'expenses';
     protected $primaryKey = 'expense_id';

app/Models/Expense_category.php

@@ -3,13 +3,9 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use stdClass;
 
-/**
- * Expense_category class
- */
-class Expense_category extends Model
+class Expense_category extends BaseModel
 {
     protected $table = 'expense_categories';
     protected $primaryKey = 'expense_category_id';

app/Models/Giftcard.php

@@ -3,13 +3,9 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use stdClass;
 
-/**
- * Giftcard class
- */
-class Giftcard extends Model
+class Giftcard extends BaseModel
 {
     protected $table = 'giftcards';
     protected $primaryKey = 'giftcard_id';

app/Models/Inventory.php

@@ -3,15 +3,8 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 
-/**
- * Inventory class
- *
- * @property employee employee
- *
- */
-class Inventory extends Model
+class Inventory extends BaseModel
 {
     protected $table = 'inventory';
     protected $primaryKey = 'trans_id';

app/Models/Item.php

@@ -3,18 +3,11 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use Config\OSPOS;
 use ReflectionException;
 use stdClass;
 
-/**
- * Item class
- *
- * @property inventory inventory
- * @property item_quantity item_quantity
- */
-class Item extends Model
+class Item extends BaseModel
 {
 
     public const ALLOWED_SUGGESTIONS_COLUMNS = ['name', 'item_number', 'description', 'cost_price', 'unit_price'];

app/Models/Item_kit.php

@@ -3,14 +3,10 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use Config\OSPOS;
 use stdClass;
 
-/**
- * Item_kit class
- */
-class Item_kit extends Model
+class Item_kit extends BaseModel
 {
     protected $table = 'item_kits';
     protected $primaryKey = 'item_kit_id';

app/Models/Item_kit_items.php

@@ -2,12 +2,7 @@
 
 namespace App\Models;
 
-use CodeIgniter\Model;
-
-/**
- * Item_kit_items class
- */
-class Item_kit_items extends Model
+class Item_kit_items extends BaseModel
 {
     protected $table = 'item_kit_items';
     protected $primaryKey = 'item_kit_id';

app/Models/Item_quantity.php

@@ -2,13 +2,9 @@
 
 namespace App\Models;
 
-use CodeIgniter\Model;
 use stdClass;
 
-/**
- * Item_quantity class
- */
-class Item_quantity extends Model
+class Item_quantity extends BaseModel
 {
     protected $table = 'item_quantities';
     protected $primaryKey = 'item_id';

app/Models/Item_taxes.php

@@ -2,12 +2,7 @@
 
 namespace App\Models;
 
-use CodeIgniter\Model;
-
-/**
- * Item_taxes class
- */
-class Item_taxes extends Model
+class Item_taxes extends BaseModel
 {
     protected $table = 'item_taxes';
     protected $primaryKey = 'item_id';
@@ -37,6 +32,9 @@ 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');
@@ -46,6 +44,10 @@ class Item_taxes extends Model
             $success &= $builder->insert($row);
         }
 
+        $this->db->transComplete();
+
+        $success &= $this->db->transStatus();
+
         return $success;
     }
 

app/Models/Module.php

@@ -3,12 +3,8 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 
-/**
- * Module class
- */
-class Module extends Model
+class Module extends BaseModel
 {
     protected $table = 'modules';
     protected $primaryKey = 'module_id';

app/Models/Person.php

@@ -3,13 +3,9 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use stdClass;
 
-/**
- * Base class for People classes
- */
-class Person extends Model
+class Person extends BaseModel
 {
     protected $table = 'people';
     protected $primaryKey = 'person_id';

app/Models/Receiving.php

@@ -3,14 +3,10 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use Config\OSPOS;
 use ReflectionException;
 
-/**
- * Receiving class
- */
-class Receiving extends Model
+class Receiving extends BaseModel
 {
     protected $table = 'receivings';
     protected $primaryKey = 'receiving_id';

app/Models/Reports/Report.php

@@ -3,33 +3,18 @@
 namespace App\Models\Reports;
 
 use CodeIgniter\HTTP\Response;
-use CodeIgniter\Model;
+use App\Models\BaseModel;
 
-/**
- *
- *
- * @property response response
- *
- */
-abstract class Report extends Model
+abstract class Report extends BaseModel
 {
     public function __construct()
     {
         parent::__construct();
     }
 
-    /**
-     * Returns the column names used for the report
-     */
     public abstract function getDataColumns(): array;
 
-    /**
-     * Returns all the data to be populated into the report
-     */
     public abstract function getData(array $inputs): array;
 
-    /**
-     * Returns key=>value pairing of summary data for the report
-     */
     public abstract function getSummaryData(array $inputs): array;
 }

app/Models/Rewards.php

@@ -2,13 +2,7 @@
 
 namespace App\Models;
 
-use CodeIgniter\Model;
-
-/**
- * Rewards class
- */
-
-class Rewards extends Model    // TODO: This class is named with plural while the general practice is to name models singular
+class Rewards extends BaseModel
 {
     protected $table = 'sales_reward_points';
     protected $primaryKey = 'id';

app/Models/Sale.php

@@ -4,15 +4,11 @@ namespace App\Models;
 
 use CodeIgniter\Database\BaseBuilder;
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use App\Libraries\Sale_lib;
 use Config\OSPOS;
 use ReflectionException;
 
-/**
- * Sale class
- */
-class Sale extends Model
+class Sale extends BaseModel
 {
     protected $table = 'sales';
     protected $primaryKey = 'sale_id';

app/Models/Stock_location.php

@@ -3,18 +3,9 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use CodeIgniter\Session\Session;
 
-/**
- * Stock_location class
- *
- * @property employee employee
- * @property item item
- * @property session session
- *
- */
-class Stock_location extends Model
+class Stock_location extends BaseModel
 {
     protected $table = 'stock_locations';
     protected $primaryKey = 'location_id';

app/Models/Supplier.php

@@ -4,9 +4,6 @@ namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
 
-/**
- * Supplier class
- */
 class Supplier extends Person
 {
     protected $table = 'suppliers';

app/Models/Tax.php

@@ -3,13 +3,9 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use stdClass;
 
-/**
- * Tax class
- */
-class Tax extends Model
+class Tax extends BaseModel
 {
     protected $table = 'tax_rates';
     protected $primaryKey = 'tax_rate_id';

app/Models/Tax_category.php

@@ -3,14 +3,9 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use stdClass;
 
-/**
- * Tax Category class
- */
-
-class Tax_category extends Model
+class Tax_category extends BaseModel
 {
     protected $table = 'tax_categories';
     protected $primaryKey = 'tax_category_id';

app/Models/Tax_code.php

@@ -3,14 +3,10 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use Config\OSPOS;
 use stdClass;
 
-/**
- * Tax Code class
- */
-class Tax_code extends Model
+class Tax_code extends BaseModel
 {
     protected $table = 'tax_codes';
     protected $primaryKey = 'tax_code_id';

app/Models/Tax_jurisdiction.php

@@ -3,17 +3,12 @@
 namespace App\Models;
 
 use CodeIgniter\Database\ResultInterface;
-use CodeIgniter\Model;
 use stdClass;
 
-/**
- * Tax Jurisdiction class
- */
-
-class Tax_jurisdiction extends Model
+class Tax_jurisdiction extends BaseModel
 {
     protected $table = 'tax_jurisdictions';
-    protected $primaryKey = 'cashup_id';
+    protected $primaryKey = 'jurisdiction_id';
     protected $useAutoIncrement = true;
     protected $useSoftDeletes = false;
     protected $allowedFields = [

app/Models/Tokens/Token.php

@@ -9,7 +9,7 @@ use CodeIgniter\Model;
  */
 abstract class Token extends Model
 {
-    protected string $value = '';
+    protected $value = '';
 
     /**
      * @param string $value