DRY pass #2, batch 4, F12: one note fetch, one tag-name match, one push-landed reply (#5372)

notes.helpers._fetch_note is the parse-id, not-purged, gated select that
_get_owned, _get_visible and _get_editable each wrote out, and note_visible
is the viewer's visibility predicate that five note reads spelled in full.
labeling.named is the case-insensitive live-name match that tags, labels
(create and rename) and the sync push each wrote; how two tag names compare
is now said in one place (#5385 will change it there). sync._landed is the
flush, read-back-the-revision and reply that four push paths ended with.

The REST routes' commit-and-serialise tails stay: each is two lines, and
whether a route refreshes the row first differs by route.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/inkwell/labeling.py

@@ -10,7 +10,7 @@ from __future__ import annotations
 from datetime import datetime, timezone
 
 from sqlalchemy import delete as sa_delete
-from sqlalchemy import select
+from sqlalchemy import func, select
 
 from .models.label import Label, NoteLabel
 from .models.note import Note
@@ -23,6 +23,13 @@ def live(owner_id):
     return (Label.owner_id == owner_id) & Label.purged_at.is_(None)
 
 
+def named(owner_id, name: str):
+    """`owner_id`'s live tag called `name`, in any case. Names are unique among an
+    owner's live tags without regard to case, because every client's store is; this
+    is the one place that says how two names compare."""
+    return live(owner_id) & (func.lower(Label.name) == name.lower())
+
+
 async def tombstone_label(db, label: Label, edited_at: datetime | None = None) -> None:
     """Delete a tag: detach it from every note and mark the row purged.
 

src/inkwell/labels.py

@@ -6,7 +6,7 @@ from sqlalchemy import func, select
 from .auth import login_required
 from .colors import normalize_color
 from .db import session_scope
-from .labeling import live, tombstone_label
+from .labeling import live, named, tombstone_label
 from .models.label import Label, NoteLabel
 from .responses import json_error, not_found, parse_uuid
 from .serialize import serialize_label
@@ -95,7 +95,7 @@ async def create_label():
         # the path that MINTED the "Groceries" beside "groceries" pair that no synced
         # client can hold, since their `labels` index is unique on `lower(name)`.
         existing = await db.scalar(
-            select(Label).where(live(g.user_id), func.lower(Label.name) == name.lower())
+            select(Label).where(named(g.user_id, name))
         )
         if existing is not None:
             return jsonify(_serialize_label(existing)), 200
@@ -129,8 +129,7 @@ async def update_label(label_id: str):
             # one be made is letting a pull fail later on a phone.
             clash = await db.scalar(
                 select(Label).where(
-                    live(g.user_id),
+                    named(g.user_id, name),
-                    func.lower(Label.name) == name.lower(),
                     Label.id != label.id,
                 )
             )

src/inkwell/notes/__init__.py

@@ -19,7 +19,6 @@ from datetime import datetime, timedelta, timezone
 from quart import Response, g, jsonify, request, send_file
 from sqlalchemy import func, literal_column, select
 
-from ..acl import visible_to_user
 from ..auth import login_required
 from ..colors import normalize_color
 from ..common import coerce_bool, iso, parse_dt
@@ -52,6 +51,7 @@ from .helpers import (
     apply_filter,
     derive_display_title,
     is_empty_note,
+    note_visible,
     store_attachment,
     top_position,
     parse_list_items,
@@ -121,9 +121,7 @@ async def list_notes():
     async with session_scope() as db:
         # Through the viewer's own pin, archive and place: on a note shared with them
         # those are theirs, not the owner's (#5176).
-        stmt = join_state(select(Note), g.user_id).where(
+        stmt = join_state(select(Note), g.user_id).where(note_visible())
-            visible_to_user("note", Note.owner_id, Note.id, g.user_id)
-        )
         stmt = apply_filter(stmt, filter_name, archived_for(g.user_id))
         # Trash is the owner's: a note its owner trashed leaves a recipient's board
         # rather than turning up in their Trash, where they could do nothing with it.
@@ -392,7 +390,7 @@ async def reorder_notes():
                 await db.scalars(
                     select(Note).where(
                         Note.id.in_(parsed),
-                        visible_to_user("note", Note.owner_id, Note.id, g.user_id),
+                        note_visible(),
                         Note.purged_at.is_(None),
                     )
                 )

src/inkwell/notes/helpers.py

@@ -103,6 +103,22 @@ async def top_position(db, owner_id) -> int:
     )
 
 
+def note_visible(permission: str | None = None):
+    """Predicate: the current user may see the note, as its owner or through a share
+    (at `permission`, when one is given). Every note read scoped to a viewer asks
+    through here."""
+    return visible_to_user("note", Note.owner_id, Note.id, g.user_id, permission=permission)
+
+
+async def _fetch_note(db, note_id: str, gate) -> Note | None:
+    """The one shape of a per-note fetch: a well-formed id, a note that isn't purged,
+    and `gate` saying who may have it. None for anything else, so the route 404s."""
+    nid = parse_uuid(note_id)
+    if nid is None:
+        return None
+    return await db.scalar(select(Note).where(Note.id == nid, gate, Note.purged_at.is_(None)))
+
+
 async def _get_owned(db, note_id: str) -> Note | None:
     """Fetch a note the current user OWNS, for every change only an owner may make:
     trash, delete, labels, reminders, attachments, previews, history and sharing
@@ -116,44 +132,21 @@ async def _get_owned(db, note_id: str) -> Note | None:
     editing or restoring one 404s. The sync push path looks rows up directly rather
     than through here, which is what still lets a client re-create an id it owns.
     """
-    nid = parse_uuid(note_id)
+    return await _fetch_note(db, note_id, Note.owner_id == g.user_id)
-    if nid is None:
-        return None
-    return await db.scalar(
-        select(Note).where(Note.id == nid, Note.owner_id == g.user_id, Note.purged_at.is_(None))
-    )
 
 
 async def _get_visible(db, note_id: str) -> Note | None:
     """Fetch a note the current user can see: theirs, or shared with them at any
     level. What a recipient may do to it from here is their own pin, archive and
     place (#5176); `_get_editable` and `_get_owned` gate the rest."""
-    nid = parse_uuid(note_id)
+    return await _fetch_note(db, note_id, note_visible())
-    if nid is None:
-        return None
-    return await db.scalar(
-        select(Note).where(
-            Note.id == nid,
-            visible_to_user("note", Note.owner_id, Note.id, g.user_id),
-            Note.purged_at.is_(None),
-        )
-    )
 
 
 async def _get_editable(db, note_id: str) -> Note | None:
     """Fetch a note the current user may change the TEXT of: its body and its
     checklist. The owner, or someone it is shared with at `edit`; a view share gets
     None, and so a 404, like a stranger (#5174)."""
-    nid = parse_uuid(note_id)
+    return await _fetch_note(db, note_id, note_visible(EDIT))
-    if nid is None:
-        return None
-    return await db.scalar(
-        select(Note).where(
-            Note.id == nid,
-            visible_to_user("note", Note.owner_id, Note.id, g.user_id, permission=EDIT),
-            Note.purged_at.is_(None),
-        )
-    )
 
 
 def _slugify(text: str) -> str:

src/inkwell/notes/tags.py

@@ -18,7 +18,7 @@ import re
 
 from sqlalchemy import func, select
 
-from ..labeling import live
+from ..labeling import named
 from ..models.label import Label, NoteLabel
 from ..models.note import Note
 from .helpers import derive_display_title
@@ -124,7 +124,7 @@ async def _find_or_create_label(db, owner_id, name: str):
     """Owner's live label id for `name` (case-insensitive match), creating it if absent.
     A deleted tag's tombstone never matches: tagging a note with it again makes a new tag."""
     existing = await db.scalar(
-        select(Label.id).where(live(owner_id), func.lower(Label.name) == name.lower())
+        select(Label.id).where(named(owner_id, name))
     )
     if existing is not None:
         return existing

src/inkwell/sync.py

@@ -21,11 +21,10 @@ from quart import Blueprint, g, jsonify, request
 from sqlalchemy import delete as sa_delete
 from sqlalchemy import func, select
 
-from .acl import visible_to_user
 from .auth import login_required
 from .common import iso, parse_dt
 from .db import session_scope
-from .labeling import live, reconcile_manual_labels, resolve_owned_label_ids, tombstone_label
+from .labeling import live, named, reconcile_manual_labels, resolve_owned_label_ids, tombstone_label
 from .models.label import Label, NoteLabel
 from .models.note import Note
 from .models.note_attachment import NoteAttachment
@@ -38,7 +37,7 @@ from .notes import (
     normalize_recurrence,
 )
 from .notes.body import write_body
-from .notes.helpers import _get_editable, _get_visible, store_attachment
+from .notes.helpers import _get_editable, _get_visible, note_visible, store_attachment
 from .responses import json_error, not_found, parse_uuid
 from .retention import purge_note
 from .serialize import serialize_label_sync
@@ -140,7 +139,7 @@ async def changes():
     # notes it would treat as its own: pinning them, labelling them, pushing them.
     with_shares = request.args.get("shares") == "1"
     if with_shares:
-        visible = visible_to_user("note", Note.owner_id, Note.id, g.user_id)
+        visible = note_visible()
     else:
         visible = Note.owner_id == g.user_id
     async with session_scope() as db:
@@ -268,6 +267,14 @@ def _result(rid, entity: str, status: str, **extra) -> dict:
     return {"id": rid, "entity": entity, "status": status, **extra}
 
 
+async def _landed(db, row, entity: str, status: str) -> dict:
+    """The reply for a push that changed `row`: flush it, read back the revision the
+    trigger gave it, and report that, so the client's cursor never runs ahead of it."""
+    await db.flush()
+    await db.refresh(row, ["sync_revision"])
+    return _result(str(row.id), entity, status, sync_revision=row.sync_revision)
+
+
 async def _apply_note(db, ch: dict, previews: list[tuple[uuid.UUID, str]]) -> dict:
     """Apply one pushed note. A note whose text changed is appended to `previews` as
     (id, final body), for the caller to queue link previews once the batch commits."""
@@ -288,9 +295,7 @@ async def _apply_note(db, ch: dict, previews: list[tuple[uuid.UUID, str]]) -> di
         if not client_wins(edited_at, note.updated_at):
             return _result(str(nid), "note", "kept", sync_revision=note.sync_revision)
         await purge_note(db, note, edited_at)
-        await db.flush()
+        return await _landed(db, note, "note", "applied")
-        await db.refresh(note, ["sync_revision"])
-        return _result(str(nid), "note", "applied", sync_revision=note.sync_revision)
 
     creating = note is None
     if creating:
@@ -321,9 +326,7 @@ async def _apply_note(db, ch: dict, previews: list[tuple[uuid.UUID, str]]) -> di
     if edited_at is not None:
         note.updated_at = edited_at
     await _apply_note_manual_labels(db, note, ch)
-    await db.flush()
+    return await _landed(db, note, "note", "created" if creating else "applied")
-    await db.refresh(note, ["sync_revision"])
-    return _result(str(nid), "note", "created" if creating else "applied", sync_revision=note.sync_revision)
 
 
 async def _apply_shared_note(db, note: Note, ch: dict, edited_at, previews: list[tuple[uuid.UUID, str]]) -> dict:
@@ -374,7 +377,7 @@ async def _apply_shared_note(db, note: Note, ch: dict, edited_at, previews: list
         join_state(select(revision_for()).select_from(Note), g.user_id).where(Note.id == note.id)
     )
     status = "kept" if kept and not applied else "applied"
-    return {"id": nid, "entity": "note", "status": status, "sync_revision": revision}
+    return _result(nid, "note", status, sync_revision=revision)
 
 
 async def _apply_label(db, ch: dict) -> dict:
@@ -396,8 +399,7 @@ async def _apply_label(db, ch: dict) -> dict:
         if not client_wins(edited_at, label.updated_at):
             return _result(str(lid), "label", "kept", sync_revision=label.sync_revision)
         await tombstone_label(db, label, edited_at)
-        await db.refresh(label, ["sync_revision"])
+        return await _landed(db, label, "label", "applied")
-        return _result(str(lid), "label", "applied", sync_revision=label.sync_revision)
 
     name = (ch.get("name") or "").strip()
     creating = label is None
@@ -407,7 +409,7 @@ async def _apply_label(db, ch: dict) -> dict:
     # id can't be an insert. A tombstone holds no name (0040), so it never clashes.
     if name:
         clash = await db.scalar(
-            select(Label.id).where(live(g.user_id), func.lower(Label.name) == name.lower(), Label.id != lid)
+            select(Label.id).where(named(g.user_id, name), Label.id != lid)
         )
         if clash is not None:
             return _result(str(lid), "label", "rejected", error="name in use")
@@ -425,9 +427,7 @@ async def _apply_label(db, ch: dict) -> dict:
         label.color = normalize_color(ch.get("color"))
     if edited_at is not None:
         label.updated_at = edited_at
-    await db.flush()
+    return await _landed(db, label, "label", "created" if creating else "applied")
-    await db.refresh(label, ["sync_revision"])
-    return _result(str(lid), "label", "created" if creating else "applied", sync_revision=label.sync_revision)
 
 
 # Entities a push may only DELETE. Each is a note's child, so deleting one bumps its