';
$display_table .= '| ' . $this->manageDisplayLayout($barcode_config['barcode_first_row'], $item, $barcode_config) . ' |
';
- $display_table .= ''.$barcode.' |
';
+ $display_table .= '' . $barcode . ' |
';
$display_table .= '| ' . $this->manageDisplayLayout($barcode_config['barcode_second_row'], $item, $barcode_config) . ' |
';
$display_table .= '| ' . $this->manageDisplayLayout($barcode_config['barcode_third_row'], $item, $barcode_config) . ' |
';
$display_table .= '
';
diff --git a/app/Libraries/Sale_lib.php b/app/Libraries/Sale_lib.php
index 0a28588bb..747406469 100644
--- a/app/Libraries/Sale_lib.php
+++ b/app/Libraries/Sale_lib.php
@@ -331,9 +331,9 @@ class Sale_lib
/**
* @return int|null
*/
- public function get_sale_type(): ?int
+ public function getSaleType(): ?int
{
- return $this->session->get('sale_type', 0);
+ return $this->session->get('sale_type') ?? 0;
}
/**
diff --git a/app/Models/Giftcard.php b/app/Models/Giftcard.php
index a12466265..8900f7b5c 100644
--- a/app/Models/Giftcard.php
+++ b/app/Models/Giftcard.php
@@ -311,12 +311,9 @@ class Giftcard extends Model
$builder->update(['value' => $value]);
}
- /**
- * Atomically decrements a gift card's value, failing if the balance is insufficient
- */
public function decrementGiftcardValue(string $giftcardNumber, float $amount): bool
{
- if ($amount <= 0) {
+ if ($amount <= 0.0) {
return false;
}
diff --git a/app/Models/Sale.php b/app/Models/Sale.php
index 77391fb04..7446d99f5 100644
--- a/app/Models/Sale.php
+++ b/app/Models/Sale.php
@@ -805,7 +805,7 @@ class Sale extends Model
// Start a transaction to assure data integrity
$this->db->transStart();
- $sale_status = $this->get_sale_status($sale_id);
+ $sale_status = $this->getSaleStatus($sale_id);
if ($update_inventory && $sale_status == COMPLETED) {
// Defect, not all item deletions will be undone?
@@ -1225,12 +1225,14 @@ class Sale extends Model
/**
* Gets the sale status for the selected sale
*/
- public function get_sale_status(int $sale_id): int
+ public function getSaleStatus(int $sale_id): ?int
{
$builder = $this->db->table('sales');
$builder->where('sale_id', $sale_id);
- return $builder->get()->getRow()->sale_status;
+ $row = $builder->get()->getRow();
+
+ return $row === null ? null : $row->sale_status;
}
/**
diff --git a/tests/Controllers/SalesControllerTest.php b/tests/Controllers/SalesControllerTest.php
index 7f6118e45..11be40fe5 100644
--- a/tests/Controllers/SalesControllerTest.php
+++ b/tests/Controllers/SalesControllerTest.php
@@ -10,20 +10,26 @@ use CodeIgniter\Config\Services;
use App\Models\Employee;
use Config\Database;
use Tests\Support\ItemFixtureTrait;
+use Tests\Support\SaleFixtureTrait;
/**
* Regression tests for GHSA-3xf6-8fmq-44wg.
*
* A cashier holding only the base "sales" grant (no "reports_sales") must
* not be able to reach the per-sale endpoints that getManage() gates
- * behind reports_sales: getRow, getEdit, postSave, getReceipt, getInvoice,
- * getSendPdf, getSendReceipt.
+ * behind reports_sales: getSearch, getRow, getEdit, postSave, getReceipt,
+ * getInvoice, getSendPdf, getSendReceipt.
+ *
+ * Also covers: getSearch() (the AJAX endpoint that
+ * supplies every row of the Sales Takings list) was the one sibling that
+ * returned the full ledger and was missing the reports_sales check.
*/
class SalesControllerTest extends CIUnitTestCase
{
use DatabaseTestTrait;
use FeatureTestTrait;
use ItemFixtureTrait;
+ use SaleFixtureTrait;
protected $migrate = true;
protected $migrateOnce = true;
@@ -214,49 +220,6 @@ class SalesControllerTest extends CIUnitTestCase
]);
}
- /**
- * Inserts a minimal completed sale row directly, bypassing Sale::save_value()
- * (which requires a full cart/inventory/tax pipeline unrelated to this
- * authorization check). Sale::get_info() inner-joins sales_items, so a
- * matching item/sales_items row is required for the sale to be found.
- */
- protected function createSale(int $employeeId): int
- {
- $unique = uniqid();
- $db = Database::connect();
-
- $db->table('items')->insert([
- 'name' => "Test Item $unique",
- 'category' => 'Test',
- 'description' => 'Test item',
- 'cost_price' => 1,
- 'unit_price' => 1,
- 'item_number' => "TEST-$unique",
- ]);
- $itemId = (int) $db->insertID();
-
- $db->table('sales')->insert([
- 'sale_time' => date('Y-m-d H:i:s'),
- 'customer_id' => null,
- 'employee_id' => $employeeId,
- 'comment' => 'test sale',
- 'invoice_number' => null,
- ]);
- $saleId = (int) $db->insertID();
-
- $db->table('sales_items')->insert([
- 'sale_id' => $saleId,
- 'item_id' => $itemId,
- 'line' => 1,
- 'quantity_purchased' => 1,
- 'item_cost_price' => 1,
- 'item_unit_price' => 1,
- 'item_location' => 1,
- ]);
-
- return $saleId;
- }
-
/**
* Seeds a single cart line directly in the session, mirroring the shape
* Sale_lib::add_item() produces, so tests can target postEditItem's
@@ -310,6 +273,20 @@ class SalesControllerTest extends CIUnitTestCase
$this->assertFalse($result['success']);
}
+ public function testCashierWithoutReportsSalesCannotGetSearch(): void
+ {
+ $cashierId = $this->createCashierEmployee();
+ $this->createSale($cashierId);
+ $this->loginAs($cashierId);
+
+ $response = $this->get('/sales/search');
+
+ $response->assertStatus(403);
+ $result = json_decode($response->getJSON(), true);
+ $this->assertFalse($result['success']);
+ $this->assertSame(lang('Sales.not_authorized'), $result['message']);
+ }
+
public function testCashierWithoutReportsSalesCannotGetEdit(): void
{
$cashierId = $this->createCashierEmployee();
@@ -395,19 +372,6 @@ class SalesControllerTest extends CIUnitTestCase
$this->assertSame(lang('Sales.not_authorized'), $result['message']);
}
- public function testCashierWithoutReportsSalesCannotGetSearch(): void
- {
- $cashierId = $this->createCashierEmployee();
- $this->createSale($cashierId);
- $this->loginAs($cashierId);
-
- $response = $this->get('/sales/search');
-
- $response->assertStatus(403);
- $result = json_decode($response->getJSON(), true);
- $this->assertFalse($result['success']);
- }
-
public function testEmployeeWithReportsSalesCanGetSearch(): void
{
$supervisorId = $this->createReportsSalesEmployee();
@@ -519,6 +483,101 @@ class SalesControllerTest extends CIUnitTestCase
$this->assertEquals('0.01', $cart[1]['price']);
}
+ public function testPostAddPaymentRejectsNegativeAmountTendered(): void
+ {
+ $cashierId = $this->createCashierWithoutChangePriceGrant();
+ $this->loginAs($cashierId);
+ $itemId = $this->createTestItem();
+ $this->seedCartLine(1, '1.00', $itemId);
+
+ $forgedPaymentType = lang('Sales.giftcard') . ':AUDIT-100';
+
+ $response = $this->post('/sales/addPayment', [
+ 'payment_type' => $forgedPaymentType,
+ 'amount_tendered' => '-500',
+ ]);
+
+ $response->assertStatus(200);
+ $response->assertSee(lang('Sales.must_enter_numeric'));
+
+ // ... and the negative payment must NOT have been added to the cart.
+ $session = Services::session();
+ $payments = $session->get('sales_payments');
+ $this->assertArrayNotHasKey($forgedPaymentType, (array) $payments);
+ }
+
+ public function testPostAddPaymentRejectsMalformedAmountTendered(): void
+ {
+ $cashierId = $this->createCashierWithoutChangePriceGrant();
+ $this->loginAs($cashierId);
+ $itemId = $this->createTestItem();
+ $this->seedCartLine(1, '1.00', $itemId);
+
+ $response = $this->post('/sales/addPayment', [
+ 'payment_type' => lang('Sales.cash'),
+ 'amount_tendered' => 'ABC123',
+ ]);
+
+ $response->assertStatus(200);
+ $response->assertSee(lang('Sales.must_enter_numeric'));
+
+ $session = Services::session();
+ $payments = $session->get('sales_payments');
+ $this->assertArrayNotHasKey(lang('Sales.cash'), (array) $payments);
+ }
+
+ public function testCashierWithoutReportsSalesCannotUnsuspend(): void
+ {
+ $victimId = $this->createReportsSalesEmployee();
+ $saleId = $this->createSuspendedSale($victimId);
+
+ $cashierId = $this->createCashierEmployee();
+ $this->loginAs($cashierId);
+
+ $response = $this->post('/sales/unsuspend', [
+ 'suspended_sale_id' => $saleId,
+ ]);
+
+ $response->assertStatus(403);
+ $result = json_decode($response->getJSON(), true);
+ $this->assertFalse($result['success']);
+
+ $session = Services::session();
+ $this->assertNotEquals($saleId, $session->get('sale_id'));
+ }
+
+ public function testEmployeeWithReportsSalesCannotUnsuspendCompletedSale(): void
+ {
+ $victimId = $this->createReportsSalesEmployee();
+ $saleId = $this->createSale($victimId);
+
+ $supervisorId = $this->createReportsSalesEmployee();
+ $this->loginAs($supervisorId);
+
+ $this->post('/sales/unsuspend', [
+ 'suspended_sale_id' => $saleId,
+ ]);
+
+ $session = Services::session();
+ $this->assertNotEquals($saleId, $session->get('sale_id'));
+ }
+
+ public function testEmployeeWithReportsSalesCanUnsuspendSale(): void
+ {
+ $ownerId = $this->createReportsSalesEmployee();
+ $saleId = $this->createSuspendedSale($ownerId);
+
+ $supervisorId = $this->createReportsSalesEmployee();
+ $this->loginAs($supervisorId);
+
+ $this->post('/sales/unsuspend', [
+ 'suspended_sale_id' => $saleId,
+ ]);
+
+ $session = Services::session();
+ $this->assertEquals($saleId, $session->get('sale_id'));
+ }
+
protected function createGiftcard(float $value): int
{
$giftcardNumber = random_int(1000000, 9999999);
@@ -719,4 +778,42 @@ class SalesControllerTest extends CIUnitTestCase
$this->assertStringNotContainsString('