Skip to content

fix(secrets): confirm before deleting a secret from the detail sidebar - #599

Open
remko48 wants to merge 3 commits into
developmentfrom
fix/must-list-bug-sweep
Open

fix(secrets): confirm before deleting a secret from the detail sidebar#599
remko48 wants to merge 3 commits into
developmentfrom
fix/must-list-bug-sweep

Conversation

@remko48

@remko48 remko48 commented Sep 2, 2026

Copy link
Copy Markdown
Member

Depends on

ConductionNL/nextcloud-vue#931 — keepiq pins @conduction/nextcloud-vue
^2.29.0, so this needs #931 merged, released, and the dependency bumped here
before it can be verified end to end. The dialog itself uses only
cnOpenModal and the modal registry, which the pinned version already
provides — the sidebar opens secret-edit, secret-move and secret-share
the same way today — so no code here waits on new library API.

The problem

The sidebar's menu called deleteSecret() straight from the click handler:
the only irreversible action in the app was also the only one with no
confirmation, on the path people actually use, because list-view rows carry no
menu of their own. The table-view row menu already confirmed, which is why
this went unnoticed.

The change

Adds SecretDeleteConfirmDialog, registered as the secret-delete modal and
opened through cnOpenModal like the sidebar's edit, move and share dialogs.
It mirrors FolderDeleteConfirmDialog: it owns the delete call and emits
deleted, so a refused delete (403 on a delegated secret, a server error, an
offline write) keeps the dialog open with the reason inline rather than closing
the sidebar behind it — which reads exactly like a delete that worked.

The wording reuses the existing Delete secret, Cancel and Failed to delete secret keys. The one new string mirrors BulkDeleteDialog's no-trash phrasing
and ships with its Dutch translation.

Reviewer note on l10n: this consumes the last unit of slack in the parity
ratchet. Every locale except nl now sits exactly at its bound, so the next
new source string will fail test:l10n unless it is translated into all ~36
locales.

The sidebar's ... menu called deleteSecret() straight from the click handler:
the only irreversible action in the app was also the only one with no
confirmation, on the path people actually use, because list-view rows carry no
... menu of their own. The table-view row menu already confirmed.

Adds SecretDeleteConfirmDialog, registered as the `secret-delete` modal and
opened through cnOpenModal like the sidebar's edit, move and share dialogs.
It mirrors FolderDeleteConfirmDialog: it owns the delete call and emits
`deleted`, so a refused delete (403 on a delegated secret, offline write)
keeps the dialog open with the reason inline rather than closing the sidebar
behind it, which reads exactly like a delete that worked.

Reuses the existing "Delete secret", "Cancel" and "Failed to delete secret"
keys; the one new warning string mirrors BulkDeleteDialog's no-trash wording
and ships with its Dutch translation. Note this consumes the last unit of
slack in the l10n parity ratchet: every locale except nl now sits exactly at
its bound, so the next new source string needs translating everywhere.
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/keepiq @ 2949638

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-02 11:11 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/keepiq @ 2fbdcbe

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 07:07 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/keepiq @ b230c7f

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 07:20 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