Skip to content

fix(fe): gate-13 — extract four inline modals, and fix the slot typo that made four settings info panels render empty - #479

Merged
rubenvdlinde merged 2 commits into
developmentfrom
fix/gate13-modal-isolation
Aug 10, 2026
Merged

fix(fe): gate-13 — extract four inline modals, and fix the slot typo that made four settings info panels render empty#479
rubenvdlinde merged 2 commits into
developmentfrom
fix/gate13-modal-isolation

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

gate-13 (modal-isolation) reported three files with inline NcModal/NcDialog markup. Fixing them surfaced a second, unrelated defect that no gate and no test could see, because its failure mode is silence — and fixing that one unmasked a third that had been latent since the day it was written.

The slot bug

AlwaysVisibleSection.vue declared <slot name="info" />. Four callers pass <template #info-content>:

  • src/views/settings/sections/UserGroupsConfiguration.vue
  • src/views/settings/sections/EmailConfiguration.vue
  • src/views/settings/sections/ArchiMateImportExport.vue
  • src/views/settings/sections/OrganizationSynchronization.vue

All four also set :has-info-content="true", so the ⓘ button rendered and opened a modal with nothing in it. Vue silently drops slot content addressed to a slot the child does not declare — no warning, no error, no failing test. The name came from CollapsibleSection.vue, which really does use info-content; only VersionInformation.vue used the working info name.

Both sections now render <slot name="info-content"><slot name="info" /></slot>, so info-content wins and info stays a fallback. All five callers render.

The bug the slot fix unmasked

The first push turned Playwright E2E red: 6 failed / 70 passed, every failure a settings test asserting "no app console errors", every one reporting the same runtime error thrown while the settings page rendered:

TypeError: Cannot read properties of undefined (reading 'name')
    at .../js/softwarecatalog-settings.js?v=999be7ef-0:2:761407

Mapping offset 2:761407 in the built bundle gives:

(0,i.eW)(" - Use placeholders like "+(0,m.v_)(e.organization.name)+" and "+(0,m.v_)(e.user.email),1)

which is EmailConfiguration.vue:385:

<li><strong>Template Variables</strong> - Use placeholders like {{ organization.name }} and {{ user.email }}</li>

Those braces are literal documentation of the e-mail template placeholders an admin can use. Vue compiles them as interpolation against the component, which has no organization and no user.

The slot rename did not introduce this — it unmasked it. Because AlwaysVisibleSection only ever declared info, Vue had been silently dropping this panel's markup, so the broken interpolation was never evaluated. Fixing the slot made the panel render for the first time in its life; it threw immediately, and it poisoned the shared "no console errors" assertion across every settings test — which is why an unrelated EolSyncSettings test went red too.

v-pre keeps the braces as documentation.

Negative result, with the search space stated: the other three newly-rendering panels (UserGroupsConfiguration, ArchiMateImportExport, OrganizationSynchronization) and VersionInformation's #info panel were all rendered and checked. Exactly one offending line exists in the whole app.

It was not the test double

The obvious, plausible, wrong conclusion was "the @nextcloud/vue stub hid it". It did not. The real, unstubbed AlwaysVisibleSection was mounted under jsdom across six usage shapes — no info content, #info only, #info-content only, both, neither, and header-actions — and all six render clean with the real NcModal. The slot-forwarding mechanism was never the problem.

The actual gap was different: sectionInfoSlot.spec.js exercised the mechanism with synthetic probe markup and never rendered any real caller's content. A mechanism test cannot see a defect in the content it does not render.

So tests/vitest/settingsInfoPanels.spec.js renders the real markup of every #info / #info-content block under src/views/settings/sections/. It carries a positive control asserting it found all five files, so a broken extractor cannot vacuously pass, and it asserts the literal braces appear in the rendered output — so "fixing" the crash by deleting the offending line fails too.

The extraction

New file From
src/dialogs/ChangePasswordDialog.vue ContactpersonenList.vue
src/dialogs/ManageUserGroupsDialog.vue ContactpersonenList.vue
src/modals/AlwaysVisibleSectionInfoModal.vue AlwaysVisibleSection.vue
src/modals/CollapsibleSectionInfoModal.vue CollapsibleSection.vue

