From 860eb28e133382c95f0f39ba4603176f1a68ac81 Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Fri, 9 Oct 2026 15:19:32 +0200 Subject: [PATCH 1/7] feat(cmdb): serve the CMDB import mapping read-only to admins GET /api/settings/cmdb-import/mapping answers the import profile and its five migration packs through CmdbImportProfile, the loader the import uses, so the overview is what the next import runs. A broken pack or a missing validator is 503 MAPPING_UNAVAILABLE with the loader's reason. Co-Authored-By: Claude Opus 5.5 --- appinfo/routes.php | 3 + lib/Controller/SettingsController.php | 80 +++++ lib/Service/Cmdb/CmdbImportProfile.php | 71 ++++ ...ettingsControllerCmdbImportMappingTest.php | 317 ++++++++++++++++++ .../Service/Cmdb/CmdbImportProfileTest.php | 46 +++ 5 files changed, 517 insertions(+) create mode 100644 tests/Unit/Controller/SettingsControllerCmdbImportMappingTest.php diff --git a/appinfo/routes.php b/appinfo/routes.php index 9fc761db..9e023fb9 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -109,6 +109,9 @@ // @spec openspec/changes/cmdb-export-import/tasks.md#task-8 ['name' => 'cmdbImport#import', 'url' => '/api/cmdb-import', 'verb' => 'POST'], ['name' => 'cmdbImport#cancel', 'url' => '/api/cmdb-import/{operationId}/cancel', 'verb' => 'POST'], + // The mapping the CMDB import uses, read-only — same posture as the import (Nextcloud admins only, CSRF). + // @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + ['name' => 'settings#getCmdbImportMapping', 'url' => '/api/settings/cmdb-import/mapping', 'verb' => 'GET'], // User Groups management routes ['name' => 'settings#getGenericUserGroups', 'url' => '/api/settings/user-groups/generic', 'verb' => 'GET'], diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 2e5fd4b2..e0c47c5b 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -26,8 +26,10 @@ use OCA\OpenRegister\Contract\ObjectServiceInterface; use OCA\OpenRegister\Service\ConfigurationService; +use OCA\Stackiq\Exception\CmdbImportException; use OCA\Stackiq\Service\ArchiMateImportService; use OCA\Stackiq\Service\ArchiMateService; +use OCA\Stackiq\Service\Cmdb\CmdbImportProfile; use OCA\Stackiq\Service\ConnectionReportService; use OCA\Stackiq\Service\EolSyncService; use OCA\Stackiq\Service\OrganizationSyncService; @@ -42,6 +44,7 @@ use OCP\IAppConfig; use OCP\IConfig; use OCP\IGroupManager; +use OCP\IL10N; use OCP\IRequest; use OCP\IUserSession; use Psr\Container\ContainerInterface; @@ -88,10 +91,13 @@ class SettingsController extends Controller { * @param EolSyncService $eolSyncService The EOL feed sync orchestration service. * @param LoggerInterface $logger The logger instance. * @param ConnectionReportService|null $connectionReports Asks integriq to look again after an email settings save. + * @param CmdbImportProfile|null $cmdbImportProfile The CMDB import profile and packs; null builds the shipped one. + * @param IL10N|null $l10n Translations of the CMDB mapping messages. * * @SuppressWarnings(PHPMD.ExcessiveParameterList) * * @spec openspec/changes/adopt-connection-registry/specs/admin-integrations/spec.md#requirement-req-stackiq-conn-002-a-save-asks-integriq-to-look-again-and-a-run-reports-what-it-met + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 */ public function __construct( $appName, @@ -108,6 +114,8 @@ public function __construct( private readonly EolSyncService $eolSyncService, private readonly LoggerInterface $logger, private readonly ?ConnectionReportService $connectionReports = null, + private readonly ?CmdbImportProfile $cmdbImportProfile = null, + private readonly ?IL10N $l10n = null, ) { parent::__construct(appName: $appName, request: $request); @@ -3883,4 +3891,76 @@ public function getEolSyncStatus(): JSONResponse { return $this->buildConfigReadErrorResponse(operationLabel: 'get EOL sync status', exception: $e); } }//end getEolSyncStatus() + + /** + * The column mapping the CMDB import uses, read-only. + * + * Loads the import profile and its packs through the loader the import + * uses (CmdbImportProfile), so the answer is what the next import runs. + * Success is the flat `{profile, packs}`; a failure uses the import's + * error envelope, so the section shows it like an import error, with the + * loader's reason in `details.reason` because the admin who edited a file + * needs to know which file and why. That reason names files and validator + * rules only, never a cell value or a person. + * + * @return JSONResponse `{profile, packs}` (200), 503 MAPPING_UNAVAILABLE, or 500 IMPORT_FAILED. + * + * @auth admin-only the import's configuration; the import writes with RBAC and multitenancy off, so its view keeps that posture. + * + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + */ + public function getCmdbImportMapping(): JSONResponse { + $profile = ($this->cmdbImportProfile ?? new CmdbImportProfile(container: $this->container)); + + try { + $overview = $profile->mappingOverview(); + } catch (CmdbImportException $e) { + $this->logger->info( + 'SettingsController: CMDB import mapping unavailable', + ['error' => $e->getErrorCode(), 'reason' => $e->getMessage()] + ); + return new JSONResponse( + [ + 'success' => false, + 'error' => $e->getErrorCode(), + 'message' => $this->translate(text: 'The import mapping cannot be shown: OpenRegister is missing or a mapping file is invalid.'), + 'details' => (object)['reason' => $e->getMessage()], + ], + $e->getHttpStatus() + ); + } catch (\Throwable $e) { + $this->logger->error('SettingsController: CMDB import mapping failed', ['exception' => $e]); + return new JSONResponse( + [ + 'success' => false, + 'error' => 'IMPORT_FAILED', + 'message' => $this->translate(text: 'The import mapping could not be read. The details are in the Nextcloud log.'), + 'details' => (object)[], + ], + Http::STATUS_INTERNAL_SERVER_ERROR + ); + }//end try + + return new JSONResponse($overview, Http::STATUS_OK); + }//end getCmdbImportMapping() + + /** + * Translate a message, or hand it back as it is when no translator was injected. + * + * IL10N is an optional constructor argument (cmdb-import-mapping-view, design D1), + * so a test that builds the controller without it gets the English text. + * + * @param string $text The English text. + * + * @return string + * + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + */ + private function translate(string $text): string { + if ($this->l10n === null) { + return $text; + } + + return $this->l10n->t($text); + }//end translate() }//end class diff --git a/lib/Service/Cmdb/CmdbImportProfile.php b/lib/Service/Cmdb/CmdbImportProfile.php index 1e8805c9..238107cf 100644 --- a/lib/Service/Cmdb/CmdbImportProfile.php +++ b/lib/Service/Cmdb/CmdbImportProfile.php @@ -625,6 +625,77 @@ public function referencedColumns(): array { return array_values(array_unique($columns)); }//end referencedColumns() + /** + * The profile and the packs as the admin settings show them: what the next import runs. + * + * Loads and validates everything first, so a broken pack throws here as + * it does when an import starts. The transform of each field mapping is + * passed through as the pack stores it, so no key the engine reads is + * hidden from the admin; `required` is normalised to a boolean. + * + * @return array{profile: array, packs: array>} + * + * @throws CmdbImportException MAPPING_UNAVAILABLE when the validator is missing, + * or the profile or a pack is unreadable or invalid. + * + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + */ + public function mappingOverview(): array { + $profile = $this->profile(); + + $packs = []; + foreach (self::TARGETS as $target) { + $pack = $this->pack(target: $target); + $mappings = []; + foreach (($pack['fieldMappings'] ?? []) as $mapping) { + if (is_array($mapping) === false) { + continue; + } + + $transform = $mapping['transform'] ?? null; + if (is_array($transform) === false) { + $transform = null; + } + + $mappings[] = [ + 'source' => (string)($mapping['source'] ?? ''), + 'target' => (string)($mapping['target'] ?? ''), + 'required' => (bool)($mapping['required'] ?? false), + 'transform' => $transform, + ]; + } + + $packs[] = [ + 'target' => $target, + 'file' => (string)($profile['packs'][$target] ?? ''), + 'id' => (string)($pack['id'] ?? ''), + 'name' => (string)($pack['name'] ?? ''), + 'version' => (string)($pack['version'] ?? ''), + 'description' => (string)($pack['description'] ?? ''), + 'fieldMappings' => $mappings, + ]; + }//end foreach + + return [ + 'profile' => [ + 'id' => (string)($profile['id'] ?? ''), + 'name' => (string)($profile['name'] ?? ''), + 'version' => (string)($profile['version'] ?? ''), + 'profileFile' => $this->profileFile, + 'sheets' => $this->sheets(), + 'sheetPrecedence' => $this->stringList(key: 'sheetPrecedence'), + 'keyColumn' => $this->keyColumn(), + 'nameColumn' => $this->nameColumn(), + 'requiredColumns' => $this->requiredColumns(), + 'dateColumns' => $this->dateColumns(), + 'idColumns' => $this->idColumns(), + 'emptyValues' => $this->emptyValues(), + 'missingRecords' => $this->missingRecordsModes(), + ], + 'packs' => $packs, + ]; + }//end mappingOverview() + /** * The loaded profile. * diff --git a/tests/Unit/Controller/SettingsControllerCmdbImportMappingTest.php b/tests/Unit/Controller/SettingsControllerCmdbImportMappingTest.php new file mode 100644 index 00000000..d45904c5 --- /dev/null +++ b/tests/Unit/Controller/SettingsControllerCmdbImportMappingTest.php @@ -0,0 +1,317 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://github.com/ConductionNL/stackiq + * + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Tests\Unit\Controller; + +require_once __DIR__ . '/../Support/CmdbTestSupport.php'; + +use OCA\Stackiq\Controller\SettingsController; +use OCA\Stackiq\Service\ArchiMateService; +use OCA\Stackiq\Service\Cmdb\CmdbImportProfile; +use OCA\Stackiq\Service\EolSyncService; +use OCA\Stackiq\Service\OrganizationSyncService; +use OCA\Stackiq\Service\ProgressTracker; +use OCA\Stackiq\Service\SettingsService; +use OCA\Stackiq\Tests\Unit\Support\CmdbTestSupport; +use OCP\App\IAppManager; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\PublicPage; +use OCP\IAppConfig; +use OCP\IGroupManager; +use OCP\IL10N; +use OCP\IRequest; +use OCP\IUserSession; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; +use Psr\Log\LoggerInterface; +use ReflectionMethod; +use RuntimeException; + +/** + * GET /api/settings/cmdb-import/mapping. + */ +class SettingsControllerCmdbImportMappingTest extends TestCase { + /** + * Nextcloud's annotation regex (ControllerMethodReflector), as AdminAuthPostureTest uses it. + */ + private const ANNOTATION = '/^\h+\*\h+@(?P[A-Z]\w+)((?P.*))?$/m'; + + /** + * Directories made by copyOfShippedDirectory(). + * + * @var array + */ + private array $directories = []; + + /** + * Remove the copied directories. + * + * @return void + */ + protected function tearDown(): void { + foreach ($this->directories as $directory) { + array_map('unlink', glob($directory . '/*.json')); + rmdir($directory); + } + + parent::tearDown(); + }//end tearDown() + + /** + * A container that knows nothing, so the validator comes from class_exists. + * + * @return ContainerInterface + */ + private function emptyContainer(): ContainerInterface { + $container = $this->createMock(ContainerInterface::class); + $container->method('has')->willReturn(false); + return $container; + }//end emptyContainer() + + /** + * A copy of the shipped profile directory, to break on purpose. + * + * @return string The directory. + */ + private function copyOfShippedDirectory(): string { + $directory = sys_get_temp_dir() . '/stackiq-cmdb-mapping-' . bin2hex(random_bytes(4)); + mkdir($directory); + foreach (glob(CmdbTestSupport::appRoot() . '/lib/Settings/cmdb-import/*.json') as $file) { + copy($file, $directory . '/' . basename($file)); + } + + $this->directories[] = $directory; + return $directory; + }//end copyOfShippedDirectory() + + /** + * The controller with the given profile, logger and translator. + * + * @param CmdbImportProfile|null $profile The profile; null lets the controller build the shipped one. + * @param LoggerInterface|MockObject|null $logger The logger. + * @param IL10N|null $l10n The translator; null answers the English text. + * + * @return SettingsController + */ + private function controller(?CmdbImportProfile $profile, LoggerInterface|MockObject|null $logger = null, ?IL10N $l10n = null): SettingsController { + $request = $this->createMock(IRequest::class); + $request->method('getParams')->willReturn([]); + + return new SettingsController( + 'stackiq', + $request, + $this->createMock(IAppConfig::class), + $this->emptyContainer(), + $this->createMock(IAppManager::class), + $this->createMock(IGroupManager::class), + $this->createMock(IUserSession::class), + $this->createMock(SettingsService::class), + $this->createMock(OrganizationSyncService::class), + $this->createMock(ArchiMateService::class), + $this->createMock(ProgressTracker::class), + $this->createMock(EolSyncService::class), + ($logger ?? $this->createMock(LoggerInterface::class)), + null, + $profile, + $l10n + ); + }//end controller() + + /** + * The route is for Nextcloud admins only, with CSRF, and declares that with a reason. + * + * No attribute at all is Nextcloud's admin gate; the declaration is the + * `@auth admin-only ` tag, as on the import routes. Checked as + * attribute and as the annotation Nextcloud's regex reads, so a comment + * line that starts with an exemption token fails too. + * + * @return void + */ + public function testTheRouteIsForNextcloudAdminsOnly(): void { + $reflection = new ReflectionMethod(SettingsController::class, 'getCmdbImportMapping'); + $docblock = (string)$reflection->getDocComment(); + + $this->assertSame([], $reflection->getAttributes(), 'getCmdbImportMapping carries no attribute'); + $this->assertMatchesRegularExpression('/^\h+\*\h+@auth admin-only \S.{19,}$/m', $docblock, 'declares @auth admin-only with a reason'); + + preg_match_all(self::ANNOTATION, $docblock, $matches); + $exemptions = [ + 'AuthorizedAdminSetting' => AuthorizedAdminSetting::class, + 'NoAdminRequired' => NoAdminRequired::class, + 'NoCSRFRequired' => NoCSRFRequired::class, + 'PublicPage' => PublicPage::class, + ]; + foreach ($exemptions as $annotation => $attribute) { + $this->assertSame([], $reflection->getAttributes($attribute), $annotation); + $this->assertNotContains($annotation, $matches['annotation'], $annotation); + } + }//end testTheRouteIsForNextcloudAdminsOnly() + + /** + * The route keeps its path and verb. + * + * @return void + */ + public function testTheRouteIsRegistered(): void { + $routes = require __DIR__ . '/../../../appinfo/routes.php'; + $byName = array_column($routes['routes'], null, 'name'); + $this->assertSame( + ['name' => 'settings#getCmdbImportMapping', 'url' => '/api/settings/cmdb-import/mapping', 'verb' => 'GET'], + $byName['settings#getCmdbImportMapping'] + ); + }//end testTheRouteIsRegistered() + + /** + * The shipped files are answered flat: the profile and the five packs the import runs, in TARGETS order. + * + * @return void + */ + public function testTheShippedMappingIsAnsweredFlat(): void { + CmdbTestSupport::loadMigrationPack(); + $profile = new CmdbImportProfile(container: $this->emptyContainer()); + + $response = $this->controller(profile: $profile)->getCmdbImportMapping(); + $data = $response->getData(); + + $this->assertSame(200, $response->getStatus()); + $this->assertSame(['profile', 'packs'], array_keys($data), 'flat envelope, no success or message key'); + + $this->assertSame(['Onbeh Applicaties CMDB', 'Beheerde Applicaties CMDB'], array_column($data['profile']['sheets'], 'name')); + $this->assertSame(['Beheer' => 'Beheer geregeld: nee'], $data['profile']['sheets'][0]['constants']); + $this->assertSame('APPID', $data['profile']['keyColumn']); + $this->assertSame('Applicatie Naam', $data['profile']['nameColumn']); + $this->assertSame(['APPID', 'Applicatie Naam'], $data['profile']['requiredColumns']); + $this->assertSame(['Datum', 'Referentie datum wijziging', 'End-of-Life Functioneel'], $data['profile']['dateColumns']); + $this->assertSame(['keep'], $data['profile']['missingRecords']); + $this->assertSame('topdesk-profile.json', $data['profile']['profileFile']); + + $this->assertSame(CmdbImportProfile::TARGETS, array_column($data['packs'], 'target')); + $this->assertCount(5, $data['packs']); + $this->assertSame( + ['stackiq-topdesk-module', 'stackiq-topdesk-manufacturer', 'stackiq-topdesk-municipality', 'stackiq-topdesk-usage', 'stackiq-topdesk-business-owner'], + array_column($data['packs'], 'id') + ); + $this->assertSame( + ['topdesk-module.json', 'topdesk-manufacturer.json', 'topdesk-municipality.json', 'topdesk-usage.json', 'topdesk-business-owner.json'], + array_column($data['packs'], 'file') + ); + + // What is shown is what runs: every pack lists as many mappings as the loader hands the engine. + foreach ($data['packs'] as $pack) { + $this->assertNotSame('', $pack['name'], $pack['target']); + $this->assertNotSame('', $pack['version'], $pack['target']); + $this->assertCount(count($profile->pack(target: $pack['target'])['fieldMappings']), $pack['fieldMappings'], $pack['target']); + } + + $module = $data['packs'][0]['fieldMappings'][0]; + $this->assertSame(['source' => 'Applicatie Naam', 'target' => 'name', 'required' => true, 'transform' => ['type' => 'trim']], $module); + $this->assertFalse($data['packs'][0]['fieldMappings'][2]['required'], 'Applicatie Code is optional'); + + $status = $data['packs'][3]['fieldMappings'][0]; + $this->assertSame('Applicatie Status', $status['source']); + $this->assertSame('status', $status['target']); + $this->assertSame('lookup', $status['transform']['type']); + $this->assertSame('In production', $status['transform']['map']['In productie']); + + $note = $data['packs'][3]['fieldMappings'][3]; + $this->assertSame('concat', $note['transform']['type']); + $this->assertSame(['Cluster', 'Applicatie Eigenaar (Afdeling)'], $note['transform']['fields']); + }//end testTheShippedMappingIsAnsweredFlat() + + /** + * A pack the validator refuses is 503 MAPPING_UNAVAILABLE, in the import's envelope, with the loader's reason. + * + * @return void + */ + public function testABrokenPackIs503WithTheReason(): void { + CmdbTestSupport::loadMigrationPack(); + $directory = $this->copyOfShippedDirectory(); + $pack = json_decode((string)file_get_contents($directory . '/topdesk-usage.json'), true); + $pack['fieldMappings'][0]['transform'] = ['type' => 'uppercase']; + file_put_contents($directory . '/topdesk-usage.json', json_encode($pack)); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('info')->with( + $this->anything(), + $this->callback(fn (array $context): bool => $context['error'] === 'MAPPING_UNAVAILABLE' && str_contains($context['reason'], 'topdesk-usage.json')) + ); + $l10n = $this->createMock(IL10N::class); + $l10n->method('t')->willReturnCallback(fn (string $text): string => 'NL ' . $text); + + $profile = new CmdbImportProfile(container: $this->emptyContainer(), directory: $directory); + $response = $this->controller(profile: $profile, logger: $logger, l10n: $l10n)->getCmdbImportMapping(); + $data = $response->getData(); + + $this->assertSame(503, $response->getStatus()); + $this->assertFalse($data['success']); + $this->assertSame('MAPPING_UNAVAILABLE', $data['error']); + $this->assertSame('NL The import mapping cannot be shown: OpenRegister is missing or a mapping file is invalid.', $data['message']); + $this->assertStringContainsString('topdesk-usage.json', $data['details']->reason); + $this->assertStringContainsString('uppercase', $data['details']->reason, 'the validator names the unknown transform'); + }//end testABrokenPackIs503WithTheReason() + + /** + * Without OpenRegister's validator the mapping is 503 too, and without a translator the English text is answered. + * + * @return void + */ + public function testAMissingValidatorIs503(): void { + $profile = new class(container: $this->emptyContainer()) extends CmdbImportProfile { + public const VALIDATOR_CLASS = 'OCA\OpenRegister\Service\MigrationPack\NoSuchValidator'; + }; + + $response = $this->controller(profile: $profile)->getCmdbImportMapping(); + $data = $response->getData(); + + $this->assertSame(503, $response->getStatus()); + $this->assertSame('MAPPING_UNAVAILABLE', $data['error']); + $this->assertSame('The import mapping cannot be shown: OpenRegister is missing or a mapping file is invalid.', $data['message']); + $this->assertSame('OpenRegister PackDefinitionValidator is not available', $data['details']->reason); + }//end testAMissingValidatorIs503() + + /** + * An unexpected error is 500 IMPORT_FAILED with a static message, and the exception is logged. + * + * @return void + */ + public function testAnUnexpectedErrorIs500AndLogged(): void { + $profile = $this->createMock(CmdbImportProfile::class); + $profile->method('mappingOverview')->willThrowException(new RuntimeException('disk on fire')); + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('error')->with( + $this->anything(), + $this->callback(fn (array $context): bool => ($context['exception'] ?? null) instanceof RuntimeException) + ); + + $response = $this->controller(profile: $profile, logger: $logger)->getCmdbImportMapping(); + $data = $response->getData(); + + $this->assertSame(500, $response->getStatus()); + $this->assertFalse($data['success']); + $this->assertSame('IMPORT_FAILED', $data['error']); + $this->assertStringNotContainsString('disk on fire', $data['message']); + $this->assertEquals((object)[], $data['details']); + }//end testAnUnexpectedErrorIs500AndLogged() +}//end class diff --git a/tests/Unit/Service/Cmdb/CmdbImportProfileTest.php b/tests/Unit/Service/Cmdb/CmdbImportProfileTest.php index 2d330258..81a12340 100644 --- a/tests/Unit/Service/Cmdb/CmdbImportProfileTest.php +++ b/tests/Unit/Service/Cmdb/CmdbImportProfileTest.php @@ -297,6 +297,52 @@ public function testAMissingPackOrValidatorIsMappingUnavailable(): void { } }//end testAMissingPackOrValidatorIsMappingUnavailable() + /** + * The overview the admin settings show is built from the validated packs, and a broken pack breaks it too. + * + * @return void + * + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + */ + public function testTheOverviewShowsWhatTheImportRuns(): void { + CmdbTestSupport::loadMigrationPack(); + $profile = new CmdbImportProfile(container: $this->emptyContainer()); + + $overview = $profile->mappingOverview(); + + $this->assertSame(['profile', 'packs'], array_keys($overview)); + $this->assertSame('topdesk-cmdb', $overview['profile']['id']); + $this->assertSame($profile->sheets(), $overview['profile']['sheets']); + $this->assertSame(['Beheerde Applicaties CMDB', 'Onbeh Applicaties CMDB'], $overview['profile']['sheetPrecedence']); + $this->assertSame($profile->keyColumn(), $overview['profile']['keyColumn']); + $this->assertSame($profile->requiredColumns(), $overview['profile']['requiredColumns']); + $this->assertSame($profile->idColumns(), $overview['profile']['idColumns']); + $this->assertSame($profile->emptyValues(), $overview['profile']['emptyValues']); + $this->assertSame(CmdbImportProfile::TARGETS, array_column($overview['packs'], 'target')); + foreach ($overview['packs'] as $pack) { + $shipped = $profile->pack(target: $pack['target']); + $this->assertSame($shipped['id'], $pack['id']); + $this->assertSame($shipped['version'], $pack['version']); + $this->assertSame(array_column($shipped['fieldMappings'], 'source'), array_column($pack['fieldMappings'], 'source'), $pack['target']); + $this->assertSame(array_column($shipped['fieldMappings'], 'transform'), array_column($pack['fieldMappings'], 'transform'), $pack['target'] . ' transforms as stored'); + foreach ($pack['fieldMappings'] as $mapping) { + $this->assertIsBool($mapping['required'], $pack['target']); + } + } + + $directory = $this->copyOfShippedDirectory(); + unlink($directory . '/topdesk-module.json'); + try { + (new CmdbImportProfile(container: $this->emptyContainer(), directory: $directory))->mappingOverview(); + $this->fail('MAPPING_UNAVAILABLE expected'); + } catch (CmdbImportException $e) { + $this->assertSame('MAPPING_UNAVAILABLE', $e->getErrorCode()); + $this->assertStringContainsString('topdesk-module.json', $e->getMessage()); + } finally { + $this->remove(directory: $directory); + } + }//end testTheOverviewShowsWhatTheImportRuns() + /** * The upload limit is readable without OpenRegister. * From 38f2ac089fbc227827b5fd48782c30d39b995d95 Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Fri, 9 Oct 2026 15:21:54 +0200 Subject: [PATCH 2/7] feat(cmdb): show the import mapping read-only in the CMDB import section A collapsible "Mapping (read-only)" block lists the profile's sheets and key columns and one table per migration pack (source column, field, required, transformation and its lookup values), with loading and error states. The section's help text now names the sheets from the endpoint, keeping PROFILE_DEFAULTS as the fallback. Co-Authored-By: Claude Opus 5.5 --- l10n/en.js | 35 +- l10n/en.json | 35 +- l10n/nl.js | 35 +- l10n/nl.json | 35 +- src/utils/cmdbImport.js | 221 +++++++++++ src/views/settings/sections/CmdbImport.vue | 38 +- .../settings/sections/CmdbImportMapping.vue | 358 ++++++++++++++++++ tests/vitest/cmdbImportMapping.spec.js | 275 ++++++++++++++ 8 files changed, 1024 insertions(+), 8 deletions(-) create mode 100644 src/views/settings/sections/CmdbImportMapping.vue create mode 100644 tests/vitest/cmdbImportMapping.spec.js diff --git a/l10n/en.js b/l10n/en.js index 8dd84a2f..60a730e5 100644 --- a/l10n/en.js +++ b/l10n/en.js @@ -1132,7 +1132,40 @@ OC.L10N.register( "The workbook holds more than {count} different texts, the most the import reads.": "The workbook holds more than {count} different texts, the most the import reads.", "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.": "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.", "In development since": "In development since", - "In use since": "In use since" + "In use since": "In use since", + "Application (module)": "Application (module)", + "Supplier organisation (Vendor)": "Supplier organisation (Vendor)", + "Municipality (from the import options)": "Municipality (from the import options)", + "Business owner (contact person)": "Business owner (contact person)", + "As is": "As is", + "Trim": "Trim", + "Date": "Date", + "Lookup": "Lookup", + "Yes/no lookup": "Yes/no lookup", + "Join": "Join", + "Constant": "Constant", + "Any other value: {value}": "Any other value: {value}", + "Joined with the columns {columns}, separated by \"{separator}\"": "Joined with the columns {columns}, separated by \"{separator}\"", + "Value: {value}": "Value: {value}", + "Read as {source}, stored as {target}": "Read as {source}, stored as {target}", + "Mapping (read-only)": "Mapping (read-only)", + "The mapping the next import runs: the sheets it reads and, per pack, which column of the export goes to which field. It is read from the files under lib/Settings/cmdb-import of the app on the server; a file changed there is overwritten by the next app update.": "The mapping the next import runs: the sheets it reads and, per pack, which column of the export goes to which field. It is read from the files under lib/Settings/cmdb-import of the app on the server; a file changed there is overwritten by the next app update.", + "Loading the mapping…": "Loading the mapping…", + "The mapping could not be loaded.": "The mapping could not be loaded.", + "{file}: {name}, version {version}": "{file}: {name}, version {version}", + "This pack maps no columns": "This pack maps no columns", + "Yes": "Yes", + "No": "No", + "Sheets read": "Sheets read", + "Match column": "Match column", + "Name column": "Name column", + "Required columns": "Required columns", + "Date columns": "Date columns", + "Column in the export": "Column in the export", + "Field": "Field", + "Transformation": "Transformation", + "The import mapping cannot be shown: OpenRegister is missing or a mapping file is invalid.": "The import mapping cannot be shown: OpenRegister is missing or a mapping file is invalid.", + "The import mapping could not be read. The details are in the Nextcloud log.": "The import mapping could not be read. The details are in the Nextcloud log." }, "nplurals=2; plural=(n != 1);" ) diff --git a/l10n/en.json b/l10n/en.json index 313f968e..5e5a3243 100644 --- a/l10n/en.json +++ b/l10n/en.json @@ -1131,6 +1131,39 @@ "The workbook holds more than {count} different texts, the most the import reads.": "The workbook holds more than {count} different texts, the most the import reads.", "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.": "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.", "In development since": "In development since", - "In use since": "In use since" + "In use since": "In use since", + "Application (module)": "Application (module)", + "Supplier organisation (Vendor)": "Supplier organisation (Vendor)", + "Municipality (from the import options)": "Municipality (from the import options)", + "Business owner (contact person)": "Business owner (contact person)", + "As is": "As is", + "Trim": "Trim", + "Date": "Date", + "Lookup": "Lookup", + "Yes/no lookup": "Yes/no lookup", + "Join": "Join", + "Constant": "Constant", + "Any other value: {value}": "Any other value: {value}", + "Joined with the columns {columns}, separated by \"{separator}\"": "Joined with the columns {columns}, separated by \"{separator}\"", + "Value: {value}": "Value: {value}", + "Read as {source}, stored as {target}": "Read as {source}, stored as {target}", + "Mapping (read-only)": "Mapping (read-only)", + "The mapping the next import runs: the sheets it reads and, per pack, which column of the export goes to which field. It is read from the files under lib/Settings/cmdb-import of the app on the server; a file changed there is overwritten by the next app update.": "The mapping the next import runs: the sheets it reads and, per pack, which column of the export goes to which field. It is read from the files under lib/Settings/cmdb-import of the app on the server; a file changed there is overwritten by the next app update.", + "Loading the mapping…": "Loading the mapping…", + "The mapping could not be loaded.": "The mapping could not be loaded.", + "{file}: {name}, version {version}": "{file}: {name}, version {version}", + "This pack maps no columns": "This pack maps no columns", + "Yes": "Yes", + "No": "No", + "Sheets read": "Sheets read", + "Match column": "Match column", + "Name column": "Name column", + "Required columns": "Required columns", + "Date columns": "Date columns", + "Column in the export": "Column in the export", + "Field": "Field", + "Transformation": "Transformation", + "The import mapping cannot be shown: OpenRegister is missing or a mapping file is invalid.": "The import mapping cannot be shown: OpenRegister is missing or a mapping file is invalid.", + "The import mapping could not be read. The details are in the Nextcloud log.": "The import mapping could not be read. The details are in the Nextcloud log." } } diff --git a/l10n/nl.js b/l10n/nl.js index 3151813d..e51e4af3 100644 --- a/l10n/nl.js +++ b/l10n/nl.js @@ -1200,7 +1200,40 @@ OC.L10N.register( "The workbook holds more than {count} different texts, the most the import reads.": "De werkmap bevat meer dan {count} verschillende teksten, het maximum dat de import leest.", "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.": "Samen verwijzen de cellen van de werkmap naar meer dan {size} gedeelde tekst, het maximum dat de import leest.", "In development since": "In ontwikkeling sinds", - "In use since": "In gebruik sinds" + "In use since": "In gebruik sinds", + "Application (module)": "Applicatie (module)", + "Supplier organisation (Vendor)": "Leveranciersorganisatie (Vendor)", + "Municipality (from the import options)": "Gemeente (uit de importopties)", + "Business owner (contact person)": "Business owner (contactpersoon)", + "As is": "Ongewijzigd", + "Trim": "Spaties weghalen", + "Date": "Datum", + "Lookup": "Opzoektabel", + "Yes/no lookup": "Ja/nee-opzoektabel", + "Join": "Samenvoegen", + "Constant": "Vaste waarde", + "Any other value: {value}": "Elke andere waarde: {value}", + "Joined with the columns {columns}, separated by \"{separator}\"": "Samengevoegd met de kolommen {columns}, gescheiden door \"{separator}\"", + "Value: {value}": "Waarde: {value}", + "Read as {source}, stored as {target}": "Gelezen als {source}, opgeslagen als {target}", + "Mapping (read-only)": "Mapping (alleen-lezen)", + "The mapping the next import runs: the sheets it reads and, per pack, which column of the export goes to which field. It is read from the files under lib/Settings/cmdb-import of the app on the server; a file changed there is overwritten by the next app update.": "De mapping die de volgende import uitvoert: de tabbladen die hij leest en, per pack, welke kolom van de export naar welk veld gaat. Hij wordt gelezen uit de bestanden onder lib/Settings/cmdb-import van de app op de server; een bestand dat daar is aangepast wordt bij de volgende app-update overschreven.", + "Loading the mapping…": "Mapping laden…", + "The mapping could not be loaded.": "De mapping kon niet worden geladen.", + "{file}: {name}, version {version}": "{file}: {name}, versie {version}", + "This pack maps no columns": "Dit pack koppelt geen kolommen", + "Yes": "Ja", + "No": "Nee", + "Sheets read": "Gelezen tabbladen", + "Match column": "Koppelkolom", + "Name column": "Naamkolom", + "Required columns": "Verplichte kolommen", + "Date columns": "Datumkolommen", + "Column in the export": "Kolom in de export", + "Field": "Veld", + "Transformation": "Transformatie", + "The import mapping cannot be shown: OpenRegister is missing or a mapping file is invalid.": "De mapping van de import kan niet worden getoond: OpenRegister ontbreekt of een mappingbestand is ongeldig.", + "The import mapping could not be read. The details are in the Nextcloud log.": "De mapping van de import kon niet worden gelezen. De details staan in het Nextcloud-logboek." }, "nplurals=2; plural=(n != 1);" ) diff --git a/l10n/nl.json b/l10n/nl.json index 680f6f85..6ab4bbf9 100644 --- a/l10n/nl.json +++ b/l10n/nl.json @@ -1199,6 +1199,39 @@ "The workbook holds more than {count} different texts, the most the import reads.": "De werkmap bevat meer dan {count} verschillende teksten, het maximum dat de import leest.", "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.": "Samen verwijzen de cellen van de werkmap naar meer dan {size} gedeelde tekst, het maximum dat de import leest.", "In development since": "In ontwikkeling sinds", - "In use since": "In gebruik sinds" + "In use since": "In gebruik sinds", + "Application (module)": "Applicatie (module)", + "Supplier organisation (Vendor)": "Leveranciersorganisatie (Vendor)", + "Municipality (from the import options)": "Gemeente (uit de importopties)", + "Business owner (contact person)": "Business owner (contactpersoon)", + "As is": "Ongewijzigd", + "Trim": "Spaties weghalen", + "Date": "Datum", + "Lookup": "Opzoektabel", + "Yes/no lookup": "Ja/nee-opzoektabel", + "Join": "Samenvoegen", + "Constant": "Vaste waarde", + "Any other value: {value}": "Elke andere waarde: {value}", + "Joined with the columns {columns}, separated by \"{separator}\"": "Samengevoegd met de kolommen {columns}, gescheiden door \"{separator}\"", + "Value: {value}": "Waarde: {value}", + "Read as {source}, stored as {target}": "Gelezen als {source}, opgeslagen als {target}", + "Mapping (read-only)": "Mapping (alleen-lezen)", + "The mapping the next import runs: the sheets it reads and, per pack, which column of the export goes to which field. It is read from the files under lib/Settings/cmdb-import of the app on the server; a file changed there is overwritten by the next app update.": "De mapping die de volgende import uitvoert: de tabbladen die hij leest en, per pack, welke kolom van de export naar welk veld gaat. Hij wordt gelezen uit de bestanden onder lib/Settings/cmdb-import van de app op de server; een bestand dat daar is aangepast wordt bij de volgende app-update overschreven.", + "Loading the mapping…": "Mapping laden…", + "The mapping could not be loaded.": "De mapping kon niet worden geladen.", + "{file}: {name}, version {version}": "{file}: {name}, versie {version}", + "This pack maps no columns": "Dit pack koppelt geen kolommen", + "Yes": "Ja", + "No": "Nee", + "Sheets read": "Gelezen tabbladen", + "Match column": "Koppelkolom", + "Name column": "Naamkolom", + "Required columns": "Verplichte kolommen", + "Date columns": "Datumkolommen", + "Column in the export": "Kolom in de export", + "Field": "Veld", + "Transformation": "Transformatie", + "The import mapping cannot be shown: OpenRegister is missing or a mapping file is invalid.": "De mapping van de import kan niet worden getoond: OpenRegister ontbreekt of een mappingbestand is ongeldig.", + "The import mapping could not be read. The details are in the Nextcloud log.": "De mapping van de import kon niet worden gelezen. De details staan in het Nextcloud-logboek." } } diff --git a/src/utils/cmdbImport.js b/src/utils/cmdbImport.js index ab7fa708..5e367e7b 100644 --- a/src/utils/cmdbImport.js +++ b/src/utils/cmdbImport.js @@ -984,3 +984,224 @@ export function reportRows(rows) { moduleUuid: row.moduleUuid ? String(row.moduleUuid) : '', })) } + +/** + * The URL of the mapping overview endpoint. + * + * @return {string} The URL + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + */ +export function mappingUrl() { + return generateUrl('/apps/stackiq/api/settings/cmdb-import/mapping') +} + +/** + * Read the mapping the import uses: the profile and one entry per pack. + * + * @param {object} options The options + * @param {object} options.http An axios-like client with get + * @return {Promise<{profile: object, packs: Array}>} The server's answer + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + */ +export async function loadCmdbMapping({ http }) { + const response = await http.get(mappingUrl()) + return response.data +} + +/** + * The names of the source sheets: the profile's when the mapping has loaded, + * the shipped defaults until then or when it could not be loaded. + * + * @param {object|null} mapping The mapping endpoint's answer, or null + * @return {Array} At least the two default names + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-offer-a-cmdb-import-section-req-cmdb-014 + */ +export function mappingSheetNames(mapping) { + const sheets = mapping?.profile?.sheets + const names = (Array.isArray(sheets) ? sheets : []) + .map((sheet) => + typeof sheet === 'string' ? sheet : String(sheet?.name ?? ''), + ) + .filter((name) => name !== '') + return names.length > 0 ? names : [...PROFILE_DEFAULTS.sheets] +} + +/** + * The words for a pack's target: what the pack's rows become. + * + * @param {string} target The target as the profile names it (module, manufacturer, …) + * @return {string} The translated name, or the target itself when it is unknown + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + */ +export function packTargetLabel(target) { + switch (target) { + case 'module': + return t('stackiq', 'Application (module)') + case 'manufacturer': + return t('stackiq', 'Supplier organisation (Vendor)') + case 'municipality': + return t('stackiq', 'Municipality (from the import options)') + case 'usage': + return t('stackiq', 'Usage') + case 'businessOwner': + return t('stackiq', 'Business owner (contact person)') + default: + return String(target ?? '') + } +} + +/** + * The words for a transformation type of OpenRegister's mapping engine. + * + * @param {string|null|undefined} type The transform type, or nothing when the value is copied as is + * @return {string} The translated name, or the type itself when it is unknown + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + */ +export function transformLabel(type) { + switch (type) { + case undefined: + case null: + case '': + return t('stackiq', 'As is') + case 'trim': + return t('stackiq', 'Trim') + case 'date': + return t('stackiq', 'Date') + case 'lookup': + return t('stackiq', 'Lookup') + case 'bool-map': + return t('stackiq', 'Yes/no lookup') + case 'concat': + return t('stackiq', 'Join') + case 'const': + return t('stackiq', 'Constant') + default: + return String(type) + } +} + +/** + * One text per value, for the details column: a JSON array is shown as its + * members, a null as a dash, anything else as a string. + * + * @param {unknown} value The stored value + * @return {string} The text + */ +function valueText(value) { + if (value === null || value === undefined) { + return '—' + } + if (Array.isArray(value)) { + return value.map((member) => valueText(member)).join(', ') + } + if (typeof value === 'object') { + return JSON.stringify(value) + } + return String(value) +} + +/** + * The details of a transformation, as lines: the pairs of a lookup, the + * extra columns of a join, the value of a constant, the formats of a date. + * A key the page does not know is listed as it is, so nothing the engine + * reads is hidden. + * + * @param {object|null} transform The transform as the pack stores it + * @return {Array} The lines, empty for a plain trim + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + */ +export function transformDetails(transform) { + if (!transform || typeof transform !== 'object') { + return [] + } + const lines = [] + const known = new Set(['type']) + if (transform.map && typeof transform.map === 'object') { + known.add('map') + for (const [from, to] of Object.entries(transform.map)) { + lines.push(`${from} → ${valueText(to)}`) + } + } + if (Object.hasOwn(transform, 'default')) { + known.add('default') + lines.push( + t( + 'stackiq', + 'Any other value: {value}', + { value: valueText(transform.default) }, + AS_TEXT, + ), + ) + } + if (Array.isArray(transform.fields)) { + known.add('fields') + known.add('separator') + lines.push( + t( + 'stackiq', + 'Joined with the columns {columns}, separated by "{separator}"', + { + columns: transform.fields + .map((field) => String(field)) + .join(', '), + separator: String(transform.separator ?? ''), + }, + AS_TEXT, + ), + ) + } + if (Object.hasOwn(transform, 'value')) { + known.add('value') + lines.push( + t( + 'stackiq', + 'Value: {value}', + { value: valueText(transform.value) }, + AS_TEXT, + ), + ) + } + if (transform.sourceFormat || transform.targetFormat) { + known.add('sourceFormat') + known.add('targetFormat') + lines.push( + t( + 'stackiq', + 'Read as {source}, stored as {target}', + { + source: String(transform.sourceFormat ?? '—'), + target: String(transform.targetFormat ?? 'Y-m-d'), + }, + AS_TEXT, + ), + ) + } + for (const [key, value] of Object.entries(transform)) { + if (!known.has(key)) { + lines.push(`${key}: ${valueText(value)}`) + } + } + return lines +} + +/** + * The rows of one pack's table: one per field mapping, in the pack's order. + * + * @param {object} pack One entry of the endpoint's `packs` + * @return {Array<{key: string, source: string, target: string, required: boolean, transform: string, details: Array}>} The rows + * @spec openspec/changes/cmdb-import-mapping-view/specs/cmdb-export-import/spec.md#requirement-the-admin-settings-shall-show-the-mapping-the-import-uses-req-cmdb-020 + */ +export function mappingRows(pack) { + const mappings = pack?.fieldMappings + if (!Array.isArray(mappings)) { + return [] + } + return mappings.map((mapping, index) => ({ + key: `${mapping?.source ?? ''}:${mapping?.target ?? ''}:${index}`, + source: String(mapping?.source ?? ''), + target: String(mapping?.target ?? ''), + required: Boolean(mapping?.required), + transform: transformLabel(mapping?.transform?.type), + details: transformDetails(mapping?.transform), + })) +} diff --git a/src/views/settings/sections/CmdbImport.vue b/src/views/settings/sections/CmdbImport.vue index f8bbc1e4..8b5a6ff6 100644 --- a/src/views/settings/sections/CmdbImport.vue +++ b/src/views/settings/sections/CmdbImport.vue @@ -89,8 +89,8 @@ 'stackiq', 'Excel workbook (.xlsx) with the sheet "{first}" or "{second}". By default the file may be at most {size}.', { - first: profileDefaults.sheets[0], - second: profileDefaults.sheets[1], + first: sheetNames[0], + second: sheetNames[1], size: formatMegabytes(profileDefaults.maxFileBytes), }, asText, @@ -361,6 +361,9 @@ + + +