diff --git a/RELEASE.md b/RELEASE.md index 9e64ff1d..86343070 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,38 +1,31 @@ -# v1.6.63 — Driver, customer and contact accounts stay out of the console +# v1.6.64 — Order reporting and test SMS with the entered credentials -## Improvements +## Improvements for reporting -- Treat `driver`, `customer` and `contact` users as managed accounts: the FleetOps profile owns them, not IAM. `User` gains `MANAGED_TYPES`, `isManagedAccount()`, `isStaffAccount()`, `canAccessConsole()`, `canHoldConsoleSession()` and a `managed()` scope. -- Promote instead of duplicating. When IAM creates or invites a team member whose email or phone belongs to a managed account in the organization, that account becomes a `user`. It gets the chosen role, permissions and policies and a join invite, and keeps its driver and customer profiles. The response carries `promoted_from`. Accepting any IAM invite also promotes a managed account and asks it to set a console password. - -## Improvements for IAM - -- Let IAM admins ask a user to verify their email or phone. `POST users/{id}/send-verification` sends a one-click link by email or SMS; it lasts 48 hours. The public `auth/confirm-contact-verification` confirms it without signing in, and refuses the link if the address changed since. `users/verify/{id}` takes a `channel` (email by default, or phone). `UserFilter` adds `email_verified`, `phone_verified`, `country` and `timezone` for the new IAM columns. +- Declare row-level expression columns with `Column::expression($name, $sql, $type)`. Bare names resolve against the table or relationship that declares it, so `JSON_EXTRACT(meta, '$.quantity')` on `payload.entities` reads the joined entity's `meta`. An expression column can be selected, filtered, sorted, grouped by and aggregated. +- Summary columns (`Column::count/sum/avg`) are flagged `aggregate` and resolve to their computation. Without grouping they return a single summary row; with grouping they sit beside the group keys. +- `Table::softDeletes()` and `Relationship::softDeletes()` leave out soft-deleted rows. On joins the filter goes in the `ON` clause, so LEFT joins keep the parent row. +- Computed columns can be group keys, conditions and sort columns, and a grouped report can be sorted by an aggregate's alias. A new `count_distinct` aggregate is available. +- Custom expressions accept the JSON functions, `DATE()`, `CAST(… AS DECIMAL(15,2))` and the other cast types, `DISTINCT`, `IN`, `GROUP_CONCAT(… ORDER BY … SEPARATOR …)`, `->`/`->>` and `INTERVAL n UNIT`. +- `public_id` and `internal_id` are no longer hidden as foreign keys, so ID columns appear in the column picker. +- Relationship columns are labelled with the whole relationship name ("Order Config Namespace", not "Order Namespace"), and aggregate labels use the column label ("Sum (Quantity)"). +- `_key` and `_import_id` are never listed or selectable, whatever a schema declares. ## Fixes -- Keep managed accounts out of the console: - - `auth/login` refuses drivers, contacts and customers. Customers keep the `customer_login_not_allowed` code; drivers and contacts get `console_access_not_allowed`. - - Session restore, bootstrap, 2FA verification, verify-email tokens and impersonation refuse drivers and contacts. - - Customers are still allowed on those endpoints because the customer portal runs inside the console and restores its session through them. -- Free a deleted user's email and phone so a new account can use them. On soft delete they move to `meta.deleted_identity`; restoring the user puts them back only if no other account has taken them. +- Test SMS Provider and Test Twilio in Admin › System Config › Services use the credentials entered in the form (fleetbase/fleetbase#680). Under Octane the Twilio client was built once per worker, so a test failed with "Credentials are required to create a Client" or reported success for the saved account. The endpoints now rebuild the client from the request's config and release it after the send. ## Security -- Never grant the Administrator role by default. Before this fix: - - Creating or inviting a user without a role gave them the Administrator role, and so full organization access. Reported for IAM › Customers › Add customer with a blank Role. - - Accepting an invite with no role also granted it, and `joinOrganization` ignored the invite's role altogether. - - Now: - - A role is required when creating or inviting a user; without one the request returns 422. - - `Company::addUser`, `Company::assignUser` and `User::assignCompany` assign no role unless one is given. - - An invite without a role joins with no role. -- Only admins or holders of the Administrator role may grant the Administrator role (403 otherwise), on create, invite and role update. +- Every computed column is validated up front, including in grouped reports. Names must be safe identifiers, and `SELECT` is forbidden. +- Schema-declared columns always take their SQL from the registry, never from the request. Group keys, aggregate columns, sort columns and condition fields must be allowed or computed columns. +- Sort direction is normalised to `asc`/`desc`, and grouped reports validate `aggregateBy.computation`. -## Reliability +## Behaviour changes -- Cover the console guards for each account type, identity release and restore, and promotion through create, invite and invite acceptance. +- In a grouped report, a selected column that is neither a group key nor aggregated is now an error instead of being dropped. +- Invalid report shapes fail with a clear message instead of an SQL error. -A database migration is not required. No configuration change is needed. The FleetOps side ships in fleetbase/fleetops#338. +A database migration is not required. No configuration change is needed. The FleetOps order report schema ships in fleetbase/fleetops v0.6.70, and the report builder changes in fleetbase/ember-ui v0.4.4. -Changes: [#264](https://github.com/fleetbase/core-api/pull/264), [#266](https://github.com/fleetbase/core-api/pull/266), [#267](https://github.com/fleetbase/core-api/pull/267). +Changes: [#269](https://github.com/fleetbase/core-api/pull/269), [#270](https://github.com/fleetbase/core-api/pull/270). diff --git a/composer.json b/composer.json index e6434874..5ef971d4 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "fleetbase/core-api", - "version": "1.6.63", + "version": "1.6.64", "description": "Core Framework and Resources for Fleetbase API", "keywords": [ "fleetbase", diff --git a/src/Http/Controllers/Internal/v1/SettingController.php b/src/Http/Controllers/Internal/v1/SettingController.php index 2cc01cac..ac994e1c 100644 --- a/src/Http/Controllers/Internal/v1/SettingController.php +++ b/src/Http/Controllers/Internal/v1/SettingController.php @@ -21,6 +21,9 @@ use Fleetbase\Services\SmsService; use Fleetbase\Support\PlatformApi; use Fleetbase\Support\Utils; +use Fleetbase\Twilio\Manager as TwilioManager; +use Fleetbase\Twilio\Support\Laravel\Facade as TwilioFacade; +use Fleetbase\Twilio\TwilioInterface; use Illuminate\Http\Request; use Illuminate\Notifications\AnonymousNotifiable; use Illuminate\Support\Arr; @@ -594,6 +597,8 @@ public function testSmsProviderConfig(AdminRequest $request) } catch (\Throwable $e) { $responseMessage = $e->getMessage(); $status = 'error'; + } finally { + $this->releaseTwilioClient(); } return response()->json([ @@ -843,12 +848,13 @@ public function testTwilioConfig(AdminRequest $request) // Set config from request config(['twilio.twilio.connections.twilio.sid' => $sid, 'twilio.twilio.connections.twilio.token' => $token, 'twilio.twilio.connections.twilio.from' => $from]); + $this->refreshTwilioClient(); $message = 'Twilio configuration is successful, SMS sent to ' . $phone . '.'; $status = 'success'; try { - \Fleetbase\Twilio\Support\Laravel\Facade::message($phone, 'This is a Twilio test from Fleetbase'); + TwilioFacade::message($phone, 'This is a Twilio test from Fleetbase'); } catch (\Twilio\Exceptions\RestException $e) { $message = $e->getMessage(); $status = 'error'; @@ -861,6 +867,8 @@ public function testTwilioConfig(AdminRequest $request) } catch (\Error $e) { $message = $e->getMessage(); $status = 'error'; + } finally { + $this->releaseTwilioClient(); } return response()->json(['status' => $status, 'message' => $message]); @@ -899,6 +907,7 @@ protected function setTemporarySmsProviderConfig(string $provider, array $provid 'services.twilio' => array_replace_recursive(config('services.twilio', []), $providerConfig), 'twilio.twilio.connections.twilio' => array_replace_recursive(config('twilio.twilio.connections.twilio', []), $providerConfig), ]); + $this->refreshTwilioClient(); } if ($provider === SmsService::PROVIDER_CALLPRO) { @@ -908,6 +917,49 @@ protected function setTemporarySmsProviderConfig(string $provider, array $provid } } + /** + * Rebuild the Twilio client from the config just applied. + * + * The Twilio manager copies its connection settings when it is built, and both the + * container singleton and the facade's static cache keep the built manager. Under + * Octane the facade cache outlives the request, so a test send kept using the + * credentials the worker first saw: it failed with "Credentials are required to create + * a Client" when none were saved, or quietly used the saved ones instead of those just + * entered. A stand-in bound in place of the real manager is left alone. + */ + protected function refreshTwilioClient(): void + { + TwilioFacade::clearResolvedInstance('twilio'); + + if (!app()->bound('twilio')) { + return; + } + + $current = app()->resolved('twilio') ? app('twilio') : null; + if ($current !== null && !($current instanceof TwilioManager)) { + return; + } + + // Build it here from this request's config: the provider's singleton closure reads + // the config of the application it was registered on, which under Octane is the + // worker's base application, not the copy this request just changed. + $manager = $current ? get_class($current) : TwilioManager::class; + $config = config('twilio.twilio', []); + + app()->instance('twilio', new $manager($config['default'] ?? 'twilio', $config['connections'] ?? [])); + app()->forgetInstance(TwilioInterface::class); + } + + /** + * Forget the facade's cached Twilio client once a test send is done, so the credentials + * under test don't outlive this request in a long-running worker and get used for real + * messages (verification codes, notifications) sent by later requests. + */ + protected function releaseTwilioClient(): void + { + TwilioFacade::clearResolvedInstance('twilio'); + } + /** * Sends a test exception to Sentry. * diff --git a/src/Support/Reporting/ComputedColumnValidator.php b/src/Support/Reporting/ComputedColumnValidator.php index f4ca9921..eb278aa2 100644 --- a/src/Support/Reporting/ComputedColumnValidator.php +++ b/src/Support/Reporting/ComputedColumnValidator.php @@ -6,6 +6,28 @@ class ComputedColumnValidator { + /** + * Keywords that are never column references. + */ + public const SQL_KEYWORDS = [ + 'INTERVAL', 'AND', 'OR', 'XOR', 'NOT', 'IS', 'NULL', 'TRUE', 'FALSE', + 'AS', 'FROM', 'WHERE', 'DIV', 'CASE', 'WHEN', 'THEN', 'ELSE', 'END', + 'DISTINCT', 'IN', 'LIKE', 'BETWEEN', 'ESCAPE', 'REGEXP', + 'ORDER', 'BY', 'ASC', 'DESC', 'SEPARATOR', + ]; + + /** + * Cast types and interval units. These are keywords after `AS` / `INTERVAL ` (e.g. + * `CAST(x AS SIGNED)`, `INTERVAL 7 DAY`) but may also be real column names (e.g. `time`). + */ + public const CONTEXTUAL_KEYWORDS = [ + // CAST / CONVERT target types + 'DECIMAL', 'SIGNED', 'UNSIGNED', 'INTEGER', 'INT', 'CHAR', 'NCHAR', 'BINARY', + 'DATE', 'DATETIME', 'TIME', 'DOUBLE', 'FLOAT', 'REAL', 'JSON', + // INTERVAL units + 'MICROSECOND', 'SECOND', 'MINUTE', 'HOUR', 'DAY', 'WEEK', 'MONTH', 'QUARTER', 'YEAR', + ]; + /** * Allowed SQL functions. */ @@ -25,6 +47,8 @@ class ComputedColumnValidator 'SECOND', 'DATE_FORMAT', 'LAST_DAY', // Get last day of month + 'DATE', // Date part of a datetime, e.g. DATE(created_at) + 'TIME', // Time part of a datetime 'DAYOFWEEK', // Get day of week (1=Sunday, 7=Saturday) 'DAYOFMONTH', // Get day of month (1-31) 'DAYOFYEAR', // Get day of year (1-366) @@ -118,6 +142,23 @@ class ComputedColumnValidator // Type Conversion 'CAST', 'CONVERT', + 'DECIMAL', // CAST(x AS DECIMAL(10,2)) + 'CHAR', // CAST(x AS CHAR(20)) + 'BINARY', + 'DOUBLE', + 'FLOAT', + + // JSON Functions, e.g. JSON_UNQUOTE(JSON_EXTRACT(meta, '$.total')) + 'JSON_EXTRACT', + 'JSON_UNQUOTE', + 'JSON_VALUE', + 'JSON_LENGTH', + 'JSON_CONTAINS', + 'JSON_CONTAINS_PATH', + 'JSON_KEYS', + 'JSON_TYPE', + 'JSON_VALID', + 'JSON_SEARCH', // Other Utility Functions 'INTERVAL', // For date arithmetic @@ -141,7 +182,7 @@ class ComputedColumnValidator 'ALTER', 'CREATE', 'GRANT', 'REVOKE', 'EXEC', 'EXECUTE', 'UNION', 'INTO', 'INFORMATION_SCHEMA', 'LOAD_FILE', 'OUTFILE', - 'DUMPFILE', 'BENCHMARK', 'SLEEP', + 'DUMPFILE', 'BENCHMARK', 'SLEEP', 'SELECT', ]; protected ReportSchemaRegistry $registry; @@ -221,7 +262,7 @@ protected function validateFunctions(string $expression): array $errors = []; // Match function calls: FUNCTION_NAME( - preg_match_all('/([A-Z_]+)\s*\(/i', $expression, $matches); + preg_match_all('/\b([A-Z_][A-Z0-9_]*)\s*\(/i', $this->removeStringLiterals($expression), $matches); if (!empty($matches[1])) { foreach ($matches[1] as $function) { @@ -342,10 +383,7 @@ protected function removeStringLiterals(string $expression): string */ protected function isKeywordOrLiteral(string $word): bool { - $keywords = array_merge($this->allowedFunctions, $this->allowedOperators, [ - 'TRUE', 'FALSE', 'NULL', 'AS', 'FROM', 'WHERE', - 'INTERVAL', 'DAY', 'MONTH', 'YEAR', 'HOUR', 'MINUTE', 'SECOND', - ]); + $keywords = array_merge($this->allowedFunctions, $this->allowedOperators, static::SQL_KEYWORDS, static::CONTEXTUAL_KEYWORDS); return in_array(strtoupper($word), $keywords) || is_numeric($word); } @@ -393,7 +431,7 @@ protected function isValidColumnReference(string $columnRef, Table $table): bool */ protected function columnExistsInTable(string $columnName, Table $table): bool { - $columns = $table->getColumns(); + $columns = $table->getAllColumns(); foreach ($columns as $column) { if ($column->getName() === $columnName) { diff --git a/src/Support/Reporting/ReportQueryConverter.php b/src/Support/Reporting/ReportQueryConverter.php index 7e215069..d84583ad 100644 --- a/src/Support/Reporting/ReportQueryConverter.php +++ b/src/Support/Reporting/ReportQueryConverter.php @@ -2,13 +2,23 @@ namespace Fleetbase\Support\Reporting; -use Fleetbase\Support\Utils; +use Fleetbase\Support\Reporting\Schema\Column; use Illuminate\Database\Query\Builder; use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; class ReportQueryConverter { + /** + * Aggregate functions a grouped report can apply. + */ + public const AGGREGATE_FUNCTIONS = ['count', 'count_distinct', 'sum', 'avg', 'min', 'max', 'group_concat']; + + /** + * How deep computed and expression columns may reference one another. + */ + protected const MAX_REFERENCE_DEPTH = 10; + protected ReportSchemaRegistry $registry; protected array $queryConfig; protected array $autoJoins = []; @@ -29,6 +39,9 @@ public function __construct(ReportSchemaRegistry $registry, array $queryConfig) /** * Extract computed columns from groupBy aggregates and add to computed_columns array. * This handles cases where the frontend sends computed column metadata in aggregateBy objects. + * + * Columns the schema declares (expression or summary columns) are left out: their SQL + * always comes from the registry, never from the client. */ protected function extractComputedColumnsFromAggregates(): void { @@ -57,10 +70,10 @@ protected function extractComputedColumnsFromAggregates(): void // Check if this is a computed column $isComputed = $aggregateBy['computed'] ?? false; - $computation = $aggregateBy['computation'] ?? null; + $computation = $aggregateBy['computation'] ?? $aggregateBy['expression'] ?? null; $name = $aggregateBy['name'] ?? null; - if ($isComputed && $computation && $name && !isset($existingComputedColumns[$name])) { + if ($isComputed && $computation && $name && !isset($existingComputedColumns[$name]) && !$this->findSchemaColumn($name)) { // Add to computed_columns array $this->queryConfig['computed_columns'][] = [ 'name' => $name, @@ -221,6 +234,11 @@ protected function buildQuery(): Builder // Always scope by company $this->applyCompanyScope($query); + // Leave out soft-deleted rows of the root table + if ($table->usesSoftDeletes()) { + $query->whereNull("{$tableName}.deleted_at"); + } + // Process auto-joins first (based on selected columns) $this->processAutoJoins($query, $tableName); @@ -286,27 +304,27 @@ protected function resolveCompanyUuid(): ?string */ protected function processAutoJoins(Builder $query, string $tableName): void { - $table = $this->registry->getTable($tableName); $autoJoinPaths = []; foreach ($this->queryConfig['columns'] ?? [] as $column) { if (!empty($column['auto_join_path'])) { // already provided by your registry as full path like "payload.pickup" $autoJoinPaths[] = $column['auto_join_path']; - } elseif (Str::contains($column['name'], '.')) { - // take all but the final segment as the relationship path - $parts = explode('.', $column['name']); - if (count($parts) >= 2) { - $relPath = implode('.', array_slice($parts, 0, -1)); - $autoJoinPaths[] = $relPath; - } } + + $this->collectJoinPathsForReference($column['name'], $autoJoinPaths); } $this->collectAutoJoinPathsFromConditions($this->queryConfig['conditions'] ?? [], $autoJoinPaths); $this->collectAutoJoinPathsFromGroupBy($this->queryConfig['groupBy'] ?? [], $autoJoinPaths); $this->collectAutoJoinPathsFromSortBy($this->queryConfig['sortBy'] ?? [], $autoJoinPaths); - $this->collectAutoJoinPathsFromComputedColumns($this->queryConfig['computed_columns'] ?? [], $autoJoinPaths, $tableName); + + // Every computed column is selected when the report is not grouped. A grouped report + // only uses the computed columns its group keys, aggregates, sorts and conditions + // reference, and those were collected above; joining the rest would multiply rows. + if (empty($this->queryConfig['groupBy'])) { + $this->collectAutoJoinPathsFromComputedColumns($this->queryConfig['computed_columns'] ?? [], $autoJoinPaths, $tableName); + } // dedupe and sort shortest->longest so parent joins first $autoJoinPaths = array_values(array_unique($autoJoinPaths)); @@ -327,11 +345,8 @@ protected function collectAutoJoinPathsFromConditions(array $conditions, array & $this->collectAutoJoinPathsFromConditions($condition['conditions'], $autoJoinPaths); } elseif (!empty($condition['field']['auto_join_path'])) { $autoJoinPaths[] = $condition['field']['auto_join_path']; - } elseif (!empty($condition['field']['name']) && Str::contains($condition['field']['name'], '.')) { - $parts = explode('.', $condition['field']['name']); - if (count($parts) >= 2) { - $autoJoinPaths[] = implode('.', array_slice($parts, 0, -1)); - } + } elseif (!empty($condition['field']['name'])) { + $this->collectJoinPathsForReference($condition['field']['name'], $autoJoinPaths); } } } @@ -339,17 +354,11 @@ protected function collectAutoJoinPathsFromConditions(array $conditions, array & protected function collectAutoJoinPathsFromGroupBy(array $groupBy, array &$autoJoinPaths): void { foreach ($groupBy as $g) { - if (!empty($g['groupBy']['name']) && str_contains($g['groupBy']['name'], '.')) { - $parts = explode('.', $g['groupBy']['name']); - if (count($parts) >= 2) { - $autoJoinPaths[] = implode('.', array_slice($parts, 0, -1)); - } + if (!empty($g['groupBy']['name'])) { + $this->collectJoinPathsForReference($g['groupBy']['name'], $autoJoinPaths); } - if (!empty($g['aggregateBy']['name']) && str_contains($g['aggregateBy']['name'], '.')) { - $parts = explode('.', $g['aggregateBy']['name']); - if (count($parts) >= 2) { - $autoJoinPaths[] = implode('.', array_slice($parts, 0, -1)); - } + if (!empty($g['aggregateBy']['name']) && $g['aggregateBy']['name'] !== '*') { + $this->collectJoinPathsForReference($g['aggregateBy']['name'], $autoJoinPaths); } } } @@ -357,11 +366,8 @@ protected function collectAutoJoinPathsFromGroupBy(array $groupBy, array &$autoJ protected function collectAutoJoinPathsFromSortBy(array $sortBy, array &$autoJoinPaths): void { foreach ($sortBy as $s) { - if (!empty($s['column']['name']) && str_contains($s['column']['name'], '.')) { - $parts = explode('.', $s['column']['name']); - if (count($parts) >= 2) { - $autoJoinPaths[] = implode('.', array_slice($parts, 0, -1)); - } + if (!empty($s['column']['name'])) { + $this->collectJoinPathsForReference($s['column']['name'], $autoJoinPaths); } } } @@ -374,14 +380,66 @@ protected function collectAutoJoinPathsFromComputedColumns(array $computedColumn continue; } - // Extract relationship paths from the expression - $paths = $this->extractRelationshipPathsFromExpression($expression, $rootTable); - foreach ($paths as $path) { - $autoJoinPaths[] = $path; + $this->collectJoinPathsFromExpression($expression, null, $autoJoinPaths); + } + } + + /** + * Collect the relationship paths a column reference needs joined. + * + * A computed column (from the query) or an expression/summary column (from the schema) + * contributes the joins its own expression needs, so `order_total` defined as + * `transaction.amount / 100` joins `transaction` even though its name has no dot. + */ + protected function collectJoinPathsForReference(string $reference, array &$autoJoinPaths, int $depth = 0): void + { + if ($depth > static::MAX_REFERENCE_DEPTH) { + return; + } + + $computed = $this->findQueryComputedColumn($reference); + if ($computed) { + $this->collectJoinPathsFromExpression($computed['expression'] ?? '', null, $autoJoinPaths, $depth + 1); + + return; + } + + $schemaColumn = $this->findSchemaColumn($reference); + if ($schemaColumn) { + [$column, $prefix] = $schemaColumn; + + if ($prefix !== null) { + $autoJoinPaths[] = $prefix; + } + + if ($column->isComputed() && $column->getComputation()) { + $this->collectJoinPathsFromExpression($column->getComputation(), $prefix, $autoJoinPaths, $depth + 1); } + + return; + } + + if (str_contains($reference, '.')) { + $autoJoinPaths[] = implode('.', array_slice(explode('.', $reference), 0, -1)); } } + /** + * Collect the relationship paths every column reference in an expression needs joined. + */ + protected function collectJoinPathsFromExpression(string $expression, ?string $prefix, array &$autoJoinPaths, int $depth = 0): void + { + $this->rewriteColumnReferences($expression, function (string $reference, bool $keywordPosition) use ($prefix, &$autoJoinPaths, $depth) { + $path = $prefix !== null ? "{$prefix}.{$reference}" : $reference; + + if (!$this->isSqlKeyword($reference, $keywordPosition, $path)) { + $this->collectJoinPathsForReference($path, $autoJoinPaths, $depth); + } + + return null; + }); + } + protected function applyAutoJoinPath(Builder $query, string $rootTable, string $fullPath): void { // Already joined? @@ -421,13 +479,23 @@ protected function applyAutoJoinPath(Builder $query, string $rootTable, string $ $joinType = $relationship->getType() ?: 'left'; // Join: {current}.{localKey} = {alias}.{foreignKey} - $query->join( - "{$relationship->getTable()} as {$alias}", - "{$currentTableOrAlias}.{$relationship->getLocalKey()}", - '=', - "{$alias}.{$relationship->getForeignKey()}", - $joinType - ); + $localColumn = "{$currentTableOrAlias}.{$relationship->getLocalKey()}"; + $foreignColumn = "{$alias}.{$relationship->getForeignKey()}"; + + if ($relationship->usesSoftDeletes()) { + // Constrain inside the ON clause so a LEFT join still keeps the parent row. + $query->join( + "{$relationship->getTable()} as {$alias}", + function ($join) use ($localColumn, $foreignColumn, $alias) { + $join->on($localColumn, '=', $foreignColumn)->whereNull("{$alias}.deleted_at"); + }, + null, + null, + $joinType + ); + } else { + $query->join("{$relationship->getTable()} as {$alias}", $localColumn, '=', $foreignColumn, $joinType); + } // record $this->autoJoins[] = [ @@ -629,17 +697,14 @@ function ($joinClause) use ($localColumn, $foreignRef, $alias, $scopeJoinCompany */ protected function buildSelectClause(Builder $query): void { - $rootTable = $this->queryConfig['table']['name']; $hasGrouping = !empty($this->queryConfig['groupBy']); if (!$hasGrouping) { - // existing behavior $selects = []; foreach ($this->queryConfig['columns'] ?? [] as $column) { - $name = $column['name']; - $alias = $column['alias'] ?? str_replace('.', '_', $name); - [$tblAlias, $col] = $this->resolveAliasAndColumn($rootTable, $name); - $selects[] = "{$tblAlias}.{$col} as `{$alias}`"; + $name = $column['name']; + $alias = $column['alias'] ?? str_replace('.', '_', $name); + $selects[] = $this->columnSql($name) . " as `{$alias}`"; } // Add computed columns @@ -652,59 +717,53 @@ protected function buildSelectClause(Builder $query): void return; } - $selects = []; + $selects = []; + $groupKeys = []; // Select grouped columns - $groupAliases = []; // track to validate orderBy later foreach ($this->queryConfig['groupBy'] as $g) { - $groupColName = $g['groupBy']['name']; - $alias = $g['groupBy']['alias'] ?? str_replace('.', '_', $groupColName); - [$tblAlias, $col] = $this->resolveAliasAndColumn($rootTable, $groupColName); - $selects[] = "{$tblAlias}.{$col} as `{$alias}`"; - $groupAliases[] = $alias; + $groupColName = $g['groupBy']['name']; + $alias = $g['groupBy']['alias'] ?? str_replace('.', '_', $groupColName); + + if (isset($groupKeys[$alias])) { + continue; + } + + $selects[] = $this->columnSql($groupColName) . " as `{$alias}`"; + $groupKeys[$alias] = true; + } + + // Summary columns (e.g. "Total Orders") aggregate on their own and sit beside the group keys. + foreach ($this->queryConfig['columns'] ?? [] as $column) { + $alias = $column['alias'] ?? str_replace('.', '_', $column['name']); + + if (!isset($groupKeys[$alias]) && $this->isAggregateColumn($column['name'])) { + $selects[] = $this->columnSql($column['name']) . " as `{$alias}`"; + $groupKeys[$alias] = true; + } } - // Add aggregates per groupBy rule (support count/sum/avg/min/max) + // Add aggregates per groupBy rule (support count/count_distinct/sum/avg/min/max/group_concat) foreach ($this->queryConfig['groupBy'] as $g) { $fn = strtolower($g['aggregateFn']['value'] ?? ''); if (!$fn) { continue; } - $by = $g['aggregateBy']['full'] ?? $g['aggregateBy']['name'] ?? '*'; + $by = $g['aggregateBy']['full'] ?? $g['aggregateBy']['name'] ?? '*'; + $aggAlias = $this->deriveAggregateAlias($g); - if ($by === '*' || $by === 'count') { - $expr = 'COUNT(*)'; - } else { - // Check if this is a computed column - $isComputed = false; - $computedExpression = null; - - foreach ($this->queryConfig['computed_columns'] ?? [] as $computedColumn) { - if ($computedColumn['name'] === $by) { - $isComputed = true; - $computedExpression = $computedColumn['expression'] ?? ''; - break; - } - } - - if ($isComputed && $computedExpression) { - // For computed columns, use the resolved expression - $resolvedExpression = $this->resolveComputedColumnReferences($computedExpression, $rootTable); - $expr = strtoupper($fn) . "({$resolvedExpression})"; - } else { - // For regular columns, use table.column format - [$tblAlias, $col] = $this->resolveAliasAndColumn($rootTable, $by); - $expr = strtoupper($fn) . "({$tblAlias}.{$col})"; - } + if (isset($groupKeys[$aggAlias])) { + continue; } - $aggAlias = $this->deriveAggregateAlias($g); - $selects[] = "{$expr} as `{$aggAlias}`"; + $selects[] = $this->aggregateExpression($fn, $by) . " as `{$aggAlias}`"; + $groupKeys[$aggAlias] = true; // decide a type/label $typeMap = [ 'count' => 'integer', + 'count_distinct' => 'integer', 'sum' => 'decimal', 'avg' => 'decimal', 'min' => 'string', // could be numeric/datetime; @@ -713,6 +772,7 @@ protected function buildSelectClause(Builder $query): void ]; $labelMap = [ 'count' => 'Count', + 'count_distinct' => 'Distinct Count', 'sum' => 'Sum', 'avg' => 'Average', 'min' => 'Min', @@ -721,26 +781,45 @@ protected function buildSelectClause(Builder $query): void ]; $this->emittedAggregates[] = [ - 'alias' => $aggAlias, - 'fn' => $fn, - 'by' => $by, - 'type' => $typeMap[$fn] ?? 'decimal', - 'label' => $labelMap[$fn] ?? strtoupper($fn), + 'alias' => $aggAlias, + 'fn' => $fn, + 'by' => $by, + 'by_label' => $by === '*' ? null : ($g['aggregateBy']['label'] ?? null), + 'type' => $typeMap[$fn] ?? 'decimal', + 'label' => $labelMap[$fn] ?? strtoupper($fn), ]; } - // (Optional) if user selected extra columns, drop them here OR auto-aggregate. - // We'll drop them to stay deterministic under ONLY_FULL_GROUP_BY. - - // Note: In grouped mode, we do NOT add computed columns as standalone SELECT items - // because they would violate ONLY_FULL_GROUP_BY. Computed columns are only used - // when explicitly referenced in aggregates (handled above in lines 580-600). + // Other selected columns are neither grouped nor aggregated; validateQueryConfig() + // rejects them so the query stays deterministic under ONLY_FULL_GROUP_BY. if ($selects) { $query->selectRaw(implode(', ', $selects)); } } + /** + * Build the SQL for one aggregate of a grouped report. + */ + protected function aggregateExpression(string $fn, string $by): string + { + if ($by === '*' || $by === 'count') { + return 'COUNT(*)'; + } + + if ($this->isAggregateColumn($by)) { + throw new \InvalidArgumentException("Column '{$by}' is already a summary value and cannot be aggregated again"); + } + + $sql = $this->columnSql($by); + + if ($fn === 'count_distinct') { + return "COUNT(DISTINCT {$sql})"; + } + + return strtoupper($fn) . "({$sql})"; + } + /** * Add foreign key columns to the query (for joins) without selecting them. */ @@ -791,14 +870,18 @@ protected function processConditions(Builder $query, array $conditions, string $ */ protected function applySingleCondition(Builder $query, array $condition, string $boolean = 'and'): void { - $field = $condition['field']['name']; + $fieldName = $condition['field']['name']; $operator = $condition['operator']['value']; - $value = $condition['value']; - $tableName = $this->queryConfig['table']['name']; + $value = $condition['value'] ?? null; - // Handle auto-join columns in conditions - [$tblAlias, $col] = $this->resolveAliasAndColumn($tableName, $field); - $field = "{$tblAlias}.{$col}"; + if ($this->isAggregateColumn($fieldName)) { + throw new \InvalidArgumentException("Column '{$fieldName}' is a summary value and cannot be used as a filter"); + } + + // Plain columns stay as identifiers (so the grammar quotes them); expression and + // computed columns are filtered on their SQL expression. + $sql = $this->columnSql($fieldName); + $field = $this->isExpressionReference($fieldName) ? DB::raw($sql) : $sql; // Apply the condition based on operator switch ($operator) { @@ -877,17 +960,13 @@ protected function buildGroupByClause(Builder $query): void return; } - $rootTable = $this->queryConfig['table']['name']; - $groupBy = []; + $groupBy = []; foreach ($this->queryConfig['groupBy'] as $g) { - [$tblAlias, $col] = $this->resolveAliasAndColumn($rootTable, $g['groupBy']['name']); - $groupBy[] = "{$tblAlias}.{$col}"; + $groupBy[] = $this->columnSql($g['groupBy']['name']); } - if ($groupBy) { - $query->groupBy($groupBy); - } + $query->groupByRaw(implode(', ', array_values(array_unique($groupBy)))); } /** @@ -899,51 +978,58 @@ protected function buildOrderByClause(Builder $query): void return; } - $rootTable = $this->queryConfig['table']['name']; - $hasGrouping = !empty($this->queryConfig['groupBy']); - - // Build a whitelist for grouped mode - $allowedOrderExprs = []; - if ($hasGrouping) { - // Grouped aliases (as emitted in buildSelectClause) - foreach ($this->queryConfig['groupBy'] as $g) { - $groupColName = $g['groupBy']['name']; - $alias = $g['groupBy']['alias'] ?? str_replace('.', '_', $groupColName); - $allowedOrderExprs["`{$alias}`"] = true; - } - // Aggregate aliases (as emitted in buildSelectClause) - foreach ($this->queryConfig['groupBy'] as $g) { - $fn = strtolower($g['aggregateFn']['value'] ?? ''); - if ($fn) { - $aggAliasBase = $g['aggregateBy']['name'] ?? $g['aggregateBy']['full'] ?? 'all'; - $aggAlias = ($fn === 'count') ? "count_{$aggAliasBase}" : "{$fn}_" . str_replace('.', '_', $aggAliasBase); - $allowedOrderExprs["`{$aggAlias}`"] = true; - } - } - } + $hasGrouping = !empty($this->queryConfig['groupBy']); + $allowedOrderExprs = $hasGrouping ? $this->groupedSelectAliases() : []; foreach ($this->queryConfig['sortBy'] as $s) { - $dir = $s['direction']['value'] ?? 'asc'; + $dir = strtolower((string) ($s['direction']['value'] ?? 'asc')) === 'desc' ? 'desc' : 'asc'; $colName = $s['column']['name']; if ($hasGrouping) { - // Try to order by a select alias first - $alias = $s['column']['alias'] ?? str_replace('.', '_', $colName); - $aliasExpr = "`{$alias}`"; - if (isset($allowedOrderExprs[$aliasExpr])) { - $query->orderByRaw("{$aliasExpr} {$dir}"); - continue; + // Only a group key or an aggregate can be ordered by in a grouped report. + $alias = $s['column']['alias'] ?? str_replace('.', '_', $colName); + if (isset($allowedOrderExprs[$alias])) { + $query->orderByRaw("`{$alias}` {$dir}"); } - // Not allowed → skip (or convert to MIN/MAX if you prefer) + + continue; + } + + if ($this->isExpressionReference($colName)) { + $query->orderByRaw($this->columnSql($colName) . " {$dir}"); continue; } - // Non-grouped mode → original behavior - [$tblAlias, $col] = $this->resolveAliasAndColumn($rootTable, $colName); + [$tblAlias, $col] = $this->resolveAliasAndColumn($this->queryConfig['table']['name'], $colName); $query->orderBy("{$tblAlias}.{$col}", $dir); } } + /** + * The select aliases of a grouped report: group keys, summary columns and aggregates. + */ + protected function groupedSelectAliases(): array + { + $aliases = []; + + foreach ($this->queryConfig['groupBy'] ?? [] as $g) { + $groupColName = $g['groupBy']['name']; + $aliases[$g['groupBy']['alias'] ?? str_replace('.', '_', $groupColName)] = true; + + if (!empty($g['aggregateFn']['value'])) { + $aliases[$this->deriveAggregateAlias($g)] = true; + } + } + + foreach ($this->queryConfig['columns'] ?? [] as $column) { + if ($this->isAggregateColumn($column['name'])) { + $aliases[$column['alias'] ?? str_replace('.', '_', $column['name'])] = true; + } + } + + return $aliases; + } + /** * Build the limit clause. */ @@ -1018,7 +1104,7 @@ protected function getSelectedColumns(): array $cols[] = [ 'name' => $agg['alias'], 'column_name' => $agg['alias'], - 'label' => $agg['label'] . ($agg['by'] === '*' ? '' : " ({$agg['by']})"), + 'label' => $agg['label'] . ($agg['by'] === '*' ? '' : ' (' . ($agg['by_label'] ?? $agg['by']) . ')'), 'type' => $agg['type'], 'auto_join_path' => null, ]; @@ -1046,10 +1132,6 @@ protected function deriveAggregateAlias(array $g): string $by = $g['aggregateBy']['name'] ?? $g['aggregateBy']['full'] ?? '*'; $base = ($by === '*' ? 'all' : str_replace('.', '_', $by)); - if ($fn === 'count') { - return "count_{$base}"; - } - return "{$fn}_{$base}"; } @@ -1092,154 +1174,251 @@ protected function buildComputedColumns(Builder $query, array &$selects): void return; } - $tableName = $this->queryConfig['table']['name']; - $validator = new ComputedColumnValidator($this->registry); - foreach ($this->queryConfig['computed_columns'] as $computedColumn) { - $name = $computedColumn['name'] ?? ''; - $expression = $computedColumn['expression'] ?? ''; + $name = $computedColumn['name'] ?? ''; - if (empty($name) || empty($expression)) { - throw new \InvalidArgumentException('Computed column must have both name and expression'); - } - - // Validate the expression, passing all computed columns so they can reference each other - $validationResult = $validator->validate($expression, $tableName, $this->queryConfig['computed_columns']); - if (!$validationResult['valid']) { - $errors = implode('; ', $validationResult['errors']); - throw new \InvalidArgumentException("Invalid computed column '{$name}': {$errors}"); - } + $this->validateComputedColumn($computedColumn); // Note: Auto-joins for computed column relationships are now created earlier in processAutoJoins() // This ensures joins exist before aggregate expressions are resolved // Resolve column references in the expression to use proper table aliases - $resolvedExpression = $this->resolveComputedColumnReferences($expression, $tableName); + $resolvedExpression = $this->resolveComputedColumnReferences($computedColumn['expression'], $this->queryConfig['table']['name']); // Add to selects $selects[] = "({$resolvedExpression}) as `{$name}`"; } } + /** + * Validate a computed column's name and expression. + */ + protected function validateComputedColumn(array $computedColumn): void + { + $name = $computedColumn['name'] ?? ''; + $expression = $computedColumn['expression'] ?? ''; + + if (empty($name) || empty($expression)) { + throw new \InvalidArgumentException('Computed column must have both name and expression'); + } + + // The name is interpolated raw as the select alias. + if (!$this->isSafeSqlIdentifier((string) $name)) { + throw new \InvalidArgumentException("Invalid computed column name '{$name}'"); + } + + // Validate the expression, passing all computed columns so they can reference each other + $validator = new ComputedColumnValidator($this->registry); + $validationResult = $validator->validate($expression, $this->queryConfig['table']['name'], $this->queryConfig['computed_columns'] ?? []); + if (!$validationResult['valid']) { + $errors = implode('; ', $validationResult['errors']); + throw new \InvalidArgumentException("Invalid computed column '{$name}': {$errors}"); + } + } + /** * Resolve column references in computed column expressions to use proper table aliases. */ protected function resolveComputedColumnReferences(string $expression, string $rootTable): string { - // Step 1: Recursively expand computed column references FIRST (before any protection) - $computedColumns = $this->queryConfig['computed_columns'] ?? []; - $computedColumnMap = []; - foreach ($computedColumns as $col) { - $computedColumnMap[$col['name']] = $col['expression']; - } - - $maxDepth = 10; // Prevent infinite recursion - $depth = 0; - while ($depth < $maxDepth) { - $changed = false; - foreach ($computedColumnMap as $name => $expr) { - if (preg_match('/\b' . preg_quote($name, '/') . '\b/', $expression)) { - $expression = preg_replace('/\b' . preg_quote($name, '/') . '\b/', '(' . $expr . ')', $expression); - $changed = true; - } - } - if (!$changed) { - break; + return $this->resolveExpression($expression, null); + } + + /** + * Resolve every column reference in an expression to SQL. + * + * With a `$prefix` (the relationship path that declares an expression column), bare + * column names resolve against that relationship's join alias instead of the root table. + */ + protected function resolveExpression(string $expression, ?string $prefix, int $depth = 0): string + { + return $this->rewriteColumnReferences($expression, function (string $reference, bool $keywordPosition) use ($prefix, $depth) { + $path = $prefix !== null ? "{$prefix}.{$reference}" : $reference; + + if ($this->isSqlKeyword($reference, $keywordPosition, $path)) { + return null; } - $depth++; + + return $this->columnSql($path, $depth + 1); + }); + } + + /** + * Resolve a column reference to the SQL that reads it. + * + * - a computed column from the query resolves to its (parenthesised) expression; + * - an expression or summary column declared in the schema resolves to its computation, + * with bare names read from the table or relationship that declares it; + * - anything else is a physical column on the root table or a joined relationship. + */ + protected function columnSql(string $reference, int $depth = 0): string + { + if ($depth > static::MAX_REFERENCE_DEPTH) { + throw new \InvalidArgumentException("Column reference '{$reference}' is circular or nested too deeply"); } - // Step 2: Now protect ALL string literals in the fully expanded expression - $stringLiterals = []; - $protectedExpression = preg_replace_callback( - "/'([^']*)'/", - function ($matches) use (&$stringLiterals) { - $placeholder = '___STRING_LITERAL_' . count($stringLiterals) . '___'; - $stringLiterals[$placeholder] = $matches[0]; // Keep the quotes + $computed = $this->findQueryComputedColumn($reference); + if ($computed) { + return '(' . $this->resolveExpression($computed['expression'] ?? '', null, $depth) . ')'; + } - return $placeholder; - }, - $expression - ); + $schemaColumn = $this->findSchemaColumn($reference); + if ($schemaColumn && $schemaColumn[0]->isComputed() && $schemaColumn[0]->getComputation()) { + return '(' . $this->resolveExpression($schemaColumn[0]->getComputation(), $schemaColumn[1], $depth) . ')'; + } - // Also protect double-quoted strings - $protectedExpression = preg_replace_callback( - '/"([^"]*)"/', - function ($matches) use (&$stringLiterals) { - $placeholder = '___STRING_LITERAL_' . count($stringLiterals) . '___'; - $stringLiterals[$placeholder] = $matches[0]; // Keep the quotes + [$tblAlias, $col] = $this->resolveAliasAndColumn($this->queryConfig['table']['name'], $reference); - return $placeholder; - }, - $protectedExpression - ); + return "{$tblAlias}.{$col}"; + } - // Step 3: Protect SQL function calls (word followed by opening parenthesis) - $sqlFunctions = []; - $protectedExpression = preg_replace_callback( - '/\b([A-Z_][A-Z0-9_]*)\s*\(/i', - function ($matches) use (&$sqlFunctions) { - $placeholder = '___SQL_FUNCTION_' . count($sqlFunctions) . '___('; - $sqlFunctions[$placeholder] = $matches[1] . '('; + /** + * Whether a reference resolves to an SQL expression rather than a physical column. + */ + protected function isExpressionReference(string $reference): bool + { + if ($this->findQueryComputedColumn($reference)) { + return true; + } - return $placeholder; - }, - $protectedExpression - ); + $schemaColumn = $this->findSchemaColumn($reference); - // Step 4: Now resolve column references in the protected expression - $resolvedExpression = preg_replace_callback( - '/\b([a-z_][a-z0-9_]*(?:\.[a-z_][a-z0-9_]*)*)\b/i', - function ($matches) use ($rootTable) { - $columnRef = $matches[1]; + return $schemaColumn !== null && $schemaColumn[0]->isComputed() && $schemaColumn[0]->getComputation() !== null; + } - // Skip SQL keywords (non-function keywords) - $keywords = [ - 'INTERVAL', 'AND', 'OR', 'NOT', 'IS', 'NULL', 'TRUE', 'FALSE', - 'AS', 'FROM', 'WHERE', 'DIV', 'CASE', 'WHEN', 'THEN', 'ELSE', 'END', - ]; + /** + * Whether a reference is a summary value that aggregates rows (e.g. `COUNT(id)`). + */ + protected function isAggregateColumn(string $reference): bool + { + $computed = $this->findQueryComputedColumn($reference); + if ($computed) { + return Column::isAggregateExpression($computed['expression'] ?? ''); + } - if (in_array(strtoupper($columnRef), $keywords) || is_numeric($columnRef)) { - return $columnRef; - } + $schemaColumn = $this->findSchemaColumn($reference); - // Skip string literal placeholders - if (strpos($columnRef, '___STRING_LITERAL_') === 0) { - return $columnRef; - } + return $schemaColumn !== null && $schemaColumn[0]->isAggregate(); + } - // Skip SQL function placeholders - if (strpos($columnRef, '___SQL_FUNCTION_') === 0) { - return $columnRef; - } + /** + * Find a computed column defined by the query. + */ + protected function findQueryComputedColumn(string $name): ?array + { + foreach ($this->queryConfig['computed_columns'] ?? [] as $computedColumn) { + if (($computedColumn['name'] ?? null) === $name) { + return $computedColumn; + } + } - // Try to resolve the column reference - try { - [$tblAlias, $col] = $this->resolveAliasAndColumn($rootTable, $columnRef); + return null; + } - return "{$tblAlias}.{$col}"; - // resolveAliasAndColumn() falls back instead of throwing for unknown references; this is defensive only. - // @codeCoverageIgnoreStart - } catch (\Exception $e) { - // If resolution fails, return as-is (might be a literal or string) - return $columnRef; - } - // @codeCoverageIgnoreEnd - }, - $protectedExpression - ); + /** + * Find a column the schema declares, on the root table or along an auto-join path. + * + * @return array{0: Column, 1: ?string}|null the column and the relationship path that declares it + */ + protected function findSchemaColumn(string $path): ?array + { + $table = $this->registry->getTable($this->queryConfig['table']['name'] ?? ''); + if (!$table) { + return null; + } + + $segments = explode('.', $path); + $name = array_pop($segments); + + if (!$segments) { + $column = $table->getColumn($name); + + return $column ? [$column, null] : null; + } - // Restore SQL functions - foreach ($sqlFunctions as $placeholder => $original) { - $resolvedExpression = str_replace($placeholder, $original, $resolvedExpression); + $context = $table; + foreach ($segments as $segment) { + $context = $this->getRelationshipFromContext($context, $segment); + if (!$context) { + return null; + } + } + + $column = $context->getColumn($name); + + return $column ? [$column, implode('.', $segments)] : null; + } + + /** + * Whether an identifier in an expression is an SQL keyword rather than a column. + * + * Cast types and interval units (DATE, TIME, YEAR, DAY, ...) are only keywords where + * they cannot be a column: right after `AS` or `INTERVAL `, or when no column of + * that name exists. + */ + protected function isSqlKeyword(string $identifier, bool $keywordPosition, string $path): bool + { + $upper = strtoupper($identifier); + + if (in_array($upper, ComputedColumnValidator::SQL_KEYWORDS, true)) { + return true; } - // Restore string literals - foreach ($stringLiterals as $placeholder => $original) { - $resolvedExpression = str_replace($placeholder, $original, $resolvedExpression); + if (!in_array($upper, ComputedColumnValidator::CONTEXTUAL_KEYWORDS, true)) { + return false; } - return $resolvedExpression; + if ($keywordPosition) { + return true; + } + + return !$this->findQueryComputedColumn($path) && !$this->findSchemaColumn($path); + } + + /** + * Rewrite each column reference in an SQL expression. + * + * String literals, function names and numbers are left alone. The callback receives the + * reference and whether it sits in a keyword position (after `AS` or `INTERVAL `), + * and returns its replacement, or null to keep it. + */ + protected function rewriteColumnReferences(string $expression, callable $rewrite): string + { + $placeholders = []; + $protect = function (string $text) use (&$placeholders): string { + $placeholder = '___PROTECTED_' . count($placeholders) . '___'; + $placeholders[$placeholder] = $text; + + return $placeholder; + }; + + // String literals first, so nothing inside them is mistaken for a column or function + $protected = (string) preg_replace_callback('/\'(?:[^\'\\\\]|\\\\.)*\'|"(?:[^"\\\\]|\\\\.)*"/s', fn ($m) => $protect($m[0]), $expression); + + // Then function names (an identifier followed by an opening parenthesis) + $protected = (string) preg_replace_callback('/\b([A-Z_][A-Z0-9_]*)(\s*\()/i', fn ($m) => $protect($m[1]) . $m[2], $protected); + + $rewritten = (string) preg_replace_callback( + '/\b([a-z_][a-z0-9_]*(?:\.[a-z_][a-z0-9_]*)*)\b/i', + function ($m) use ($rewrite, $protected) { + [$reference, $offset] = $m[1]; + + if (str_starts_with($reference, '___PROTECTED_')) { + return $reference; + } + + $before = substr($protected, 0, $offset); + $keywordPosition = (bool) preg_match('/\bAS\s+$/i', $before) || (bool) preg_match('/\bINTERVAL\s+\S+\s+$/i', $before); + + return $rewrite($reference, $keywordPosition) ?? $reference; + }, + $protected, + -1, + $count, + PREG_OFFSET_CAPTURE + ); + + return strtr($rewritten, $placeholders); } /** @@ -1270,7 +1449,7 @@ protected function validateQueryConfig(): void } // Validate columns - foreach ($this->queryConfig['columns'] as $column) { + foreach ($this->queryConfig['columns'] ?? [] as $column) { if (!$this->isConfiguredColumnAllowed($tableName, $column['name'])) { throw new \InvalidArgumentException("Column '{$column['name']}' is not allowed for table '{$tableName}'"); } @@ -1283,13 +1462,46 @@ protected function validateQueryConfig(): void } } + // Computed columns are validated up front: a grouped report resolves them inside + // aggregates and group keys without ever selecting them on their own. + foreach ($this->queryConfig['computed_columns'] ?? [] as $computedColumn) { + $this->validateComputedColumn($computedColumn); + } + foreach ($this->queryConfig['groupBy'] ?? [] as $g) { $groupAlias = $g['groupBy']['alias'] ?? null; if ($groupAlias !== null && !$this->isSafeSqlIdentifier((string) $groupAlias)) { throw new \InvalidArgumentException("Invalid group-by alias '{$groupAlias}'"); } + + $this->assertReferenceAllowed($g['groupBy']['name'] ?? '', 'Group by column'); + + if ($this->isAggregateColumn($g['groupBy']['name'])) { + throw new \InvalidArgumentException("Column '{$g['groupBy']['name']}' is a summary value and cannot be grouped by"); + } + + $fn = strtolower($g['aggregateFn']['value'] ?? ''); + if ($fn !== '' && !in_array($fn, static::AGGREGATE_FUNCTIONS, true)) { + throw new \InvalidArgumentException("Aggregate function '{$fn}' is not supported"); + } + + $by = $g['aggregateBy']['full'] ?? $g['aggregateBy']['name'] ?? '*'; + if ($fn !== '' && $by !== '*' && $by !== 'count') { + $this->assertReferenceAllowed($by, 'Aggregate column'); + } + } + + foreach ($this->queryConfig['sortBy'] ?? [] as $s) { + $sortColumn = $s['column']['name'] ?? ''; + if (empty($this->queryConfig['groupBy'])) { + $this->assertReferenceAllowed($sortColumn, 'Sort column'); + } elseif (!$this->isSafeSqlIdentifier((string) ($s['column']['alias'] ?? str_replace('.', '_', $sortColumn)))) { + throw new \InvalidArgumentException("Invalid sort column '{$sortColumn}'"); + } } + $this->validateConditionReferences($this->queryConfig['conditions'] ?? []); + // Validate manual joins: the join target must be a registered table, and every // identifier interpolated raw into the JOIN clause (table/alias/keys/localTable) // must be a safe SQL identifier. Without this, applyManualJoin() would splice @@ -1315,33 +1527,61 @@ protected function validateQueryConfig(): void $this->queryConfig['groupBy'] ); - foreach ($this->queryConfig['columns'] as $col) { - $isGrouped = in_array($col['name'], $groupCols, true); - $isComputed = !empty($col['computed']) && $col['computed'] === true; - if (!$isGrouped && !$isComputed) { + // A column picked only to be aggregated (e.g. the "distance" in SUM(distance)) is fine. + $aggregatedCols = []; + foreach ($this->queryConfig['groupBy'] as $g) { + if (!empty($g['aggregateFn']['value'])) { + $aggregatedCols[] = $g['aggregateBy']['name'] ?? null; + $aggregatedCols[] = $g['aggregateBy']['full'] ?? null; + } + } + + foreach ($this->queryConfig['columns'] ?? [] as $col) { + $isGrouped = in_array($col['name'], $groupCols, true); + $isAggregated = in_array($col['name'], $aggregatedCols, true) || $this->isAggregateColumn($col['name']); + if (!$isGrouped && !$isAggregated) { throw new \InvalidArgumentException("Column '{$col['name']}' must be grouped or aggregated when GROUP BY is used"); } } + } else { + // Without grouping, summary columns (e.g. "Total Orders") collapse the result to one + // row, so they cannot sit beside per-row columns. + $columns = $this->queryConfig['columns'] ?? []; + $summary = array_filter($columns, fn ($col) => $this->isAggregateColumn($col['name'])); + $perRow = array_filter($columns, fn ($col) => !$this->isAggregateColumn($col['name'])); + if ($summary && $perRow) { + $summaryNames = implode(', ', array_map(fn ($col) => $col['name'], $summary)); + throw new \InvalidArgumentException("Summary columns ({$summaryNames}) can only be combined with other columns when the report is grouped"); + } + } + } + + /** + * Assert that a referenced column is a computed column of the query or an allowed schema column. + */ + protected function assertReferenceAllowed(string $reference, string $context): void + { + if ($this->findQueryComputedColumn($reference) || $this->isConfiguredColumnAllowed($this->queryConfig['table']['name'], $reference)) { + return; } - // Autostrip non grouped columns (optional) - we can add this as an option later - // if (!empty($this->queryConfig['groupBy'])) { - // $groupCols = array_map( - // fn ($g) => $g['groupBy']['name'], - // $this->queryConfig['groupBy'] - // ); - - // // Keep only grouped or computed (aggregated) columns - // $this->queryConfig['columns'] = array_values(array_filter( - // $this->queryConfig['columns'], - // function ($col) use ($groupCols) { - // $isGrouped = in_array($col['name'], $groupCols, true); - // $isComputed = !empty($col['computed']); // e.g. COUNT(...), AVG(...), etc. - // return $isGrouped || $isComputed; - // } - // )); - // // (Optional) log/warn that some columns were dropped - // } + throw new \InvalidArgumentException("{$context} '{$reference}' is not allowed for table '{$this->queryConfig['table']['name']}'"); + } + + /** + * Assert that every condition field is an allowed reference. + */ + protected function validateConditionReferences(array $conditions): void + { + foreach ($conditions as $condition) { + if (isset($condition['conditions'])) { + $this->validateConditionReferences($condition['conditions']); + + continue; + } + + $this->assertReferenceAllowed($condition['field']['name'] ?? '', 'Condition column'); + } } /** @@ -1396,58 +1636,10 @@ protected function isConfiguredColumnAllowed(string $tableName, string $columnNa */ protected function extractRelationshipPathsFromExpression(string $expression, string $rootTable): array { - // First, expand any computed column references in the expression - $computedColumns = $this->queryConfig['computed_columns'] ?? []; - $computedColumnMap = []; - foreach ($computedColumns as $col) { - $computedColumnMap[$col['name']] = $col['expression']; - } - - // Recursively expand computed column references - $maxDepth = 10; - $depth = 0; - $expandedExpression = $expression; - while ($depth < $maxDepth) { - $changed = false; - foreach ($computedColumnMap as $name => $expr) { - if (preg_match('/\b' . preg_quote($name, '/') . '\b/', $expandedExpression)) { - $expandedExpression = preg_replace('/\b' . preg_quote($name, '/') . '\b/', '(' . $expr . ')', $expandedExpression); - $changed = true; - } - } - if (!$changed) { - break; - } - $depth++; - } - - // Now extract all column references that look like relationship paths - // We need to match patterns like: word.word.word (but not inside string literals) - - // First, remove string literals to avoid matching inside them - $cleanedExpression = preg_replace("/'[^']*'/", '', $expandedExpression); - $cleanedExpression = preg_replace('/"[^"]*"/', '', $cleanedExpression); - - // Match column references with dots (relationship paths) - preg_match_all('/\b([a-z_][a-z0-9_]*\.[a-z_][a-z0-9_]*(?:\.[a-z_][a-z0-9_]*)*)\b/i', $cleanedExpression, $matches); - - if (empty($matches[1])) { - return []; - } - - // Extract unique relationship paths (everything except the final column name) - $relationshipPaths = []; - foreach ($matches[1] as $columnPath) { - $parts = explode('.', $columnPath); - if (count($parts) >= 2) { - // Remove the last part (column name) to get the relationship path - array_pop($parts); - $relationshipPath = implode('.', $parts); - $relationshipPaths[$relationshipPath] = true; - } - } + $paths = []; + $this->collectJoinPathsFromExpression($expression, null, $paths); - return array_keys($relationshipPaths); + return array_values(array_unique($paths)); } /** diff --git a/src/Support/Reporting/ReportQueryValidator.php b/src/Support/Reporting/ReportQueryValidator.php index f33cda12..98be4653 100644 --- a/src/Support/Reporting/ReportQueryValidator.php +++ b/src/Support/Reporting/ReportQueryValidator.php @@ -149,7 +149,7 @@ protected function validateColumn(array $column, int $index, array $availableCol } // Check if column exists - if (!in_array($column['name'], $availableColumns)) { + if (!in_array($column['name'], $availableColumns) && !$this->registry->isColumnAllowed($tableName, $column['name'])) { $this->errors[] = "Column '{$column['name']}' does not exist in table '{$tableName}'"; } @@ -356,7 +356,7 @@ protected function validateGroupBy(array $queryConfig): void 'groupBy' => 'required|array', 'groupBy.name' => 'required|string', 'aggregateFn' => 'sometimes|array', - 'aggregateFn.value' => 'required_with:aggregateFn|string|in:count,sum,avg,min,max,group_concat', + 'aggregateFn.value' => 'required_with:aggregateFn|string|in:' . implode(',', ReportQueryConverter::AGGREGATE_FUNCTIONS), 'aggregateBy' => 'sometimes|array', 'aggregateBy.name' => 'required_with:aggregateBy|string', ]); @@ -413,11 +413,11 @@ protected function validateSortBy(array $queryConfig): void } } - // Validate sort field exists + // Validate sort field exists (a grouped report may also sort by an aggregate's alias) $sortField = $sortItem['column']['name']; $tableName = $sortItem['column']['table'] ?? $queryConfig['table']['name']; - if (!$this->isFieldAvailable($sortField, $tableName, $queryConfig)) { + if (!$this->isFieldAvailable($sortField, $tableName, $queryConfig) && !$this->isAggregateAlias($sortField, $queryConfig)) { $this->errors[] = "Sort By {$index}: Field '{$sortField}' is not available"; } } @@ -489,7 +489,13 @@ protected function isFieldAvailable(string $fieldName, string $tableName, array if ($tableName === $queryConfig['table']['name']) { $mainColumns = $this->registry->getTableColumns($tableName); $mainColumnNames = array_column($mainColumns, 'name'); - if (in_array($fieldName, $mainColumnNames)) { + if (in_array($fieldName, $mainColumnNames) || $this->registry->isColumnAllowed($tableName, $fieldName)) { + return true; + } + + // Computed columns defined by the query can be grouped, aggregated, sorted and filtered + $computedNames = array_column($queryConfig['computed_columns'] ?? [], 'name'); + if (in_array($fieldName, $computedNames, true)) { return true; } } @@ -510,6 +516,26 @@ protected function isFieldAvailable(string $fieldName, string $tableName, array return false; } + /** + * Whether a name is the select alias of one of the query's group-by aggregates. + */ + protected function isAggregateAlias(string $name, array $queryConfig): bool + { + foreach ($queryConfig['groupBy'] ?? [] as $groupItem) { + $fn = strtolower($groupItem['aggregateFn']['value'] ?? ''); + if ($fn === '') { + continue; + } + + $by = $groupItem['aggregateBy']['name'] ?? $groupItem['aggregateBy']['full'] ?? '*'; + if ($name === $fn . '_' . ($by === '*' ? 'all' : str_replace('.', '_', $by))) { + return true; + } + } + + return false; + } + /** * Determine if a requested join matches a relationship returned by the schema registry. */ diff --git a/src/Support/Reporting/ReportSchemaRegistry.php b/src/Support/Reporting/ReportSchemaRegistry.php index 24dfed16..3b3c693e 100644 --- a/src/Support/Reporting/ReportSchemaRegistry.php +++ b/src/Support/Reporting/ReportSchemaRegistry.php @@ -2,6 +2,7 @@ namespace Fleetbase\Support\Reporting; +use Fleetbase\Support\Reporting\Schema\Column; use Fleetbase\Support\Reporting\Schema\Table; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Str; @@ -167,18 +168,22 @@ private function flattenRelationshipColumns($relationship, string $path, array $ { $out = []; - $shortPrefix = $this->shortRelationshipLabel(end($labelTrail)); // "Pickup Location" → "Pickup" + $prefix = $this->shortRelationshipLabel(end($labelTrail)); // "Pickup Location" → "Pickup" // Columns directly on this relationship foreach ($relationship->getColumns() as $column) { + // Internal bookkeeping columns are never reportable + if (Column::isSystemColumnName($column->getName())) { + continue; + } + $arr = $column->toArray(); // Ensure the machine name carries the full path (so filters/queries work unambiguously) $arr['name'] = "{$path}.{$column->getName()}"; - // Human label with context - // e.g., "Pickup Street 1" or "Dropoff City" - $arr['label'] = trim($shortPrefix . ' ' . $arr['label']); + // Human label with context, e.g. "Pickup Street 1" or "Order Config Namespace" + $arr['label'] = $this->relationshipColumnLabel($prefix, (string) $arr['label']); // Useful metadata $arr['auto_join_path'] = $path; // e.g., "payload.pickup" @@ -199,34 +204,45 @@ private function flattenRelationshipColumns($relationship, string $path, array $ } /** - * Normalize relationship label into a short prefix for columns. + * Label a relationship column with its relationship, without repeating words. + * + * Examples: + * "Order Config" + "Namespace" → "Order Config Namespace" + * "Transaction" + "Transaction ID" → "Transaction ID" + * "Tracking" + "Tracking" → "Tracking" + */ + private function relationshipColumnLabel(string $prefix, string $label): string + { + if ($prefix === '' || $label === '') { + return trim($prefix . ' ' . $label); + } + + // The column label already names its relationship (e.g. "Transaction ID") + if (stripos($label . ' ', $prefix . ' ') === 0) { + return $label; + } + + return $prefix . ' ' . $label; + } + + /** + * Normalize relationship label into a prefix for its columns. * Examples: * "Pickup Location" → "Pickup" - * "Dropoff Location" → "Dropoff" + * "Order Config" → "Order Config" * "Order Payload" → "Payload". */ private function shortRelationshipLabel(string $label): string { // Remove common suffixes like "Location" - $label = preg_replace('/\s+Location$/i', '', $label); - - // If the label has multiple words, prefer the first ("Pickup Location" → "Pickup") - // But for "Order Payload" we prefer the last ("Payload") so nested pickup/dropoff can still prepend naturally - $parts = preg_split('/\s+/', trim($label)); - if (!$parts || count($parts) === 0) { - // @codeCoverageIgnoreStart - // preg_split() on a string returns at least one part unless the PCRE call fails. - return trim($label); - // @codeCoverageIgnoreEnd - } + $label = trim(preg_replace('/\s+Location$/i', '', $label)); - // Special case: if it contains "Payload", keep "Payload" + // Special case: payload columns read as "Payload ..." however the relationship is labelled if (stripos($label, 'payload') !== false) { return 'Payload'; } - // Default: first word - return $parts[0]; + return $label; } /** @@ -310,7 +326,7 @@ public function isColumnAllowed(string $tableName, string $columnPath): bool $finalCol = array_pop($segments); // e.g. "street1" $table = $this->getTable($tableName); - if (!$table) { + if (!$table || Column::isSystemColumnName($finalCol)) { return false; } diff --git a/src/Support/Reporting/Schema/Column.php b/src/Support/Reporting/Schema/Column.php index 735b1d6b..fde3b3d6 100644 --- a/src/Support/Reporting/Schema/Column.php +++ b/src/Support/Reporting/Schema/Column.php @@ -6,6 +6,24 @@ class Column { + /** + * Identifier columns that end in `_id` but are not foreign keys. + */ + public const IDENTIFIER_COLUMNS = ['public_id', 'internal_id']; + + /** + * Internal bookkeeping columns that are never reportable, whatever a schema declares. + */ + public const SYSTEM_COLUMNS = ['_key', '_import_id']; + + /** + * Whether a column name is internal bookkeeping that reports must never expose. + */ + public static function isSystemColumnName(string $name): bool + { + return in_array($name, static::SYSTEM_COLUMNS, true); + } + protected string $name; protected string $label; protected string $type; @@ -18,6 +36,7 @@ class Column protected bool $aggregatable = false; protected bool $hidden = false; protected bool $computed = false; + protected bool $aggregate = false; protected ?string $computation = null; protected ?\Closure $transformer = null; protected array $meta = []; @@ -40,17 +59,50 @@ public static function make(string $name, string $type = 'string'): self /** * Create a computed column. + * + * Whether the computation is an aggregate (COUNT/SUM/AVG/MIN/MAX/GROUP_CONCAT) is detected + * from the expression unless `$options['aggregate']` says otherwise. */ public static function computed(string $name, string $computation, string $type = 'string', array $options = []): self { return static::make($name, $type) ->setComputed(true) ->setComputation($computation) + ->setAggregate(isset($options['aggregate']) ? (bool) $options['aggregate'] : static::isAggregateExpression($computation)) ->setAggregatable(isset($options['aggregatable']) ? (bool) $options['aggregatable'] : false) ->setSortable(isset($options['sortable']) ? (bool) $options['sortable'] : false) ->setSearchable(isset($options['searchable']) ? (bool) $options['searchable'] : false); } + /** + * Create a row-level expression column, such as a value read out of a JSON column. + * + * Bare column names in the expression resolve against the table or relationship that + * declares the column, so `JSON_EXTRACT(meta, '$.total')` declared on a nested + * `payload.entities` relationship reads the joined entity's meta. Unlike an aggregate + * computed column, an expression column behaves like a regular column: it can be + * selected, filtered, sorted, grouped by and aggregated. + */ + public static function expression(string $name, string $expression, string $type = 'string'): self + { + $column = static::make($name, $type) + ->setComputed(true) + ->setComputation($expression) + ->setAggregate(false); + + $column->aggregatable = $column->determineAggregatable($type); + + return $column; + } + + /** + * Whether an SQL expression is an aggregate, i.e. it starts with an aggregate function. + */ + public static function isAggregateExpression(string $expression): bool + { + return (bool) preg_match('/^\s*\(?\s*(COUNT|SUM|AVG|MIN|MAX|GROUP_CONCAT)\s*\(/i', $expression); + } + /** * Create a count column. */ @@ -279,6 +331,22 @@ public function isComputed(): bool return $this->computed; } + /** + * Whether this is a computed column whose computation aggregates rows (COUNT, SUM, ...). + */ + public function isAggregate(): bool + { + return $this->computed && $this->aggregate; + } + + /** + * Whether this is a computed column that yields a value per row (an expression column). + */ + public function isExpression(): bool + { + return $this->computed && !$this->aggregate && $this->computation !== null; + } + public function getComputation(): ?string { return $this->computation; @@ -308,7 +376,22 @@ public function getMeta(?string $key = null) */ public function isForeignKey(): bool { - return Str::endsWith($this->name, '_uuid') || Str::endsWith($this->name, '_id'); + return static::isForeignKeyName($this->name); + } + + /** + * Whether a column name looks like a foreign key (`*_uuid` / `*_id`). + * + * A record's own identifiers (`public_id`, `internal_id`) end in `_id` but are not + * foreign keys; they are usually the first thing a report needs. + */ + public static function isForeignKeyName(string $name): bool + { + if (in_array($name, static::IDENTIFIER_COLUMNS, true)) { + return false; + } + + return Str::endsWith($name, '_uuid') || Str::endsWith($name, '_id'); } /** @@ -341,6 +424,7 @@ public function toArray(): array 'aggregatable' => $this->aggregatable, 'hidden' => $this->hidden, 'computed' => $this->computed, + 'aggregate' => $this->isAggregate(), 'computation' => $this->computation, 'transformer' => $this->hasTransformer(), 'meta' => $this->meta, @@ -370,6 +454,13 @@ protected function setComputation(string $computation): self return $this; } + protected function setAggregate(bool $aggregate): self + { + $this->aggregate = $aggregate; + + return $this; + } + protected function setAggregatable(bool $aggregatable): self { $this->aggregatable = $aggregatable; diff --git a/src/Support/Reporting/Schema/Relationship.php b/src/Support/Reporting/Schema/Relationship.php index 7f706b1f..bc50e5cd 100644 --- a/src/Support/Reporting/Schema/Relationship.php +++ b/src/Support/Reporting/Schema/Relationship.php @@ -14,6 +14,7 @@ class Relationship protected string $foreignKey; protected bool $enabled = true; protected bool $autoJoin = false; // Optional auto-join feature + protected bool $softDeletes = false; protected ?string $description = null; protected array $columns = []; protected array $nestedRelationships = []; @@ -141,6 +142,16 @@ public function autoJoin(bool $autoJoin = true): self return $this; } + /** + * Mark the related table as soft-deleting, so joins leave out rows whose `deleted_at` is set. + */ + public function softDeletes(bool $softDeletes = true): self + { + $this->softDeletes = $softDeletes; + + return $this; + } + /** * Add columns to the relationship. */ @@ -251,11 +262,30 @@ public function isAutoJoin(): bool return $this->autoJoin; } + public function usesSoftDeletes(): bool + { + return $this->softDeletes; + } + public function getColumns(): array { return $this->columns; } + /** + * Get a column declared directly on this relationship. + */ + public function getColumn(string $name): ?Column + { + foreach ($this->columns as $column) { + if ($column->getName() === $name) { + return $column; + } + } + + return null; + } + public function getNestedRelationships(): array { return $this->nestedRelationships; @@ -350,6 +380,7 @@ public function toArray(): array 'foreign_key' => $this->foreignKey, 'enabled' => $this->enabled, 'auto_join' => $this->autoJoin, + 'soft_deletes' => $this->softDeletes, 'description' => $this->description, 'columns' => array_map(fn ($column) => $column->toArray(), $this->columns), 'nested_relationships' => array_map(fn ($rel) => $rel->toArray(), $this->nestedRelationships), diff --git a/src/Support/Reporting/Schema/Table.php b/src/Support/Reporting/Schema/Table.php index 021ddee9..0704f28e 100644 --- a/src/Support/Reporting/Schema/Table.php +++ b/src/Support/Reporting/Schema/Table.php @@ -16,6 +16,7 @@ class Table protected array $relationships = []; protected array $excludedColumns = []; protected bool $supportsAggregates = true; + protected bool $softDeletes = false; protected ?int $maxRows = null; protected bool $cacheable = true; protected int $cacheTtl = 3600; @@ -175,6 +176,16 @@ public function supportsAggregates(bool $supports = true): self return $this; } + /** + * Mark the table as soft-deleting, so reports leave out rows whose `deleted_at` is set. + */ + public function softDeletes(bool $softDeletes = true): self + { + $this->softDeletes = $softDeletes; + + return $this; + } + /** * Set the maximum number of rows that can be returned. */ @@ -281,6 +292,11 @@ public function getSupportsAggregates(): bool return $this->supportsAggregates; } + public function usesSoftDeletes(): bool + { + return $this->softDeletes; + } + public function getMaxRows(): ?int { return $this->maxRows; @@ -318,6 +334,7 @@ public function getVisibleColumns(): array return array_filter($this->getAllColumns(), function ($column) { return !$column->isHidden() && !in_array($column->getName(), $this->excludedColumns) + && !Column::isSystemColumnName($column->getName()) && !$this->isForeignKeyColumn($column->getName()); }); } @@ -398,7 +415,8 @@ public function isColumnAllowed(string $name): bool } return !$column->isHidden() - && !in_array($name, $this->excludedColumns); + && !in_array($name, $this->excludedColumns) + && !Column::isSystemColumnName($name); } /** @@ -439,6 +457,7 @@ public function toArray(): array 'manual_join_relationships' => array_map(fn ($rel) => $rel->toArray(), $this->getManualJoinRelationships()), 'excluded_columns' => $this->excludedColumns, 'supports_aggregates' => $this->supportsAggregates, + 'soft_deletes' => $this->softDeletes, 'max_rows' => $this->maxRows, 'cacheable' => $this->cacheable, 'cache_ttl' => $this->cacheTtl, @@ -452,7 +471,7 @@ public function toArray(): array */ protected function isForeignKeyColumn(string $name): bool { - return Str::endsWith($name, '_uuid') || Str::endsWith($name, '_id'); + return Column::isForeignKeyName($name); } /** diff --git a/tests/Unit/Http/SettingControllerExternalProbesTest.php b/tests/Unit/Http/SettingControllerExternalProbesTest.php index daedf6aa..4f6c04ee 100644 --- a/tests/Unit/Http/SettingControllerExternalProbesTest.php +++ b/tests/Unit/Http/SettingControllerExternalProbesTest.php @@ -47,7 +47,9 @@ function setting_controller_external_probe_request(array $input = []): AdminRequ } afterEach(function () { - app()->forgetInstance('twilio'); + // Unbind rather than just forget the instance: a binding left behind by these tests + // would satisfy later files that expect twilio to be unbound. + app()->offsetUnset('twilio'); Facade::clearResolvedInstances(); }); @@ -211,3 +213,108 @@ function setting_controller_external_probe_request(array $input = []): AdminRequ ]) ->and(config('sentry.dsn'))->toBeNull(); }); + +/** + * A real Twilio manager that records which credentials a send would use instead of + * calling Twilio. + */ +class SettingControllerRecordingTwilioManager extends Fleetbase\Twilio\Manager +{ + public static array $sent = []; + + public function message(string $to, string $message, array $mediaUrls = [], array $params = []): Twilio\Rest\Api\V2010\Account\MessageInstance + { + $connection = (new ReflectionProperty(Fleetbase\Twilio\Manager::class, 'settings'))->getValue($this)['twilio']; + static::$sent[] = ['to' => $to, 'sid' => $connection['sid'], 'token' => $connection['token'], 'from' => $connection['from']]; + + throw new RuntimeException('recorded'); + } +} + +function setting_controller_bind_recording_twilio(): void +{ + SettingControllerRecordingTwilioManager::$sent = []; + + // Bound the way the Twilio service provider binds the real manager. Under Octane its + // closure reads the config of the worker's base application, not the request's copy, + // so it is modelled here with the config captured when the binding was registered. + $bootConfig = config('twilio.twilio'); + app()->singleton('twilio', fn () => new SettingControllerRecordingTwilioManager($bootConfig['default'] ?? 'twilio', $bootConfig['connections'])); + + // Resolved before the test runs with the saved credentials, as happens at boot or in an + // earlier request handled by the same Octane worker. + app('twilio'); + Fleetbase\Twilio\Support\Laravel\Facade::getFacadeRoot(); +} + +function setting_controller_twilio_facade_is_cached(): bool +{ + $resolved = (new ReflectionProperty(Facade::class, 'resolvedInstance'))->getValue(); + + return isset($resolved['twilio']); +} + +test('test twilio config sends with the credentials entered, not those the client was built with', function () { + setting_controller_external_probe_fixtures(['twilio.twilio.default' => 'twilio']); + setting_controller_bind_recording_twilio(); + + $response = (new SettingController())->testTwilioConfig(setting_controller_external_probe_request([ + 'sid' => 'entered-sid', + 'token' => 'entered-token', + 'from' => '+15555550999', + 'phone' => '+15555550123', + ])); + + expect($response->getData(true)['message'])->toBe('recorded') + ->and(SettingControllerRecordingTwilioManager::$sent)->toBe([ + ['to' => '+15555550123', 'sid' => 'entered-sid', 'token' => 'entered-token', 'from' => '+15555550999'], + ]) + ->and(setting_controller_twilio_facade_is_cached())->toBeFalse('the credentials under test do not outlive the request'); +}); + +test('test sms provider config sends through twilio with the credentials entered', function () { + setting_controller_external_probe_fixtures(['twilio.twilio.default' => 'twilio']); + setting_controller_bind_recording_twilio(); + + $response = (new SettingController())->testSmsProviderConfig(setting_controller_external_probe_request([ + 'provider' => 'twilio', + 'phone' => '+15555550123', + 'config' => ['sid' => 'entered-sid', 'token' => 'entered-token', 'from' => '+15555550999'], + ])); + + expect($response->getData(true))->toMatchArray(['status' => 'error', 'message' => 'recorded']) + ->and(SettingControllerRecordingTwilioManager::$sent)->toBe([ + ['to' => '+15555550123', 'sid' => 'entered-sid', 'token' => 'entered-token', 'from' => '+15555550999'], + ]) + ->and(setting_controller_twilio_facade_is_cached())->toBeFalse(); +}); + +test('a stand-in bound in place of the twilio manager is left in place', function () { + setting_controller_external_probe_fixtures(); + $twilio = new SettingControllerTwilioFake(); + app()->instance('twilio', $twilio); + + (new SettingController())->testTwilioConfig(setting_controller_external_probe_request([ + 'sid' => 'entered-sid', + 'token' => 'entered-token', + 'from' => '+15555550999', + 'phone' => '+15555550123', + ])); + + expect(app('twilio'))->toBe($twilio) + ->and($twilio->messages)->toBe([['+15555550123', 'This is a Twilio test from Fleetbase']]); +}); + +test('a twilio manager bound but not yet built is built from the config just applied', function () { + setting_controller_external_probe_fixtures(['twilio.twilio.default' => 'twilio']); + $bootConfig = config('twilio.twilio'); + app()->singleton('twilio', fn () => new SettingControllerRecordingTwilioManager($bootConfig['default'], $bootConfig['connections'])); + config(['twilio.twilio.connections.twilio.sid' => 'entered-sid']); + + $refresh = new ReflectionMethod(SettingController::class, 'refreshTwilioClient'); + $refresh->invoke(new SettingController()); + + $manager = app('twilio'); + expect(get_class($manager))->toBe(Fleetbase\Twilio\Manager::class) + ->and((new ReflectionProperty(Fleetbase\Twilio\Manager::class, 'settings'))->getValue($manager)['twilio']['sid'])->toBe('entered-sid'); +}); diff --git a/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php b/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php new file mode 100644 index 00000000..654ed3fd --- /dev/null +++ b/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php @@ -0,0 +1,575 @@ + payload -> entities (the line + * items), totals kept in JSON meta, soft-deleted rows, and a second tenant that must never leak. + */ + +function order_reporting_registry(): ReportSchemaRegistry +{ + $registry = new ReportSchemaRegistry(); + $registry->setCacheEnabled(false); + + $registry->registerTable( + Table::make('orders') + ->softDeletes() + ->excludeColumns(['uuid', 'deleted_at']) + ->columns([ + Column::make('id', 'integer'), + Column::make('public_id'), + Column::make('internal_id'), + Column::make('payload_uuid'), + Column::make('status'), + Column::make('type'), + Column::make('time', 'integer'), + Column::make('meta', 'json'), + Column::make('created_at', 'datetime'), + Column::expression('order_total', "CAST(JSON_UNQUOTE(JSON_EXTRACT(meta, '$.total')) AS DECIMAL(15,2)) / 100.0", 'decimal'), + ]) + ->computedColumns([ + Column::count('total_orders', 'id'), + Column::sum('sum_order_total', 'order_total'), + ]) + ->relationships([ + Relationship::hasAutoJoin('payload', 'payloads') + ->softDeletes() + ->localKey('payload_uuid') + ->foreignKey('uuid') + ->columns([Column::make('public_id')]) + ->with([ + Relationship::hasAutoJoin('entities', 'entities') + ->softDeletes() + ->localKey('uuid') + ->foreignKey('payload_uuid') + ->columns([ + Column::make('name'), + Column::make('price', 'decimal'), + Column::make('meta', 'json'), + Column::expression('quantity', "CAST(JSON_UNQUOTE(JSON_EXTRACT(meta, '$.quantity')) AS DECIMAL(15,2))", 'decimal'), + Column::expression('line_total', 'price * quantity', 'decimal'), + ]), + ]), + Relationship::hasAutoJoin('tracking_number', 'tracking_numbers') + ->localKey('tracking_number_uuid') + ->foreignKey('uuid') + ->columns([Column::make('tracking_number')]), + ]) + ); + + return $registry; +} + +function order_reporting_database(): void +{ + $connectionConfig = ['driver' => 'sqlite', 'database' => ':memory:', 'prefix' => '']; + $container = bind_test_container(); + + session()->flush(); + session(['company' => 'company-1']); + + $capsule = new Capsule($container); + $capsule->addConnection($connectionConfig, 'testing'); + $capsule->setEventDispatcher(new Dispatcher($container)); + $capsule->setAsGlobal(); + $capsule->bootEloquent(); + + config(['database.default' => 'testing', 'database.connections.testing' => $connectionConfig]); + + $databaseManager = $capsule->getDatabaseManager(); + $databaseManager->setDefaultConnection('testing'); + $container->instance('db', $databaseManager); + Facade::clearResolvedInstance('db'); + + $connection = $capsule->getConnection('testing'); + // SQLite has JSON_EXTRACT but not MySQL's JSON_UNQUOTE; its JSON_EXTRACT already unquotes. + $connection->getPdo()->sqliteCreateFunction('JSON_UNQUOTE', fn ($value) => $value, 1); + + $schema = $connection->getSchemaBuilder(); + $schema->create('orders', function ($table) { + $table->increments('id'); + $table->string('uuid'); + $table->string('public_id'); + $table->string('internal_id')->nullable(); + $table->string('company_uuid'); + $table->string('payload_uuid')->nullable(); + $table->string('tracking_number_uuid')->nullable(); + $table->string('status'); + $table->string('type')->nullable(); + $table->integer('time')->nullable(); + $table->json('meta')->nullable(); + $table->timestamp('created_at')->nullable(); + $table->timestamp('deleted_at')->nullable(); + }); + $schema->create('payloads', function ($table) { + $table->string('uuid'); + $table->string('public_id'); + $table->timestamp('deleted_at')->nullable(); + }); + $schema->create('entities', function ($table) { + $table->string('uuid'); + $table->string('payload_uuid'); + $table->string('name'); + $table->string('price')->nullable(); + $table->json('meta')->nullable(); + $table->timestamp('deleted_at')->nullable(); + }); + $schema->create('tracking_numbers', function ($table) { + $table->string('uuid'); + $table->string('tracking_number'); + }); + + $connection->table('tracking_numbers')->insert([ + ['uuid' => 'tn-1', 'tracking_number' => 'TRK-0001'], + ]); + $connection->table('payloads')->insert([ + ['uuid' => 'payload-1', 'public_id' => 'payload_1', 'deleted_at' => null], + ['uuid' => 'payload-2', 'public_id' => 'payload_2', 'deleted_at' => null], + ['uuid' => 'payload-3', 'public_id' => 'payload_3', 'deleted_at' => null], + ['uuid' => 'payload-4', 'public_id' => 'payload_4', 'deleted_at' => null], + ['uuid' => 'payload-5', 'public_id' => 'payload_5', 'deleted_at' => null], + ]); + $connection->table('entities')->insert([ + // September order 1: 3 burgers + 1 fries + ['uuid' => 'e-1', 'payload_uuid' => 'payload-1', 'name' => 'Burger', 'price' => '8.50', 'meta' => json_encode(['quantity' => 3]), 'deleted_at' => null], + ['uuid' => 'e-2', 'payload_uuid' => 'payload-1', 'name' => 'Fries', 'price' => '3.00', 'meta' => json_encode(['quantity' => 1]), 'deleted_at' => null], + // September order 2: 4 fries, 1 burger, and a removed line that must not count + ['uuid' => 'e-3', 'payload_uuid' => 'payload-2', 'name' => 'Fries', 'price' => '3.00', 'meta' => json_encode(['quantity' => 4]), 'deleted_at' => null], + ['uuid' => 'e-4', 'payload_uuid' => 'payload-2', 'name' => 'Burger', 'price' => '8.50', 'meta' => json_encode(['quantity' => 1]), 'deleted_at' => null], + ['uuid' => 'e-5', 'payload_uuid' => 'payload-2', 'name' => 'Milkshake', 'price' => '5.00', 'meta' => json_encode(['quantity' => 50]), 'deleted_at' => '2026-09-10 00:00:00'], + // August order + ['uuid' => 'e-6', 'payload_uuid' => 'payload-3', 'name' => 'Milkshake', 'price' => '5.00', 'meta' => json_encode(['quantity' => 9]), 'deleted_at' => null], + // Deleted order and another tenant's order + ['uuid' => 'e-7', 'payload_uuid' => 'payload-4', 'name' => 'Burger', 'price' => '8.50', 'meta' => json_encode(['quantity' => 100]), 'deleted_at' => null], + ['uuid' => 'e-8', 'payload_uuid' => 'payload-5', 'name' => 'Fries', 'price' => '3.00', 'meta' => json_encode(['quantity' => 100]), 'deleted_at' => null], + ]); + $connection->table('orders')->insert([ + ['uuid' => 'order-1', 'public_id' => 'order_1', 'internal_id' => 'INT-1', 'company_uuid' => 'company-1', 'payload_uuid' => 'payload-1', 'tracking_number_uuid' => 'tn-1', 'status' => 'completed', 'type' => 'storefront', 'time' => 30, 'meta' => json_encode(['total' => '2850']), 'created_at' => '2026-09-03 10:00:00', 'deleted_at' => null], + ['uuid' => 'order-2', 'public_id' => 'order_2', 'internal_id' => 'INT-2', 'company_uuid' => 'company-1', 'payload_uuid' => 'payload-2', 'tracking_number_uuid' => null, 'status' => 'created', 'type' => 'storefront', 'time' => 45, 'meta' => json_encode(['total' => '2050']), 'created_at' => '2026-09-20 18:30:00', 'deleted_at' => null], + ['uuid' => 'order-3', 'public_id' => 'order_3', 'internal_id' => 'INT-3', 'company_uuid' => 'company-1', 'payload_uuid' => 'payload-3', 'tracking_number_uuid' => null, 'status' => 'completed', 'type' => 'storefront', 'time' => 20, 'meta' => json_encode(['total' => '4500']), 'created_at' => '2026-08-15 09:00:00', 'deleted_at' => null], + ['uuid' => 'order-4', 'public_id' => 'order_4', 'internal_id' => 'INT-4', 'company_uuid' => 'company-1', 'payload_uuid' => 'payload-4', 'tracking_number_uuid' => null, 'status' => 'canceled', 'type' => 'storefront', 'time' => 10, 'meta' => json_encode(['total' => '85000']), 'created_at' => '2026-09-05 12:00:00', 'deleted_at' => '2026-09-06 00:00:00'], + ['uuid' => 'order-5', 'public_id' => 'order_5', 'internal_id' => 'INT-5', 'company_uuid' => 'company-2', 'payload_uuid' => 'payload-5', 'tracking_number_uuid' => null, 'status' => 'completed', 'type' => 'storefront', 'time' => 10, 'meta' => json_encode(['total' => '30000']), 'created_at' => '2026-09-07 12:00:00', 'deleted_at' => null], + ]); +} + +function order_reporting_run(array $config): array +{ + order_reporting_database(); + + return (new ReportQueryConverter(order_reporting_registry(), ['table' => ['name' => 'orders']] + $config))->execute(); +} + +function order_reporting_call(object $target, string $method, mixed ...$arguments): mixed +{ + $reflection = new ReflectionMethod($target, $method); + + return $reflection->invoke($target, ...$arguments); +} + +$septemberCondition = [ + 'field' => ['name' => 'created_at'], + 'operator' => ['value' => 'between'], + 'value' => ['2026-09-01 00:00:00', '2026-09-30 23:59:59'], // date-drift-ok: filters fixture rows by their own created_at, never compared to now() +]; + +test('order reporting ranks the products sold this month through payload entities', function () use ($septemberCondition) { + $result = order_reporting_run([ + 'columns' => [ + ['name' => 'payload.entities.name', 'label' => 'Item Name'], + ['name' => 'payload.entities.quantity', 'label' => 'Item Quantity'], + ], + 'conditions' => [$septemberCondition], + 'groupBy' => [[ + 'groupBy' => ['name' => 'payload.entities.name', 'label' => 'Item Name'], + 'aggregateFn' => ['value' => 'sum'], + 'aggregateBy' => ['name' => 'payload.entities.quantity', 'label' => 'Item Quantity', 'computed' => true, 'computation' => 'client supplied SQL is ignored'], + ]], + 'sortBy' => [[ + 'column' => ['name' => 'sum_payload_entities_quantity'], + 'direction' => ['value' => 'desc'], + ]], + ]); + + expect($result['success'])->toBeTrue() + // Fries 1 + 4, Burger 3 + 1; the removed milkshake line, the deleted order, the August + // order and the other tenant's order are all left out. + ->and(array_map(fn ($row) => [(array) $row][0], $result['data']))->toEqual([ + ['payload_entities_name' => 'Fries', 'sum_payload_entities_quantity' => 5], + ['payload_entities_name' => 'Burger', 'sum_payload_entities_quantity' => 4], + ]) + ->and($result['meta']['query_sql'])->toContain('orders_payload_entities.meta') + ->and($result['meta']['query_sql'])->not->toContain('client supplied') + ->and(collect($result['columns'])->firstWhere('name', 'sum_payload_entities_quantity')['label'])->toBe('Sum (Item Quantity)'); +}); + +test('order reporting totals this month in a single summary row', function () use ($septemberCondition) { + $result = order_reporting_run([ + 'columns' => [ + ['name' => 'total_orders', 'computed' => true], + ['name' => 'sum_order_total', 'computed' => true], + ], + 'conditions' => [$septemberCondition], + ]); + + expect($result['success'])->toBeTrue() + ->and($result['data'])->toHaveCount(1) + ->and((array) $result['data'][0])->toEqual(['total_orders' => 2, 'sum_order_total' => 49]); +}); + +test('order reporting lists orders with identifiers tracking numbers and json totals', function () { + $result = order_reporting_run([ + 'columns' => [ + ['name' => 'public_id'], + ['name' => 'internal_id'], + ['name' => 'tracking_number.tracking_number'], + ['name' => 'order_total'], + ], + 'computed_columns' => [[ + 'name' => 'total_with_tax', + 'expression' => "ROUND(CAST(JSON_UNQUOTE(JSON_EXTRACT(meta, '$.total')) AS DECIMAL(15,2)) / 100.0 * 1.1, 2)", + ]], + 'conditions' => [[ + 'field' => ['name' => 'order_total'], + 'operator' => ['value' => 'gt'], + 'value' => 25, + ]], + 'sortBy' => [[ + 'column' => ['name' => 'total_with_tax'], + 'direction' => ['value' => 'DESC'], + ]], + ]); + + expect($result['success'])->toBeTrue() + ->and(array_map(fn ($row) => (array) $row, $result['data']))->toEqual([ + ['public_id' => 'order_3', 'internal_id' => 'INT-3', 'tracking_number_tracking_number' => null, 'order_total' => 45, 'total_with_tax' => 49.5], + ['public_id' => 'order_1', 'internal_id' => 'INT-1', 'tracking_number_tracking_number' => 'TRK-0001', 'order_total' => 28.5, 'total_with_tax' => 31.35], + ]); +}); + +test('order reporting groups by a computed month bucket and counts distinct orders across line items', function () { + $result = order_reporting_run([ + 'columns' => [['name' => 'payload.entities.name']], + 'computed_columns' => [ + ['name' => 'order_month', 'expression' => 'SUBSTR(created_at, 1, 7)'], + ], + 'groupBy' => [ + [ + 'groupBy' => ['name' => 'order_month', 'computed' => true], + 'aggregateFn' => ['value' => 'count_distinct'], + 'aggregateBy' => ['name' => 'public_id'], + ], + [ + 'groupBy' => ['name' => 'order_month', 'computed' => true], + 'aggregateFn' => ['value' => 'count'], + 'aggregateBy' => ['name' => 'payload.entities.name'], + ], + [ + 'groupBy' => ['name' => 'order_month', 'computed' => true], + 'aggregateFn' => ['value' => 'sum'], + 'aggregateBy' => ['name' => 'payload.entities.line_total'], + ], + ], + 'sortBy' => [[ + 'column' => ['name' => 'order_month'], + 'direction' => ['value' => 'asc'], + ]], + ]); + + expect($result['success'])->toBeTrue() + ->and(array_map(fn ($row) => (array) $row, $result['data']))->toEqual([ + ['order_month' => '2026-08', 'count_distinct_public_id' => 1, 'count_payload_entities_name' => 1, 'sum_payload_entities_line_total' => 45], + ['order_month' => '2026-09', 'count_distinct_public_id' => 2, 'count_payload_entities_name' => 4, 'sum_payload_entities_line_total' => 49], + ]) + ->and($result['meta']['query_sql'])->toContain('COUNT(DISTINCT orders.public_id)') + ->and(collect($result['columns'])->firstWhere('name', 'count_distinct_public_id')['label'])->toBe('Distinct Count (public_id)'); +}); + +test('order reporting keeps summary columns beside group keys and sorts by them', function () { + $result = order_reporting_run([ + 'columns' => [ + ['name' => 'status'], + ['name' => 'total_orders'], + ], + 'groupBy' => [[ + 'groupBy' => ['name' => 'status'], + 'aggregateFn' => ['value' => 'sum'], + 'aggregateBy' => ['name' => 'order_total'], + ]], + 'sortBy' => [ + ['column' => ['name' => 'total_orders'], 'direction' => ['value' => 'desc']], + ['column' => ['name' => 'not_selected'], 'direction' => ['value' => 'asc']], + ], + ]); + + expect($result['success'])->toBeTrue() + ->and(array_map(fn ($row) => (array) $row, $result['data']))->toEqual([ + ['status' => 'completed', 'total_orders' => 2, 'sum_order_total' => 73.5], + ['status' => 'created', 'total_orders' => 1, 'sum_order_total' => 20.5], + ]) + ->and($result['meta']['query_sql'])->not->toContain('not_selected'); +}); + +test('order reporting keeps an order whose line items were all removed on a left join', function () { + order_reporting_database(); + Capsule::table('entities')->where('payload_uuid', 'payload-3')->update(['deleted_at' => '2026-09-01 00:00:00']); + + $result = (new ReportQueryConverter(order_reporting_registry(), [ + 'table' => ['name' => 'orders'], + 'columns' => [['name' => 'public_id'], ['name' => 'payload.entities.name']], + 'sortBy' => [['column' => ['name' => 'public_id'], 'direction' => ['value' => 'asc']]], + ]))->execute(); + + expect($result['success'])->toBeTrue() + ->and(collect($result['data'])->where('public_id', 'order_3')->pluck('payload_entities_name')->all())->toBe([null]); +}); + +test('order reporting refuses report shapes that cannot produce valid sql', function (array $config, string $message) { + $result = order_reporting_run($config); + + expect($result['success'])->toBeFalse() + ->and($result['error'])->toContain($message); +})->with([ + 'summary beside per-row columns' => [ + ['columns' => [['name' => 'public_id'], ['name' => 'total_orders']]], + 'Summary columns (total_orders) can only be combined', + ], + 'aggregating a summary column' => [ + ['columns' => [['name' => 'status']], 'groupBy' => [['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'sum'], 'aggregateBy' => ['name' => 'total_orders']]]], + "Column 'total_orders' is already a summary value", + ], + 'grouping by a summary column' => [ + ['columns' => [['name' => 'total_orders']], 'groupBy' => [['groupBy' => ['name' => 'total_orders'], 'aggregateFn' => ['value' => 'count'], 'aggregateBy' => ['name' => '*']]]], + 'cannot be grouped by', + ], + 'filtering on a summary column' => [ + ['columns' => [['name' => 'total_orders']], 'conditions' => [['field' => ['name' => 'total_orders'], 'operator' => ['value' => 'gt'], 'value' => 1]]], + 'cannot be used as a filter', + ], + 'an ungrouped column' => [ + ['columns' => [['name' => 'status'], ['name' => 'type']], 'groupBy' => [['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'count'], 'aggregateBy' => ['name' => '*']]]], + "Column 'type' must be grouped or aggregated", + ], + 'an unsupported aggregate' => [ + ['columns' => [['name' => 'status']], 'groupBy' => [['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'median'], 'aggregateBy' => ['name' => 'time']]]], + "Aggregate function 'median' is not supported", + ], + 'an unknown group key' => [ + ['columns' => [['name' => 'status']], 'groupBy' => [['groupBy' => ['name' => 'status`) --'], 'aggregateFn' => ['value' => 'count'], 'aggregateBy' => ['name' => '*']]]], + 'Group by column', + ], + 'an unknown aggregate column' => [ + ['columns' => [['name' => 'status']], 'groupBy' => [['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'sum'], 'aggregateBy' => ['name' => 'password']]]], + "Aggregate column 'password'", + ], + 'an unknown sort column' => [ + ['columns' => [['name' => 'status']], 'sortBy' => [['column' => ['name' => 'secret'], 'direction' => ['value' => 'asc']]]], + "Sort column 'secret'", + ], + 'an unsafe grouped sort alias' => [ + ['columns' => [['name' => 'status']], 'groupBy' => [['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'count'], 'aggregateBy' => ['name' => '*']]], 'sortBy' => [['column' => ['name' => 'x` desc; --'], 'direction' => ['value' => 'asc']]]], + 'Invalid sort column', + ], + 'an unknown nested condition column' => [ + ['columns' => [['name' => 'status']], 'conditions' => [['conditions' => [['field' => ['name' => 'secret'], 'operator' => ['value' => 'eq'], 'value' => 1]]]]], + "Condition column 'secret'", + ], + 'an unsafe computed column name' => [ + ['columns' => [['name' => 'status']], 'computed_columns' => [['name' => 'x` from users --', 'expression' => 'time * 2']]], + 'Invalid computed column name', + ], + 'a computed column subquery' => [ + ['columns' => [['name' => 'status']], 'computed_columns' => [['name' => 'leak', 'expression' => '(SELECT 1)']]], + 'forbidden SQL keyword: SELECT', + ], + 'a computed column hidden in a grouped aggregate' => [ + ['columns' => [['name' => 'status']], 'groupBy' => [['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'sum'], 'aggregateBy' => ['name' => 'evil', 'computed' => true, 'computation' => 'SLEEP(5)']]]], + 'forbidden SQL keyword: SLEEP', + ], + 'circular computed columns' => [ + ['columns' => [['name' => 'status']], 'computed_columns' => [['name' => 'loop_a', 'expression' => 'loop_b + 1'], ['name' => 'loop_b', 'expression' => 'loop_a + 1']]], + 'circular or nested too deeply', + ], +]); + +test('order reporting resolves keywords json arrows and relationship prefixes in expressions', function () { + order_reporting_database(); + + $converter = new ReportQueryConverter(order_reporting_registry(), [ + 'table' => ['name' => 'orders'], + 'columns' => [['name' => 'public_id'], ['name' => 'payload.entities.name']], + 'computed_columns' => [['name' => 'doubled', 'expression' => 'time * 2']], + ]); + // Building the query records the join aliases that relationship columns resolve against. + order_reporting_call($converter, 'buildQuery'); + + $resolve = fn (string $expression) => order_reporting_call($converter, 'resolveComputedColumnReferences', $expression, 'orders'); + + expect($resolve("CAST(JSON_EXTRACT(meta, '$.total') AS SIGNED)"))->toBe("CAST(JSON_EXTRACT(orders.meta, '$.total') AS SIGNED)") + ->and($resolve('CAST(time AS CHAR)'))->toBe('CAST(orders.time AS CHAR)') + ->and($resolve('DATE_ADD(created_at, INTERVAL 7 DAY)'))->toBe('DATE_ADD(orders.created_at, INTERVAL 7 DAY)') + ->and($resolve('DATE_ADD(created_at, INTERVAL time MINUTE)'))->toBe('DATE_ADD(orders.created_at, INTERVAL orders.time MINUTE)') + ->and($resolve("meta->>'$.total' + doubled"))->toBe("orders.meta->>'$.total' + (orders.time * 2)") + ->and($resolve("COUNT(DISTINCT public_id) + LENGTH('status')"))->toBe("COUNT(DISTINCT orders.public_id) + LENGTH('status')") + ->and($resolve("GROUP_CONCAT(status ORDER BY created_at DESC SEPARATOR ', ')"))->toBe("GROUP_CONCAT(orders.status ORDER BY orders.created_at DESC SEPARATOR ', ')") + ->and($resolve('time + DAY + year'))->toBe('orders.time + DAY + year') + ->and($resolve('payload.entities.line_total'))->toBe('(orders_payload_entities.price * (CAST(JSON_UNQUOTE(JSON_EXTRACT(orders_payload_entities.meta, \'$.quantity\')) AS DECIMAL(15,2))))'); + + $paths = order_reporting_call($converter, 'extractRelationshipPathsFromExpression', 'payload.entities.line_total + tracking_number.tracking_number', 'orders'); + expect($paths)->toEqualCanonicalizing(['payload.entities', 'tracking_number']); + + $deepPaths = []; + (new ReflectionMethod($converter, 'collectJoinPathsForReference'))->invokeArgs($converter, ['doubled', &$deepPaths, 99]); + expect($deepPaths)->toBe([]); +}); + +test('report schema columns separate identifiers from foreign keys and expressions from aggregates', function () { + $table = order_reporting_registry()->getTable('orders'); + + $visible = array_map(fn ($column) => $column->getName(), array_values($table->getVisibleColumns())); + expect($visible)->toContain('public_id', 'internal_id', 'order_total', 'total_orders') + ->not->toContain('payload_uuid', 'uuid') + ->and(Column::make('vendor_id')->isForeignKey())->toBeTrue() + ->and(Column::make('public_id')->isForeignKey())->toBeFalse(); + + $expression = Column::expression('order_total', "JSON_EXTRACT(meta, '$.total')", 'decimal'); + expect($expression->isComputed())->toBeTrue() + ->and($expression->isExpression())->toBeTrue() + ->and($expression->isAggregate())->toBeFalse() + ->and($expression->isAggregatable())->toBeTrue() + ->and($expression->toArray())->toMatchArray(['computed' => true, 'aggregate' => false, 'computation' => "JSON_EXTRACT(meta, '$.total')"]) + ->and(Column::expression('label', "CONCAT(name, '!')")->isAggregatable())->toBeFalse(); + + $count = Column::count('total_orders', 'id'); + expect($count->isAggregate())->toBeTrue() + ->and($count->isExpression())->toBeFalse() + ->and($count->toArray()['aggregate'])->toBeTrue() + ->and(Column::computed('share', ' (SUM(total) / 2)')->isAggregate())->toBeTrue() + ->and(Column::computed('flag', 'IF(total > 2, 1, 0)')->isAggregate())->toBeFalse() + ->and(Column::computed('forced', 'total', 'decimal', ['aggregate' => true])->isAggregate())->toBeTrue() + ->and(Column::make('plain')->isAggregate())->toBeFalse() + ->and(Column::isAggregateExpression('group_concat(name)'))->toBeTrue(); + + $payload = $table->getRelationship('payload'); + expect($payload->usesSoftDeletes())->toBeTrue() + ->and($payload->toArray()['soft_deletes'])->toBeTrue() + ->and($payload->getColumn('public_id'))->toBeInstanceOf(Column::class) + ->and($payload->getColumn('missing'))->toBeNull() + ->and($table->usesSoftDeletes())->toBeTrue() + ->and($table->toArray()['soft_deletes'])->toBeTrue() + ->and(Table::make('plain')->usesSoftDeletes())->toBeFalse() + ->and(Relationship::make('plain', 'plain')->usesSoftDeletes())->toBeFalse(); +}); + +test('report schema registry exposes nested relationship and expression columns for the report builder', function () { + $registry = order_reporting_registry(); + $columns = collect($registry->getTableColumns('orders'))->keyBy('name'); + + expect($columns->keys()->all())->toContain('public_id', 'payload.public_id', 'payload.entities.name', 'payload.entities.quantity', 'tracking_number.tracking_number') + ->and($columns['payload.entities.quantity']['auto_join_path'])->toBe('payload.entities') + ->and($columns['payload.entities.quantity']['computed'])->toBeTrue() + ->and($columns['total_orders']['aggregate'])->toBeTrue() + ->and($registry->isColumnAllowed('orders', 'payload.entities.line_total'))->toBeTrue() + ->and($registry->isColumnAllowed('orders', 'payload.entities.uuid'))->toBeFalse(); +}); + +test('order reporting emits a repeated aggregate once and reports an unregistered table', function () { + $result = order_reporting_run([ + 'columns' => [['name' => 'status']], + 'groupBy' => [ + ['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'count'], 'aggregateBy' => ['name' => '*']], + ['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'count'], 'aggregateBy' => ['name' => '*']], + ], + ]); + + expect($result['success'])->toBeTrue() + ->and(substr_count($result['meta']['query_sql'], 'COUNT(*)'))->toBe(1); + + $missing = (new ReportQueryConverter(order_reporting_registry(), [ + 'table' => ['name' => 'invoices'], + 'columns' => [['name' => 'status']], + 'groupBy' => [['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'sum'], 'aggregateBy' => ['name' => 'amount', 'computed' => true, 'computation' => 'total']]], + ]))->execute(); + + expect($missing['success'])->toBeFalse() + ->and($missing['error'])->toBe("Table 'invoices' is not registered"); +}); + +test('report schema registry labels relationship columns with the whole relationship name without repeating it', function () { + $registry = new ReportSchemaRegistry(); + $registry->setCacheEnabled(false); + $registry->registerTable( + Table::make('orders') + ->columns([Column::make('public_id')->label('ID')]) + ->relationships([ + Relationship::hasAutoJoin('order_config', 'order_configs') + ->label('Order Config') + ->localKey('order_config_uuid') + ->columns([Column::make('namespace')->label('Namespace')]), + Relationship::hasAutoJoin('transaction', 'transactions') + ->label('Transaction') + ->columns([ + Column::make('public_id')->label('Transaction ID'), + Column::make('gateway')->label('Payment Gateway'), + Column::make('memo')->label('Transactions Memo'), + ]), + Relationship::hasAutoJoin('tracking', 'tracking_numbers') + ->label('Tracking') + ->columns([Column::make('tracking_number')->label('Tracking')]) + ->with([ + Relationship::hasAutoJoin('status', 'tracking_statuses') + ->label('Tracking Status') + ->localKey('status_uuid') + ->columns([Column::make('city')->label('City')]), + ]), + Relationship::hasAutoJoin('pickup', 'places') + ->label('Pickup Location') + ->columns([Column::make('city')->label('City')]), + ]) + ); + + $labels = collect($registry->getTableColumns('orders'))->pluck('label', 'name'); + + expect($labels['order_config.namespace'])->toBe('Order Config Namespace') + ->and($labels['transaction.public_id'])->toBe('Transaction ID') + ->and($labels['transaction.gateway'])->toBe('Transaction Payment Gateway') + ->and($labels['transaction.memo'])->toBe('Transaction Transactions Memo', 'a longer word is not mistaken for the relationship name') + ->and($labels['tracking.tracking_number'])->toBe('Tracking') + ->and($labels['tracking.status.city'])->toBe('Tracking Status City') + ->and($labels['pickup.city'])->toBe('Pickup City'); +}); + +test('report schemas never expose internal bookkeeping columns', function () { + $registry = new ReportSchemaRegistry(); + $registry->setCacheEnabled(false); + $registry->registerTable( + Table::make('orders') + ->columns([Column::make('_key'), Column::make('_import_id'), Column::make('status')]) + ->relationships([ + Relationship::hasAutoJoin('payload', 'payloads') + ->columns([Column::make('_key'), Column::make('type')]), + ]) + ); + + $names = collect($registry->getTableColumns('orders'))->pluck('name')->all(); + + expect($names)->toBe(['status', 'payload.type']) + ->and($registry->isColumnAllowed('orders', '_key'))->toBeFalse() + ->and($registry->isColumnAllowed('orders', '_import_id'))->toBeFalse() + ->and($registry->isColumnAllowed('orders', 'payload._key'))->toBeFalse() + ->and($registry->isColumnAllowed('orders', 'payload.type'))->toBeTrue() + ->and(Column::isSystemColumnName('_key'))->toBeTrue() + ->and(Column::isSystemColumnName('key'))->toBeFalse(); + + order_reporting_database(); + $result = (new ReportQueryConverter($registry, ['table' => ['name' => 'orders'], 'columns' => [['name' => '_key']]]))->execute(); + expect($result['success'])->toBeFalse() + ->and($result['error'])->toContain("Column '_key' is not allowed"); +}); diff --git a/tests/Unit/Reporting/ReportQueryConverterTest.php b/tests/Unit/Reporting/ReportQueryConverterTest.php index 83d94132..8ad7ea7d 100644 --- a/tests/Unit/Reporting/ReportQueryConverterTest.php +++ b/tests/Unit/Reporting/ReportQueryConverterTest.php @@ -1064,7 +1064,7 @@ function report_converter_property(object $target, string $property, mixed $valu ->and($result['data'])->toHaveCount(2) ->and((float) $result['data'][0]->sum_gross_total)->toBe(251.0) ->and((float) $result['data'][1]->sum_gross_total)->toBe(150.0) - ->and($result['meta']['query_sql'])->toContain('SUM(orders.total * 2)') + ->and($result['meta']['query_sql'])->toContain('SUM((orders.total * 2))') ->and($result['columns'])->toContainEqual([ 'name' => 'gross_total', 'column_name' => 'gross_total', @@ -1076,7 +1076,7 @@ function report_converter_property(object $target, string $property, mixed $valu ->and($result['columns'])->toContainEqual([ 'name' => 'sum_gross_total', 'column_name' => 'sum_gross_total', - 'label' => 'Sum (gross_total)', + 'label' => 'Sum (Gross Total)', 'type' => 'decimal', 'auto_join_path' => null, ]); diff --git a/tests/Unit/Reporting/ReportQueryValidatorTest.php b/tests/Unit/Reporting/ReportQueryValidatorTest.php index 33c46f00..95a8467a 100644 --- a/tests/Unit/Reporting/ReportQueryValidatorTest.php +++ b/tests/Unit/Reporting/ReportQueryValidatorTest.php @@ -736,3 +736,34 @@ function valid_report_query_config(array $overrides = []): array ->and($result['summary']['complexity'])->toBe('medium') ->and($result['summary']['estimated_performance'])->toBe('slow'); }); + +test('report query validator accepts computed group keys distinct counts and sorting by an aggregate alias', function () { + $validator = report_validator_fixture(); + + $result = $validator->validate([ + 'table' => ['name' => 'orders'], + 'columns' => [['name' => 'status'], ['name' => 'total']], + 'computed_columns' => [['name' => 'order_month', 'expression' => "DATE_FORMAT(created_at, '%Y-%m')"]], + 'groupBy' => [ + ['groupBy' => ['name' => 'status']], + ['groupBy' => ['name' => 'order_month'], 'aggregateFn' => ['value' => 'count_distinct'], 'aggregateBy' => ['name' => 'public_id']], + ['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'sum'], 'aggregateBy' => ['name' => 'payload.pickup.city']], + ], + 'sortBy' => [ + ['column' => ['name' => 'count_distinct_public_id'], 'direction' => ['value' => 'desc']], + ['column' => ['name' => 'sum_payload_pickup_city'], 'direction' => ['value' => 'asc']], + ], + ]); + + expect($result['errors'])->toBe([]) + ->and($result['valid'])->toBeTrue(); + + $unknownSort = $validator->validate([ + 'table' => ['name' => 'orders'], + 'columns' => [['name' => 'status']], + 'groupBy' => [['groupBy' => ['name' => 'status'], 'aggregateFn' => ['value' => 'count'], 'aggregateBy' => ['name' => '*']]], + 'sortBy' => [['column' => ['name' => 'sum_total'], 'direction' => ['value' => 'asc']]], + ]); + + expect($unknownSort['errors'])->toBe(["Sort By 0: Field 'sum_total' is not available"]); +});