From 45e988d75cfc1dc7f78294403e6f910caad2ab7a Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 11 Aug 2026 10:35:02 +0200 Subject: [PATCH 1/2] test(e2e): cover 10 gate-19 scenarios with real tests, and fix the icon-remove 404 they found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gate-19 e2e-coverage on openbuild: 186 -> 176 uncovered scenarios, all closed with tests that actually drive the surface. No scenario was closed by adding an annotation to something that does not exercise it, and no exclude was added. app-icon-management (3 scenarios) --------------------------------- Two of the three were ALREADY covered by real, running tests in tests/e2e/iconUpload.spec.ts that simply carried no @e2e tag — they are annotated there rather than duplicated. The third, user-removes-the-dark-icon, was covered by nothing, and writing it found a product defect. IconUploadSection.removeIcon() called DELETE .../objects/{register}/{schema}/{uuid}/files/app-icon-dark.svg but OpenRegister's files#delete route constrains fileId to \d+, so a filename never matched the route and Nextcloud answered its HTML 404 page. The catch painted the generic "Remove failed" string, so the button looked implemented and could never work. Measured both ways on a live instance: DELETE by filename -> 404, DELETE by the numeric id from GET .../files -> 200 and the attachment is gone. removeIcon() now resolves the numeric id from the object's file index first, and the test pins the numeric shape so the regression cannot return looking green. The three tests this file used to hold were unconditional test.skip stubs citing "openbuild#41: admin UI not functional". That reason was stale — the detail page and the whole icon section render — but un-skipping alone would have produced three green tests asserting expect(page.locator('main')).toBeVisible(). version-lifecycle-ui (7 scenarios, new file) -------------------------------------------- All 17 of this spec's scenarios were uncovered; VersionHistory.vue and ManifestLayersDetail.vue implement the surface in full. Covers the slug-based list endpoint, one row per version, the production marker, click-to-open scoped and canonical, per-row Edit carrying ?_version=, and New draft cloning the production manifest while SHARING its register. REQ-OBV-VLU-007 (the Open-app split button) and REQ-OBV-VLU-008 (the NL catalogue) are deliberately left uncovered and counted rather than claimed. Verification ------------ Driven against a disposable Nextcloud, never the shared dev container (this suite writes). All 10 tests green. Proven in both directions: the dark-icon test was RED on the real 404 before the product fix and GREEN after, and a planted true positive (expecting _version=PLANTED-TRUE-POSITIVE) turned the Edit test RED and reverting turned it GREEN again. Every write arms waitForResponse() BEFORE the interaction that fires it; no networkidle, no sleeps, no conditional skips that would let a test pass by not running. --- src/dialogs/IconUploadSection.vue | 46 ++- tests/e2e/iconUpload.spec.ts | 12 + .../spec-coverage/app-icon-management.spec.ts | 324 ++++++++++++------ .../version-lifecycle-ui.spec.ts | 310 +++++++++++++++++ 4 files changed, 579 insertions(+), 113 deletions(-) create mode 100644 tests/e2e/spec-coverage/version-lifecycle-ui.spec.ts diff --git a/src/dialogs/IconUploadSection.vue b/src/dialogs/IconUploadSection.vue index 1c027629c..14feca377 100644 --- a/src/dialogs/IconUploadSection.vue +++ b/src/dialogs/IconUploadSection.vue @@ -359,12 +359,33 @@ export default { * emits `updated` with a `null` ref. Never throws: a failed request surfaces in * `uploadError`. * + * ⚠️ THE DELETE IS ADDRESSED BY NUMERIC FILE ID, NOT BY FILENAME. + * + * This method used to call + * `DELETE .../objects/{register}/{schema}/{uuid}/files/app-icon-dark.svg`. + * OpenRegister's route for that verb is + * + * ['name' => 'files#delete', + * 'url' => '/api/objects/{register}/{schema}/{id}/files/{fileId}', + * 'verb' => 'DELETE', + * 'requirements' => ['id' => '[^/]+', 'fileId' => '\d+']] + * + * — `fileId` is constrained to `\d+`, so a filename never matched the + * route at all and Nextcloud answered its HTML **404** page. The + * `catch` below then painted the generic "Remove failed" string, so the + * button looked implemented and could never work. Measured both ways on + * a live instance: DELETE by filename → 404; DELETE by the numeric id + * from `GET .../files` → 200 and the attachment is gone. + * + * The id is therefore resolved from the object's own file index, where + * each entry carries `{ id: , title: '' }`. + * * @param {'light'|'dark'} variant - Which icon slot to clear; selects both the * attached filename to delete (`app-icon.svg` / `app-icon-dark.svg`) and the * Application field to null out (`icon` / `iconDark`). * @return {Promise} * - * @spec openspec/changes/retrofit-2026-05-26-creation-wizard-ui/tasks.md#task-4 + * @spec openspec/specs/app-icon-management/spec.md#user-removes-the-dark-icon */ async removeIcon(variant) { if (!this.objectUuid) return @@ -374,11 +395,26 @@ export default { const field = variant === 'dark' ? 'iconDark' : 'icon' try { - // 1. Delete the file from OR. - const deleteUrl = generateUrl( - `/apps/openregister/api/objects/${REGISTER}/${SCHEMA}/${this.objectUuid}/files/${filename}`, + // 1. Resolve the attachment's NUMERIC id, then delete by it. + const filesUrl = generateUrl( + `/apps/openregister/api/objects/${REGISTER}/${SCHEMA}/${this.objectUuid}/files`, ) - await axios.delete(deleteUrl) + const listed = await axios.get(filesUrl) + const attachment = (listed?.data?.results || []) + .find((f) => f?.title === filename) + + if (!attachment?.id) { + // The ref points at a file OR no longer holds. Nothing to + // detach — fall through and clear the ref so the record stops + // advertising an attachment that is not there. + // eslint-disable-next-line no-console + console.warn(`[openbuild] no OR attachment named ${filename}; clearing the ref only`) + } else { + const deleteUrl = generateUrl( + `/apps/openregister/api/objects/${REGISTER}/${SCHEMA}/${this.objectUuid}/files/${attachment.id}`, + ) + await axios.delete(deleteUrl) + } // 2. Clear the ref on the Application (partial merge, not replace). const patchUrl = generateUrl( diff --git a/tests/e2e/iconUpload.spec.ts b/tests/e2e/iconUpload.spec.ts index 47e42ee6d..881b01be5 100644 --- a/tests/e2e/iconUpload.spec.ts +++ b/tests/e2e/iconUpload.spec.ts @@ -128,6 +128,12 @@ test.describe('Icon upload on the Application detail page (spec A task 7.5)', () await expect(inputs.nth(1)).toHaveAttribute('accept', '.svg') }) + // @e2e app-icon-management::non-svg-file-is-rejected-client-side + // + // The scenario is "the uploader displays an inline error message and does not + // submit the file to OR". Both halves are asserted below: the inline error by + // its literal text, and the negative half by recording every POST the page + // issues and requiring the list to be empty. test('a non-SVG pick is rejected inline and never reaches the server', async ({ page, request }) => { const { objectId } = await resolveApp(request) await openIconsTab(page, objectId) @@ -156,6 +162,12 @@ test.describe('Icon upload on the Application detail page (spec A task 7.5)', () expect(uploads, 'a rejected file must not be uploaded').toEqual([]) }) + // @e2e app-icon-management::user-uploads-a-light-icon + // + // The scenario's three clauses map onto the assertions below: the file is + // POSTed to OR's attachment endpoint and `icon.ref` is patched (proven by + // reading the Application back independently rather than trusting optimistic + // UI state), and the light-background preview renders the SVG. test('uploading an SVG persists it on the Application and shows it in the preview', async ({ page, request }) => { // Two navigations plus an upload round-trip. The 30s project default is // sized for single-navigation tests; every assertion below keeps its own diff --git a/tests/e2e/spec-coverage/app-icon-management.spec.ts b/tests/e2e/spec-coverage/app-icon-management.spec.ts index e677a78fe..348c08670 100644 --- a/tests/e2e/spec-coverage/app-icon-management.spec.ts +++ b/tests/e2e/spec-coverage/app-icon-management.spec.ts @@ -2,127 +2,235 @@ // SPDX-FileCopyrightText: 2026 Conduction B.V. /** - * E2E coverage for app-icon-management spec — REQ-OBICON-004 UI scenarios. + * E2E coverage for the `app-icon-management` spec — the remove half of + * REQ-OBICON-004. * - * Icon section on the Application detail page: - * - user-uploads-a-light-icon - * - user-removes-the-dark-icon - * - non-svg-file-is-rejected-client-side + * UN-QUARANTINED 2026-08-11. This file used to hold three unconditional + * `test.skip`s citing "Conduction/openbuild#41: openbuild admin UI not + * functional in this build — no application detail / icon / template-clone UI + * renders". That reason was stale on both counts, and each half was checked + * against a live instance before this rewrite: * - * Backend REQ-OBICON-001/002/003/005 are excluded (verified by Newman/PHPUnit). + * 1. The detail page renders. `src/manifest.json` declares page + * `VirtualAppDetail` at `/applications/:objectId` with a sidebar tab + * `{ id: "icons", label: "Icons", component: "ApplicationIconTab" }`. + * 2. The icon UI exists in full — `src/dialogs/IconUploadSection.vue` ships + * both slots, both `accept=".svg"` inputs, the previews, the Remove + * buttons and the client-side extension check. + * + * The old bodies asserted `expect(page.locator('main')).toBeVisible()` and + * wrapped their only real assertion in `if (await x.count() > 0)`, so simply + * removing `.skip` would have produced three green tests that drove nothing. + * + * Two of the three scenarios were ALREADY covered by real, running tests that + * merely lacked the annotation — `tests/e2e/iconUpload.spec.ts` proves + * `user-uploads-a-light-icon` and `non-svg-file-is-rejected-client-side` + * against this same surface. They are annotated there rather than duplicated + * here. What no test anywhere exercised is the REMOVE path, so that is what + * this file now contains. + * + * Backend REQ-OBICON-001/002/003/005 are excluded in the spec itself (PHPUnit + + * Newman) and are not repeated here. */ -import { test, expect, type Page } from '@playwright/test' +import { test, expect, type Page, type APIRequestContext } from '@playwright/test' +import { E2E_BASE_URL as BASE } from '../support/baseUrl' +import { suppressSupportDialog, suppressSetupWizard } from '../support/appFixture' + +/** The seeded fixture app both icon suites drive. */ +const HELLO_WORLD_SLUG = 'hello-world' + +/** OR register/schema the Application record lives in (IconUploadSection.vue). */ +const OR_OBJECT_PATH = 'apps/openregister/api/objects/openbuild/application' -const BASE = process.env.PLAYWRIGHT_BASE_URL ?? 'http://localhost:8080' +/** A minimal but genuinely valid SVG — OR writes the content verbatim. */ +const MINIMAL_SVG = '' + + '' -/** Navigate to the applications list and return the URL of the first Hello World card. */ -async function getFirstAppDetailUrl(page: Page): Promise { - await page.goto(`${BASE}/apps/openbuild/applications`) - const card = page.getByRole('link', { name: /Hello World/i }).first() - await expect(card).toBeVisible({ timeout: 15_000 }) - const href = await card.getAttribute('href') - return href ? (href.startsWith('http') ? href : `${BASE}${href}`) : `${BASE}/apps/openbuild/applications` +/** + * Resolve the seeded Application and its OR object id. + * + * Every step is an assertion, not a skip: the fixture is seeded by globalSetup, + * so "not found" means the seeding broke and skipping would hide it. + * + * @param request Playwright API request context. + * @return {Promise<{objectId: string, app: Record}>} The record. + */ +async function resolveApp(request: APIRequestContext): Promise<{ objectId: string, app: Record }> { + const res = await request.get(`${BASE}/index.php/apps/openbuild/api/applications`, { + headers: { 'OCS-APIRequest': 'true' }, + }) + expect(res.ok(), 'the applications API must answer').toBeTruthy() + const body = await res.json() + const rows: Array> = Array.isArray(body) ? body : (body.results ?? []) + const app = rows.find((a) => (a.slug ?? a['@self']?.slug) === HELLO_WORLD_SLUG) + expect(app, `the seeded "${HELLO_WORLD_SLUG}" Application must exist`).toBeTruthy() + const found = app as Record + const objectId = found['@self']?.id || found.uuid || found.id + expect(objectId, 'the Application must carry an object id').toBeTruthy() + return { objectId: String(objectId), app: found } } -// @e2e app-icon-management::user-uploads-a-light-icon -// QUARANTINED (Conduction/openbuild#41): openbuild admin UI not functional in this build — no application detail / icon / template-clone UI renders. Re-enable when #41 is fixed. -test.skip('REQ-OBICON-004 — detail page exposes icon upload section', async ({ page }) => { - // @e2e app-icon-management::user-uploads-a-light-icon - await page.goto(`${BASE}/apps/openbuild/applications`) - const card = page.getByRole('link', { name: /Hello World/i }).first() - await expect(card).toBeVisible({ timeout: 15_000 }) - await card.click() - - // Wait for detail page to load - await page.waitForURL(/\/applications\//, { timeout: 15_000 }) - - // Look for the sidebar or a tab that would contain icon management - // The spec says REQ-OBICON-004 adds an Icon section to the Application detail page - // We verify the page has loaded and contains the expected tab navigation - await expect(page.locator('main')).toBeVisible({ timeout: 10_000 }) - - // Check the page does not show a white screen / hard error - const errorOverlay = page.locator('[class*="error"], [data-error]') - const errorCount = await errorOverlay.count() - // If any error element, make sure it isn't a fatal crash - if (errorCount > 0) { - const errorText = await errorOverlay.first().textContent() - expect(errorText, 'page must not show a fatal error').not.toMatch(/fatal|crash|500|undefined is not/i) +/** + * Open the Application detail page, expand the sidebar, activate the Icons tab. + * + * `CnDetailPage` seeds `sidebarOpen: false`, so the manifest-declared tabs are + * simply not in the DOM until `NcAppSidebar`'s own `.app-sidebar__toggle` is + * clicked. Omitting that step is why the first draft of this rewrite timed out + * looking for a tab that could not exist yet. + * + * @param page Playwright page. + * @param objectId The Application's OR object id. + * @return {Promise} + */ +async function openIconsTab(page: Page, objectId: string): Promise { + await page.goto(`/apps/openbuild/applications/${objectId}`, { waitUntil: 'domcontentloaded' }) + await expect( + page.locator('.ob-detail-header__name'), + 'the detail header must render before the sidebar is driven', + ).toBeVisible({ timeout: 20_000 }) + + const sidebar = page.locator('[data-testid="cn-object-sidebar"]') + if (!(await sidebar.isVisible().catch(() => false))) { + await page.locator('.app-sidebar__toggle').first().click() } + await expect(sidebar, 'the object sidebar must open').toBeVisible({ timeout: 15_000 }) - // The detail page should render at minimum the application name heading + await page.getByRole('tab', { name: /^icons$/i }).first().click() await expect( - page.getByText('Hello World').first(), - 'application name must be visible on the detail page', - ).toBeVisible({ timeout: 10_000 }) -}) + page.locator('[data-testid="cn-object-sidebar-tab-icons"]'), + 'the Icons tab panel must render', + ).toBeVisible({ timeout: 15_000 }) + await expect( + page.locator('.ob-icon-section'), + 'ApplicationIconTab must mount IconUploadSection', + ).toBeVisible({ timeout: 15_000 }) +} -// @e2e app-icon-management::user-removes-the-dark-icon -// QUARANTINED (Conduction/openbuild#41): openbuild admin UI not functional in this build — no application detail / icon / template-clone UI renders. Re-enable when #41 is fixed. -test.skip('REQ-OBICON-004 — icon tab/section is accessible on the detail page', async ({ page }) => { - // @e2e app-icon-management::user-removes-the-dark-icon - await page.goto(`${BASE}/apps/openbuild/applications`) - const card = page.getByRole('link', { name: /Hello World/i }).first() - await expect(card).toBeVisible({ timeout: 15_000 }) - await card.click() - await page.waitForURL(/\/applications\//, { timeout: 15_000 }) - - // The detail page should have sidebar tabs; look for an icon-related tab - // The spec says the icon section is on the Application detail page - // It may be a tab label or a section heading - const possibleIconTab = page.locator( - '[role="tab"], button, a', - ).filter({ hasText: /icon|image|brand/i }) - - // Either the icon tab exists (full implementation) or the detail page loads without white-screen - const iconTabCount = await possibleIconTab.count() - if (iconTabCount > 0) { - // Tab exists — click it and verify no crash - await possibleIconTab.first().click() - await expect(page.locator('main')).toBeVisible({ timeout: 5_000 }) - } else { - // Tab not yet wired to a specific label — detail page must at minimum load - await expect( - page.getByText('Hello World').first(), - ).toBeVisible({ timeout: 10_000 }) - } -}) +/** + * The light-icon or dark-icon row of the section. + * + * The two rows share every class name; only their label text separates them, so + * that is what addresses them. Index-based addressing would silently follow a + * reorder and start asserting about the wrong slot. + * + * @param page Playwright page. + * @param variant Which slot. + * @return The row locator. + */ +function iconRow(page: Page, variant: 'Light' | 'Dark') { + return page.locator('.ob-icon-section__row') + .filter({ has: page.locator('.ob-icon-section__label', { hasText: `${variant} icon` }) }) +} + +test.describe('app-icon-management — removing an icon (REQ-OBICON-004)', () => { + test.beforeEach(async ({ page }) => { + await suppressSupportDialog(page) + await suppressSetupWizard(page) + }) -// @e2e app-icon-management::non-svg-file-is-rejected-client-side -// QUARANTINED (Conduction/openbuild#41): openbuild admin UI not functional in this build — no application detail / icon / template-clone UI renders. Re-enable when #41 is fixed. -test.skip('REQ-OBICON-004 — non-SVG upload is rejected (icon section validation)', async ({ page }) => { - // @e2e app-icon-management::non-svg-file-is-rejected-client-side - await page.goto(`${BASE}/apps/openbuild/applications`) - const card = page.getByRole('link', { name: /Hello World/i }).first() - await expect(card).toBeVisible({ timeout: 15_000 }) - await card.click() - await page.waitForURL(/\/applications\//, { timeout: 15_000 }) - - // Look for a file input in the icon section - const fileInputs = page.locator('input[type="file"]') - const fileInputCount = await fileInputs.count() - - if (fileInputCount > 0) { - // Try to upload a non-SVG file; the client-side validator should reject it - const fileInput = fileInputs.first() - await fileInput.setInputFiles({ - name: 'test-image.png', - mimeType: 'image/png', - buffer: Buffer.from('PNG_FAKE_CONTENT'), + // @e2e app-icon-management::user-removes-the-dark-icon + test('Remove in the dark slot deletes the OR attachment and clears iconDark.ref', async ({ page, request }) => { + // Two navigations plus two write round-trips; the 30s project default is + // sized for single-navigation tests. + test.setTimeout(120_000) + + const { objectId } = await resolveApp(request) + + // PRECONDITION, established over the API: a dark icon IS attached. The + // Remove button renders behind `v-if="darkRef"`, so without this the test + // would "pass" by never finding a button to click — the exact shape of a + // test that measures nothing. + const seedUpload = await request.post(`${BASE}/index.php/${OR_OBJECT_PATH}/${objectId}/files`, { + data: { name: 'app-icon-dark.svg', content: MINIMAL_SVG }, }) - // An inline error message should appear - const errorMsg = page.locator( - '[class*="error"], [role="alert"], .nc-error-message', - ).filter({ hasText: /svg|format|invalid|type/i }) - const errorCount = await errorMsg.count() - // If icon upload UI renders, non-SVG should surface an inline error - // (if icon tab not yet visible, the test passes vacuously because the UI is not built) - if (errorCount > 0) { - await expect(errorMsg.first()).toBeVisible({ timeout: 5_000 }) - } - } + expect(seedUpload.ok(), 'seeding the dark attachment must succeed').toBeTruthy() + // A LIGHT icon is seeded too, deliberately: the scenario's last clause is + // that the dark slot falls back to the light icon, and that fallback is + // only meaningful if a light icon exists. Without this the test would be + // order-dependent on whichever suite last uploaded one. + const seedLight = await request.post(`${BASE}/index.php/${OR_OBJECT_PATH}/${objectId}/files`, { + data: { name: 'app-icon.svg', content: MINIMAL_SVG }, + }) + expect(seedLight.ok(), 'seeding the light attachment must succeed').toBeTruthy() + const seedPatch = await request.patch(`${BASE}/index.php/${OR_OBJECT_PATH}/${objectId}`, { + data: { icon: { ref: 'app-icon.svg' }, iconDark: { ref: 'app-icon-dark.svg' } }, + }) + expect(seedPatch.ok(), 'seeding icon + iconDark refs must succeed').toBeTruthy() + + await openIconsTab(page, objectId) - // Whether or not the file input renders, the page must not crash - await expect(page.locator('main')).toBeVisible({ timeout: 5_000 }) + const dark = iconRow(page, 'Dark') + const removeBtn = dark.locator('.ob-icon-section__remove-btn') + await expect( + removeBtn, + 'the Remove button must render while a dark icon is attached — its absence would make this test vacuous', + ).toBeVisible({ timeout: 20_000 }) + + // Arm both writes BEFORE the click that fires them. Waiting afterwards + // races the XHR, and `waitForLoadState('networkidle')` is banned by + // gate-58 precisely because it does not wait for an XHR at all. + // The DELETE is addressed by the attachment's NUMERIC id, not its + // filename: OpenRegister's `files#delete` route constrains `fileId` to + // `\d+`, so a filename does not match the route and Nextcloud answers its + // HTML 404 page. Asserting the numeric shape here is what stops that + // regression from coming back looking like a passing test. + const deleteDone = page.waitForResponse( + (r) => new RegExp(`${OR_OBJECT_PATH}/${objectId}/files/\\d+$`).test(new URL(r.url()).pathname) + && r.request().method() === 'DELETE', + { timeout: 30_000 }, + ) + const patchDone = page.waitForResponse( + (r) => r.url().includes(`${OR_OBJECT_PATH}/${objectId}`) + && r.request().method() === 'PATCH', + { timeout: 30_000 }, + ) + + await removeBtn.click() + + // "the frontend calls OR's delete-attachment endpoint for the iconDark file" + const deleteRes = await deleteDone + expect(deleteRes.status(), 'the delete-attachment call must answer 2xx').toBeGreaterThanOrEqual(200) + expect(deleteRes.status(), 'the delete-attachment call must answer 2xx').toBeLessThan(300) + + // "and clears the top-level iconDark.ref from the Application" + const patchRes = await patchDone + expect( + patchRes.request().postDataJSON(), + 'the PATCH must null the top-level iconDark ref, not replace the object', + ).toEqual({ iconDark: null }) + expect(patchRes.status(), 'the ref-clearing PATCH must answer 2xx').toBeLessThan(300) + + // The button is bound to `v-if="darkRef"`, so its disappearance is the + // component's own statement that the ref is gone. + await expect( + removeBtn, + 'the Remove button must disappear once the ref is cleared', + ).toHaveCount(0, { timeout: 15_000 }) + + // "the preview area falls back to showing the light icon in the + // dark-background slot" — i.e. the slot must NOT go blank. + // + // The first draft of this assertion demanded the dark preview render no + // at all, and it failed — correctly. The slot keeps rendering an + // image and lets `/apps/openbuild/icons/{slug}-dark.svg` serve the + // fallback; that server-side fallback chain is REQ-OBICON-002, which the + // spec excludes to PHPUnit. So what belongs here is the DOM-observable + // half: the slot still shows something rather than collapsing. + await expect( + dark.locator('.ob-icon-section__preview--dark img.ob-icon-section__preview-img'), + 'the dark slot must keep showing a preview (the light-icon fallback), not go blank', + ).toHaveCount(1, { timeout: 15_000 }) + await expect( + dark.locator('.ob-icon-section__preview-empty'), + 'the dark slot must not fall back to the em-dash placeholder', + ).toHaveCount(0) + + // And the record really lost it — read it back rather than trusting + // optimistic UI state. + await expect.poll(async () => { + const { app: reloaded } = await resolveApp(request) + return reloaded.iconDark ?? null + }, { timeout: 30_000 }).toBeFalsy() + }) }) diff --git a/tests/e2e/spec-coverage/version-lifecycle-ui.spec.ts b/tests/e2e/spec-coverage/version-lifecycle-ui.spec.ts new file mode 100644 index 000000000..b594098bb --- /dev/null +++ b/tests/e2e/spec-coverage/version-lifecycle-ui.spec.ts @@ -0,0 +1,310 @@ +// SPDX-License-Identifier: EUPL-1.2 +// SPDX-FileCopyrightText: 2026 Conduction B.V. + +/** + * E2E coverage for the `version-lifecycle-ui` spec — the maintainer cockpit for + * OpenBuild's two-object version model. + * + * NEW FILE 2026-08-11. Every one of this spec's 17 scenarios was uncovered by + * gate-19: no Playwright test anywhere in the suite referenced a single one of + * them, even though `src/views/VersionHistory.vue` and + * `src/views/ManifestLayersDetail.vue` implement the whole surface. + * + * The surface, read from source rather than guessed: + * + * route /apps/openbuild/applications/{objectId}/manifest + * (manifest.json page `ApplicationManifestDetail`) + * list GET /apps/openbuild/api/applications/{slug}/versions + * (VersionHistory.vue — the SLUG-based endpoint, which is the + * whole point of REQ-OBV-VLU-001) + * row .version-history__row, production row also --current + * marker .version-history__badge--production + * open openVersion() -> /apps/openbuild/builder/{slug} + * (+ '?_version=' + slug when not production) + * edit editVersion() -> /apps/openbuild/builder/{slug}/pages + * (+ '?_version=' + slug when not production) + * new draft ManifestLayersDetail.createDraft() -> + * POST /apps/openbuild/api/applications/{slug}/versions + * + * WHAT IS NOT HERE, AND WHY. REQ-OBV-VLU-007 (the "Open app" split button in + * ApplicationDetailActions.vue) and REQ-OBV-VLU-008 (the NL catalogue) are a + * different component and a build-time artefact respectively; they stay + * uncovered and counted rather than being claimed by a test that does not drive + * them. Nothing in this file is annotated for a scenario it does not exercise. + */ + +import { test, expect, type Page } from '@playwright/test' +import { E2E_BASE_URL as BASE } from '../support/baseUrl' +import { suppressSupportDialog, suppressSetupWizard } from '../support/appFixture' +import { ensureVersionChain, listVersions } from '../support/versionChain' + +/** + * A dedicated fixture app. + * + * `ensureVersionChain` provisions `development` -> `staging` -> `production`, + * so the list has a production row AND non-production rows — without both, + * half of these scenarios cannot be distinguished from each other. + * + * Deliberately not `hello-world`: the New-draft scenario WRITES a version, and + * hello-world's single-version shape is asserted by other suites. + */ +const SLUG = 'pw-vlu' +const NAME = 'PW Version Lifecycle' + +/** + * Open the Manifest detail page, which is where `VersionHistory` is routed. + * + * @param page Playwright page. + * @param objectId The Application's OR object id. + * @return {Promise} + */ +async function openManifestDetail(page: Page, objectId: string): Promise { + await page.goto(`${BASE}/apps/openbuild/applications/${objectId}/manifest`, { + waitUntil: 'domcontentloaded', + }) + await expect( + page.locator('.version-history'), + 'the Manifest detail page must render the VersionHistory panel', + ).toBeVisible({ timeout: 20_000 }) +} + +/** + * Resolve the fixture Application's OR object id. + * + * Asserted, never skipped: the fixture is provisioned in `beforeEach`, so a + * miss means the provisioning broke. + * + * @param page Playwright page. + * @return {Promise} The object id. + */ +async function appObjectId(page: Page): Promise { + const res = await page.request.get( + `${BASE}/index.php/apps/openregister/api/objects/openbuild/application` + + `?slug=${encodeURIComponent(SLUG)}&_limit=1`, + ) + expect(res.ok(), 'the Application lookup must succeed').toBeTruthy() + const rows = (await res.json()).results || [] + expect(rows.length, `the "${SLUG}" fixture Application must exist`).toBeGreaterThan(0) + const id = rows[0]['@self']?.id || rows[0].uuid || rows[0].id + expect(id, 'the Application must carry an object id').toBeTruthy() + return String(id) +} + +/** The row locator for a version addressed by its visible name. */ +function rowFor(page: Page, versionName: string) { + return page.locator('.version-history__row') + .filter({ has: page.locator('.version-history__row-title', { hasText: versionName }) }) +} + +test.describe('version-lifecycle-ui — the version list on the Manifest detail page', () => { + // The row actions sit in a right-hand column that collapses at the default + // 1280x720, which puts Open/Edit/Release behind an overflow. + test.use({ viewport: { width: 1600, height: 1200 } }) + + test.beforeEach(async ({ page }) => { + await suppressSupportDialog(page) + await suppressSetupWizard(page) + await ensureVersionChain(page, SLUG, NAME) + }) + + // @e2e version-lifecycle-ui::slug-is-passed-to-the-version-list + // @e2e version-lifecycle-ui::versions-render-for-an-app-with-at-least-one-version + test('REQ-OBV-VLU-001 — the list is fetched by SLUG and renders one row per version', async ({ page }) => { + const objectId = await appObjectId(page) + + // The scenario is specifically that the SLUG-based endpoint is called — + // the bug it guards is the view falling back to applicationUuid. Arm the + // wait BEFORE navigating so the request cannot be missed. + const versionsCall = page.waitForResponse( + (r) => r.url().includes(`/apps/openbuild/api/applications/${SLUG}/versions`) + && r.request().method() === 'GET', + { timeout: 30_000 }, + ) + + await page.goto(`${BASE}/apps/openbuild/applications/${objectId}/manifest`, { + waitUntil: 'domcontentloaded', + }) + + const res = await versionsCall + expect(res.status(), 'the slug-based versions endpoint must answer 2xx').toBeLessThan(300) + + // "the version list renders one row per version (not the empty state)" + const expected = await listVersions(page, SLUG) + expect(expected.length, 'the fixture must carry at least one version').toBeGreaterThan(0) + + await expect( + page.locator('.version-history__empty'), + 'the empty state must NOT be shown for an app that has versions', + ).toHaveCount(0) + await expect( + page.locator('.version-history__row'), + 'one row per ApplicationVersion', + ).toHaveCount(expected.length, { timeout: 20_000 }) + }) + + // @e2e version-lifecycle-ui::production-version-is-marked + test('REQ-OBV-VLU-004 — exactly one row carries the production marker', async ({ page }) => { + const objectId = await appObjectId(page) + await openManifestDetail(page, objectId) + + const marker = page.locator('.version-history__badge--production') + await expect( + marker, + 'the production version must carry a marker distinct from the other rows', + ).toHaveCount(1, { timeout: 20_000 }) + await expect(marker).toHaveText('Production') + + // The marker must be ON the production row, not merely present somewhere: + // the row that carries it is also the one flagged `--current`. + await expect( + page.locator('.version-history__row--current .version-history__badge--production'), + 'the marker must sit on the row the view considers production', + ).toHaveCount(1) + + // And the other rows must NOT carry it — a marker on every row would + // satisfy a naive "is it visible" check while marking nothing. + const rows = await page.locator('.version-history__row').count() + expect(rows, 'the fixture chain must give more than one row').toBeGreaterThan(1) + }) + + // @e2e version-lifecycle-ui::click-a-non-production-version-opens-it-scoped + test('REQ-OBV-VLU-002 — activating a non-production row opens the builder scoped to it', async ({ page }) => { + // THREE page loads: the fixture provisioning in beforeEach, the manifest + // detail page, and then the builder — which is a SEPARATE webpack entry + // (`src/builder.js`), so it is a cold bundle fetch, not an SPA route + // change. The project default of 30s is sized for single-navigation + // tests and the whole TEST ran out of budget at 39.9s, which surfaced as + // a `waitForURL` timeout and read like a navigation that never happened. + // This is the same allowance iconUpload.spec.ts already makes; the + // assertion itself is unchanged and still exact. + test.setTimeout(120_000) + + const objectId = await appObjectId(page) + await openManifestDetail(page, objectId) + + // `staging` is a draft in the fixture chain, so it is never production. + const staging = rowFor(page, 'staging') + await expect(staging, 'the fixture must render a "staging" row').toHaveCount(1, { timeout: 20_000 }) + await expect( + staging.locator('.version-history__badge--production'), + '"staging" must not be the production row, or this scenario tests nothing', + ).toHaveCount(0) + + await staging.locator('.version-history__btn', { hasText: 'Open' }).first().click() + + await page.waitForURL( + (url) => url.pathname.endsWith(`/apps/openbuild/builder/${SLUG}`) + && url.searchParams.get('_version') === 'staging', + { timeout: 30_000 }, + ) + }) + + // @e2e version-lifecycle-ui::click-the-production-version-opens-the-canonical-url + test('REQ-OBV-VLU-002 — activating the production row opens the canonical URL with no _version', async ({ page }) => { + // Three page loads, one of them the standalone builder bundle — see the + // note on the non-production case above. + test.setTimeout(120_000) + + const objectId = await appObjectId(page) + await openManifestDetail(page, objectId) + + const production = page.locator('.version-history__row--current') + await expect(production, 'the production row must be identifiable').toHaveCount(1, { timeout: 20_000 }) + + await production.locator('.version-history__btn', { hasText: 'Open' }).first().click() + + await page.waitForURL( + (url) => url.pathname.endsWith(`/apps/openbuild/builder/${SLUG}`) + && !url.searchParams.has('_version'), + { timeout: 30_000 }, + ) + }) + + // @e2e version-lifecycle-ui::edit-a-version-opens-the-designer-with-the-version-param + test('REQ-OBV-VLU-003 — per-row Edit opens the designer carrying ?_version=', async ({ page }) => { + // Three page loads, the last being the page designer — see the note on + // the non-production case above. + test.setTimeout(120_000) + + const objectId = await appObjectId(page) + await openManifestDetail(page, objectId) + + const staging = rowFor(page, 'staging') + await expect(staging).toHaveCount(1, { timeout: 20_000 }) + + const edit = staging.locator('.version-history__btn', { hasText: 'Edit' }) + await expect( + edit, + 'Edit must be offered to an editor+ caller (the run is admin, so canEdit is true)', + ).toHaveCount(1) + + await edit.first().click() + + await page.waitForURL( + (url) => url.pathname.endsWith(`/apps/openbuild/builder/${SLUG}/pages`) + && url.searchParams.get('_version') === 'staging', + { timeout: 30_000 }, + ) + }) + + // @e2e version-lifecycle-ui::new-draft-clones-production-manifest-and-shares-its-register + test('REQ-OBV-VLU-005 — New draft posts a draft that clones production manifest and shares its register', async ({ page }) => { + // A navigation, a list round-trip and a create round-trip. + test.setTimeout(120_000) + + const objectId = await appObjectId(page) + await openManifestDetail(page, objectId) + + const before = await listVersions(page, SLUG) + const productionRow = before.find((v) => v?.slug === 'production') + expect(productionRow, 'the fixture must carry a production version').toBeTruthy() + + const newDraft = page.getByRole('button', { name: 'New draft' }) + await expect( + newDraft, + 'New draft must be offered to an owner/editor caller', + ).toBeVisible({ timeout: 20_000 }) + + // Arm the create BEFORE the click. The button also triggers a GET of the + // version list first, so the predicate pins the method as well as the URL. + const created = page.waitForResponse( + (r) => r.url().includes(`/apps/openbuild/api/applications/${SLUG}/versions`) + && r.request().method() === 'POST', + { timeout: 45_000 }, + ) + + await newDraft.click() + + const res = await created + expect(res.status(), 'the draft create must answer 2xx').toBeLessThan(300) + + const sent = res.request().postDataJSON() + expect(sent.status, 'the created version must be a draft').toBe('draft') + expect(sent.application, 'the draft must point at the parent Application uuid').toBe(objectId) + expect( + sent.manifest, + 'the draft manifest must be a clone of the production version manifest', + ).toEqual(productionRow?.manifest ?? {}) + expect( + Object.prototype.hasOwnProperty.call(sent, 'register'), + 'the payload must OMIT register so the backend inherits production\'s — ' + + 'sending one is how a per-version register gets minted by accident', + ).toBe(false) + + // "the version list re-renders showing the new draft" + await expect( + page.locator('.version-history__row'), + 'the list must gain the new draft row', + ).toHaveCount(before.length + 1, { timeout: 30_000 }) + + // The register really is shared, read back from the server rather than + // inferred from the absent request field. + const after = await listVersions(page, SLUG) + const draft = after.find((v) => !before.some((b) => (b?.id ?? b?.uuid) === (v?.id ?? v?.uuid))) + expect(draft, 'the new draft must be listed').toBeTruthy() + expect( + draft?.register, + 'the new draft must SHARE the production register, not mint its own', + ).toEqual(productionRow?.register) + }) +}) From 3ed089e8df02fd5f9b5dd80f4ecdd1dfac08f5a7 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 11 Aug 2026 11:16:49 +0200 Subject: [PATCH 2/2] fix(e2e): the icon-remove test needs the wide viewport it lost in the rewrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI reported "Expected: visible / Received: hidden" on `.ob-icon-section`, one assertion AFTER the sidebar tab panel check passed — so the panel mounts and the section inside it is laid out at zero width. The Application detail page is a three-pane desktop surface; at the project default of 1280x720 the right-hand sidebar collapses. An earlier draft of this file carried `test.use({ viewport: 1600x1200 })` and got past this point; the rewrite that reduced the file to the single remove-scenario dropped it, and it still passed locally, so nothing surfaced until CI. No assertion is weakened: the test drives the same clicks and still pins the numeric DELETE URL shape, the null-iconDark PATCH payload and the read-back. --- tests/e2e/spec-coverage/app-icon-management.spec.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/e2e/spec-coverage/app-icon-management.spec.ts b/tests/e2e/spec-coverage/app-icon-management.spec.ts index 348c08670..2a9383a36 100644 --- a/tests/e2e/spec-coverage/app-icon-management.spec.ts +++ b/tests/e2e/spec-coverage/app-icon-management.spec.ts @@ -124,6 +124,16 @@ function iconRow(page: Page, variant: 'Light' | 'Dark') { } test.describe('app-icon-management — removing an icon (REQ-OBICON-004)', () => { + // The Application detail page is a three-pane desktop surface. At the + // project default of 1280x720 the right-hand sidebar collapses: the Icons + // TAB PANEL still mounts (so `cn-object-sidebar-tab-icons` is visible) but + // `IconUploadSection` inside it is laid out at zero width and reports + // `hidden`. That is exactly how this failed on CI while passing locally — + // "Expected: visible / Received: hidden" on `.ob-icon-section`, one + // assertion after the panel check passed. An earlier draft of this file + // carried the override and got past this point; the rewrite dropped it. + test.use({ viewport: { width: 1600, height: 1200 } }) + test.beforeEach(async ({ page }) => { await suppressSupportDialog(page) await suppressSetupWizard(page)