diff --git a/appinfo/routes.php b/appinfo/routes.php index 553ccc6db..f18911f8d 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -17,6 +17,7 @@ // First-time setup wizard (ADR-042) - the standard CnSetupWizard contract. ['name' => 'setup#status', 'url' => '/api/setup/status', 'verb' => 'GET'], ['name' => 'setup#runAction', 'url' => '/api/setup/action/{actionId}', 'verb' => 'POST', 'requirements' => ['actionId' => '[a-z0-9\\-]+']], + ['name' => 'setup#saveConfig', 'url' => '/api/setup/config', 'verb' => 'POST'], ['name' => 'metrics#index', 'url' => '/api/metrics', 'verb' => 'GET'], ['name' => 'health#index', 'url' => '/api/health', 'verb' => 'GET'], diff --git a/composer.lock b/composer.lock index 0957318f6..accff0e8f 100644 --- a/composer.lock +++ b/composer.lock @@ -5881,16 +5881,16 @@ }, { "name": "conduction/hydra-gates", - "version": "v1.14.0", + "version": "v1.15.0", "source": { "type": "git", "url": "https://github.com/ConductionNL/.github.git", - "reference": "7150f7f7327109cafce40ca23ca3310ee2c10aa8" + "reference": "0bc214023be78aac94142035a9988986cfccccbc" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/ConductionNL/.github/zipball/7150f7f7327109cafce40ca23ca3310ee2c10aa8", - "reference": "7150f7f7327109cafce40ca23ca3310ee2c10aa8", + "url": "https://api.github.com/repos/ConductionNL/.github/zipball/0bc214023be78aac94142035a9988986cfccccbc", + "reference": "0bc214023be78aac94142035a9988986cfccccbc", "shasum": "" }, "require": { @@ -5929,9 +5929,9 @@ "support": { "docs": "https://github.com/ConductionNL/.github/blob/main/hydra-gates/README.md", "issues": "https://github.com/ConductionNL/.github/issues", - "source": "https://github.com/ConductionNL/.github/tree/v1.14.0" + "source": "https://github.com/ConductionNL/.github/tree/v1.15.0" }, - "time": "2026-09-04T14:51:44+00:00" + "time": "2026-09-04T16:24:04+00:00" }, { "name": "cyclonedx/cyclonedx-library", diff --git a/l10n/nl.js b/l10n/nl.js index ed000fe8e..424e4e858 100644 --- a/l10n/nl.js +++ b/l10n/nl.js @@ -1,6 +1,14 @@ OC.L10N.register( "integriq", { + "Load example data?": "Voorbeeldgegevens laden?", + "Example data fills the lists, detail pages and dashboards so you can see the app working straight away. Pick \"None\" on a production install.": "Voorbeeldgegevens vullen de lijsten, detailpagina’s en dashboards, zodat je de app meteen ziet werken. Kies \"Geen\" op een productieomgeving.", + "Load the example data": "Laad de voorbeeldgegevens", + "Loads what you picked. The data is obviously sample data, it is safe to run more than once, and you can delete it afterwards.": "Laadt wat je koos. De gegevens zijn herkenbaar voorbeeldgegevens, je kunt dit meer dan een keer uitvoeren en je kunt ze daarna verwijderen.", + "None, I will set this up myself": "Geen, ik richt dit zelf in", + "Nothing is imported. You start with an empty app and add your own data.": "Er wordt niets geïmporteerd. Je begint met een lege app en voegt zelf gegevens toe.", + "Example data": "Voorbeeldgegevens", + "Sample values for every schema this app supplies, generated from the schemas themselves. It shows the lists, detail pages and dashboards working rather than telling a story. Safe to run more than once, and you can delete it afterwards.": "Voorbeeldwaarden voor elk schema dat deze app levert, gegenereerd uit de schema’s zelf. Het laat de lijsten, detailpagina’s en dashboards werkend zien in plaats van een verhaal te vertellen. Veilig om vaker uit te voeren, en je kunt het daarna verwijderen.", "Welcome": "Welkom", "A short setup to get this app ready. Nothing here is required; you can close it and come back later.": "Een korte installatie om deze app klaar te zetten. Niets hiervan is verplicht; je kunt dit sluiten en later terugkomen.", "Demo data (optional)": "Demovoorbeelddata (optioneel)", diff --git a/l10n/nl.json b/l10n/nl.json index 253cbb238..76fc218f3 100644 --- a/l10n/nl.json +++ b/l10n/nl.json @@ -1,5 +1,13 @@ { "translations": { + "Load example data?": "Voorbeeldgegevens laden?", + "Example data fills the lists, detail pages and dashboards so you can see the app working straight away. Pick \"None\" on a production install.": "Voorbeeldgegevens vullen de lijsten, detailpagina’s en dashboards, zodat je de app meteen ziet werken. Kies \"Geen\" op een productieomgeving.", + "Load the example data": "Laad de voorbeeldgegevens", + "Loads what you picked. The data is obviously sample data, it is safe to run more than once, and you can delete it afterwards.": "Laadt wat je koos. De gegevens zijn herkenbaar voorbeeldgegevens, je kunt dit meer dan een keer uitvoeren en je kunt ze daarna verwijderen.", + "None, I will set this up myself": "Geen, ik richt dit zelf in", + "Nothing is imported. You start with an empty app and add your own data.": "Er wordt niets geïmporteerd. Je begint met een lege app en voegt zelf gegevens toe.", + "Example data": "Voorbeeldgegevens", + "Sample values for every schema this app supplies, generated from the schemas themselves. It shows the lists, detail pages and dashboards working rather than telling a story. Safe to run more than once, and you can delete it afterwards.": "Voorbeeldwaarden voor elk schema dat deze app levert, gegenereerd uit de schema’s zelf. Het laat de lijsten, detailpagina’s en dashboards werkend zien in plaats van een verhaal te vertellen. Veilig om vaker uit te voeren, en je kunt het daarna verwijderen.", "Welcome": "Welkom", "A short setup to get this app ready. Nothing here is required; you can close it and come back later.": "Een korte installatie om deze app klaar te zetten. Niets hiervan is verplicht; je kunt dit sluiten en later terugkomen.", "Demo data (optional)": "Demovoorbeelddata (optioneel)", diff --git a/lib/Controller/SetupController.php b/lib/Controller/SetupController.php index c09fc38a4..979c413f0 100644 --- a/lib/Controller/SetupController.php +++ b/lib/Controller/SetupController.php @@ -65,6 +65,19 @@ class SetupController extends Controller { */ private const DEMO_DECIDED_KEY = 'demo_data_decided'; + /** + * App-config key holding the dataset the operator picked. + * + * The wizard's `choice` step writes it through `POST /api/setup/config`, and + * the `run-action` step that follows reads it back. Two steps rather than + * one because `CnSetupWizard::runAction()` posts to + * `/api/setup/action/{action}` with no body: an action cannot carry the + * answer, so the answer has to be stored before the action runs. + * + * @var string + */ + private const DATASET_KEY = 'demo_dataset'; + /** * Constructor. * @@ -99,19 +112,77 @@ public function __construct( #[AuthorizedAdminSetting(IntegriqAdmin::class)] public function status(): JSONResponse { $demoDecided = $this->appConfig->getValueString(Application::APP_ID, self::DEMO_DECIDED_KEY, '') !== ''; + $picked = $this->appConfig->getValueString(Application::APP_ID, self::DATASET_KEY, ''); return new JSONResponse( data: [ 'version' => self::SETUP_VERSION, 'completed' => true, + // The choice step reads its options from here: it declares + // `optionsSource: datasets` and no options of its own, so a + // dataset missing from this list is a dataset nobody can pick. + 'datasets' => $this->demoDataService->listChoices(), 'steps' => [ - 'demo-data' => ['done' => $demoDecided], + 'demo-data' => ['done' => ($picked !== '')], + // "None" is an ANSWER, so the load step is finished the moment + // it is chosen: there is nothing left for the operator to run. + 'load-demo-data' => [ + 'done' => ($demoDecided === true || $picked === DemoDataService::NONE_DATASET), + ], ], ] ); }//end status() + /** + * Persist the wizard's `choice` answer. + * + * @return JSONResponse `{ success, config }`. + * + * @spec exclude Setup config write; ADR-042 contract, no per-app behavioural spec. + */ + #[AuthorizedAdminSetting(IntegriqAdmin::class)] + public function saveConfig(): JSONResponse { + // 🔴 ONE NAMED KEY, NEVER A CALLER-SUPPLIED ONE. The body arrives from + // the browser and this app's own settings share the appconfig namespace, + // so looping over the posted keys would let this endpoint write any of + // them. The key is written in the source; only its value comes from the + // request. + $value = $this->request->getParam(self::DATASET_KEY); + if ($value === null) { + return new JSONResponse(data: ['success' => true, 'config' => []]); + } + + // The step is not `multiple`, but the wizard's contract allows a list, so + // both shapes are read rather than one of them reaching `(string)`. + $submitted = $value; + if (is_array($value) === true) { + $submitted = ($value[0] ?? null); + } + + if (is_scalar($submitted) === false) { + return new JSONResponse( + data: ['success' => false, 'message' => 'A dataset is named by a string.'], + statusCode: Http::STATUS_BAD_REQUEST, + ); + } + + $datasetId = (string)$submitted; + $known = array_column($this->demoDataService->listChoices(), 'id'); + if (in_array($datasetId, $known, true) === false) { + return new JSONResponse( + data: ['success' => false, 'message' => 'No dataset is called "' . $datasetId . '".'], + statusCode: Http::STATUS_BAD_REQUEST, + ); + } + + $this->appConfig->setValueString(Application::APP_ID, self::DATASET_KEY, $datasetId); + + return new JSONResponse(data: ['success' => true, 'config' => [self::DATASET_KEY => $datasetId]]); + + }//end saveConfig() + /** * Run a privileged server-side setup action. * @@ -125,15 +196,23 @@ public function status(): JSONResponse { */ #[AuthorizedAdminSetting(IntegriqAdmin::class)] public function runAction(string $actionId): JSONResponse { - if ($actionId === 'install-demo-data') { - return $this->installDemoData(); + // `install-demo-data` is the id the step used before it asked WHICH + // dataset, and it still means "import the one this app ships". Kept so + // an older manifest, a runbook or a script that posts it keeps working. + if ($actionId === 'load-demo-data' || $actionId === 'install-demo-data') { + return $this->loadDataset(actionId: $actionId); } // DECLINING IS AN ANSWER — see DEMO_DECIDED_KEY. + // + // 🔴 AND IT ANSWERS *BOTH* STEPS. The wizard now has a choice step and a + // run-action step; closing only the second leaves the first outstanding, + // and CnAppRoot opens the wizard while ANY optional step is outstanding. if ($actionId === 'skip-demo-data') { + $this->appConfig->setValueString(Application::APP_ID, self::DATASET_KEY, DemoDataService::NONE_DATASET); $this->appConfig->setValueString(Application::APP_ID, self::DEMO_DECIDED_KEY, 'skipped'); - return new JSONResponse(data: ['success' => true, 'message' => 'Demo data skipped.']); + return new JSONResponse(data: ['success' => true, 'message' => 'No example data was loaded.']); } return new JSONResponse( @@ -144,7 +223,10 @@ public function runAction(string $actionId): JSONResponse { }//end runAction() /** - * Import the shipped demo dataset. + * Import the dataset the operator picked in the previous step. + * + * @param string $actionId The action that asked, which decides whether an + * unanswered choice is refused or means the shipped set. * * Reports the FAILURE rather than a quiet success: an operator who asked for * demo data and got none must be told, which is why DemoDataService::install() @@ -152,7 +234,31 @@ public function runAction(string $actionId): JSONResponse { * * @return JSONResponse `{ success, message }`. */ - private function installDemoData(): JSONResponse { + private function loadDataset(string $actionId): JSONResponse { + $picked = $this->appConfig->getValueString(Application::APP_ID, self::DATASET_KEY, ''); + + // The legacy id carries no answer, so it means the shipped dataset. A + // caller that posts it has said which one by posting it. + if ($actionId === 'install-demo-data' && $picked === '') { + $picked = DemoDataService::DEMO_DATASET; + } + + // 🔴 NO SILENT DEFAULT. Importing here because the operator clicked Run + // one step early would plant example objects nobody asked for, which is + // the failure this whole step exists to avoid. + if ($picked === '') { + return new JSONResponse( + data: ['success' => false, 'message' => 'Pick a dataset first.'], + statusCode: Http::STATUS_BAD_REQUEST, + ); + } + + if ($picked === DemoDataService::NONE_DATASET) { + $this->appConfig->setValueString(Application::APP_ID, self::DEMO_DECIDED_KEY, 'skipped'); + + return new JSONResponse(data: ['success' => true, 'message' => 'No example data was loaded.']); + } + try { $imported = $this->demoDataService->install(); } catch (\Throwable $e) { @@ -176,5 +282,5 @@ private function installDemoData(): JSONResponse { ] ); - }//end installDemoData() + }//end loadDataset() }//end class diff --git a/lib/Service/DemoDataService.php b/lib/Service/DemoDataService.php index ef792dfad..44bfaa19f 100644 --- a/lib/Service/DemoDataService.php +++ b/lib/Service/DemoDataService.php @@ -84,6 +84,108 @@ public function isAvailable(): bool { return is_file($this->descriptorPath()) === true; }//end isAvailable() + /** + * The answer that means "plant nothing". + * + * 🔴 NOT THE ABSENCE OF AN ANSWER. An operator who declines has FINISHED the + * step; a step that can never be marked done reopens the wizard over every + * page (nextcloud-vue#806). + * + * @var string + */ + public const NONE_DATASET = 'none'; + + /** + * The id of the dataset this app ships. + * + * @var string + */ + public const DEMO_DATASET = 'demo'; + + /** + * Every answer the wizard's choice step may offer, declining included. + * + * 🔴 THE SERVER OWNS THIS LIST, AND THAT IS THE POINT. The step declares + * `optionsSource: datasets` and no options of its own, so the label, the + * description and the object count come from the descriptor that will + * actually be imported. A manifest that restated them could disagree with + * what lands, and nothing would notice. + * + * @return array The answers. + * + * @spec exclude Demo-data choice list; ADR-111 rule 1 has no per-app behavioural spec. + */ + public function listChoices(): array { + $choices = [ + [ + 'id' => self::NONE_DATASET, + 'label' => 'None, I will set this up myself', + 'description' => 'Nothing is imported. You start with an empty app and add your own data.', + 'objectCount' => 0, + 'icon' => 'CloseCircleOutline', + ], + ]; + + $objects = $this->shippedObjectCount(); + if ($objects !== null) { + $choices[] = [ + 'id' => self::DEMO_DATASET, + 'label' => 'Example data', + // 🔴 NO NUMBER IN THIS SENTENCE. The wizard runs a card's + // description through the app's translation function, which is a + // literal lookup, so an interpolated count would make the string + // untranslatable and leave a Dutch operator reading English. The + // count travels as `objectCount` and the card renders it as a + // stat, with a label the library translates. + 'description' => ( + 'Sample values for every schema this app supplies, generated from the schemas ' + . 'themselves. It shows the lists, detail pages and dashboards working rather ' + . 'than telling a story. Safe to run more than once, and you can delete it ' + . 'afterwards.' + ), + 'objectCount' => $objects, + 'icon' => 'DatabaseOutline', + ]; + } + + return $choices; + + }//end listChoices() + + /** + * How many objects the shipped descriptor carries, or null when it ships none. + * + * Counted from the FILE, so the card promises the number that will actually + * be imported. A missing or malformed descriptor returns null and the app + * then offers only "None" — honest, rather than an import that cannot run. + * + * @return integer|null The object count, or null when there is no usable descriptor. + */ + private function shippedObjectCount(): ?int { + $path = $this->descriptorPath(); + if (is_file($path) === false) { + return null; + } + + $raw = file_get_contents($path); + if ($raw === false) { + return null; + } + + $data = json_decode($raw, true); + if (is_array($data) === false) { + return null; + } + + $components = ($data['components'] ?? []); + if (is_array($components) === false || is_array(($components['objects'] ?? null)) === false) { + return 0; + } + + return count($components['objects']); + + }//end shippedObjectCount() + /** * Import the demo dataset. * diff --git a/package-lock.json b/package-lock.json index a3daaf4ad..acb411bb6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "EUPL-1.2", "dependencies": { "@codemirror/lang-json": "^6.0.1", - "@conduction/nextcloud-vue": "^2.31.1", + "@conduction/nextcloud-vue": "^2.37.0", "@mdi/js": "^7.4.47", "@nextcloud/auth": "^2.6.0", "@nextcloud/axios": "~2.6.0", @@ -2181,9 +2181,9 @@ } }, "node_modules/@conduction/nextcloud-vue": { - "version": "2.31.1", - "resolved": "https://registry.npmjs.org/@conduction/nextcloud-vue/-/nextcloud-vue-2.31.1.tgz", - "integrity": "sha512-tF1/7yNaBgxj5iHhpNuxVdUrPugfy2ePQZILl7y+fFJTq68tR7WPs8EqPin8aaCBc6ZxgWp1ciyY+NW9rXBMYA==", + "version": "2.37.0", + "resolved": "https://registry.npmjs.org/@conduction/nextcloud-vue/-/nextcloud-vue-2.37.0.tgz", + "integrity": "sha512-3+c+vPHlcswZS0+yEO+HjaFTkgI0ET0Lk33LEmMnZc/E7rDV6lPILq9LNlmxC04WuyQNILx4+zsckkSMYEg7hQ==", "license": "EUPL-1.2", "dependencies": { "@ckpack/vue-color": "^1.6.0", diff --git a/package.json b/package.json index 5467c0628..45fcc7443 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ ], "dependencies": { "@codemirror/lang-json": "^6.0.1", - "@conduction/nextcloud-vue": "^2.31.1", + "@conduction/nextcloud-vue": "^2.37.0", "@mdi/js": "^7.4.47", "@nextcloud/auth": "^2.6.0", "@nextcloud/axios": "~2.6.0", diff --git a/src/manifest.json b/src/manifest.json index 6e7def7e9..416e08c9a 100644 --- a/src/manifest.json +++ b/src/manifest.json @@ -12,11 +12,21 @@ }, { "id": "demo-data", + "type": "choice", + "display": "cards", + "optionsSource": "datasets", + "configKey": "demo_dataset", + "title": "Load example data?", + "required": false, + "body": "Example data fills the lists, detail pages and dashboards so you can see the app working straight away. Pick \"None\" on a production install." + }, + { + "id": "load-demo-data", "type": "run-action", - "action": "install-demo-data", - "title": "Demo data (optional)", + "action": "load-demo-data", + "title": "Load the example data", "required": false, - "body": "Load a small example dataset so the lists, detail pages and dashboards show a working product straight away. The data is obviously sample data, it is safe to run more than once, and it can be removed afterwards. Skip this on a production install." + "body": "Loads what you picked. The data is obviously sample data, it is safe to run more than once, and you can delete it afterwards." }, { "id": "done", diff --git a/tests/Unit/Controller/SetupControllerTest.php b/tests/Unit/Controller/SetupControllerTest.php index 4681d8bc8..3b4c49a2c 100644 --- a/tests/Unit/Controller/SetupControllerTest.php +++ b/tests/Unit/Controller/SetupControllerTest.php @@ -38,38 +38,204 @@ protected function setUp(): void { ); } - public function testStatusReportsTheDemoDataStep(): void { + public function testStatusReportsBothDemoDataSteps(): void { $this->appConfig->method('getValueString')->willReturn(''); + $this->demoData->method('listChoices')->willReturn([]); $data = $this->controller->status()->getData(); // Absence is the defect this guards: a step the wizard is never told // about cannot be offered and cannot be completed. $this->assertArrayHasKey('demo-data', $data['steps']); + $this->assertArrayHasKey('load-demo-data', $data['steps']); $this->assertFalse($data['steps']['demo-data']['done']); + $this->assertFalse($data['steps']['load-demo-data']['done']); // This app declares no REQUIRED step, so setup must never gate the app. $this->assertTrue($data['completed']); $this->assertSame(1, $data['version']); } - public function testStatusReportsTheStepDoneOnceDecided(): void { - $this->appConfig->method('getValueString')->willReturn('skipped'); + public function testStatusCarriesTheOptionListTheChoiceStepReads(): void { + // 🔴 THIS RESPONSE *IS* THE OPTION LIST. The step declares + // `optionsSource: datasets` and carries no options of its own, so a + // dataset missing here is a dataset nobody can pick. + $this->appConfig->method('getValueString')->willReturn(''); + $this->demoData->method('listChoices')->willReturn([ + ['id' => 'none', 'label' => 'None', 'description' => 'Nothing.', 'objectCount' => 0, 'icon' => 'CloseCircleOutline'], + ['id' => 'demo', 'label' => 'Example data', 'description' => 'Sample values.', 'objectCount' => 66, 'icon' => 'DatabaseOutline'], + ]); + + $data = $this->controller->status()->getData(); + + $this->assertSame(['none', 'demo'], array_column($data['datasets'], 'id')); + // A card renders all three; an entry missing one renders a blank card. + $this->assertSame('Sample values.', $data['datasets'][1]['description']); + $this->assertSame(66, $data['datasets'][1]['objectCount']); + $this->assertSame('DatabaseOutline', $data['datasets'][1]['icon']); + } + + public function testChoosingNoneClosesBothStepsWithoutRunningAnything(): void { + // 🔴 THE DEFECT THIS FIXES. Every app in this fleet implemented + // `skip-demo-data` and NO manifest step could reach it, so declining was + // unsayable: the step stayed `done: false` and CnAppRoot reopened the + // wizard over every page, for ever, unless the operator imported data + // they did not want. + $this->appConfig->method('getValueString') + ->willReturnCallback(static fn (string $app, string $key): string + => ($key === 'demo_dataset' ? 'none' : '')); + $this->demoData->method('listChoices')->willReturn([]); $data = $this->controller->status()->getData(); $this->assertTrue($data['steps']['demo-data']['done']); + $this->assertTrue($data['steps']['load-demo-data']['done']); } - public function testSkippingIsAnAnswerAndIsRecorded(): void { - // Declining must be persisted, otherwise the wizard re-offers the import - // on every visit and "no thanks" is impossible to express. + public function testTheChoiceIsPersisted(): void { + $request = $this->createMock(IRequest::class); + $request->method('getParam')->willReturn('demo'); + $controller = new SetupController($request, $this->appConfig, $this->logger, $this->demoData); + $this->demoData->method('listChoices')->willReturn([ + ['id' => 'none', 'label' => 'None', 'description' => '', 'objectCount' => 0, 'icon' => ''], + ['id' => 'demo', 'label' => 'Example data', 'description' => '', 'objectCount' => 66, 'icon' => ''], + ]); + + $this->appConfig->expects($this->once()) + ->method('setValueString') + ->with('integriq', 'demo_dataset', 'demo'); + + $data = $controller->saveConfig()->getData(); + + $this->assertTrue($data['success']); + $this->assertSame('demo', $data['config']['demo_dataset']); + } + + public function testAnUnknownDatasetIsRejectedRatherThanStored(): void { + // Storing it would leave the load step pointing at nothing, so the + // failure would surface one step later with no clue why. + $request = $this->createMock(IRequest::class); + $request->method('getParam')->willReturn('atlantis'); + $controller = new SetupController($request, $this->appConfig, $this->logger, $this->demoData); + $this->demoData->method('listChoices')->willReturn([ + ['id' => 'none', 'label' => 'None', 'description' => '', 'objectCount' => 0, 'icon' => ''], + ]); + + $this->appConfig->expects($this->never())->method('setValueString'); + + $response = $controller->saveConfig(); + + $this->assertSame(400, $response->getStatus()); + $this->assertFalse($response->getData()['success']); + } + + public function testPostingNothingIsNotAnAnswerAndStoresNothing(): void { + // The wizard posts the whole config patch, so a step that has not been + // answered posts no key at all. That is not an error and it is not a + // choice either. + $request = $this->createMock(IRequest::class); + $request->method('getParam')->willReturn(null); + $controller = new SetupController($request, $this->appConfig, $this->logger, $this->demoData); + + $this->appConfig->expects($this->never())->method('setValueString'); + + $data = $controller->saveConfig()->getData(); + + $this->assertTrue($data['success']); + $this->assertSame([], $data['config']); + } + + public function testAListIsAcceptedBecauseTheWizardContractAllowsOne(): void { + // The step is not `multiple`, but the same endpoint serves steps that + // are, so an array must not reach `(string)` and become "Array". + $request = $this->createMock(IRequest::class); + $request->method('getParam')->willReturn(['demo']); + $controller = new SetupController($request, $this->appConfig, $this->logger, $this->demoData); + $this->demoData->method('listChoices')->willReturn([ + ['id' => 'demo', 'label' => 'Example data', 'description' => '', 'objectCount' => 1, 'icon' => ''], + ]); + $this->appConfig->expects($this->once()) ->method('setValueString') - ->with('integriq', 'demo_data_decided', 'skipped'); + ->with('integriq', 'demo_dataset', 'demo'); + + $this->assertTrue($controller->saveConfig()->getData()['success']); + } + + public function testAValueThatIsNotAStringIsRefused(): void { + // The body is whatever the browser posted. A nested array would + // otherwise reach `(string)` and raise a fatal. + $request = $this->createMock(IRequest::class); + $request->method('getParam')->willReturn([['demo']]); + $controller = new SetupController($request, $this->appConfig, $this->logger, $this->demoData); + + $this->appConfig->expects($this->never())->method('setValueString'); + + $this->assertSame(400, $controller->saveConfig()->getStatus()); + } + + public function testChoosingNoneAndThenRunningImportsNothing(): void { + // 🔴 THE LOAD STEP STILL RUNS AFTER "None". It must record the decision + // and import nothing, rather than refusing: refusing would leave the + // step open and reopen the wizard. + $this->appConfig->method('getValueString') + ->willReturnCallback(static fn (string $app, string $key): string + => ($key === 'demo_dataset' ? 'none' : '')); + $this->demoData->expects($this->never())->method('install'); + + $data = $this->controller->runAction('load-demo-data')->getData(); + + $this->assertTrue($data['success']); + $this->assertStringContainsString('No example data', $data['message']); + } + + public function testLoadingWithoutAChoiceRefusesRatherThanGuessing(): void { + // 🔴 NO SILENT DEFAULT. Importing because the operator clicked Run one + // step early would plant example objects nobody asked for. + $this->appConfig->method('getValueString')->willReturn(''); + $this->demoData->expects($this->never())->method('install'); + + $response = $this->controller->runAction('load-demo-data'); + + $this->assertSame(400, $response->getStatus()); + $this->assertFalse($response->getData()['success']); + } + + public function testTheLegacyActionStillImportsTheShippedDataset(): void { + // `install-demo-data` was the id before the step asked WHICH dataset. A + // runbook or script that still posts it must keep working, and it names + // the shipped set by naming itself. + $this->appConfig->method('getValueString')->willReturn(''); + $this->demoData->method('install') + ->willReturn(['objects' => 30, 'registers' => 1, 'schemas' => 4]); + + $data = $this->controller->runAction('install-demo-data')->getData(); + + $this->assertTrue($data['success']); + $this->assertStringContainsString('30', $data['message']); + } + + public function testSkippingClosesBOTHStepsOrTheWizardNeverCloses(): void { + // Declining must be persisted, otherwise the wizard re-offers the import + // on every visit and "no thanks" is impossible to express. + // + // AND IT MUST ANSWER BOTH STEPS. Splitting the single `demo-data` step + // into a choice plus a run-action gives the wizard two outstanding + // steps, and CnAppRoot opens the wizard while ANY optional step is + // outstanding — so closing only the second is the same bug in a new + // shape. + $written = []; + $this->appConfig->method('setValueString') + ->willReturnCallback(static function (string $app, string $key, string $value) use (&$written): bool { + $written[$key] = $value; + + return true; + }); $response = $this->controller->runAction('skip-demo-data'); $this->assertTrue($response->getData()['success']); + $this->assertSame('skipped', $written['demo_data_decided'] ?? null); + $this->assertSame('none', $written['demo_dataset'] ?? null, 'skipping IS choosing none'); } public function testUnknownActionIs404(): void { @@ -79,7 +245,8 @@ public function testUnknownActionIs404(): void { $this->assertFalse($response->getData()['success']); } - public function testInstallReportsHowMuchLanded(): void { + public function testLoadReportsHowMuchLanded(): void { + $this->appConfig->method('getValueString')->willReturn('demo'); $this->demoData->method('install') ->willReturn(['objects' => 30, 'registers' => 1, 'schemas' => 4]); @@ -87,7 +254,7 @@ public function testInstallReportsHowMuchLanded(): void { ->method('setValueString') ->with('integriq', 'demo_data_decided', 'installed'); - $data = $this->controller->runAction('install-demo-data')->getData(); + $data = $this->controller->runAction('load-demo-data')->getData(); $this->assertTrue($data['success']); // A success message that names no count cannot be told apart from an @@ -95,7 +262,8 @@ public function testInstallReportsHowMuchLanded(): void { $this->assertStringContainsString('30', $data['message']); } - public function testAFailedInstallIsReportedAndLeavesTheStepUNDECIDED(): void { + public function testAFailedLoadIsReportedAndLeavesTheStepUNDECIDED(): void { + $this->appConfig->method('getValueString')->willReturn('demo'); $this->demoData->method('install') ->willThrowException(new RuntimeException('OpenRegister is not installed.')); @@ -106,7 +274,7 @@ public function testAFailedInstallIsReportedAndLeavesTheStepUNDECIDED(): void { $this->appConfig->expects($this->never())->method('setValueString'); $this->logger->expects($this->once())->method('error'); - $response = $this->controller->runAction('install-demo-data'); + $response = $this->controller->runAction('load-demo-data'); $this->assertSame(500, $response->getStatus()); $this->assertFalse($response->getData()['success']); diff --git a/tests/Unit/Service/DemoDataServiceTest.php b/tests/Unit/Service/DemoDataServiceTest.php index 3e9a5391c..45227caee 100644 --- a/tests/Unit/Service/DemoDataServiceTest.php +++ b/tests/Unit/Service/DemoDataServiceTest.php @@ -66,6 +66,67 @@ public function testIsAvailableIsTrueWithADescriptor(): void { $this->assertTrue($this->service()->isAvailable()); } + public function testDecliningIsOfferedEvenWhenNoDatasetShips(): void { + // 🔴 "NO THANKS" HAS TO BE SAYABLE. Every app in this fleet implemented a + // `skip-demo-data` action that no manifest step could reach, so the step + // stayed outstanding and CnAppRoot reopened the wizard over every page + // unless the operator imported data they did not want. + $choices = $this->service()->listChoices(); + + $this->assertSame(['none'], array_column($choices, 'id')); + $this->assertNotSame('', $choices[0]['description']); + $this->assertNotSame('', $choices[0]['icon']); + } + + public function testTheShippedDatasetIsOfferedWithTheCountItActuallyCarries(): void { + // The card promises a number, so the number has to come from the file + // that will be imported rather than from a manifest that could disagree + // with it. + file_put_contents( + $this->descriptor(), + json_encode(['components' => ['objects' => [['a' => 1], ['b' => 2], ['c' => 3]]]]) + ); + + $choices = $this->service()->listChoices(); + + $this->assertSame(['none', 'demo'], array_column($choices, 'id')); + $this->assertSame(3, $choices[1]['objectCount']); + $this->assertNotSame('', $choices[1]['label']); + $this->assertNotSame('', $choices[1]['description']); + } + + public function testAMalformedDescriptorOffersNothingRatherThanAnImportThatCannotRun(): void { + file_put_contents($this->descriptor(), 'not json at all'); + + $this->assertSame(['none'], array_column($this->service()->listChoices(), 'id')); + } + + public function testTheOfferedDescriptionCarriesNoNumber(): void { + // 🔴 THE WIZARD TRANSLATES A CARD'S DESCRIPTION BY LITERAL LOOKUP. A + // count interpolated into the sentence would make it untranslatable and + // leave a Dutch operator reading English. The count travels separately, + // as `objectCount`. + file_put_contents( + $this->descriptor(), + json_encode(['components' => ['objects' => [['a' => 1]]]]) + ); + + $demo = $this->service()->listChoices()[1]; + + $this->assertDoesNotMatchRegularExpression('/\d/', $demo['description']); + } + + public function testADescriptorWithNoObjectsBlockOffersTheSetWithNoCount(): void { + // A descriptor can ship schemas and no objects. That is a real dataset + // with nothing to count, not a broken file, so it stays on offer. + file_put_contents($this->descriptor(), json_encode(['components' => ['schemas' => []]])); + + $choices = $this->service()->listChoices(); + + $this->assertSame(['none', 'demo'], array_column($choices, 'id')); + $this->assertSame(0, $choices[1]['objectCount']); + } + public function testInstallThrowsWhenNoDatasetShips(): void { $this->expectException(RuntimeException::class); $this->expectExceptionMessageMatches('/No demo dataset/');