fix: Remove deleted filter and primary key from insert

- Remove deleted=0 filter from existence check (allow soft-deleted updates)
- Remove primary key from insert payload to avoid conflicts
- Cleaner approach for upsert logic

Address CodeRabbit review feedback

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.
+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!
 
 [![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/Expenses.php

@@ -91,7 +91,7 @@ class Expenses extends Secure_Controller
      */
     public function getView(int $expense_id = NEW_ENTRY): string
     {
-        $data = [];
+        $data = [];    // TODO: Duplicated code
 
         $data['expenses_info'] = $this->expense->get_info($expense_id);
         $expense_id = $data['expenses_info']->expense_id;

app/Controllers/Items.php

@@ -482,9 +482,9 @@ class Items extends Secure_Controller
         foreach ($result as &$item) {
             if (isset($item['item_number']) && empty($item['item_number']) && $this->config['barcode_generate_if_empty']) {
                 if (isset($item['item_id'])) {
-                    $save_item = ['item_number' => $item['item_number']];
-                    $this->item->save_value($save_item, $item['item_id']);
-                }
+                     $save_item = ['item_number' => $item['item_number'], 'item_id' => $item['item_id']];
+                     $this->item->saveValue($save_item);
+                 }
             }
         }
         $data['items'] = $result;
@@ -663,7 +663,12 @@ 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)) {
+        // For updates, include item_id in data array
+        if ($item_id !== NEW_ENTRY) {
+            $item_data['item_id'] = $item_id;
+        }
+
+        if ($this->item->saveValue($item_data)) {
             $success = true;
             $new_item = false;
 
@@ -826,8 +831,8 @@ class Items extends Secure_Controller
      */
     public function getRemoveLogo($item_id): ResponseInterface
     {
-        $item_data = ['pic_filename' => null];
-        $result = $this->item->save_value($item_data, $item_id);
+        $item_data = ['pic_filename' => null, 'item_id' => $item_id];
+        $result = $this->item->saveValue($item_data);
 
         return $this->response->setJSON(['success' => $result]);
     }
@@ -1039,7 +1044,7 @@ class Items extends Secure_Controller
                             return $value !== null && strlen($value);
                         });
 
-                        if (!$isFailedRow && $this->item->save_value($itemData, $itemId)) {
+                        if (!$isFailedRow && $this->item->saveValue($itemData)) {
                             $this->save_tax_data($row, $itemData);
                             $this->save_inventory_quantities($row, $itemData, $allowedStockLocations, $employeeId);
                             $csvAttributeValues = $this->extractAttributeData($row);
@@ -1312,8 +1317,8 @@ class Items extends Secure_Controller
                 $images = glob(FCPATH . "uploads/item_pics/$item->pic_filename.*");
                 if (sizeof($images) > 0) {
                     $new_pic_filename = pathinfo($images[0], PATHINFO_BASENAME);
-                    $item_data = ['pic_filename' => $new_pic_filename];
-                    $this->item->save_value($item_data, $item->item_id);
+                    $item_data = ['pic_filename' => $new_pic_filename, 'item_id' => $item->item_id];
+                    $this->item->saveValue($item_data);
                 }
             }
         }

app/Controllers/Receivings.php

@@ -11,7 +11,6 @@ use App\Models\Item_kit;
 use App\Models\Receiving;
 use App\Models\Stock_location;
 use App\Models\Supplier;
-use App\Traits\Controller\Shared;
 use CodeIgniter\HTTP\ResponseInterface;
 use Config\OSPOS;
 use Config\Services;
