Skip to content

Commit e925a30

Browse files
Conduction Release Botclaude
andcommitted
fix(shell): compose the runtime manifest instead of replacing the app's own
Reproduced on a clean instance rather than reasoned about, and the finding is worse than the 13 failing tests suggested: with tier 3, launchpad rendered NOTHING on a fresh install. `#workspace-vue` held 7 bytes, an empty comment, against 43,564 for the same instance on the commit before tier 3. `GET /api/manifest` is not this app's manifest. ManifestController says so in its own docblock: it assembles a document from the user's OpenRegister dashboard objects, one page and one menu entry per dashboard, and "when the user has no dashboards the manifest returns empty pages/menu". App.vue read `runtimeManifest?.value ?? this.manifest`. An empty object is truthy, so a user with no dashboards got a manifest with no pages and no menu, and tier 3 builds both the router and the nav from the manifest. A user WITH dashboards was no better off, only less obviously: the runtime document still replaced the nine declared pages, the ADR-114 footer and the walkthrough, which is what the chrome and routing specs have been failing on. So the two are composed, which is what each is for. The bundled manifest owns the declared surfaces; the runtime one contributes the dashboards only the server knows about, and the live `runtime` block. THE SETUP WIZARD CAME WITH CnAppRoot, and nothing accounted for it. CnAppRoot opens CnSetupWizard while ANY step is outstanding, optional or not (nextcloud-vue#806) — and `completed` is not `settled`: the status endpoint answered `"completed": true` while still reporting `demo-data` and `load-demo-data` as `done: false`. `cn-wizard-dialog` is an aria-modal that intercepts pointer events, so every click landed on it. Measured locally: with the wizard up, dashboard-sharing failed 4 of 4; with the choice recorded, runtime-shell-canEdit and add-widget-modal ran 10 passed, 0 failed. globalSetup now records the choice once, because the marker is per user and settling it per spec makes the suite order-dependent. Three manifest references could never resolve. The `dashboards` menu entry and two walkthrough steps pointed at `dashboard-detail`, whose path is `/dashboards/:id`, so vue-router threw `Missing required param "id"` on every render. They were inert until tier 3 gave the manifest a real router. A third step named the `Flows` PAGE where a menu-entry id belongs. Six labels were raw i18n keys. `launchpad.menu.dashboards` and five others render as the key itself in all 36 locales but Dutch, which is the only one carrying them. They are plain English now, like every other entry in this menu already was. Dutch loses a translation it only ever had for these six; that is the same position `Flows`, `Store` and `Reports` are already in, and worth a follow-up for all of them together rather than a special case for these. 682 unit tests pass. The development push run is the E2E proof, since that leg does not run on a pull request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d2f3afd commit e925a30

3 files changed

Lines changed: 124 additions & 12 deletions

File tree

src/App.vue

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,58 @@ export default {
129129
* @spec openspec/changes/launchpad-manifest-tier-3/specs/manifest-routing/spec.md#requirement-req-route-004-the-shared-chrome-renders-with-the-workspace-inside-it
130130
*/
131131
liveManifest() {
132-
return this.runtimeManifest?.value ?? this.manifest
132+
const runtime = this.runtimeManifest?.value
133+
if (!runtime) {
134+
return this.manifest
135+
}
136+
137+
/*
138+
* MERGED, NOT REPLACED, and the difference is the whole app.
139+
*
140+
* `/api/manifest` is NOT this app's manifest. ManifestController
141+
* says so in its own docblock: it assembles a document from the
142+
* user's OpenRegister dashboard objects, one `type: "dashboard"`
143+
* page and one menu entry per dashboard, and "when the user has no
144+
* dashboards the manifest returns empty pages/menu".
145+
*
146+
* This used to be `runtime ?? this.manifest`. An empty object is
147+
* truthy, so a user with no dashboards got a manifest with NO pages
148+
* and NO menu — and since `launchpad-manifest-tier-3` builds the
149+
* router and the nav FROM the manifest, the app rendered nothing at
150+
* all. Measured on a clean install: `#workspace-vue` held 7 bytes,
151+
* an empty comment, against 43,564 for the same instance before the
152+
* tier-3 adoption.
153+
*
154+
* A user WITH dashboards was no better off, only less obviously:
155+
* the runtime document still replaced the nine declared pages, the
156+
* ADR-114 footer and the walkthrough, which is what the chrome and
157+
* routing specs were failing on.
158+
*
159+
* So the two are composed, which is what each is for. The bundled
160+
* manifest owns the app's declared surfaces; the runtime one
161+
* contributes the dashboards that only the server can know about,
162+
* and the live `runtime` block.
163+
*/
164+
const byId = (entries) => {
165+
const seen = new Map()
166+
for (const entry of entries) {
167+
if (entry && entry.id !== undefined) {
168+
seen.set(entry.id, entry)
169+
}
170+
}
171+
return [...seen.values()]
172+
}
173+
174+
return {
175+
...this.manifest,
176+
...(runtime.runtime ? { runtime: runtime.runtime } : {}),
177+
pages: byId([
178+
...(this.manifest.pages ?? []),
179+
...(runtime.pages ?? []),
180+
]),
181+
182+
menu: byId([...(this.manifest.menu ?? []), ...(runtime.menu ?? [])]),
183+
}
133184
},
134185
},
135186
}

