fix(extra-fields): не ALTER site_content, валидация key (#645)

core/components/minishop3/lexicon/en/default.inc.php

@@ -169,6 +169,8 @@
 $_lang['ms3_err_ns'] = 'This field is required';
 $_lang['ms3_err_product_key_required'] = 'Product key is required';
 $_lang['ms3_err_field_key_required'] = 'Field key is required';
+$_lang['ms3_err_extra_field_class_unsupported'] = 'This model class cannot host extra fields. Use a model with its own database table (e.g. msProductData).';
+$_lang['ms3_err_extra_field_key_invalid'] = 'Field key must contain only Latin letters, digits, and underscores.';
 $_lang['ms3_err_fields_required'] = 'Fields array is required';
 $_lang['ms3_err_field_nf'] = 'Field not found';
 $_lang['ms3_err_ae'] = 'This field must be unique';

core/components/minishop3/lexicon/en/vue.inc.php

@@ -297,6 +297,7 @@
 $_lang['ms3_vue_error_loading_fields'] = 'Failed to load fields list';
 $_lang['ms3_vue_validation'] = 'Validation';
 $_lang['ms3_vue_validation_key_required'] = 'Please specify field name (key)';
+$_lang['ms3_vue_validation_key_invalid'] = 'Field key must contain only Latin letters, digits, and underscores';
 $_lang['ms3_vue_validation_dbtype_required'] = 'Please specify database type (dbtype)';
 $_lang['ms3_vue_field_created'] = 'created';
 $_lang['ms3_vue_error_creating'] = 'Creation Error';

core/components/minishop3/lexicon/ru/default.inc.php

@@ -169,6 +169,8 @@
 $_lang['ms3_err_ns'] = 'Это поле обязательно';
 $_lang['ms3_err_product_key_required'] = 'Не указан ключ товара';
 $_lang['ms3_err_field_key_required'] = 'Не указан ключ поля';
+$_lang['ms3_err_extra_field_class_unsupported'] = 'Этот класс модели не может содержать дополнительные поля. Используйте модель с собственной таблицей БД (например, msProductData).';
+$_lang['ms3_err_extra_field_key_invalid'] = 'Ключ поля может содержать только латинские буквы, цифры и подчёркивание.';
 $_lang['ms3_err_fields_required'] = 'Требуется массив полей';
 $_lang['ms3_err_field_nf'] = 'Поле не найдено';
 $_lang['ms3_err_ae'] = 'Это поле должно быть уникально';

core/components/minishop3/lexicon/ru/vue.inc.php

@@ -297,6 +297,7 @@
 $_lang['ms3_vue_error_loading_fields'] = 'Не удалось загрузить список полей';
 $_lang['ms3_vue_validation'] = 'Валидация';
 $_lang['ms3_vue_validation_key_required'] = 'Укажите имя поля (key)';
+$_lang['ms3_vue_validation_key_invalid'] = 'Ключ поля может содержать только латинские буквы, цифры и подчёркивание';
 $_lang['ms3_vue_validation_dbtype_required'] = 'Укажите тип данных БД (dbtype)';
 $_lang['ms3_vue_field_created'] = 'создано';
 $_lang['ms3_vue_error_creating'] = 'Ошибка создания';

core/components/minishop3/src/Services/ExtraFieldsService.php

@@ -6,6 +6,7 @@
 use MiniShop3\Model\msProductField;
 use MiniShop3\Services\ExtraFields\KeyValueFieldService;
 use MiniShop3\Services\ExtraFields\RepeaterFieldService;
+use MiniShop3\Services\Grid\GridColumnRules;
 use MiniShop3\Utils\ExtraFields;
 use MODX\Revolution\modX;
 use Phinx\Config\Config;
@@ -103,27 +104,41 @@ public function deleteField(int $id): array
             return ['success' => false, 'message' => 'Field not found'];
         }
 
