feat(notes): save a note from its editor

apps/web/e2e/notes-edit.spec.ts

@@ -1,3 +1,5 @@
+import type { Page, Request } from '@playwright/test';
+
 import { home } from './demo';
 import { expect, test } from './fixtures';
 
@@ -7,7 +9,7 @@ const riff = '---\ntags: [guitar]\n---\n# Riff\n\nPlay it **slow**.\n';
 test.beforeEach(async ({ page }) => {
   await page.route('/api/home', (route) => route.fulfill({ json: home }));
   await page.route('/api/notes/files/Songs/Riff.md', (route) =>
-    route.fulfill({ body: riff, contentType: 'text/markdown; charset=utf-8' })
+    route.fulfill({ body: riff, contentType: 'text/markdown; charset=utf-8', headers: { ETag: '"1"' } })
   );
 });
 
@@ -45,3 +47,64 @@ test('a note offers no Edit while the notes service saves no note', async ({ pag
   await expect(page.getByRole('article')).toContainText('Play it slow.');
   await expect(page.getByRole('button', { name: 'Edit' })).toHaveCount(0);
 });
+
+// Answers the note's saves with status, and keeps their requests.
+async function saves(page: Page, status: number): Promise<Request[]> {
+  const requests: Request[] = [];
+  await page.route('/api/notes/files/Songs/Riff.md', (route) => {
+    if (route.request().method() !== 'PUT') {
+      return route.fallback();
+    }
+    requests.push(route.request());
+    return route.fulfill({ status, headers: status === 204 ? { ETag: '"2"' } : {} });
+  });
+  return requests;
+}
+
+// The note's page from a notes service that saves notes, with the note in the editor and " Then fast." typed in it.
+async function typed(page: Page) {
+  await page.route('/api/notes', (route) => route.fulfill({ json: { files, editable: true } }));
+  await page.goto('/notes/Songs/Riff');
+  await page.getByRole('button', { name: 'Edit' }).click();
+  const line = page.getByRole('textbox', { name: 'Markdown' }).locator('.cm-line').nth(5);
+  await line.click();
+  await page.keyboard.press('End');
+  await page.keyboard.type(' Then fast.');
+  return line;
+}
+
+test('Save sends the note with the version it loaded, then shows the note and says it saved', async ({ page }) => {
+  const requests = await saves(page, 204);
+  await typed(page);
+
+  await page.getByRole('button', { name: 'Save' }).click();
+
+  await expect(page.getByRole('article')).toContainText('Play it slow. Then fast.');
+  await expect(page.getByText('Saved', { exact: true })).toBeVisible();
+  await expect(page.getByRole('button', { name: 'Edit' })).toBeFocused();
+  expect(requests).toHaveLength(1);
+  expect(requests[0]?.headers()).toMatchObject({ 'if-match': '"1"', 'content-type': 'text/markdown; charset=utf-8' });
+  expect(requests[0]?.postData()).toBe(riff.replace('**slow**.', '**slow**. Then fast.'));
+});
+
+test('a note changed since it loaded keeps the editor and the text, and says so', async ({ page }) => {
+  await saves(page, 412);
+  const line = await typed(page);
+
+  await page.getByRole('button', { name: 'Save' }).click();
+
+  await expect(page.getByText('Not saved: the note has changed since it loaded')).toBeVisible();
+  await expect(line).toHaveText('Play it **slow**. Then fast.');
+  await expect(page.getByRole('article')).toHaveCount(0);
+});
+
+test('⌘S, or Ctrl+S, in the editor saves', async ({ page }, testInfo) => {
+  test.skip(testInfo.project.name === 'phone', 'a phone has no ⌘ or Ctrl key without a keyboard');
+  const requests = await saves(page, 204);
+  await typed(page);
+
+  await page.keyboard.press('ControlOrMeta+s');
+
+  await expect(page.getByText('Saved', { exact: true })).toBeVisible();
+  expect(requests).toHaveLength(1);
+});

apps/web/package.json

@@ -17,6 +17,7 @@
     "@codemirror/lang-markdown": "6.5.2",
     "@codemirror/lang-yaml": "6.1.3",
     "@codemirror/language": "6.13.1",
+    "@codemirror/state": "6.7.6",
     "@codemirror/view": "6.43.15",
     "@fontsource-variable/jetbrains-mono": "5.3.0",
     "@fontsource-variable/space-grotesk": "5.3.0",

apps/web/src/notes/NoteEditor.svelte

@@ -1,12 +1,12 @@
 <script lang="ts">
-import { noteEditor } from './editor';
+import { type EditorPage, noteEditor } from './editor';
 
-// A note's Markdown in the editor, in place of the rendered note. A chunk of its own, with CodeMirror, which loads the
-// first time Edit is pressed.
-let { markdown }: { markdown: string } = $props();
+// A note's Markdown in the editor, in place of the rendered note, and the page that takes its text and saves it. A
+// chunk of its own, with CodeMirror, which loads the first time Edit is pressed.
+let { markdown, page }: { markdown: string; page: EditorPage } = $props();
 </script>
 
-<div class="editor" {@attach noteEditor(markdown)}></div>
+<div class="editor" {@attach noteEditor(markdown, page)}></div>
 
 <style>
 /* CodeMirror's own elements, which Svelte can't scope: a form field's box (decision 112), as tall as the note, with

apps/web/src/notes/NotePage.save.test.ts

@@ -0,0 +1,88 @@
+import { flushSync } from 'svelte';
+import { EditorView } from '@codemirror/view';
+import { fireEvent, render, screen } from '@testing-library/svelte';
+import { afterEach, beforeEach, expect, test, vi } from 'vitest';
+
+import NotePage from './NotePage.svelte';
+import { NotesState } from './state.svelte';
+import { noteAt } from './tree';
+
+const toDo = noteAt('/notes/Daily/To%20do')!;
+const markdown = '# To do\n\n- [ ] Tune';
+const typedText = `${markdown} now`;
+
+let fetchMock: ReturnType<typeof vi.fn<typeof fetch>>;
+
+beforeEach(() => {
+  fetchMock = vi.fn<typeof fetch>().mockResolvedValueOnce(new Response(markdown, { headers: { ETag: '"1"' } }));
+  vi.stubGlobal('fetch', fetchMock);
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+});
+
+// The note's page from a notes service that saves notes, with the note in the editor and " now" typed at its end.
+async function typed() {
+  const notes = new NotesState();
+  notes.files = ['Daily/To do.md'];
+  notes.editable = true;
+  render(NotePage, { props: { note: toDo, notes } });
+  await fireEvent.click(await screen.findByRole('button', { name: 'Edit' }));
+  const view = EditorView.findFromDOM(await screen.findByRole('textbox', { name: 'Markdown' }))!;
+  view.dispatch({ changes: { from: markdown.length, insert: ' now' }, userEvent: 'input.type' });
+  return { notes, view };
+}
+
+function button(name: string): HTMLElement {
+  return screen.getByRole('button', { name });
+}
+
+test('Save follows Cancel, and saves the typed text, then shows the note, says so and gives Edit the focus', async () => {
+  await typed();
+  expect(screen.getAllByRole('button').map((each) => each.textContent?.trim())).toEqual(['Cancel', 'Save']);
+  expect(screen.getByRole('status').textContent).toBe('');
+  fetchMock.mockResolvedValueOnce(new Response(null, { status: 204, headers: { ETag: '"2"' } }));
+
+  await fireEvent.click(button('Save'));
+
+  expect(await screen.findByRole('article')).toHaveTextContent('Tune now');
+  expect(fetchMock).toHaveBeenLastCalledWith(
+    '/api/notes/files/Daily/To%20do.md',
+    expect.objectContaining({ method: 'PUT', body: typedText })
+  );
+  expect(screen.getByRole('status')).toHaveTextContent('Saved');
+  expect(button('Edit')).toHaveFocus();
+  expect(screen.queryByRole('button', { name: 'Save' })).not.toBeInTheDocument();
+});
+
+test('says so while a save runs, when Save, Cancel and the editor wait, and when the note has changed', async () => {
+  const { view } = await typed();
+  const put = Promise.withResolvers<Response>();
+  fetchMock.mockReturnValueOnce(put.promise);
+
+  await fireEvent.click(button('Save'));
+
+  expect(screen.getByRole('status')).toHaveTextContent('Saving…');
+  expect(button('Save')).toHaveAttribute('aria-disabled', 'true');
+  expect(button('Cancel')).toHaveAttribute('aria-disabled', 'true');
+  view.dispatch({ changes: { from: 0, insert: 'Late. ' }, userEvent: 'input.type' });
+  expect(view.state.doc.toString()).toBe(typedText);
+  put.resolve(new Response(null, { status: 412 }));
+  expect(await screen.findByText('Not saved: the note has changed since it loaded')).toBeInTheDocument();
+  expect(button('Save')).toHaveAttribute('aria-disabled', 'false');
+  expect(view.state.doc.toString()).toBe(typedText);
+});
+
+test('keeps Save and Cancel while the editor is open, when a sync says that notes no longer saves', async () => {
+  const { notes } = await typed();
+
+  flushSync(() => {
+    notes.editable = false;
+  });
+  expect(button('Save')).toBeInTheDocument();
+  await fireEvent.click(button('Cancel'));
+
+  expect(await screen.findByRole('article')).toHaveTextContent('Tune');
+  expect(screen.queryByRole('button')).not.toBeInTheDocument();
+});

apps/web/src/notes/NotePage.svelte

@@ -1,5 +1,6 @@
 <script lang="ts">
 import PageHead from '../shell/PageHead.svelte';
+import { saveLabel } from './labels';
 import NoteBody from './NoteBody.svelte';
 import { ShownNote } from './shown.svelte';
 import type { NotesState } from './state.svelte';
@@ -21,16 +22,33 @@ $effect(() => {
   <PageHead eyebrow={note.folder}>{note.title}</PageHead>
   <div class="content">
     {#if shown.body.kind === 'loaded'}
-      {#if notes.editable}
+      <!-- The buttons stay while the editor is open, even if a sync says that notes no longer saves. -->
+      {#if notes.editable || shown.editing}
+        <!--
+          Save follows Cancel, so Cancel takes Edit's place, and Shift-Tab from the editor, which comes after them,
+          reaches Save first. aria-disabled, not disabled, so the focus stays on a button while its save runs.
+        -->
         <div class="tools form-row">
-          <button class="form-button" type="button" onclick={(event) => shown.toggle(event.currentTarget)}>
+          <button
+            class="form-button"
+            type="button"
+            aria-disabled={shown.busy}
+            onclick={() => shown.toggle()}
+            {@attach shown.button}
+          >
             {shown.editing ? 'Cancel' : 'Edit'}
           </button>
+          {#if shown.editing}
+            <button class="form-button" type="button" aria-disabled={shown.busy} onclick={() => void shown.save()}>
+              Save
+            </button>
+          {/if}
+          <p class="form-status" role="status">{saveLabel(shown.status)}</p>
         </div>
       {/if}
       {#if shown.editing}
         {#await import('./NoteEditor.svelte') then { default: NoteEditor }}
-          <NoteEditor markdown={shown.body.markdown} />
+          <NoteEditor markdown={shown.body.markdown} page={shown} />
         {:catch}
           <p class="failed">Couldn't load the editor.</p>
         {/await}

apps/web/src/notes/api.test.ts

@@ -1,6 +1,6 @@
 import { afterEach, expect, test, vi } from 'vitest';
 
-import { fetchNote, fetchNotes, fileUrl, syncNotes } from './api';
+import { fetchNote, fetchNotes, fileUrl, saveNote, syncNotes } from './api';
 
 afterEach(() => {
   vi.unstubAllGlobals();
@@ -49,14 +49,22 @@ test('gives a file’s URL, each name in its path escaped', () => {
   expect(fileUrl('My art/a b%?.png')).toBe('/api/notes/files/My%20art/a%20b%25%3F.png');
 });
 
-test('loads a note through the gateway, each name in its path escaped', async () => {
-  const fetchMock = vi.fn<typeof fetch>().mockResolvedValue(new Response('# To do\n'));
+const version = '"9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"';
+
+test('loads a note and its version through the gateway, each name in its path escaped', async () => {
+  const fetchMock = vi.fn<typeof fetch>().mockResolvedValue(new Response('# To do\n', { headers: { ETag: version } }));
   vi.stubGlobal('fetch', fetchMock);
 
-  await expect(fetchNote('Daily/To do #1?.md')).resolves.toBe('# To do\n');
+  await expect(fetchNote('Daily/To do #1?.md')).resolves.toEqual({ markdown: '# To do\n', version });
   expect(fetchMock).toHaveBeenCalledExactlyOnceWith('/api/notes/files/Daily/To%20do%20%231%3F.md');
 });
 
+test('gives a blank version for a note answered without one', async () => {
+  vi.stubGlobal('fetch', vi.fn<typeof fetch>().mockResolvedValue(new Response('# To do\n')));
+
+  await expect(fetchNote('Daily/To do.md')).resolves.toEqual({ markdown: '# To do\n', version: '' });
+});
+
 test('gives no note when the vault has none at the path', async () => {
   vi.stubGlobal('fetch', vi.fn<typeof fetch>().mockResolvedValue(new Response('no such file', { status: 404 })));
 
@@ -74,3 +82,44 @@ test('fails to load a note when the gateway can not be reached', async () => {
 
   await expect(fetchNote('Daily/To do.md')).rejects.toThrow('Failed to fetch');
 });
+
+test('saves a note through the gateway, if it is still at the version it loaded, and gives its new version', async () => {
+  const saved = '"60303ae22b998861bce3b28f33eec1be758a213c86c93c076dbe9f558c11c752"';
+  const fetchMock = vi
+    .fn<typeof fetch>()
+    .mockResolvedValue(new Response(null, { status: 204, headers: { ETag: saved } }));
+  vi.stubGlobal('fetch', fetchMock);
+
+  await expect(saveNote('Daily/To do #1?.md', '# To do\n\n- [ ] Tune\n', version)).resolves.toBe(saved);
+  expect(fetchMock).toHaveBeenCalledExactlyOnceWith('/api/notes/files/Daily/To%20do%20%231%3F.md', {
+    method: 'PUT',
+    headers: { 'Content-Type': 'text/markdown; charset=utf-8', 'If-Match': version },
+    body: '# To do\n\n- [ ] Tune\n',
+  });
+});
+
+test('gives a blank version for a save answered without one', async () => {
+  vi.stubGlobal('fetch', vi.fn<typeof fetch>().mockResolvedValue(new Response(null, { status: 204 })));
+
+  await expect(saveNote('Daily/To do.md', 'Tune', version)).resolves.toBe('');
+});
+
+test('gives no version when the note has changed since it loaded', async () => {
+  vi.stubGlobal('fetch', vi.fn<typeof fetch>().mockResolvedValue(new Response(null, { status: 412 })));
+
+  await expect(saveNote('Daily/To do.md', 'Tune', version)).resolves.toBeUndefined();
+});
+
+test.each([400, 428, 500, 502])('fails when the gateway answers a save with %i', async (status) => {
+  vi.stubGlobal('fetch', vi.fn<typeof fetch>().mockResolvedValue(new Response(null, { status })));
+
+  await expect(saveNote('Daily/To do.md', 'Tune', version)).rejects.toThrow(
+    `PUT /api/notes/files/Daily/To%20do.md: ${status}`
+  );
+});
+
+test('fails to save a note when the gateway can not be reached', async () => {
+  vi.stubGlobal('fetch', vi.fn<typeof fetch>().mockRejectedValue(new TypeError('Failed to fetch')));
+
+  await expect(saveNote('Daily/To do.md', 'Tune', version)).rejects.toThrow('Failed to fetch');
+});

apps/web/src/notes/api.ts

@@ -31,11 +31,18 @@ export function fileUrl(path: string): string {
   return `/api/notes/files/${path.split('/').map(encodeURIComponent).join('/')}`;
 }
 
+/** A note's Markdown, and its version, which a save sends back (decision 400). */
+export interface NoteFile {
+  markdown: string;
+  /** The note's ETag, the SHA-256 of its bytes in quotes; blank when the answer had none. */
+  version: string;
+}
+
 /**
- * Loads the note at path in the vault, such as "Work/Plans/Q3.md": its Markdown, or undefined when the vault has no
- * such note. Fails when the gateway can't be reached or answers with another error.
+ * Loads the note at path in the vault, such as "Work/Plans/Q3.md": its Markdown and version, or undefined when the
+ * vault has no such note. Fails when the gateway can't be reached or answers with another error.
  */
-export async function fetchNote(path: string): Promise<string | undefined> {
+export async function fetchNote(path: string): Promise<NoteFile | undefined> {
   const url = fileUrl(path);
   const response = await fetch(url);
   if (response.status === 404) {
@@ -44,5 +51,25 @@ export async function fetchNote(path: string): Promise<string | undefined> {
   if (!response.ok) {
     throw new Error(`GET ${url}: ${response.status}`);
   }
-  return response.text();
+  return { markdown: await response.text(), version: response.headers.get('ETag') ?? '' };
+}
+
+/**
+ * Saves markdown as the note at path, unless the note has changed since version: the saved note's version, or
+ * undefined when the note has changed. Fails when the gateway can't be reached or answers with another error.
+ */
+export async function saveNote(path: string, markdown: string, version: string): Promise<string | undefined> {
+  const url = fileUrl(path);
+  const response = await fetch(url, {
+    method: 'PUT',
+    headers: { 'Content-Type': 'text/markdown; charset=utf-8', 'If-Match': version },
+    body: markdown,
+  });
+  if (response.status === 412) {
+    return undefined;
+  }
+  if (!response.ok) {
+    throw new Error(`PUT ${url}: ${response.status}`);
+  }
+  return response.headers.get('ETag') ?? '';
 }