From dd6998fc439a51f1bb8f7d32de40dc040c6784ac Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Mon, 10 Aug 2026 17:18:07 +0600 Subject: [PATCH 1/2] refactor(grid): split GridConfigService into repository and collaborators Phase A of #365: persistence, column-type validators, and relation JOIN grouping move out of the god-service so grid column work stays reviewable. Empty keep-list on save no longer treats missing names as wipe-all. --- .../Services/Grid/GridColumnTypeValidator.php | 226 +++++ .../Services/Grid/GridConfigRepository.php | 145 +++ .../Grid/GridRelationFieldExtractor.php | 91 ++ .../src/Services/Grid/OptionColumnSpec.php | 2 +- .../src/Services/GridConfigService.php | 858 +++++------------- .../tests/GridColumnTypeValidatorTest.php | 92 ++ .../GridConfigRepositoryEmptyKeepTest.php | 36 + .../Grid/GridRelationFieldExtractorTest.php | 60 ++ .../tests/stubs/GridValidatorModxStub.php | 29 + 9 files changed, 895 insertions(+), 644 deletions(-) create mode 100644 core/components/minishop3/src/Services/Grid/GridColumnTypeValidator.php create mode 100644 core/components/minishop3/src/Services/Grid/GridConfigRepository.php create mode 100644 core/components/minishop3/src/Services/Grid/GridRelationFieldExtractor.php create mode 100644 core/components/minishop3/tests/GridColumnTypeValidatorTest.php create mode 100644 core/components/minishop3/tests/Unit/Services/Grid/GridConfigRepositoryEmptyKeepTest.php create mode 100644 core/components/minishop3/tests/Unit/Services/Grid/GridRelationFieldExtractorTest.php create mode 100644 core/components/minishop3/tests/stubs/GridValidatorModxStub.php diff --git a/core/components/minishop3/src/Services/Grid/GridColumnTypeValidator.php b/core/components/minishop3/src/Services/Grid/GridColumnTypeValidator.php new file mode 100644 index 000000000..adc0c81a1 --- /dev/null +++ b/core/components/minishop3/src/Services/Grid/GridColumnTypeValidator.php @@ -0,0 +1,226 @@ +} + */ +final class GridColumnTypeValidator +{ + /** @var list */ + private const ALLOWED_AGGREGATIONS = ['COUNT', 'SUM', 'AVG', 'MIN', 'MAX']; + + /** @var list */ + private const ALLOWED_ACTION_HANDLERS = [ + 'edit', 'delete', 'view', 'refresh', 'addresses', 'publish', 'duplicate', + ]; + + /** @var list */ + private const ALLOWED_SEVERITIES = ['secondary', 'success', 'info', 'warn', 'danger']; + + /** @var array */ + private const RELATION_REQUIRED = [ + 'table' => 'relation.table is required', + 'foreignKey' => 'relation.foreignKey is required', + 'displayField' => 'relation.displayField is required', + ]; + + public function __construct(private modX $modx) + { + } + + /** + * @param array $config + * @return ValidationResult + */ + public function validateForType(string $type, array $config, string $fieldName = ''): array + { + return match ($type) { + 'template' => $this->validateTemplateConfig($config), + 'relation' => $this->validateRelationConfig($config), + 'computed' => $this->validateComputedConfig($config), + 'actions' => $this->validateActionsConfig($config), + 'option' => $this->validateOptionConfig($config, $fieldName), + default => ['success' => true], + }; + } + + /** + * @param array $config + * @return ValidationResult + */ + public function validateTemplateConfig(array $config): array + { + if (empty($config['template'])) { + return ['success' => false, 'message' => 'template is required for template field']; + } + + if (!is_string($config['template'])) { + return ['success' => false, 'message' => 'template must be a string']; + } + + return ['success' => true]; + } + + /** + * @param array $config + * @return ValidationResult + */ + public function validateRelationConfig(array $config): array + { + $relation = $config['relation'] ?? []; + + foreach (self::RELATION_REQUIRED as $key => $message) { + if (empty($relation[$key])) { + return ['success' => false, 'message' => $message]; + } + } + + $aggregation = $relation['aggregation'] ?? null; + if ($aggregation !== null && !in_array($aggregation, self::ALLOWED_AGGREGATIONS, true)) { + return [ + 'success' => false, + 'message' => 'Invalid aggregation type. Allowed: ' . implode(', ', self::ALLOWED_AGGREGATIONS), + ]; + } + + $tableOrModel = $relation['table']; + $isModel = str_contains($tableOrModel, '\\') || str_contains($tableOrModel, '::'); + + if ($isModel) { + try { + $tableName = $this->modx->getTableName($tableOrModel); + + if (empty($tableName)) { + return ['success' => false, 'message' => "Unable to get table name for model: {$tableOrModel}"]; + } + + $config['relation']['resolvedTableName'] = $tableName; + } catch (\Exception $e) { + return [ + 'success' => false, + 'message' => "Invalid model class: {$tableOrModel}. " . $e->getMessage(), + ]; + } + } else { + $tablePrefix = $this->modx->config['table_prefix'] ?? ''; + if ($tablePrefix !== '' && !str_starts_with($tableOrModel, $tablePrefix)) { + $tableOrModel = $tablePrefix . $tableOrModel; + } + $config['relation']['resolvedTableName'] = $tableOrModel; + } + + return ['success' => true, 'config' => $config]; + } + + /** + * @param array $config + * @return ValidationResult + */ + public function validateComputedConfig(array $config): array + { + $computed = $config['computed'] ?? []; + + if (empty($computed['className'])) { + return ['success' => false, 'message' => 'computed.className is required']; + } + + if (!class_exists($computed['className'])) { + return ['success' => false, 'message' => "Class {$computed['className']} not found"]; + } + + $interfaces = class_implements($computed['className']) ?: []; + if (!isset($interfaces[ComputedFieldInterface::class])) { + return ['success' => false, 'message' => 'Class must implement ComputedFieldInterface']; + } + + return ['success' => true]; + } + + /** + * @param array $config + * @return ValidationResult + */ + public function validateActionsConfig(array $config): array + { + $actions = $config['actions'] ?? []; + + if (!is_array($actions)) { + return ['success' => false, 'message' => 'actions must be an array']; + } + + $actionNames = []; + + foreach ($actions as $index => $action) { + if (empty($action['name'])) { + return ['success' => false, 'message' => "actions[{$index}].name is required"]; + } + + if (in_array($action['name'], $actionNames, true)) { + return ['success' => false, 'message' => "Duplicate action name: {$action['name']}"]; + } + $actionNames[] = $action['name']; + + if (empty($action['handler'])) { + return ['success' => false, 'message' => "actions[{$index}].handler is required"]; + } + + $handler = $action['handler']; + if ( + !in_array($handler, self::ALLOWED_ACTION_HANDLERS, true) + && !str_starts_with($handler, 'custom:') + ) { + $this->modx->log( + modX::LOG_LEVEL_INFO, + "[GridColumnTypeValidator] Custom handler used: {$handler} in action {$action['name']}" + ); + } + + if (!empty($action['severity']) && !in_array($action['severity'], self::ALLOWED_SEVERITIES, true)) { + return [ + 'success' => false, + 'message' => "Invalid severity for action {$action['name']}. Allowed: " + . implode(', ', self::ALLOWED_SEVERITIES), + ]; + } + } + + return ['success' => true]; + } + + /** + * @param array $config + * @return ValidationResult + */ + public function validateOptionConfig(array $config, string $fieldName = ''): array + { + $option = $config['option'] ?? []; + + if (empty($option['key'])) { + return ['success' => false, 'message' => 'option.key is required for option field']; + } + + $key = (string) $option['key']; + if (!OptionColumnSpec::isValidOptionKey($key)) { + return ['success' => false, 'message' => 'option.key must contain only letters, numbers and underscores']; + } + + if ($fieldName !== '' && !OptionColumnSpec::isValidFieldName($fieldName)) { + return [ + 'success' => false, + 'message' => "Field name '{$fieldName}' is not allowed for option columns: " + . 'it collides with a builtin product column or contains invalid characters. ' + . "Use a name like 'option_{$key}' instead.", + ]; + } + + return ['success' => true]; + } +} diff --git a/core/components/minishop3/src/Services/Grid/GridConfigRepository.php b/core/components/minishop3/src/Services/Grid/GridConfigRepository.php new file mode 100644 index 000000000..968d7e07f --- /dev/null +++ b/core/components/minishop3/src/Services/Grid/GridConfigRepository.php @@ -0,0 +1,145 @@ + + */ + public function findByGridKey(string $gridKey, bool $includeHidden = false): array + { + $where = ['grid_key' => $gridKey]; + if (!$includeHidden) { + $where['visible'] = true; + } + + return $this->collectSorted($where); + } + + /** + * @return list + */ + public function findFilterableByGridKey(string $gridKey): array + { + return $this->collectSorted([ + 'grid_key' => $gridKey, + 'filterable' => true, + ]); + } + + public function findOne(string $gridKey, string $fieldName): ?msGridField + { + /** @var msGridField|null $field */ + $field = $this->modx->getObject(msGridField::class, [ + 'grid_key' => $gridKey, + 'field_name' => $fieldName, + ]); + + return $field instanceof msGridField ? $field : null; + } + + public function nextSortOrder(string $gridKey): int + { + $query = $this->modx->newQuery(msGridField::class); + $query->where(['grid_key' => $gridKey]); + $query->sortby('sort_order', 'DESC'); + $query->limit(1); + $lastField = $this->modx->getObject(msGridField::class, $query); + + return $lastField ? (int) $lastField->get('sort_order') + 1 : 1; + } + + /** + * @param array $attributes + */ + public function create(array $attributes): msGridField + { + /** @var msGridField $field */ + $field = $this->modx->newObject(msGridField::class); + $field->fromArray($attributes); + + return $field; + } + + public function save(msGridField $field): bool + { + return (bool) $field->save(); + } + + public function remove(msGridField $field): bool + { + return (bool) $field->remove(); + } + + /** + * Non-system fields whose names are not in $fieldNamesToKeep (candidates for delete on save). + * + * Empty keep list returns [] — never wipe all non-system rows. Callers that omit every + * field name must not treat that as "delete everything" (xPDO `NOT IN ()` is also invalid). + * + * @param list $fieldNamesToKeep + * @return list + */ + public function findNonSystemNotIn(string $gridKey, array $fieldNamesToKeep): array + { + if ($fieldNamesToKeep === []) { + return []; + } + + /** @var list $fields */ + return array_values($this->modx->getCollection(msGridField::class, [ + 'grid_key' => $gridKey, + 'is_system' => false, + 'field_name:NOT IN' => $fieldNamesToKeep, + ]) ?: []); + } + + /** + * @return array + */ + public function decodeConfig(mixed $config): array + { + if (is_string($config)) { + $decoded = json_decode($config, true); + + return is_array($decoded) ? $decoded : []; + } + + return is_array($config) ? $config : []; + } + + /** + * @param array $config + */ + public function encodeConfig(array $config): string + { + return (string) json_encode($config, JSON_UNESCAPED_UNICODE); + } + + /** + * @param array $where + * @return list + */ + private function collectSorted(array $where): array + { + $query = $this->modx->newQuery(msGridField::class); + $query->where($where); + $query->sortby('sort_order', 'ASC'); + + /** @var list $fields */ + return array_values($this->modx->getCollection(msGridField::class, $query) ?: []); + } +} diff --git a/core/components/minishop3/src/Services/Grid/GridRelationFieldExtractor.php b/core/components/minishop3/src/Services/Grid/GridRelationFieldExtractor.php new file mode 100644 index 000000000..8979d2403 --- /dev/null +++ b/core/components/minishop3/src/Services/Grid/GridRelationFieldExtractor.php @@ -0,0 +1,91 @@ + */ + private const MODEL_MAP = [ + 'msOrder' => 'MiniShop3\\Model\\msOrder', + 'msOrderStatus' => 'MiniShop3\\Model\\msOrderStatus', + 'msOrderAddress' => 'MiniShop3\\Model\\msOrderAddress', + 'msOrderProduct' => 'MiniShop3\\Model\\msOrderProduct', + 'msDelivery' => 'MiniShop3\\Model\\msDelivery', + 'msPayment' => 'MiniShop3\\Model\\msPayment', + 'msProduct' => 'MiniShop3\\Model\\msProduct', + 'msProductData' => 'MiniShop3\\Model\\msProductData', + 'msCategory' => 'MiniShop3\\Model\\msCategory', + 'msCategoryMember' => 'MiniShop3\\Model\\msCategoryMember', + 'msVendor' => 'MiniShop3\\Model\\msVendor', + 'msCustomer' => 'MiniShop3\\Model\\msCustomer', + 'modUser' => 'MODX\\Revolution\\modUser', + 'modUserProfile' => 'MODX\\Revolution\\modUserProfile', + 'modResource' => 'MODX\\Revolution\\modResource', + ]; + + /** + * @param list> $gridFields + * @return array + * }> + */ + public function extract(array $gridFields): array + { + $relationGroups = []; + + foreach ($gridFields as $field) { + if (($field['type'] ?? 'model') !== 'relation') { + continue; + } + + $relation = $field['relation'] ?? null; + if (!$this->isCompleteRelation($relation)) { + continue; + } + + $table = (string) $relation['table']; + $foreignKey = (string) $relation['foreignKey']; + $displayField = (string) $relation['displayField']; + $fieldName = (string) ($field['name'] ?? ''); + $groupKey = "{$table}_{$foreignKey}"; + + if (!isset($relationGroups[$groupKey])) { + $relationGroups[$groupKey] = [ + 'table' => $table, + 'modelClass' => self::MODEL_MAP[$table] ?? null, + 'foreignKey' => $foreignKey, + 'alias' => "rel_{$table}_{$foreignKey}", + 'fields' => [], + ]; + } + + $relationGroups[$groupKey]['fields'][] = [ + 'name' => $fieldName, + 'displayField' => $displayField, + ]; + } + + return $relationGroups; + } + + /** + * @param mixed $relation + * @phpstan-assert-if-true array{table: mixed, foreignKey: mixed, displayField: mixed} $relation + */ + private function isCompleteRelation(mixed $relation): bool + { + return is_array($relation) + && !empty($relation['table']) + && !empty($relation['foreignKey']) + && !empty($relation['displayField']); + } +} diff --git a/core/components/minishop3/src/Services/Grid/OptionColumnSpec.php b/core/components/minishop3/src/Services/Grid/OptionColumnSpec.php index 299e311d7..65bb24178 100644 --- a/core/components/minishop3/src/Services/Grid/OptionColumnSpec.php +++ b/core/components/minishop3/src/Services/Grid/OptionColumnSpec.php @@ -9,7 +9,7 @@ */ final readonly class OptionColumnSpec { - /** Same rule as {@see \MiniShop3\Services\GridConfigService::validateOptionConfig()} */ + /** Same rule as {@see \MiniShop3\Services\Grid\GridColumnTypeValidator::validateOptionConfig()} */ private const OPTION_KEY_PATTERN = '/^[a-z0-9_]+$/i'; /** Same rule as OPTION_KEY_PATTERN; fieldName also lands in `SELECT … AS \`{name}\``. */ diff --git a/core/components/minishop3/src/Services/GridConfigService.php b/core/components/minishop3/src/Services/GridConfigService.php index e1d913d81..afb4119ed 100644 --- a/core/components/minishop3/src/Services/GridConfigService.php +++ b/core/components/minishop3/src/Services/GridConfigService.php @@ -1,70 +1,77 @@ */ + private const SAVE_CONFIG_KEYS = [ + 'template', 'type', 'format', 'actions', + 'relation', + 'computed', + 'option', + 'source_field', 'color_field', + 'decimals', 'currency', 'currency_position', 'thousands_separator', 'decimal_separator', + 'unit', 'unit_position', + 'editable', 'editor_type', 'editor_options', 'editor_reference', 'editor_combo_endpoint', + ]; + + public function __construct( + modX $modx, + ?GridConfigRepository $repository = null, + ?GridColumnTypeValidator $typeValidator = null, + ?GridRelationFieldExtractor $relationExtractor = null, + ) { $this->modx = $modx; + $this->repository = $repository ?? new GridConfigRepository($modx); + $this->typeValidator = $typeValidator ?? new GridColumnTypeValidator($modx); + $this->relationExtractor = $relationExtractor ?? new GridRelationFieldExtractor(); } /** - * Get grid columns configuration - * - * @param string $gridKey Grid key (customers, orders, products, etc.) - * @return array + * @return list> */ public function getGridConfig(string $gridKey, bool $includeHidden = false): array { - $query = $this->modx->newQuery(msGridField::class); - $where = ['grid_key' => $gridKey]; - if (!$includeHidden) { - $where['visible'] = true; - } - $query->where($where); - $query->sortby('sort_order', 'ASC'); - $fields = []; - $collection = $this->modx->getCollection(msGridField::class, $query); - - foreach ($collection as $field) { - $config = $field->get('config'); - // xPDO 3 with phptype='json' automatically deserializes JSON to array - if (is_string($config)) { - $config = json_decode($config, true) ?: []; - } elseif (!is_array($config)) { - $config = []; - } + foreach ($this->repository->findByGridKey($gridKey, $includeHidden) as $field) { + $config = $this->repository->decodeConfig($field->get('config')); $fieldData = [ 'name' => $field->get('field_name'), 'label' => $this->resolveLabel($field), - 'visible' => (bool)$field->get('visible'), - 'sortable' => (bool)$field->get('sortable'), - 'filterable' => (bool)$field->get('filterable'), - 'frozen' => (bool)$field->get('frozen'), + 'visible' => (bool) $field->get('visible'), + 'sortable' => (bool) $field->get('sortable'), + 'filterable' => (bool) $field->get('filterable'), + 'frozen' => (bool) $field->get('frozen'), 'width' => $field->get('width'), 'minWidth' => $field->get('min_width'), - 'isSystem' => (bool)$field->get('is_system'), + 'isSystem' => (bool) $field->get('is_system'), ]; - // Merge additional configuration from JSON - if (!empty($config)) { + if ($config !== []) { $fieldData = array_merge($fieldData, $config); } @@ -74,15 +81,8 @@ public function getGridConfig(string $gridKey, bool $includeHidden = false): arr return $fields; } - /** - * Get label considering lexicon_key - * - * @param msGridField $field - * @return string - */ protected function resolveLabel(msGridField $field): string { - // Priority: direct label > lexicon_key > field_name $label = $field->get('label'); if (!empty($label)) { return $label; @@ -92,316 +92,202 @@ protected function resolveLabel(msGridField $field): string if (!empty($lexiconKey)) { $this->modx->lexicon->load('minishop3:vue'); $translated = $this->modx->lexicon($lexiconKey); - - // If translation found (key itself not returned) if ($translated !== $lexiconKey) { return $translated; } } - // Fallback - field_name return $field->get('field_name'); } /** - * Save grid configuration - * - * @param string $gridKey - * @param array $fields - * @return bool + * @param list> $fields */ public function saveGridConfig(string $gridKey, array $fields): bool { try { - // Get list of field names to keep $fieldNamesToKeep = []; foreach ($fields as $index => $fieldData) { $fieldName = $fieldData['name'] ?? null; - if (!$fieldName) { continue; } $fieldNamesToKeep[] = $fieldName; - - $field = $this->modx->getObject(msGridField::class, [ - 'grid_key' => $gridKey, - 'field_name' => $fieldName, - ]); - + $field = $this->repository->findOne($gridKey, $fieldName); if (!$field) { - $this->modx->log(modX::LOG_LEVEL_WARN, - "[GridConfigService] Field not found: {$gridKey}.{$fieldName}"); + $this->modx->log( + modX::LOG_LEVEL_WARN, + "[GridConfigService] Field not found: {$gridKey}.{$fieldName}" + ); continue; } - // Update basic parameters - if (isset($fieldData['label'])) { - $field->set('label', $fieldData['label']); - } - if (isset($fieldData['visible'])) { - $field->set('visible', (bool)$fieldData['visible']); - } - // Use array index as sort_order to preserve drag & drop order - $field->set('sort_order', $index); - - if (isset($fieldData['sortable'])) { - $field->set('sortable', (bool)$fieldData['sortable']); - } - if (isset($fieldData['filterable'])) { - $field->set('filterable', (bool)$fieldData['filterable']); - } - if (isset($fieldData['frozen'])) { - $field->set('frozen', (bool)$fieldData['frozen']); - } - if (isset($fieldData['width'])) { - $field->set('width', $fieldData['width']); - } - if (isset($fieldData['minWidth'])) { - $field->set('min_width', $fieldData['minWidth']); - } - - // Update JSON config (additional parameters) - // Start with existing config to preserve values not sent - $existingConfig = $field->get('config'); - if (is_string($existingConfig)) { - $existingConfig = json_decode($existingConfig, true) ?: []; - } elseif (!is_array($existingConfig)) { - $existingConfig = []; - } + $this->applySavePayload($field, $fieldData, $index); - $config = $existingConfig; - $configKeys = [ - 'template', 'type', 'format', 'actions', - // relation type - 'relation', - // computed type - 'computed', - // option type - 'option', - // badge type - 'source_field', 'color_field', - // datetime type - // (format is already included) - // price type - 'decimals', 'currency', 'currency_position', 'thousands_separator', 'decimal_separator', - // weight type - 'unit', 'unit_position', - // inline edit (category-products) - 'editable', 'editor_type', 'editor_options', 'editor_reference', 'editor_combo_endpoint', - ]; - foreach ($configKeys as $key) { + $config = $this->repository->decodeConfig($field->get('config')); + foreach (self::SAVE_CONFIG_KEYS as $key) { if (array_key_exists($key, $fieldData)) { $config[$key] = $fieldData[$key]; } } - if (($config['editor_type'] ?? '') !== GridColumnEditorType::COMBO) { - unset($config['editor_reference'], $config['editor_combo_endpoint']); - } - - $comboCheck = GridEditorReferenceRegistry::validateComboEditorConfig($config); - if (!$comboCheck['success']) { - $this->modx->log(modX::LOG_LEVEL_ERROR, - '[GridConfigService] Combo editor validation failed for ' . $gridKey . '.' . $fieldName . ': ' . ($comboCheck['message'] ?? '')); + $normalized = $this->normalizeComboEditorConfig($config); + if (!$normalized['success']) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[GridConfigService] Combo editor validation failed for ' + . $gridKey . '.' . $fieldName . ': ' . ($normalized['message'] ?? '') + ); return false; } - $field->set('config', json_encode($config, JSON_UNESCAPED_UNICODE)); + $field->set('config', $this->repository->encodeConfig($normalized['config'])); + if (!$this->repository->save($field)) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + "[GridConfigService] Failed to save field: {$gridKey}.{$fieldName}" + ); - if (!$field->save()) { - $this->modx->log(modX::LOG_LEVEL_ERROR, - "[GridConfigService] Failed to save field: {$gridKey}.{$fieldName}"); return false; } } - // Delete fields not in new list (only non-system) - $fieldsToDelete = $this->modx->getCollection(msGridField::class, [ - 'grid_key' => $gridKey, - 'field_name:NOT IN' => $fieldNamesToKeep, - 'is_system' => false, // Protect system fields - ]); - - foreach ($fieldsToDelete as $field) { + foreach ($this->repository->findNonSystemNotIn($gridKey, $fieldNamesToKeep) as $field) { $fieldName = $field->get('field_name'); - $this->modx->log(modX::LOG_LEVEL_INFO, - "[GridConfigService] Deleting field: {$gridKey}.{$fieldName}"); - - if (!$field->remove()) { - $this->modx->log(modX::LOG_LEVEL_ERROR, - "[GridConfigService] Failed to delete field: {$gridKey}.{$fieldName}"); + $this->modx->log(modX::LOG_LEVEL_INFO, "[GridConfigService] Deleting field: {$gridKey}.{$fieldName}"); + if (!$this->repository->remove($field)) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + "[GridConfigService] Failed to delete field: {$gridKey}.{$fieldName}" + ); } } return true; } catch (\Exception $e) { - $this->modx->log(modX::LOG_LEVEL_ERROR, - "[GridConfigService] Error saving grid config: " . $e->getMessage()); + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[GridConfigService] Error saving grid config: ' . $e->getMessage() + ); + return false; } } /** - * Delete field from grid configuration - * - * @param string $gridKey - * @param string $fieldName - * @return array + * @param array $fieldData + */ + private function applySavePayload(msGridField $field, array $fieldData, int $index): void + { + if (isset($fieldData['label'])) { + $field->set('label', $fieldData['label']); + } + if (isset($fieldData['visible'])) { + $field->set('visible', (bool) $fieldData['visible']); + } + $field->set('sort_order', $index); + if (isset($fieldData['sortable'])) { + $field->set('sortable', (bool) $fieldData['sortable']); + } + if (isset($fieldData['filterable'])) { + $field->set('filterable', (bool) $fieldData['filterable']); + } + if (isset($fieldData['frozen'])) { + $field->set('frozen', (bool) $fieldData['frozen']); + } + if (isset($fieldData['width'])) { + $field->set('width', $fieldData['width']); + } + if (isset($fieldData['minWidth'])) { + $field->set('min_width', $fieldData['minWidth']); + } + } + + /** + * @return array{success: bool, message: string} */ public function deleteField(string $gridKey, string $fieldName): array { try { - $field = $this->modx->getObject(msGridField::class, [ - 'grid_key' => $gridKey, - 'field_name' => $fieldName, - ]); - + $field = $this->repository->findOne($gridKey, $fieldName); if (!$field) { return [ 'success' => false, - 'message' => "Field not found: {$gridKey}.{$fieldName}" + 'message' => "Field not found: {$gridKey}.{$fieldName}", ]; } - // Protect system fields if ($field->get('is_system')) { return [ 'success' => false, - 'message' => 'Cannot delete system field' + 'message' => 'Cannot delete system field', ]; } - if ($field->remove()) { - $this->modx->log(modX::LOG_LEVEL_INFO, - "[GridConfigService] Field deleted: {$gridKey}.{$fieldName}"); + if ($this->repository->remove($field)) { + $this->modx->log( + modX::LOG_LEVEL_INFO, + "[GridConfigService] Field deleted: {$gridKey}.{$fieldName}" + ); return [ 'success' => true, - 'message' => 'Field deleted successfully' - ]; - } else { - return [ - 'success' => false, - 'message' => 'Failed to delete field from database' + 'message' => 'Field deleted successfully', ]; } + + return [ + 'success' => false, + 'message' => 'Failed to delete field from database', + ]; } catch (\Exception $e) { - $this->modx->log(modX::LOG_LEVEL_ERROR, - "[GridConfigService] Error deleting field: " . $e->getMessage()); + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[GridConfigService] Error deleting field: ' . $e->getMessage() + ); return [ 'success' => false, - 'message' => 'Error deleting field: ' . $e->getMessage() + 'message' => 'Error deleting field: ' . $e->getMessage(), ]; } } /** - * Add new field to grid configuration - * - * @param string $gridKey - * @param array $data - * @return array + * @param array $data + * @return array{success: bool, message: string, field?: array} */ public function addField(string $gridKey, array $data): array { try { - // Basic validation if (empty($data['field_name'])) { return ['success' => false, 'message' => 'field_name is required']; } if (!preg_match('/^[a-z0-9_]+$/i', $data['field_name'])) { - return ['success' => false, 'message' => 'field_name must contain only letters, numbers and underscores']; + return [ + 'success' => false, + 'message' => 'field_name must contain only letters, numbers and underscores', + ]; } - // Check uniqueness - $exists = $this->modx->getObject(msGridField::class, [ - 'grid_key' => $gridKey, - 'field_name' => $data['field_name'], - ]); - - if ($exists) { + if ($this->repository->findOne($gridKey, $data['field_name'])) { return ['success' => false, 'message' => 'Field with this name already exists']; } - // Validation by type - $type = $data['type'] ?? 'model'; - $config = $data['config'] ?? []; - - switch ($type) { - case 'template': - $validation = $this->validateTemplateConfig($config); - if (!$validation['success']) { - return $validation; - } - break; - - case 'relation': - $validation = $this->validateRelationConfig($config); - if (!$validation['success']) { - return $validation; - } - // Use updated config with resolvedTableName - if (isset($validation['config'])) { - $config = $validation['config']; - } - break; - - case 'computed': - $validation = $this->validateComputedConfig($config); - if (!$validation['success']) { - return $validation; - } - break; - - case 'actions': - $validation = $this->validateActionsConfig($config); - if (!$validation['success']) { - return $validation; - } - break; - - case 'option': - $validation = $this->validateOptionConfig( - $config, - (string) ($data['field_name'] ?? '') - ); - if (!$validation['success']) { - return $validation; - } - break; - } - - // Add type to config - $config['type'] = $type; - - if (($config['editor_type'] ?? '') !== GridColumnEditorType::COMBO) { - unset($config['editor_reference'], $config['editor_combo_endpoint']); - } - - $comboCheck = GridEditorReferenceRegistry::validateComboEditorConfig($config); - if (!$comboCheck['success']) { - return ['success' => false, 'message' => $comboCheck['message'] ?? 'Invalid combo editor configuration']; - } - - // Get maximum sort_order - $maxSortOrder = 0; - $query = $this->modx->newQuery(msGridField::class); - $query->where(['grid_key' => $gridKey]); - $query->sortby('sort_order', 'DESC'); - $query->limit(1); - $lastField = $this->modx->getObject(msGridField::class, $query); - if ($lastField) { - $maxSortOrder = (int)$lastField->get('sort_order'); + $prepared = $this->prepareTypedConfig( + (string) ($data['type'] ?? 'model'), + is_array($data['config'] ?? null) ? $data['config'] : [], + (string) $data['field_name'] + ); + if (!$prepared['success']) { + return $prepared; } - // Create field - $field = $this->modx->newObject(msGridField::class); - $field->fromArray([ + $field = $this->repository->create([ 'grid_key' => $gridKey, 'field_name' => $data['field_name'], 'label' => $data['label'] ?? $data['field_name'], @@ -411,300 +297,121 @@ public function addField(string $gridKey, array $data): array 'frozen' => $data['frozen'] ?? false, 'width' => $data['width'] ?? null, 'min_width' => $data['min_width'] ?? null, - 'sort_order' => $maxSortOrder + 1, + 'sort_order' => $this->repository->nextSortOrder($gridKey), 'is_system' => false, - 'config' => json_encode($config, JSON_UNESCAPED_UNICODE), + 'config' => $this->repository->encodeConfig($prepared['config']), ]); - if ($field->save()) { - $this->modx->log(modX::LOG_LEVEL_INFO, - "[GridConfigService] Field created: {$gridKey}.{$data['field_name']}"); + if ($this->repository->save($field)) { + $this->modx->log( + modX::LOG_LEVEL_INFO, + "[GridConfigService] Field created: {$gridKey}.{$data['field_name']}" + ); return [ 'success' => true, 'message' => 'Field created successfully', - 'field' => $field->toArray() + 'field' => $field->toArray(), ]; - } else { - return ['success' => false, 'message' => 'Failed to save field']; } + return ['success' => false, 'message' => 'Failed to save field']; } catch (\Exception $e) { - $this->modx->log(modX::LOG_LEVEL_ERROR, - "[GridConfigService] Error adding field: " . $e->getMessage()); + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[GridConfigService] Error adding field: ' . $e->getMessage() + ); return [ 'success' => false, - 'message' => 'Error adding field: ' . $e->getMessage() + 'message' => 'Error adding field: ' . $e->getMessage(), ]; } } /** - * Update existing field in grid configuration - * - * @param string $gridKey - * @param string $fieldName - * @param array $data - * @return array + * @param array $data + * @return array{success: bool, message: string, field?: array} */ public function updateField(string $gridKey, string $fieldName, array $data): array { try { - // Find existing field - $field = $this->modx->getObject(msGridField::class, [ - 'grid_key' => $gridKey, - 'field_name' => $fieldName, - ]); - + $field = $this->repository->findOne($gridKey, $fieldName); if (!$field) { return ['success' => false, 'message' => "Field not found: {$gridKey}.{$fieldName}"]; } - // Protect system fields from type changes if ($field->get('is_system')) { return ['success' => false, 'message' => 'Cannot modify system field']; } - // Get field type - $type = $data['type'] ?? 'model'; - $config = $data['config'] ?? []; - - // Validation by type - switch ($type) { - case 'template': - $validation = $this->validateTemplateConfig($config); - if (!$validation['success']) { - return $validation; - } - break; - case 'relation': - $validation = $this->validateRelationConfig($config); - if (!$validation['success']) { - return $validation; - } - // Use updated config with resolvedTableName - if (isset($validation['config'])) { - $config = $validation['config']; - } - break; - case 'computed': - $validation = $this->validateComputedConfig($config); - if (!$validation['success']) { - return $validation; - } - break; - case 'actions': - $validation = $this->validateActionsConfig($config); - if (!$validation['success']) { - return $validation; - } - break; - case 'option': - $validation = $this->validateOptionConfig( - $config, - (string) ($data['field_name'] ?? $fieldName) - ); - if (!$validation['success']) { - return $validation; - } - break; - } - - // Add type to config - $config['type'] = $type; - - if (($config['editor_type'] ?? '') !== GridColumnEditorType::COMBO) { - unset($config['editor_reference'], $config['editor_combo_endpoint']); + $prepared = $this->prepareTypedConfig( + (string) ($data['type'] ?? 'model'), + is_array($data['config'] ?? null) ? $data['config'] : [], + (string) ($data['field_name'] ?? $fieldName) + ); + if (!$prepared['success']) { + return $prepared; } - $comboCheck = GridEditorReferenceRegistry::validateComboEditorConfig($config); - if (!$comboCheck['success']) { - return ['success' => false, 'message' => $comboCheck['message'] ?? 'Invalid combo editor configuration']; - } - - // Update field if (isset($data['label'])) { $field->set('label', $data['label']); } if (isset($data['visible'])) { - $field->set('visible', (bool)$data['visible']); + $field->set('visible', (bool) $data['visible']); } if (isset($data['sortable'])) { - $field->set('sortable', (bool)$data['sortable']); + $field->set('sortable', (bool) $data['sortable']); } if (isset($data['filterable'])) { - $field->set('filterable', (bool)$data['filterable']); + $field->set('filterable', (bool) $data['filterable']); } if (isset($data['frozen'])) { - $field->set('frozen', (bool)$data['frozen']); + $field->set('frozen', (bool) $data['frozen']); } if (isset($data['width'])) { $field->set('width', $data['width'] ?: null); } - // Update JSON config - $field->set('config', json_encode($config, JSON_UNESCAPED_UNICODE)); + $field->set('config', $this->repository->encodeConfig($prepared['config'])); - if ($field->save()) { - $this->modx->log(modX::LOG_LEVEL_INFO, - "[GridConfigService] Field updated: {$gridKey}.{$fieldName}"); + if ($this->repository->save($field)) { + $this->modx->log( + modX::LOG_LEVEL_INFO, + "[GridConfigService] Field updated: {$gridKey}.{$fieldName}" + ); return [ 'success' => true, 'message' => 'Field updated successfully', - 'field' => $field->toArray() + 'field' => $field->toArray(), ]; - } else { - return ['success' => false, 'message' => 'Failed to save field']; } + return ['success' => false, 'message' => 'Failed to save field']; } catch (\Exception $e) { - $this->modx->log(modX::LOG_LEVEL_ERROR, - "[GridConfigService] Error updating field: " . $e->getMessage()); + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[GridConfigService] Error updating field: ' . $e->getMessage() + ); return [ 'success' => false, - 'message' => 'Error updating field: ' . $e->getMessage() + 'message' => 'Error updating field: ' . $e->getMessage(), ]; } } /** - * Validate Template field configuration - * - * @param array $config - * @return array - */ - protected function validateTemplateConfig(array $config): array - { - if (empty($config['template'])) { - return ['success' => false, 'message' => 'template is required for template field']; - } - - if (!is_string($config['template'])) { - return ['success' => false, 'message' => 'template must be a string']; - } - - return ['success' => true]; - } - - /** - * Validate Relation field configuration - * - * @param array $config - * @return array - */ - protected function validateRelationConfig(array $config): array - { - $relation = $config['relation'] ?? []; - - if (empty($relation['table'])) { - return ['success' => false, 'message' => 'relation.table is required']; - } - - if (empty($relation['foreignKey'])) { - return ['success' => false, 'message' => 'relation.foreignKey is required']; - } - - if (empty($relation['displayField'])) { - return ['success' => false, 'message' => 'relation.displayField is required']; - } - - // Validate aggregation - $aggregation = $relation['aggregation'] ?? null; - $allowedAggregations = ['COUNT', 'SUM', 'AVG', 'MIN', 'MAX']; - - if ($aggregation !== null && !in_array($aggregation, $allowedAggregations)) { - return ['success' => false, 'message' => "Invalid aggregation type. Allowed: " . implode(', ', $allowedAggregations)]; - } - - // Determine type: model or direct table name - $tableOrModel = $relation['table']; - $isModel = strpos($tableOrModel, '\\') !== false || strpos($tableOrModel, '::') !== false; - - if ($isModel) { - // This is model class - get table name - try { - $tableName = $this->modx->getTableName($tableOrModel); - - if (empty($tableName)) { - return ['success' => false, 'message' => "Unable to get table name for model: {$tableOrModel}"]; - } - - // Save resolved table name in config for query usage - $config['relation']['resolvedTableName'] = $tableName; - } catch (\Exception $e) { - return ['success' => false, 'message' => "Invalid model class: {$tableOrModel}. " . $e->getMessage()]; - } - } else { - // This is direct table name - add table prefix if not already present - $tablePrefix = $this->modx->config['table_prefix'] ?? ''; - if ($tablePrefix !== '' && !str_starts_with($tableOrModel, $tablePrefix)) { - $tableOrModel = $tablePrefix . $tableOrModel; - } - $config['relation']['resolvedTableName'] = $tableOrModel; - } - - return ['success' => true, 'config' => $config]; - } - - /** - * Validate Computed field configuration - * - * @param array $config - * @return array - */ - protected function validateComputedConfig(array $config): array - { - $computed = $config['computed'] ?? []; - - if (empty($computed['className'])) { - return ['success' => false, 'message' => 'computed.className is required']; - } - - // Check that class exists - if (!class_exists($computed['className'])) { - return ['success' => false, 'message' => "Class {$computed['className']} not found"]; - } - - // Check that class implements required interface - $interfaces = class_implements($computed['className']); - if (!isset($interfaces['MiniShop3\\Interfaces\\ComputedFieldInterface'])) { - return ['success' => false, 'message' => "Class must implement ComputedFieldInterface"]; - } - - return ['success' => true]; - } - - /** - * Get filterable fields from grid configuration - * - * @param string $gridKey Grid key - * @return array Array of filterable field configs + * @return array}> */ public function getFilterableFields(string $gridKey): array { - $query = $this->modx->newQuery(msGridField::class); - $query->where([ - 'grid_key' => $gridKey, - 'filterable' => true, - ]); - $query->sortby('sort_order', 'ASC'); - $filters = []; - $collection = $this->modx->getCollection(msGridField::class, $query); - foreach ($collection as $field) { + foreach ($this->repository->findFilterableByGridKey($gridKey) as $field) { $fieldName = $field->get('field_name'); - $config = $field->get('config'); - - // Parse JSON config - if (is_string($config)) { - $config = json_decode($config, true) ?: []; - } elseif (!is_array($config)) { - $config = []; - } + $config = $this->repository->decodeConfig($field->get('config')); $filters[$fieldName] = [ 'field_name' => $fieldName, @@ -718,188 +425,53 @@ public function getFilterableFields(string $gridKey): array } /** - * Validate Actions field configuration - * - * @param array $config - * @return array + * @param list> $gridFields + * @return array */ - protected function validateActionsConfig(array $config): array + public function extractRelationFields(array $gridFields): array { - $actions = $config['actions'] ?? []; - - // Actions can be empty array - this is allowed - if (!is_array($actions)) { - return ['success' => false, 'message' => 'actions must be an array']; - } - - // Built-in Vue manager handlers (see vueManager/src/actionRegistry.js) - $allowedHandlers = ['edit', 'delete', 'view', 'refresh', 'addresses', 'publish', 'duplicate']; - $actionNames = []; - - foreach ($actions as $index => $action) { - // Required field name - if (empty($action['name'])) { - return ['success' => false, 'message' => "actions[{$index}].name is required"]; - } - - // Check name uniqueness - if (in_array($action['name'], $actionNames)) { - return ['success' => false, 'message' => "Duplicate action name: {$action['name']}"]; - } - $actionNames[] = $action['name']; - - // Required field handler - if (empty($action['handler'])) { - return ['success' => false, 'message' => "actions[{$index}].handler is required"]; - } - - // Handler must be either built-in or start with 'custom:' - $handler = $action['handler']; - if (!in_array($handler, $allowedHandlers) && strpos($handler, 'custom:') !== 0) { - // Allow any handlers - they can be registered by plugins - // But log warning - $this->modx->log(modX::LOG_LEVEL_INFO, - "[GridConfigService] Custom handler used: {$handler} in action {$action['name']}"); - } - - // Validate severity if specified - if (!empty($action['severity'])) { - $allowedSeverities = ['secondary', 'success', 'info', 'warn', 'danger']; - if (!in_array($action['severity'], $allowedSeverities)) { - return ['success' => false, 'message' => "Invalid severity for action {$action['name']}. Allowed: " . implode(', ', $allowedSeverities)]; - } - } - } - - return ['success' => true]; + return $this->relationExtractor->extract($gridFields); } /** - * Validate Option field configuration + * Type rules + combo editor cleanup for add/update payloads. * - * @param array $config - * @return array + * @param array $config + * @return array{success: bool, message?: string, config?: array} */ - protected function validateOptionConfig(array $config, string $fieldName = ''): array + private function prepareTypedConfig(string $type, array $config, string $fieldName): array { - $option = $config['option'] ?? []; - - if (empty($option['key'])) { - return ['success' => false, 'message' => 'option.key is required for option field']; + $validation = $this->typeValidator->validateForType($type, $config, $fieldName); + if (!$validation['success']) { + return $validation; } - - $key = (string) $option['key']; - if (!OptionColumnSpec::isValidOptionKey($key)) { - return ['success' => false, 'message' => 'option.key must contain only letters, numbers and underscores']; + if (isset($validation['config'])) { + $config = $validation['config']; } - if ($fieldName !== '' && !OptionColumnSpec::isValidFieldName($fieldName)) { - return [ - 'success' => false, - 'message' => "Field name '{$fieldName}' is not allowed for option columns: " - . "it collides with a builtin product column or contains invalid characters. " - . "Use a name like 'option_{$key}' instead.", - ]; - } + $config['type'] = $type; - return ['success' => true]; + return $this->normalizeComboEditorConfig($config); } /** - * Extract relation fields from grid config and group by table+foreignKey - * for efficient JOIN building - * - * @param array $gridFields Array of grid field configs - * @return array Grouped relation fields structure: - * [ - * 'table_foreignKey' => [ - * 'table' => 'msOrderStatus', - * 'modelClass' => 'MiniShop3\Model\msOrderStatus', - * 'foreignKey' => 'status_id', - * 'alias' => 'rel_msOrderStatus_status_id', - * 'fields' => [ - * ['name' => 'status_name', 'displayField' => 'name'], - * ['name' => 'status_color', 'displayField' => 'color'], - * ] - * ] - * ] + * @param array $config + * @return array{success: bool, message?: string, config?: array} */ - public function extractRelationFields(array $gridFields): array + private function normalizeComboEditorConfig(array $config): array { - $relationGroups = []; - - foreach ($gridFields as $field) { - // Skip non-relation fields - if (($field['type'] ?? 'model') !== 'relation') { - continue; - } - - $relation = $field['relation'] ?? null; - if (!$relation || empty($relation['table']) || empty($relation['foreignKey']) || empty($relation['displayField'])) { - continue; - } - - $table = $relation['table']; - $foreignKey = $relation['foreignKey']; - $displayField = $relation['displayField']; - $fieldName = $field['name']; - - // Group key: table + foreignKey (same table with same FK = one JOIN) - $groupKey = "{$table}_{$foreignKey}"; - - if (!isset($relationGroups[$groupKey])) { - // Resolve model class from table name - $modelClass = $this->resolveModelClass($table); - - // Generate unique alias for this JOIN - $alias = "rel_{$table}_{$foreignKey}"; - - $relationGroups[$groupKey] = [ - 'table' => $table, - 'modelClass' => $modelClass, - 'foreignKey' => $foreignKey, - 'alias' => $alias, - 'fields' => [], - ]; - } + if (($config['editor_type'] ?? '') !== GridColumnEditorType::COMBO) { + unset($config['editor_reference'], $config['editor_combo_endpoint']); + } - // Add field to this group - $relationGroups[$groupKey]['fields'][] = [ - 'name' => $fieldName, - 'displayField' => $displayField, + $comboCheck = GridEditorReferenceRegistry::validateComboEditorConfig($config); + if (!$comboCheck['success']) { + return [ + 'success' => false, + 'message' => $comboCheck['message'] ?? 'Invalid combo editor configuration', ]; } - return $relationGroups; - } - - /** - * Resolve full model class name from short table name - * - * @param string $tableName Short table name (e.g. 'msOrderStatus') - * @return string|null Full class name or null if not found - */ - protected function resolveModelClass(string $tableName): ?string - { - // Known MiniShop3 model mappings - $modelMap = [ - 'msOrder' => 'MiniShop3\\Model\\msOrder', - 'msOrderStatus' => 'MiniShop3\\Model\\msOrderStatus', - 'msOrderAddress' => 'MiniShop3\\Model\\msOrderAddress', - 'msOrderProduct' => 'MiniShop3\\Model\\msOrderProduct', - 'msDelivery' => 'MiniShop3\\Model\\msDelivery', - 'msPayment' => 'MiniShop3\\Model\\msPayment', - 'msProduct' => 'MiniShop3\\Model\\msProduct', - 'msProductData' => 'MiniShop3\\Model\\msProductData', - 'msCategory' => 'MiniShop3\\Model\\msCategory', - 'msCategoryMember' => 'MiniShop3\\Model\\msCategoryMember', - 'msVendor' => 'MiniShop3\\Model\\msVendor', - 'msCustomer' => 'MiniShop3\\Model\\msCustomer', - 'modUser' => 'MODX\\Revolution\\modUser', - 'modUserProfile' => 'MODX\\Revolution\\modUserProfile', - 'modResource' => 'MODX\\Revolution\\modResource', - ]; - - return $modelMap[$tableName] ?? null; + return ['success' => true, 'config' => $config]; } } diff --git a/core/components/minishop3/tests/GridColumnTypeValidatorTest.php b/core/components/minishop3/tests/GridColumnTypeValidatorTest.php new file mode 100644 index 000000000..c881f89ec --- /dev/null +++ b/core/components/minishop3/tests/GridColumnTypeValidatorTest.php @@ -0,0 +1,92 @@ +validateForType('template', []); +$assertTrue(($failTemplate['success'] ?? true) === false, 'template empty fails'); + +$okTemplate = $validator->validateForType('template', ['template' => '{{name}}']); +$assertTrue(($okTemplate['success'] ?? false) === true, 'template string ok'); + +$failOption = $validator->validateForType('option', []); +$assertTrue(($failOption['success'] ?? true) === false, 'option empty fails'); + +$okOption = $validator->validateForType('option', ['option' => ['key' => 'color']], 'option_color'); +$assertTrue(($okOption['success'] ?? false) === true, 'option ok'); + +$collision = $validator->validateForType('option', ['option' => ['key' => 'color']], 'price'); +$assertTrue(($collision['success'] ?? true) === false, 'option field name collision'); + +$dupActions = $validator->validateForType('actions', [ + 'actions' => [ + ['name' => 'edit', 'handler' => 'edit'], + ['name' => 'edit', 'handler' => 'delete'], + ], +]); +$assertTrue(($dupActions['success'] ?? true) === false, 'duplicate action name fails'); + +$okActions = $validator->validateForType('actions', [ + 'actions' => [ + ['name' => 'edit', 'handler' => 'edit'], + ['name' => 'delete', 'handler' => 'delete'], + ], +]); +$assertTrue(($okActions['success'] ?? false) === true, 'actions ok'); + +$modelOk = $validator->validateForType('model', []); +$assertTrue(($modelOk['success'] ?? false) === true, 'model type always ok'); + +$failRelation = $validator->validateForType('relation', []); +$assertTrue(($failRelation['success'] ?? true) === false, 'relation empty fails'); + +$okRelation = $validator->validateForType('relation', [ + 'relation' => [ + 'table' => 'ms3_orders', + 'foreignKey' => 'id', + 'displayField' => 'num', + ], +]); +$assertTrue(($okRelation['success'] ?? false) === true, 'relation table ok'); +$assertTrue( + ($okRelation['config']['relation']['resolvedTableName'] ?? '') === 'modx_ms3_orders', + 'relation resolvedTableName gets table_prefix' +); + +$failAgg = $validator->validateForType('relation', [ + 'relation' => [ + 'table' => 'ms3_orders', + 'foreignKey' => 'id', + 'displayField' => 'num', + 'aggregation' => 'MEDIAN', + ], +]); +$assertTrue(($failAgg['success'] ?? true) === false, 'invalid aggregation fails'); + +fwrite(STDOUT, "OK: GridColumnTypeValidatorTest\n"); +exit(0); diff --git a/core/components/minishop3/tests/Unit/Services/Grid/GridConfigRepositoryEmptyKeepTest.php b/core/components/minishop3/tests/Unit/Services/Grid/GridConfigRepositoryEmptyKeepTest.php new file mode 100644 index 000000000..8f5b1b1b2 --- /dev/null +++ b/core/components/minishop3/tests/Unit/Services/Grid/GridConfigRepositoryEmptyKeepTest.php @@ -0,0 +1,36 @@ +findNonSystemNotIn('orders', [])); + } +} diff --git a/core/components/minishop3/tests/Unit/Services/Grid/GridRelationFieldExtractorTest.php b/core/components/minishop3/tests/Unit/Services/Grid/GridRelationFieldExtractorTest.php new file mode 100644 index 000000000..5815fc73e --- /dev/null +++ b/core/components/minishop3/tests/Unit/Services/Grid/GridRelationFieldExtractorTest.php @@ -0,0 +1,60 @@ +extract([ + [ + 'name' => 'status_name', + 'type' => 'relation', + 'relation' => [ + 'table' => 'msOrderStatus', + 'foreignKey' => 'status_id', + 'displayField' => 'name', + ], + ], + [ + 'name' => 'status_color', + 'type' => 'relation', + 'relation' => [ + 'table' => 'msOrderStatus', + 'foreignKey' => 'status_id', + 'displayField' => 'color', + ], + ], + [ + 'name' => 'pagetitle', + 'type' => 'model', + ], + ]); + + self::assertCount(1, $groups); + $group = $groups['msOrderStatus_status_id']; + self::assertSame('MiniShop3\\Model\\msOrderStatus', $group['modelClass']); + self::assertCount(2, $group['fields']); + self::assertSame('rel_msOrderStatus_status_id', $group['alias']); + } + + public function testSkipsIncompleteRelation(): void + { + $extractor = new GridRelationFieldExtractor(); + $groups = $extractor->extract([ + [ + 'name' => 'broken', + 'type' => 'relation', + 'relation' => ['table' => 'msVendor'], + ], + ]); + + self::assertSame([], $groups); + } +} diff --git a/core/components/minishop3/tests/stubs/GridValidatorModxStub.php b/core/components/minishop3/tests/stubs/GridValidatorModxStub.php new file mode 100644 index 000000000..773184793 --- /dev/null +++ b/core/components/minishop3/tests/stubs/GridValidatorModxStub.php @@ -0,0 +1,29 @@ + */ + public $config = ['table_prefix' => 'modx_']; + + public function __construct() + { + } + + public function log($level, $msg = '', $target = '', $def = '', $file = '', $line = ''): void + { + } + + public function getTableName($className, $escape = true): string + { + return ''; + } +} From 0389c767fd5876480153b423767fb2d8133fa090 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Mon, 10 Aug 2026 17:20:05 +0600 Subject: [PATCH 2/2] fix(grid): assign collection before @var for PHPStan PHPStan rejects @var on a return expression where $fields does not exist. --- .../minishop3/src/Services/Grid/GridConfigRepository.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/components/minishop3/src/Services/Grid/GridConfigRepository.php b/core/components/minishop3/src/Services/Grid/GridConfigRepository.php index 968d7e07f..2982a3566 100644 --- a/core/components/minishop3/src/Services/Grid/GridConfigRepository.php +++ b/core/components/minishop3/src/Services/Grid/GridConfigRepository.php @@ -100,11 +100,13 @@ public function findNonSystemNotIn(string $gridKey, array $fieldNamesToKeep): ar } /** @var list $fields */ - return array_values($this->modx->getCollection(msGridField::class, [ + $fields = array_values($this->modx->getCollection(msGridField::class, [ 'grid_key' => $gridKey, 'is_system' => false, 'field_name:NOT IN' => $fieldNamesToKeep, ]) ?: []); + + return $fields; } /** @@ -140,6 +142,8 @@ private function collectSorted(array $where): array $query->sortby('sort_order', 'ASC'); /** @var list $fields */ - return array_values($this->modx->getCollection(msGridField::class, $query) ?: []); + $fields = array_values($this->modx->getCollection(msGridField::class, $query) ?: []); + + return $fields; } }