Code Quality¶

Accurate reference for Conflux's code quality tooling configuration and enforced architecture rules.

Running Quality Checks¶

All commands run inside the Docker container.

# Run all checks (Pint + PHPStan + Rector + security audit)
docker compose exec php composer quality

# Fix code style
docker compose exec php vendor/bin/pint --dirty

# Static analysis
docker compose exec php composer quality:phpstan

# Rector dry-run (preview changes without applying)
docker compose exec php composer quality:rector

# Apply Rector changes
docker compose exec php vendor/bin/rector process

# CVE audit
docker compose exec php composer quality:security

Tooling Configuration¶

Laravel Pint¶

Style fixer using the Laravel preset with additional rules:

Rector¶

Automated code upgrades and dead-code removal.

PHPStan / Larastan¶

Static analysis at level 10 via Larastan.

Architecture Rules¶

Enforced by Pest in tests/Unit/ArchTest.php. Violations fail the test suite.

The public-method-documentation rule excludes App\Filament, App\Data, App\Providers, and App\Logging (declarative or framework-contract methods that don't narrate a design decision), and excludes any constructor that is only promoted properties with an empty body.

The PHPDoc block needs at least a one-line summary of what the method does (or why, if the what is obvious from the signature). No @param/@return needed — type hints already cover that (enforced at PHPStan level 10); add them only for something a type hint can't express (e.g. a Closure signature, an array shape).

/**
 * Resolve the integration registered for the connection's system driver.
 */
public function resolve(SystemDriver $driver): IntegrationContract

Lazy Loading Guard¶

App\Providers\AppServiceProvider::boot() calls:

Model::preventLazyLoading();

if (! $this->app->environment(['local', 'testing'])) {
    Model::handleLazyLoadingViolationUsing(function (Model $model, string $key): void {
        $this->app->make(LazyLoadingViolationReporter::class)($model, $key);
    });
}

Detection (preventLazyLoading()) is now unconditional — every environment engages the guard. What differs is the reaction, driven by whether a violation callback is registered:

• local and testing register no callback, so a lazy-loaded relationship still throws Illuminate\Database\LazyLoadingViolationException, exactly as before. This is deliberate: a fast, hard failure during development and CI is what surfaces N+1s before they ship.
• staging and production register App\Services\LazyLoadingViolationReporter as the callback, so a violation is recorded as telemetry instead of thrown. Staging still behaves like production here, not like development — see ADR 0027 for the full reasoning, including why this is a callback re-resolved from the container on every violation rather than a single resolved instance.

Model::shouldBeStrict() is not used here: alongside preventLazyLoading() it also enables preventSilentlyDiscardingAttributes(), which broke unrelated Filament virtual-field handling. Only the lazy-loading piece was adopted.

When the guard trips during development or in a test, the fix is almost always eager loading at the query site — see App\Filament\Resources\Users\Tables\UserTable::configure(), which added ->modifyQueryUsing(fn (Builder $query): Builder => $query->with('roles')) after the role column's state() closure was found lazy-loading roles per row.

Reporting Lazy Loading Violations in Staging/Production¶

App\Services\LazyLoadingViolationReporter turns a lazy-loading violation into an OpenTelemetry span event (lazy_loading_violation) instead of an exception, so a queue job in staging or production degrades one relationship's data rather than dying outright.

• Skip condition parity: Laravel's own preventLazyLoading() skips the throw (but not the callback) for ! $model->exists || $model->wasRecentlyCreated — that check only guards the default throw behaviour, so the reporter re-implements the same condition in handle(). Without it, every freshly created, unsaved model would be reported as a false positive.
• Dedup: keyed by "{$model::class}:{$key}", so a real N+1 — which invokes the callback once per row — reports only once per distinct model+relation pair, bounding the number of span events a single N+1 can produce.
• Backtrace sampling: only the first 5 distinct violations (by dedup key) in a scope's lifetime capture a trimmed backtrace (10 frames, formatted as Class->method() / Class::method() strings). This is a deliberate cost/actionability trade-off, not applied per occurrence.
• handle() vs __invoke(): handle(Model $model, string $key): ?array contains all the decision logic and returns a plain array or null; __invoke() is a thin wrapper whose only job is the one line touching Span::getCurrent()->addEvent(). This split exists because there is no test infrastructure in this codebase for asserting an event reached a span — handle() is fully unit-testable, __invoke() is intentionally left thin and uncovered.
• Scoped container binding: LazyLoadingViolationReporter is bound via $this->app->scoped(...) in AppServiceProvider::register() — the first scoped() binding in this codebase. Laravel's queue worker calls $app->forgetScopedInstances() after every job, so the reporter's in-memory dedup state and backtrace budget reset automatically between queue jobs (and between HTTP requests, since this app runs one PHP-FPM process per request rather than Octane) with no manual cleanup.
• No active span: Span::getCurrent() always returns a valid SpanInterface (a no-op when nothing is active), so addEvent() is a safe no-op outside a span. HTTP requests, CLI commands, and queue jobs all have an active span already (see the CommandStarting/JobProcessing listeners in AppServiceProvider).

Test Structure¶

Tests are split into three tiers, each in its own directory:

tests/
├── Unit/          # No DB, no HTTP. Instantiate classes directly (new Model()).
│   └── Models/   # Cast shapes, relationship types, trait usage.
├── Integration/   # DB required. RefreshDatabase applied automatically via Pest.php.
│   └── Models/   # Scopes, factory round-trips, helper methods that persist.
└── Feature/       # Full HTTP stack — routes, responses, auth flows.

Rules: - Unit tests must never touch the database. Use new ClassName() directly. - Integration tests get RefreshDatabase automatically — do not declare it per-file. - Feature tests cover HTTP behaviour end-to-end. - Mirror the app/ directory structure inside each tier (e.g. Unit/Models/, Integration/Models/). - Group related tests with describe() blocks (e.g. casts, relationships, scopes, traits). Test names inside a describe() should omit the group prefix — test('status as enum') not test('casts status as enum'). - For API endpoints that wire several layers together, add end-to-end coverage in addition to the mocked Feature tests — see End-to-End Tests below.

Testing the Lazy Loading Guard on Table/Listing Pages¶

Illuminate\Database\Eloquent\Builder::hydrate() only sets the preventsLazyLoading flag on hydrated models if (count($items) > 1). In other words, Laravel's lazy-loading guard (see Lazy Loading Guard above) silently does not engage when a query returns exactly one row.

This means a Feature test asserting the guard's behaviour on a table or listing page needs two or more rows to be meaningful — with a single row the test passes whether or not the N+1 fix is in place, guard active or not. tests/Feature/Filament/UserResourceTest.php's 'super admin can access the user list' test creates a second User::factory() with a role assigned for exactly this reason: with one user, LazyLoadingViolationException never fires regardless of eager loading; with two, it fires correctly without the fix and passes with it.

The same two-or-more-rows principle applies even where the lazy-loading guard is not involved at all. App\Filament\Resources\SystemConnections\Tables\SystemConnectionsTable computes the full_access, external_user_id and base_url columns' accessibility from a single SystemConnection::fullyAccessibleBy($user)->pluck('id')->flip()->all() set built once per table build, rather than a per-row exists() query — worth noting that preventLazyLoading() would not catch a regression back to per-row queries here, since this is ad-hoc querying, not a lazy-loaded relationship access. A test asserting the columns' correctness still needs two rows of differing accessibility — see SystemConnectionResourceTest's 'full_access, external_user_id and base_url columns are correct per record without per-row queries' — for the same reason as above: with one row, a column's per-record state looks correct whether it was looked up individually or read from a precomputed set.

A fourth directory, tests/Live/, exists outside this tiering for smoke tests against a real external system — see Live Smoke Tests below. It is not part of composer test/CI.

Live Smoke Tests¶

tests/Live/ holds tests that call a real, reachable external system instead of Http::fake() — a defense against mocks encoding a wrong assumption about a third-party API's actual behaviour. A mocked test proves the code handles the payload we believe the system sends; only a live one proves the system sends it.

tests/Live/PrestaShopLiveConsentTest.php (CFX-174) exists because exactly that happened: every mocked PrestaShopIntegration::pullConsent() test assumed PrestaShop's single-customer endpoint returns {"customer": {...}}, but a live shop returns the list endpoint's {"customers": [{...}]} shape even for a single-id request when an explicit field-list display is used — the mocks couldn't have caught it because they encoded the same wrong assumption the production code made.

tests/Live/WooCommerceLiveConsentTest.php (CFX-93) covers the assumption WooCommerce consent stands or falls on: that the store exposes the connector plugin's conflux_newsletter user meta on the WC REST customer payload at all. WooCommerce filters customer meta against WC_Customer_Data_Store::$internal_meta_keys — a list that is WooCommerce's to change, not ours — so nothing in Conflux's own test suite can vouch for it. See ADR-0026.

tests/Live/UserPlatformLiveVerificationTest.php covers a different kind of assumption: not a payload shape, but that an endpoint documented by a third party actually exists and behaves as documented. What UserPlatformIntegration::performVerification() verifies with came from user.com's published API documentation, not from a running workspace, so this is the only test that can confirm GET /api/public/attributes/ is real, is authenticated the way the docs say, and answers for a workspace without being handed a contact. See User Platform Integration reference.

Not part of the standard suite. tests/Live is registered in tests/Pest.php (pest()->extend(TestCase::class)->in('Live')) so Pest knows how to run it, but it is deliberately not listed in phpunit.xml's <testsuites> — composer test, php artisan test, and CI never discover or run it. Run it explicitly:

APP_ENV=local PRESTASHOP_LIVE_BASE_URL=https://www.prestashop.test \
PRESTASHOP_LIVE_API_KEY=<api_key> PRESTASHOP_LIVE_CUSTOMER_ID=<a real customer id> \
composer test:live

APP_ENV=local WOOCOMMERCE_LIVE_BASE_URL=https://www.woocommerce.test \
WOOCOMMERCE_LIVE_CONSUMER_KEY=ck_... WOOCOMMERCE_LIVE_CONSUMER_SECRET=cs_... \
WOOCOMMERCE_LIVE_CUSTOMER_ID=<a real customer id> \
composer test:live

USER_PLATFORM_LIVE_SUBDOMAIN=<workspace subdomain, without .user.com> \
USER_PLATFORM_LIVE_API_TOKEN=<64-char token> \
composer test:live -- --filter UserPlatformLiveVerification

APP_ENV=local is required for a shop or store on a self-signed/local certificate: PrestaShopIntegration::client() and WooCommerceIntegration::client() only skip TLS verification when app()->isLocal() is true, and phpunit.xml's own APP_ENV=testing does not satisfy that — without it every request fails with a cURL 60 (self-signed certificate). This does not apply to the Positive User tier: a real workspace runs on a real certificate, so UserPlatformClient makes no TLS accommodation to opt out of.

The WooCommerce consent tests additionally need the meta set on that customer. Until the connector plugin renders the newsletter checkbox, seed it by hand in the store's container: wp user meta update <customer_id> conflux_newsletter 1.

Rules: - Skip, never fail, when unconfigured. Every test (or a shared beforeEach) must call $this->markTestSkipped(...) when the required env vars are unset or the target system does not respond to a quick reachability probe — so this directory is always safe to include in a wider run by accident, and safe for every developer who hasn't set up a live target. - Read credentials from environment variables, never hardcode a connection. Build the model in memory (new SystemConnection(['base_url' => ..., 'credentials' => [...]])) — do not depend on a specific SystemConnection row existing in any database, since these tests must run identically for any developer against their own local target. - Compare against the mocked-path mapping, not just "did it not throw." Where practical, assert the live result agrees with a parallel call through the already-mocked-and-tested code path (e.g. bulk export vs. pullConsent() for the same record) — that agreement is the actual signal a real bug like the one above breaks.

Tests that write¶

tests/Live/PrestaShopLiveConsentWriteTest.php covers the other direction: whether a real shop accepts what PrestaShopOutboundWriter sends. A mocked test cannot answer that at all — the fake returns whatever the test told it to, so the three things the writer actually depends on (that a PATCH with an XML body is accepted, that a body naming one field leaves the rest alone, that an unknown id answers 404 rather than 400) were assumptions taken from reading PrestaShop's source until this test measured them.

tests/Live/WooCommerceLiveConsentWriteTest.php does the same for WooCommerceOutboundWriter, against a failure mode PrestaShop does not have: WordPress refuses protected meta and WooCommerce filters customer meta against WC_Customer_Data_Store::$internal_meta_keys, and a refused key does not fail the request — the store answers 200 and drops the value. "Did the write succeed" is therefore the wrong question; the test reads the customer back and asserts the value is still there. It pins a second silent assumption alongside it: that a REST customer update advances date_modified_gmt, the instant the read path maps into consentUpdatedAt. A store that left it standing would have ConsentRecorder discard every later consent state as stale, with nothing failing anywhere.

A test that writes carries two extra obligations:

• Restore what you changed, in a finally. The write test flips the flag it found and flips it back whether the assertions passed or not, so a failing run does not leave the target in the state that failed.
• State what a restore does not undo. A restore is rarely byte-exact, because the target writes fields of its own alongside the one being tested, and the comparison has to exclude those. Name each excluded field and say why it is excluded, next to the exclusion — a bare list reads as "fields that do not matter", when in truth it is the record of the side effects the code under test has. For the PrestaShop consent write those are newsletter_date_add (PrestaShop stamps it when the flag goes to 1 and leaves it standing when the flag goes back to 0) and ip_registration_newsletter (PrestaShop rewrites an empty string to NULL on the same save). For the WooCommerce one it is date_modified/date_modified_gmt, which the store advances on any save — including the restoring write, so a run moves it twice.

Point a write test at a disposable shop — the local sg-prestashop environment on https://www.prestashop.test, or sg-woocommerce on https://www.woocommerce.test — never at anything whose data anybody relies on.

For getting either shop running, issuing credentials with the right permissions, and putting a customer back after a write run, see Run the Live Tests Against a Real Shop. That guide also carries a caution worth reading before trusting a green run: the timezone regression test cannot fail against a shop that is itself on UTC, because the conversion it checks is then the identity.

End-to-End Tests¶

Endpoints that span multiple layers (controller → registry/service → integration → outbound HTTP) should have at least one end-to-end Feature test that drives the real chain and fakes only the outermost boundary, rather than mocking internal collaborators:

• Don't mock internal collaborators (e.g. the IntegrationRegistry). Fake only outbound calls with Http::fake().
• Register test integrations on the real singleton registry, e.g. app(IntegrationRegistry::class)->register(SystemDriver::WooCommerce, HttpVerifyingTestIntegration::class).
• Reuse the shared doubles in tests/Support/: FakeIntegration (closure-driven, for unit-level template tests) and HttpVerifyingTestIntegration (makes a real Http call, for e2e wiring).
• Assert both sides: the persisted state and the outbound request via Http::assertSent(...), so credential/payload plumbing is covered.
• Keep them alongside the mocked Feature tests — the mocked tests pin controller logic; the e2e test pins the wiring the mocks skip (registry resolution, VerifiesConnections::verify() exception handling, the HTTP boundary).

Reference: tests/Feature/Api/RegisterSystemConnectionE2eTest.php.

PHPStan in Tests¶

PHPStan level 10 runs over the tests/ directory. Key rules that apply specifically to tests:

• Null-narrowing after Pest assertions: ->toBeInstanceOf() in a Pest chain does not narrow nullable types for PHPStan. Use assert($var instanceof SomeClass) immediately after the expect call to narrow the type before accessing sub-properties.
• Test integration doubles: Prefer the shared doubles in tests/Support/ (FakeIntegration, HttpVerifyingTestIntegration) over inline anonymous AbstractIntegration subclasses. Any subclass you do write declares only the capabilities the test needs — each as a Can* interface plus its trait — and must implement that trait's hooks. On fetchBulkExportBatches(), carry a /** @param Closure(Collection<int, DomainRecord>): void $onBatch */ PHPDoc, otherwise PHPStan infers a narrower generic type and reports a covariance error.
• Mocking IntegrationContract: Mockery::mock() is typed as MockInterface here (no Mockery PHPStan extension). Annotate the variable /** @var IntegrationContract $integration */ so it satisfies typed parameters, keep the expectation on one line (assign the closure to a variable first), and suppress the resulting analysis noise with // @phpstan-ignore method.notFound, method.nonObject.
• Avoid redundant assert(): For properties typed via #[DataCollectionOf(Foo::class)], PHPStan already knows the element type. A redundant assert($item instanceof Foo) triggers instanceof.alwaysTrue — skip it and access properties directly.