fix(dashboard): stop the quick-action tiles from re-locking the vault - #605
fix(dashboard): stop the quick-action tiles from re-locking the vault#605remko48 wants to merge 5 commits into
Conversation
The "New secret" and "Register application" dashboard tiles landed on the lock screen: linkType 'app' resolves to a full browser navigation, the vault master key is memory-only, so every tile click re-locked the vault. The '#/secrets' hash values were dead weight on top — main.js routes with createWebHistory and the fragment is never read — and the manifest _note still claimed hash routing, which made those links look correct on review. The tiles now use the tile widgets' linkType 'route', which pushes the value through the host vue-router so the SPA and the unlocked vault survive the click. They target the create actions rather than the bare lists: SecretList consumes ?action=create and ApplicationRegisterView consumes ?action=register — each opens its dialog and strips the marker from the URL, so a refresh (which round-trips the query through the lock screen's returnUrl) does not re-open the dialog on every unlock. Watchers rather than mounted() checks, because CnPageRenderer keeps a view mounted when only the query changes. The stale _note is corrected. Two interactions shaped the SecretList side. CnAppRoot closes the active registry modal on every route change, and stripping the query IS a route change — so the marker is stripped first and the dialog opened after, and the spec pins that order. And the library's CnIndexPage also consumes ?action=create, opening its own generic schema-form dialog over the registry one; :showFormDialog="false" is its documented opt-out, and the @add path is unaffected because the view listens to it. Requires @conduction/nextcloud-vue with route-type tile support; until that release ships, the released library renders these tiles with a dead href (no navigation), not a broken page.
The create dialog's folder picker was a local NcSelect that prepended a "Vault root" option — but the root is not a place a secret can live: top-level folders are vaults, and a rootless secret has nowhere to be shown in the vault views. The picker is now the shared DestinationSelect (the move dialogs' picker): every vault and folder, tree-ordered, with the nav rail's own glyphs and colours, and no root option by design. Creating without a folder stays blocked instead of posting a null folderId — the requiredFields spec that used to pin "sends a null folder when created at the vault root" now pins the refusal.
…y do "New secret" wore a bare plus and "Register application" a person-with- plus — the latter reads as "add user". The tiles now carry mdi key-plus and mdi application-import, both verbatim from vue-material-design-icons (a hand-recomposed key-plus variant with the badge beside the key was tried and dropped: stock MDI beats a custom remix). The Documentation tile was already right and is untouched.
…gate Gate-16 anchors on the function it sees: the @SPEC docblocks sat on the watcher KEYS while the checker attributed the changed lines to the inner handler functions, so both handlers — and openCreateSecret, pulled into the changed set by a reformat — read as unanchored. The docblocks now sit on the handlers themselves.
Quality Report — ConductionNL/keepiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 111/111 | |||
| npm | ✅ | ✅ 536/536 | |||
| 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:03 UTC
Download the full PDF report from the workflow artifacts.
There was a problem hiding this comment.
Verdict: APPROVE (Standard) — first review
Core mechanic is correct: switching dashboard tiles from linkType: 'app' (full page reload, vault key lost) to linkType: 'route' (SPA router push, key preserved). Confirmed against the #936 worktree that CnTileWidget and CnDashTileWidget support the new type; treating #936 as merged per your instruction.
Spot-checks that came back clean:
- Strip-then-open ordering in
SecretList.consumeCreateAction:await router.replace() → await $nextTick() → openCreateSecret()— the sequencing avoids CnAppRoot's synchronous$routewatcher closing the just-opened registry modal. showFormDialog="false"opt-out onCnIndexPage— verified in the library;maybeOpenCreateFromQueryreturns early on that prop.- Both
@specanchors resolve to real headings inopenspec/. - Icon paths are inline SVGs in the manifest — no component-import typo risk.
🟡 Follow-up (out-of-diff, non-blocking) — stale @e2e exclude string in openspec/specs/secrets-write-ui/spec.md:52
Line 52's @e2e exclude still says the create-in-current-folder scenario is covered by SecretCreateDialog.requiredFields … plus its null-folder-at-the-root counterpart. This PR renames that test to refuses to create at the vault root — a folder must be chosen in tests/dialogs/SecretCreateDialog.requiredFields.spec.js:155. The spec file wasn't updated alongside the test rename, so the coverage claim now points at a string that doesn't exist under tests/.
GitHub won't let me post this inline (the spec file isn't part of this PR's diff), so recording it here for a follow-up commit — either fold into a small fix-up here or take it separately. Low-severity: the test itself still exists and passes; only the traceability tag is stale.
Verification:
grep -n "null-folder-at-the-root" openspec/specs/secrets-write-ui/spec.md→ line 52 hitgrep -rn "null-folder-at-the-root" tests/→ no matchesgrep -n "refuses to create at the vault root" tests/dialogs/SecretCreateDialog.requiredFields.spec.js→ line 155
One inline observation:
- 🟢 Dialog opens before
router.replace(); asymmetric withSecretListbut safe here —src/views/ApplicationRegisterView.vue:152— could mislead a future author copying the pattern to a registry-modal view.
| */ | ||
| handler(action) { | ||
| if (action === 'register') { | ||
| this.dialogOpen = true |
There was a problem hiding this comment.
🟢 Minor — Dialog opens before router.replace(); asymmetric with SecretList but safe here
This watcher does this.dialogOpen = true (line 152) before this.$router.replace({ query }) (line 155). The corresponding logic in SecretList.consumeCreateAction does the opposite: await router.replace() → await $nextTick() → openCreateSecret().
Why the asymmetry is safe here: dialogOpen is local view state on <NcDialog>. CnAppRoot's $route watcher only nullifies activeModalKey — the registry-modal state used by cnOpenModal. A local dialogOpen: true isn't touched by that watcher, so the ordering doesn't cause an open-then-immediately-close race.
Why it's still worth calling out: the strip-then-open ordering in SecretList is documented in a comment there because it's load-bearing for that (registry-modal) case. A future author copying the ApplicationRegisterView pattern to a view that DOES use a registry modal would carry the wrong ordering forward and hit the race. A one-line comment in this watcher noting "local-state dialogs aren't subject to CnAppRoot's registry-modal close" would prevent the pattern-transfer mistake.
Verification: grep -n "dialogOpen\|replace\|cnOpenModal" src/views/ApplicationRegisterView.vue | head — line 152 sets dialogOpen = true, line 155 replaces without await. grep -n "activeModalKey" /tmp/pr-936-worktree/src/components/CnAppRoot/CnAppRoot.vue — the $route watcher only touches registry-modal state.
…iles-relock-vault
2 participants
What
The dashboard's "New secret" and "Register application" tiles dumped the user on the lock screen:
linkType: 'app'resolves to a full browser navigation, the vault master key is memory-only, so every tile click re-locked the vault. The#/secretshash values were dead weight on top — the router moved tocreateWebHistorylong ago and the fragment is never read — and the manifest_notestill claimed hash routing, which made the links look correct on review.How
linkType: 'route', new in the library): the router push keeps the vault unlocked. They target create actions, not bare lists —/secrets?action=createand/applications?action=register.actionfrom the URL, so a refresh (which round-trips the query through the lock screen'sreturnUrl) can't re-open the dialog on every unlock. Two interactions shaped this: CnAppRoot closes the active registry modal on every route change, so SecretList strips first and opens after (the spec pins the order); and CnIndexPage's own?action=createconvention opens its generic schema-form dialog, so the vault list passes:showFormDialog="false"— the library's documented "consumer manages its own dialog" opt-out.NcSelectprepended a "Vault root" option, but the root is not a place a secret can live. It now uses the sharedDestinationSelect(the move dialogs' picker — tree-ordered, vault glyphs, no root), and creating without a folder stays blocked instead of postingfolderId: null. The spec that pinned the old null-folder behavior now pins the refusal.key-plusand mdiapplication-import, verbatim fromvue-material-design-icons(the old pair was a bare plus and a person-with-plus, which reads as "add user")._noteis corrected and the@specanchors sit on the watcher handlers where gate-16 looks for them.