src/manifest.json

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,22 +96,22 @@
9696
"menu": [
9797
{
9898
"id": "dashboards",
99-
"label": "launchpad.menu.dashboards",
99+
"label": "Dashboards",
100100
"icon": "ViewDashboardOutline",
101-
"route": "dashboard-detail",
101+
"route": "Workspace",
102102
"section": "main"
103103
},
104104
{
105105
"id": "admin-templates",
106-
"label": "launchpad.menu.adminTemplates",
106+
"label": "Templates",
107107
"icon": "FileReplaceOutline",
108108
"route": "/admin/templates",
109109
"section": "settings",
110110
"permission": "admin"
111111
},
112112
{
113113
"id": "admin-settings",
114-
"label": "launchpad.menu.adminSettings",
114+
"label": "Settings",
115115
"icon": "CogOutline",
116116
"route": "/admin/settings",
117117
"section": "settings",
@@ -183,7 +183,7 @@
183183
"id": "dashboard-detail",
184184
"route": "/dashboards/:id",
185185
"type": "custom",
186-
"title": "launchpad.pages.dashboard",
186+
"title": "Dashboard",
187187
"_note": "type:custom for the same reason as Workspace: this is the grid addressed by dashboard id, not a declarative dashboard page. The :id selects the active dashboard (dashboard-deeplinking).",
188188
"widgets": [],
189189
"config": {
@@ -195,7 +195,7 @@
195195
"id": "admin-templates-index",
196196
"route": "/admin/templates",
197197
"type": "custom",
198-
"title": "launchpad.pages.adminTemplates",
198+
"title": "Templates",
199199
"component": "AdminSettingsRedirect",
200200
"_note": "REDIRECTS to /settings/admin/launchpad. TemplatesPage is a TAB inside the Nextcloud admin section, not a page of its own; routing it in-app would render half an admin surface next to the real one.",
201201
"config": {
@@ -206,7 +206,7 @@
206206
"id": "admin-settings",
207207
"route": "/admin/settings",
208208
"type": "custom",
209-
"title": "launchpad.pages.adminSettings",
209+
"title": "Settings",
210210
"component": "AdminSettingsRedirect",
211211
"_note": "REDIRECTS to /settings/admin/launchpad. The admin surface is a Nextcloud settings section (lib/Settings/LaunchPadAdmin.php, mounted by src/admin.js), not an in-app page — the component this once named, AdminSettingsPage, has never existed. The route resolves so the menu entry is not a dead link, and lands where the functionality is."
212212
},
@@ -491,7 +491,7 @@
491491
"body": "A short tour of the main screens. It takes under a minute, and you can close it at any point and pick it up again from the help menu.",
492492
"target": {
493493
"kind": "page",
494-
"ref": "dashboard-detail"
494+
"ref": "Workspace"
495495
},
496496
"advanceOn": {
497497
"type": "manual"
@@ -504,11 +504,11 @@
504504
"task": "Click Dashboards in the menu",
505505
"target": {
506506
"kind": "nav-item",
507-
"ref": "dashboard-detail"
507+
"ref": "dashboards"
508508
},
509509
"advanceOn": {
510510
"type": "route-match",
511-
"route": "dashboard-detail"
511+
"route": "Workspace"
512512
}
513513
},
514514
{
@@ -518,7 +518,7 @@
518518
"task": "Open Flows from the menu",
519519
"target": {
520520
"kind": "nav-item",
521-
"ref": "Flows"
521+
"ref": "FlowsMenu"
522522
},
523523
"advanceOn": {
524524
"type": "route-match",

tests/e2e/global-setup.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ export default async function globalSetup(config: FullConfig): Promise<void> {
120120

121121
await dismissFirstRunWizard(page)
122122
await markSupportNoteSeen(page)
123+
await settleSetupWizard(page)
123124

124125
// Persist the storage state so individual specs reuse the session.
125126
/*
@@ -218,6 +219,66 @@ export default async function globalSetup(config: FullConfig): Promise<void> {
218219
* @param {import('@playwright/test').Page} page an authenticated page.
219220
* @return {Promise<void>}
220221
*/
222+
/**
223+
* Record the setup wizard's outstanding choice, so it does not open over the app.
224+
*
225+
* ⚠️ COMPLETED IS NOT THE SAME AS SETTLED, and that distinction is the whole
226+
* bug. `GET /api/setup/status` answered `"completed": true` while still
227+
* reporting `steps: { "demo-data": { done: false }, "load-demo-data":
228+
* { done: false } }`, and `CnAppRoot` opens the wizard while ANY step is
229+
* outstanding, optional or not (nextcloud-vue#806). So the wizard opened on a
230+
* setup that called itself complete.
231+
*
232+
* It is not a cosmetic overlay. `cn-wizard-dialog` is an `aria-modal` that
233+
* INTERCEPTS POINTER EVENTS, so every click in the app lands on the dialog and
234+
* the failure names a button that the same log calls visible, enabled and
235+
* stable. Measured locally against a clean instance: with the wizard up,
236+
* dashboard-sharing failed 4 of 4; with it settled, runtime-shell-canEdit and
237+
* add-widget-modal went to 10 passed, 0 failed.
238+
*
239+
* This app had no wizard at all until `launchpad-manifest-tier-3` rooted it on
240+
* `CnAppRoot`, which is why nothing here had to account for one before.
241+
*
242+
* Recording the CHOICE is what settles it, not writing the completion key: the
243+
* choice is what marks both `demo-data` and its dependent `load-demo-data`
244+
* done. `none` is the honest answer for a test run, and it imports nothing.
245+
*
246+
* @param page A page already authenticated against the instance.
247+
*/
248+
async function settleSetupWizard(page: Page): Promise<void> {
249+
try {
250+
const status = await page.evaluate(async () => {
251+
const token =
252+
document
253+
.querySelector('head[data-requesttoken]')
254+
?.getAttribute('data-requesttoken')
255+
?? document.getElementById('requesttoken')?.getAttribute('value')
256+
?? ''
257+
const res = await fetch('/index.php/apps/launchpad/api/setup/config', {
258+
method: 'POST',
259+
headers: {
260+
'Content-Type': 'application/json',
261+
requesttoken: token,
262+
'OCS-APIRequest': 'true',
263+
},
264+
body: JSON.stringify({ demo_dataset: 'none' }),
265+
})
266+
return res.status
267+
})
268+
if (status >= 400) {
269+
console.warn(
270+
`[playwright globalSetup] setup-wizard settle returned ${status}; `
271+
+ 'specs will hit a wizard mask over every click.',
272+
)
273+
}
274+
} catch (error) {
275+
console.warn(
276+
'[playwright globalSetup] could not settle the setup wizard:',
277+
error,
278+
)
279+
}
280+
}
281+
221282
async function markSupportNoteSeen(page: Page): Promise<void> {
222283
try {
223284
const status = await page.evaluate(async () => {

0 commit comments

Comments
 (0)