Repository navigation
Settings: hide the deletion token modal without opening deletion to all users - #8319
Conversation
…ll users Until now the only way to stop the one-time "Save your pad deletion token" modal from interrupting every new creator was allowPadDeletionByAllUsers, which also lets any editor delete the pad -- a data-loss risk operators did not want. Add a suppressPadDeletionTokenModal setting (with a SUPPRESS_PAD_DELETION_TOKEN_MODAL container env var) that hides the modal while leaving deletion rights untouched: the token is still issued and still usable to delete the pad from another device, and non-creators still cannot delete. It only stops the modal from being shown on arrival. The server still sends padDeletionToken in clientVars, so a client that wants to surface it on demand (e.g. from a settings panel) can; only the interrupting modal is suppressed. Add a regression test asserting the token is still issued and canDeleteWithoutToken stays false when the modal is suppressed.
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoSuppress the pad deletion token modal without changing deletion rights
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
1.
|
Addresses review feedback on ether#8319. 1. Creators could not retrieve the token. The modal is the only place the plaintext token is ever shown, and the server stores only a hash, so a creator who did not save it on arrival could never delete the pad from another device. The suppressed branch now hands the token to a new disclosure in the pad settings panel (#delete-pad-token-reveal), which shows the value with a copy button. It is unhidden only for the session that actually received a token under suppression, so nothing is revealed to later visits (which get null anyway) or to non-creators. The modal itself stays hidden. 2. The CHANGELOG edit had rewritten existing UTF-8 punctuation to mojibake across historical entries. Restored the file from develop and re-added only the new ether#7996 bullet, this time keeping the original bytes intact. Add a source-lint regression test covering the settings control and the client wiring, plus locale strings for the new labels.
|
Addressed both review findings in 3a8a97a.
Also added a source-lint regression test ( This comment was posted by an AI agent (OpenHands) on behalf of the contributor. |
|
Status: ready for review on the current head (
@JohnMcLear whenever you have a moment — this one plus #8314 and #8320 are all ready for a look. Happy to rebase or adjust anything. |
|
Yea I like this, reviewing now |
|
Few bits I don't like, UX is awful, UI is lazy. If you are going to get an AI Agent to work on the code, please click through/test it first! |
Follow-up review changes for ether#7996: - Replace the "Show the pad deletion token" disclosure with a button that shows the token in a read-only input, selects it and copies it to the clipboard. Move it above the delete controls instead of under them. - Replace the source-text vitest with Playwright tests that drive the real flow: no modal, one click shows and copies the token, and a second browser deletes the pad with it. Also check that the button is not offered when the modal is not suppressed. - Back up suppressPadDeletionTokenModal in the socketio spec so the new test doesn't leak it into later specs. - Drop the CHANGELOG entry: on current develop it landed in the already released 3.3.7 section. Release notes are written at release time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VigRrw8y5o5FaKH2JSzoAv
|
Thanks @sqmyou, this is a good fix. I pushed a few review changes on top (083a6a3) and merged develop in:
Tests: the deletion-token Playwright specs pass in Chromium and Firefox (12/12), backend 1843 passing, vitest 840 passing, ts-check clean. 🤖 Generated with Claude Code |
The show-and-copy button sat on its own between the settings and the delete buttons, and disappeared after a reload because the server only sends the token once. It is now a "Show and copy deletion token" toggle at the end of the User Settings column, like the other toggles. Turning it on shows the token, selects it and copies it to the clipboard. Under suppressPadDeletionTokenModal the browser keeps the token in localStorage per pad, so the toggle is still there after a reload. Anything that can read that storage could already delete the pad as the creator. It is only offered where this browser can delete the pad, and the saved token is cleared when the pad is deleted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VigRrw8y5o5FaKH2JSzoAv
|
One more change after a click-through (1bf72db): the suppressed token is now a "Show and copy deletion token" toggle at the end of the User Settings column, like the other toggles, instead of a button near the delete controls. It also survives a reload now. The server only sends the token once, so under 🤖 Generated with Claude Code |
What and why
Fixes #7996.
Until now the only way to stop the one-time "Save your pad deletion token" modal from interrupting every new creator was
allowPadDeletionByAllUsers: true. That setting does exactly what it says — it also lets any editor delete the pad, even if they are not the creator — so operators had to choose between an annoying modal and a data-loss risk.This adds
suppressPadDeletionTokenModal(defaultfalse). When enabled it hides the modal without widening deletion rights:clientVars.padDeletionToken, so a client that wants to surface it on demand, e.g. from a settings panel, still can);canDeleteWithoutTokenstaysfalseand non-creators still cannot delete.It only stops the modal from being shown on arrival.
The setting is also exposed to the container image as
SUPPRESS_PAD_DELETION_TOKEN_MODAL, and documented insettings.json.template/settings.json.docker.Files
src/node/utils/Settings.ts— new setting + defaultsrc/node/handler/PadMessageHandler.ts— sendsuppressPadDeletionTokenModalin clientVarssrc/static/js/pad.ts— skipshowDeletionTokenModalIfPresent()when suppressedsrc/static/js/types/SocketIOMessage.ts— type the new client varsettings.json.template,settings.json.docker— docs + env varsrc/tests/backend/specs/socketio.ts— regression testTesting
New regression test in the socketio spec asserts that with the setting on the creator still receives a token,
suppressPadDeletionTokenModalistrue, andcanDeleteWithoutTokenremainsfalse(i.e. deletion rights are unchanged). I verified the test fails without the server-side change (45 passing / 1 failing) and passes with it (46 passing):pnpm run ts-checkpasses.Note:
pnpm run lintcurrently fails ondevelopfor an unrelated reason — ESLint 10 no longer loads the repo's.eslintrc.cjs(flat-config migration), soeslint .cannot find a config at all. Not touched here.