ContactpersonenList.vue drops 1563 → 954 lines. Password validation, the HIBP pwned-check, the debounce watcher and group selection move into the dialog that owns them; the parent only opens them and reacts to events. updateContactpersoonGroups stays in the parent because it mutates the parent's own organisationData — the dialog reports groups up rather than reaching into it. Because both dialogs mount fresh per open, data() is the state reset the parent used to spell out by hand and beforeUnmount is the timeout cleanup. Every t('softwarecatalog', …) string is preserved verbatim.

Two info-modal files rather than one shared component, deliberately: the two sections render materially different DOM (NcModal's own title chrome + a bare body, versus a hand-painted <h2> + Close footer + ~90 lines of :deep() typography). Sharing them would need a variant flag switching between two disjoint templates and two disjoint stylesheets, and converging them would have changed one section's rendered output.

Both directions

Slot wiring. A vitest spec mounts each section with #info-content supplied. Against the pre-fix wiring:

 FAIL  tests/vitest/sectionInfoSlot.spec.js > renders #info-content inside the info modal
AssertionError: expected false to be true
 FAIL  > prefers #info-content over #info when both are supplied
      Tests  2 failed | 5 passed (7)

The #info case still passed there — which is what shows the test isolates the bug rather than the harness.

Info-panel rendering. With v-pre reverted, settingsInfoPanels.spec.js reproduces the exact CI error string:

 FAIL  tests/vitest/settingsInfoPanels.spec.js > EmailConfiguration.vue renders its info panel without throwing
AssertionError: expected TypeError: Cannot read properties of unde… to be null
  "message": "Cannot read properties of undefined (reading 'name')"
 FAIL  > EmailConfiguration shows its placeholder braces literally
AssertionError: expected '' to contain '{{ organization.name }}'
      Tests  2 failed | 5 passed (7)

After both fixes: 234 passed (22 files).

[gate-13] modal-isolation: FAIL — 3 file(s) with inline modal/dialog   →   PASS

No gate got worse

19 = 291, 25 = 41, 26 = 3 — unchanged. 46 now passes (fixed by 274790f, included on this branch). 5 → 3 failing gates over 65 gate lines.

Worth recording, because it nearly cost an afternoon: an earlier baseline appeared to flip six gates. That baseline had been captured while npm ci was still running — the world changed under the measurement, not the code. Re-measured with node_modules present in both arms, gate-13 is the only verdict that changes. A baseline taken against a half-built environment measures the environment.

Toolchain notes

Vitest could not mount an SFC: no Vue plugin, and environment: 'node'. This adds @vitejs/plugin-vue + jsdom as devDependencies, a @nextcloud/vue stub alongside the existing router/dialogs/l10n stubs, and makes vitest.config.js an async factory so the ESM-only plugin can be dynamic-imported from a CommonJS config. The default environment stays node; the new specs opt into jsdom per-file, so no existing spec changes behaviour.

@vue/compiler-dom is added as a devDependency (@vue/compiler-sfc was already one) for settingsInfoPanels.spec.js. That spec compiles and renders each info-panel template fragment rather than mounting the whole section component — deliberately. Mounting a section drags in the Pinia stores, axios and the Nextcloud runtime that this OFFLINE suite exists to do without; it was attempted and fails on an unrelated CJS resolution issue (@nextcloud/vue's exports map declares no require condition). Rendering the fragment reproduces the real failure exactly, with none of that weight.

npm run lint 0 errors (475 pre-existing warnings) · npm run build compiles · 234/234 unit tests pass.

…made four settings info panels render empty

gate-13 (modal-isolation) reported three files with inline NcModal/NcDialog
markup. Fixing them surfaced a second, unrelated defect that no gate and no
test could see, because its failure mode is silence.

THE SLOT BUG

AlwaysVisibleSection declared `<slot name="info" />`. Four callers pass
`<template #info-content>`:

  UserGroupsConfiguration, EmailConfiguration,
  ArchiMateImportExport, OrganizationSynchronization

All four also set `:has-info-content="true"`, so the (i) button rendered and
opened a modal with nothing in it. Vue drops slot content addressed to a slot
the child does not declare — no warning, no error, no failing test. The name
came from CollapsibleSection, which does use `info-content`; only
VersionInformation used the working `info` name.

Both sections now render `<slot name="info-content"><slot name="info" /></slot>`
so `info-content` wins and `info` remains a fallback. All five callers render.

THE EXTRACTION

  src/dialogs/ChangePasswordDialog.vue      <- ContactpersonenList
  src/dialogs/ManageUserGroupsDialog.vue    <- ContactpersonenList
  src/modals/AlwaysVisibleSectionInfoModal.vue
  src/modals/CollapsibleSectionInfoModal.vue

ContactpersonenList drops 1563 -> 954 lines. Password validation, the HIBP
pwned-check, the debounce watcher and the group selection all move into the
dialog that owns them; the parent now only opens them and reacts to events.
`updateContactpersoonGroups` stays in the parent because it mutates the
parent's own organisationData — the dialog reports groups up rather than
reaching into it. Since both dialogs mount fresh per open, `data()` IS the
state reset the parent used to spell out by hand and `beforeUnmount` IS the
timeout cleanup. Every t('softwarecatalog', ...) string is preserved verbatim.

Two info-modal files rather than one shared component: the two sections render
materially different DOM (NcModal's own title chrome and a bare body, versus a
hand-painted h2 + Close footer + ~90 lines of :deep() typography). Sharing them
would need a variant flag switching between two disjoint templates and two
disjoint stylesheets, and converging them would have changed one section's
rendered output.

BOTH DIRECTIONS

A new vitest spec mounts each section with #info-content supplied. Against the
pre-fix wiring:

  FAIL tests/vitest/sectionInfoSlot.spec.js > renders #info-content inside the info modal
  AssertionError: expected false to be true
  FAIL > prefers #info-content over #info when both are supplied
  Tests  2 failed | 5 passed (7)

The #info case still PASSED there, which is what shows the test isolates the
bug rather than the harness. After the fix: 227 passed (21 files).

  [gate-13] modal-isolation: FAIL - 3 file(s) -> PASS

No other gate moved: 19=291, 25=41, 26=3. (An earlier baseline appeared to
flip six gates; that baseline was captured while `npm ci` was still running.
Re-measured with node_modules present in both arms, gate-13 is the only
verdict that changes.)

TOOLCHAIN

Vitest could not mount an SFC: no Vue plugin, and environment 'node'. Added
@vitejs/plugin-vue + jsdom as devDependencies, a @nextcloud/vue stub alongside
the existing router/dialogs/l10n stubs, and made vitest.config.js an async
factory so the ESM-only plugin can be dynamic-imported from a CommonJS config.
The default environment stays 'node'; the new spec opts into jsdom per-file, so
no existing spec changes behaviour.

lint 0 errors; build compiles; 227/227 unit tests pass.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ 4657574

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-vue-demi
test-l10n
composer ✅ 128/128
npm ✅ 718/718
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-10 20:52 UTC

Download the full PDF report from the workflow artifacts.

…osed

The mis-named `info-content` slot had been hiding a second bug. Because
AlwaysVisibleSection only declared `<slot name="info" />`, Vue silently
dropped the four callers that passed `#info-content` — so their panels
were never rendered, and nothing could fail on them.

EmailConfiguration's panel documents the e-mail template placeholders:

    Use placeholders like {{ organization.name }} and {{ user.email }}

Those braces are meant literally, but Vue compiles them as interpolation
against the component, which has no `organization` and no `user`. The
moment the slot name was fixed and the panel rendered for the first time
it threw `Cannot read properties of undefined (reading 'name')`, which
tripped the shared "no console errors" assertion in every Playwright
settings test.

`v-pre` keeps the braces as documentation. The other four info panels
were checked and render clean.

Also adds tests/vitest/settingsInfoPanels.spec.js, which renders the REAL
markup of every info panel under src/views/settings/sections/. The
existing sectionInfoSlot.spec.js proves the slot MECHANISM forwards
content, but it does so with synthetic probe markup — which is precisely
why it could not see this. Verified both ways: without `v-pre` the new
spec reproduces the exact TypeError from CI.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ 56b875c

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-vue-demi
test-l10n
composer ✅ 128/128
npm ✅ 718/718
PHPUnit
Newman ⏭️
Playwright
Hydra gates

Quality workflow — 2026-08-10 21:32 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit f699b5a into development Aug 10, 2026
29 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/gate13-modal-isolation branch August 10, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant