mirror of
https://github.com/opensourcepos/opensourcepos.git
synced 2026-10-28 19:18:58 -04:00
master
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 objecttothisandClaude Sonnet 4.6
|
74e825ba6f |
feat(stock-locations): add default location and sort order support (#4643)
Refactor Stock_location model to camelCase naming and introduce
is_default and sort_order fields with full migration, UI, and
item-sync support. Multiple follow-up fixes consolidated.
Database
- Add is_default (TINYINT, backfilled to oldest non-deleted row) and
sort_order (nullable unsigned TINYINT unique, dense 0..N-1) columns
via new migration; MySQL 5.7-compatible backfill using user variables
instead of ROW_NUMBER(); down() drops both columns
Model
- Rename show_locations → showLocations (camelCase convention)
- Add getDefaultLocationId(), setDefaultLocation(), saveSortOrder();
validate location existence before setting as default
- delete() compacts sort_order via two-pass CASE update to avoid
collisions after prior reorders
- Atomic sort_order assignment via explicit SQL COUNT query
Controller / Config
- Modularize save logic into sanitizeSubmittedLocations(),
deleteRemovedLocations(), saveSubmittedLocations(),
saveLocationOrderAndDefault(); wrap in single transaction
- Support stock_location_new inputs for new-row creation
- Resolve new-* ID placeholders to real IDs post-save
- Abort loop on save failure; default missing POST arrays to []
- Validate that at least one location array is non-empty
Views
- Add Default column with center-aligned radio buttons; disable for
unsaved rows; use attr() instead of data() for location IDs
- Escape dynamic location IDs against XSS in all affected partials
- form_count_details.php: fall back to default location when current
is undefined; guard quantity input similarly
- Add aria-label to default radio inputs
Libraries / Items
- Widen Item_lib::get_item_location() and get_stock_destination() to
nullable return types
- Guard null location_id in Items::getRow() and getGenerateBarcodes()
- Item::get_multiple_info(): accept ?int $location_id; short-circuit
with where('item_id', -1) instead of always-false where('1','0')
i18n
- Add stock_location_default, stock_location_default_header,
stock_location_name_header across all locale files
Tests
- Stock_locationTest: getDefaultLocationId, setDefaultLocation,
saveSortOrder, delete compaction, saveValue; rename to
testMethod_Scenario convention
- Add ConfigControllerTest / StockLocationsControllerTest with
AdminAuthTrait covering CRUD, reorder, default assignment, and
malformed/empty/blank payload validation
- DatabaseBootstrapTrait-based setup for ItemTest and Stock_locationTest
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
||
|
objecttothisandClaude Sonnet 5
|
af48fa4357 |
fix(tests): standardize DB bootstrap/OSPOS mocking, fix stale-prefix bug (#4771)
Adds DatabaseBootstrapTrait and OsposConfigFixtureTrait to replace duplicated per-file setUp()/tearDown() bootstrap and OSPOS config mock boilerplate across test classes, bringing them in line with the ItemsControllerTest gold-standard shape. Fixes the real root cause of tabular_helperTest's suite-order-dependent failure: dropAllForeignKeyConstraints()/foreignKeyExists()/ dropColumnIfExists() temporarily blank the shared connection's DBPrefix via overridePrefix() but never restored it if the query between the two calls threw (e.g. migration_helperTest's mocked-failure tests) - permanently corrupting prefixed table lookups for the rest of the process. Wrapped each in try/finally. Also widens executeScript() to accept an explicit connection so migration SQL can't silently run against the wrong DB group. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
objecttothisandClaude Sonnet 5
|
0015b7575c |
Revert "Merge remote-tracking branch 'OpensourcePOS/master' into fix-unit-test-failures" and test-standardization commit (#4770)
These landed on master by accident via a local branch that was mistakenly created tracking OpensourcePOS/master directly, rather than pushed as its own branch. Reverting to restore master to its prior state; the reverted work will come back through a proper PR instead. This reverts commit |
||
|
objecttothisandClaude Sonnet 5
|
37cc1b15e7 |
fix(tests): standardize DB bootstrap/OSPOS mocking, fix stale-prefix bug
Adds DatabaseBootstrapTrait and OsposConfigFixtureTrait to replace duplicated per-file setUp()/tearDown() bootstrap and OSPOS config mock boilerplate across test classes, bringing them in line with the ItemsControllerTest gold-standard shape. Fixes the real root cause of tabular_helperTest's suite-order-dependent failure: dropAllForeignKeyConstraints()/foreignKeyExists()/ dropColumnIfExists() temporarily blank the shared connection's DBPrefix via overridePrefix() but never restored it if the query between the two calls threw (e.g. migration_helperTest's mocked-failure tests) - permanently corrupting prefixed table lookups for the rest of the process. Wrapped each in try/finally. Also widens executeScript() to accept an explicit connection so migration SQL can't silently run against the wrong DB group. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
|
e20fb2a792 |
feat(db): add MySQL 8.4 support (#4048)
Add MySQL 8.4 compatibility, including a utf8mb4 conversion migration, hardened foreign key handling, and expanded CI database coverage. Migrations: - Add Mysql84Utf8mb4Conversion migration using sqlscripts/3.4.3_mysql84_utf8mb4_conversion.sql via executeScript() - Drop ospos_people composite index before conversion, since its prefix length exceeds the 3072-byte key limit under utf8mb4 - Add assertNoCollationCollisions() pre-flight check to fail fast on unique values that collide under utf8mb4_unicode_520_ci, instead of hitting "Duplicate entry" partway through ALTER TABLE - Wrap the conversion in try/catch so foreign keys are recreated on failure; log restore errors separately so they don't mask the original exception - Throw RuntimeException if re-adding the people index fails - Remove the re-add of the sales_items_taxes FK in down(), which MySQL 8.4 rejects without a unique key - Revert edits to historical migrations; fix FK handling and table names migration_helper: - Add shared dropAllForeignKeyConstraints() and recreateForeignKeyConstraints(); table/column now optional, dropping all FKs in the schema when omitted - Match CONSTRAINT_SCHEMA when joining KEY_COLUMN_USAGE to REFERENTIAL_CONSTRAINTS to avoid cross-schema name collisions - Rebuild FK lookup with the query builder and bound values instead of interpolated SQL CI/tests: - Add mysql:5.7, mariadb:10.6 and mariadb:10.11 to PHPUnit matrix - Use master instead of latest image tag; improve CI robustness - Mock table() in FailedQueryConnectionTrait so query builder chains work against the failing connection Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> Co-authored-by: aliworksx08 <57456290+aliworksx08@users.noreply.github.com> Co-authored-by: jekkos <jeroen.peelaerts@gmail.com> Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
jekkosandobjecttothis
|
625bb3536a |
fix(taxes): reject negative tax rates and percentages (#4730)
* fix(taxes): reject negative tax rates and percentages
Reject negative tax values across all tax entry points so they can
never be persisted and produce inverted sales totals:
- Items::postSave / postBulkUpdate: validate tax_percents.* as
nonNegativeDecimal (was unvalidated, persisted raw).
- Items::save_tax_data (CSV import): fail the row on a negative
Tax N Percent instead of silently dropping the tax.
- Taxes::postSave: reject negative tax_rate before saving.
- Config::postSaveTax: reject negative default tax rates.
Adds non-negative regression tests for Items, Taxes and Config, plus
boundary tests proving a zero tax rate/percent is still accepted. New
language strings for the rejection messages.
* test: adopt test{Method}_{Purpose} naming convention for new tests
Align the negative-tax test method names with the convention established
in ConfigTest (testPostSave_RejectsNegativeTaxPercent, etc.), per review.
* fix(config): add language entries to all locales, extract TaxFixtureTrait
* fix(i18n): place tax_rate_non_negative in alphabetical order
Move the new tax_rate_non_negative entry to its correct alphabetical
position (after tax_rate_error_adding_updating) in all locale files.
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
|
||
|
jekkosandobjecttothis
|
9fef0bd88f |
fix(models): handle query() returning false in result-consuming methods (#4734)
* fix(models): handle query() returning false in result-consuming methods As of CI 4.1.2, Connection::query() returns false when a query fails (previously an empty result object). Guard the model/helper methods that consume the result so a failed query degrades to an empty result instead of throwing "Call to member function getResultArray() on false". - Tax::get_taxes() - Sale::get_all_suspended() - Reports\Inventory_low::getData() - migration_helper: dropAllForeignKeyConstraints(), indexExists(), primaryKeyExists() Adds QueryFalseGuardTest forcing query() to return false. Closes #3634. * test(query-false): mock concrete driver instead of abstract BaseConnection onlyMethods() does not stub the 16 abstract methods on BaseConnection, so mocking it produced an invalid double. Mock the concrete driver class (resolved from the live test connection) instead. * fix(3634): fail loudly when migration metadata queries return false Model read methods degrade to an empty result on a failed query, but the migration metadata helpers (indexExists, primaryKeyExists, dropAllForeignKeyConstraints) must not mistake a failed query for 'not found' and proceed with schema changes. They now throw a DatabaseException so the migration aborts loudly. - Add an optional injectable BaseConnection param for testability. - Add missing docstrings (docstring-coverage check). - Add regression tests asserting the helpers throw on a failed query. * fix(3634): use supplied connection for FK drop; stub error() in tests Address CodeRabbit review: dropAllForeignKeyConstraints now builds its Forge from the supplied connection (consistent with the lookup) instead of always the default one. Test mock also stubs error() to avoid a warning on the constructor-disabled connection. * fix(3634): revert FK drop to Database::forge() (driver-specific forge) new Forge($db) creates the base Forge which does not implement dropForeignKey for the driver ('This feature is not available'), breaking migrations. Restore Database::forge(), which loads the driver-specific Forge via the factory. The optional connection param is retained solely for testing the fail-loud behavior on the lookup path. * test(3634): add regression test for Inventory_low::getData query-failure guard Covers the low-stock report's degrade-to-empty behavior on a failed query, addressing the CodeRabbit note that it lacked a focused test. * test(3634): adopt test{Method}_{Purpose} naming convention Align QueryFalseGuardTest method names with the convention from #4730 (testGetTaxes_ReturnsEmptyArrayWhenQueryFails, etc.), per review. * test: split QueryFalseGuardTest into per-component test files Apply CodeRabbit review: extract the shared failing-connection helpers into tests/Support/FailedQueryConnectionTrait.php and move each test to its component's test file (TaxTest, SaleTest, Inventory_lowTest, migration_helperTest). Use fresh model instances instead of the shared model() singleton so the injected failing connection does not leak into other tests. * style(tests): remove redundant docblocks from query-failure regression tests --------- Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com> |
||
|
objecttothisandOllama
|
1b8ee2e3c1 |
feat(items): optimize search, attribute filtering, and sort for items view (#4652)
feat(items): optimize search, attribute filtering, and sort for items view (#4652) BREAKING CHANGE: none ## Search & Query Optimization - Split item search into two-phase query: Phase A resolves qualifying IDs, Phase B joins scoped display tables for better performance and readability - Sanitize `definition_ids` via `array_map` to prevent injection vulnerabilities - Introduce subquery for SUM aggregation to prevent over-counting across joins - Add validation requiring both start and end dates before applying date range filter ## Attribute Search (fixes #2919, #2722) - Add `SHOW_IN_SEARCH` flag (value 8) to Attribute model to separate searchability from table visibility - Add `parse_attribute_search()` to parse syntax like `color:blue AND size:large` - Add `applyNamedAttributeSearch()` supporting decimal and date types with locale-aware parsing - Support AND/OR logic for multi-attribute queries ## Sorting - Add `get_attribute_sort_definition_id()` to detect attribute column sorting - Join attribute tables dynamically when sorting by attribute columns - Use `MAX()` for consistent results when sorting by attribute values - Replace static sort column list with dynamic headers via `itemSortColumns()` - Add `sanitizeSortColumnAttribute()` to validate attribute definition IDs as sort columns ## Tax & Data Row - Streamline tax computation in `getItemDataRow()` ## Low Inventory Filter - Require valid `stock_location_id` before applying low inventory filter - Add conditional logic to sort by sum of quantities across all locations when `stock_location_id` is invalid ## Localization - Add `show_in_search` / `show_in_search_visibility` strings to all language files - Translated: de-DE, es-ES, fr, it; English placeholder for remaining locales - Unify single-quote style across all attribute language files ## Refactoring & Style - Adopt camelCase naming throughout (variables, helpers, methods) - Replace `sanitizeSortColumnAttribute` with reusable `sanitizeSortColumn` from `Secure_Controller` - Simplify column key extraction using `array_key_first` - Apply PSR-12 formatting ## Tests - Add tests for tax computation, quantity aggregation (single- and multi-location), named attribute search, free-text parsing, and date/decimal type handling - Add `ensureStockLocation` helper to auto-create missing stock locations in tests - Refactor tests to handle config cache issues Co-authored-by: Ollama <ollama@steganos.dev> |
||
|
jekkos
|
28755dfd50 |
fix(tests): resolve all phpunit failures — clean-DB suite green (#4626) (#4691)
* fix(tests): resolve all phpunit failures (#4626) Bring the phpunit suite from 153 failures to 0 (281 tests passing): - Employee: decouple grants block from save_value success; restructure save_employee new-employee + disallowed-grants early return - Sale: unify sales_payments_temp schema (add sale_cash_refund, reference_code) so both creators produce an identical superset table - Employees controller: provide placeholder password/hash in testing env so new-employee insert succeeds and grant logic is testable - TestDatabaseBootstrapSeeder: reset shared connection table-name cache after bootstrap reset to avoid stale listTables()/tableExists() results - Config: fix postSaveLocale validation rule syntax - Test data: use unique employee usernames to avoid UNIQUE constraint collisions latching strict-mode transStatus=false on the shared conn - Various test-file and language-string corrections * test: consolidate employee fixtures in shared trait Route test employee creation through a single EmployeeFixtureTrait that delegates to Employee::save_employee(), so fixtures exercise the same production code path instead of raw DB inserts. Removes six near-duplicate helpers across EmployeeTest, SalesControllerTest, and EmployeesControllerTest while preserving each test's specific grant set. Closes a piece of the fixture-scattering flagged in #4626. Closes #4626 * test: add global DROP/CREATE grant and commit theme fixtures * fix(ci): remove redundant symlink step, set working encryption key * fix(ci): run phpunit with --no-coverage to avoid no-driver warning * fix: address code review findings - Config: restore strict locale validation (min required|integer|>0) and fix max cross-field check with a new gte_field rule (CI4's greater_than_equal_to[field] does not resolve the field value) - Tests: assert rejection for non-numeric/zero/negative/min>max limits - .env.example: remove shared hard-coded encryption.key (auto-generates); document Docker env-var usage - phpunit.yml: scope CREATE/DROP grant to ospos_test.* and provision a per-run encryption key as an env var * feat: support ENCRYPTION_KEY env var for encryption key Read ENCRYPTION_KEY as a fallback for the encryption key when the config value is empty. This is a supported, reliable path for Docker / container deploys and CI, avoiding reliance on the raw dotted encryption.key env var. * fix: align Summary_report temp tables with Sale temp table schema Summary_report created sales_items_taxes_temp and sales_payments_temp with fewer columns than the canonical create_temp_table() in Sale.php. A later reader expecting those columns hit a schema-mismatch SQL error on the shared temp tables. Add internal_tax/sales_tax (sales_items_taxes_temp) and reference_code (sales_payments_temp) so all creators emit the identical column set. |
||
|
objecttothis
|
9ecabf6f41 |
fix(sales): harden unsuspend with auth, status gating, and null safety
- Require reports_sales grant on postUnsuspend; return 403 on denial - Reject unsuspend of non-SUSPENDED sales; skip silently on invalid state - Move clear_all() after validation so an invalid sale_id no longer wipes the active in-progress cart - Null-guard get_sale_status() on missing row instead of fatal property access; widen return type to ?int - Fix getSaleType null-coalescing — CI4 session default only fires when key is unset, not when value is null - Rename get_sale_type → getSaleType, sale_id → saleId (PSR-12 camelCase) - Extract SaleFixtureTrait with createSale()/createSuspendedSale(); add regression coverage for auth denial, status gating, and cart preservation |
||
|
objecttothis
|
29a9b1a7e7 |
Bugfix: Resolve Race Condition in Rewards and Gift Card Spending (#4640)
* Implement atomic updates for gift card and reward point decrements, enhance error handling for insufficient balances, and add regression tests for concurrency safety. * Add translations for insufficient gift card balance and reward points error messages across all supported languages. * Reorder `clear_suspended_sale_detail` call to ensure transactional consistency. * Reorder `clear_all` call to align with success and error handling logic. * Ensure soft-deleted gift cards are excluded in balance updates. * Refactor change_quantity logic with atomic upserts, improve error handling for insufficient stock, and update related tests and constants. * Added check for NEW_ENTRY * Added unit tests to test changes. * Fix class name casing in ItemQuantityTest for consistency. * Fix Bulgarian translations for insufficient balance error messages in Sales module. * Fix Greek translations for insufficient balance error messages in Sales module. * Fix Armenian translations for insufficient balance error messages in Sales module. * Fix Tamil translations for insufficient balance error messages in Sales module. * Implement race condition testing for database methods with concurrent process support. * Fix class name casing in ItemTest for consistency. * Improve concurrent process handling in race condition tests; add readiness and synchronization barriers. * Improve handling of process I/O streams and timeout management in race condition tests. * Add test for decrementing gift card value when marked as deleted * Add `finally` block to ensure proper cleanup in async database race condition tests * Improve error handling and timeout management in async database race condition tests. * Refactor test utilities to use shared `EmployeeFixtureTrait` and `ItemFixtureTrait`. * Track process exit codes explicitly in race condition tests for improved error detection and debugging. * Improve error handling in `ConcurrentDbRaceTrait` by adding exceptions for `mysqli_poll` and `mysqli_reap_async_query`. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> --------- Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> |