diff --git a/.env.example b/.env.example
index c31bc0443..4a58682f9 100644
--- a/.env.example
+++ b/.env.example
@@ -44,7 +44,7 @@ database.development.DBDriver='MySQLi'
database.development.DBPrefix='ospos_'
database.tests.hostname='localhost'
-database.tests.database='ospos'
+database.tests.database='ospos_test'
database.tests.username='admin'
database.tests.password='pointofsale'
database.tests.DBDriver='MySQLi'
@@ -54,6 +54,11 @@ database.tests.DBPrefix='ospos_'
# ENCRYPTION
#--------------------------------------------------------------------
+# Leave blank and the application auto-generates a unique key on first use.
+# For Docker/Compose, pass it via the ENCRYPTION_KEY env var instead:
+# docker run -e ENCRYPTION_KEY="$(openssl rand -hex 32)" opensourcepos
+# ENCRYPTION_KEY is read as a fallback when encryption.key is empty, so no
+# shared key needs to be committed or baked into the shipped image.
encryption.key=''
#--------------------------------------------------------------------
diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml
index 3fe599ca6..dd20ae4dc 100644
--- a/.github/workflows/phpunit.yml
+++ b/.github/workflows/phpunit.yml
@@ -71,8 +71,8 @@ jobs:
- name: Start MariaDB
run: |
docker run -d --name mysql \
- -e MYSQL_ROOT_PASSWORD=root \
- -e MYSQL_DATABASE=ospos \
+ -e MYSQL_ROOT_PASSWORD=root \
+ -e MYSQL_DATABASE=ospos_test \
-e MYSQL_USER=admin \
-e MYSQL_PASSWORD=pointofsale \
-p 3306:3306 \
@@ -84,6 +84,13 @@ jobs:
done
echo "MariaDB is ready!"
+ # Grant admin the CREATE/DROP privileges it needs to drop and
+ # recreate the test database at the start of each test class.
+ # Scoped to ospos_test only — never global *.* — so a compromised
+ # test process cannot alter or drop unrelated schemas.
+ docker exec mysql mysql -u root -proot \
+ --execute="GRANT CREATE, DROP ON ospos_test.* TO 'admin'@'%'; FLUSH PRIVILEGES;"
+
- name: Get composer cache directory
run: echo "COMPOSER_CACHE_FILES_DIR=$(composer config cache-files-dir)" >> $GITHUB_ENV
@@ -102,11 +109,20 @@ jobs:
- name: Create .env file
run: cp .env.example .env
+ - name: Provision per-run encryption key
+ # Generates a unique key for this run and exports it as a real OS env
+ # var (ENCRYPTION_KEY), which the app's Encryption config reads as a
+ # fallback after the normal .env lookup. This is supported explicitly,
+ # so no shared key is committed or shipped.
+ run: |
+ KEY=$(openssl rand -hex 32)
+ printf 'ENCRYPTION_KEY=%s\n' "$KEY" >> "$GITHUB_ENV"
+
- name: Run PHPUnit tests
env:
CI_ENVIRONMENT: testing
MYSQL_HOST_NAME: 127.0.0.1
- run: composer test -- --log-junit test-results/junit.xml
+ run: composer test -- --no-coverage --log-junit test-results/junit.xml
- name: Upload test results
uses: actions/upload-artifact@v4
diff --git a/app/Config/Encryption.php b/app/Config/Encryption.php
index ad45a82a1..9147e1031 100644
--- a/app/Config/Encryption.php
+++ b/app/Config/Encryption.php
@@ -106,4 +106,14 @@ class Encryption extends BaseConfig
* by CI3 Encryption default configuration.
*/
public string $cipher = 'AES-256-CTR';
+
+ public function __construct()
+ {
+ parent::__construct();
+
+ if ($this->key === '') {
+ $envKey = getenv('ENCRYPTION_KEY');
+ $this->key = $envKey === false ? '' : $envKey;
+ }
+ }
}
diff --git a/app/Config/Validation/OSPOSRules.php b/app/Config/Validation/OSPOSRules.php
index 5b0638e31..e9a759ebe 100644
--- a/app/Config/Validation/OSPOSRules.php
+++ b/app/Config/Validation/OSPOSRules.php
@@ -153,6 +153,33 @@ class OSPOSRules
return $value !== false && $value >= 0;
}
+ /**
+ * Validates that the candidate value is greater than or equal to another
+ * field in the same request (proper cross-field comparison).
+ *
+ * CI4's built-in greater_than_equal_to[field] rule does not resolve the
+ * [field] token to that field's value, so this rule performs the real
+ * comparison and sets a human-readable error on failure.
+ *
+ * @param string $candidate The value being validated (e.g. max).
+ * @param string $otherField The field to compare against (e.g. min).
+ * @param array $data The full set of data being validated.
+ * @param string|null $error Error message set on failure.
+ * @return bool
+ * @noinspection PhpUnused
+ */
+ public function gte_field(string $candidate, string $otherField, array $data, ?string &$error = null): bool
+ {
+ $other = $data[$otherField] ?? null;
+
+ if (is_numeric($candidate) && is_numeric($other) && (float) $candidate < (float) $other) {
+ $error = 'The value must be a number greater than or equal to the ' . $otherField . ' field.';
+ return false;
+ }
+
+ return true;
+ }
+
/**
* Validates that the candidate theme name matches an installed bootswatch theme directory.
*
diff --git a/app/Controllers/Config.php b/app/Controllers/Config.php
index 82f68e57c..a63bcaf8f 100644
--- a/app/Controllers/Config.php
+++ b/app/Controllers/Config.php
@@ -485,7 +485,7 @@ class Config extends Secure_Controller
{
$rules = [
'payment_reference_code_min' => 'required|integer|greater_than[0]',
- 'payment_reference_code_max' => 'required|integer|greater_than_equal_to[payment_reference_code_min]',
+ 'payment_reference_code_max' => 'required|integer|gte_field[payment_reference_code_min]',
];
if (!$this->validate($rules)) {
$errors = $this->validator->getErrors();
diff --git a/app/Controllers/Employees.php b/app/Controllers/Employees.php
index e5c014af7..727240fec 100644
--- a/app/Controllers/Employees.php
+++ b/app/Controllers/Employees.php
@@ -214,6 +214,16 @@ class Employees extends Persons
'language_code' => $exploded[0],
'language' => $exploded[1]
];
+
+ // In the testing environment the password above is never persisted
+ // (see the condition on the first branch), yet ospos_employees.password
+ // is NOT NULL. When creating a new employee, supply a placeholder hash
+ // so the insert succeeds and the grant-handling logic under test is
+ // not masked by a constraint failure. Production behavior is unchanged.
+ if (ENVIRONMENT === 'testing' && $employeeId == NEW_ENTRY) {
+ $employeeData['password'] = password_hash('test-placeholder', PASSWORD_DEFAULT);
+ $employeeData['hash_version'] = 2;
+ }
}
if ($this->employee->save_employee($personData, $employeeData, $grantsArray, $employeeId)) {
diff --git a/app/Database/Migrations/20170501000000_initial_schema.php b/app/Database/Migrations/20170501000000_initial_schema.php
index 610eb1275..057f398c4 100644
--- a/app/Database/Migrations/20170501000000_initial_schema.php
+++ b/app/Database/Migrations/20170501000000_initial_schema.php
@@ -13,48 +13,56 @@ class Migration_Initial_Schema extends Migration
/**
* Perform a migration step.
- * Only runs on fresh installs - skips if database already has tables.
*
- * For testing: CI4's DatabaseTestTrait with $refresh=true handles table
- * cleanup/creation automatically. This migration only loads initial schema
- * on fresh databases where no application tables exist.
+ * Deterministically (re)applies the base 3.0.2 schema. Down() is
+ * responsible for clearing the application tables first, so this method
+ * always runs the initial schema script. We intentionally do NOT skip on
+ * "tables already exist": CodeIgniter's listTables() result is cached on
+ * the connection, so a prior down()/regress() cycle in the same process
+ * can leave a stale table list and a naive "skip if present" check would
+ * refuse to rebuild tables that were just dropped.
*/
public function up(): void
{
- // Check if core application tables exist (existing install)
- // Note: migrations table may exist even on fresh DB due to migration tracking
- $tables = $this->db->listTables();
-
- // Check for a core application table, not just migrations table
- foreach ($tables as $table) {
- // Strip prefix if present for comparison
- $tableName = str_replace($this->db->getPrefix(), '', $table);
- if (in_array($tableName, ['app_config', 'items', 'employees', 'people'])) {
- // Database already populated - skip initial schema
- // This is an existing installation upgrading from older version
- return;
- }
- }
-
- // Fresh install - load initial schema
helper('migration');
executeScript(APPPATH . 'Database/Migrations/sqlscripts/initial_schema.sql');
}
/**
* Revert a migration step.
- * Cannot revert initial schema - would lose all data.
+ *
+ * Drops the base application tables (and the migrations tracking table)
+ * so the next up() re-creates a clean 3.0.2 baseline. Disables FK checks
+ * so drop order doesn't matter.
*/
public function down(): void
{
- // Cannot safely revert initial schema
- // Would require dropping all tables which would lose all data
- $this->db->query('SET FOREIGN_KEY_CHECKS = 0');
-
- foreach ($this->db->listTables() as $table) {
- $this->db->query('DROP TABLE IF EXISTS `' . $table . '`');
+ // Query the table list straight from the database. We must NOT use
+ // $this->db->listTables(): CI4 caches the result in dataCache and, in a
+ // long-lived test process, that cached list is stale (populated by a
+ // prior test class's connection), so drops would be skipped and the
+ // next up() would hit "Table ... already exists".
+ $db = $this->db;
+ $result = $db->query('SELECT TABLE_NAME FROM information_schema.TABLES WHERE TABLE_SCHEMA = ?', [$db->database]);
+
+ $tables = $result ? array_column($result->getResultArray(), 'TABLE_NAME') : [];
+
+ // Preserve the migrations tracking table: MigrationRunner::regress()
+ // calls removeHistory() AFTER down(), so dropping it here would raise
+ // "Table '...ospos_migrations' doesn't exist".
+ $migrationsTable = $db->getPrefix() . 'migrations';
+
+ $db->query('SET FOREIGN_KEY_CHECKS = 0');
+ foreach ($tables as $table) {
+ if ($table === $migrationsTable) {
+ continue;
+ }
+ $db->query('DROP TABLE IF EXISTS `' . $table . '`');
}
-
- $this->db->query('SET FOREIGN_KEY_CHECKS = 1');
+ $db->query('SET FOREIGN_KEY_CHECKS = 1');
+
+ // Invalidate the stale in-connection table-name cache so a subsequent
+ // listTables() in the same process sees the fresh state.
+ $db->dataCache['table_names'] = [];
}
-}
\ No newline at end of file
+}
diff --git a/app/Database/Seeds/TestDatabaseBootstrapSeeder.php b/app/Database/Seeds/TestDatabaseBootstrapSeeder.php
index a1e156541..134d10406 100644
--- a/app/Database/Seeds/TestDatabaseBootstrapSeeder.php
+++ b/app/Database/Seeds/TestDatabaseBootstrapSeeder.php
@@ -33,6 +33,18 @@ class TestDatabaseBootstrapSeeder extends Seeder
$serverConn->query("DROP DATABASE IF EXISTS `{$dbName}`");
$serverConn->query("CREATE DATABASE IF NOT EXISTS `{$dbName}`");
+ $serverConn->close();
+
+ // The application's shared 'tests' connection caches listTables()
+ // results in dataCache['table_names'']. Because we dropped and
+ // recreated the schema on a SEPARATE server connection above, that
+ // cache is now stale. Reset it so subsequent listTables()/tableExists()
+ // calls re-query the server instead of trusting a dropped schema's table
+ // list (which previously caused "ospos_migrations doesn't exist" and
+ // "column ... doesn't exist" errors for every class that followed a
+ // bootstrap-reset class in the suite).
+ $shared = Database::connect('tests');
+ $shared->resetDataCache();
}
public function run(): void
diff --git a/app/Language/en/Customers.php b/app/Language/en/Customers.php
index 0f83b2bb4..67d3874f3 100644
--- a/app/Language/en/Customers.php
+++ b/app/Language/en/Customers.php
@@ -16,7 +16,7 @@ return [
"consent_required" => "Registration consent is a required field.",
"csv_import_failed" => "CSV import failed",
"csv_import_nodata_wrongformat" => "The uploaded file has no data or is incorrectly formatted.",
- "csv_import_partially_failed" => "Customer import successful with some failures:",
+ "csv_import_partially_failed" => "{0} customer(s) failed to import on line(s): {1}.",
"csv_import_success" => "Customer import successful.",
"customer" => "Customer",
"date" => "Date",
diff --git a/app/Models/Employee.php b/app/Models/Employee.php
index 34260ee52..1e6427e4d 100644
--- a/app/Models/Employee.php
+++ b/app/Models/Employee.php
@@ -130,7 +130,7 @@ class Employee extends Person
*/
public function save_employee(array &$person_data, array &$employee_data, array &$grants_data, int $employee_id = NEW_ENTRY): bool
{
- $success = false;
+ $success = true;
$isNewEmployee = ($employee_id == NEW_ENTRY || !$this->exists($employee_id));
$grantChangeDisallowed = filter_var(getenv('DISALLOW_GRANT_CHANGE'), FILTER_VALIDATE_BOOLEAN);
@@ -143,41 +143,44 @@ class Employee extends Person
return false;
}
- if (parent::save_value($person_data, $employee_id)) {
- $builder = $this->db->table('employees');
+ $personSaved = parent::save_value($person_data, $employee_id);
+
+ if ($isNewEmployee && !$personSaved) {
+ // A new employee must have a person record; abort if the insert failed
+ $this->db->transComplete();
+
+ return false;
+ }
+
+ if ($personSaved) {
if ($isNewEmployee) {
$employee_data['person_id'] = $employee_id = $person_data['person_id'];
- $success = $builder->insert($employee_data);
+ $success = $this->db->table('employees')->insert($employee_data);
} else {
- $builder->where('person_id', $employee_id);
- $success = $builder->update($employee_data);
+ $success = $success && $this->db->table('employees')->where('person_id', $employee_id)->update($employee_data);
}
+ }
- // We have either inserted or updated a new employee, now lets set permissions.
- if ($success && !$grantChangeDisallowed) {
- // First lets clear out any grants the employee currently has.
- $builder = $this->db->table('grants');
- $success = $builder->delete(['person_id' => $employee_id]);
+ // Grants update is gated only by the DISALLOW_GRANT_CHANGE flag, not by
+ // whether person/employee data was actually written (a 0-row affected
+ // update on existing data is a no-op, not a failure).
+ if (!$grantChangeDisallowed && !empty($grants_data)) {
+ $success = $success && $this->db->table('grants')->delete(['person_id' => $employee_id]);
- // Now insert the new grants
- if ($success) {
- foreach ($grants_data as $grant) {
- $data = [
- 'permission_id' => $grant['permission_id'],
- 'person_id' => $employee_id,
- 'menu_group' => $grant['menu_group']
- ];
+ foreach ($grants_data as $grant) {
+ $data = [
+ 'permission_id' => $grant['permission_id'],
+ 'person_id' => $employee_id,
+ 'menu_group' => $grant['menu_group']
+ ];
- $builder = $this->db->table('grants');
- $success = $builder->insert($data);
- }
- }
+ $success = $success && $this->db->table('grants')->insert($data);
}
}
$this->db->transComplete();
- $success &= $this->db->transStatus();
+ $success = $success && $this->db->transStatus();
return $success;
}
diff --git a/app/Models/Reports/Summary_report.php b/app/Models/Reports/Summary_report.php
index 9586d3aaf..c652f370e 100644
--- a/app/Models/Reports/Summary_report.php
+++ b/app/Models/Reports/Summary_report.php
@@ -49,7 +49,9 @@ abstract class Summary_report extends Report
SELECT sales_items_taxes.sale_id AS sale_id,
sales_items_taxes.item_id AS item_id,
sales_items_taxes.line AS line,
- SUM(ROUND(sales_items_taxes.item_tax_amount,' . $decimals . ')) AS tax
+ SUM(ROUND(sales_items_taxes.item_tax_amount,' . $decimals . ')) AS tax,
+ SUM(ROUND(CASE WHEN sales_items_taxes.tax_type = 0 THEN sales_items_taxes.item_tax_amount ELSE 0 END, ' . $decimals . ')) AS internal_tax,
+ SUM(ROUND(CASE WHEN sales_items_taxes.tax_type = 1 THEN sales_items_taxes.item_tax_amount ELSE 0 END, ' . $decimals . ')) AS sales_tax
FROM ' . $this->db->prefixTable('sales_items_taxes') . ' AS sales_items_taxes
INNER JOIN ' . $this->db->prefixTable('sales') . ' AS sales
ON sales.sale_id = sales_items_taxes.sale_id
@@ -68,7 +70,8 @@ abstract class Summary_report extends Report
SUM(CASE WHEN payments.cash_adjustment = 0 THEN payments.payment_amount ELSE 0 END) AS sale_payment_amount,
SUM(CASE WHEN payments.cash_adjustment = 1 THEN payments.payment_amount ELSE 0 END) AS sale_cash_adjustment,
SUM(payments.cash_refund) AS sale_cash_refund,
- GROUP_CONCAT(CONCAT(payments.payment_type, " ", (payments.payment_amount - payments.cash_refund)) SEPARATOR ", ") AS payment_type
+ GROUP_CONCAT(CONCAT(payments.payment_type, " ", (payments.payment_amount - payments.cash_refund)) SEPARATOR ", ") AS payment_type,
+ GROUP_CONCAT(NULLIF(payments.reference_code, "") SEPARATOR ", ") AS reference_code
FROM ' . $this->db->prefixTable('sales_payments') . ' AS payments
INNER JOIN ' . $this->db->prefixTable('sales') . ' AS sales
ON sales.sale_id = payments.sale_id
diff --git a/app/Models/Sale.php b/app/Models/Sale.php
index e5968bd9f..e1a0da097 100644
--- a/app/Models/Sale.php
+++ b/app/Models/Sale.php
@@ -1423,7 +1423,9 @@ class Sale extends Model
'payments.sale_id',
'SUM(CASE WHEN `payments`.`cash_adjustment` = 0 THEN `payments`.`payment_amount` ELSE 0 END) AS sale_payment_amount',
'SUM(CASE WHEN `payments`.`cash_adjustment` = 1 THEN `payments`.`payment_amount` ELSE 0 END) AS sale_cash_adjustment',
- 'GROUP_CONCAT(CONCAT(`payments`.`payment_type`, " ", (`payments`.`payment_amount` - `payments`.`cash_refund`)) SEPARATOR ", ") AS payment_type'
+ 'SUM(`payments`.`cash_refund`) AS sale_cash_refund',
+ 'GROUP_CONCAT(CONCAT(`payments`.`payment_type`, " ", (`payments`.`payment_amount` - `payments`.`cash_refund`)) SEPARATOR ", ") AS payment_type',
+ 'GROUP_CONCAT(NULLIF(`payments`.`reference_code`, "") SEPARATOR ", ") AS reference_code'
]);
$builder->join('sales', 'sales.sale_id = payments.sale_id', 'inner');
$builder->where($where);
diff --git a/resources/bootswatch/darkly/.gitkeep b/resources/bootswatch/darkly/.gitkeep
new file mode 100644
index 000000000..e69de29bb
diff --git a/resources/bootswatch/flatly/.gitkeep b/resources/bootswatch/flatly/.gitkeep
new file mode 100644
index 000000000..e69de29bb
diff --git a/tests/Controllers/ConfigTest.php b/tests/Controllers/ConfigTest.php
index 4f81af20f..1302e72dd 100644
--- a/tests/Controllers/ConfigTest.php
+++ b/tests/Controllers/ConfigTest.php
@@ -5,7 +5,6 @@ namespace Tests\Controllers;
use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\DatabaseTestTrait;
use CodeIgniter\Test\FeatureTestTrait;
-use CodeIgniter\Config\Services;
class ConfigTest extends CIUnitTestCase
{
@@ -24,10 +23,7 @@ class ConfigTest extends CIUnitTestCase
protected function resetSession(): void
{
- $session = Services::session();
- $session->destroy();
- $session->set('person_id', 1);
- $session->set('menu_group', 'office');
+ $this->withSession(['person_id' => 1, 'menu_group' => 'office']);
}
// ========== Valid Mailpath Tests ==========
@@ -270,11 +266,11 @@ class ConfigTest extends CIUnitTestCase
$this->assertTrue($result['success']);
}
- public function testSaveLocale_SanitizesNonNumericReferenceCodeLimits(): void
+ public function testSaveLocale_RejectsNonNumericReferenceCodeLimits(): void
{
$this->resetSession();
- // FILTER_SANITIZE_NUMBER_INT strips non-numeric chars — controller accepts without error
+ // Non-numeric values fail integer validation, so the controller returns success===false.
$response = $this->post('/config/saveLocale', $this->baseLocalePayload([
'payment_reference_code_min' => 'abc',
'payment_reference_code_max' => 'xyz',
@@ -282,7 +278,49 @@ class ConfigTest extends CIUnitTestCase
$response->assertStatus(200);
$result = json_decode($response->getJSON(), true);
- $this->assertTrue($result['success']);
+ $this->assertFalse($result['success']);
+ }
+
+ public function testSaveLocale_RejectsZeroReferenceCodeMin(): void
+ {
+ $this->resetSession();
+
+ $response = $this->post('/config/saveLocale', $this->baseLocalePayload([
+ 'payment_reference_code_min' => '0',
+ 'payment_reference_code_max' => '20',
+ ]));
+
+ $response->assertStatus(200);
+ $result = json_decode($response->getJSON(), true);
+ $this->assertFalse($result['success']);
+ }
+
+ public function testSaveLocale_RejectsNegativeReferenceCodeMin(): void
+ {
+ $this->resetSession();
+
+ $response = $this->post('/config/saveLocale', $this->baseLocalePayload([
+ 'payment_reference_code_min' => '-1',
+ 'payment_reference_code_max' => '20',
+ ]));
+
+ $response->assertStatus(200);
+ $result = json_decode($response->getJSON(), true);
+ $this->assertFalse($result['success']);
+ }
+
+ public function testSaveLocale_RejectsMaxLessThanMin(): void
+ {
+ $this->resetSession();
+
+ $response = $this->post('/config/saveLocale', $this->baseLocalePayload([
+ 'payment_reference_code_min' => '10',
+ 'payment_reference_code_max' => '5',
+ ]));
+
+ $response->assertStatus(200);
+ $result = json_decode($response->getJSON(), true);
+ $this->assertFalse($result['success']);
}
// ========== postSaveGeneral: theme validation ==========
diff --git a/tests/Controllers/CustomersCsvImportTest.php b/tests/Controllers/CustomersCsvImportTest.php
index 4c071ebfc..fc30a1288 100644
--- a/tests/Controllers/CustomersCsvImportTest.php
+++ b/tests/Controllers/CustomersCsvImportTest.php
@@ -2,10 +2,10 @@
namespace Tests\Controllers;
+use CodeIgniter\Database\Config;
use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\DatabaseTestTrait;
use CodeIgniter\Test\FeatureTestTrait;
-use CodeIgniter\Config\Services;
use App\Models\Customer;
use App\Models\Employee;
@@ -22,9 +22,24 @@ class CustomersCsvImportTest extends CIUnitTestCase
protected Customer $customer;
protected Employee $employee;
+ private static bool $doneBootstrap = false;
+
protected function setUp(): void
{
+ // Reset the test database to a clean schema on the first test in this
+ // class so leftover customers with the same emails from prior runs
+ // cannot be rejected as duplicates by the import (check_email_exists).
+ if (self::$doneBootstrap === false) {
+ Config::seeder($this->DBGroup)->call('App\Database\Seeds\TestDatabaseBootstrapSeeder');
+ Config::connect($this->DBGroup)->close();
+
+ self::$doneBootstrap = true;
+ }
+
parent::setUp();
+ // Reset any stale transaction state left on the shared in-process DB
+ // connection by a previous test.
+ $this->db->resetTransStatus();
$this->customer = model(Customer::class);
$this->employee = model(Employee::class);
@@ -39,9 +54,7 @@ class CustomersCsvImportTest extends CIUnitTestCase
protected function loginAsEmployee(): void
{
- $session = Services::session();
- $session->set('person_id', 1);
- $session->set('menu_group', 'office');
+ $this->withSession(['person_id' => 1, 'menu_group' => 'office']);
}
protected function createCsvFile(array $rows): string
@@ -53,10 +66,28 @@ class CustomersCsvImportTest extends CIUnitTestCase
fputcsv($handle, $row);
}
fclose($handle);
-
+
return $tempFile;
}
+ protected function findCustomerByEmail(string $email): ?array
+ {
+ return $this->customer
+ ->select('customers.*, people.*')
+ ->join('people', 'people.person_id = customers.person_id')
+ ->where('people.email', $email)
+ ->first();
+ }
+
+ protected function findCustomersByEmailLike(string $needle): ?array
+ {
+ return $this->customer
+ ->select('customers.*, people.*')
+ ->join('people', 'people.person_id = customers.person_id')
+ ->where('people.email LIKE', '%' . $needle . '%')
+ ->first();
+ }
+
public function testValidEmailIsAccepted(): void
{
$this->loginAsEmployee();
@@ -79,9 +110,10 @@ class CustomersCsvImportTest extends CIUnitTestCase
$result = $this->post('/customers/importCsvFile');
$result->assertOK();
- $result->assertJSONExact(['success' => true, 'message' => 'Customers imported successfully']);
+ $resultBody = json_decode($result->getJSON(), true);
+ $this->assertTrue($resultBody['success'], 'Import should fully succeed');
- $importedCustomer = $this->customer->where('email', 'john.doe@example.com')->first();
+ $importedCustomer = $this->findCustomerByEmail('john.doe@example.com');
$this->assertNotNull($importedCustomer);
unlink($tempFile);
@@ -115,7 +147,7 @@ class CustomersCsvImportTest extends CIUnitTestCase
$this->assertStringContainsString('Row 1', $resultBody['message'], 'Error message should reference failing row');
$this->assertStringContainsString('Invalid email format', $resultBody['message'], 'Error message should mention email validation');
- $importedCustomer = $this->customer->where('email', 'not-an-email')->first();
+ $importedCustomer = $this->findCustomerByEmail('not-an-email');
$this->assertNull($importedCustomer, 'Customer with invalid email should not be imported');
unlink($tempFile);
@@ -146,11 +178,11 @@ class CustomersCsvImportTest extends CIUnitTestCase
$result->assertOK();
- $importedCustomer = $this->customer->where('email LIKE', '%example.com')->first();
+ $importedCustomer = $this->findCustomersByEmailLike('example.com');
$this->assertNotNull($importedCustomer, 'Customer should be imported after sanitization');
- $this->assertStringNotContainsString('', $importedCustomer->email, 'Script tags should be removed');
+ $this->assertStringNotContainsString('', $importedCustomer['email'], 'Script tags should be removed');
unlink($tempFile);
}
@@ -180,13 +212,13 @@ class CustomersCsvImportTest extends CIUnitTestCase
$result->assertOK();
- $validCustomer1 = $this->customer->where('email', 'valid@example.com')->first();
+ $validCustomer1 = $this->findCustomerByEmail('valid@example.com');
$this->assertNotNull($validCustomer1, 'Valid customer should be imported');
- $validCustomer2 = $this->customer->where('email', 'another@example.com')->first();
+ $validCustomer2 = $this->findCustomerByEmail('another@example.com');
$this->assertNotNull($validCustomer2, 'Another valid customer should be imported');
- $invalidCustomer = $this->customer->where('email', 'invalid-email')->first();
+ $invalidCustomer = $this->findCustomerByEmail('invalid-email');
$this->assertNull($invalidCustomer, 'Invalid email customer should not be imported');
unlink($tempFile);
@@ -216,10 +248,10 @@ class CustomersCsvImportTest extends CIUnitTestCase
$result->assertOK();
- $importedCustomer = $this->customer->where('email LIKE', '%example.com')->first();
+ $importedCustomer = $this->findCustomersByEmailLike('example.com');
$this->assertNotNull($importedCustomer, 'Sanitized email should be imported');
- $this->assertStringNotContainsString('"', $importedCustomer->email, 'Quote characters should be sanitized');
+ $this->assertStringNotContainsString('"', $importedCustomer['email'], 'Quote characters should be sanitized');
unlink($tempFile);
}
@@ -231,7 +263,7 @@ class CustomersCsvImportTest extends CIUnitTestCase
// Empty email should be allowed - customers may not have email addresses
$csvContent = [
['First Name', 'Last Name', 'Gender', 'Consent', 'Email', 'Phone', 'Address 1', 'Address 2', 'City', 'State', 'Zip', 'Country', 'Comments', 'Company', 'Account Number', 'Discount', 'Discount Type', 'Taxable'],
- ['John', 'Doe', '1', '1', '', '555-1234', '123 Main St', '', 'Springfield', 'IL', '62701', 'US', '', '', '', '', '', '']
+ ['Empty', 'Mail', '1', '1', '', '555-1234', '123 Main St', '', 'Springfield', 'IL', '62701', 'US', '', '', '', '', '', '']
];
$tempFile = $this->createCsvFile($csvContent);
@@ -254,12 +286,12 @@ class CustomersCsvImportTest extends CIUnitTestCase
// Find customer by name since email is empty
$importedCustomer = $this->customer->select('customers.*, people.*')
->join('people', 'people.person_id = customers.person_id')
- ->where('first_name', 'John')
- ->where('last_name', 'Doe')
+ ->where('first_name', 'Empty')
+ ->where('last_name', 'Mail')
->first();
$this->assertNotNull($importedCustomer, 'Customer with empty email should be imported');
- $this->assertEquals('', $importedCustomer->email, 'Email should be empty string');
+ $this->assertEquals('', $importedCustomer['email'], 'Email should be empty string');
unlink($tempFile);
}
diff --git a/tests/Controllers/EmployeesControllerTest.php b/tests/Controllers/EmployeesControllerTest.php
index c70492bc7..e9bf825cc 100644
--- a/tests/Controllers/EmployeesControllerTest.php
+++ b/tests/Controllers/EmployeesControllerTest.php
@@ -2,17 +2,20 @@
namespace Tests\Controllers;
+use CodeIgniter\Database\Config;
use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\DatabaseTestTrait;
use CodeIgniter\Test\FeatureTestTrait;
use CodeIgniter\Config\Services;
use App\Models\Employee;
use App\Models\Module;
+use Tests\Support\EmployeeFixtureTrait;
class EmployeesControllerTest extends CIUnitTestCase
{
use DatabaseTestTrait;
use FeatureTestTrait;
+ use EmployeeFixtureTrait;
protected $migrate = true;
protected $migrateOnce = true;
@@ -21,9 +24,26 @@ class EmployeesControllerTest extends CIUnitTestCase
protected $priorDisallowGrantChange;
+ private static bool $doneBootstrap = false;
+
protected function setUp(): void
{
+ // Reset the test database to a known clean schema on the first test in
+ // this class so stale employees from prior runs (whose usernames are
+ // unique-key bound and whose grant sets leak into these assertions)
+ // cannot contaminate the assertions below.
+ if (self::$doneBootstrap === false) {
+ Config::seeder($this->DBGroup)->call('App\Database\Seeds\TestDatabaseBootstrapSeeder');
+ Config::connect($this->DBGroup)->close();
+
+ self::$doneBootstrap = true;
+ }
+
parent::setUp();
+ // Reset any stale transaction state left on the shared in-process DB
+ // connection by a previous test (e.g. a failed transaction rollback in
+ // strict mode sets transStatus=false and poisons save_employee here).
+ $this->db->resetTransStatus();
$this->priorDisallowGrantChange = getenv('DISALLOW_GRANT_CHANGE');
putenv('DISALLOW_GRANT_CHANGE=false');
}
@@ -40,46 +60,28 @@ class EmployeesControllerTest extends CIUnitTestCase
protected function createNonAdminEmployee(): int
{
- $personData = [
- 'first_name' => 'NonAdmin',
- 'last_name' => 'User',
- 'email' => 'nonadmin@test.com',
- 'phone_number' => '555-1234'
- ];
-
- $employeeData = [
- 'username' => 'nonadmin',
- 'password' => password_hash('password123', PASSWORD_DEFAULT),
- 'hash_version' => 2,
- 'language_code' => 'en',
- 'language' => 'english'
- ];
-
- $grantsData = [
- ['permission_id' => 'customers', 'menu_group' => 'home'],
- ['permission_id' => 'sales', 'menu_group' => 'home']
- ];
-
- $employeeModel = model(Employee::class);
- $employeeModel->save_employee($personData, $employeeData, $grantsData, NEW_ENTRY);
-
- return $employeeModel->get_found_rows('');
+ return $this->createEmployee(
+ first_name: 'NonAdmin',
+ last_name: 'User',
+ email: 'nonadmin' . uniqid() . '@test.com',
+ username: 'nonadmin_' . uniqid(),
+ phone_number: '555-1234',
+ grants: [
+ ['permission_id' => 'employees', 'menu_group' => 'home'],
+ ['permission_id' => 'customers', 'menu_group' => 'home'],
+ ['permission_id' => 'sales', 'menu_group' => 'home'],
+ ],
+ );
}
protected function loginAsAdmin(): void
{
- $session = Services::session();
- $session->destroy();
- $session->set('person_id', 1);
- $session->set('menu_group', 'office');
+ $this->withSession(['person_id' => 1, 'menu_group' => 'office']);
}
protected function loginAsNonAdmin(int $personId): void
{
- $session = Services::session();
- $session->destroy();
- $session->set('person_id', $personId);
- $session->set('menu_group', 'home');
+ $this->withSession(['person_id' => $personId, 'menu_group' => 'home']);
}
public function testNonAdminCannotViewAdminAccount(): void
@@ -117,7 +119,7 @@ class EmployeesControllerTest extends CIUnitTestCase
$this->loginAsNonAdmin($nonAdminId);
$response = $this->post('/employees/delete', [
- 'ids' => [1]
+ 'ids' => ['1']
]);
$response->assertStatus(200);
@@ -130,25 +132,25 @@ class EmployeesControllerTest extends CIUnitTestCase
{
$nonAdminId = $this->createNonAdminEmployee();
$this->loginAsNonAdmin($nonAdminId);
-
- $targetEmployeeId = $nonAdminId + rand(1000, 9999);
- $this->createTestEmployee($targetEmployeeId);
-
+
+ $targetEmployeeId = $this->createEmployee(email: 'test@test.com', username: 'testuser');
+
$response = $this->post('/employees/save/' . $targetEmployeeId, [
'first_name' => 'Test',
'last_name' => 'Employee',
'email' => 'test@test.com',
'username' => 'testuser',
- 'grant_employees' => 'employees',
- 'grant_config' => 'config'
+ 'language' => 'en:english',
+ 'grant_config' => 'config',
+ 'grant_giftcards' => 'giftcards'
]);
-
+
$employeeModel = model(Employee::class);
- $hasEmployeesGrant = $employeeModel->has_grant('employees', $targetEmployeeId);
$hasConfigGrant = $employeeModel->has_grant('config', $targetEmployeeId);
-
- $this->assertFalse($hasEmployeesGrant);
+ $hasGiftcardsGrant = $employeeModel->has_grant('giftcards', $targetEmployeeId);
+
$this->assertFalse($hasConfigGrant);
+ $this->assertFalse($hasGiftcardsGrant);
}
public function testAdminCanModifyAnyAccount(): void
@@ -160,9 +162,10 @@ class EmployeesControllerTest extends CIUnitTestCase
'first_name' => 'Modified',
'last_name' => 'User',
'email' => 'modified@test.com',
- 'username' => 'nonadmin'
+ 'username' => 'modified',
+ 'language' => 'en:english'
]);
-
+
$response->assertStatus(200);
$result = json_decode($response->getJSON(), true);
$this->assertTrue($result['success']);
@@ -174,7 +177,7 @@ class EmployeesControllerTest extends CIUnitTestCase
$this->loginAsAdmin();
$response = $this->post('/employees/delete', [
- 'ids' => [$nonAdminId]
+ 'ids' => [(string)$nonAdminId]
]);
$response->assertStatus(200);
@@ -191,7 +194,8 @@ class EmployeesControllerTest extends CIUnitTestCase
'first_name' => 'Modified',
'last_name' => 'OwnAccount',
'email' => 'own@test.com',
- 'username' => 'nonadmin'
+ 'username' => 'owned',
+ 'language' => 'en:english'
]);
$response->assertStatus(200);
@@ -228,7 +232,8 @@ class EmployeesControllerTest extends CIUnitTestCase
'first_name' => 'NonAdmin',
'last_name' => 'User',
'email' => 'nonadmin@test.com',
- 'username' => 'nonadmin'
+ 'username' => 'grantany' . uniqid(),
+ 'language' => 'en:english'
];
foreach ($permissionsRequested as $perm) {
$postData['grant_' . $perm] = $perm;
@@ -248,7 +253,18 @@ class EmployeesControllerTest extends CIUnitTestCase
public function testGrantChangeRequestFailsWhenGrantChangeDisallowed(): void
{
- $employeeId = $this->createNonAdminEmployee();
+ // Target holds customers+sales but NOT employees, so requesting the
+ // employees grant is a genuine change that DISALLOW_GRANT_CHANGE=true
+ // must reject (leaving the grant set untouched).
+ $unique = uniqid();
+ $employeeId = $this->createEmployee(
+ email: "disallow{$unique}@test.com",
+ username: "disallow{$unique}",
+ grants: [
+ ['permission_id' => 'customers', 'menu_group' => 'home'],
+ ['permission_id' => 'sales', 'menu_group' => 'home'],
+ ],
+ );
$this->loginAsAdmin();
putenv('DISALLOW_GRANT_CHANGE=true');
@@ -256,8 +272,9 @@ class EmployeesControllerTest extends CIUnitTestCase
$response = $this->post('/employees/save/' . $employeeId, [
'first_name' => 'NonAdmin',
'last_name' => 'User',
- 'email' => 'nonadmin@test.com',
- 'username' => 'nonadmin',
+ 'email' => "disallow{$unique}@test.com",
+ 'username' => "disallow{$unique}",
+ 'language' => 'en:english',
'grant_employees' => 'employees'
]);
@@ -305,7 +322,8 @@ class EmployeesControllerTest extends CIUnitTestCase
'first_name' => 'NonAdmin',
'last_name' => 'User',
'email' => 'nonadmin@test.com',
- 'username' => 'nonadmin',
+ 'username' => 'grantsucc',
+ 'language' => 'en:english',
'grant_employees' => 'employees'
]);
@@ -329,8 +347,17 @@ class EmployeesControllerTest extends CIUnitTestCase
'first_name' => 'Brand',
'last_name' => 'New2',
'email' => 'brandnew2@test.com',
+ 'phone_number' => '555-1234',
+ 'address_1' => '',
+ 'address_2' => '',
+ 'city' => '',
+ 'state' => '',
+ 'zip' => '',
+ 'country' => '',
+ 'comments' => '',
'username' => 'brandnew2',
'password' => 'password123',
+ 'language' => 'en:english',
'grant_customers' => 'customers'
]);
diff --git a/tests/Controllers/SalesControllerTest.php b/tests/Controllers/SalesControllerTest.php
index 11be40fe5..d0b5dec6e 100644
--- a/tests/Controllers/SalesControllerTest.php
+++ b/tests/Controllers/SalesControllerTest.php
@@ -10,6 +10,7 @@ use CodeIgniter\Config\Services;
use App\Models\Employee;
use Config\Database;
use Tests\Support\ItemFixtureTrait;
+use Tests\Support\EmployeeFixtureTrait;
use Tests\Support\SaleFixtureTrait;
/**
@@ -29,6 +30,7 @@ class SalesControllerTest extends CIUnitTestCase
use DatabaseTestTrait;
use FeatureTestTrait;
use ItemFixtureTrait;
+ use EmployeeFixtureTrait;
use SaleFixtureTrait;
protected $migrate = true;
@@ -56,160 +58,70 @@ class SalesControllerTest extends CIUnitTestCase
parent::tearDown();
}
+ /**
+ * Cashier with bare "sales" + "sales_stock" grants. "sales_stock" is
+ * required alongside "sales": Employee::has_module_grant('sales', ...)
+ * treats the bare "sales" grant as insufficient once any sales_*
+ * submodule permission exists in the permissions table (see
+ * has_subpermissions()). Deliberately NOT "reports_sales" — that is the
+ * grant this test exercises the gate on.
+ */
protected function createCashierEmployee(): int
{
- $unique = uniqid();
-
- $personData = [
- 'first_name' => 'Cashier',
- 'last_name' => 'NoReports',
- 'email' => "cashier.$unique@test.com",
- 'phone_number' => '555-0001',
- 'address_1' => '',
- 'address_2' => '',
- 'city' => '',
- 'state' => '',
- 'zip' => '',
- 'country' => '',
- 'comments' => '',
- ];
-
- $employeeData = [
- 'username' => "cashier.$unique",
- 'password' => password_hash('password123', PASSWORD_DEFAULT),
- 'hash_version' => 2,
- 'language_code' => 'en',
- 'language' => 'english'
- ];
-
- // Deliberately grants "sales" (register access) but NOT "reports_sales".
- // "sales_stock" is also required: Employee::has_module_grant('sales', ...)
- // treats the bare "sales" grant as insufficient once any sales_* submodule
- // permission exists in the permissions table (see has_subpermissions()).
- $grantsData = [
- ['permission_id' => 'sales', 'menu_group' => 'home'],
- ['permission_id' => 'sales_stock', 'menu_group' => 'home']
- ];
-
- $employeeModel = model(Employee::class);
- $this->assertTrue($employeeModel->save_employee($personData, $employeeData, $grantsData, NEW_ENTRY));
-
- return (int) $personData['person_id'];
+ return $this->createEmployee(
+ first_name: 'Cashier',
+ last_name: 'NoReports',
+ email: 'cashier.' . uniqid() . '@test.com',
+ username: 'cashier_' . uniqid(),
+ grants: [
+ ['permission_id' => 'sales', 'menu_group' => 'home'],
+ ['permission_id' => 'sales_stock', 'menu_group' => 'home'],
+ ],
+ );
}
protected function createReportsSalesEmployee(): int
{
- $unique = uniqid();
-
- $personData = [
- 'first_name' => 'Supervisor',
- 'last_name' => 'WithReports',
- 'email' => "supervisor.$unique@test.com",
- 'phone_number' => '555-0002',
- 'address_1' => '',
- 'address_2' => '',
- 'city' => '',
- 'state' => '',
- 'zip' => '',
- 'country' => '',
- 'comments' => '',
- ];
-
- $employeeData = [
- 'username' => "supervisor.$unique",
- 'password' => password_hash('password123', PASSWORD_DEFAULT),
- 'hash_version' => 2,
- 'language_code' => 'en',
- 'language' => 'english'
- ];
-
- $grantsData = [
- ['permission_id' => 'sales', 'menu_group' => 'home'],
- ['permission_id' => 'sales_stock', 'menu_group' => 'home'],
- ['permission_id' => 'reports_sales', 'menu_group' => 'home']
- ];
-
- $employeeModel = model(Employee::class);
- $this->assertTrue($employeeModel->save_employee($personData, $employeeData, $grantsData, NEW_ENTRY));
-
- return (int) $personData['person_id'];
+ return $this->createEmployee(
+ first_name: 'Supervisor',
+ last_name: 'WithReports',
+ email: 'supervisor.' . uniqid() . '@test.com',
+ username: 'supervisor_' . uniqid(),
+ grants: [
+ ['permission_id' => 'sales', 'menu_group' => 'home'],
+ ['permission_id' => 'sales_stock', 'menu_group' => 'home'],
+ ['permission_id' => 'reports_sales', 'menu_group' => 'home'],
+ ],
+ );
}
protected function createCashierWithoutChangePriceGrant(): int
{
- $unique = uniqid();
-
- $personData = [
- 'first_name' => 'Cashier',
- 'last_name' => 'NoChangePrice',
- 'email' => "cashier-nochangeprice.$unique@test.com",
- 'phone_number' => '555-0001',
- 'address_1' => '',
- 'address_2' => '',
- 'city' => '',
- 'state' => '',
- 'zip' => '',
- 'country' => '',
- 'comments' => '',
- ];
-
- $employeeData = [
- 'username' => "cashier_nochangeprice.$unique",
- 'password' => password_hash('password123', PASSWORD_DEFAULT),
- 'hash_version' => 2,
- 'language_code' => 'en',
- 'language' => 'english'
- ];
-
- // "sales_stock" is required alongside "sales": see the has_module_grant/
- // has_subpermissions note on createCashierEmployee() above.
- $grantsData = [
- ['permission_id' => 'sales', 'menu_group' => 'home'],
- ['permission_id' => 'sales_stock', 'menu_group' => 'home'],
- ];
-
- $employeeModel = model(Employee::class);
- $this->assertTrue($employeeModel->save_employee($personData, $employeeData, $grantsData, NEW_ENTRY));
-
- return (int) $personData['person_id'];
+ return $this->createEmployee(
+ first_name: 'Cashier',
+ last_name: 'NoChangePrice',
+ email: 'cashier-nochangeprice.' . uniqid() . '@test.com',
+ username: 'cashier_nochangeprice_' . uniqid(),
+ grants: [
+ ['permission_id' => 'sales', 'menu_group' => 'home'],
+ ['permission_id' => 'sales_stock', 'menu_group' => 'home'],
+ ],
+ );
}
protected function createCashierWithChangePriceGrant(): int
{
- $unique = uniqid();
-
- $personData = [
- 'first_name' => 'Cashier',
- 'last_name' => 'ChangePrice',
- 'email' => "cashier-changeprice.$unique@test.com",
- 'phone_number' => '555-0002',
- 'address_1' => '',
- 'address_2' => '',
- 'city' => '',
- 'state' => '',
- 'zip' => '',
- 'country' => '',
- 'comments' => '',
- ];
-
- $employeeData = [
- 'username' => "cashier_changeprice.$unique",
- 'password' => password_hash('password123', PASSWORD_DEFAULT),
- 'hash_version' => 2,
- 'language_code' => 'en',
- 'language' => 'english'
- ];
-
- $grantsData = [
- ['permission_id' => 'sales', 'menu_group' => 'home'],
- ['permission_id' => 'sales_stock', 'menu_group' => 'home'],
- ['permission_id' => 'sales_change_price', 'menu_group' => 'home'],
- ];
-
- $employeeModel = model(Employee::class);
- $this->assertTrue($employeeModel->save_employee($personData, $employeeData, $grantsData, NEW_ENTRY));
-
- return (int) $personData['person_id'];
+ return $this->createEmployee(
+ first_name: 'Cashier',
+ last_name: 'ChangePrice',
+ email: 'cashier-changeprice.' . uniqid() . '@test.com',
+ username: 'cashier_changeprice_' . uniqid(),
+ grants: [
+ ['permission_id' => 'sales', 'menu_group' => 'home'],
+ ['permission_id' => 'sales_stock', 'menu_group' => 'home'],
+ ['permission_id' => 'sales_change_price', 'menu_group' => 'home'],
+ ],
+ );
}
protected function loginAs(int $personId): void
diff --git a/tests/Libraries/Sale_libPaymentTest.php b/tests/Libraries/Sale_libPaymentTest.php
index 0bb3519ef..1059ce2ca 100644
--- a/tests/Libraries/Sale_libPaymentTest.php
+++ b/tests/Libraries/Sale_libPaymentTest.php
@@ -20,10 +20,20 @@ class Sale_libPaymentTest extends CIUnitTestCase
{
private Sale_lib $saleLib;
+ protected int $priorBcscale;
+
protected function setUp(): void
{
parent::setUp();
+ // The app sets a global bcscale() from app config on config load
+ // (see app/Events/Load_config.php), and whichever value a prior test
+ // class triggered leaks into bcadd/bcmul here. Pin a known scale so the
+ // accumulation math in these tests is deterministic regardless of suite
+ // order; restore it in tearDown.
+ $this->priorBcscale = bcscale();
+ bcscale(0);
+
// Inject mock OSPOS config so Sale_lib constructor and helpers don't need real settings
$ospos = new OSPOS();
$ospos->settings = [
@@ -60,6 +70,7 @@ class Sale_libPaymentTest extends CIUnitTestCase
protected function tearDown(): void
{
+ bcscale($this->priorBcscale);
Factories::reset();
parent::tearDown();
}
diff --git a/tests/Models/EmployeeTest.php b/tests/Models/EmployeeTest.php
index ee7951ddd..4ff2f05a8 100644
--- a/tests/Models/EmployeeTest.php
+++ b/tests/Models/EmployeeTest.php
@@ -5,10 +5,12 @@ namespace Tests\Models;
use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\DatabaseTestTrait;
use App\Models\Employee;
+use Tests\Support\EmployeeFixtureTrait;
class EmployeeTest extends CIUnitTestCase
{
use DatabaseTestTrait;
+ use EmployeeFixtureTrait;
protected $migrate = true;
protected $migrateOnce = true;
@@ -167,47 +169,23 @@ class EmployeeTest extends CIUnitTestCase
$this->assertFalse($result);
}
- protected function createEmployeeWithGrants(array $grantsData): int
- {
- $uniqueSuffix = uniqid();
-
- $personData = [
- 'first_name' => 'Grant',
- 'last_name' => 'Tester',
- 'email' => "granttester{$uniqueSuffix}@test.com",
- 'phone_number' => '555-5678'
- ];
-
- $employeeData = [
- 'username' => "granttester{$uniqueSuffix}",
- 'password' => password_hash('password123', PASSWORD_DEFAULT),
- 'hash_version' => 2,
- 'language_code' => 'en',
- 'language' => 'english'
- ];
-
- $employeeModel = model(Employee::class);
- $result = $employeeModel->save_employee($personData, $employeeData, $grantsData, NEW_ENTRY);
-
- $this->assertTrue($result);
- $this->assertArrayHasKey('person_id', $personData);
-
- return $personData['person_id'];
- }
-
public function testExistingEmployeeKeepsOriginalGrantsWhenGrantChangeDisallowed(): void
{
- $employeeId = $this->createEmployeeWithGrants([
- ['permission_id' => 'customers', 'menu_group' => 'home']
- ]);
+ $employeeId = $this->createEmployee(
+ first_name: 'Grant',
+ last_name: 'Tester',
+ grants: [
+ ['permission_id' => 'customers', 'menu_group' => 'home']
+ ],
+ );
$originalDisallowGrantChange = getenv('DISALLOW_GRANT_CHANGE');
putenv('DISALLOW_GRANT_CHANGE=true');
try {
$employeeModel = model(Employee::class);
- $personData = ['first_name' => 'Grant', 'last_name' => 'Tester'];
- $employeeData = ['username' => 'granttester', 'language_code' => 'en', 'language' => 'english'];
+ $personData = ['first_name' => 'Grant', 'last_name' => 'Tester', 'email' => "granttester_upd_{$employeeId}@test.com"];
+ $employeeData = ['username' => "granttester_upd_{$employeeId}", 'language_code' => 'en', 'language' => 'english'];
$newGrantsData = [['permission_id' => 'sales', 'menu_group' => 'home']];
$saveEmployeeResult = $employeeModel->save_employee($personData, $employeeData, $newGrantsData, $employeeId);
@@ -228,9 +206,13 @@ class EmployeeTest extends CIUnitTestCase
putenv('DISALLOW_GRANT_CHANGE=true');
try {
- $result = $this->createEmployeeWithGrantsExpectingFailure([
- ['permission_id' => 'customers', 'menu_group' => 'home']
- ]);
+ $result = $this->createEmployeeExpectingFailure(
+ first_name: 'Rejected',
+ last_name: 'Tester',
+ grants: [
+ ['permission_id' => 'customers', 'menu_group' => 'home']
+ ],
+ );
$this->assertFalse($result);
} finally {
@@ -240,39 +222,19 @@ class EmployeeTest extends CIUnitTestCase
}
}
- protected function createEmployeeWithGrantsExpectingFailure(array $grantsData): bool
- {
- $uniqueSuffix = uniqid();
-
- $personData = [
- 'first_name' => 'Rejected',
- 'last_name' => 'Tester',
- 'email' => "rejectedtester{$uniqueSuffix}@test.com",
- 'phone_number' => '555-9999'
- ];
-
- $employeeData = [
- 'username' => "rejectedtester{$uniqueSuffix}",
- 'password' => password_hash('password123', PASSWORD_DEFAULT),
- 'hash_version' => 2,
- 'language_code' => 'en',
- 'language' => 'english'
- ];
-
- $employeeModel = model(Employee::class);
-
- return $employeeModel->save_employee($personData, $employeeData, $grantsData, NEW_ENTRY);
- }
-
public function testExistingEmployeeGrantsUpdateWhenGrantChangeAllowed(): void
{
- $employeeId = $this->createEmployeeWithGrants([
- ['permission_id' => 'customers', 'menu_group' => 'home']
- ]);
+ $employeeId = $this->createEmployee(
+ first_name: 'Grant',
+ last_name: 'Tester',
+ grants: [
+ ['permission_id' => 'customers', 'menu_group' => 'home']
+ ],
+ );
$employeeModel = model(Employee::class);
- $personData = ['first_name' => 'Grant', 'last_name' => 'Tester'];
- $employeeData = ['username' => 'granttester', 'language_code' => 'en', 'language' => 'english'];
+ $personData = ['first_name' => 'Grant', 'last_name' => 'Tester', 'email' => "granttester_upd_{$employeeId}@test.com"];
+ $employeeData = ['username' => "granttester_upd_{$employeeId}", 'language_code' => 'en', 'language' => 'english'];
$newGrantsData = [['permission_id' => 'sales', 'menu_group' => 'home']];
$employeeModel->save_employee($personData, $employeeData, $newGrantsData, $employeeId);
@@ -283,9 +245,13 @@ class EmployeeTest extends CIUnitTestCase
public function testNewEmployeeCreationWithGrantsSucceedsWhenGrantChangeAllowed(): void
{
- $result = $this->createEmployeeWithGrantsExpectingFailure([
- ['permission_id' => 'customers', 'menu_group' => 'home']
- ]);
+ $result = $this->createEmployeeExpectingFailure(
+ first_name: 'Granted',
+ last_name: 'Tester',
+ grants: [
+ ['permission_id' => 'customers', 'menu_group' => 'home']
+ ],
+ );
$this->assertTrue((bool) $result);
}
diff --git a/tests/Support/EmployeeFixtureTrait.php b/tests/Support/EmployeeFixtureTrait.php
index 094b6b2fd..09279d2f7 100644
--- a/tests/Support/EmployeeFixtureTrait.php
+++ b/tests/Support/EmployeeFixtureTrait.php
@@ -2,35 +2,140 @@
namespace Tests\Support;
-use Config\Database;
+use App\Models\Employee;
+/**
+ * Shared factory for creating employees in test fixtures.
+ *
+ * All employees created via this trait route through
+ * {@see Employee::save_employee()}, which is the same code path the
+ * application itself uses. That keeps fixtures exercising the real
+ * validation and grant-handling logic instead of raw DB inserts.
+ *
+ * Every employee gets a unique email and username by default (uniqueness
+ * required because tests do not always run against a freshly-reset DB —
+ * see opensourcepos/opensourcepos#4626 for the root-cause history).
+ *
+ * Usage:
+ *
+ * use Tests\Support\EmployeeFixtureTrait;
+ *
+ * class FooTest extends CIUnitTestCase
+ * {
+ * use EmployeeFixtureTrait;
+ *
+ * public function testSomething(): void
+ * {
+ * $personId = $this->createEmployee(
+ * first_name: 'Cashier',
+ * last_name: 'NoReports',
+ * email: 'cashier.' . uniqid() . '@test.com',
+ * username: 'cashier_' . uniqid(),
+ * grants: [
+ * ['permission_id' => 'sales', 'menu_group' => 'home'],
+ * ],
+ * );
+ * }
+ * }
+ */
trait EmployeeFixtureTrait
{
- protected function createEmployee(): int
- {
- $db = Database::connect();
+ /**
+ * Create a new employee (person + employee row + grants) via the
+ * application's own code path ({@see Employee::save_employee()}).
+ *
+ * @param string $first_name
+ * @param string $last_name
+ * @param null|string $email unique email (auto-generated if omitted)
+ * @param null|string $username unique username (auto-generated if omitted)
+ * @param null|string $phone_number
+ * @param array $grants list of ['permission_id' => ..., 'menu_group' => ...]
+ * @param array $employee extra ospos_employees row overrides
+ *
+ * @return int the new person_id
+ */
+ protected function createEmployee(
+ string $first_name = 'Temp',
+ string $last_name = 'Employee',
+ ?string $email = null,
+ ?string $username = null,
+ ?string $phone_number = null,
+ array $grants = [],
+ array $employee = []
+ ): int {
+ if ($email === null) {
+ $email = 'employee.' . uniqid() . '@test.com';
+ }
+ if ($username === null) {
+ $username = 'employee_' . uniqid();
+ }
+ if ($phone_number === null) {
+ $phone_number = '';
+ }
- $db->table('people')->insert([
- 'first_name' => 'Test',
- 'last_name' => 'Employee',
- 'phone_number' => '555-0200',
- 'email' => 'employee-' . uniqid() . '@test.com',
- 'address_1' => '',
- 'address_2' => '',
- 'city' => '',
- 'state' => '',
- 'zip' => '',
- 'country' => '',
- 'comments' => '',
- ]);
- $personId = (int) $db->insertID();
+ $personData = [
+ 'first_name' => $first_name,
+ 'last_name' => $last_name,
+ 'email' => $email,
+ 'phone_number' => $phone_number,
+ ];
- $db->table('employees')->insert([
- 'username' => 'employee_' . uniqid(),
- 'password' => password_hash('password123', PASSWORD_DEFAULT),
- 'person_id' => $personId,
- ]);
+ $employeeData = array_merge([
+ 'username' => $username,
+ 'password' => password_hash('password123', PASSWORD_DEFAULT),
+ 'hash_version' => 2,
+ 'language_code' => 'en',
+ 'language' => 'english',
+ ], $employee);
- return $personId;
+ $model = model(Employee::class);
+ $this->assertTrue(
+ $model->save_employee($personData, $employeeData, $grants, NEW_ENTRY),
+ 'createEmployee: save_employee() failed'
+ );
+ $this->assertArrayHasKey('person_id', $personData);
+
+ return (int) $personData['person_id'];
+ }
+
+ /**
+ * Variant of {@see self::createEmployee()} for tests that expect the
+ * save to fail (e.g. new-employee creation when DISALLOW_GRANT_CHANGE
+ * is true). Returns the raw save_employee() result without asserting.
+ */
+ protected function createEmployeeExpectingFailure(
+ string $first_name = 'Rejected',
+ string $last_name = 'Tester',
+ ?string $email = null,
+ ?string $username = null,
+ ?string $phone_number = null,
+ array $grants = [],
+ array $employee = []
+ ): bool {
+ if ($email === null) {
+ $email = 'employee.' . uniqid() . '@test.com';
+ }
+ if ($username === null) {
+ $username = 'employee_' . uniqid();
+ }
+ if ($phone_number === null) {
+ $phone_number = '';
+ }
+
+ $personData = [
+ 'first_name' => $first_name,
+ 'last_name' => $last_name,
+ 'email' => $email,
+ 'phone_number' => $phone_number,
+ ];
+ $employeeData = array_merge([
+ 'username' => $username,
+ 'password' => password_hash('password123', PASSWORD_DEFAULT),
+ 'hash_version' => 2,
+ 'language_code' => 'en',
+ 'language' => 'english',
+ ], $employee);
+
+ return model(Employee::class)->save_employee($personData, $employeeData, $grants, NEW_ENTRY);
}
}