Commit Graph
12 Commits
Author SHA1 Message Date
jekkos 1eaa22a226 docs: add test-method naming convention to AGENTS.md (#4735)
Name test methods test{MethodUnderTest}_{Behavior} (e.g.
testPostSave_RejectsNegativeTaxPercent) so they read as a
self-documenting sentence and need no docblock.
2026-10-06 09:01:56 +02:00
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>
2026-10-02 14:56:20 +04:00
184918d914 fix(security): handle special characters in .env key values and improve insertion logic (#4656)
* fix(security): handle special characters in `.env` key values and improve insertion logic

- Escape backslashes and dollar signs in `applyEnvKeyReplacement` to prevent unintended value corruption.
- Ensure new keys are inserted after `encryption.key` for better organization and manageability.
- Add explicit cast to int to prevent wrong concatenation operator warning.

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* fix(security): handle null return in `applyEnvKeyReplacement` and ensure proper `.env` updates

- Update `applyEnvKeyReplacement` to return `null` on failure, improving error handling.
- Adjust calls to `atomicWriteFile` with updated content to prevent unintended behavior.

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* fix(security): improve error logging and exception messages in file locking

- Add detailed logging for file open and locking errors in `security_helper`.
- Remove unused `helper` and `checkThrottleEncryption` calls from `Events` for cleanup.

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* fix(security): improve atomic file write and handle encryption key placement

- Throw `RandomException` for better error reporting in `atomicWriteFile`.
- Simplify Windows-specific `rename()` fallback logic.
- Fix `encryption.key` assignment order to ensure consistency in `.env` updates.

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* fix(security): improve `.env` file handling and add unit tests for helper functions

- Suppress warnings in `file_get_contents` to prevent unnecessary error logs.
- Update `applyEnvKeyReplacement` to use `preg_replace_callback` for better safety.
- Add comprehensive unit tests for `security_helper` functions to ensure `.env` updates and key management work as expected.

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* fix(security): enhance `.env` update logic and add robust exception handling

- Add `RandomException` to improve error reporting in encryption key management.
- Introduce environment file locking for safer `.env` updates.
- Ensure `applyEnvKeyReplacement` properly handles and inserts old key comments.
- Replace direct file writes with `atomicWriteFile` for consistency.

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* fix(security): refactor `.env` file initialization and encryption key handling

- Introduce `initializeEnvFile` for reusable `.env` setup logic.
- Add `backupEnvFile` and `writeNewEncryptionKey` for robust key management with backups.
- Simplify and clean up redundant `.env` handling code paths.

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* fix(security): clarify `checkEncryption` docblock return value description

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* fix(security): escape backslashes and dollar signs in `applyEnvKeyReplacement`

- Ensure `applyEnvKeyReplacement` properly escapes special characters when inserting or appending `.env` keys.
- Add new unit tests to validate correct handling of backslashes and dollar signs.

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* fix(i18n): add localized error messages and improve error reporting in `security_helper`

- Add missing translations for error messages across multiple language files.
- Update `security_helper` to use localized exception messages with placeholders.

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>

* Redesign encryption/throttle key provisioning as read-only runtime

- checkEncryption()/checkThrottleEncryption() are now read-only guards that
  throw when no valid key is provisioned, instead of writing .env at
  request time.
- Add rotateEncryptionKey() and provisionThrottleKey() for explicit,
  idempotent provisioning.
- Add php spark env:provision (app/Commands/EnvProvision.php) so Docker can
  provision keys once at container startup before any request.
- Add app/Libraries/CI3SecretConverter.php shared CI3->CI4 secret converter
  (AES-128-CBC decrypt + CI4 re-encrypt/verify/save) used by both the
  interactive migration and the docker startup path.
- Refactor convertToCI4 migration to use the shared converter.
- Persist .env in a named volume and run spark env:provision on boot; stop
  baking .env into the shipped image.
- Add guard/rotation/throttle + converter tests; clean up orphaned
  msg_pwd_required language keys across all locales.

* fix: save CI4 ciphertext in env:provision and bind-mount a .env file

Addresses CodeRabbit review on PR #4656:

- env:provision CI3 branch was persisting *plaintext* secrets (saveAll($plain))
  instead of the CI4 ciphertext, unlike the ConvertToCI4 migration. Now
  encrypts with encryptAll(), verifies the round trip, and saves the ciphertext.
- The ospos_env named volume mounted at /app/.env made .env a directory, so
  atomicWriteFile's rename() failed and spark env:provision could not start apache.
  Switch to a bind mount of a host file (./.env) which persists and stays a file.
- Add a regression test asserting the command persists ciphertext (not plaintext).

* chore: trim redundant docblocks in EnvProvision and provision throttle.key in CI

Follow up on @objecttothis review comments:
- app/Commands/EnvProvision.php: remove the boilerplate docblocks the
  property names already convey (group/name/usage/description, run()),
  the two inline step comments, the anyNonEmpty() param docblock, and the
  legacySecretsPresent() docblock. Keeps the class-level docblock since it
  is the only place that states the read-only runtime design + the
  never-persist-plaintext invariant.
- .github/workflows/phpunit.yml: provision a per-run throttle.key the same
  way the encryption key is already provisioned. The PR makes
  checkThrottleEncryption() a read-only guard that throws when
  env('throttle.key') is unset; CI only started exporting ENCRYPTION_KEY,
  so every test that goes through the Throttle filter (7 ThrottleTest
  cases + 4 LoginTest cases) failed with
  "No throttle key is provisioned. Run `php spark env:provision`".
  Writing `throttle.key=<KEY>` into .env matches what
  `php spark env:provision` does on a real container start.

* fix(ci): write throttle.key into .env instead of exporting an OS env var

The previous attempt exported throttle.key via GITHUB_ENV, but CodeIgniter's
env() helper resolves in the order $_ENV[$key] ?? $_SERVER[$key] ?? getenv($key),
and DotEnv populates $_ENV['throttle.key'] from the .env file first. Because the
.env (copied from .env.example) ships with the empty placeholder throttle.key='',
that $_ENV entry exists as '' and short-circuits the ?? chain before getenv()
is reached — so the OS env var was never consulted and every Throttle/Login test
still threw 'No throttle key is provisioned'.

Write the per-run key into the .env file itself (sed-replacing the empty
placeholder), which is exactly what `php spark env:provision` does in
production and is the single source env() actually reads from.

Verify the replacement happened (grep -Eq '^throttle\.key=.') so a future change
to the placeholder format fails the run loudly instead of silently breaking
the 11 throttle-dependent tests.

* fix(security): restore CI3->CI4 auto-provisioning gated by .env writability

checkEncryption()/checkThrottleEncryption() again provision the keys
inline when .env is writable (empty key -> generate; short key -> decrypt,
rotate, re-encrypt, verify, persist legacy CI3 secrets). When .env is not
writable they assume the key was provisioned externally (e.g. docker
env:provision) and throw. Update helper tests to match and correct the
EnvProvision docblock that claimed the runtime was strictly read-only.

* test(security): make short-key conversion branch injectable and test it

checkEncryption() now accepts an optional CI3SecretConverter so the
CI3->CI4 conversion branch can be exercised in unit tests without a
database. Adds testCheckEncryptionConvertsCi3ShortKeyWhenEnvWritable
which seeds CI3-era ciphertexts via a fake Appconfig model and asserts
the key is rotated and the payload verifies back to the original
plaintext.

* fix(security): abort on backup/read/saveAll failure to avoid data loss

Three related data-integrity fixes:

- backupEnvFile() now returns true/false based on whether the backup
  actually exists and is readable. rotateEncryptionKey() aborts before
  destroying the key when the backup could not be written to disk.

- rotateEncryptionKey() and provisionThrottleKey() throw
  RuntimeException(Error.unable_to_read_env_file) when the .env read
  fails, instead of silently replacing the whole file with an empty
  string. This prevents a permission error from wiping all keys.

- checkEncryption() and EnvProvision::run() now both roll back to the
  backup with abortEncryptionConversion() when the post-rotation
  saveAll() throws, matching the migration path (which already did this).
  A failing fake Appconfig is used to exercise this in the new
  testCheckEncryptionRollsBackWhenSaveAllFails test.

* fix(ci): skip comment job in deploy-pr.yml when prepare was not run

The comment job had if: always(), so it ran even when the prepare job
was skipped (e.g. review was not approved). With PR_NUMBER empty the gh
api call posted to issues//comments, received a 404, and the entire run
showed up as failure. Guard the job with
needs.prepare.result == 'success' so it only runs when PR_NUMBER is valid.

* address coderabbit open items: placeholder guards, message neutrality, ar-EG alignment

- backupEnvFile(): fail when mkdir() or either chmod() fails, so the
  pre-rotation backup is actually persisted before the key is replaced
- email/message config views: only show the 'already set' placeholder when
  the secret is actually present (prevented false positives on fresh installs)
- Error.unable_to_create_env_file / .unable_to_read_env_file (en + en-GB):
  use key-neutral wording since both keys are provisioned with the same keys
- ar-EG/Error.php: align all => arrows on the longest key

Item 7 (filesystem test isolation) is a larger refactor — the tests are
serial on CI and tearDown() restores state per test. Left for follow-up.

* test(security): isolate helper FS tests via Config\SecurityEnv

Introduce Config\SecurityEnv holding envPath/backupPath/lockPath so the
security helper reads its target paths from shared configuration instead of
hardcoded ROOTPATH/WRITEPATH literals. security_helperTest.php now redirects
all three to a unique per-run sandbox under sys_get_temp_dir() and tears it
down in tearDown(), so the suite no longer reads/writes the repository's real
.env and is safe to run in parallel.

No helper signature changes; production callers unaffected.

Addresses CodeRabbit item 7 (issue #4700).

Co-Authored-By: opencode <bot@opencode.ai>

* fix(security): run key-conversion as one locked transaction

Address CodeRabbit Major findings from the 4th re-review of the env
helper and its callers:

1. Hold .env.lock for the entire CI3 -> CI4 conversion transaction
   (backup -> rotate -> re-encrypt -> verify -> persist -> cleanup) so a
   concurrent worker cannot interleave a key write between the rotation
   and the ciphertext save. Split rotateEncryptionKey into a lock-free
   core (rotateEncryptionKeyUnlock) plus the existing lock wrapper and a
   new rotateEncryptionKeyTransaction that owns the lock across the full
   unit and performs both the in-lock rollback (abortEncryptionConversion)
   and the in-lock backup removal on success.

2. Treat the legacy value '0' as non-empty data so key rotation still
   persists the re-encrypted ciphertext when '0' is the only stored
   secret (array_filter would have dropped it and skipped saveAll).

3. Wrap the post-rotation re-encrypt/verify/saveAll sequence in a
   catch (Throwable) across all three call-sites so CI4
   EncryptionException, ReflectionException from batch_save, a failed
   round-trip verify, and any other failure all roll the .env key back
   to the pre-rotation state.

4. In Docker Compose, use long-syntax bind with create_host_path: false
   and document in INSTALL.md that the host .env must be a regular file
   (a missing one is no longer auto-created as a directory, and the
   mount now rejects a missing source on Compose implementations that
   support the flag).

Files touched: app/Helpers/security_helper.php, app/Commands/EnvProvision.php,
app/Database/Migrations/20220127000000_convertToCI4.php, docker-compose.yml,
INSTALL.md. All 4 existing helper tests still pass via CI.

* fix(security): make abortEncryptionConversion fail loudly on restore failure

The rollback path restored the .env backup with a suppressed
file_put_contents() and an unchecked file_get_contents(). If the restore
failed after the key had already been rotated, .env was left holding the new
CI4 key while the DB still held CI3-era ciphertext, so the data became
undecryptable after the next restart.

Now the backup read is checked for false and the restore goes through the
existing atomicWriteFile() helper; either failure throws so the error is
surfaced instead of silently corrupting the config. Adds a regression test
that forces an unreadable backup and asserts the throw plus that .env is
left untouched.

* fix(security): guard abortEncryptionConversion backup read before touching it

Validate the backup is a regular readable file (is_file/is_readable) before
reading it, so a missing/malformed backup fails loudly instead of emitting a
file_get_contents() warning. The unreadable-backup regression test now
exercises this guard rather than relying on a promoted warning.

---------

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: jekkos <jeroen.peelaerts@gmail.com>
Co-authored-by: jekkos <jekkos@users.noreply.github.com>
Co-authored-by: opencode <bot@opencode.ai>
2026-09-21 17:35:45 +02:00
objecttothisandcoderabbitai[bot] b610ae28ac fix(validation): broaden sendmail path regex, expand i18n, strip advisory IDs
fix(validation): allow Windows sendmail paths, tighten shell metachar exclusions

Broaden PLAIN_FILESYSTEM_PATH_STRICT to accept real-world sendmail formats
while blocking command injection characters not needed in valid paths.

- OSPOSRules.php: allow space, colon, backslash for Windows paths
  (e.g. C:\wamp64\...) and trailing args (-t -i); still excludes
  ampersand, backtick, subshell, redirect, and cmd.exe metacharacters
- OSPOSRulesTest.php: add cases for Windows paths, trailing args, and
  injection payloads
- Remove 7 ConfigTest assertions that expected metacharacter rejection;
  add acceptance test for sendmail path with trailing args

i18n(lang): expand mailpath_invalid message across all locales

- Fill previously empty mailpath_invalid keys across all locales
- Update existing translations (de-CH, de-DE, es-ES, es-MX, fr, nl-BE,
  nl-NL) to reflect newly allowed characters; nl locales corrected from
  English loanwords to proper Dutch terms
- Add missing key to ckb/Config.php

docs: remove security advisory IDs from public-facing files

- AGENTS.md: extend no-advisory-ID rule to documentation and URLs
- INSTALL.md: drop GHSA reference and advisory link from Host Header
  Injection guidance; rationale and fix instructions remain intact

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
2026-09-10 18:01:12 +04:00
objecttothis 6db3dde491 Bugfix tax names (#4677)
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>
2026-09-01 10:59:29 +04:00
objecttothis d63d31700d Ensure payload data is escaped to prevent XSS (#4664)
fix(barcode): escape payload fields to prevent XSS; PSR-12 refactor

- Apply `esc()` to name, ID, item number, category, and company name in `Barcode_lib` payloads
- Remove redundant `urldecode()` in `Item_kitsController` to prevent triple decoding
- Rename variables and methods to camelCase across barcode, item_kits, and tests
- Add type hint for `$layoutType` parameter in `manageDisplayLayout`
- Add/update unit tests covering escaping and HTTP response assertions

Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
2026-08-28 10:08:31 +04:00
objecttothis cc93c31355 fix(items): add explicit sentinel value for clearing supplier in bulk edit (#4617)
* 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.
2026-08-20 11:24:50 +04:00
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>
2026-08-20 02:24:23 +04:00
objecttothis 8bd2a51cb5 fix(items): validate item_number and skip receiving quantity default for temp items (#4621)
* fix(items): validate item_number and skip receiving quantity default for temp items

- Validate item_number against alpha_numeric_punct rule, return JSON error on failure
- Add item_number_invalid language string

* i18n: reorder Items language keys and add item_number_invalid string

Add item_number_invalid translation across all locale files and
resort surrounding keys alphabetically to match key ordering
convention.

* PSR-12 refactoring.

- Change local variable to camelCase.
- Use single quote in language files.

* i18n: translate item_number_invalid string in ta, th, tl, zh-Hans

Item_number_invalid key had English placeholder text in Tamil,
Thai, Tagalog, Chinese Simplified language files. Translate to
match each locale.

* fix(items): use FormatRules for item_number validation

* refactor(items): use camelCase for variable names

* refactor(items): use camelCase for variable names in Items controller
2026-08-18 02:51:42 +04:00
objecttothisandTravis Garrison 231e716c14 bugfix(reports): crash on detailed sales report when sale has multiple payments with reference codes (#4599)
* fix(sales): aggregate reference codes for multi-payment sales

Use GROUP_CONCAT to combine multiple payment reference codes into a
single comma-separated value, and group only by sale_id to correctly
handle sales with multiple payment records.

Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>

* docs(agents): update AGENTS.md with expanded coding standards

- Clarify PSR-12 enforcement via PHP-CS-Fixer config reference
- Bump minimum PHP version requirement from 8.1 to 8.2
- Add JavaScript const/let/var convention rule
- Reorganize coding standards section for clarity

Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>

* fix(sales): exclude empty strings from payment reference code aggregation

Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>

---------

Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
Co-authored-by: Travis Garrison <travis@chiraqbookstore.com>
2026-07-28 14:23:54 +04:00
objecttothisandTravis Garrison 9c542efaf6 Feature: Payment reference code (#4587)
* 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>
2026-07-10 13:49:05 +04:00
Ollama e01dad728f Add AGENTS.md with coding guidelines for AI agents 2026-03-16 18:02:50 +00:00