@@ -19,7 +18,6 @@ use ReflectionException;
 
 class Receivings extends Secure_Controller
 {
-    use Shared;
     private Receiving_lib $receiving_lib;
     private Token_lib $token_lib;
     private Barcode_lib $barcode_lib;
@@ -210,7 +208,7 @@ class Receivings extends Secure_Controller
         $quantity = parse_quantity($this->request->getPost('quantity'));
         $raw_receiving_quantity = parse_quantity($this->request->getPost('receiving_quantity'));
 
-        $description = $this->request->getPost('description', FILTER_SANITIZE_FULL_SPECIAL_CHARS);
+        $description = $this->request->getPost('description', FILTER_SANITIZE_FULL_SPECIAL_CHARS);    // TODO: Duplicated code
         $serialnumber = $this->request->getPost('serialnumber', FILTER_SANITIZE_FULL_SPECIAL_CHARS) ?? '';
         $discount_type = $this->request->getPost('discount_type', FILTER_SANITIZE_NUMBER_INT);
         $discount = $discount_type
@@ -427,10 +425,19 @@ class Receivings extends Secure_Controller
         $employee_info = $this->employee->get_info($receiving_info['employee_id']);
         $data['employee'] = $employee_info->first_name . ' ' . $employee_info->last_name;
 
-        $supplier_id = $this->receiving_lib->get_supplier();
+        $supplier_id = $this->receiving_lib->get_supplier();    // TODO: Duplicated code
         if ($supplier_id != -1) {
             $supplier_info = $this->supplier->get_info($supplier_id);
-            $this->buildSupplierInfo($supplier_info, $data);
+            $data['supplier'] = $supplier_info->company_name;
+            $data['first_name'] = $supplier_info->first_name;
+            $data['last_name'] = $supplier_info->last_name;
+            $data['supplier_email'] = $supplier_info->email;
+            $data['supplier_address'] = $supplier_info->address_1;
+            if (!empty($supplier_info->zip) or !empty($supplier_info->city)) {
+                $data['supplier_location'] = $supplier_info->zip . ' ' . $supplier_info->city;
+            } else {
+                $data['supplier_location'] = '';
+            }
         }
 
         $data['print_after_sale'] = false;
@@ -467,9 +474,18 @@ class Receivings extends Secure_Controller
 
         $supplier_id = $this->receiving_lib->get_supplier();
 
-        if ($supplier_id != -1) {
+        if ($supplier_id != -1) {    // TODO: Duplicated Code... replace -1 with a constant
             $supplier_info = $this->supplier->get_info($supplier_id);
-            $this->buildSupplierInfo($supplier_info, $data);
+            $data['supplier'] = $supplier_info->company_name;
+            $data['first_name'] = $supplier_info->first_name;
+            $data['last_name'] = $supplier_info->last_name;
+            $data['supplier_email'] = $supplier_info->email;
+            $data['supplier_address'] = $supplier_info->address_1;
+            if (!empty($supplier_info->zip) or !empty($supplier_info->city)) {
+                $data['supplier_location'] = $supplier_info->zip . ' ' . $supplier_info->city;
+            } else {
+                $data['supplier_location'] = '';
+            }
         }
 
         $data['print_after_sale'] = $this->receiving_lib->is_print_after_sale();

app/Controllers/Reports.php

@@ -128,8 +128,8 @@ class Reports extends Secure_Controller
      * @param string $location_id
      * @return string
      */
-    public function summary_sales(string $start_date, string $end_date, string $sale_type, string $location_id = 'all'): string
-    {
+    public function summary_sales(string $start_date, string $end_date, string $sale_type, string $location_id = 'all'): string    // TODO: Perhaps these need to be passed as an array?  Too many parameters in the signature.
+    {   // TODO: Duplicated code
         $this->clearCache();
 
         $inputs = [
@@ -176,7 +176,7 @@ class Reports extends Secure_Controller
      * @return string
      */
     public function summary_categories(string $start_date, string $end_date, string $sale_type, string $location_id = 'all'): string
-    {
+    {   // TODO: Duplicated code
         $this->clearCache();
 
         $inputs = [
@@ -493,7 +493,7 @@ class Reports extends Secure_Controller
      * @return string
      */
     public function summary_sales_taxes(string $start_date, string $end_date, string $sale_type, string $location_id = 'all'): string
-    {
+    {   // TODO: Duplicated code
         $this->clearCache();
 
         $inputs = [

app/Controllers/Sales.php

@@ -20,7 +20,6 @@ use App\Models\Stock_location;
 use App\Models\Tokens\Token_invoice_count;
 use App\Models\Tokens\Token_customer;
 use App\Models\Tokens\Token_invoice_sequence;
-use App\Traits\Controller\Shared;
 use CodeIgniter\HTTP\ResponseInterface;
 use Config\Services;
 use Config\OSPOS;
@@ -29,7 +28,6 @@ use stdClass;
 
 class Sales extends Secure_Controller
 {
-    use Shared;
     protected $helpers = ['file'];
     private Barcode_lib $barcode_lib;
     private Email_lib $email_lib;
@@ -733,7 +731,7 @@ class Sales extends Secure_Controller
             $data["customer_comments"] = $customer_info->comments;
             $data['tax_id'] = $customer_info->tax_id;
         }
-        $tax_details = $this->tax_lib->get_taxes($data['cart']);
+        $tax_details = $this->tax_lib->get_taxes($data['cart']);    // TODO: Duplicated code
         $data['taxes'] = $tax_details[0];
         $data['discount'] = $this->sale_lib->get_discount();
         $data['payments'] = $this->sale_lib->get_payments();
@@ -745,7 +743,7 @@ class Sales extends Secure_Controller
         $data['payments_total'] = $totals['payment_total'];
         $data['payments_cover_total'] = $totals['payments_cover_total'];
         $data['cash_rounding'] = $this->session->get('cash_rounding');
-        $data['cash_mode'] = $this->session->get('cash_mode');
+        $data['cash_mode'] = $this->session->get('cash_mode');    // TODO: Duplicated code
         $data['prediscount_subtotal'] = $totals['prediscount_subtotal'];
         $data['cash_total'] = $totals['cash_total'];
         $data['non_cash_total'] = $totals['total'];
@@ -1100,7 +1098,7 @@ class Sales extends Secure_Controller
         $data['subtotal'] = $totals['subtotal'];
         $data['payments_total'] = $totals['payment_total'];
         $data['payments_cover_total'] = $totals['payments_cover_total'];
-        $data['cash_mode'] = $this->session->get('cash_mode');.
+        $data['cash_mode'] = $this->session->get('cash_mode');    // TODO: Duplicated code.
         $data['prediscount_subtotal'] = $totals['prediscount_subtotal'];
         $data['cash_total'] = $totals['cash_total'];
         $data['non_cash_total'] = $totals['total'];
@@ -1128,15 +1126,35 @@ class Sales extends Secure_Controller
         $data['quote_number'] = $sale_info['quote_number'];
         $data['sale_status'] = $sale_info['sale_status'];
 
-        $data['company_info'] = $this->buildCompanyInfo();
+        $data['company_info'] = implode("\n", [$this->config['address'], $this->config['phone']]);    // TODO: Duplicated code.
+
+        if ($this->config['account_number']) {
+            $data['company_info'] .= "\n" . lang('Sales.account_number') . ": " . $this->config['account_number'];
+        }
+        if ($this->config['tax_id'] != '') {
+            $data['company_info'] .= "\n" . lang('Sales.tax_id') . ": " . $this->config['tax_id'];
+        }
 
         $data['barcode'] = $this->barcode_lib->generate_receipt_barcode($data['sale_id']);
         $data['print_after_sale'] = false;
         $data['price_work_orders'] = false;
 
-        $modeData = $this->getSaleModeLabel($this->sale_lib->get_mode());
-        $data['mode_label'] = $modeData['mode_label'];
-        $data['customer_required'] = $modeData['customer_required'];
+        if ($this->sale_lib->get_mode() == 'sale_invoice') {    // TODO: Duplicated code.
+            $data['mode_label'] = lang('Sales.invoice');
+            $data['customer_required'] = lang('Sales.customer_required');
+        } elseif ($this->sale_lib->get_mode() == 'sale_quote') {
+            $data['mode_label'] = lang('Sales.quote');
+            $data['customer_required'] = lang('Sales.customer_required');
+        } elseif ($this->sale_lib->get_mode() == 'sale_work_order') {
+            $data['mode_label'] = lang('Sales.work_order');
+            $data['customer_required'] = lang('Sales.customer_required');
+        } elseif ($this->sale_lib->get_mode() == 'return') {
+            $data['mode_label'] = lang('Sales.return');
+            $data['customer_required'] = lang('Sales.customer_optional');
+        } else {
+            $data['mode_label'] = lang('Sales.receipt');
+            $data['customer_required'] = lang('Sales.customer_optional');
+        }
 
         $invoice_type = $this->config['invoice_type'];
         if (!Sale_lib::isValidInvoiceType($invoice_type)) {
@@ -1174,7 +1192,7 @@ class Sales extends Secure_Controller
         $data['stock_locations'] = $this->stock_location->get_allowed_locations('sales');
         $data['stock_location'] = $this->sale_lib->get_sale_location();
         $data['tax_exclusive_subtotal'] = $this->sale_lib->get_subtotal(true, true);
-        $tax_details = $this->tax_lib->get_taxes($data['cart']);.
+        $tax_details = $this->tax_lib->get_taxes($data['cart']);    // TODO: Duplicated code.
         $data['taxes'] = $tax_details[0];
         $data['discount'] = $this->sale_lib->get_discount();
         $data['payments'] = $this->sale_lib->get_payments();
@@ -1192,7 +1210,7 @@ class Sales extends Secure_Controller
         // cash_mode indicates whether this sale is going to be processed using cash_rounding
         $cash_mode = $this->session->get('cash_mode');
         $data['cash_mode'] = $cash_mode;
-        $data['prediscount_subtotal'] = $totals['prediscount_subtotal'];.
+        $data['prediscount_subtotal'] = $totals['prediscount_subtotal'];    // TODO: Duplicated code.
         $data['cash_total'] = $totals['cash_total'];
         $data['non_cash_total'] = $totals['total'];
         $data['cash_amount_due'] = $totals['cash_amount_due'];
@@ -1239,9 +1257,23 @@ class Sales extends Secure_Controller
         $data['quote_number'] = $this->sale_lib->get_quote_number();
         $data['work_order_number'] = $this->sale_lib->get_work_order_number();
 
-        $modeData = $this->getSaleModeLabel($this->sale_lib->get_mode());
-        $data['mode_label'] = $modeData['mode_label'];
-        $data['customer_required'] = $modeData['customer_required'];
+        // TODO: the if/else set below should be converted to a switch
+        if ($this->sale_lib->get_mode() == 'sale_invoice') {    // TODO: Duplicated code.
+            $data['mode_label'] = lang('Sales.invoice');
+            $data['customer_required'] = lang('Sales.customer_required');
+        } elseif ($this->sale_lib->get_mode() == 'sale_quote') {
+            $data['mode_label'] = lang('Sales.quote');
+            $data['customer_required'] = lang('Sales.customer_required');
+        } elseif ($this->sale_lib->get_mode() == 'sale_work_order') {
+            $data['mode_label'] = lang('Sales.work_order');
+            $data['customer_required'] = lang('Sales.customer_required');
+        } elseif ($this->sale_lib->get_mode() == 'return') {
+            $data['mode_label'] = lang('Sales.return');
+            $data['customer_required'] = lang('Sales.customer_optional');
+        } else {
+            $data['mode_label'] = lang('Sales.receipt');
+            $data['customer_required'] = lang('Sales.customer_optional');
+        }
 
         return view("sales/register", $data);
     }

app/Controllers/Taxes.php

@@ -8,14 +8,12 @@ use App\Models\Tax;
 use App\Models\Tax_category;
 use App\Models\Tax_code;
 use App\Models\Tax_jurisdiction;
-use App\Traits\Controller\Shared;
 use CodeIgniter\HTTP\ResponseInterface;
 use Config\OSPOS;
 use Config\Services;
 
 class Taxes extends Secure_Controller
 {
-    use Shared;
     private array $config;
     private Tax_lib $tax_lib;
     private Tax $tax;
@@ -142,26 +140,44 @@ class Taxes extends Secure_Controller
     {
         $tax_code_info = $this->tax->get_info($tax_code);
 
-        $default_tax_category_id = 1; // Tax category id is always the default tax category    // TODO: This variable is not used anywhere in the code
-        $default_tax_category = $this->tax->get_tax_category($default_tax_category_id);    // TODO: This variable is not used anywhere in the code
+        $default_tax_category_id = 1; // Tax category id is always the default tax category    // TODO: Replace 1 with constant
+        $default_tax_category = $this->tax->get_tax_category($default_tax_category_id);    // TODO: this variable is never used in the code.
 
         $tax_rate_info = $this->tax->get_rate_info($tax_code, $default_tax_category_id);
 
-        $data['rounding_options'] = Rounding_mode::get_rounding_options();
-        $data['html_rounding_options'] = $this->get_html_rounding_options();
-
         if ($this->config['tax_included']) {
             $data['default_tax_type'] = Tax_lib::TAX_TYPE_INCLUDED;
         } else {
             $data['default_tax_type'] = Tax_lib::TAX_TYPE_EXCLUDED;
         }
 
-        if ($tax_code == NEW_ENTRY) {
-            $taxData = $this->initDefaultTaxCodeData();
-            $data = array_merge($data, $taxData);
+        $data['rounding_options'] = Rounding_mode::get_rounding_options();
+        $data['html_rounding_options'] = $this->get_html_rounding_options();
+
+        if ($tax_code == NEW_ENTRY) {   // TODO: Duplicated code
+            $data['tax_code'] = '';
+            $data['tax_code_name'] = '';
+            $data['tax_code_type'] = '0';
+            $data['city'] = '';
+            $data['state'] = '';
+            $data['tax_rate'] = '0.0000';
+            $data['rate_tax_code'] = '';
+            $data['rate_tax_category_id'] = 1;
+            $data['tax_category'] = '';
+            $data['add_tax_category'] = '';
+            $data['rounding_code'] = '0';
         } else {
-            $taxData = $this->buildTaxCodeData($tax_code_info, $tax_rate_info);
-            $data = array_merge($data, $taxData);
+            $data['tax_code'] = $tax_code;
+            $data['tax_code_name'] = $tax_code_info->tax_code_name;
+            $data['tax_code_type'] = $tax_code_info->tax_code_type;
+            $data['city'] = $tax_code_info->city;
+            $data['state'] = $tax_code_info->state;
+            $data['rate_tax_code'] = $tax_code_info->rate_tax_code;
+            $data['rate_tax_category_id'] = $tax_code_info->rate_tax_category_id;
+            $data['tax_category'] = $tax_code_info->tax_category;
+            $data['add_tax_category'] = '';
+            $data['tax_rate'] = $tax_rate_info->tax_rate;
+            $data['rounding_code'] = $tax_rate_info->rounding_code;
         }
 
         $tax_rates = [];
@@ -284,7 +300,7 @@ class Taxes extends Secure_Controller
      */
     public function getView_tax_jurisdictions(int $tax_code = NEW_ENTRY): string // TODO: This appears to be called no where in the code.
     {
-        $tax_code_info = $this->tax->get_info($tax_code);
+        $tax_code_info = $this->tax->get_info($tax_code);    // TODO: Duplicated code
 
         $default_tax_category_id = 1; // Tax category id is always the default tax category
         $default_tax_category = $this->tax->get_tax_category($default_tax_category_id);    // TODO: This variable is not used anywhere in the code

app/Database/Migrations/20170502221506_sales_tax_data.php

@@ -4,15 +4,16 @@ namespace App\Database\Migrations;
 
 use App\Libraries\Tax_lib;
 use App\Models\Appconfig;
-use App\Traits\Database\SalesTaxMigration;
 use CodeIgniter\Database\Migration;
 use CodeIgniter\Database\ResultInterface;
 
+/**
+ * @property tax_lib tax_lib
+ * @property appconfig appconfig
+ */
 class Migration_Sales_Tax_Data extends Migration
 {
-    use SalesTaxMigration;
-    
-    public const ROUND_UP = 5;
+    public const ROUND_UP = 5;    // TODO: These need to be moved to constants.php
     public const ROUND_DOWN = 6;
     public const HALF_FIVE = 7;
     public const YES = '1';
@@ -326,18 +327,79 @@ class Migration_Sales_Tax_Data extends Migration
         }
     }
 
-    public function clean(string $string): string
+    /**
+     * @param string $string
+     * @return string
+     */
+    public function clean(string $string): string    // TODO: $string is not a good name for this variable
     {
-        return $this->cleanIdentifier($string);
+        $string = str_replace(' ', '-', $string); // Replaces all spaces with hyphens.
+
+        return preg_replace('/[^A-Za-z0-9\-]/', '', $string); // Removes special chars.
     }
 
+    /**
+     * @param array $sales_taxes
+     * @return void
+     */
     public function apply_invoice_taxing(array &$sales_taxes): void
     {
-        $this->applyInvoiceTaxing($sales_taxes);
+        if (!empty($sales_taxes)) {    // TODO: Duplicated code
+            $sort = [];
+
+            foreach ($sales_taxes as $key => $value) {
+                $sort['print_sequence'][$key] = $value['print_sequence'];
+            }
+
+            array_multisort($sort['print_sequence'], SORT_ASC, $sales_taxes);
+        }
+
+        $decimals = totals_decimals();
+
+        foreach ($sales_taxes as $row_number => $sales_tax) {
+            $sales_taxes[$row_number]['sale_tax_amount'] = $this->get_sales_tax_for_amount($sales_tax['sale_tax_basis'], $sales_tax['tax_rate'], $sales_tax['rounding_code'], $decimals);
+        }
     }
 
+    /**
+     * @param array $sales_taxes
+     * @return void
+     */
     public function round_sales_taxes(array &$sales_taxes): void
     {
-        $this->roundSalesTaxes($sales_taxes, self::ROUND_UP, self::ROUND_DOWN, self::HALF_FIVE);
+        if (!empty($sales_taxes)) {
+            $sort = [];
+            foreach ($sales_taxes as $k => $v) {
+                $sort['print_sequence'][$k] = $v['print_sequence'];
+            }
+            array_multisort($sort['print_sequence'], SORT_ASC, $sales_taxes);
+        }
+
+        $decimals = totals_decimals();
+
+        foreach ($sales_taxes as $row_number => $sales_tax) {
+            $sale_tax_amount = (float)$sales_tax['sale_tax_amount'];
+            $rounding_code = $sales_tax['rounding_code'];
+            $rounded_sale_tax_amount = $sale_tax_amount;
+
+            if (
+                $rounding_code == PHP_ROUND_HALF_UP
+                || $rounding_code == PHP_ROUND_HALF_DOWN
+                || $rounding_code == PHP_ROUND_HALF_EVEN
+                || $rounding_code == PHP_ROUND_HALF_ODD
+            ) {
+                $rounded_sale_tax_amount = round($sale_tax_amount, $decimals, $rounding_code);
+            } elseif ($rounding_code == Migration_Sales_Tax_Data::ROUND_UP) {
+                $fig = (int) str_pad('1', $decimals, '0');
+                $rounded_sale_tax_amount = (ceil($sale_tax_amount * $fig) / $fig);
+            } elseif ($rounding_code == Migration_Sales_Tax_Data::ROUND_DOWN) {
+                $fig = (int) str_pad('1', $decimals, '0');
+                $rounded_sale_tax_amount = (floor($sale_tax_amount * $fig) / $fig);
+            } elseif ($rounding_code == Migration_Sales_Tax_Data::HALF_FIVE) {
+                $rounded_sale_tax_amount = round($sale_tax_amount / 5) * 5;
+            }
+
+            $sales_taxes[$row_number]['sale_tax_amount'] = $rounded_sale_tax_amount;
+        }
     }
 }

app/Database/Migrations/20200202000000_taxamount.php

@@ -2,22 +2,25 @@
 
 namespace App\Database\Migrations;
 
-use App\Traits\Database\SalesTaxMigration;
 use CodeIgniter\Database\Migration;
 use App\Libraries\Tax_lib;
 use App\Models\Appconfig;
 use CodeIgniter\Database\ResultInterface;
 
+/**
+ *
+ *
+ * @property appconfig appconfig
+ * @property tax_lib tax_lib
+ */
 class Migration_TaxAmount extends Migration
 {
-    use SalesTaxMigration;
-    
     public const ROUND_UP = 5;
     public const ROUND_DOWN = 6;
     public const HALF_FIVE = 7;
     public const YES = '1';
     public const VAT_TAX = '0';
-    public const SALES_TAX = '1';
+    public const SALES_TAX = '1';    // TODO: It appears that this constant is never used
     private Appconfig $appconfig;
 
     public function __construct()
@@ -302,18 +305,79 @@ class Migration_TaxAmount extends Migration
         }
     }
 
-    public function clean(string $string): string
+    /**
+     * @param string $string
+     * @return string
+     */
+    public function clean(string $string): string    // TODO: This can probably go into the migration helper as it's used it more than one migration. Also, $string needs to be refactored to a different name.
     {
-        return $this->cleanIdentifier($string);
+        $string = str_replace(' ', '-', $string); // Replaces all spaces with hyphens.
+
+        return preg_replace('/[^A-Za-z0-9\-]/', '', $string); // Removes special chars.
     }
 
+    /**
+     * @param array $sales_taxes
+     * @return void
+     */
     public function apply_invoice_taxing(array &$sales_taxes): void
     {
-        $this->applyInvoiceTaxing($sales_taxes);
+        if (!empty($sales_taxes)) {    // TODO: Duplicated code
+            $sort = [];
+            foreach ($sales_taxes as $k => $v) {
+                $sort['print_sequence'][$k] = $v['print_sequence'];
+            }
+            array_multisort($sort['print_sequence'], SORT_ASC, $sales_taxes);
+        }
+
+        $decimals = totals_decimals();
+
+        foreach ($sales_taxes as $row_number => $sales_tax) {
+            $sales_taxes[$row_number]['sale_tax_amount'] = $this->get_sales_tax_for_amount($sales_tax['sale_tax_basis'], $sales_tax['tax_rate'], $sales_tax['rounding_code'], $decimals);
+        }
     }
 
+    /**
+     * @param array $sales_taxes
+     * @return void
+     */
     public function round_sales_taxes(array &$sales_taxes): void
     {
-        $this->roundSalesTaxes($sales_taxes, self::ROUND_UP, self::ROUND_DOWN, self::HALF_FIVE);
+        if (!empty($sales_taxes)) {
+            $sort = [];
+
+            foreach ($sales_taxes as $k => $v) {
+                $sort['print_sequence'][$k] = $v['print_sequence'];
+            }
+
+            array_multisort($sort['print_sequence'], SORT_ASC, $sales_taxes);
+        }
+
+        $decimals = totals_decimals();
+
+        foreach ($sales_taxes as $row_number => $sales_tax) {
+            $sale_tax_amount = (float)$sales_tax['sale_tax_amount'];
+            $rounding_code = $sales_tax['rounding_code'];
+            $rounded_sale_tax_amount = $sale_tax_amount;
+
+            if (
+                $rounding_code == PHP_ROUND_HALF_UP    // TODO: This block of if/elseif statements can be converted to a switch.
+                || $rounding_code == PHP_ROUND_HALF_DOWN
+                || $rounding_code == PHP_ROUND_HALF_EVEN
+                || $rounding_code == PHP_ROUND_HALF_ODD
+            ) {
+                $rounded_sale_tax_amount = round($sale_tax_amount, $decimals, $rounding_code);
+            } elseif ($rounding_code == Migration_TaxAmount::ROUND_UP) {
+                $fig = (int) str_pad('1', $decimals, '0');
+                $rounded_sale_tax_amount = (ceil($sale_tax_amount * $fig) / $fig);
+            } elseif ($rounding_code == Migration_TaxAmount::ROUND_DOWN) {
+                $fig = (int) str_pad('1', $decimals, '0');
+                $rounded_sale_tax_amount = (floor($sale_tax_amount * $fig) / $fig);
+            } elseif ($rounding_code == Migration_TaxAmount::HALF_FIVE) {
+                $rounded_sale_tax_amount = round($sale_tax_amount / 5) * 5;
+            }
+
+            $sales_taxes[$row_number]['sale_tax_amount'] = $rounded_sale_tax_amount;
+        }
     }
 }

app/Models/Attribute.php

@@ -472,7 +472,7 @@ class Attribute extends Model
             }
         } elseif ($from_type === DROPDOWN) {
             if (in_array($to_type, [TEXT, CHECKBOX], true)) {
-                if ($to_type === CHECKBOX) {
+                if ($to_type === CHECKBOX) {    // TODO: Duplicated code.
                     $checkbox_attribute_values = $this->checkbox_attribute_values($definition_id);
 
                     $this->db->transStart();

app/Models/Customer.php

@@ -423,7 +423,7 @@ class Customer extends Person
         $builder->orLike('phone_number', $search);
         $builder->orLike('account_number', $search);
         $builder->orLike('company_name', $search);
-        $builder->orLike('CONCAT(first_name, " ", last_name)', $search);
+        $builder->orLike('CONCAT(first_name, " ", last_name)', $search);    // TODO: Duplicated code.
         $builder->groupEnd();
         $builder->where('deleted', 0);
 

app/Models/Item.php

@@ -436,32 +436,62 @@ class Item extends Model
 
     /**
      * Inserts or updates an item
+     * 
+     * If the primary key (item_id) is present in the data array and the record exists,
+     * it will update the existing record. Otherwise, it will insert a new record.
+     * 
+     * @param array $data The item data to save (passed by reference to set item_id on insert)
+     * @return bool True on success, false on failure
      */
-    public function save_value(array &$item_data, int $item_id = NEW_ENTRY): bool    // TODO: need to bring this in line with parent or change the name
+    public function saveValue(array &$data): bool
     {
-        $builder = $this->db->table('items');
+        $primaryKey = $this->primaryKey;
+        $id = $data[$primaryKey] ?? NEW_ENTRY;
 
-        if ($item_id < 1 || !$this->exists($item_id, true)) {
-            if ($builder->insert($item_data)) {
-                $item_data['item_id'] = (int)$this->db->insertID();
-                if ($item_id < 1) {
-                    $builder = $this->db->table('items');
-                    $builder->where('item_id', $item_data['item_id']);
-                    $builder->update(['low_sell_item_id' => $item_data['item_id']]);
-                }
-
-                return true;
+        // If id > 0 and record exists by primary key only, update it
+        if ($id > 0) {
+            // Check existence strictly by primary key (regardless of soft-delete status)
+            $builder = $this->db->table('items');
+            $builder->where($primaryKey, $id);
+            $exists = $builder->countAllResults() > 0;
+            
+            if ($exists) {
+                // Remove primary key from data array for update
+                $updateData = $data;
+                unset($updateData[$primaryKey]);
+                
+                $builder = $this->db->table('items');
+                $builder->where($primaryKey, $id);
+                return $builder->update($updateData);
             }
-
-            return false;
-        } else {
-            $item_data['item_id'] = $item_id;
         }
 
+        // Insert new record with transaction for atomicity
+        $this->db->transBegin();
+        
+        // Remove primary key from insert payload if present
+        $insertData = $data;
+        unset($insertData[$primaryKey]);
+        
         $builder = $this->db->table('items');
-        $builder->where('item_id', $item_id);
-
-        return $builder->update($item_data);
+        $success = $builder->insert($insertData);
+        
+        if ($success) {
+            $data[$primaryKey] = (int)$this->db->insertID();
+            
+            // Update low_sell_item_id for new items
+            $builder = $this->db->table('items');
+            $builder->where($primaryKey, $data[$primaryKey]);
+            $success = $builder->update(['low_sell_item_id' => $data[$primaryKey]]);
+        }
+        
+        if ($success) {
+            $this->db->transCommit();
+            return true;
+        }
+        
+        $this->db->transRollback();
+        return false;
     }
 
     /**
@@ -1079,9 +1109,9 @@ class Item extends Model
         $total_quantity = $old_total_quantity + $items_received;
         $average_price = bcdiv(bcadd(bcmul((string)$items_received, (string)$new_price), bcmul((string)$old_total_quantity, (string)$old_price)), (string)$total_quantity);
 
-        $data = ['cost_price' => $average_price];
+        $data = ['cost_price' => $average_price, 'item_id' => $item_id];
 
-        return $this->save_value($data, $item_id);
+        return $this->saveValue($data);
     }
 
     /**

app/Models/Module.php

@@ -116,7 +116,7 @@ class Module extends Model
     public function get_allowed_office_modules(int $person_id): ResultInterface
     {
         $menus = ['office', 'both'];
-        $builder = $this->db->table('modules');
+        $builder = $this->db->table('modules');    // TODO: Duplicated code
         $builder->join('permissions', 'permissions.permission_id = modules.module_id');
         $builder->join('grants', 'permissions.permission_id = grants.permission_id');
         $builder->where('person_id', $person_id);

app/Models/Reports/Detailed_sales.php

@@ -3,21 +3,32 @@
 namespace App\Models\Reports;
 
 use App\Models\Sale;
-use App\Traits\Models\Reports\SaleTypeFilter;
 
+/**
+ *
+ *
+ * @property sale sale
+ *
+ */
 class Detailed_sales extends Report
 {
-    use SaleTypeFilter;
-
+    /**
+     * @param array $inputs
+     * @return void
+     */
     public function create(array $inputs): void
     {
+        // Create our temp tables to work with the data in our report
         $sale = model(Sale::class);
         $sale->create_temp_table($inputs);
     }
 
+    /**
+     * @return array
+     */
     public function getDataColumns(): array
     {
-        return [
+        return [    // TODO: Duplicated code
             'summary' => [
                 ['id'            => lang('Reports.sale_id')],
                 ['type_code'     => lang('Reports.code_type')],
@@ -108,11 +119,47 @@ class Detailed_sales extends Report
             MAX(payment_type) AS payment_type,
             MAX(comment) AS comment');
 
-        if ($inputs['location_id'] != 'all') {
+        if ($inputs['location_id'] != 'all') {    // TODO: Duplicated code
             $builder->where('item_location', $inputs['location_id']);
         }
 
-        $this->applySaleTypeFilter($builder, $inputs['sale_type'], false);
+        switch ($inputs['sale_type']) {
+            case 'complete':
+                $builder->where('sale_status', COMPLETED);
+                $builder->groupStart();
+                $builder->where('sale_type', SALE_TYPE_POS);
+                $builder->orWhere('sale_type', SALE_TYPE_INVOICE);
+                $builder->orWhere('sale_type', SALE_TYPE_RETURN);
+                $builder->groupEnd();
+                break;
+
+            case 'sales':
+                $builder->where('sale_status', COMPLETED);
+                $builder->groupStart();
+                $builder->where('sale_type', SALE_TYPE_POS);
+                $builder->orWhere('sale_type', SALE_TYPE_INVOICE);
+                $builder->groupEnd();
+                break;
+
+            case 'quotes':
+                $builder->where('sale_status', SUSPENDED);
+                $builder->where('sale_type', SALE_TYPE_QUOTE);
+                break;
+
+            case 'work_orders':
+                $builder->where('sale_status', SUSPENDED);
+                $builder->where('sale_type', SALE_TYPE_WORK_ORDER);
+                break;
+
+            case 'canceled':
+                $builder->where('sale_status', CANCELED);
+                break;
+
+            case 'returns':
+                $builder->where('sale_status', COMPLETED);
+                $builder->where('sale_type', SALE_TYPE_RETURN);
+                break;
+        }
 
         $builder->groupBy('sale_id');
         $builder->orderBy('MAX(sale_time)');
@@ -162,16 +209,56 @@ class Detailed_sales extends Report
         return $data;
     }
 
+    /**
+     * @param array $inputs
+     * @return array
+     */
     public function getSummaryData(array $inputs): array
     {
         $builder = $this->db->table('sales_items_temp');
         $builder->select('SUM(subtotal) AS subtotal, SUM(tax) AS tax, SUM(total) AS total, SUM(cost) AS cost, SUM(profit) AS profit');
 
-        if ($inputs['location_id'] != 'all') {
+        if ($inputs['location_id'] != 'all') {    // TODO: Duplicated code
             $builder->where('item_location', $inputs['location_id']);
         }
 
-        $this->applySaleTypeFilter($builder, $inputs['sale_type'], false);
+        switch ($inputs['sale_type']) {
+            case 'complete':
+                $builder->where('sale_status', COMPLETED);
+                $builder->groupStart();
+                $builder->where('sale_type', SALE_TYPE_POS);
+                $builder->orWhere('sale_type', SALE_TYPE_INVOICE);
+                $builder->orWhere('sale_type', SALE_TYPE_RETURN);
+                $builder->groupEnd();
+                break;
+
+            case 'sales':
+                $builder->where('sale_status', COMPLETED);
+                $builder->groupStart();
+                $builder->where('sale_type', SALE_TYPE_POS);
+                $builder->orWhere('sale_type', SALE_TYPE_INVOICE);
+                $builder->groupEnd();
+                break;
+
+            case 'quotes':
+                $builder->where('sale_status', SUSPENDED);
+                $builder->where('sale_type', SALE_TYPE_QUOTE);
+                break;
+
+            case 'work_orders':
+                $builder->where('sale_status', SUSPENDED);
+                $builder->where('sale_type', SALE_TYPE_WORK_ORDER);
+                break;
+
+            case 'canceled':
+                $builder->where('sale_status', CANCELED);
+                break;
+
+            case 'returns':
+                $builder->where('sale_status', COMPLETED);
+                $builder->where('sale_type', SALE_TYPE_RETURN);
+                break;
+        }
 
         return $builder->get()->getRowArray();
     }

app/Models/Reports/Specific_customer.php

@@ -93,7 +93,7 @@ class Specific_customer extends Report
             MAX(payment_type) AS payment_type,
             MAX(comment) AS comment');
 
-        $builder->where('customer_id', $inputs['customer_id']);
+        $builder->where('customer_id', $inputs['customer_id']);    // TODO: Duplicated code
 
         if ($inputs['payment_type'] == 'invoices') {
             $builder->where('sale_type', SALE_TYPE_INVOICE);
@@ -139,7 +139,7 @@ class Specific_customer extends Report
                 break;
         }
 
-        $builder->groupBy('sale_id');
+        $builder->groupBy('sale_id');    // TODO: Duplicated code
         $builder->orderBy('MAX(sale_time)');
 
         $data = [];

app/Models/Reports/Specific_discount.php

@@ -27,7 +27,7 @@ class Specific_discount extends Report
      * @return array
      */
     public function getDataColumns(): array
-    {
+    {    // TODO: Duplicated code
         return [
             'summary' => [
                 ['id'            => lang('Reports.sale_id')],
@@ -95,7 +95,7 @@ class Specific_discount extends Report
             MAX(payment_type) AS payment_type,
             MAX(comment) AS comment');
 
-        $builder->where('discount >=', $inputs['discount']);
+        $builder->where('discount >=', $inputs['discount']);    // TODO: Duplicated code
         $builder->where('discount_type', $inputs['discount_type']);
 
         switch ($inputs['sale_type']) {
@@ -136,7 +136,7 @@ class Specific_discount extends Report
                 break;
         }
 
-        $builder->groupBy('sale_id');
+        $builder->groupBy('sale_id');    // TODO: Duplicated code
         $builder->orderBy('MAX(sale_time)');
 
         $data = [];
@@ -168,7 +168,7 @@ class Specific_discount extends Report
         $builder = $this->db->table('sales_items_temp');
         $builder->select('SUM(subtotal) AS subtotal, SUM(tax) AS tax, SUM(total) AS total, SUM(cost) AS cost, SUM(profit) AS profit');
 
-        $builder->where('discount >=', $inputs['discount']);
+        $builder->where('discount >=', $inputs['discount']);    // TODO: Duplicated code
         $builder->where('discount_type', $inputs['discount_type']);
 
         // TODO: this needs to be converted to a switch statement

app/Models/Reports/Specific_employee.php

@@ -93,7 +93,7 @@ class Specific_employee extends Report
             MAX(payment_type) AS payment_type,
             MAX(comment) AS comment');
 
-        $builder->where('employee_id', $inputs['employee_id']);
+        $builder->where('employee_id', $inputs['employee_id']);    // TODO: Duplicated code
 
         switch ($inputs['sale_type']) {
             case 'complete':
@@ -164,7 +164,7 @@ class Specific_employee extends Report
     {
         $builder = $this->db->table('sales_items_temp');
         $builder->select('SUM(subtotal) AS subtotal, SUM(tax) AS tax, SUM(total) AS total, SUM(cost) AS cost, SUM(profit) AS profit');
-        $builder->where('employee_id', $inputs['employee_id']);
+        $builder->where('employee_id', $inputs['employee_id']);    // TODO: Duplicated code
 
         // TODO: this needs to be converted to a switch statement
         if ($inputs['sale_type'] == 'complete') {

app/Models/Reports/Specific_supplier.php

@@ -77,7 +77,7 @@ class Specific_supplier extends Report
             MAX(discount_type) AS discount_type,
             MAX(discount) AS discount');
 
-        $builder->where('supplier_id', $inputs['supplier_id']);
+        $builder->where('supplier_id', $inputs['supplier_id']);    // TODO: Duplicated code
 
         switch ($inputs['sale_type']) {
             case 'complete':

app/Models/Reports/Summary_expenses_categories.php

@@ -2,14 +2,14 @@
 
 namespace App\Models\Reports;
 
-use App\Traits\Models\Reports\ReportDateFilter;
 use Config\OSPOS;
 
 class Summary_expenses_categories extends Summary_report
 {
-    use ReportDateFilter;
-
-    protected function _get_data_columns(): array
+    /**
+     * @return array[]
+     */
+    protected function _get_data_columns(): array    // TODO: Hungarian notation
     {
         return [
             ['category_name'    => lang('Reports.expenses_category')],
@@ -19,47 +19,6 @@ class Summary_expenses_categories extends Summary_report
         ];
     }
 
-    public function getData(array $inputs): array
-    {
-        $config = config(OSPOS::class)->settings;
-
-        $builder = $this->db->table('expenses AS expenses');
-        $builder->select('expense_categories.category_name AS category_name, COUNT(expenses.expense_id) AS count, SUM(expenses.amount) AS total_amount, SUM(expenses.tax_amount) AS total_tax_amount');
-        $builder->join('expense_categories AS expense_categories', 'expense_categories.expense_category_id = expenses.expense_category_id', 'LEFT');
-
-        if (empty($config['date_or_time_format'])) {
-            $builder->where('DATE(expenses.date) BETWEEN ' . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']));
-        } else {
-            $builder->where('expenses.date BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date'])));
-        }
-
-        $builder->where('expenses.deleted', 0);
-
-        $builder->groupBy('expense_categories.category_name');
-        $builder->orderBy('expense_categories.category_name');
-
-        return $builder->get()->getResultArray();
-    }
-
-    public function getSummaryData(array $inputs): array
-    {
-        $config = config(OSPOS::class)->settings;
-
-        $builder = $this->db->table('expenses AS expenses');
-        $builder->select('SUM(expenses.amount) AS expenses_total_amount, SUM(expenses.tax_amount) AS expenses_total_tax_amount');
-
-        if (empty($config['date_or_time_format'])) {
-            $builder->where('DATE(expenses.date) BETWEEN ' . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']));
-        } else {
-            $builder->where('expenses.date BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date']));
-        }
-
-        $builder->where('expenses.deleted', 0);
-
-        return $builder->get()->getRowArray();
-    }
-}
-
     /**
      * @param array $inputs
      * @return array
@@ -72,7 +31,8 @@ class Summary_expenses_categories extends Summary_report
         $builder->select('expense_categories.category_name AS category_name, COUNT(expenses.expense_id) AS count, SUM(expenses.amount) AS total_amount, SUM(expenses.tax_amount) AS total_tax_amount');
         $builder->join('expense_categories AS expense_categories', 'expense_categories.expense_category_id = expenses.expense_category_id', 'LEFT');
 
-        if (empty($config['date_or_time_format'])) {
+        // TODO: convert this to ternary notation
+        if (empty($config['date_or_time_format'])) {    // TODO: Duplicated code
             $builder->where('DATE(expenses.date) BETWEEN ' . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']));
         } else {
             $builder->where('expenses.date BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date'])));
@@ -86,6 +46,10 @@ class Summary_expenses_categories extends Summary_report
         return $builder->get()->getResultArray();
     }
 
+    /**
+     * @param array $inputs
+     * @return array
+     */
     public function getSummaryData(array $inputs): array
     {
         $config = config(OSPOS::class)->settings;
@@ -93,7 +57,7 @@ class Summary_expenses_categories extends Summary_report
         $builder = $this->db->table('expenses AS expenses');
         $builder->select('SUM(expenses.amount) AS expenses_total_amount, SUM(expenses.tax_amount) AS expenses_total_tax_amount');
 
-        if (empty($config['date_or_time_format'])) {
+        if (empty($config['date_or_time_format'])) {    // TODO: Duplicated code
             $builder->where('DATE(expenses.date) BETWEEN ' . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']));
         } else {
             $builder->where('expenses.date BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date'])));

app/Models/Reports/Summary_payments.php

@@ -2,14 +2,14 @@
 
 namespace App\Models\Reports;
 
-use App\Traits\Models\Reports\ReportDateFilter;
 use Config\OSPOS;
 
 class Summary_payments extends Summary_report
 {
-    use ReportDateFilter;
-
-    protected function _get_data_columns(): array
+    /**
+     * @return array[]
+     */
+    protected function _get_data_columns(): array    // TODO: Hungarian notation
     {
         return [
             ['trans_group'    => lang('Reports.trans_group')],
@@ -22,9 +22,13 @@ class Summary_payments extends Summary_report
         ];
     }
 
+    /**
+     * @param array $inputs
+     * @return array
+     */
     public function getData(array $inputs): array
     {
-        $cash_payment = lang('Sales.cash');
+        $cash_payment = lang('Sales.cash');    // TODO: This is never used.  Should it be?
         $config = config(OSPOS::class)->settings;
 
         $separator[] = [
@@ -37,7 +41,14 @@ class Summary_payments extends Summary_report
             'trans_due'      => ''
         ];
 
-        $where = $this->buildDateWhereClause($inputs);
+        $where = '';    // TODO: Duplicated code
+
+        // TODO: this needs to be converted to ternary notation
+        if (empty($config['date_or_time_format'])) {
+            $where .= 'DATE(sale_time) BETWEEN ' . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']);
+        } else {
+            $where .= 'sale_time BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date']));
+        }
 
         $this->create_summary_payments_temp_tables($where);
 

app/Models/Reports/Summary_report.php

@@ -2,20 +2,25 @@
 
 namespace App\Models\Reports;
 
-use App\Traits\Models\Reports\ReportDateFilter;
-use App\Traits\Models\Reports\SaleTypeFilter;
-use CodeIgniter\Database\BaseBuilder;
 use Config\OSPOS;
+use CodeIgniter\Database\BaseBuilder;
 
 abstract class Summary_report extends Report
 {
-    use ReportDateFilter;
-    use SaleTypeFilter;
-
-    private function __common_select(array $inputs, &$builder): void
+    /**
+     * Private interface implementing the core basic functionality for all reports
+     */
+    private function __common_select(array $inputs, &$builder): void    // TODO: Hungarian notation
     {
         $config = config(OSPOS::class)->settings;
-        $where = $this->buildDateWhereClause($inputs);
+        // TODO: convert to using QueryBuilder. Use App/Models/Reports/Summary_taxes.php getData() as a reference template
+        $where = '';    // TODO: Duplicated code
+
+        if (empty($config['date_or_time_format'])) {
+            $where .= 'DATE(sale_time) BETWEEN ' . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']);
+        } else {
+            $where .= 'sale_time BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date']));
+        }
 
         $decimals = totals_decimals();
 
@@ -105,16 +110,48 @@ abstract class Summary_report extends Report
     {
         $config = config(OSPOS::class)->settings;
 
-        $this->applyDateFilter($builder, $inputs);
+        // TODO: Probably going to need to rework these since you can't reference $builder without it's instantiation.
+        if (empty($config['date_or_time_format'])) {    // TODO: Duplicated code
+            $builder->where('DATE(sales.sale_time) BETWEEN ' . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']));
+        } else {
+            $builder->where('sales.sale_time BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date'])));
+        }
 
         if ($inputs['location_id'] != 'all') {
             $builder->where('sales_items.item_location', $inputs['location_id']);
         }
 
-        $this->applySaleTypeFilter($builder, $inputs['sale_type']);
+        if ($inputs['sale_type'] == 'complete') {
+            $builder->where('sales.sale_status', COMPLETED);
+            $builder->groupStart();
+            $builder->where('sales.sale_type', SALE_TYPE_POS);
+            $builder->orWhere('sales.sale_type', SALE_TYPE_INVOICE);
+            $builder->orWhere('sales.sale_type', SALE_TYPE_RETURN);
+            $builder->groupEnd();
+        } elseif ($inputs['sale_type'] == 'sales') {
+            $builder->where('sales.sale_status', COMPLETED);
+            $builder->groupStart();
+            $builder->where('sales.sale_type', SALE_TYPE_POS);
+            $builder->orWhere('sales.sale_type', SALE_TYPE_INVOICE);
+            $builder->groupEnd();
+        } elseif ($inputs['sale_type'] == 'quotes') {
+            $builder->where('sales.sale_status', SUSPENDED);
+            $builder->where('sales.sale_type', SALE_TYPE_QUOTE);
+        } elseif ($inputs['sale_type'] == 'work_orders') {
+            $builder->where('sales.sale_status', SUSPENDED);
+            $builder->where('sales.sale_type', SALE_TYPE_WORK_ORDER);
+        } elseif ($inputs['sale_type'] == 'canceled') {
+            $builder->where('sales.sale_status', CANCELED);
+        } elseif ($inputs['sale_type'] == 'returns') {
+            $builder->where('sales.sale_status', COMPLETED);
+            $builder->where('sales.sale_type', SALE_TYPE_RETURN);
+        }
     }
 
-    abstract protected function _get_data_columns(): array;
+    /**
+     * Protected class interface implemented by derived classes where required
+     */
+    abstract protected function _get_data_columns(): array;    // TODO: hungarian notation
 
     /**
      * @param array $inputs

app/Models/Reports/Summary_sales_taxes.php

@@ -2,13 +2,10 @@
 
 namespace App\Models\Reports;
 
-use App\Traits\Models\Reports\ReportDateFilter;
 use Config\OSPOS;
 
 class Summary_sales_taxes extends Summary_report
 {
-    use ReportDateFilter;
-
     private array $config;
 
     public function __construct()
@@ -17,7 +14,10 @@ class Summary_sales_taxes extends Summary_report
         $this->config = config(OSPOS::class)->settings;
     }
 
-    protected function _get_data_columns(): array
+    /**
+     * @return array[]
+     */
+    protected function _get_data_columns(): array    // TODO: hungarian notation
     {
         return [
             ['reporting_authority' => lang('Reports.authority')],
@@ -28,16 +28,35 @@ class Summary_sales_taxes extends Summary_report
         ];
     }
 
-    protected function _where(array $inputs, object &$builder): void
+    /**
+     * @param array $inputs
+     * @param object $builder
+     * @return void
+     */
+    protected function _where(array $inputs, object &$builder): void    // TODO: hungarian notation
     {
         $builder->where('sales.sale_status', COMPLETED);
-        $this->applyDateFilter($builder, $inputs, 'sales', 'sale_time');
+
+        if (empty($this->config['date_or_time_format'])) {    // TODO: Duplicated code
+            $builder->where('DATE(sales.sale_time) BETWEEN ' . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']));
+        } else {
+            $builder->where('sales.sale_time BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date'])));
+        }
     }
 
+    /**
+     * @param array $inputs
+     * @return array
+     */
     public function getData(array $inputs): array
     {
         $builder = $this->db->table('sales_taxes');
-        $this->applyDateFilter($builder, $inputs, 'sales_taxes', 'sale_time');
+
+        if (empty($this->config['date_or_time_format'])) {
+            $builder->where('DATE(sale_time) BETWEEN ' . $inputs['start_date'] . ' AND ' . $inputs['end_date']);
+        } else {
+            $builder->where('sale_time BETWEEN ' . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date'])));
+        }
 
         $builder->select('reporting_authority, jurisdiction_name, tax_category, tax_rate, SUM(sale_tax_amount) AS tax');
         $builder->join('sales', 'sales_taxes.sale_id = sales.sale_id', 'left');

app/Traits/Controller/Shared.php

@@ -1,130 +0,0 @@
-<?php
-
-namespace App\Traits\Controller;
-
-use Config\OSPOS;
-
-/**
- * Shared trait for common controller functionality
- */
-trait Shared
-{
-    /**
-     * Build supplier info array for views
-     * 
-     * @param object $supplier_info Supplier info object
-     * @param array $data Data array to populate
-     * @return void
-     */
-    protected function buildSupplierInfo(object $supplier_info, array &$data): void
-    {
-        $data['supplier'] = $supplier_info->company_name;
-        $data['first_name'] = $supplier_info->first_name;
-        $data['last_name'] = $supplier_info->last_name;
-        $data['supplier_email'] = $supplier_info->email;
-        $data['supplier_address'] = $supplier_info->address_1;
-        if (!empty($supplier_info->zip) || !empty($supplier_info->city)) {
-            $data['supplier_location'] = $supplier_info->zip . ' ' . $supplier_info->city;
-        } else {
-            $data['supplier_location'] = '';
-        }
-    }
-
-    /**
-     * Get mode label and customer required text based on sale mode
-     * 
-     * @param string $mode The sale mode
-     * @return array{mode_label: string, customer_required: string}
-     */
-    protected function getSaleModeLabel(string $mode): array
-    {
-        return match ($mode) {
-            'sale_invoice' => [
-                'mode_label' => lang('Sales.invoice'),
-                'customer_required' => lang('Sales.customer_required')
-            ],
-            'sale_quote' => [
-                'mode_label' => lang('Sales.quote'),
-                'customer_required' => lang('Sales.customer_required')
-            ],
-            'sale_work_order' => [
-                'mode_label' => lang('Sales.work_order'),
-                'customer_required' => lang('Sales.customer_required')
-            ],
-            'return' => [
-                'mode_label' => lang('Sales.return'),
-                'customer_required' => lang('Sales.customer_optional')
-            ],
-            default => [
-                'mode_label' => lang('Sales.receipt'),
-                'customer_required' => lang('Sales.customer_optional')
-            ]
-        };
-    }
-
-    /**
-     * Build company info string from config
-     * 
-     * @return string
-     */
-    protected function buildCompanyInfo(): string
-    {
-        $config = config(OSPOS::class)->settings;
-        $company_info = implode("\n", [$config['address'], $config['phone']]);
-
-        if (!empty($config['account_number'])) {
-            $company_info .= "\n" . lang('Sales.account_number') . ": " . $config['account_number'];
-        }
-        if (!empty($config['tax_id'])) {
-            $company_info .= "\n" . lang('Sales.tax_id') . ": " . $config['tax_id'];
-        }
-
-        return $company_info;
-    }
-
-    /**
-     * Initialize default tax code data for new entry
-     * 
-     * @return array
-     */
-    protected function initDefaultTaxCodeData(): array
-    {
-        return [
-            'tax_code' => '',
-            'tax_code_name' => '',
-            'tax_code_type' => '0',
-            'city' => '',
-            'state' => '',
-            'tax_rate' => '0.0000',
-            'rate_tax_code' => '',
-            'rate_tax_category_id' => 1,
-            'tax_category' => '',
-            'add_tax_category' => '',
-            'rounding_code' => '0'
-        ];
-    }
-
-    /**
-     * Populate tax code data from existing tax code info
-     * 
-     * @param object $tax_code_info Tax code info object
-     * @param object $tax_rate_info Tax rate info object
-     * @return array
-     */
-    protected function buildTaxCodeData(object $tax_code_info, object $tax_rate_info): array
-    {
-        return [
-            'tax_code' => $tax_code_info->tax_code,
-            'tax_code_name' => $tax_code_info->tax_code_name,
-            'tax_code_type' => $tax_code_info->tax_code_type,
-            'city' => $tax_code_info->city,
-            'state' => $tax_code_info->state,
-            'rate_tax_code' => $tax_code_info->rate_tax_code,
-            'rate_tax_category_id' => $tax_code_info->rate_tax_category_id,
-            'tax_category' => $tax_code_info->tax_category,
-            'add_tax_category' => '',
-            'tax_rate' => $tax_rate_info->tax_rate,
-            'rounding_code' => $tax_rate_info->rounding_code
-        ];
-    }
-}

app/Traits/Database/SalesTaxMigration.php

@@ -1,72 +0,0 @@
-<?php
-
-namespace App\Traits\Database;
-
-trait SalesTaxMigration
-{
-    protected function cleanIdentifier(string $string): string
-    {
-        $string = str_replace(' ', '-', $string);
-        return preg_replace('/[^A-Za-z0-9\-]/', '', $string);
-    }
-
-    protected function applyInvoiceTaxing(array &$salesTaxes): void
-    {
-        if (!empty($salesTaxes)) {
-            $sort = [];
-            foreach ($salesTaxes as $k => $v) {
-                $sort['print_sequence'][$k] = $v['print_sequence'];
-            }
-            array_multisort($sort['print_sequence'], SORT_ASC, $salesTaxes);
-        }
-
-        $decimals = totals_decimals();
-
-        foreach ($salesTaxes as $rowNumber => $salesTax) {
-            $salesTaxes[$rowNumber]['sale_tax_amount'] = $this->getSalesTaxForAmount(
-                $salesTax['sale_tax_basis'],
-                $salesTax['tax_rate'],
-                $salesTax['rounding_code'],
-                $decimals
-            );
-        }
-    }
-
-    protected function roundSalesTaxes(array &$salesTaxes, int $halfUp = 1, int $roundUp = 5, int $roundDown = 6, int $halfFive = 7): void
-    {
-        if (!empty($salesTaxes)) {
-            $sort = [];
-            foreach ($salesTaxes as $k => $v) {
-                $sort['print_sequence'][$k] = $v['print_sequence'];
-            }
-            array_multisort($sort['print_sequence'], SORT_ASC, $salesTaxes);
-        }
-
-        $decimals = totals_decimals();
-
-        foreach ($salesTaxes as $rowNumber => $salesTax) {
-            $saleTaxAmount = (float)$salesTax['sale_tax_amount'];
-            $roundingCode = $salesTax['rounding_code'];
-            $roundedSaleTaxAmount = $saleTaxAmount;
-
-            if (
-                $roundingCode == PHP_ROUND_HALF_UP
-                || $roundingCode == PHP_ROUND_HALF_DOWN
-                || $roundingCode == PHP_ROUND_HALF_EVEN
-                || $roundingCode == PHP_ROUND_HALF_ODD
-            ) {
-                $roundedSaleTaxAmount = round($saleTaxAmount, $decimals, $roundingCode);
-            } elseif ($roundingCode == $roundUp) {
-                $fig = (int) str_pad('1', $decimals, '0');
-                $roundedSaleTaxAmount = (ceil($saleTaxAmount * $fig) / $fig);
-            } elseif ($roundingCode == $roundDown) {
-                $fig = (int) str_pad('1', $decimals, '0');
-                $roundedSaleTaxAmount = (floor($saleTaxAmount * $fig) / $fig);
-            } elseif ($roundingCode == $halfFive) {
-                $roundedSaleTaxAmount = round($saleTaxAmount / 5) * 5;
-            }
-
-            $salesTaxes[$rowNumber]['sale_tax_amount'] = $roundedSaleTaxAmount;
-        }
-    }
-}

app/Traits/Models/Reports/ReportDateFilter.php

@@ -1,30 +0,0 @@
-<?php
-
-namespace App\Traits\Models\Reports;
-
-use CodeIgniter\Database\BaseBuilder;
-use Config\OSPOS;
-
-trait ReportDateFilter
-{
-    protected function buildDateWhereClause(array $inputs, string $dateColumn = 'sale_time'): string
-    {
-        $config = config(OSPOS::class)->settings;
-        
-        if (empty($config['date_or_time_format'])) {
-            return "DATE({$dateColumn}) BETWEEN " . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']);
-        }
-        return "{$dateColumn} BETWEEN " . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date']));
-    }
-
-    protected function applyDateFilter(BaseBuilder $builder, array $inputs, string $tablePrefix = 'sales', string $column = 'sale_time'): void
-    {
-        $config = config(OSPOS::class)->settings;
-        
-        if (empty($config['date_or_time_format'])) {
-            $builder->where("DATE({$tablePrefix}.{$column}) BETWEEN " . $this->db->escape($inputs['start_date']) . ' AND ' . $this->db->escape($inputs['end_date']));
-        } else {
-            $builder->where("{$tablePrefix}.{$column} BETWEEN " . $this->db->escape(rawurldecode($inputs['start_date'])) . ' AND ' . $this->db->escape(rawurldecode($inputs['end_date'])));
-        }
-    }
-}

app/Traits/Models/Reports/SaleTypeFilter.php

@@ -1,39 +0,0 @@
-<?php
-
-namespace App\Traits\Models\Reports;
-
-use CodeIgniter\Database\BaseBuilder;
-
-trait SaleTypeFilter
-{
-    protected function applySaleTypeFilter(BaseBuilder $builder, string $saleType, bool $usePrefix = true): void
-    {
-        $prefix = $usePrefix ? 'sales.' : '';
-        
-        if ($saleType === 'complete') {
-            $builder->where("{$prefix}sale_status", COMPLETED);
-            $builder->groupStart();
-            $builder->where("{$prefix}sale_type", SALE_TYPE_POS);
-            $builder->orWhere("{$prefix}sale_type", SALE_TYPE_INVOICE);
-            $builder->orWhere("{$prefix}sale_type", SALE_TYPE_RETURN);
-            $builder->groupEnd();
-        } elseif ($saleType === 'sales') {
-            $builder->where("{$prefix}sale_status", COMPLETED);
-            $builder->groupStart();
-            $builder->where("{$prefix}sale_type", SALE_TYPE_POS);
-            $builder->orWhere("{$prefix}sale_type", SALE_TYPE_INVOICE);
-            $builder->groupEnd();
-        } elseif ($saleType === 'quotes') {
-            $builder->where("{$prefix}sale_status", SUSPENDED);
-            $builder->where("{$prefix}sale_type", SALE_TYPE_QUOTE);
-        } elseif ($saleType === 'work_orders') {
-            $builder->where("{$prefix}sale_status", SUSPENDED);
-            $builder->where("{$prefix}sale_type", SALE_TYPE_WORK_ORDER);
-        } elseif ($saleType === 'canceled') {
-            $builder->where("{$prefix}sale_status", CANCELED);
-        } elseif ($saleType === 'returns') {
-            $builder->where("{$prefix}sale_status", COMPLETED);
-            $builder->where("{$prefix}sale_type", SALE_TYPE_RETURN);
-        }
-    }
-}

tests/Controllers/ItemsCsvImportTest.php

@@ -237,7 +237,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $row = $this->db->table('items')
             ->where('item_number', $itemData['item_number'])
@@ -268,7 +268,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $locationId = 1;
         $quantity = 100;
@@ -298,7 +298,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $inventoryData = [
             'trans_inventory' => 50,
@@ -329,7 +329,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $taxesData = [
             ['name' => 'VAT', 'percent' => 20],
@@ -406,7 +406,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
                 'deleted' => false
             ];
 
-            $this->assertTrue($this->item->save_value($itemData));
+            $this->assertTrue($this->item->saveValue($itemData));
         }
 
         $item1 = $this->item->get_info_by_id_or_number('ITEM-A');
@@ -430,7 +430,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($originalData));
+        $this->assertTrue($this->item->saveValue($originalData));
 
         $updatedData = [
             'item_id' => $originalData['item_id'],
@@ -443,7 +443,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($updatedData, $updatedData['item_id']));
+        $this->assertTrue($this->item->saveValue($updatedData));
 
         $updatedItem = $this->item->get_info($updatedData['item_id']);
         $this->assertEquals('Updated Name', $updatedItem->name);
@@ -464,7 +464,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($originalData));
+        $this->assertTrue($this->item->saveValue($originalData));
 
         $definitionData = [
             'definition_name' => 'Color',
@@ -510,7 +510,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $definitionData = [
             'definition_name' => 'Color',
@@ -553,7 +553,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         // Mock Attribute DROPDOWN
         $definitionData = [
@@ -604,7 +604,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $locationId = 1;
 
@@ -633,7 +633,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $savedItem = $this->item->get_info($itemData['item_id']);
         $this->assertEquals(-1, (int)$savedItem->reorder_level);
@@ -672,7 +672,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
                 'deleted' => 0
             ];
 
-            $this->assertTrue($this->item->save_value($itemData));
+            $this->assertTrue($this->item->saveValue($itemData));
 
             $savedItem = $this->item->get_info($itemData['item_id']);
 
@@ -702,7 +702,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $savedItem = $this->item->get_info($itemData['item_id']);
         $this->assertEquals('8471', $savedItem->hsn_code);
@@ -719,7 +719,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $locations = [
             'Warehouse' => 100,
@@ -792,7 +792,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $this->assertIsInt($itemData['item_id']);
         $this->assertGreaterThan(0, $itemData['item_id']);
@@ -812,7 +812,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $exists = $this->item->exists($itemData['item_id']);
         $this->assertTrue($exists);
@@ -858,7 +858,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $taxesData = [];
         if (is_numeric($csvRow['Tax 1 Percent']) && $csvRow['Tax 1 Name'] !== '') {
@@ -1032,7 +1032,7 @@ class ItemsCsvImportTest extends CIUnitTestCase
             'deleted' => 0
         ];
 
-        $this->assertTrue($this->item->save_value($itemData));
+        $this->assertTrue($this->item->saveValue($itemData));
 
         $uniqueId = uniqid();
         $locations = ['Warehouse' . $uniqueId, 'Store' . $uniqueId];