Skip to content

fix(nav): drop the hand-rolled Admin settings link - #1468

Merged
rubenvdlinde merged 2 commits into
developmentfrom
fix/app-chrome-drop-the-hand-rolled-admin-link
Sep 3, 2026
Merged

fix(nav): drop the hand-rolled Admin settings link#1468
rubenvdlinde merged 2 commits into
developmentfrom
fix/app-chrome-drop-the-hand-rolled-admin-link

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Why

ADR-110 Decision 2 and ADR-114 Decision 2: CnAppNav auto-prepends an Admin settings link to /settings/admin/<appId> for instance admins (ADR-079), and no app declares its own.

shillinq declared GeneralSettings pointing at the same place, so the settings foldout carried two copies of one link. The app's copy had no isAdmin gating, which the shell's does, so it rendered for users who cannot open the page it points at.

No capability is lost. The shell already draws the link.

What

The fragment held nothing else, so it goes with the entry, and the id leaves menu-layout.json#settingsSection with it. Features & roadmap moves from order 91 to 100 (ADR-114 Decision 1 reserves 95 for Reports).

The e2e this would have broken

tests/e2e/chart-of-accounts.spec.ts asserted this exact entry by data-testid. It now asserts DeadlineCalendarSettings, the first id the settingsSection list carries.

Deliberately an id this app owns: asserting on a shell-drawn entry would smoke the library rather than this manifest, and would keep passing over an app whose settings foldout went empty. That spec has now been repointed twice, and its comment records both moves.

Part of the ADR-114 app-chrome rollout (ConductionNL/hydra#643, gate ConductionNL/.github#668).

🤖 Generated with Claude Code

ADR-110 Decision 2 and ADR-114 Decision 2: CnAppNav auto-prepends an Admin
settings link to /settings/admin/<appId> for instance admins (ADR-079), and no
app declares its own.

shillinq declared `GeneralSettings` pointing at the same place, so the settings
foldout carried two copies of one link. The app's copy had no isAdmin gating,
which the shell's does, so it rendered for users who cannot open the page it
points at. No capability is lost: the shell already draws the link.

The fragment held nothing else, so it goes with the entry, and the id leaves
menu-layout.json#settingsSection with it.

tests/e2e/chart-of-accounts.spec.ts asserted this exact entry. It now asserts
DeadlineCalendarSettings, the first id the settingsSection list carries.
Deliberately an id this app OWNS: asserting on a shell-drawn entry would smoke
the library rather than this manifest, and would keep passing over an app whose
settings foldout went empty. That spec has now been repointed twice, and its
comment records both moves.

Also moves Features & roadmap from order 91 to 100: ADR-114 Decision 1 fixes
the footer orders fleet-wide and reserves 95 for Reports.
src/manifest.d.shell.json is generated output that IS committed, and src/main.js
builds the router table and the sidebar from the committed copy. Dropping the
GeneralSettings fragment left the shell one menu id and one fragment stale.

The drift is invisible locally because prebuild/predev/prewatch/pretest:unit all
regenerate the file before anything reads it. Only the committed copy was
stale, and only validate-manifest-shell looks at it.
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/shillinq @ 22a1569

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-manifest-shell
format
check-l10n-js
check-schema-l10n
check-manifest-budget
check-markers
check-registers
check-seeds
check-fragment-required
check-nav-reachability
check-job-registration
test-l10n
composer ✅ 106/106
npm ✅ 537/537
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates

Quality workflow — 2026-09-03 09:54 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit fa04615 into development Sep 3, 2026
57 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/app-chrome-drop-the-hand-rolled-admin-link branch September 3, 2026 10:33
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/shillinq @ 7323ba4

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-manifest-shell
format
check-l10n-js
check-schema-l10n
check-manifest-budget
check-markers
check-registers
check-seeds
check-fragment-required
check-nav-reachability
check-job-registration
test-l10n
composer ✅ 106/106
npm ✅ 537/537
app:check-code ⏭️
info.xml
REUSE
PHPUnit
Newman
Playwright ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates

Quality workflow — 2026-09-03 10:35 UTC

Download the full PDF report from the workflow artifacts.

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