This repository was archived by the owner on May 29, 2026. It is now read-only.
fix(#75): namespace manifest config.schema + test backfill + Playwright env#78
Merged
rubenvdlinde merged 4 commits intoMay 16, 2026
Merged
Conversation
…er (closes #75) `substituteRegisterSlug` only rewrote `config.register` to the per-version slug. The seeded `config.schema` value (`hello-message`) was left bare even though `provisionRegister` namespaces every seed schema as `{appSlug}-{versionSlug}-{originalSlug}`. Net effect: the manifest pointed at a schema slug that didn't exist in the per-version register, so the insights service couldn't resolve the schema-set and every tier's KPI cards read the same global aggregate (openbuilt#75 — all 3 pills showed `Object count: 246` regardless of selected tier). Generalise to `substituteVersionContext(manifest, registerSlug, schemaSlugPrefix)`. When the prefix is non-empty, every non-empty `config.schema` that doesn't already start with the prefix is rewritten to `{prefix}{originalSchemaSlug}`. The original `substituteRegisterSlug` entry point now delegates with an empty prefix (preserves existing unit tests / external callers). Also backfill the cascading test regressions from PR #74 / #76: - tests/store/schemas.spec.js — `registerObjectType` signature changed from 3-arg `(type, 'schemas', register)` to 4-arg with slugs map (`'api', { registerSlug: ... }`) when the schema store was retargeted at OR's `/api/schemas` CRUD. - tests/views/SchemaDesigner.spec.js — fixture slug needs the `{appSlug}-` prefix to survive the new client-side filter. - tests/Unit/Controller/ApplicationsControllerTest.php + tests/Unit/Controller/ApplicationsControllerDiffTest.php — add the `manifestResolver` constructor arg. - tests/Unit/Repair/MigrateToVersionedModelTest.php — rewrite the short-circuit test to match the new row-shape detection (the old "schema-exists" probe was the very bug fixed in #69). Verified: PHPUnit 204/204, Vitest 487/487.
…irects Two unrelated fixes to the Playwright config and login helpers that together unblock every spec from the Playwright baseline (24/26 failed → login itself worked but post-login waits/asserts timed out). 1. Drop `OCS-APIRequest: true` from `use.extraHTTPHeaders`. The header makes Nextcloud treat every browser request as an API call — including the HTML page loads — so the login form's redirect goes out as an empty JSON response with no Location header. Result: the browser stays on `/login` and every `waitForURL(/\/apps\//)` after login times out. Specs that need the header on their API calls (versionRouting, applicationDetailOverview, etc.) already set it explicitly on `request.fetch`; the global was redundant. 2. Relax the post-login `waitForURL` regex from `/index\.php\/apps\//` to `/\/apps\//` in template-gallery.spec.ts + export-zip.spec.ts. Pretty-URL rewrites in modern NC drop the `/index.php/` prefix on redirects. Net effect locally: full Playwright run now drives every spec to its content assertions instead of dying in `beforeEach`. Remaining failures are real test-logic regressions (e.g. `.template-gallery` selector mismatch) that this PR doesn't touch — they're tracked separately.
The 'OpenRegister-backed Application CRUD' folder still POSTed
manifest/version/status directly on the Application object — the
pre-spec-C model where those lived on Application itself. Per ADR-002
the manifest + status + semver moved to ApplicationVersion, so 5 of
the 6 assertions silently dropped to `undefined` and the flow died at
the lifecycle transition.
Rewritten flow:
1. POST plain Application (no manifest / status fields).
2. GET Application by uuid — assert slug round-trip only.
3. POST ApplicationVersion under that Application via the OpenBuilt
version-aware endpoint (carries the manifest now).
4. PUT the Application's `productionVersion` pointer to the new
version UUID.
5. POST publish transition on the VERSION (transitions live there
under the new state machine).
6. GET /api/applications/{slug}/manifest — should resolve via
ManifestResolverService to the version's manifest.
7. DELETE the version + per-version register via the OB endpoint.
Note: step 5 currently returns 500 against a fresh dev container —
likely a separate publish-from-newman-context bug worth filing,
not a test-suite issue. Net Newman now goes 26/-5 (was 23/-6); the
remaining 5 failures cascade from the 500 on step 5.
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-16 15:12 UTC
Download the full PDF report from the workflow artifacts.
…aywright session reuse ## #77 — cross-user register slug collision OR's register slug is organisation-wide unique. \`provisionPerAppRegister\` unconditionally used \`openbuilt-{slug}\`, which silently shared a register across users when two callers cloned the same template. Compatible fix: keep the legacy un-namespaced slug WHEN the existing register is owned by the caller (idempotent re-clones, single-tenant installs). When a different user owns the legacy slug, fall back to \`openbuilt-{ownerUid}-{slug}\` so the cross-user case provisions a fresh register instead of joining the other user's. Adds \`findOrCreateRegister\` + \`extractRegisterOwner\` helpers (tolerant of either an \`owner\` getter on the entity or an \`@self.owner\` block in the JSON shape). ## Playwright session reuse + path fix Two unrelated test-infrastructure issues that were blocking the whole e2e suite: 1. \`global-setup.ts\` logs in once at startup and writes the browser storage state to \`tests/e2e/.auth/admin.json\`. Every spec then reuses the session — no more per-spec \`loginAsAdmin\` helper wrestling with NC's brute-force throttle when workers ran in parallel. 2. \`fullyParallel: false\` + \`workers: 1\` — serial spec execution keeps the brute-force counter quiet. 3. applicationCard.spec.ts + iconUpload.spec.ts: \`page.goto('/apps/openbuilt')\` landed on the Dashboard widget page (route \`/\`), not the Virtual apps index (route \`/applications\`). Updated to \`/apps/openbuilt/applications\` so \`.ob-app-card\` actually renders. \`.auth/\` is gitignored. PHPUnit 204/204 still green.
Contributor
Quality Report — ConductionNL/openbuilt @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-05-16 15:20 UTC
Download the full PDF report from the workflow artifacts.
This was referenced May 16, 2026
Closed
rubenvdlinde
added a commit
that referenced
this pull request
May 17, 2026
…openbuilt#79, partially closes #9 #10) (#81) * fix(e2e+bootstrap): clean URLs, shared session, manifest validator, lifecycle schema test Drives openbuilt#79 (Playwright debt) and openbuilt#10 tasks 4.3 / 5.2 (bootstrap verification) in one branch. ## #79 — Playwright spec fixes - `/index.php/apps/openbuilt/{path}` was being rewritten by NC32 to `/apps/openbuilt/` (dropping the trailing path). Vue Router never saw `/applications` etc., so every page-nav locator timed out. Use clean URLs throughout. API calls (`/index.php/apps/openbuilt/api/...`) are unaffected and stay as-is. - Remove the per-spec `loginAsAdmin` / `beforeEach` form login from schema-designer, template-gallery, and page-designer. globalSetup (added in PR #78) already writes a shared storageState for every spec — the duplicate per-spec login was racing the brute-force throttle and timing out when the page wasn't on /login. - version-rollback navigates to `/apps/openbuilt/applications` (not the dashboard) so the ApplicationCards are present. - rbac-403 navigates to `/applications` to find the empty-state copy + an outsider user is auto-provisioned in dev (occ user:add). Net Playwright on dev container: 24 failed → fewer (full re-run in flight; spec listing surface dropped from 0% to ~30%+ passing). ## #10 — Bootstrap verification (4.3 + 5.2) - `scripts/check-manifest.js` + `npm run check:manifest` validate the openbuilt shell manifest and the wizard seed against the canonical `@conduction/nextcloud-vue/src/schemas/app-manifest.schema.json` schema. The wizard seed's `{registerSlug}` placeholder is substituted with a syntactically-valid slug before validation so the structural shape is what's being asserted. - `tests/scripts/check-manifest.spec.js` (4 Vitest cases) cover the happy path, hand-rolled validity, missing-required-property failure path, and the `{registerSlug}` placeholder substitution. - `tests/Unit/ApplicationVersionLifecycleSchemaTest.php` (5 PHPUnit cases) asserts the declarative state machine in `lib/Settings/openbuilt_register.json` is well-formed: initial state is `draft`, the three states are draft/published/archived, the three transitions are publish/archive/reopen with the right from→to pairs, the publish transition declares the `upsert_relation` that maintains BuiltAppRoute, and disallowed transitions (draft→archived etc.) are absent. Task 4.4 (visual smoke on fresh docker compose up) and the container-bound end-to-end transition test for 5.2 remain — they need a real fresh container which is out of scope for this PR. * test(page-editor): vitest coverage for the 3 stub sub-editors (openbuilt#9 task 7.1) The CustomPageEditor / FormPageEditor / etc. already had Vitest specs; the three remaining v1.1 sub-editors did not. Filling the gap: - tests/components/page-editor/StubPageEditor.spec.js (6 tests) — raw-JSON round-trip, parse-error path, external config re-seed, stability when the watcher re-emits an identical config. - tests/components/page-editor/DashboardPageEditor.spec.js (5 tests) — validatedConfigKeys contract, update(key, value) round-trip, empty-array + null deletion semantics. - tests/components/page-editor/DetailPageEditor.spec.js (11 tests) — full update + sidebar-shape transition matrix, routeParams parser, updateSidebarKey preservation, updateSidebarPropsTabs cleanup. The useRegisterPicker composable is mocked so the test never hits OR. All three editors now have parity with their already-tested siblings. Vitest 487 → 513. PHPUnit 212/212 still green. * test+docs(page-editor): manifest round-trip Vitest + README Visual-designer section (openbuilt#9 tasks 7.2 + 8.1) ## Task 7.2 — manifest round-trip Vitest tests/composables/manifestRoundTrip.spec.js asserts that the canonical manifests (OpenBuilt shell + wizard seed) survive a JSON.parse → JSON.stringify → JSON.parse cycle without losing information AND still validate against the ADR-024 schema after the round-trip. Strict bytewise equality is too brittle (key ordering varies between authors), so the test checks deep-equal + schema-still-valid + idempotency-on-second-round-trip. Three additional cases cover synthetic manifests with nested config blocks (sort, columns) to make sure the editor never flattens or coerces. Note: `lib/Resources/template/src/manifest.json` is a Mustache scaffold (`{{appId}}` etc.) and is intentionally NOT in the validator target list — it's rendered at export time, not consumed as a runtime manifest. ## Task 8.1 — README Visual-designer section New "Visual designer (Design tab + Raw JSON fallback)" section above the chained-spec list explains: - Design tab is the default; type-aware sub-editors per page type; carries inline-mark validator + undo/redo - Raw JSON tab is the integrator fallback; edits round-trip losslessly through parse → stringify → parse (covered by the spec above) - How to run `npm run check:manifest` locally to validate manifest shapes against the canonical schema Vitest 513 → 521. PHPUnit still 212/212 green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #75 (insights cards read the same numbers across all tiers).
Three problems addressed in one branch:
1. Insights fan-out used the wrong schema slug (#75)
The wizard's manifest template hard-coded `config.schema: hello-message` but `provisionRegister` namespaces every seed schema as `{appSlug}-{versionSlug}-{originalSlug}`. `substituteRegisterSlug` only rewrote the register, so `config.schema` pointed at a non-existent slug. `ApplicationInsightsService::deriveSchemaIds` then returned an empty schema-set, falling back to whatever OR exposed at the global level — which is why every pill showed `Object count: 246`.
Generalised the helper to `substituteVersionContext(manifest, registerSlug, schemaSlugPrefix)` and rewrote each `config.schema` to `{prefix}{slug}` (idempotent on already-namespaced slugs). Three new PHPUnit cases cover the happy path, idempotency, and empty-prefix backwards compatibility.
2. Cascading test regressions from PR #74 / #76
PHPUnit was red against `development` (16 errors): `ApplicationsControllerTest` + `ApplicationsControllerDiffTest` instantiated the controller without the new `manifestResolver` arg, and `MigrateToVersionedModelTest` still asserted the old schema-exists short-circuit. Vitest was red (4 failures) because `useSchemasStore` moved from a 3-arg `registerObjectType` call to a 4-arg `(type, 'schemas', 'api', { registerSlug })` shape, and `SchemaDesigner` now filters by `{appSlug}-` slug prefix.
Updated all six test files. Verified: PHPUnit 204/204, Vitest 487/487 on HEAD.
3. Playwright env issues
4. Newman legacy CRUD folder rewrite
`OpenRegister-backed Application CRUD` in the main collection was still POSTing manifest/version/status directly on the Application object — pre-spec-C model. Rewrote the flow against ADR-002: POST Application → POST Version (carries manifest) → PUT productionVersion pointer → POST publish transition on the version → GET /api/applications/{slug}/manifest. Newman now 26/-5 (was 23/-6).
Test plan
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com