* Fix cache-backend prune crash and stale-cache-on-switch bugs
Two real bugs were found by Codex review on FOSSBilling/FOSSBilling#4221
(a backport of the Redis/Memcached cache backend from #4202 to
0.8-next) that are also present on main.
1. onBeforeAdminCronRun() unconditionally called $cache->prune(),
which only FilesystemAdapter implements. Since #4202 lets
$di['cache'] be a RedisAdapter/MemcachedAdapter, that raised an
undefined-method Error - not caught by the surrounding
catch (\Exception), since Error doesn't extend Exception - aborting
the admin cron before other scheduled tasks (billing, maintenance,
etc.) ran for anyone using Redis or Memcached. Now guarded behind a
PruneableInterface check.
2. Config::setConfig() writes the new config to disk before flushing
the cache, and CacheFactory::clearAll()/clearNamespace() both read
the *live* config when called with no arguments - which by then is
already the new driver/host/database. So update_cache_settings()
switching an admin from e.g. Redis database 0 to Redis database 1
(or Redis to filesystem) only ever cleared the newly configured
backend; entries left in the previous one were never touched and
could reappear if the admin switched back. Both clearAll() and
clearNamespace() now accept an optional explicit $cacheConfig, and
update_cache_settings() captures the outgoing config before the
switch and explicitly clears it afterwards (both the general
namespaces and the Doctrine metadata cache namespace), mirroring
what Config::setConfig() already does for the new config.
Both fixes are best-effort, matching the surrounding code's posture -
a failure clearing the old backend must not throw or block saving the
new config.
* Move hasRedisExtension() into shared Pest bootstrap
AdminTest.php relied on hasRedisExtension() being defined by
CacheFactoryTest.php, which only works because Pest happens to load
that file first when the whole suite runs. Moving it into
tests/Pest.php's shared bootstrap makes it available to any test file
regardless of load order.
Addresses a CodeRabbit review comment on FOSSBilling/FOSSBilling#4222.
* ✨ Show promo redemption history on the promo detail page
Adds a Redemption History table to the product promotion detail page,
listing the individual clients, orders, invoices, phase, status, and
discount amount for every redemption of a promo code. The admin API
(product_promo_redemption_get_list) and backing data model already
existed; the promo list page even linked to a #promo-redemptions
anchor that didn't exist yet. This wires it up.
Refs #588
* 🔒 Don't auto-delete install/ when APP_ENV is ambiguous
checkInstaller() (src/load.php) runs on every bootstrap and, once the
app is installed, deletes the src/install/ directory automatically.
It gated that irreversible delete on Environment::isProduction(),
which silently treats a missing or unrecognized APP_ENV as production.
APP_ENV=dev is only set inside DDEV's web container, so any process
that touches load.php outside of it (a bare `php console.php`, a
stray `php -S`, etc.) resolves to production by default and wipes
install/ with no warning. This happened in a local dev checkout.
Add Environment::isExplicitlyProduction(), which is only true when
APP_ENV is literally "prod", and require it specifically for this
delete. isProduction()'s existing default-to-production behavior is
left untouched elsewhere, since other checks rely on it (email-sending
gates, DNS validation strictness, admin nudges) in ways that are
either non-destructive or where failing toward production is correct.
Also logs before removing the directory instead of doing so silently.
* 🐛 Prevent request query params from overriding the promo_id
CodeRabbit review: merge(request) ran last in the redemption list query,
so a crafted ?promo_id= query param could override the promo actually
being viewed and show another promotion's redemption data. Force
promo_id last so it always reflects the page's own promo, while still
letting per_page/page follow the pagination controls as intended.
* Guard Activity index template against missing client/staff nested data
The activity log API at Activity/Api/Admin.php:38-49 only injects a
nested 'client' or 'staff' key when the corresponding *_id is set
(via isset() which returns false for null). System events and
client-only events therefore have no 'client' key, while staff-only
events have no 'client' key.
Under strict_variables, '{% if event.client %}' (line 191) throws
"Key 'client' ... does not exist" for any event without a client.
Switch the conditional to event.client_id / event.staff_id (always
present as SQL columns) and add '|default("")' to the nested
event.client.email / event.client.name / event.staff.email / event.staff.name
accesses so deleted-but-referenced records render gracefully.
* Guard Security iplookup template against empty country/asn data
When the GeoIP or ASN lookup fails (IncompleteRecord from MaxMind), the
Service catches and substitutes [] for the missing data. Under
strict_variables, the empty mapping at record.asn / record.country
causes line 48 to throw "sequence/mapping is empty".
Twig's |default filter on a GetAttrExpression compiles to a safe
is-defined check, so it correctly falls through to the placeholder
when the underlying array is empty or the key is missing.
Verified:
- /admin/security/iplookup?ip=192.168.1.1 (private IP, empty asn)
renders the row with 'Unknown' ASN cells.
- /admin/security/iplookup?ip=8.8.8.8 still renders
'15169' / 'GOOGLE' / 'United States'.
* Initialize notes/rel arrays in Support toApiArray so templates render without errors
Two related issues surfaced under strict_variables on the admin support
ticket page:
1. toApiArray only set $data['notes'] inside the foreach loop over
ticket notes. Tickets without any notes therefore had no 'notes' key
and 'ticket.notes|length' in mod_support_ticket.html.twig:42 threw
'Key "notes" does not exist'.
2. _getRelDetails returned [] (empty array) when a ticket has no
related entity. mod_support_ticket.html.twig sets {% set task =
ticket.rel %} and then accesses task.task, task.type, task.status
etc. on the next lines. Any access into an empty mapping throws
under strict_variables.
Fixes:
- Service::toApiArray: always initialize $data['notes'] = [] for admin
identity so the key is present even when empty.
- Service::_getRelDetails: always return the base structure with all
fields (id, type, task, new_value, status) set to null when there
is no related entity. Skip the DB lookups for the non-rel case.
Template hardening (defense-in-depth, in case other code paths return
a different shape):
- mod_support_ticket.html.twig:42: 'ticket.notes|length' -> 'is defined and
ticket.notes|length'.
- mod_support_ticket.html.twig:222: 'ticket.notes' -> 'ticket.notes|default([])'.
- mod_support_tickets.html.twig:238: same is defined guard.
Verified on dev:
- /admin/support/ticket/1 renders without errors.
- API now returns rel as {id: null, type: null, ...} and notes: [] for
tickets without related data.
- 1777 local tests pass, PHPStan clean.
* Harden tests and add Twig/ToApi linters
* Remove redundant setAccessible calls in tests
* Update cspell.json word list
* Add UrlAwarePermissiveContainer & trim errors
* Add geoip stub and fix ParserFactory usage
* Enforce and apply parentheses for new expressions
* Import template macros and fix invoices tab id
* Handle nulls in Twig extensions and add tests
* Enable Twig strict_variables and add tests
* Improve Twig strict-variables test harness
Fix several undefined-variable issues in admin templates and harden the strict-variables test harness.
- Templates: guard against missing variables by using default() for order_actions and checking header/type in partial_extensions.twig.
- Loader: CombinedTwigLoader now registers theme and module template paths with namespaces (module_area), and includes email template directories.
- Variable collection: VariableCollectorVisitor now collects keys passed via {% embed %} and {% include %} with { ... } so embedded templates get proper stubs.
- Renderer: StrictTemplateRenderer gains better relative template naming for module templates, additional logging for kb_article parsing, and broader classification of render errors as test-infra (typed-return mismatches, NumberFormatter issues, callable/iterable errors, loader quirks) to reduce false positives.
- Tests: add baseline marker files for strict findings, adjust StrictVariablesTest to treat infra findings as informational while failing on real-bug findings, and write findings JSON for review.
Overall these changes reduce noise from the render-everything harness and make the strict-variables suite more robust and actionable.
* Bump test memory limit to 2G via ini_set in bootstrap
The strict-variables render-everything harness parses every FOSSBilling
template and allocates many stubs and AST nodes, which exceeds the
default 128M limit and causes the pest process to exit with 255
mid-suite. Setting the limit in tests/Pest.php means it applies to both
local 'composer test' and CI 'pest --ci' invocations without needing
to modify the workflow file.
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Scope error_reporting suppression to the render-everything harness
* Replace unsupported 'now' Twig global with 'now'|date filter
The reserved 'now' Twig global is only defined in some contexts and is
not present in the install or admin email-template environments. Under
strict_variables it throws a fatal 'Variable "now" does not exist'
error, which crashes the installer and the email template playground.
Both call sites use the date filter to format the current year, so the
correct fix is to feed the filter a string literal: 'now'|date('Y').
This is the same pattern we used for the admin_default and huraga theme
layouts.
Discovered by hitting the install page on the dev instance after the
strict_variables rollout.
* Make sitemap tolerant of missing product.updated_at
The product guest API (Box\Mod\Product\Service::toApiArray) returns
updated_at only for admin requests, not for guests. The sitemap uses
the guest API, so product.updated_at was always undefined for it.
Before strict_variables this was harmless: the date filter silently
returned null and the template rendered. With strict_variables enabled,
the missing key throws 'Key updated_at does not exist', which
get_custom_page catches and converts to a 404.
Pipe through |default('now') so we fall back to the current date when
the field is missing, while still using the real value when present.
Discovered by the live test 'urls return ok status' failing on sitemap.xml
in CI.
* Remove stray blank line in StrictTemplateRenderer
* Fix four templates for strict_variables Cypress failures
The Cypress tests run against a fresh install with strict_variables
enabled, which catches undefined key access in templates that the
non-strict mode silently allowed.
1. mod_invoice_index.html.twig:432
productId is a JavaScript variable, not a Twig variable. The
Twig expression tried to evaluate it server-side and threw
'Variable productId does not exist'. Build the URL with JS
template literal interpolation instead.
2. mod_client_profile.html.twig:158
guest.client_custom_fields is not a real field in the guest
API response. The current lookup threw 'sequence/mapping is
empty' when no custom fields exist. Default to an empty
mapping, then null, and add a truthy guard.
3. mod_support_ticket.html.twig:60
ticket.rel_type is only populated for tickets linked to an
order; the guest response omits it otherwise. Default to
empty string so the comparison still works.
4. mod_support_ticket.html.twig:80
message.author exposes only name and role for client-side
messages, no email. Pass through |default('') so the avatar
function gets a string and renders its fallback.
* Fix client invoice page for strict_variables
The tab labels on the client invoice index reference
unpaid_invoices.total and paid_invoices.total on lines 33/37
to show count badges, but the corresponding {% set %} calls
sit inside the table bodies on lines 53/85. Under
strict_variables this throws 'Variable unpaid_invoices does
not exist' before the lists are ever fetched.
Move both fetches to the top of the content block so the
variables are defined when first read.
* Allow guest checkout rendering under strict vars
* Rename test helper and tweak test expectations
* Guard optional adapter form fields in gateway and build_form templates
The admin payment gateway edit page and the domain registrar build_form
macro both test element[1].description directly in an if condition,
and both read gateway.config[name] / values[name] without a default.
Built-in adapters (Stripe, PayPalEmail, Custom, the Custom registrar)
define form fields with only a label and sometimes validators or
required_when, so on a fresh install the page throws:
Key 'description' for sequence/mapping with keys 'label, validators'
does not exist in 'mod_invoice_gateway.html.twig' at line 115.
The outer '?? ''' on the inner expression does not help: the
exception is thrown when evaluating the if condition, not when
rendering the markdown. With strict_variables on, that becomes a 500.
Replace each 'if element[1].description' with the short-circuit
'is defined and ...' pattern already used in the same file for
required_when, and drop the now-redundant '?? ''' from the inner
expression. Guard the gateway.config[name] / values[name] reads
with |default('') (or |default([]) for the iterable check) so
fields that have not been configured yet do not throw on a fresh
or upgraded install.
Verified on the dev instance:
/admin/invoice/gateway/{1,2} HTTP 200
/admin/servicedomain/registrar/{1,2} HTTP 200
PayPal gateway form save+reload round-trips correctly.
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Improve type safety, auth checks and PHPStan fixes (#3250)
* Improve type safety, auth checks and PHPStan fixes
* Remove @phpstan-ignore annotations
* Improve type safety, auth checks and PHPStan fixes
* Remove @phpstan-ignore annotations
* Remove redundant @phpstan-ignore comment
* Improve code robustness and reliability (#3248)
* Harden code against null/undefined values
* Update dependency lockfiles
* Sanitize ticket input and use IDs in email list
* Restructure tests and add test utilities
* Move E2E tests to `Library\E2E` namespace
* Update `php-build` action ref in workflows
* Switch tests to Pest and add test scaffolding
* Refactor E2E tests to Pest-style tests
m
Skip E2E tests when env not configured
* Refactor Unit tests and update test config
Add test datasets and refine test helpers
* Add DEBUG guard, fix API JSON decode, update tests
* Refactor tests: add types and Mockery expectations
* Adjust tests, phpstan config, and composer script
* Instantiate test APIs locally, remove beforeEach
* Refactor runtime checks, types & logging
* Apply PHP Code Styling Changes
m
* Add PHPStan ignores, test renames & small fixes
* Enable Pest tests in CI workflow
* Remove legacy tests; add new unit tests/config
* Avoid undefined index in fingerprint comparison
* Ignore properties absent in both fingerprints
* Remove public_ticket_create API method
* Add Fingerprint unit tests
* Remove em dash from fingerprint test name
* Switch to Pest; assorted bug fixes & refactors
* Prefer getDi() and guard DEBUG define
* Refactor tests: DBAL, Antispam rename, helpers
* Refactor email tests to use Doctrine entities
* Update tests, use DBAL mocks & improve Formbuilder admin
* Validate plan_id and update unit tests
* Replace error_log with DI logger and update tests
* Expect db->store in Invoice Service test
* Use DI logger instead of error_log; fix tests
* Fix typo in Hook Service test
* Fix Staff test spelling fixture
* Complete Pest test migration
* Fix Product test spelling fixtures
* Fix Product test spelling fixtures
* Run live tests with Pest and load helpers
* Migrate tests to ApiClient; remove legacy APIHelper
* Fix Email test spelling
* Remove unused PHPUnit bridge
* Organize Cypress end-to-end tests
* Document Pest test directory root
* Remove noisy Pest comments
Adam Daley