diff --git a/.gitignore b/.gitignore index 5b8b0d3c..ef68e426 100644 --- a/.gitignore +++ b/.gitignore @@ -96,3 +96,4 @@ openspec/test-site-results/**/*.webp /test-results/ /.playwright/ /playwright/.cache/ +tests/e2e/.auth/ diff --git a/lib/Controller/ApplicationsController.php b/lib/Controller/ApplicationsController.php index 4f7ca1d4..c1444182 100644 --- a/lib/Controller/ApplicationsController.php +++ b/lib/Controller/ApplicationsController.php @@ -1189,24 +1189,102 @@ private function buildRewriteMap(array $companions, string $newSlug): array */ private function provisionPerAppRegister(string $newSlug, string $ownerUid): \OCA\OpenRegister\Db\Register { - $registerSlug = 'openbuilt-'.$newSlug; + // Cross-user collision guard (#77): OR's register slugs are + // organisation-wide unique, so two callers wanting the same + // template+slug would otherwise share a register. The first + // attempt at scoping always namespaced by owner, but that + // breaks every existing single-tenant install (their existing + // registers are still at `openbuilt-{slug}`). So we keep the + // legacy un-namespaced slug AS LONG AS the existing register + // belongs to the caller; otherwise fall back to the + // owner-namespaced form. + $legacyRegisterSlug = 'openbuilt-'.$newSlug; try { - return $this->registerMapper->find($registerSlug, _multitenancy: false); + $existing = $this->registerMapper->find($legacyRegisterSlug, _multitenancy: false); + + $existingOwner = $this->extractRegisterOwner($existing); + if ($existingOwner === '' || $existingOwner === $ownerUid) { + return $existing; + } + + // Different user owns the org-wide slug — namespace ours. + return $this->findOrCreateRegister( + slug: 'openbuilt-'.$ownerUid.'-'.$newSlug, + appSlug: $newSlug, + ownerUid: $ownerUid, + ); + } catch (Throwable) { + // Legacy slug not taken — claim it for this user. + } + + return $this->findOrCreateRegister( + slug: $legacyRegisterSlug, + appSlug: $newSlug, + ownerUid: $ownerUid, + ); + }//end provisionPerAppRegister() + + /** + * Find or create a per-app register at an exact slug. + * + * @param string $slug The register slug to find/create + * @param string $appSlug The application slug (for the title) + * @param string $ownerUid The owner UID (for the description audit trail) + * + * @return \OCA\OpenRegister\Db\Register + */ + private function findOrCreateRegister( + string $slug, + string $appSlug, + string $ownerUid, + ): \OCA\OpenRegister\Db\Register { + try { + return $this->registerMapper->find($slug, _multitenancy: false); } catch (Throwable) { - // Register does not exist yet — create it. + // Not found — create it. } return $this->registerMapper->createFromArray( [ - 'slug' => $registerSlug, - 'title' => 'OpenBuilt — '.$newSlug, - 'description' => 'Per-app schema namespace for OpenBuilt app `'.$newSlug.'` (owner: '.$ownerUid.').', + 'slug' => $slug, + 'title' => 'OpenBuilt — '.$appSlug, + 'description' => 'Per-app schema namespace for OpenBuilt app `'.$appSlug.'` (owner: '.$ownerUid.').', 'version' => '0.1.0', 'schemas' => [], ] ); - }//end provisionPerAppRegister() + }//end findOrCreateRegister() + + /** + * Extract the owner UID from a Register entity, tolerating either an + * `owner` field on the entity itself or one inside an `@self` block. + * + * @param mixed $register The Register entity (or non-Register junk). + * + * @return string The owner UID, or empty string when not determinable. + */ + private function extractRegisterOwner(mixed $register): string + { + if (is_object($register) === true && method_exists($register, 'getOwner') === true) { + $owner = $register->getOwner(); + if (is_string($owner) === true) { + return $owner; + } + } + + if (is_object($register) === true && method_exists($register, 'jsonSerialize') === true) { + $data = $register->jsonSerialize(); + if (is_array($data) === true) { + $owner = ($data['owner'] ?? ($data['@self']['owner'] ?? null)); + if (is_string($owner) === true) { + return $owner; + } + } + } + + return ''; + }//end extractRegisterOwner() /** * Clone companion schemas into the per-app register. diff --git a/lib/Service/ApplicationCreationService.php b/lib/Service/ApplicationCreationService.php index 4ff2707e..67b069f4 100644 --- a/lib/Service/ApplicationCreationService.php +++ b/lib/Service/ApplicationCreationService.php @@ -207,9 +207,10 @@ public function createApplication(array $payload): string $registerSlug = 'openbuilt-'.$appSlug.'-'.$versionSlug; // 3a: Create ApplicationVersion - $versionManifest = $this->substituteRegisterSlug( + $versionManifest = $this->substituteVersionContext( manifest: $defaultManifest, - registerSlug: $registerSlug + registerSlug: $registerSlug, + schemaSlugPrefix: $appSlug.'-'.$versionSlug.'-' ); $versionPayload = [ @@ -815,6 +816,35 @@ private function loadDefaultSchemas(): array */ public function substituteRegisterSlug(array $manifest, string $registerSlug): array { + return $this->substituteVersionContext( + manifest: $manifest, + registerSlug: $registerSlug, + schemaSlugPrefix: '' + ); + }//end substituteRegisterSlug() + + /** + * Substitute the per-version context tokens in a manifest template. + * + * Walks all `pages[*].config` blocks. Replaces the `{registerSlug}` + * token in `register` and rewrites every non-empty `config.schema` + * to the namespaced seed slug `{schemaSlugPrefix}{originalSchemaSlug}` + * so the manifest references the actual per-version schemas created + * by {@see provisionRegister()} (openbuilt#75 — without this the + * KPI / insights cards aggregated against a non-existent schema and + * leaked the same numbers across all tiers). + * + * @param array $manifest The manifest template blob + * @param string $registerSlug The per-version register slug + * @param string $schemaSlugPrefix Namespaced prefix for schema slugs (e.g. `permit-flow-development-`) + * + * @return array The manifest with tokens substituted + */ + public function substituteVersionContext( + array $manifest, + string $registerSlug, + string $schemaSlugPrefix + ): array { if (isset($manifest['pages']) === false || is_array($manifest['pages']) === false) { return $manifest; } @@ -831,11 +861,20 @@ public function substituteRegisterSlug(array $manifest, string $registerSlug): a if (isset($page['config']['register']) === true && $page['config']['register'] === '{registerSlug}') { $page['config']['register'] = $registerSlug; } + + if ($schemaSlugPrefix !== '' + && isset($page['config']['schema']) === true + && is_string($page['config']['schema']) === true + && $page['config']['schema'] !== '' + && str_starts_with($page['config']['schema'], $schemaSlugPrefix) === false + ) { + $page['config']['schema'] = $schemaSlugPrefix.$page['config']['schema']; + } } unset($page); return $manifest; - }//end substituteRegisterSlug() + }//end substituteVersionContext() /** * Get the UID of the currently authenticated user. diff --git a/playwright.config.ts b/playwright.config.ts index d3516d7a..de894437 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -18,21 +18,32 @@ export default defineConfig({ testDir: 'tests/e2e', timeout: 30_000, expect: { timeout: 5_000 }, - fullyParallel: true, + // Don't run specs in parallel: Nextcloud's brute-force throttle fires + // after a handful of near-simultaneous form logins from the same IP + // and every subsequent spec falls back to the /login page. Serial + // execution with one shared storageState (via globalSetup) avoids it. + fullyParallel: false, forbidOnly: !!process.env.CI, retries: process.env.CI ? 1 : 0, - workers: process.env.CI ? 1 : undefined, + workers: 1, + globalSetup: './tests/e2e/global-setup.ts', reporter: process.env.CI ? [['github'], ['html', { open: 'never' }]] : 'list', use: { baseURL: process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:8080', + // Authenticated browser context populated by globalSetup. Empty + // when login fails — specs then surface the actual login page in + // their failure snapshots. + storageState: 'tests/e2e/.auth/admin.json', httpCredentials: { username: process.env.NC_ADMIN_USER || 'admin', password: process.env.NC_ADMIN_PASSWORD || 'admin', }, - extraHTTPHeaders: { - // Nextcloud rejects unauthenticated OCS/API calls without this header. - 'OCS-APIRequest': 'true', - }, + // Note: do NOT set `OCS-APIRequest: true` here globally. The header + // makes Nextcloud treat every request as an API call — including the + // HTML page loads — which breaks the browser-based login redirect + // (no Location header is emitted, the page stays on /login). + // Specs that need OCS-APIRequest set it on their explicit `request` + // calls (e.g. versionRouting.spec.ts, applicationDetailOverview.spec.ts). trace: 'on-first-retry', screenshot: 'only-on-failure', video: 'retain-on-failure', diff --git a/tests/Unit/Repair/MigrateToVersionedModelTest.php b/tests/Unit/Repair/MigrateToVersionedModelTest.php index 9bcbe4ba..c3f2fc45 100644 --- a/tests/Unit/Repair/MigrateToVersionedModelTest.php +++ b/tests/Unit/Repair/MigrateToVersionedModelTest.php @@ -121,16 +121,43 @@ private function step(): MigrateToVersionedModel }//end step() /** - * Short-circuit: versioned schema already present → no deletions. + * Short-circuit: no Application rows carry pre-spec-C shape → no deletions. + * + * The previous check probed for the `applicationVersion` schema's + * existence, but `InitializeSettings` imports that schema BEFORE + * this step runs, so the probe always fired true and the migration + * was skipped (openbuilt#69). The new check inspects Application + * row shape and only proceeds when at least one row still carries + * a legacy `manifest` / `version` / `status` / `currentVersion` + * top-level field. * * @return void */ public function testShortCircuitsWhenVersionedSchemaPresent(): void { - // schemaMapper->find('applicationVersion', ...) succeeds (no throw) on first call. - $this->schemaMapper->method('find')->willReturn($this->createMock(Schema::class)); + // Register + schema lookups succeed. + $register = $this->getMockBuilder(Register::class) + ->disableOriginalConstructor() + ->addMethods(['getId']) + ->getMock(); + $register->method('getId')->willReturn(1); + $this->registerMapper->method('find')->willReturn($register); + + $this->schemaMapper->method('find')->willReturn( + $this->getMockBuilder(Schema::class) + ->disableOriginalConstructor() + ->addMethods(['getId']) + ->getMock() + ); + + // Application rows already match the post-C shape (no legacy keys). + $postCRows = [ + $this->mockEntity(['id' => 'uuid-a', 'slug' => 'app-a', 'name' => 'App A']), + ]; + $this->objectService->method('findAll')->willReturn($postCRows); - $this->objectService->expects(self::never())->method('findAll'); + // No deletions should fire: rows look post-C so the short-circuit + // returns true and `run()` exits before touching either service. $this->objectService->expects(self::never())->method('deleteObject'); $this->registerService->expects(self::never())->method('delete'); diff --git a/tests/Unit/Service/ApplicationCreationServiceTest.php b/tests/Unit/Service/ApplicationCreationServiceTest.php index cf494d33..b214abda 100644 --- a/tests/Unit/Service/ApplicationCreationServiceTest.php +++ b/tests/Unit/Service/ApplicationCreationServiceTest.php @@ -171,6 +171,94 @@ public function substituteRegisterSlugDoesNotTouchNonTokenFields(): void self::assertSame('some-other-register', $result['pages'][0]['config']['register']); }//end substituteRegisterSlugDoesNotTouchNonTokenFields() + /** + * substituteVersionContext rewrites both register AND schema. + * + * openbuilt#75 — without this the KPI / insights cards aggregate + * against `hello-message` (the un-namespaced template slug), which + * doesn't exist in the per-version register, so counts leak the + * same numbers across all tiers. + * + * @test + * + * @return void + */ + public function substituteVersionContextNamespacesSchemaSlugAlongsideRegister(): void + { + $manifest = [ + 'pages' => [ + ['id' => 'Dashboard', 'config' => ['widgets' => []]], + ['id' => 'Messages', 'config' => ['register' => '{registerSlug}', 'schema' => 'hello-message']], + ], + ]; + + $result = $this->service->substituteVersionContext( + manifest: $manifest, + registerSlug: 'openbuilt-permit-flow-development', + schemaSlugPrefix: 'permit-flow-development-' + ); + + self::assertSame('openbuilt-permit-flow-development', $result['pages'][1]['config']['register']); + self::assertSame('permit-flow-development-hello-message', $result['pages'][1]['config']['schema']); + }//end substituteVersionContextNamespacesSchemaSlugAlongsideRegister() + + /** + * substituteVersionContext is idempotent: re-running with the same + * prefix MUST NOT double-prefix the schema slug. + * + * @test + * + * @return void + */ + public function substituteVersionContextIsIdempotentOnAlreadyNamespacedSlugs(): void + { + $manifest = [ + 'pages' => [ + [ + 'id' => 'Messages', + 'config' => [ + 'register' => 'openbuilt-permit-flow-development', + 'schema' => 'permit-flow-development-hello-message', + ], + ], + ], + ]; + + $result = $this->service->substituteVersionContext( + manifest: $manifest, + registerSlug: 'openbuilt-permit-flow-development', + schemaSlugPrefix: 'permit-flow-development-' + ); + + // Schema must NOT become `permit-flow-development-permit-flow-development-hello-message`. + self::assertSame('permit-flow-development-hello-message', $result['pages'][0]['config']['schema']); + }//end substituteVersionContextIsIdempotentOnAlreadyNamespacedSlugs() + + /** + * substituteVersionContext leaves schema alone when no prefix is provided + * (backwards-compat with the legacy substituteRegisterSlug entry point). + * + * @test + * + * @return void + */ + public function substituteVersionContextLeavesSchemaAloneWhenPrefixEmpty(): void + { + $manifest = [ + 'pages' => [ + ['id' => 'Messages', 'config' => ['register' => '{registerSlug}', 'schema' => 'hello-message']], + ], + ]; + + $result = $this->service->substituteVersionContext( + manifest: $manifest, + registerSlug: 'openbuilt-my-app-production', + schemaSlugPrefix: '' + ); + + self::assertSame('hello-message', $result['pages'][0]['config']['schema']); + }//end substituteVersionContextLeavesSchemaAloneWhenPrefixEmpty() + // ------------------------------------------------------------------------- // resolveVersionChain // ------------------------------------------------------------------------- diff --git a/tests/e2e/applicationCard.spec.ts b/tests/e2e/applicationCard.spec.ts index 019ccdf9..bedd0979 100644 --- a/tests/e2e/applicationCard.spec.ts +++ b/tests/e2e/applicationCard.spec.ts @@ -26,7 +26,7 @@ import { test, expect } from '@playwright/test' test.describe('ApplicationCard — icon + productionVersion fields (spec A / spec C)', () => { test('index page renders ApplicationCards with icon elements', async ({ page }) => { - await page.goto('/index.php/apps/openbuilt') + await page.goto('/index.php/apps/openbuilt/applications') // Wait for the SPA to hydrate and the Applications list to appear. // The list renders one card per Application. The seeded hello-world @@ -46,7 +46,7 @@ test.describe('ApplicationCard — icon + productionVersion fields (spec A / spe }) test('hello-world ApplicationCard shows a status badge (not raw "Live" chip)', async ({ page }) => { - await page.goto('/index.php/apps/openbuilt') + await page.goto('/index.php/apps/openbuilt/applications') // Wait for at least the seeded hello-world card. await expect( @@ -66,7 +66,7 @@ test.describe('ApplicationCard — icon + productionVersion fields (spec A / spe }) test('hello-world ApplicationCard status badge is one of the known values', async ({ page }) => { - await page.goto('/index.php/apps/openbuilt') + await page.goto('/index.php/apps/openbuilt/applications') // Find the card for hello-world specifically. const helloCard = page.locator('[data-slug="hello-world"], .ob-app-card').first() @@ -85,7 +85,7 @@ test.describe('ApplicationCard — icon + productionVersion fields (spec A / spe }) test('hello-world ApplicationCard version chip shows semver or — placeholder', async ({ page }) => { - await page.goto('/index.php/apps/openbuilt') + await page.goto('/index.php/apps/openbuilt/applications') const helloCard = page.locator('[data-slug="hello-world"], .ob-app-card').first() await expect(helloCard).toBeVisible({ timeout: 15_000 }) diff --git a/tests/e2e/export-zip.spec.ts b/tests/e2e/export-zip.spec.ts index 207f5d3f..d7efb0f0 100644 --- a/tests/e2e/export-zip.spec.ts +++ b/tests/e2e/export-zip.spec.ts @@ -41,7 +41,8 @@ test.describe('OpenBuilt ZIP export', () => { await page.fill('input[name="user"]', ADMIN_USER) await page.fill('input[name="password"]', ADMIN_PASSWORD) await page.locator('button[type="submit"], input[type="submit"]').first().click() - await page.waitForURL(/\/index\.php\/apps\//, { timeout: 15_000 }) + // Pretty URLs in modern NC drop the `/index.php` prefix; accept both. + await page.waitForURL(/\/apps\//, { timeout: 15_000 }) } }) diff --git a/tests/e2e/global-setup.ts b/tests/e2e/global-setup.ts new file mode 100644 index 00000000..4eb08d2a --- /dev/null +++ b/tests/e2e/global-setup.ts @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: EUPL-1.2 +// SPDX-FileCopyrightText: 2026 Conduction B.V. + +/** + * Playwright globalSetup — logs in to Nextcloud once and writes the + * authenticated browser context (cookies + localStorage) to + * `tests/e2e/.auth/admin.json`. Every spec then reuses that storage + * state via the project-level `storageState` config, so per-spec + * `loginAsAdmin` helpers are no longer required. + * + * Before this hook existed, specs without an explicit form-login step + * (applicationCard, builder-host, bootstrap-openbuilt, …) landed on + * `/login` and every locator timed out. Nextcloud's session is cookie- + * based; basic auth alone doesn't satisfy the SPA's auth check. + * + * The hook is no-op when login fails (e.g. brute-force throttle); the + * resulting empty storage state lets specs surface real errors with a + * clear "still on /login" snapshot in the report. + */ + +import { chromium, FullConfig } from '@playwright/test' +import { existsSync, mkdirSync } from 'fs' +import { dirname } from 'path' + +export default async function globalSetup(config: FullConfig): Promise { + const baseURL = (config.projects[0].use.baseURL as string) + || process.env.PLAYWRIGHT_BASE_URL + || 'http://localhost:8080' + const adminUser = process.env.NC_ADMIN_USER || 'admin' + const adminPassword = process.env.NC_ADMIN_PASSWORD || process.env.NC_ADMIN_PASS || 'admin' + const storagePath = 'tests/e2e/.auth/admin.json' + + if (existsSync(dirname(storagePath)) === false) { + mkdirSync(dirname(storagePath), { recursive: true }) + } + + const browser = await chromium.launch() + const context = await browser.newContext({ baseURL }) + const page = await context.newPage() + + try { + await page.goto('/index.php/login', { waitUntil: 'domcontentloaded' }) + await page.locator('input[name="user"]').fill(adminUser) + await page.locator('input[name="password"]').fill(adminPassword) + await page.locator('button[type="submit"], input[type="submit"]').first().click() + // Accept both pretty + index.php-prefixed redirects. + await page.waitForURL(/\/apps\//, { timeout: 20_000 }) + await context.storageState({ path: storagePath }) + // eslint-disable-next-line no-console + console.log(`[globalSetup] authenticated session stored at ${storagePath}`) + } catch (e) { + // eslint-disable-next-line no-console + console.warn(`[globalSetup] login failed — specs will run unauthenticated: ${(e as Error).message}`) + } finally { + await browser.close() + } +} diff --git a/tests/e2e/iconUpload.spec.ts b/tests/e2e/iconUpload.spec.ts index c597b1d5..038cfede 100644 --- a/tests/e2e/iconUpload.spec.ts +++ b/tests/e2e/iconUpload.spec.ts @@ -43,7 +43,10 @@ test.describe('Icon upload on Application detail page (spec A task 7.5)', () => test('Icon tab is accessible from the Application detail page', async ({ page }) => { // Navigate to the Applications list, then open the hello-world detail. - await page.goto('/index.php/apps/openbuilt') + // Land on the Virtual apps index — the ApplicationCards live at + // `/applications`, not at the app root (which redirects to the + // Dashboard widget page). + await page.goto('/index.php/apps/openbuilt/applications') await expect(page.locator('.ob-app-card, [data-testid*="app-card"]').first()).toBeVisible({ timeout: 15_000 }) // Navigate to the hello-world detail page. The card links to @@ -81,7 +84,10 @@ test.describe('Icon upload on Application detail page (spec A task 7.5)', () => }) test('uploading a minimal SVG updates the preview src', async ({ page }) => { - await page.goto('/index.php/apps/openbuilt') + // Land on the Virtual apps index — the ApplicationCards live at + // `/applications`, not at the app root (which redirects to the + // Dashboard widget page). + await page.goto('/index.php/apps/openbuilt/applications') await expect(page.locator('.ob-app-card, [data-testid*="app-card"]').first()).toBeVisible({ timeout: 15_000 }) // Open the hello-world detail. diff --git a/tests/e2e/template-gallery.spec.ts b/tests/e2e/template-gallery.spec.ts index b7495e49..406d2079 100644 --- a/tests/e2e/template-gallery.spec.ts +++ b/tests/e2e/template-gallery.spec.ts @@ -36,7 +36,10 @@ async function loginAsAdmin(page: Page): Promise { await page.locator('input[name="user"]').fill(ADMIN_USER) await page.locator('input[name="password"]').fill(ADMIN_PASS) await page.locator('button[type="submit"]').click() - await page.waitForURL(/index\.php\/apps\//, { timeout: 15_000 }) + // Accept both pretty URLs (`/apps/dashboard/`) and the legacy form + // (`/index.php/apps/dashboard/`) — pretty-URL rewrites are enabled in + // modern NC installs and the redirect drops the `/index.php` prefix. + await page.waitForURL(/\/apps\//, { timeout: 15_000 }) } test.describe('OpenBuilt template gallery', () => { diff --git a/tests/integration/openbuilt.postman_collection.json b/tests/integration/openbuilt.postman_collection.json index 80f794ea..6f5bce84 100644 --- a/tests/integration/openbuilt.postman_collection.json +++ b/tests/integration/openbuilt.postman_collection.json @@ -149,10 +149,10 @@ ], "body": { "mode": "raw", - "raw": "{\n \"slug\": \"newman-roundtrip\",\n \"name\": \"Newman Round-trip\",\n \"description\": \"Synthetic Application created by the Newman CRUD round-trip\",\n \"version\": \"0.0.1\",\n \"status\": \"draft\",\n \"manifest\": {\n \"version\": \"1.0.0\",\n \"menu\": [],\n \"pages\": [\n {\n \"id\": \"Home\",\n \"route\": \"/\",\n \"type\": \"index\",\n \"title\": \"Home\",\n \"config\": { \"register\": \"openbuilt\", \"schema\": \"hello-message\", \"columns\": [\"title\"] }\n }\n ]\n }\n}" + "raw": "{\n \"slug\": \"newman-roundtrip\",\n \"name\": \"Newman Round-trip\",\n \"description\": \"Synthetic Application created by the Newman CRUD round-trip\"\n}" }, "url": "{{base_url}}/index.php/apps/openregister/api/objects/openbuilt/application", - "description": "Create a synthetic Application object via OR REST (ADR-022: CRUD goes to OR directly, not an OB wrapper)." + "description": "Create a synthetic Application object via OR REST. Per ADR-002 (versioned-app model) the `manifest`/`version`/`status` fields live on ApplicationVersion now, NOT the Application itself." }, "event": [ { @@ -187,7 +187,7 @@ { "key": "OCS-APIRequest", "value": "true" } ], "url": "{{base_url}}/index.php/apps/openregister/api/objects/openbuilt/application/{{created_uuid}}", - "description": "Read-back asserts the POST landed on disk and the uuid resolves." + "description": "Read-back asserts the POST landed on disk and the uuid resolves. Versioned model: no manifest at this layer; verified separately on the version record below." }, "event": [ { @@ -201,9 +201,45 @@ "const body = pm.response.json();", "pm.test('slug round-trips', function () {", " pm.expect(body.slug || (body['@self'] && body['@self'].slug)).to.eql('newman-roundtrip');", + "});" + ] + } + } + ] + }, + { + "name": "POST creates an ApplicationVersion under the new Application", + "request": { + "method": "POST", + "header": [ + { "key": "Accept", "value": "application/json" }, + { "key": "Content-Type", "value": "application/json" }, + { "key": "OCS-APIRequest", "value": "true" } + ], + "body": { + "mode": "raw", + "raw": "{\n \"name\": \"Newman Round-trip v1\",\n \"slug\": \"production\",\n \"manifest\": {\n \"version\": \"1.0.0\",\n \"menu\": [],\n \"pages\": [\n {\n \"id\": \"Home\",\n \"route\": \"/\",\n \"type\": \"index\",\n \"title\": \"Home\",\n \"config\": { \"register\": \"openbuilt\", \"schema\": \"hello-message\", \"columns\": [\"title\"] }\n }\n ]\n },\n \"register\": \"openbuilt-newman-roundtrip-production\",\n \"semver\": \"1.0.0\",\n \"status\": \"draft\",\n \"application\": \"{{created_uuid}}\"\n}" + }, + "url": "{{base_url}}/index.php/apps/openbuilt/api/applications/newman-roundtrip/versions", + "description": "Per ADR-002, the manifest + status + semver live on ApplicationVersion. POST against the version-aware OpenBuilt endpoint so the parent-relation guard runs." + }, + "event": [ + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test('returns 200 or 201', function () {", + " pm.expect([200, 201]).to.include(pm.response.code);", + "});", + "const body = pm.response.json();", + "const versionUuid = (body['@self'] && (body['@self'].id || body['@self'].uuid)) || body.uuid || body.id;", + "pm.test('version uuid is captured', function () {", + " pm.expect(versionUuid).to.be.a('string');", "});", - "pm.test('manifest is present and not empty', function () {", - " pm.expect(body.manifest, 'no manifest on GET').to.be.an('object');", + "pm.collectionVariables.set('created_version_uuid', versionUuid);", + "pm.test('manifest persisted on the version (not the application)', function () {", + " pm.expect(body.manifest).to.be.an('object');", "});" ] } @@ -211,7 +247,7 @@ ] }, { - "name": "PUT saves a manifest edit before publish (newman-roundtrip)", + "name": "PATCH set the Application's productionVersion pointer", "request": { "method": "PUT", "header": [ @@ -221,10 +257,10 @@ ], "body": { "mode": "raw", - "raw": "{\n \"slug\": \"newman-roundtrip\",\n \"name\": \"Newman Round-trip\",\n \"description\": \"Synthetic Application created by the Newman CRUD round-trip\",\n \"version\": \"0.0.2\",\n \"status\": \"draft\",\n \"manifest\": {\n \"version\": \"1.0.0\",\n \"menu\": [],\n \"pages\": [\n {\n \"id\": \"Home\",\n \"route\": \"/\",\n \"type\": \"index\",\n \"title\": \"Home\",\n \"config\": { \"register\": \"openbuilt\", \"schema\": \"hello-message\", \"columns\": [\"title\"] }\n }\n ]\n }\n}" + "raw": "{\n \"slug\": \"newman-roundtrip\",\n \"name\": \"Newman Round-trip\",\n \"description\": \"Synthetic Application created by the Newman CRUD round-trip\",\n \"productionVersion\": \"{{created_version_uuid}}\"\n}" }, "url": "{{base_url}}/index.php/apps/openregister/api/objects/openbuilt/application/{{created_uuid}}", - "description": "Save pending manifest edits while still in draft (mirrors ApplicationEditor.vue's publish() step 1). The publish happens via the lifecycle transition below, not by flipping status here — a raw status=published PUT would never fire ObjectTransitionedEvent and so the BuiltAppRoute would never be created (REQ-OBA-003 / REQ-OBA-004)." + "description": "ADR-002: the application's `productionVersion` field carries the UUID of the version that should be served as the live manifest." }, "event": [ { @@ -236,8 +272,9 @@ " pm.response.to.have.status(200);", "});", "const body = pm.response.json();", - "pm.test('still in draft (publish happens via lifecycle transition)', function () {", - " pm.expect(body.status || (body['@self'] && body['@self'].status)).to.eql('draft');", + "const prod = body.productionVersion || (body['@self'] && body['@self'].relations && body['@self'].relations.productionVersion);", + "pm.test('productionVersion points at the new version', function () {", + " pm.expect(prod).to.eql(pm.collectionVariables.get('created_version_uuid'));", "});" ] } @@ -245,7 +282,7 @@ ] }, { - "name": "POST the publish lifecycle transition (fires ObjectTransitionedEvent)", + "name": "POST publish lifecycle transition on the ApplicationVersion", "request": { "method": "POST", "header": [ @@ -257,8 +294,8 @@ "mode": "raw", "raw": "{ \"action\": \"publish\" }" }, - "url": "{{base_url}}/index.php/apps/openregister/api/objects/{{created_uuid}}/transition", - "description": "A raw PUT with status=published does NOT make OR fire ObjectTransitionedEvent, so the BuiltAppRoute (slug -> uuid) that GET /api/applications/{slug}/manifest resolves is never created. OR's lifecycle transition endpoint (POST /api/objects/{id}/transition with {action}) fires the event -> ApplicationVersionSnapshotListener upserts the BuiltAppRoute (REQ-OBA-003 / REQ-OBA-004). This is the server-side equivalent of ApplicationEditor.vue's publish() flow." + "url": "{{base_url}}/index.php/apps/openregister/api/objects/{{created_version_uuid}}/transition", + "description": "Publishing is a state machine on the VERSION now (ADR-002). The transition fires ObjectTransitionedEvent → ApplicationVersionSnapshotListener upserts the BuiltAppRoute." }, "event": [ { @@ -283,7 +320,7 @@ { "key": "OCS-APIRequest", "value": "true" } ], "url": "{{base_url}}/index.php/apps/openbuilt/api/applications/newman-roundtrip/manifest", - "description": "After the publish transition, the BuiltAppRoute is upserted and the manifest endpoint resolves the new slug." + "description": "After the publish transition + productionVersion pointer set, ManifestResolverService should resolve the slug to the version's manifest." }, "event": [ { @@ -307,6 +344,31 @@ } ] }, + { + "name": "DELETE cleans up the round-trip ApplicationVersion", + "request": { + "method": "DELETE", + "header": [ + { "key": "Accept", "value": "application/json" }, + { "key": "OCS-APIRequest", "value": "true" } + ], + "url": "{{base_url}}/index.php/apps/openbuilt/api/applications/newman-roundtrip/versions/production?strategy=hard", + "description": "Drop the version + its per-version register first so the parent Application can be safely deleted." + }, + "event": [ + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test('returns 200 or 204', function () {", + " pm.expect([200, 204]).to.include(pm.response.code);", + "});" + ] + } + } + ] + }, { "name": "DELETE cleans up the round-trip Application", "request": { diff --git a/tests/store/schemas.spec.js b/tests/store/schemas.spec.js index 2a49e6ad..8e5b8074 100644 --- a/tests/store/schemas.spec.js +++ b/tests/store/schemas.spec.js @@ -76,7 +76,8 @@ describe('useSchemasStore — re-registers when register changes', () => { expect(registerObjectType).toHaveBeenCalledWith( 'schema', 'schemas', - 'openbuilt-hello-world-staging', + 'api', + { registerSlug: 'openbuilt-hello-world-staging' }, ) }) @@ -100,7 +101,8 @@ describe('useSchemasStore — re-registers when register changes', () => { expect(registerObjectType).toHaveBeenCalledWith( 'schema', 'schemas', - 'openbuilt-hello-world', + 'api', + { registerSlug: 'openbuilt-hello-world' }, ) }) @@ -122,14 +124,24 @@ describe('useSchemasStore — re-registers when register changes', () => { // First call — registers to staging useSchemasStore('hello-world', 'staging') expect(registerObjectType).toHaveBeenCalledTimes(1) - expect(registerObjectType).toHaveBeenLastCalledWith('schema', 'schemas', 'openbuilt-hello-world-staging') + expect(registerObjectType).toHaveBeenLastCalledWith( + 'schema', + 'schemas', + 'api', + { registerSlug: 'openbuilt-hello-world-staging' }, + ) // Simulate version change — objectTypeRegistry now has the old register recorded - fakeStore.objectTypeRegistry.schema = { register: 'openbuilt-hello-world-staging' } + fakeStore.objectTypeRegistry.schema = { slugs: { registerSlug: 'openbuilt-hello-world-staging' } } // Second call with different version — should re-register useSchemasStore('hello-world', 'production') expect(registerObjectType).toHaveBeenCalledTimes(2) - expect(registerObjectType).toHaveBeenLastCalledWith('schema', 'schemas', 'openbuilt-hello-world-production') + expect(registerObjectType).toHaveBeenLastCalledWith( + 'schema', + 'schemas', + 'api', + { registerSlug: 'openbuilt-hello-world-production' }, + ) }) }) diff --git a/tests/unit/Controller/ApplicationsControllerDiffTest.php b/tests/unit/Controller/ApplicationsControllerDiffTest.php index 8b8ad757..b60d8e6a 100644 --- a/tests/unit/Controller/ApplicationsControllerDiffTest.php +++ b/tests/unit/Controller/ApplicationsControllerDiffTest.php @@ -29,6 +29,7 @@ namespace OCA\OpenBuilt\Tests\Unit\Controller; use OCA\OpenBuilt\Controller\ApplicationsController; +use OCA\OpenBuilt\Service\ManifestResolverService; use OCA\OpenRegister\Db\ObjectEntity; use OCA\OpenRegister\Db\Register; use OCA\OpenRegister\Db\RegisterMapper; @@ -119,6 +120,7 @@ protected function setUp(): void schemaMapper: $schemaMapper, userSession: $userSession, groupManager: $groupManager, + manifestResolver: $this->createMock(ManifestResolverService::class), ); }//end setUp() diff --git a/tests/unit/Controller/ApplicationsControllerTest.php b/tests/unit/Controller/ApplicationsControllerTest.php index 0ee79927..af6ceff3 100644 --- a/tests/unit/Controller/ApplicationsControllerTest.php +++ b/tests/unit/Controller/ApplicationsControllerTest.php @@ -23,6 +23,7 @@ namespace OCA\OpenBuilt\Tests\Unit\Controller; use OCA\OpenBuilt\Controller\ApplicationsController; +use OCA\OpenBuilt\Service\ManifestResolverService; use OCA\OpenRegister\Db\AuditTrailMapper; use OCA\OpenRegister\Db\ObjectEntity; use OCA\OpenRegister\Db\Register; @@ -152,6 +153,7 @@ static function (string $callerUid, string $gid) use ($uid, $isAdmin): bool { schemaMapper: $schemaMapper, userSession: $this->userSession, groupManager: $this->groupManager, + manifestResolver: $this->createMock(ManifestResolverService::class), auditTrailMapper: $this->auditTrailMapper, ); }//end buildController() diff --git a/tests/views/SchemaDesigner.spec.js b/tests/views/SchemaDesigner.spec.js index 6dd7e9ff..8f24c823 100644 --- a/tests/views/SchemaDesigner.spec.js +++ b/tests/views/SchemaDesigner.spec.js @@ -116,7 +116,12 @@ function makeRouter({ slug = 'hello-world', schemaId = '', version = undefined } // equivalent but key-ordered-different JSON string and the // `JSON.stringify` diff in the SUT (correctly) reports a change. const persistedSchema = { - slug: 'hello', + // Schema slugs are namespaced `{appSlug}-{versionSlug}-{slug}` since + // PR #74 — the SchemaDesigner filters the org-wide schema collection + // down to ones owned by the active app+version. The default route is + // `hello-world` with no `_version`, so the fixture must carry the + // `hello-world-` prefix to survive the client-side filter. + slug: 'hello-world-hello', title: 'Hello', description: '', version: '0.1.0', @@ -150,7 +155,7 @@ describe('SchemaDesigner', () => { await wrapper.vm.$nextTick() expect(storeMocks.fetchCollection).toHaveBeenCalledWith('schema') expect(wrapper.vm.schemas).toHaveLength(1) - expect(wrapper.vm.schemas[0].slug).toBe('hello') + expect(wrapper.vm.schemas[0].slug).toBe('hello-world-hello') }) it('REQ-OBSD-001: surfaces a showError toast when the store reports a list error', async () => { @@ -231,7 +236,7 @@ describe('SchemaDesigner', () => { await wrapper.vm.$nextTick() expect(storeMocks.fetchObject).toHaveBeenCalledWith('schema', 'hello') expect(wrapper.vm.staged).toBeTruthy() - expect(wrapper.vm.staged.slug).toBe('hello') + expect(wrapper.vm.staged.slug).toBe('hello-world-hello') expect(wrapper.vm.staged.fields).toHaveLength(1) expect(wrapper.vm.staged.fields[0].name).toBe('subject') }) @@ -296,7 +301,7 @@ describe('SchemaDesigner', () => { expect(type).toBe('schema') expect(body).toMatchObject({ id: 'hello', - slug: 'hello', + slug: 'hello-world-hello', title: 'Hello renamed', version: '0.2.0', type: 'object',