Resolve language-file conflicts by taking master's English-removal
cleanup across non-English locales, while stripping the MailChimp
keys master reintroduced (already moved to MailchimpPlugin on this
branch) and reapplying this branch's plugins/integrations rename and
new Module.php plugins keys where master lacked them.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(sales): allow gift card credit for returns + surface errors
- Add incrementGiftcardValue() to Giftcard model for crediting cards
- In Sale::save_value(), use incrementGiftcardValue() for returns
with negative gift card payments (credits the card)
- Add error_message rendering to register view so failed completions
show the error instead of silently reloading
Fixes#4779
---------
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
build/refactor/fix: tighten tooling, rename accessors to camelCase, and correct model bugs
* build: tighten gitignore, lockfile, and script config
- gitignore /build and /.php-cs-fixer.cache
- allow core-js@3.50.0 postinstall script in package.json
* refactor: rename snake_case data accessors to camelCase
Rename get_info, get_multiple_info, get_sale_items, get_sale_payments,
and save_customer across Customer, Employee, Item, Sale, and Supplier
models to PSR-12 camelCase, updating every call site in controllers,
libraries, and views.
* test: update calls to match renamed camelCase accessors
Update HomeTest, ItemsControllerTest, and ItemTest to call
getInfo()/getMultipleInfo() instead of the old snake_case model methods.
* fix: correct independent bugs in Item, Item_taxes, and Attribute models
- Item::delete()/delete_list(): let $this->db->transStatus() alone decide
success instead of accumulating a bitwise-AND over intermediate results,
which could report failure (and trigger error toasts) even when the
delete had actually completed
- Item_taxes: correct $table property from 'item_taxes' to 'items_taxes'
- Attribute::getDefinitionNames(): pass an array to
QueryBuilder->whereNotIn(), which requires one
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
Resolve language file conflicts: take master's translations, strip
MailChimp phrases (MailChimp is now a plugin instead of core), and
retain plugin-system additions (plugins/plugins_configuration keys).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
Add a new detailed item sales report that shows all items sold per row
in an exportable format, including sale transaction information. The
existing detailed transaction sales report lacked per-item row display
and export support, and the items summary report did not classify by
sale.
- Wire sale and discount filters for the report
- Validate detailed item sales route filters
- Add English fallback translations for other languages
- Use blank quotes for other language translations
- Normalize language key placement
- Use PSR-style names throughout the report
* 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>
* fix(appconfig): refresh cached settings once and avoid stale same-request reads
Addresses issue #3693, which has two related problems:
1. Repeated cache refreshes. Appconfig::batch_save() called save() per key,
and save() unconditionally deleted + reloaded + re-cached the settings after
every single write. A settings page with dozens of keys therefore refreshed
the cache dozens of times in one request. batch_save() now persists all keys
in the transaction and refreshes the cached settings exactly once after it
commits successfully. save() keeps refreshing so direct single saves still
work.
2. Stale same-request reads. Controllers copy config(OSPOS::class)->settings at
construction time, so after a save in the same request the local copy is out
of date. Config::postSaveInvoice() read the stale invoice_enable /
default_register_mode values and sent the user to the wrong screen (e.g.
opening a new sale instead of the invoice form after saving an enabled
invoice). It now reads the freshly refreshed settings.
Adds tests/Models/AppconfigTest.php covering batch_save() and save() refreshing
the cached settings after a successful write.
* test(3693): adopt test{Method}_{Purpose} naming convention
Align AppconfigTest method names with the convention from #4730
(testBatchSave_PersistsAllKeysAndRefreshesCachedSettings, etc.), per review.
* style(config): remove inline comments per review feedback
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
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>
Per the project's policy of treating security advisory IDs as
secret-like, remove the identifiers embedded in source/test comments
and CHANGELOG entries. Each keeps its human-readable description (and
PR number where present), so traceability is preserved. No logic changes.
Add getExportDataBulk() to Item model for plugin use.
- Fetches item_id, item_number, name, description, unit_price,
and pic_filename for multiple item IDs in single query
- Returns results keyed by item_id for easy lookup
- Guards against empty input array
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
- Broaden OSPOSRules filesystem path regex to allow space, colon,
and backslash for Windows sendmail paths while still excluding
shell metacharacters used unescaped in popen(); update docblock
accordingly.
- Restore/translate mailpath_invalid message across all language
files so users get a real error string instead of empty text.
- Guard Config controller license rendering against malformed or
non-array npm-prod/npm-dev LICENSES JSON, skipping entries
missing required keys instead of raising warnings.
- Document Node.js 20+ requirement in BUILD.md for license
reporting dependency.
- Remove security advisory IDs/URLs from INSTALL.md and code
comments per new AGENTS.md policy against referencing CVE/GHSA
identifiers anywhere in the repo.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* 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.
- Add `editable` override support in `bootstrap_tables_locale.php` to fix misaligned language file lookups for plugins.
- Update documentation explaining the issue and providing a resolution.
- Adjust `Sales` and `Attribute` logic for clearer naming (`sale_type` to `saleType`) and better attribute mapping (`attribute_id` added).
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
- 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
* Validate gift-card payment amounts (GHSA-9847)
Close the negative gift-card amount minting vector: when a forged
payment_type like 'Gift Card:<number>' reaches the catch-all validation
branch, a negative amount_tendered previously passed decimal_locale and was
then routed into Giftcard::decrementGiftcardValue, where value - (-N)
increased the balance (store credit minted at will).
- Add nonNegativeDecimal rule + 'Sales.negative_amount_tendered' message to
the catch-all amount_tendered rules in Sales::postAddPayment(); add the
language key to all 46 locale files (populated in en, empty elsewhere).
- Guard Giftcard::decrementGiftcardValue() against non-positive amounts so
the sink itself can no longer add balance from an inverted subtraction.
- Regression tests: controller-level rejection of negative amount_tendered
and model-level rejection of negative/zero decrements.
* Address PR review: align locale keys, drop advisory refs, add decimal_locale message
- Align negative_amount_tendered '=> with all other keys (46 locale files)
- Remove docblock + inline comment above decrementGiftcardValue()
- Remove GHSA ID and attack-detail description from test; scrub redundant comment
- Add decimal_locale message override + focused malformed-amount test
* Fix formatting and spacing in SalesControllerTest
* fix(lang): remove duplicate negative amount tendered key
Consolidate 'negative_amount_invalid' and 'negative_amount_tendered'
translation keys in Sales.php across all locale files. Both keys held
identical messages, causing redundant translation maintenance.
- Drop 'negative_amount_invalid' key, keep 'negative_amount_tendered'
- Move existing translated text into 'negative_amount_tendered' where
it was previously empty
- Applied across all app/Language/*/Sales.php locale files
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(sales): allow negative amount_tendered in return mode
Return transactions legitimately produce negative amount_due and
prefilled amount_tendered values, but validation rules previously
enforced nonNegativeDecimal unconditionally, blocking valid returns.
- Detect return mode via sale_lib->get_mode() in Sales::process
- Build amount_tendered rule conditionally: skip nonNegativeDecimal
check when in return mode, keep it for sale/giftcard flows
- Apply the conditional rule to both giftcard and standard payment
branches
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* test: update expected error message in negative payment test
Sales controller now returns generic numeric-validation message
instead of specific negative-amount message for negative tendered
amounts. Update test assertion to match new lang key.
- tests/Controllers/SalesControllerTest.php: assert
Sales.must_enter_numeric instead of
Sales.negative_amount_tendered
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* test: remove regression tests for GHSA-9847 negative amount fix
Drop testDecrementGiftcardValueRejectsNegativeAmount and
testDecrementGiftcardValueRejectsZeroAmount from GiftcardTest.
- Remove coverage for decrementGiftcardValue() rejecting
non-positive amounts (negative/zero) in tests/Models/GiftcardTest.php
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix: prevent duplicate items when editing imported rows
Item::exists() matched on item_id OR item_number and required exactly
one row, so a numeric barcode colliding with another item_id caused
saves to insert duplicates. Treat any match as existing.
Also removes the comment explaining numeric barcode matching behavior.
Fixes#4584
- Validate paymentType is a non-empty string before processing
- Reject negative or zero amounts for all payment types
- Enforce full payment coverage before completing a sale
- Bypass coverage check for invoice and quote mode sales
- Require a valid gift card number before decrementing value;
rollback and return insufficient balance error on missing input
- Add "amount_due_not_covered" and "negative_amount_invalid"
translations across 40+ locales
- Add test coverage for gift card validation, negative amounts,
and quote/invoice zero-payment completion
- Rename snake_case locals to camelCase in postComplete (no behavior change)
bugfix(items, validation): reject unsafe tax names and fix payments temp table collision
- Add unicode_alpha_numeric_punct rule (OSPOSRules) to allow accented/CJK
chars in text fields while blocking HTML-unsafe chars (<, >) as
defense-in-depth against injection
- Items controller: extract validateItemFields/validateBulkUpdateFields,
validate tax_names on save and bulk update using new rule; add shared
validateFields helper in Secure_Controller to DRY up validation +
JSON error response
- Escape tax_group output in sales/quote.php and receipt_email.php views
to harden output encoding at render time
- Rename sales_payments_temp -> sales_report_payments_temp (Summary_report)
and -> sales_search_payments_temp (Sale model) to avoid name collision
between concurrently-created temp tables
- AGENTS.md: document alignment rule for => columns when inserting new
language keys
Tests:
- Add ItemsControllerTest covering postSave/bulkupdate tax_names validation
- Reject <, > in tax_names on /items/save and /items/bulkupdate
- Verify unicode and apostrophe-containing tax names are accepted
- Cover CSV import helpers: header generation (basic, multiple locations,
attributes), stock-location/attribute header builders, get_csv_file
parsing (plain, BOM-prefixed, multi-row)
- Validate required-header detection for import templates
- Remove outdated tax name test from SalesControllerTest
- Simplify Database class references in SalesControllerTest
i18n:
- Add tax_name_invalid translation to Items.php for 20+ locales, inserted
alphabetically after tax_category in each file
- Normalize quote style in ar-EG/Items.php to single quotes
- Add ka/Items.php Georgian locale scaffold
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
- Sales::getSearch now enforces reports_sales grant before running
search, returns 403 with lang message when missing
- Rename snake_case helpers/methods to camelCase across
Sales controller, Sale model, and tabular_helper
(get_sale_data_row -> getSaleDataRow, get_payments_summary ->
getPaymentsSummary, sales_headers -> salesHeaders, etc.)
- Config/OSPOS: reset DB data cache before checking app_config
table existence to avoid stale schema cache in tests
- TestDatabaseBootstrapSeeder: expose static reset() so tests can
rebuild schema once per class instead of only via seeder run()
- SalesControllerTest: bootstrap DB once per class, seed once,
refresh app settings each setUp, add tests for search endpoint
authorization (cashier denied, supervisor allowed), move
createTestItem into shared ItemFixtureTrait
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix(employees): harden permissions UI and access control
- Prevent admins from removing their own minimum module grants (employees, home, office)
- Add session_status check before session regeneration
- Disable submit button and return no_access view for AJAX requests
- Replace fade class with active-only for Bootstrap compatibility
- Update permission toggle selectors to .module-toggle
- Add error_cannot_remove_own_minimum_grant translations for Armenian, Bulgarian, Georgian, Swedish, and Ukrainian
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix(sales): gate per-record endpoints behind reports_sales grant (REDACTED)
Cashiers holding only the base `sales` grant could reach per-sale endpoints
(getRow, getEdit, postSave, getReceipt, getInvoice, getSendPdf, getSendReceipt)
that require `reports_sales`. getManage() enforced this at the list level, but
individual endpoints did not re-check. Regression tests added.
Auth:
- Introduce `IsLoggedIn` filter to centralize login checks across controllers
- Replace custom `AccessDeniedRedirectException` with built-in `RedirectException`
Employees:
- Add `DISALLOW_PASSWORD_CHANGE` and `DISALLOW_GRANT_CHANGE` env vars to restrict
credential and permission changes in locked-down environments
- Extract `hasGrantsChanged()` to streamline `postSave`
Refactor:
- Rename snake_case variables to camelCase in Sales, Items, and Employees
controllers for PSR-12 compliance
- Use explicit `db_connect()` for transaction clarity in Items controller
Fixes:
- SMTP config entries fall back to defaults via null coalescing
- Migration uses `DROP FOREIGN KEY` instead of `DROP CONSTRAINT`
- Password hash upgrade only sets session on successful `hash_version` update
- Correct lang key for unknown error in Module model
Language:
- Translate `error_grant_change_disallowed` / `error_password_change_disallowed`
across all 44 supported locales with => alignment matching en reference
- Fix "cannot be deleted" messages and misc typos across ~15 language files
Tests:
- Bootstrap seeder only once in ItemsCsvImportTest; close connection after
- Restore `DISALLOW_GRANT_CHANGE` in teardown to prevent side effects
- Use `uniqid()` for test user data to avoid collisions
Signed-off-by: 17935339+objecttothis@users.noreply.github.com
* fix(items): add explicit sentinel value for clearing supplier in bulk edit
This fixes a regression introduced in the fix for [REDACTED]
Introduce `Item::CLEAR_SUPPLIER_OPTION = 'NONE'` to distinguish between
\"leave supplier_id unchanged\" (empty string) and \"clear supplier_id\"
(sentinel). Previously, empty string was ambiguous.
- Add `CLEAR_SUPPLIER_OPTION` constant with doc comment explaining intent
- Update supplier dropdown to include sentinel as first real option
- Shift empty string to mean \"do nothing\" across all bulk edit fields
* test(items): add regression tests for mass assignment in bulk edit
Cover [REDACTED]: Item::update_multiple() bypasses model
$allowedFields via Query Builder, allowing unintended field writes
during bulk edit operations.
* fix(items): add type validation to bulk-edit field filter
filterBulkEditFields now validates field values before accepting them:
- Non-scalar values are rejected (array injection guard)
- Price/quantity fields are locale-parsed to floats, invalid strings skipped
- Boolean fields must be 0 or 1, other values skipped
- supplier_id must be numeric; CLEAR_SUPPLIER_OPTION still nulls it
Update tests to assert parsed types (float for prices, int for
supplier_id) and replace the fill-all-fields fixture with a realistic
input that only covers fields a form would actually submit.
* test(items): add supplier cleanup and helper methods to bulk update tests
- Track created supplier person IDs for teardown cleanup
- Delete supplier records in tearDown to prevent test pollution
- Extract item/supplier creation into reusable helper methods
* style(tests): rename variables to camelCase in ItemBulkUpdateTest
* refactor(items): rename snake_case variables to camelCase
Convert Item model, Items controller, and bulk update tests to
PSR-compliant camelCase naming per project conventions.
- Rename update_multiple to updateMultiple in Item model
- Rename local variables (item_data, items_to_update, tax_names, etc.)
to camelCase across Items controller and Item model
- Update ItemBulkUpdateTest to use new updateMultiple method name
- Reorder and update AGENTS.md naming conventions
* style(tests): convert snake_case variables to camelCase in ItemBulkUpdateTest
Rename local variables and property names to camelCase for PSR-12
consistency, matching convention used elsewhere in new test code.
* 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>
* fix(xss): remove redundant escaping that double-encoded item attribute values
- Remove esc()/html_entity_decode() calls now that output is escaped
at render time by the framework, preventing double-encoding of
special characters in attribute names, units, and definition values
- Fix employee_name form_input value fields to stop pre-escaping
before form_input applies its own escaping
- Reorder Items.php use statements and add missing BaseConnection import
- Change items/manage.php start_date from let to plain assignment for
proper reassignment scope
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* test(sales): add regression tests for permission checks on sales endpoints
- Ensure role-based permissions correctly restrict access to sensitive actions like price edits, receipt/invoice views, and report generation.
- Add tests for both granted and restricted user scenarios to validate the behavior.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* fix(attributes): validate `attribute_value` before processing
- Add checks to ensure `attribute_value` is a non-empty string in `postSaveAttributeValue` and `postDeleteDropdownAttributeValue` methods.
- Return error response if validation fails to prevent invalid data handling.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* fix(attributes): improve error handling and optimize affected items processing
- Use `array_column` for extracting item IDs to streamline logic.
- Add JSON validation with `JSON_THROW_ON_ERROR` and return proper error response for invalid `definition_values`.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* test(sales): enable database refresh for consistent test state
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* test(sales): assert unauthorized message is displayed on restricted access
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* refactor(attributes): use camelCase for `attributeValue` in controller methods
- Standardize variable naming in `postSaveAttributeValue` and `postDeleteDropdownAttributeValue` methods by switching to camelCase.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
---------
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
Co-authored-by: Travis Garrison <travis@chiraqbookstore.com>
Renames the Attribute-specific definition methods from snake_case to
camelCase and updates every call site:
get_definition_by_name -> getDefinitionByName
get_definition_names -> getDefinitionNames
get_definition_values -> getDefinitionValues
get_definitions_by_type -> getDefinitionsByType
get_definitions_by_flags -> getDefinitionsByFlags
get_definition_flags -> getDefinitionFlags
Also documents getDefinitionByName()'s return contract: a single
definition row as an associative array, or [] when none matches,
matching the getRowArray() behaviour introduced in #4464.
get_found_rows() and get_total_rows() are deliberately left alone -
they are declared across 15 models and renaming them only here would
break that shared convention.
Refs #4622
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix: wrap postSave() in single transaction for atomicity
- Remove internal transaction from Item_taxes->save_value() to allow controller-level transaction
- Wrap entire save sequence (item, taxes, quantities, inventory, attributes) in single transaction
- Ensure all operations succeed or all fail together
- Prevents partial writes when saveItemAttributes() fails after item/tax/quantity saves succeed
Fixes#4474
* fix: Use explicit transBegin/transCommit/transRollback for atomicity
- Replace transStart/transComplete with transBegin/transCommit/transRollback
- Check all success conditions before committing
- Explicit rollback on failure
Address CodeRabbit review feedback
* refactor(items): rename postSave locals to camelCase per PSR-12
Convert snake_case variables to camelCase in postSave() and related
item-save logic to match project naming convention for new methods.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
---------
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
Co-authored-by: Ollama <ollama@steganos.dev>
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: Travis Garrison <travis@chiraqbookstore.com>
- Add `idMatchesPluginConvention()` guard to `BasePlugin` — rejects
module/permission ids that don't match `{plugin_id}` or `{plugin_id}_*`
- Add `unregisterPluginModules()` to `PluginManager` — called automatically
on uninstall; removes all modules and sub-permissions by convention prefix
- Plugins no longer need to call `unregisterModule()` in `uninstall()`
- Update README to document enforced naming convention, auto-cleanup
behavior, and corrected language key / file location guidance
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
- Rename get_giftcard_id to getGiftcardId (PSR-12 camelCase)
- Fix return type from bool to int|false
- Replace loose == with strict === comparison
- Update all call sites in Giftcards controller and Sale model
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
Co-authored-by: Travis Garrison <travis@chiraqbookstore.com>
* Add `reference_code` to sale payment queries and group by statements
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* Refactor `Sales` controller to improve payment handling readability and replace snake_case with camelCase variables
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* Add missing translations for `Sales` language file and include new keys like `must_enter_rrn` and `reference_code`
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* Refactor `Sales` payment handling to use camelCase and extend `addPayment` with `referenceCode` support
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* Refactor `Sales` models, controllers, and libraries to adopt camelCase naming conventions and improve readability
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* Add translations and updates for `must_enter_reference_code` and `reference_code` across language files and update `Sales` controller to replace `must_enter_rrn` with the new key
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* feat(sales): add reference code input and payment type helper
- Add `get_reference_code_payment_types()` to locale_helper as single
source of truth for card-requiring payment types
- Add reference code row to register view, shown/hidden via JS based
on selected payment type
- Fix payment type dropdown width to 100% for consistent layout
- Add min-width to payment buttons and right-padding to button group
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* feat(config): add payment reference code length configuration
- Add payment_reference_code_min and payment_reference_code_max fields
to Config controller save logic
- Add translation keys for reference code length limits across all
language files (min/max label + section header)
- Align array key formatting in Config controller for readability
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* style(lang): normalize string delimiters to single quotes across all language files
Convert double-quoted array keys and values to single quotes in all
app/Language/*/Config.php and app/Language/*/Sales.php variants.
No translation content changed — formatting only.
Also add Localization section to AGENTS.md documenting language file
conventions for new keys and fallback behavior.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* feat(lang): add payment reference code length translations
Add localized strings for payment_reference_code_length_limits,
payment_reference_code_length_max_label, and
payment_reference_code_length_min_label across all supported locales.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* test(config,sales): add payment reference code min/max validation tests
- Add baseLocalePayload() helper in ConfigTest for postSaveLocale tests
- Add testSaveLocale_AcceptsValidReferenceCodeMinMax and related boundary tests
- Add Sale_libPaymentTest for payment reference code validation in Sale_lib
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* fix(sales): add type-specific validation for amount_tendered
Gift card payments use amount_tendered as giftcard number (integer);
cash/other payments require decimal_locale format. Apply correct
validation rule per payment type instead of generic required.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* fix(lang): replace self-closing </br> with <br> in all locales
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* fix(sales): use configurable precision in discount comparison
Replace hardcoded precision 2 with totals_decimals() when comparing
discount against item total via bccomp/bcmul, so discount validation
respects the configured decimal precision setting.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* fix(lang): correct Azerbaijani translations in Config.php
Replace placeholder/mismatched strings with accurate translations:
- email_mailpath, email_smtp_pass, invoice_email_message
- number_locale_invalid/required, receipt_template
- reward_configuration, right, tax_decimals, theme
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* feat(sales): add reference code support to payment edit flow
- Add reference_code field to new payment row in sale edit form
- Persist reference_code on insert in Sale model
- Validate reference_code_new in Sales controller using configurable min/max length rules
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* feat(lang): add Georgian (ka) language stubs for Config and Sales
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* Match the fallback maximum reference_code length to the maximum of the field in the db
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* Bug fixes
- Add validation of UI settings to prevent overridden values from being passed.
- Correct maximum value in JS for payment_reference_code maximum length to 40.
- Fix bug causing copy_entire_sale() to incorrectly copy the reference code and cash_adjustment
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
---------
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
Co-authored-by: Travis Garrison <travis@chiraqbookstore.com>
- Add `$pluginData` parameter to `Events::trigger` calls for key events (e.g., `sale_completed`, `customer_saved`)
- Update core views to include `data-plugin-form` attribute for plugin-field serialization
- Bundle `plugin_data_helper.js` to handle form field extraction for plugins
- Extend README.md with comprehensive documentation on using `$pluginData`
- Refactor language files to support `install` translations and streamline UI labels
- Add new view hooks for sales receipt and register UI extension
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
- Intercept uninstall action to show confirmation modal before AJAX call
- Change uninstall to soft-delete: set installed=0 instead of purging all config
- Add deleteAllNonControlForPlugin to preserve control rows on uninstall
- Add isPluginInstalled helper to PluginManager for installed state checks
- Fix isInstalled guard to treat installed=0 as not installed
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
Rename get_info, get_sale_items, get_sale_payments across Item, Sale
models and all call sites in controllers, libraries, and tests.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
- Add `runPendingMigrations()` to `PluginManager` for executing plugin-specific migrations.
- Create `PluginMigrationModel` for managing plugin migration versions.
- Add migration for creating the `plugin_migrations` table to track migration states.
Signed-off-by: objec <objecttothis@gmail.com>
