From aec5e90411065df3269eaba9fe1cc460d52fe133 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 30 Jul 2026 09:47:53 +0600 Subject: [PATCH] refactor(mgr-api): extract extra-fields CRUD into ExtraFieldsController Move inline route closures to a Manager controller with DI-backed ExtraFieldsService; routes only dispatch. Adds getField() on the service. --- .../minishop3/config/routes/manager.php | 124 +----------- .../Api/Manager/ExtraFieldsController.php | 190 ++++++++++++++++++ .../minishop3/src/ServiceRegistry.php | 4 + .../src/Services/ExtraFieldsService.php | 39 +++- .../tests/ExtraFieldsControllerRoutesTest.php | 85 ++++++++ 5 files changed, 321 insertions(+), 121 deletions(-) create mode 100644 core/components/minishop3/src/Controllers/Api/Manager/ExtraFieldsController.php create mode 100644 core/components/minishop3/tests/ExtraFieldsControllerRoutesTest.php diff --git a/core/components/minishop3/config/routes/manager.php b/core/components/minishop3/config/routes/manager.php index 1b0f0ffe..018bbe47 100644 --- a/core/components/minishop3/config/routes/manager.php +++ b/core/components/minishop3/config/routes/manager.php @@ -174,128 +174,24 @@ $router->group('/extra-fields', function($router) use ($modx) { $router->get('', function($params) use ($modx) { - try { - /** @var \MiniShop3\Services\ExtraFieldsService $service */ - $service = new \MiniShop3\Services\ExtraFieldsService($modx); - - $class = $_GET['class'] ?? null; - $criteria = $class ? ['class' => $class] : []; - - $fields = $service->getFields($criteria); - - return Response::success([ - 'fields' => $fields, - 'total' => count($fields) - ]); - } catch (\Exception $e) { - $modx->log(\MODX\Revolution\modX::LOG_LEVEL_ERROR, '[ExtraFields API] ' . $e->getMessage()); - return Response::error('Failed to load extra fields: ' . $e->getMessage(), HttpStatus::INTERNAL_SERVER_ERROR); - } + $controller = new \MiniShop3\Controllers\Api\Manager\ExtraFieldsController($modx); + return $controller->getList(array_merge($_GET, $params)); }); $router->get('/{id}', function($params) use ($modx) { - $id = (int)($params['id'] ?? 0); - - if (!$id) { - return Response::error('Field ID is required', HttpStatus::BAD_REQUEST); - } - - $field = $modx->getObject(\MiniShop3\Model\msExtraField::class, $id); - - if (!$field) { - return Response::error('Field not found', HttpStatus::NOT_FOUND); - } - - $data = $field->toArray(); - - $extraFieldsUtil = new \MiniShop3\Utils\ExtraFields($modx); - $data['column_exists'] = $extraFieldsUtil->columnExists($field->get('class'), $field->get('key')); - - return Response::success(['field' => $data]); + $controller = new \MiniShop3\Controllers\Api\Manager\ExtraFieldsController($modx); + return $controller->get($params); }); $router->post('', function($params) use ($modx) { - try { - $data = json_decode(file_get_contents('php://input'), true); - - if (empty($data)) { - return Response::error('Request body is empty', HttpStatus::BAD_REQUEST); - } - - /** @var \MiniShop3\Services\ExtraFieldsService $service */ - $service = new \MiniShop3\Services\ExtraFieldsService($modx); - - $result = $service->createField($data); - - if (!$result['success']) { - return Response::error($result['message'], HttpStatus::BAD_REQUEST); - } - - return Response::success([ - 'message' => $result['message'], - 'field' => $result['data'], - 'migration' => $result['migration'] - ]); - } catch (\Exception $e) { - $modx->log(\MODX\Revolution\modX::LOG_LEVEL_ERROR, '[ExtraFields API] ' . $e->getMessage()); - return Response::error('Failed to create field: ' . $e->getMessage(), HttpStatus::INTERNAL_SERVER_ERROR); - } + $controller = new \MiniShop3\Controllers\Api\Manager\ExtraFieldsController($modx); + return $controller->create(); }); $router->put('/{id}', function($params) use ($modx) { - $id = (int)($params['id'] ?? 0); - - if (!$id) { - return Response::error('Field ID is required', HttpStatus::BAD_REQUEST); - } - - try { - $data = json_decode(file_get_contents('php://input'), true); - - if (empty($data)) { - return Response::error('Request body is empty', HttpStatus::BAD_REQUEST); - } - - /** @var \MiniShop3\Services\ExtraFieldsService $service */ - $service = new \MiniShop3\Services\ExtraFieldsService($modx); - - $result = $service->updateField($id, $data); - - if (!$result['success']) { - return Response::error($result['message'], HttpStatus::BAD_REQUEST); - } - - return Response::success([ - 'message' => $result['message'], - 'field' => $result['data'] - ]); - } catch (\Exception $e) { - $modx->log(\MODX\Revolution\modX::LOG_LEVEL_ERROR, '[ExtraFields API] ' . $e->getMessage()); - return Response::error('Failed to update field: ' . $e->getMessage(), HttpStatus::INTERNAL_SERVER_ERROR); - } + $controller = new \MiniShop3\Controllers\Api\Manager\ExtraFieldsController($modx); + return $controller->update($params); }); $router->delete('/{id}', function($params) use ($modx) { - $id = (int)($params['id'] ?? 0); - - if (!$id) { - return Response::error('Field ID is required', HttpStatus::BAD_REQUEST); - } - - try { - /** @var \MiniShop3\Services\ExtraFieldsService $service */ - $service = new \MiniShop3\Services\ExtraFieldsService($modx); - - $result = $service->deleteField($id); - - if (!$result['success']) { - return Response::error($result['message'], HttpStatus::BAD_REQUEST); - } - - return Response::success([ - 'message' => $result['message'], - 'migration' => $result['migration'] - ]); - } catch (\Exception $e) { - $modx->log(\MODX\Revolution\modX::LOG_LEVEL_ERROR, '[ExtraFields API] ' . $e->getMessage()); - return Response::error('Failed to delete field: ' . $e->getMessage(), HttpStatus::INTERNAL_SERVER_ERROR); - } + $controller = new \MiniShop3\Controllers\Api\Manager\ExtraFieldsController($modx); + return $controller->delete($params); }); }, [ diff --git a/core/components/minishop3/src/Controllers/Api/Manager/ExtraFieldsController.php b/core/components/minishop3/src/Controllers/Api/Manager/ExtraFieldsController.php new file mode 100644 index 00000000..d2ee8bf3 --- /dev/null +++ b/core/components/minishop3/src/Controllers/Api/Manager/ExtraFieldsController.php @@ -0,0 +1,190 @@ +modx = $modx; + /** @var ExtraFieldsService $service */ + $service = $modx->services->get('ms3_extra_fields'); + $this->service = $service; + } + + /** + * GET /api/mgr/extra-fields + */ + public function getList(array $params = []): array + { + try { + $class = $params['class'] ?? null; + $criteria = $class ? ['class' => $class] : []; + $fields = $this->service->getFields($criteria); + + return Response::success([ + 'fields' => $fields, + 'total' => count($fields), + ])->getData(); + } catch (\Exception $e) { + $this->logError($e); + + return Response::error( + 'Failed to load extra fields: ' . $e->getMessage(), + HttpStatus::INTERNAL_SERVER_ERROR + )->getData(); + } + } + + /** + * GET /api/mgr/extra-fields/{id} + */ + public function get(array $params = []): array + { + $id = (int)($params['id'] ?? 0); + + if (!$id) { + return Response::error('Field ID is required', HttpStatus::BAD_REQUEST)->getData(); + } + + $field = $this->service->getField($id); + + if ($field === null) { + return Response::error('Field not found', HttpStatus::NOT_FOUND)->getData(); + } + + return Response::success(['field' => $field])->getData(); + } + + /** + * POST /api/mgr/extra-fields + */ + public function create(): array + { + try { + $data = $this->readJsonBody(); + + if ($data === []) { + return Response::error('Request body is empty', HttpStatus::BAD_REQUEST)->getData(); + } + + $result = $this->service->createField($data); + + if (!$result['success']) { + return Response::error($result['message'], HttpStatus::BAD_REQUEST)->getData(); + } + + return Response::success([ + 'message' => $result['message'], + 'field' => $result['data'], + 'migration' => $result['migration'], + ])->getData(); + } catch (\Exception $e) { + $this->logError($e); + + return Response::error( + 'Failed to create field: ' . $e->getMessage(), + HttpStatus::INTERNAL_SERVER_ERROR + )->getData(); + } + } + + /** + * PUT /api/mgr/extra-fields/{id} + */ + public function update(array $params = []): array + { + $id = (int)($params['id'] ?? 0); + + if (!$id) { + return Response::error('Field ID is required', HttpStatus::BAD_REQUEST)->getData(); + } + + try { + $data = $this->readJsonBody(); + + if ($data === []) { + return Response::error('Request body is empty', HttpStatus::BAD_REQUEST)->getData(); + } + + $result = $this->service->updateField($id, $data); + + if (!$result['success']) { + return Response::error($result['message'], HttpStatus::BAD_REQUEST)->getData(); + } + + return Response::success([ + 'message' => $result['message'], + 'field' => $result['data'], + ])->getData(); + } catch (\Exception $e) { + $this->logError($e); + + return Response::error( + 'Failed to update field: ' . $e->getMessage(), + HttpStatus::INTERNAL_SERVER_ERROR + )->getData(); + } + } + + /** + * DELETE /api/mgr/extra-fields/{id} + */ + public function delete(array $params = []): array + { + $id = (int)($params['id'] ?? 0); + + if (!$id) { + return Response::error('Field ID is required', HttpStatus::BAD_REQUEST)->getData(); + } + + try { + $result = $this->service->deleteField($id); + + if (!$result['success']) { + return Response::error($result['message'], HttpStatus::BAD_REQUEST)->getData(); + } + + return Response::success([ + 'message' => $result['message'], + 'migration' => $result['migration'], + ])->getData(); + } catch (\Exception $e) { + $this->logError($e); + + return Response::error( + 'Failed to delete field: ' . $e->getMessage(), + HttpStatus::INTERNAL_SERVER_ERROR + )->getData(); + } + } + + /** + * @return array + */ + private function readJsonBody(): array + { + $data = json_decode(file_get_contents('php://input'), true); + + return is_array($data) ? $data : []; + } + + private function logError(\Exception $e): void + { + $this->modx->log(modX::LOG_LEVEL_ERROR, '[ExtraFields API] ' . $e->getMessage()); + } +} diff --git a/core/components/minishop3/src/ServiceRegistry.php b/core/components/minishop3/src/ServiceRegistry.php index 04980615..adedef12 100644 --- a/core/components/minishop3/src/ServiceRegistry.php +++ b/core/components/minishop3/src/ServiceRegistry.php @@ -63,6 +63,10 @@ class ServiceRegistry 'class' => \MiniShop3\Services\ExtraFields\RepeaterFieldService::class, 'interface' => null, ], + 'ms3_extra_fields' => [ + 'class' => \MiniShop3\Services\ExtraFieldsService::class, + 'interface' => null, + ], 'ms3_product_image' => [ 'class' => \MiniShop3\Services\Product\ProductImageService::class, 'interface' => null, diff --git a/core/components/minishop3/src/Services/ExtraFieldsService.php b/core/components/minishop3/src/Services/ExtraFieldsService.php index 5d4d6677..dd7f073f 100644 --- a/core/components/minishop3/src/Services/ExtraFieldsService.php +++ b/core/components/minishop3/src/Services/ExtraFieldsService.php @@ -145,17 +145,42 @@ public function getFields(array $criteria = []): array $result = []; foreach ($fields as $field) { - $data = $field->toArray(); + $result[] = $this->formatField($field); + } - $data['column_exists'] = $this->extraFieldsUtil->columnExists( - $field->get('class'), - $field->get('key') - ); + return $result; + } + + /** + * Get single extra field by ID (with column_exists flag). + * + * @return array|null + */ + public function getField(int $id): ?array + { + /** @var msExtraField|null $field */ + $field = $this->modx->getObject(msExtraField::class, $id); - $result[] = $data; + if (!$field) { + return null; } - return $result; + return $this->formatField($field); + } + + /** + * @return array + */ + private function formatField(msExtraField $field): array + { + $data = $field->toArray(); + + $data['column_exists'] = $this->extraFieldsUtil->columnExists( + $field->get('class'), + $field->get('key') + ); + + return $data; } /** diff --git a/core/components/minishop3/tests/ExtraFieldsControllerRoutesTest.php b/core/components/minishop3/tests/ExtraFieldsControllerRoutesTest.php new file mode 100644 index 00000000..4ac39f26 --- /dev/null +++ b/core/components/minishop3/tests/ExtraFieldsControllerRoutesTest.php @@ -0,0 +1,85 @@ +group('/extra-fields'"); +if ($start === false) { + $fail("extra-fields group not found"); +} + +$nextGroup = strpos($src, "\$router->group('/customers'", $start); +if ($nextGroup === false) { + $fail('anchor after /extra-fields (/customers) not found'); +} + +$block = substr($src, $start, $nextGroup - $start); + +if (str_contains($block, 'getObject')) { + $fail('extra-fields block must not call getObject'); +} + +if (str_contains($block, "file_get_contents('php://input')")) { + $fail('extra-fields block must not parse request body'); +} + +if (str_contains($block, 'new \\MiniShop3\\Services\\ExtraFieldsService')) { + $fail('extra-fields block must not instantiate ExtraFieldsService'); +} + +if (!str_contains($block, 'ExtraFieldsController')) { + $fail('extra-fields block must dispatch to ExtraFieldsController'); +} + +$controllerPath = $root . '/src/Controllers/Api/Manager/ExtraFieldsController.php'; +if (!is_readable($controllerPath)) { + $fail('ExtraFieldsController.php missing'); +} + +$controllerSrc = file_get_contents($controllerPath); +if ($controllerSrc === false) { + $fail('ExtraFieldsController read failed'); +} + +foreach (['getList', 'get', 'create', 'update', 'delete'] as $method) { + if (!preg_match('/function\s+' . preg_quote($method, '/') . '\s*\(/', $controllerSrc)) { + $fail("ExtraFieldsController missing method {$method}"); + } +} + +if (!str_contains($controllerSrc, "services->get('ms3_extra_fields')")) { + $fail('ExtraFieldsController must resolve ms3_extra_fields from DI'); +} + +$registryPath = $root . '/src/ServiceRegistry.php'; +$registry = file_get_contents($registryPath); +if ($registry === false || !str_contains($registry, "'ms3_extra_fields'")) { + $fail('ServiceRegistry must register ms3_extra_fields'); +} + +$servicePath = $root . '/src/Services/ExtraFieldsService.php'; +$serviceSrc = file_get_contents($servicePath); +if ($serviceSrc === false || !str_contains($serviceSrc, 'function getField(')) { + $fail('ExtraFieldsService must expose getField()'); +} + +echo "OK ExtraFieldsControllerRoutesTest\n";