From 77f45429e122194e9b70bd52d617675fd03de8d2 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 25 Sep 2026 10:30:42 +0800 Subject: [PATCH 1/5] feat(reporting): expression columns, computed group keys and safer report queries Order reports could not answer basic questions such as "which products sold the most this month" or "what did orders total this month". This extends the reporting framework so extension schemas and the report builder can express them. Schema - Column::expression() declares a row-level SQL expression (e.g. a value read out of a JSON column). Bare names resolve against the table or relationship that declares it, so an expression on payload.entities reads the entity's meta. Expression columns can be selected, filtered, sorted, grouped and aggregated. - Aggregate computed columns (Column::count/sum/...) are now flagged `aggregate` and resolve to their computation instead of a non-existent physical column. Without grouping they produce a summary row; with grouping they sit beside the group keys. - Table/Relationship::softDeletes() leave out rows whose deleted_at is set (on the root table, and inside the ON clause of joins). - public_id and internal_id are no longer hidden as foreign keys. Queries - Group by, filter and sort by computed columns; sort a grouped report by an aggregate's alias; new count_distinct aggregate. - Computed expressions accept JSON_EXTRACT/JSON_UNQUOTE/JSON_VALUE and friends, DATE(), CAST(... AS DECIMAL(p,s)) and other cast targets, DISTINCT, IN, the ->/->> operators, and INTERVAL units, without misreading keywords as columns. - Aggregate labels use the column label ("Sum (Quantity)"). Hardening - Computed columns are validated in grouped reports too (a client-supplied aggregateBy computation was previously used unvalidated), their names must be safe identifiers, SELECT is forbidden, and schema-declared columns always take their SQL from the registry rather than the request. - Group keys, aggregate columns, sort columns and condition fields must be allowed columns or computed columns; sort direction is normalised. --- .../Reporting/ComputedColumnValidator.php | 52 +- .../Reporting/ReportQueryConverter.php | 850 +++++++++++------- .../Reporting/ReportQueryValidator.php | 36 +- src/Support/Reporting/Schema/Column.php | 80 +- src/Support/Reporting/Schema/Relationship.php | 31 + src/Support/Reporting/Schema/Table.php | 19 +- ...ReportQueryConverterOrderReportingTest.php | 503 +++++++++++ .../Reporting/ReportQueryConverterTest.php | 4 +- .../Reporting/ReportQueryValidatorTest.php | 31 + 9 files changed, 1261 insertions(+), 345 deletions(-) create mode 100644 tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php 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/Schema/Column.php b/src/Support/Reporting/Schema/Column.php index 735b1d6b..2f5e4a89 100644 --- a/src/Support/Reporting/Schema/Column.php +++ b/src/Support/Reporting/Schema/Column.php @@ -6,6 +6,11 @@ class Column { + /** + * Identifier columns that end in `_id` but are not foreign keys. + */ + public const IDENTIFIER_COLUMNS = ['public_id', 'internal_id']; + protected string $name; protected string $label; protected string $type; @@ -18,6 +23,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 +46,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 +318,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 +363,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 +411,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 +441,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..682c729d 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; @@ -439,6 +455,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 +469,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/Reporting/ReportQueryConverterOrderReportingTest.php b/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php new file mode 100644 index 00000000..79803e1a --- /dev/null +++ b/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php @@ -0,0 +1,503 @@ + 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'], +]; + +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"); +}); 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"]); +}); From 85a23a206b735e45ff33ed376f53210d3d5b71db Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 25 Sep 2026 12:09:42 +0800 Subject: [PATCH 2/5] test(reporting): mark the fixture month filter as clock-independent for the date drift check --- tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php b/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php index 79803e1a..61982059 100644 --- a/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php +++ b/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php @@ -177,7 +177,7 @@ function order_reporting_call(object $target, string $method, mixed ...$argument $septemberCondition = [ 'field' => ['name' => 'created_at'], 'operator' => ['value' => 'between'], - 'value' => ['2026-09-01 00:00:00', '2026-09-30 23:59:59'], + '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) { From fd32df49fa99839ccbb0f8db0018b7a993187d04 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 25 Sep 2026 12:58:25 +0800 Subject: [PATCH 3/5] fix(reporting): label relationship columns with the whole relationship name, hide system columns - A relationship column was labelled with only the first word of its relationship, so Order Config's namespace read "Order Namespace" and Customer Vendor's name read "Customer Name". Use the whole relationship label ("Order Config Namespace"), and don't repeat it when the column label already starts with it ("Transaction ID", not "Transaction Transaction ID"). - _key and _import_id are internal bookkeeping: never list or allow them, whatever a schema declares, on the root table or through a relationship. --- .../Reporting/ReportSchemaRegistry.php | 58 +++++++++------ src/Support/Reporting/Schema/Column.php | 13 ++++ src/Support/Reporting/Schema/Table.php | 4 +- ...ReportQueryConverterOrderReportingTest.php | 72 +++++++++++++++++++ 4 files changed, 125 insertions(+), 22 deletions(-) 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 2f5e4a89..fde3b3d6 100644 --- a/src/Support/Reporting/Schema/Column.php +++ b/src/Support/Reporting/Schema/Column.php @@ -11,6 +11,19 @@ class Column */ 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; diff --git a/src/Support/Reporting/Schema/Table.php b/src/Support/Reporting/Schema/Table.php index 682c729d..0704f28e 100644 --- a/src/Support/Reporting/Schema/Table.php +++ b/src/Support/Reporting/Schema/Table.php @@ -334,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()); }); } @@ -414,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); } /** diff --git a/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php b/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php index 61982059..654ed3fd 100644 --- a/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php +++ b/tests/Unit/Reporting/ReportQueryConverterOrderReportingTest.php @@ -501,3 +501,75 @@ function order_reporting_call(object $target, string $method, mixed ...$argument 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"); +}); From 916e70f820c0a12c7bfa72712def3efc535415a5 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 25 Sep 2026 16:07:35 +0800 Subject: [PATCH 4/5] fix(settings): send test SMS with the credentials entered in the console "Test SMS Provider" failed with "Credentials are required to create a Client" even with a Twilio SID and token filled in (fleetbase/fleetbase#680). The test endpoints apply the entered credentials to config, but the Twilio manager copies its settings when it is built and is cached twice: as a container singleton and in the facade's static cache. Under Octane that facade cache outlives the request, and the provider's singleton closure reads the worker's base config rather than the request's copy. So a test send used whatever the worker first built: empty credentials when none were saved (the reported error), or the saved ones instead of those just entered. - After applying the entered credentials, both test endpoints rebuild the manager from this request's config and clear the facade cache. A stand-in bound in place of the real manager is left alone. - Once the test send is done the facade cache is cleared again, so the credentials under test are not reused by later requests in the worker. Fixes fleetbase/fleetbase#680 --- .../Internal/v1/SettingController.php | 54 ++++++++- .../SettingControllerExternalProbesTest.php | 109 +++++++++++++++++- 2 files changed, 161 insertions(+), 2 deletions(-) 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/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'); +}); From 7c45a949d729d25590d4ff19b602acf10ce0ac94 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Fri, 25 Sep 2026 16:11:02 +0800 Subject: [PATCH 5/5] chore(release): v1.6.64 --- RELEASE.md | 45 +++++++++++++++++++-------------------------- composer.json | 2 +- 2 files changed, 20 insertions(+), 27 deletions(-) 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",