-        try {
-            $migrationFile = $this->migrationGenerator->generateDropColumnMigration($field);
-        } catch (\Exception $e) {
-            return ['success' => false, 'message' => 'Migration generation error: ' . $e->getMessage()];
-        }
+        $class = (string) $field->get('class');
+        $migrationName = null;
+        $migrationOutput = '';
 
-        $migrationResult = $this->runMigrations();
+        // STI / unsupported hosts (e.g. legacy msProduct): metadata only — never ALTER site_content.
+        if ($this->migrationGenerator->canHostExtraField($class)) {
+            try {
+                $migrationFile = $this->migrationGenerator->generateDropColumnMigration($field);
+            } catch (\Exception $e) {
+                return ['success' => false, 'message' => 'Migration generation error: ' . $e->getMessage()];
+            }
 
-        if (!$migrationResult['success']) {
+            $migrationResult = $this->runMigrations();
+
+            if (!$migrationResult['success']) {
+                @unlink($migrationFile);
+                return $migrationResult;
+            }
+
+            $migrationName = basename($migrationFile);
+            $migrationOutput = $migrationResult['output'] ?? '';
             @unlink($migrationFile);
-            return $migrationResult;
+            $this->modx->log(
+                modX::LOG_LEVEL_INFO,
+                "[ExtraFieldsService] Migration file deleted: {$migrationName}"
+            );
+        } else {
+            $this->modx->log(
+                modX::LOG_LEVEL_INFO,
+                "[ExtraFieldsService] Skipping drop migration for unsupported class {$class}; removing metadata only"
+            );
         }
 
-        @unlink($migrationFile);
-        $this->modx->log(
-            modX::LOG_LEVEL_INFO,
-            "[ExtraFieldsService] Migration file deleted: " . basename($migrationFile)
-        );
-
         // Only delete msProductField for product-related models
-        if ($field->get('class') === 'MiniShop3\\Model\\msProductData') {
+        if ($class === 'MiniShop3\\Model\\msProductData') {
             $this->deleteProductFieldsByName($field->get('key'));
         }
 
@@ -134,8 +149,8 @@ public function deleteField(int $id): array
         return [
             'success' => true,
             'message' => 'Field deleted successfully',
-            'migration' => basename($migrationFile),
-            'output' => $migrationResult['output'] ?? ''
+            'migration' => $migrationName,
+            'output' => $migrationOutput,
         ];
     }
 
@@ -199,18 +214,36 @@ private function formatField(msExtraField $field): array
      */
     private function validateFieldData(array $data): array
     {
+        $this->modx->lexicon->load('minishop3:default');
+
         $required = ['class', 'key', 'dbtype', 'phptype'];
 
         foreach ($required as $fieldName) {
             if (empty($data[$fieldName])) {
                 return [
                     'success' => false,
-                    'message' => "Field '{$fieldName}' is required",
+                    'message' => $this->modx->lexicon('ms3_err_ns'),
                     'field' => $fieldName
                 ];
             }
         }
 
+        if (!$this->migrationGenerator->canHostExtraField((string) $data['class'])) {
+            return [
+                'success' => false,
+                'message' => $this->modx->lexicon('ms3_err_extra_field_class_unsupported'),
+                'field' => 'class',
+            ];
+        }
+
+        if (!GridColumnRules::isValidSqlIdentifier((string) $data['key'])) {
+            return [
+                'success' => false,
+                'message' => $this->modx->lexicon('ms3_err_extra_field_key_invalid'),
+                'field' => 'key',
+            ];
+        }
+
         $exists = $this->modx->getObject(msExtraField::class, [
             'class' => $data['class'],
             'key' => $data['key']

core/components/minishop3/src/Services/MigrationGenerator.php

@@ -13,7 +13,10 @@ class MigrationGenerator
     public function __construct(modX $modx)
     {
         $this->modx = $modx;
-        $this->migrationsPath = MODX_CORE_PATH . 'components/minishop3/migrations/';
+        // MODX_CORE_PATH is the site core/; fallback is this package root (…/components/minishop3).
+        $this->migrationsPath = defined('MODX_CORE_PATH')
+            ? rtrim((string) \MODX_CORE_PATH, '/\\') . '/components/minishop3/migrations/'
+            : dirname(__DIR__, 2) . '/migrations/';
     }
 
     /**
@@ -59,7 +62,7 @@ public function generateDropColumnMigration(msExtraField $field): string
      */
     private function renderAddColumnTemplate(string $className, msExtraField $field): string
     {
-        $tableName = $this->getTableName($field->get('class'));
+        $tableName = $this->resolveTableName($field->get('class'));
         $columnName = $field->get('key');
         $dbtype = $this->mapDbTypeToPhinx($field->get('dbtype'));
         $precision = $this->parsePrecision($field);
@@ -118,7 +121,7 @@ public function change(): void
      */
     private function renderDropColumnTemplate(string $className, msExtraField $field): string
     {
-        $tableName = $this->getTableName($field->get('class'));
+        $tableName = $this->resolveTableName($field->get('class'));
         $columnName = $field->get('key');
         $indexName = $field->hasIndex() ? $field->getIndexName() : '';
 
@@ -270,49 +273,63 @@ private function parseAttributes(msExtraField $field): string
     }
 
     /**
-     * Get table name from model class
+     * Whether the model class has its own DB table (not STI / modResource inheritance).
      */
-    private function getTableName(string $class): string
+    public function canHostExtraField(string $class): bool
     {
-        // MiniShop3\Model\msProductData → ms3_products
-        $tableMap = [
-            'MiniShop3\\Model\\msProductData' => 'ms3_products',
-            'MiniShop3\\Model\\msVendor' => 'ms3_vendors',
-            'MiniShop3\\Model\\msOrder' => 'ms3_orders',
-            'MiniShop3\\Model\\msCategory' => 'ms3_categories',
-            'MiniShop3\\Model\\msOrderProduct' => 'ms3_order_products',
-            'MiniShop3\\Model\\msOrderAddress' => 'ms3_order_addresses',
-        ];
+        return $this->ownTableName($class) !== null;
+    }
+
+    /**
+     * Unprefixed, unquoted table name for Phinx (table_prefix is applied in phinx.php).
+     */
+    public function resolveTableName(string $class): string
+    {
+        $table = $this->ownTableName($class);
+        if ($table === null) {
+            throw new \InvalidArgumentException("Model class cannot host extra fields: {$class}");
+        }
+
+        return $table;
+    }
 
-        if (isset($tableMap[$class])) {
-            return $tableMap[$class];
+    /**
+     * Own logical table from mysql metaMap, or null when the class inherits another table.
+     */
+    private function ownTableName(string $class): ?string
+    {
+        $mysqlClass = str_replace('\\Model\\', '\\Model\\mysql\\', $class);
+        if (!str_starts_with($mysqlClass, 'MiniShop3\\Model\\mysql\\') || !class_exists($mysqlClass)) {
+            return null;
+        }
+
+        $table = $mysqlClass::$metaMap['table'] ?? null;
+        if (!is_string($table) || $table === '') {
+            return null;
         }
 
-        // If class not in map, try to get from xPDO
-        $object = $this->modx->newObject($class);
-        if ($object) {
-            return $this->modx->getTableName($class);
+        $table = trim($table, '`');
+        if (str_contains($table, '.')) {
+            $parts = explode('.', $table);
+            $table = (string) end($parts);
         }
 
-        throw new \Exception("Cannot determine table name for class: {$class}");
+        if ($table === '' || str_contains($table, '`')) {
+            return null;
+        }
+
+        return $table;
     }
 
     /**
      * Get short table name for migration class
      */
     private function getTableShortName(string $class): string
     {
-        // MiniShop3\Model\msProductData → Products
-        $map = [
-            'MiniShop3\\Model\\msProductData' => 'Products',
-            'MiniShop3\\Model\\msVendor' => 'Vendors',
-            'MiniShop3\\Model\\msOrder' => 'Orders',
-            'MiniShop3\\Model\\msCategory' => 'Categories',
-            'MiniShop3\\Model\\msOrderProduct' => 'OrderProducts',
-            'MiniShop3\\Model\\msOrderAddress' => 'OrderAddresses',
-        ];
+        $table = $this->resolveTableName($class);
+        $base = preg_replace('/^ms3_/', '', $table) ?? $table;
 
-        return $map[$class] ?? 'Table';
+        return $this->toCamelCase($base);
     }
 
     private function toCamelCase(string $str): string

core/components/minishop3/tests/MigrationGeneratorTableResolveTest.php

@@ -0,0 +1,96 @@
+<?php
+
+/**
+ * Smoke: MigrationGenerator resolves unprefixed ms3_* table names from xPDO metaMap (#645).
+ *
+ * Run: php tests/MigrationGeneratorTableResolveTest.php
+ */
+
+declare(strict_types=1);
+
+require __DIR__ . '/../vendor/autoload.php';
+
+if (!defined('MODX_CORE_PATH')) {
+    define('MODX_CORE_PATH', dirname(__DIR__) . '/');
+}
+
+require_once __DIR__ . '/support/xpdo_om_stub.php';
+require_once __DIR__ . '/support/modresource_stub.php';
+
+use MiniShop3\Model\msCategory;
+use MiniShop3\Model\msProduct;
+use MiniShop3\Model\msProductData;
+use MiniShop3\Model\msVendor;
+use MiniShop3\Services\MigrationGenerator;
+use MODX\Revolution\modX;
+
+if (!class_exists(modX::class, false)) {
+    require __DIR__ . '/stubs/ModxStub.php';
+}
+
+$fail = static function (string $message): never {
+    fwrite(STDERR, "FAIL MigrationGeneratorTableResolveTest: {$message}\n");
+    exit(1);
+};
+
+$assertSame = static function ($expected, $actual, string $case) use ($fail): void {
+    if ($actual !== $expected) {
+        $fail($case . ': expected ' . var_export($expected, true) . ', got ' . var_export($actual, true));
+    }
+};
+
+$assertTrue = static function (bool $value, string $case) use ($fail): void {
+    if (!$value) {
+        $fail($case);
+    }
+};
+
+$assertFalse = static function (bool $value, string $case) use ($fail): void {
+    if ($value) {
+        $fail($case);
+    }
+};
+
+$modx = new class extends modX {
+    public function loadClass($className, $fqn = true, $path = '', $force = false)
+    {
+        return class_exists($className);
+    }
+
+    public function getOption($key, $options = null, $default = null)
+    {
+        if ($key === 'dbtype') {
+            return 'mysql';
+        }
+
+        return $default;
+    }
+
+    public function log($level, $msg, $target = '', $def = '', $file = '', $line = ''): void
+    {
+    }
+};
+
+$generator = new MigrationGenerator($modx);
+
+$productDataTable = $generator->resolveTableName(msProductData::class);
+$assertSame('ms3_products', $productDataTable, 'msProductData → ms3_products');
+$assertFalse(str_contains($productDataTable, '`'), 'table name must not contain backticks');
+$assertFalse(str_contains($productDataTable, 'site_content'), 'must not resolve to modResource table');
+
+$assertSame('ms3_vendors', $generator->resolveTableName(msVendor::class), 'msVendor → ms3_vendors');
+
+$assertTrue($generator->canHostExtraField(msProductData::class), 'msProductData can host extra fields');
+$assertFalse($generator->canHostExtraField(msProduct::class), 'msProduct must be rejected (modResource STI)');
+$assertFalse($generator->canHostExtraField(msCategory::class), 'msCategory must be rejected (modResource STI)');
+
+foreach ([msProduct::class, msCategory::class] as $unsupportedClass) {
+    try {
+        $generator->resolveTableName($unsupportedClass);
+        $fail('resolveTableName must throw for ' . $unsupportedClass);
+    } catch (\InvalidArgumentException) {
+        // expected
+    }
+}
+
+echo "OK MigrationGeneratorTableResolveTest\n";