fix(versions): render all version pills + filter schemas by tier#74
Merged
Conversation
Four bugs blocking the dev→staging→prod workflow on the application detail view. #68 — version-list endpoint returned 0 rows. Root cause: OR's `searchObjects` doesn't match the `application` relation field reliably across inline + @self.relations shapes. Drop the relation filter from the OR query and apply it client-side on the normalised row. We expect ~3 rows per app, so the cost is trivial. #69 — `MigrateToVersionedModel` short-circuited every run because `InitializeSettings` imports the versioned schema BEFORE the migration step runs, so the "does schema exist" probe always fired true. Inspect Application row shape instead: a row carrying any of `manifest` / `version` / `status` / `currentVersion` is pre-spec-C data that needs migrating; absence means we're already versioned (or fresh install). #72 — Schema designer listed every schema in the organisation because `useSchemasStore` was pointed at a non-existent endpoint shape. Re-slot the URL segments onto OR's existing `/api/schemas` CRUD and filter client-side to `{appSlug}-{versionSlug}-` prefixed slugs (wizard convention from #71). #73 — Version pills only rendered Production. Two issues: - `refreshApplication` merged `@self` over `data`, so OR's internal metadata clobbered the user's description field. - Version rows carry the UUID in `id` (mirrored from `@self.id`) but the pill/chain code reads `v.uuid`. Normalise once at load so `v.uuid` is always present, then the chain walker resolves the full `development → staging → production` promotesTo chain. Bump info.xml to 0.3.5 to bust NC's asset-cache hash on the rebuilt openbuilt-main.js.
This was referenced May 16, 2026
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 12:29 UTC
Download the full PDF report from the workflow artifacts.
This was referenced May 16, 2026
rubenvdlinde
added a commit
that referenced
this pull request
May 16, 2026
…ht env (#78) * fix(wizard): namespace manifest config.schema alongside config.register (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. * fix(e2e): remove global OCS-APIRequest header + accept pretty-URL redirects 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. * test(newman): rewrite legacy CRUD folder for spec-C versioned model 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. * fix(#77): owner-namespaced register slug on cross-user collision + Playwright 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.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Four bugs blocking the dev → staging → prod workflow on the application detail view.
searchObjectsdoesn't match theapplicationrelation field reliably across inline +@self.relationsshapes. Drop the relation filter and apply it client-side on the normalised row (≈3 rows per app).MigrateToVersionedModelalways short-circuited.InitializeSettingsimports the versioned schema BEFORE the migration runs, so the "schema exists?" probe always fired true. Inspect Application row shape instead./api/schemasCRUD and filter client-side to{appSlug}-{versionSlug}-prefixed slugs.id(mirrored from@self.id) but pill/chain code readv.uuid. Normalise once at load. Also stop merging@selfoverdatainrefreshApplication.Bumps
info.xmlto 0.3.5 to bust NC's asset-cache hash.Test plan
Verified end-to-end on a fresh Permit Flow app (uuid
09f72a5f-…) with 3 versions:?_version={slug}and re-scopes register cardopenbuilt-permit-flow-{tier}for the selected tier/builder/permit-flow/schemas?_version=developmentshows onlypermit-flow-development-hello-message(not all org schemas)Closes #68, closes #69, closes #72, closes #73.