From 667563a2df119726b87cf68f66c810e612bff545 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Wed, 16 Sep 2026 08:38:06 +0200 Subject: [PATCH] fix(tests): parse info.xml from a string, and format the connection registry files simplexml_load_file() returns false under the Nextcloud bootstrap, because lib/base.php installs a libxml external entity loader that returns null and that resolver also handles the primary document. Reading the bytes and parsing a string never touches it. Also runs prettier over the files the connection registry wave merged unformatted, so the Frontend Check (format) job passes. --- src/App.vue | 4 +- .../__tests__/connectionRegistry.spec.js | 43 +++++++++---- src/services/connectionRegistry.js | 15 +++-- .../Settings/ConnectionsDeclarationTest.php | 9 ++- tests/e2e/integrations-page.spec.ts | 60 ++++++++++++++----- 5 files changed, 99 insertions(+), 32 deletions(-) diff --git a/src/App.vue b/src/App.vue index 69b0a842..ccca12c2 100644 --- a/src/App.vue +++ b/src/App.vue @@ -162,7 +162,9 @@ export default { * Integrations page (adopt-connection-registry); nextcloud-vue * 2.46.0 ships neither as a built-in. Static, so not reactive. */ - formatters: createConnectionFormatters((source) => ncT('launchpad', source)), + formatters: createConnectionFormatters((source) => + ncT('launchpad', source), + ), /** * The header-action handler map. CnIndexPage resolves a handler diff --git a/src/services/__tests__/connectionRegistry.spec.js b/src/services/__tests__/connectionRegistry.spec.js index cf4bda55..2d52d484 100644 --- a/src/services/__tests__/connectionRegistry.spec.js +++ b/src/services/__tests__/connectionRegistry.spec.js @@ -42,9 +42,14 @@ describe('connection formatters', () => { const formatters = createConnectionFormatters(translate) it('labels all six statuses, limited included', () => { - expect(Object.keys(CONNECTION_STATUS_LABELS).sort()).toEqual( - ['configured', 'error', 'limited', 'simulated', 'unavailable', 'unconfigured'], - ) + expect(Object.keys(CONNECTION_STATUS_LABELS).sort()).toEqual([ + 'configured', + 'error', + 'limited', + 'simulated', + 'unavailable', + 'unconfigured', + ]) expect(formatters.connectionStatus('configured')).toBe('t:Configured') expect(formatters.connectionStatus('limited')).toBe('t:Limited') expect(formatters.connectionStatus('unconfigured')).toBe('t:Not configured') @@ -70,7 +75,9 @@ describe('connection formatters', () => { }) it('offers Open settings only when the row has a settings link', () => { - expect(formatters.connectionSettingsLabel('/settings/admin/launchpad')).toBe('t:Open settings') + expect(formatters.connectionSettingsLabel('/settings/admin/launchpad')).toBe( + 't:Open settings', + ) expect(formatters.connectionSettingsLabel('')).toBe('') expect(formatters.connectionSettingsLabel(undefined)).toBe('') expect(formatters.connectionSettingsLabel(null)).toBe('') @@ -109,8 +116,12 @@ describe('Add integration handler', () => { handlers.openIntegriqConnections() - expect(INTEGRIQ_CONNECTIONS_PATH).toBe('/apps/integriq/connections?app=launchpad&link=1') - expect(opened).toEqual(['/index.php/apps/integriq/connections?app=launchpad&link=1']) + expect(INTEGRIQ_CONNECTIONS_PATH).toBe( + '/apps/integriq/connections?app=launchpad&link=1', + ) + expect(opened).toEqual([ + '/index.php/apps/integriq/connections?app=launchpad&link=1', + ]) }) }) @@ -144,15 +155,20 @@ describe('the Integrations page declaration', () => { const formatters = createConnectionFormatters(translate) for (const column of page.config.columns.filter((c) => c.formatter)) { - expect(typeof formatters[column.formatter], column.formatter).toBe('function') + expect(typeof formatters[column.formatter], column.formatter).toBe( + 'function', + ) } // The REAL map CnAppRoot receives, not a fresh copy: CnIndexPage // resolves a handler name against it and silently falls back to // emit-only when the name is missing. - const { default: customComponents } = await import('../../customComponents.js') + const { default: customComponents } = + await import('../../customComponents.js') for (const action of page.config.headerActions) { - expect(typeof customComponents[action.handler], action.handler).toBe('function') + expect(typeof customComponents[action.handler], action.handler).toBe( + 'function', + ) } const app = read('src', 'App.vue') @@ -164,7 +180,10 @@ describe('the Integrations page declaration', () => { it('names an icon src/icons.js registers', () => { const icons = read('src', 'icons.js') - for (const icon of [menu.icon, ...page.config.headerActions.map((a) => a.icon)]) { + for (const icon of [ + menu.icon, + ...page.config.headerActions.map((a) => a.icon), + ]) { expect(icons).toContain(`\n\t${icon},`) } }) @@ -177,6 +196,8 @@ describe('the Integrations page declaration', () => { const merged = applyManifestFragments(base, [fragment]) expect(merged.pages.filter((p) => p.id === 'Integrations')).toHaveLength(1) - expect(merged.menu.filter((m) => m.id === 'IntegrationsMenu')).toHaveLength(1) + expect(merged.menu.filter((m) => m.id === 'IntegrationsMenu')).toHaveLength( + 1, + ) }) }) diff --git a/src/services/connectionRegistry.js b/src/services/connectionRegistry.js index 4cdc8c13..60f839f7 100644 --- a/src/services/connectionRegistry.js +++ b/src/services/connectionRegistry.js @@ -20,7 +20,8 @@ * Where Add integration lands: integriq's Connections overview, preset to this * app and opening the link-a-source dialog (hydra connection-registry D9). */ -export const INTEGRIQ_CONNECTIONS_PATH = '/apps/integriq/connections?app=launchpad&link=1' +export const INTEGRIQ_CONNECTIONS_PATH = + '/apps/integriq/connections?app=launchpad&link=1' /** * The English label for each of the six registry statuses (design D3). @@ -54,9 +55,11 @@ export function createConnectionFormatters(translate) { * @spec openspec/changes/adopt-connection-registry/specs/app-connections/spec.md#requirement-req-lp-conn-004-an-admin-reads-the-connections-on-an-integrations-page */ connectionStatus(value) { - const source = typeof value === 'string' && Object.hasOwn(CONNECTION_STATUS_LABELS, value) - ? CONNECTION_STATUS_LABELS[value] - : null + const source = + typeof value === 'string' + && Object.hasOwn(CONNECTION_STATUS_LABELS, value) + ? CONNECTION_STATUS_LABELS[value] + : null return source ? translate(source) : String(value ?? '') }, @@ -69,7 +72,9 @@ export function createConnectionFormatters(translate) { * @spec openspec/changes/adopt-connection-registry/specs/app-connections/spec.md#requirement-req-lp-conn-004-an-admin-reads-the-connections-on-an-integrations-page */ connectionSettingsLabel(value) { - return typeof value === 'string' && value.length > 0 ? translate('Open settings') : '' + return typeof value === 'string' && value.length > 0 + ? translate('Open settings') + : '' }, } } diff --git a/tests/Unit/Settings/ConnectionsDeclarationTest.php b/tests/Unit/Settings/ConnectionsDeclarationTest.php index 723b682a..c25e9101 100644 --- a/tests/Unit/Settings/ConnectionsDeclarationTest.php +++ b/tests/Unit/Settings/ConnectionsDeclarationTest.php @@ -164,7 +164,14 @@ public function testTheSchemaRefusesAnUnknownField(): void { * @return void */ public function testTheFileNamesThisApp(): void { - $infoXml = simplexml_load_file($this->root() . '/appinfo/info.xml'); + // Deliberately file_get_contents() + simplexml_load_string() rather than + // simplexml_load_file(). Under the Nextcloud bootstrap lib/base.php calls + // libxml_set_external_entity_loader() with a loader returning null, and + // that resolver also handles the primary document, so load_file() returns + // false for a well-formed info.xml. Parsing a string never touches it. + $infoXml = simplexml_load_string( + (string)file_get_contents($this->root() . '/appinfo/info.xml') + ); $this->assertNotFalse(condition: $infoXml); $this->assertSame(expected: (string) $infoXml->id, actual: $this->declaration()['app']); diff --git a/tests/e2e/integrations-page.spec.ts b/tests/e2e/integrations-page.spec.ts index 0ebdb747..3477f6a6 100644 --- a/tests/e2e/integrations-page.spec.ts +++ b/tests/e2e/integrations-page.spec.ts @@ -43,7 +43,8 @@ import { BASE_URL as BASE } from './support/baseUrl.ts' const APP = '/index.php/apps/launchpad' /** Integriq's objects endpoint for LaunchPad's connection rows. */ -const CONNECTIONS_API = '/index.php/apps/openregister/api/objects/integriq/app_connection?app=launchpad&_limit=50' +const CONNECTIONS_API = + '/index.php/apps/openregister/api/objects/integriq/app_connection?app=launchpad&_limit=50' /** LaunchPad's registry settings, the one writer of the registry keys. */ const STORE_CONFIG_API = `${APP}/api/store/config` @@ -89,14 +90,18 @@ async function adminApi(): Promise { * @param api An admin request context. * @return The rows by key. */ -async function rowsByKey(api: APIRequestContext): Promise>> { +async function rowsByKey( + api: APIRequestContext, +): Promise>> { const res = await api.get(CONNECTIONS_API) expect(res.ok(), `list integriq/app_connection -> ${res.status()}`).toBeTruthy() const body = await res.json() const byKey: Record> = {} for (const row of (body.results ?? []) as Record[]) { // A row from another app here means the bare filter was dropped. - expect(String(row.app), 'a connection row from another app').toBe('launchpad') + expect(String(row.app), 'a connection row from another app').toBe( + 'launchpad', + ) byKey[String(row.key)] = row } return byKey @@ -114,7 +119,10 @@ async function rowsByKey(api: APIRequestContext): Promise { const list = await api.get(CONNECTIONS_API) const rows = list.ok() ? ((await list.json()).results ?? []) : [] - const row = rows.find((r: Record) => r.key === 'dashboard-registry' && r.app === 'launchpad') + const row = rows.find( + (r: Record) => + r.key === 'dashboard-registry' && r.app === 'launchpad', + ) return `${String(row?.status ?? '')} ${String(row?.statusMessage ?? '')}` } @@ -125,7 +133,11 @@ async function registryRow(api: APIRequestContext): Promise { * @param url The registry URL to save. * @param block What to do while it is saved. */ -async function withRegistryUrl(api: APIRequestContext, url: string, block: () => Promise): Promise { +async function withRegistryUrl( + api: APIRequestContext, + url: string, + block: () => Promise, +): Promise { const before = await api.get(STORE_CONFIG_API) expect(before.ok(), `registry config read -> ${before.status()}`).toBeTruthy() const previous = String((await before.json())?.registryUrl ?? '') @@ -146,7 +158,9 @@ async function withRegistryUrl(api: APIRequestContext, url: string, block: () => * @param page The Playwright page. */ async function openIntegrations(page: Page): Promise { - await page.goto(`${APP}/settings/integrations?app=launchpad`, { timeout: 60_000 }) + await page.goto(`${APP}/settings/integrations?app=launchpad`, { + timeout: 60_000, + }) await expect(page.locator('.cn-index-page')).toBeVisible({ timeout: 30_000 }) } @@ -161,14 +175,20 @@ test.describe('Integrations over the connection registry', () => { await api.dispose() }) - test('lists the six declared connections, all of them LaunchPad\'s', async ({ page }) => { + test("lists the six declared connections, all of them LaunchPad's", async ({ + page, + }) => { const byKey = await rowsByKey(api) expect(Object.keys(byKey).sort()).toEqual(DECLARED.map((d) => d.key).sort()) - expect(String(byKey['dashboard-registry']?.settingsUrl ?? '')).toBe('/settings/admin/launchpad?tab=sharing#section-dashboard-registry') + expect(String(byKey['dashboard-registry']?.settingsUrl ?? '')).toBe( + '/settings/admin/launchpad?tab=sharing#section-dashboard-registry', + ) await openIntegrations(page) for (const { title } of DECLARED) { - await expect(page.getByRole('row', { name: new RegExp(title, 'i') })).toHaveCount(1) + await expect( + page.getByRole('row', { name: new RegExp(title, 'i') }), + ).toHaveCount(1) } }) @@ -184,17 +204,23 @@ test.describe('Integrations over the connection registry', () => { await withRegistryUrl(api, UNREACHABLE_REGISTRY, async () => { // One search. Its own answer is not under test: it answers the // unreachable outcome, as it did before this change. What it reports is. - const search = await api.get(STORE_SEARCH_API, { failOnStatusCode: false }) + const search = await api.get(STORE_SEARCH_API, { + failOnStatusCode: false, + }) expect(search.status(), `store search -> ${search.status()}`).toBe(200) expect((await search.json()).outcome).toBe('store_unreachable') await expect .poll(() => registryRow(api), { timeout: 15_000 }) - .toBe('error The last search could not reach the dashboard registry at registry.example.invalid.') + .toBe( + 'error The last search could not reach the dashboard registry at registry.example.invalid.', + ) }) }) - test('sends Add integration to integriq instead of offering a form', async ({ page }) => { + test('sends Add integration to integriq instead of offering a form', async ({ + page, + }) => { await openIntegrations(page) // No generic Add button: a row nothing declared has nothing to check. @@ -204,8 +230,14 @@ test.describe('Integrations over the connection registry', () => { // catalogues this change ships, and nothing forces the E2E locale. await page.locator('[data-testid="cn-actions"] button').first().click() await Promise.all([ - page.waitForURL(/\/apps\/integriq\/connections\?app=launchpad&link=1$/, { timeout: 30_000 }), - page.getByRole('menuitem', { name: /Add integration|Integratie toevoegen/i }).click(), + page.waitForURL(/\/apps\/integriq\/connections\?app=launchpad&link=1$/, { + timeout: 30_000, + }), + page + .getByRole('menuitem', { + name: /Add integration|Integratie toevoegen/i, + }) + .click(), ]) }) })