Repository navigation
feat(notes): save a note from its editor - #180
Merged
Merged
Conversation
Save, or Cmd/Ctrl+S, sends the editor's text with PUT and If-Match naming the version the page loaded, the note's ETag. A 204 shows the saved note at once and says so; a 412, from a note changed since it loaded, keeps the editor and the text and says so; any other failure keeps them and says the save failed. While the save runs, the editor takes no typing and Save and Cancel wait. Decision 402 now also records where Save sits, the status line, and why the note renders before the PUT.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brief
Goal. The note editor saves. Save, or ⌘S, sends the note with
PUT /api/notes/files/<path>andIf-Matchnaming the version the page loaded, which is the note'sETag(decisions 400 and 402). A saved note shows at once. A note that has changed since it loaded gets a 412: the editor keeps the user's text and says so.Scope.
api.ts:fetchNotereturns the note's Markdown and its version, from theETag.saveNotesends thePUTwithIf-Matchandtext/markdown; charset=utf-8. A 204 gives the new version, a 412 gives undefined, and any other answer throws.ShownNote:save().editor.ts:NotePage:aria-disabledwhile a save runs.labels.ts:saveLabel.@codemirror/state6.7.6 becomes a direct dependency, forEditorState.changeFilter. The lockfile already had it, through the other CodeMirror packages.PUT.note-save.Out of scope.
PUThandler, its store andeditable. They come with the store's step, which waits on §12's question.note-unsaved, maybe later.Acceptance criteria.
PUT /api/notes/files/<path>,Content-Type: text/markdown; charset=utf-8, andIf-Matchnaming the version the page loaded.ETagwith the next save, which starts from the saved text.aria-disabledand do nothing;QA
shown.test.ts, new, runsShownNoteagainst a stubbedfetch:api.test.tschecks thePUT's URL, method, headers and body, the 204'sETag, and a blank version when the 204 has none.shown.test.ts).NotePage.save.test.ts, new, a test types into the real CodeMirror during the save and finds the text unchanged.editor.test.tschecks that the editor takes no change while busy.shown.test.tscovers the 412, a 500, and an unreachable gateway.api.test.ts:editor.test.ts, Ctrl+S saves and its keydown's default is prevented, so the browser doesn't save the page. jsdom gives CodeMirror Ctrl, not ⌘.If-Match, the content type and the body.ETag, the one branch of the new code that no test took. A mutation of it now fails that test.bin/e2e/gateway) and the real notes service (bin/notes) on a scratch vault. Only/api/noteswas stubbed, to sayeditable. In both Chromium and WebKit:PUTpasses the gateway's cross-origin protection. Notes, which has noPUTyet, answers 405 (a refused request would get 403), and the page says "Save failed".If-Matchis theETagthe GET gave, the note's SHA-256.@codemirror/state6.7.6 is the version the lockfile already had, and the newest on npm (2026-09-22).api.ts,editor.tsandlabels.tshave every line, branch and function covered.shown.svelte.tshas all 45 lines and 13 functions covered, and 18 of 19 branches. The one left isfiles ?? []insave(), which only satisfies the type, since a loaded note always has the vault's list.Review
No blocking findings. Non-blocking:
ETagleaves the version blank, so the next save sends an emptyIf-Match, which matches no version. The store's step should answer it as it answers a missingIf-Match, with a 428. Notes always sends a note'sETag(decision 400).note-unsavedmay take that up.Gates
mise run checkpassed on aa7784c:fmt:check,gen:check, golangci-lint with 0 issues, jactionlint, the compose lint, andgovulncheckwith no vulnerabilities.tscand knip.pnpm audit.Screenshots
Demo data, a made-up note, Tokyo time. The phone shots come from WebKit.
To be merged under the user's standing OK once CI's
checkis green on a head withorigin/mainmerged in.