src/Controllers/Rest/TrackingRestController.php (1)

136-144: Consider adding a comment explaining the scalar_keys purpose.

The scalar_keys array includes params like 'lt', 'dc', 'ob' that aren't in the route args. This is intentional—these are interaction tracking params that arrive via sendBeacon text/plain, bypassing REST route validation. A brief comment clarifying this distinction would improve maintainability.

Add clarifying comment

-            // Sanitize known scalar keys from REST params (skip when empty — common for sendBeacon)
+            // Sanitize known scalar keys from REST params (skip when empty — common for sendBeacon).
+            // These include interaction params (lt, dc, ob) that arrive via text/plain and bypass
+            // REST route validation, as well as params defined in route args.
             if (!empty($rest_params)) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Controllers/Rest/TrackingRestController.php` around lines 136 - 144, Add
a short clarifying comment above the scalar_keys array in the
TrackingRestController (near the rest_params sanitization block) explaining that
these keys (e.g., 'lt', 'dc', 'ob', etc.) are interaction/tracking parameters
that can arrive via sendBeacon with text/plain and therefore bypass REST route
validation; mention that they’re intentionally sanitized here using
sanitize_text_field and wp_unslash before further processing.

tests/stubs/slimstat-tracker-stub.php (1)

6-19: Stub signatures don't match production implementations.

Based on the relevant code snippets provided:

1. Tracker::slimtrack_ajax() (Line 9): Production returns void (implicitly via Ajax::handle()), but stub returns '1'. Tests relying on the return value won't reflect production behavior.

2. ConsentHandler::handleBannerConsent() (Line 17): Production signature is (bool $return_json = true, array $data = []), but stub uses ($json = true, $data = null). The $data = null default vs array $data = [] type mismatch could cause different behavior if the stub is ever invoked with default arguments.

For unit tests that only verify the controller's merge logic, this is acceptable. However, if future tests rely on these stubs to validate return values or parameter handling, they may produce false positives.

Align stub signatures with production

 namespace SlimStat\Tracker {
     if (!class_exists(__NAMESPACE__ . '\Tracker')) {
         class Tracker {
-            public static function slimtrack_ajax() { return '1'; }
+            public static function slimtrack_ajax(): void {}
         }
     }
 }

 namespace SlimStat\Services\Privacy {
     if (!class_exists(__NAMESPACE__ . '\ConsentHandler')) {
         class ConsentHandler {
-            public static function handleBannerConsent($json = true, $data = null) { return true; }
+            public static function handleBannerConsent(bool $return_json = true, array $data = []) { return true; }
         }
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/stubs/slimstat-tracker-stub.php` around lines 6 - 19, The stub
signatures diverge from production: update
SlimStat\Tracker\Tracker::slimtrack_ajax to match the production behavior (no
return / void) instead of returning '1', and change
SlimStat\Services\Privacy\ConsentHandler::handleBannerConsent signature to use
(bool $return_json = true, array $data = []) with matching default values and
types (avoid $data = null); adjust the stub bodies to mirror production
side-effects/return expectations so tests don't get false positives.

tests/e2e/helpers/setup.ts (1)

270-276: Type declaration for dt_out may not match database reality.

The return type declares dt_out: number, but this column can be NULL in the database when no exit time has been recorded (e.g., for pageviews without an outbound click). Consider using dt_out: number | null to accurately reflect the database schema.

Proposed type fix

-export async function getLatestStatFull(testMarker: string): Promise<{ id: number; resource: string; outbound_resource: string | null; dt_out: number; country: string; city: string } | null> {
+export async function getLatestStatFull(testMarker: string): Promise<{ id: number; resource: string; outbound_resource: string | null; dt_out: number | null; country: string; city: string } | null> {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/e2e/helpers/setup.ts` around lines 270 - 276, The return type of
getLatestStatFull declares dt_out as number but the wp_slim_stats.dt_out column
can be NULL; update the function's Promise return type to use dt_out: number |
null and ensure any call sites handle a nullable dt_out (identify function
getLatestStatFull, the dt_out field in the SELECT, and getPool()/wp_slim_stats
as relevant symbols).

tests/stubs/tracking-stubs.php (1)

22-31: get_body_params() stub always returns empty array.

The production code (TrackingRestController.php:167-172) uses get_body_params() as a fallback for extracting banner_consent and banner_consent_nonce from POST body. The stub returning [] means this fallback path cannot be tested. If banner consent testing becomes necessary, this stub will need enhancement.

Optional: Make body_params configurable

 if (!class_exists('WP_REST_Request')) {
     class WP_REST_Request {
         private array $params;
-        public function __construct(array $params = []) { $this->params = $params; }
+        private array $body_params;
+        public function __construct(array $params = [], array $body_params = []) {
+            $this->params = $params;
+            $this->body_params = $body_params;
+        }
         public function get_params(): array { return $this->params; }
         public function get_param(string $key) { return $this->params[$key] ?? null; }
-        public function get_body_params(): array { return []; }
+        public function get_body_params(): array { return $this->body_params; }
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/stubs/tracking-stubs.php` around lines 22 - 31, The WP_REST_Request
test stub's get_body_params() always returns an empty array, preventing tests
from exercising the fallback path in TrackingRestController::get_body_params
usage; modify the WP_REST_Request stub class to accept and store body params
(e.g., add a constructor parameter or setter for $bodyParams and a private array
$bodyParams property) and update get_body_params() to return that stored array
so tests can supply POST body keys like banner_consent and banner_consent_nonce
when needed; keep existing get_param/get_params behavior intact.

tests/e2e/sendbeacon-interaction.spec.ts (1)

63-63: Consider replacing fixed timeout with dynamic wait.

The waitForTimeout(5000) is a brittle approach that may cause flakiness in CI environments with varying load. Consider polling the database until the stat appears or waiting for a specific network response.

Alternative: Poll for stat to appear

// Replace fixed timeout with polling
let pageviewStat = null;
for (let i = 0; i < 50; i++) {
  pageviewStat = await getLatestStatFull(marker);
  if (pageviewStat && pageviewStat.id > 0) break;
  await page.waitForTimeout(100);
}
expect(pageviewStat).not.toBeNull();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/e2e/sendbeacon-interaction.spec.ts` at line 63, The test uses a brittle
fixed wait via page.waitForTimeout(5000); replace it with a dynamic wait that
polls for the expected stat instead of sleeping: repeatedly call
getLatestStatFull(marker) (or wait for the specific network response) until
pageviewStat shows the expected id/value or a short timeout is reached, then
assert pageviewStat is not null; update the test to remove page.waitForTimeout
and use the polling loop (or Playwright's waitForResponse) to avoid flakiness.