From 63955bbe9711585f65d38bdecd0463d3eb77d7de Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 7 Oct 2026 17:13:58 -0400 Subject: [PATCH] sync: recipients keep their own pin, archive and place on shared notes A note_user_state row per (note, recipient) holds what used to be the owner's columns as far as anyone else could tell. The board filters and orders through the viewer's own state; PATCH and reorder write it for a note shared at any level; the feed's revision for a shared note is the later of the note's and the caller's row, so a recipient's pin reaches their devices and no one else's. Push takes the three with their own `state_at` stamp (protocol 7, `shared_state`), so pinning a copy whose text is behind never makes that text win over the owner's edit. A body is only stamped as an edit when it changed. Co-Authored-By: Claude Opus 5.5 --- alembic/versions/0036_note_user_state.py | 55 +++++++++++++ docs/sync.md | 32 ++++++-- src/inkwell/models/all.py | 1 + src/inkwell/models/note_user_state.py | 35 ++++++++ src/inkwell/note_state.py | 100 +++++++++++++++++++++++ src/inkwell/notes/__init__.py | 53 +++++++++--- src/inkwell/notes/helpers.py | 29 +++++-- src/inkwell/notes/serialize.py | 8 +- src/inkwell/sync.py | 97 ++++++++++++++-------- tests/test_integration.py | 82 ++++++++++++++++++- 10 files changed, 432 insertions(+), 60 deletions(-) create mode 100644 alembic/versions/0036_note_user_state.py create mode 100644 src/inkwell/models/note_user_state.py create mode 100644 src/inkwell/note_state.py diff --git a/alembic/versions/0036_note_user_state.py b/alembic/versions/0036_note_user_state.py new file mode 100644 index 0000000..1151159 --- /dev/null +++ b/alembic/versions/0036_note_user_state.py @@ -0,0 +1,55 @@ +"""note_user_state: a recipient's own pin, archive and place for a shared note + +Revision ID: 0036 +Revises: 0035 +Create Date: 2026-10-07 + +Pin, archive and board order are personal organization, and until now they were +columns on `notes`, so on a shared note they could only ever mean the owner's +(#5176). This table holds them for everyone else: one row per (note, person it is +shared with) who has pinned, archived or moved it. The owner keeps the note's own +columns; a recipient with no row sees the note unpinned, unarchived, in the owner's +order. + +`sync_revision` is stamped by the same `ts_set_sync_revision()` trigger the notes +and labels use (0015), so a recipient's change reaches their own devices on the one +cursor they already page by, and never moves the note on anyone else's. + +## Downgrade + +Drops the table. Recipients lose their own pins and archives; the notes are +untouched. +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects.postgresql import UUID + +revision = "0036" +down_revision = "0035" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "note_user_state", + sa.Column("note_id", UUID(as_uuid=True), sa.ForeignKey("notes.id", ondelete="CASCADE"), nullable=False), + sa.Column("user_id", UUID(as_uuid=True), sa.ForeignKey("users.id", ondelete="CASCADE"), nullable=False), + sa.Column("pinned", sa.Boolean(), nullable=False, server_default=sa.false()), + sa.Column("archived", sa.Boolean(), nullable=False, server_default=sa.false()), + sa.Column("position", sa.Integer(), nullable=True), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False, server_default=sa.func.now()), + sa.Column("sync_revision", sa.BigInteger(), nullable=True), + sa.PrimaryKeyConstraint("note_id", "user_id"), + ) + op.create_index("ix_note_user_state_user_revision", "note_user_state", ["user_id", "sync_revision"]) + op.execute( + "CREATE TRIGGER trg_note_user_state_sync_revision BEFORE INSERT OR UPDATE ON note_user_state " + "FOR EACH ROW EXECUTE PROCEDURE ts_set_sync_revision()" + ) + + +def downgrade() -> None: + op.execute("DROP TRIGGER IF EXISTS trg_note_user_state_sync_revision ON note_user_state") + op.drop_index("ix_note_user_state_user_revision", table_name="note_user_state") + op.drop_table("note_user_state") diff --git a/docs/sync.md b/docs/sync.md index d6dad80..c7c2233 100644 --- a/docs/sync.md +++ b/docs/sync.md @@ -67,6 +67,10 @@ syncs everything else. - v6 (#5175): shared notes — see "Shared notes" below. Opt-in per request (`changes?shares=1`), so it is the `shares` feature and the floor stays: a v5 client never asks and gets exactly its own notes. + - v7 (#5176): a recipient's own pin, archive and position on a shared note — + see "Shared notes". Additive, so it is the `shared_state` feature and the + floor stays: a v6 client pushes only text, and reads its own state as if the + note were simply unpinned. - **Additive change** (a new field, a new capability) → add a `sync_features` name. Do **not** raise a minimum. Old clients keep working. - **Breaking change only** → raise `MIN_CLIENT_PROTOCOL_VERSION` (or the client's @@ -230,6 +234,13 @@ sequence and page on the same cursor as notes and labels (three streams now; the smallest full boundary wins). Sharing with the person again deletes their revocation, so a device that never saw it is never told to drop a note it has. +**Pin, archive and position are personal** (`shared_state`, v7). On a note shared +with the caller, the feed's `pinned`, `archived` and `position` are the caller's +own: unpinned and unarchived until they change them, and the owner's `position` +until they move it. They live in a per-person row with a revision of its own, so +the note's `sync_revision` in the feed is the later of the note's and that row's — +a recipient pinning a note reaches their devices and no one else's. + ## Push — `POST /api/sync/push` Body: `{ "changes": [ ... ] }` (max 1000 per batch). Each change: @@ -267,12 +278,21 @@ Body: `{ "changes": [ ... ] }` (max 1000 per batch). Each change: the same as one that never existed. Removals are explicit rather than "the note's current attachment set" on purpose: push runs before pull, so a set would delete an attachment another device added that this one has not seen. -- **A note someone else owns** (`shares`): a recipient at `edit` may push an upsert - and only its `body` is applied, under the same last-write-wins — every other - field is ignored, since pinning, archiving, reminders, labels and deleting stay - the owner's. A `view` recipient's change, or any delete, is `rejected` ("only its - owner can change that"); a note the caller can't see at all answers the generic - "cannot apply". +- **A note someone else owns** (`shares`): an upsert may carry two things, each + under its own last-write-wins. + - Its `body`, from a recipient at `edit`, compared on `edited_at` against the + note. From a `view` recipient a body is `rejected` ("only its owner can change + that"). + - Their own `pinned`, `archived` and `position` (`shared_state`), from anyone it + is shared with, compared on **`state_at`** against what they last set. A + client stamps `state_at` apart from the text's `edited_at` so that pinning a + copy whose text is out of date never makes that text look newer than the + owner's edit. Without `state_at` the three are ignored. + + Trashing, reminders, labels and deleting stay the owner's: those fields are + ignored and a delete is `rejected`. The answer is `applied` if either part + applied, else `kept`, with the revision as that caller's feed would show it. A + note the caller can't see at all answers the generic "cannot apply". ### Conflict resolution — last-write-wins + history diff --git a/src/inkwell/models/all.py b/src/inkwell/models/all.py index ad40538..7515ebc 100644 --- a/src/inkwell/models/all.py +++ b/src/inkwell/models/all.py @@ -12,6 +12,7 @@ from . import ( # noqa: F401 note_attachment, note_link_preview, note_revision, + note_user_state, password_reset, saved_filter, settings, diff --git a/src/inkwell/models/note_user_state.py b/src/inkwell/models/note_user_state.py new file mode 100644 index 0000000..70790a8 --- /dev/null +++ b/src/inkwell/models/note_user_state.py @@ -0,0 +1,35 @@ +from __future__ import annotations + +import uuid +from datetime import datetime + +from sqlalchemy import BigInteger, Boolean, DateTime, ForeignKey, Integer, func +from sqlalchemy.dialects.postgresql import UUID +from sqlalchemy.orm import Mapped, mapped_column + +from . import Base + + +class NoteUserState(Base): + """A recipient's own pin, archive and place for a note shared with them (#5176). + + The owner's live on the note itself. `position` null means "where the owner put + it", so a recipient who only pins a note doesn't freeze its order. `updated_at` + is when the recipient last changed any of it, which is what last-write-wins + compares between their devices. See `note_state`. + """ + + __tablename__ = "note_user_state" + + note_id: Mapped[uuid.UUID] = mapped_column( + UUID(as_uuid=True), ForeignKey("notes.id", ondelete="CASCADE"), primary_key=True + ) + user_id: Mapped[uuid.UUID] = mapped_column( + UUID(as_uuid=True), ForeignKey("users.id", ondelete="CASCADE"), primary_key=True + ) + pinned: Mapped[bool] = mapped_column(Boolean(), nullable=False, server_default=func.false()) + archived: Mapped[bool] = mapped_column(Boolean(), nullable=False, server_default=func.false()) + position: Mapped[int | None] = mapped_column(Integer(), nullable=True) + updated_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False, server_default=func.now()) + # Drawn by the row trigger (migration 0036), like notes and labels. + sync_revision: Mapped[int | None] = mapped_column(BigInteger(), nullable=True) diff --git a/src/inkwell/note_state.py b/src/inkwell/note_state.py new file mode 100644 index 0000000..fc4d6ad --- /dev/null +++ b/src/inkwell/note_state.py @@ -0,0 +1,100 @@ +"""How a note sits on one person's board: pinned, archived, and where (#5176). + +For the owner that is the note's own columns. For someone it is shared with it is +their `NoteUserState` row, so the owner pinning a shared shopping list doesn't +reorder anyone else's board, and a recipient archiving it doesn't hide it from the +owner. With no row, a recipient sees it unpinned, unarchived, in the owner's order. + +The SQL expressions read through an outer join on the viewer's row, which +`join_state` adds; every query that filters or orders by them needs it. +""" +from __future__ import annotations + +import uuid +from datetime import datetime + +from sqlalchemy import and_, case, false, func, select +from sqlalchemy.dialects.postgresql import insert + +from .models.note import Note +from .models.note_user_state import NoteUserState + +# The fields a recipient may set, which are exactly the ones this module holds. +OWN_FIELDS = ("pinned", "archived", "position") + + +def join_state(stmt, viewer: uuid.UUID): + """Outer-join the viewer's own row for each note, for the expressions below.""" + return stmt.outerjoin( + NoteUserState, and_(NoteUserState.note_id == Note.id, NoteUserState.user_id == viewer) + ) + + +def pinned_for(viewer: uuid.UUID): + return case((Note.owner_id == viewer, Note.pinned), else_=func.coalesce(NoteUserState.pinned, false())) + + +def archived_for(viewer: uuid.UUID): + return case((Note.owner_id == viewer, Note.archived), else_=func.coalesce(NoteUserState.archived, false())) + + +def position_for(viewer: uuid.UUID): + return case((Note.owner_id == viewer, Note.position), else_=func.coalesce(NoteUserState.position, Note.position)) + + +def revision_for(): + """The note's revision as the viewer's devices see it: an edit to the note, or a + change to their own state, whichever came later. An owner has no row, so theirs + is the note's own.""" + return func.greatest(Note.sync_revision, func.coalesce(NoteUserState.sync_revision, 0)) + + +async def states_for(db, note_ids: list, viewer: uuid.UUID) -> dict: + """note_id -> the viewer's own row, for the notes that have one.""" + if not note_ids: + return {} + rows = await db.scalars( + select(NoteUserState).where(NoteUserState.note_id.in_(note_ids), NoteUserState.user_id == viewer) + ) + return {s.note_id: s for s in rows.all()} + + +def own_view(note: Note, state: NoteUserState | None) -> dict: + """The serialized fields this module decides, for someone who doesn't own the note.""" + return { + "pinned": state.pinned if state is not None else False, + "archived": state.archived if state is not None else False, + "position": state.position if state is not None and state.position is not None else note.position, + } + + +async def set_own_state(db, note_id: uuid.UUID, user_id: uuid.UUID, changes: dict, at: datetime) -> bool: + """Apply a recipient's pin, archive or position, if `at` is no older than what + they last set from another device (last-write-wins, as for every other change). + Fields absent from `changes` keep their value. Returns whether it applied. + + The caller has already found the note visible to `user_id`: this writes the row + it is asked to, which is what lets one helper serve the web and sync alike.""" + values: dict = {} + if "pinned" in changes: + values["pinned"] = bool(changes["pinned"]) + if "archived" in changes: + values["archived"] = bool(changes["archived"]) + if isinstance(changes.get("position"), int) and not isinstance(changes["position"], bool): + values["position"] = changes["position"] + existing = await db.scalar( + select(NoteUserState.updated_at).where(NoteUserState.note_id == note_id, NoteUserState.user_id == user_id) + ) + if existing is not None and at < existing: + return False + if not values: + return True + await db.execute( + insert(NoteUserState) + .values(note_id=note_id, user_id=user_id, updated_at=at, **values) + .on_conflict_do_update( + index_elements=[NoteUserState.note_id, NoteUserState.user_id], + set_={**values, "updated_at": at}, + ) + ) + return True diff --git a/src/inkwell/notes/__init__.py b/src/inkwell/notes/__init__.py index 1331a12..807e269 100644 --- a/src/inkwell/notes/__init__.py +++ b/src/inkwell/notes/__init__.py @@ -31,6 +31,7 @@ from ..models.note import Note from ..models.note_attachment import NoteAttachment from ..models.note_link_preview import NoteLinkPreview from ..models.note_revision import NoteRevision +from ..note_state import OWN_FIELDS, archived_for, join_state, pinned_for, position_for, set_own_state from .checklist import ( append_item, parse_items, @@ -51,6 +52,7 @@ from .helpers import ( _attachment_ext, _get_editable, _get_owned, + _get_visible, _header_filename, _safe_filename, _slugify, @@ -124,8 +126,12 @@ async def list_notes(): if shared not in (None, "", "with_me"): return json_error("invalid shared", 400) async with session_scope() as db: - stmt = select(Note).where(visible_to_user("note", Note.owner_id, Note.id, g.user_id)) - stmt = apply_filter(stmt, filter_name) + # 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( + 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. if filter_name == "trash": @@ -164,7 +170,9 @@ async def list_notes(): elif sort == "created": stmt = stmt.order_by(Note.created_at.desc()) else: - stmt = stmt.order_by(Note.pinned.desc(), Note.position.desc(), Note.updated_at.desc()) + stmt = stmt.order_by( + pinned_for(g.user_id).desc(), position_for(g.user_id).desc(), Note.updated_at.desc() + ) notes = (await db.scalars(stmt)).all() return jsonify({"notes": await _serialize_notes(db, notes, g.user_id)}) @@ -381,15 +389,27 @@ async def reorder_notes(): return json_error("invalid id", 400) parsed.append(parsed_id) async with session_scope() as db: - owned = { + visible = { n.id: n - for n in (await db.scalars(select(Note).where(Note.owner_id == g.user_id, Note.id.in_(parsed)))).all() + for n in ( + await db.scalars( + select(Note).where( + Note.id.in_(parsed), visible_to_user("note", Note.owner_id, Note.id, g.user_id) + ) + ) + ).all() } + now = datetime.now(timezone.utc) total = len(parsed) for index, nid in enumerate(parsed): - note = owned.get(nid) - if note is not None: + note = visible.get(nid) + if note is None: + continue + # A note shared with the caller moves on their board only (#5176). + if note.owner_id == g.user_id: note.position = total - index + else: + await set_own_state(db, nid, g.user_id, {"position": total - index}, now) await db.commit() return jsonify({"ok": True}) @@ -441,19 +461,28 @@ async def get_note(note_id: str): async def update_note(note_id: str): data = await request.get_json(silent=True) or {} async with session_scope() as db: - note = await _get_editable(db, note_id) + note = await _get_visible(db, note_id) if note is None: return not_found() - # Someone the note is shared with at `edit` may change its text and nothing - # else. They can already read it, so saying so is no oracle. - if note.owner_id != g.user_id and set(data) - {"body"}: - return json_error("only the owner can change that", 403) + owned = note.owner_id == g.user_id + if not owned: + # Someone the note is shared with may pin and archive it on their own + # board (#5176), and change its text if it was shared at `edit`; nothing + # else. They can already read it, so saying so is no oracle. Text from a + # view share 404s, as it did before they could do anything at all. + if set(data) - {"body", *OWN_FIELDS}: + return json_error("only the owner can change that", 403) + if "body" in data and await _get_editable(db, note_id) is None: + return not_found() changed = False if isinstance(data.get("body"), str): # Version history: the first write of an editing session snapshots, not # every write — see revisions.should_snapshot. Writing often is what lets # a client autosave instead of hoarding text until it closes. changed = await write_body(db, note, data["body"]) + if not owned: + await set_own_state(db, note.id, g.user_id, data, datetime.now(timezone.utc)) + return await _commit_note(db, note, changed) if "pinned" in data: note.pinned = bool(data["pinned"]) if "archived" in data: diff --git a/src/inkwell/notes/helpers.py b/src/inkwell/notes/helpers.py index 0329c31..712c5f9 100644 --- a/src/inkwell/notes/helpers.py +++ b/src/inkwell/notes/helpers.py @@ -70,25 +70,28 @@ def parse_list_items(raw: object) -> list[str]: return [s.strip() for s in raw if isinstance(s, str) and s.strip()] -def apply_filter(stmt, filter_name: str): +def apply_filter(stmt, filter_name: str, archived): """Narrow a notes query to one board view. + `archived` is the viewer's archive flag (`note_state.archived_for`): a note + someone shared with you is archived on your board when YOU archived it (#5176). + Every branch excludes purge tombstones — content-less rows kept only so the sync feed can tell offline clients a note is gone (see `retention.purge_note`). The active/archived branches get that for free from `deleted_at IS NULL`, since a tombstone keeps the timestamp; Trash is the one view that has to say so. """ if filter_name == "archived": - return stmt.where(Note.deleted_at.is_(None), Note.archived.is_(True)) + return stmt.where(Note.deleted_at.is_(None), archived.is_(True)) if filter_name == "trash": return stmt.where(Note.deleted_at.is_not(None), Note.purged_at.is_(None)) - return stmt.where(Note.deleted_at.is_(None), Note.archived.is_(False)) + return stmt.where(Note.deleted_at.is_(None), archived.is_(False)) 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, pin, archive, attachments, previews, history and - sharing (#5174). + trash, delete, labels, reminders, attachments, previews, history and sharing + (#5174). Pin and archive are everyone's own (#5176). Someone the note is shared with gets None here, and so a 404, even when they can read the note: a 403 would confirm the note exists to a caller who only has its @@ -106,6 +109,22 @@ async def _get_owned(db, note_id: str) -> Note | 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) + 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 diff --git a/src/inkwell/notes/serialize.py b/src/inkwell/notes/serialize.py index c5cbfbe..4764033 100644 --- a/src/inkwell/notes/serialize.py +++ b/src/inkwell/notes/serialize.py @@ -11,6 +11,7 @@ from ..models.note import Note from ..models.note_attachment import NoteAttachment from ..models.note_link_preview import NoteLinkPreview from ..models.user import User +from ..note_state import own_view, states_for from .checklist import parse_items @@ -141,12 +142,15 @@ async def _serialize_notes(db, notes: list, viewer=None) -> list: """The API shape of each note. With `viewer`, each also says how the viewer holds it, and a note someone shared with them comes without the owner's labels: labels are personal, and a recipient's board files nothing under someone else's tags. - Sync passes no viewer, since its feed is the caller's own notes.""" + For the same reason its pin, archive and position are the viewer's own (#5176). + Sync passes no viewer to a client that asked only for its own notes.""" ids = [n.id for n in notes] labels_map = await _labels_for_notes(db, ids) attach_map = await _attachments_for_notes(db, ids) preview_map = await _previews_for_notes(db, ids) sharing = await _sharing_for_notes(db, notes, viewer) if viewer is not None else {} + foreign = [n.id for n in notes if viewer is not None and n.owner_id != viewer] + own_states = await states_for(db, foreign, viewer) out = [] for n in notes: data = n.serialize() @@ -157,5 +161,7 @@ async def _serialize_notes(db, notes: list, viewer=None) -> list: data["previews"] = preview_map.get(n.id, []) if viewer is not None: data.update(sharing[n.id]) + if not mine: + data.update(own_view(n, own_states.get(n.id))) out.append(data) return out diff --git a/src/inkwell/sync.py b/src/inkwell/sync.py index b590874..b2600ea 100644 --- a/src/inkwell/sync.py +++ b/src/inkwell/sync.py @@ -31,13 +31,14 @@ from .models.note import Note from .models.note_attachment import NoteAttachment from .models.note_link_preview import NoteLinkPreview from .models.share_revocation import ShareRevocation +from .note_state import OWN_FIELDS, join_state, revision_for, set_own_state from .notes import ( _serialize_notes, normalize_color, normalize_recurrence, ) from .notes.body import write_body -from .notes.helpers import _get_editable, store_attachment, unlink_media +from .notes.helpers import _get_editable, _get_visible, store_attachment, unlink_media from .responses import json_error, not_found, parse_uuid from .settings import get_setting from .retention import purge_note @@ -86,7 +87,10 @@ MAX_PUSH = 1000 # per-batch change cap # `revoked` list of notes that have left it; push takes body edits to notes shared at # `edit`. Additive and opt-in, so the floor stays: a v5 client never asks, and gets # its own notes exactly as before. -SYNC_PROTOCOL_VERSION = 6 +# v7 (#5176): a recipient's own pin, archive and place. On a shared note the feed +# carries the caller's own, and push takes them, stamped `state_at`, for a note shared +# at any level. Additive, so the floor stays: a v6 client pushes only text, as before. +SYNC_PROTOCOL_VERSION = 7 MIN_CLIENT_PROTOCOL_VERSION = 3 # Named capabilities beyond the base protocol. An ADDITIVE change earns a name @@ -102,6 +106,7 @@ SYNC_FEATURES: tuple[str, ...] = ( "revisions", # an overwritten version snapshots into note history "attachment_sync", # upload by client id + attachment/preview deletes in push (v5) "shares", # notes shared with the caller in the feed, `revoked`, edit-share pushes (v6) + "shared_state", # a recipient's own pin, archive and position on a shared note (v7) ) @@ -166,10 +171,16 @@ async def changes(): visible = Note.owner_id == g.user_id async with session_scope() as db: # Every note the caller can see, in any state (active/archived/trash/purged), - # since a client mirrors everything; ordered by the shared revision. + # since a client mirrors everything; ordered by the shared revision. On a note + # shared with the caller, pinning it moves their own state row and not the + # note, so its revision here is whichever of the two moved last (#5176). + revision = revision_for() note_rows = ( - await db.scalars( - select(Note).where(visible, Note.sync_revision > since).order_by(Note.sync_revision).limit(limit) + await db.execute( + join_state(select(Note, revision), g.user_id) + .where(visible, revision > since) + .order_by(revision) + .limit(limit) ) ).all() label_rows = ( @@ -195,7 +206,7 @@ async def changes(): cursor, has_more = _page_cursor( [ - [n.sync_revision for n in note_rows], + [rev for _, rev in note_rows], [lb.sync_revision for lb in label_rows], [rev for _, rev in revoked_rows], ], @@ -203,16 +214,16 @@ async def changes(): limit, ) # Trim each stream to the shared watermark so the feeds stay aligned. - note_rows = [n for n in note_rows if n.sync_revision <= cursor] + note_rows = [(n, rev) for n, rev in note_rows if rev <= cursor] label_rows = [lb for lb in label_rows if lb.sync_revision <= cursor] revoked = [str(nid) for nid, rev in revoked_rows if rev <= cursor] # Note bodies come from the shared note serializer; sync adds the two # delta-only fields on top. With shares, each note also says how the caller # holds it, and a note shared with them comes without the owner's labels. - notes_out = await _serialize_notes(db, note_rows, g.user_id if with_shares else None) - for data, n in zip(notes_out, note_rows): - data["sync_revision"] = n.sync_revision + notes_out = await _serialize_notes(db, [n for n, _ in note_rows], g.user_id if with_shares else None) + for data, (n, rev) in zip(notes_out, note_rows): + data["sync_revision"] = rev data["purged_at"] = iso(n.purged_at) page = { @@ -346,32 +357,54 @@ async def _apply_note(db, ch: dict, previews: list[tuple[uuid.UUID, str]]) -> di async def _apply_shared_note(db, note: Note, ch: dict, edited_at, previews: list[tuple[uuid.UUID, str]]) -> dict: - """Apply a pushed change to a note someone else owns (#5175). + """Apply a pushed change to a note someone else owns (#5175, #5176). - Someone it is shared with at `edit` may change its text, exactly as in the web - app: the body goes through `write_body` under the same last-write-wins, and every - other field in the change is ignored, since pin, archive, reminders, labels and - deletion are the owner's. Anything else is rejected. A note the caller cannot see - at all gets the same generic answer as an id that doesn't exist, so the reply - can't be used to learn that someone else's id is real. + Two separate things can arrive, each under its own last-write-wins: + + - Its text, from someone it is shared with at `edit`, exactly as in the web app: + the body goes through `write_body`, compared on `edited_at` against the note. + - Their own pin, archive and position, from anyone it is shared with, compared on + `state_at` against what they last set. A client stamps that apart from the + text's time so that pinning a note never makes a stale copy of its text look + newer than the owner's edit. + + Trash, reminders, labels and deletion are the owner's; anything asking for them + is rejected. A note the caller cannot see at all gets the same generic answer as + an id that doesn't exist, so the reply can't be used to learn that someone else's + id is real. """ nid = str(note.id) - if ch.get("op", "upsert") == "upsert" and await _get_editable(db, nid) is not None: - if not client_wins(edited_at, note.updated_at): - return {"id": nid, "entity": "note", "status": "kept", "sync_revision": note.sync_revision} - body = ch["body"] if isinstance(ch.get("body"), str) else note.body - if await write_body(db, note, body): - previews.append((note.id, note.body)) - if edited_at is not None: - note.updated_at = edited_at - await db.flush() - await db.refresh(note, ["sync_revision"]) - return {"id": nid, "entity": "note", "status": "applied", "sync_revision": note.sync_revision} - visible = await db.scalar( - select(Note.id).where(Note.id == note.id, visible_to_user("note", Note.owner_id, Note.id, g.user_id)) + if await _get_visible(db, nid) is None: + return {"id": nid, "entity": "note", "status": "rejected", "error": "cannot apply"} + owner_only = {"id": nid, "entity": "note", "status": "rejected", "error": "only its owner can change that"} + if ch.get("op", "upsert") != "upsert": + return owner_only + applied = kept = False + if isinstance(ch.get("body"), str): + if await _get_editable(db, nid) is None: + return owner_only + if client_wins(edited_at, note.updated_at): + applied = True + if await write_body(db, note, ch["body"]): + previews.append((note.id, note.body)) + # Only when the text changed: an unchanged body pushed beside a pin + # is not an edit, and must not move the note on for its owner. + if edited_at is not None: + note.updated_at = edited_at + else: + kept = True + state_at = parse_dt(ch.get("state_at")) + if state_at is not None: + if await set_own_state(db, note.id, g.user_id, {k: ch[k] for k in OWN_FIELDS if k in ch}, state_at): + applied = True + else: + kept = True + await db.flush() + revision = await db.scalar( + join_state(select(revision_for()).select_from(Note), g.user_id).where(Note.id == note.id) ) - error = "only its owner can change that" if visible is not None else "cannot apply" - return {"id": nid, "entity": "note", "status": "rejected", "error": error} + status = "kept" if kept and not applied else "applied" + return {"id": nid, "entity": "note", "status": status, "sync_revision": revision} async def _apply_label(db, ch: dict) -> dict: diff --git a/tests/test_integration.py b/tests/test_integration.py index 72215ed..b79677c 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -51,7 +51,7 @@ pytestmark = pytest.mark.integration # Every table the tests touch, child-first so FKs never block the truncate. # RESTART IDENTITY + CASCADE keeps this honest if a table gains children later. -_TABLES = "notes, note_revisions, note_labels, note_link_previews, labels, shares, share_revocations, invites, password_resets, users" +_TABLES = "notes, note_revisions, note_labels, note_link_previews, labels, shares, share_revocations, note_user_state, invites, password_resets, users" @pytest_asyncio.fixture @@ -1292,9 +1292,9 @@ async def test_a_view_share_writes_nothing_and_an_edit_share_writes_only_text(ap assert "fromguest" in owner_tags assert (await (await recipient.get("/api/labels")).get_json())["labels"] == [] - # Everything else stays the owner's. - assert (await recipient.patch(f"/api/notes/{nid}", json={"pinned": True})).status_code == 403 - assert (await recipient.patch(f"/api/notes/{nid}", json={"body": "x", "archived": True})).status_code == 403 + # Everything else but their own pin and archive (#5176) stays the owner's. + assert (await recipient.patch(f"/api/notes/{nid}", json={"remind_at": "2099-01-01T00:00:00Z"})).status_code == 403 + assert (await recipient.patch(f"/api/notes/{nid}", json={"body": "x", "recurrence": "daily"})).status_code == 403 assert (await recipient.post(f"/api/notes/{nid}/trash")).status_code == 404 assert (await recipient.get(f"/api/notes/{nid}/revisions")).status_code == 404 @@ -1303,6 +1303,41 @@ async def test_a_view_share_writes_nothing_and_an_edit_share_writes_only_text(ap assert "- [x] milk" in body and "eggs" in body +async def test_owner_and_recipient_each_pin_archive_and_order_their_own(app_client, db): + recipient, stranger, people = await _three_people(app_client) + older = await _owners_note(app_client, "older") + nid = await _owners_note(app_client, "newer") + for note_id in (older, nid): + await _share(app_client, note_id, people["recipient"], "view") + + async def pinned(client) -> bool: + return (await (await client.get(f"/api/notes/{nid}")).get_json())["pinned"] + + # A view share is enough: pinning is organizing your own board, not changing the note. + mine = await recipient.patch(f"/api/notes/{nid}", json={"pinned": True}) + assert mine.status_code == 200, await mine.get_data(as_text=True) + assert (await mine.get_json())["pinned"] is True + assert (await pinned(recipient), await pinned(app_client)) == (True, False) + await app_client.patch(f"/api/notes/{nid}", json={"pinned": True}) + await recipient.patch(f"/api/notes/{nid}", json={"pinned": False}) + assert (await pinned(recipient), await pinned(app_client)) == (False, True) + assert (await stranger.patch(f"/api/notes/{nid}", json={"pinned": True})).status_code == 404 + + # Archived by the recipient, it leaves their board and never the owner's. + await recipient.patch(f"/api/notes/{nid}", json={"archived": True}) + assert await _board_ids(recipient) == [older] + assert await _board_ids(recipient, "filter=archived") == [nid] + assert nid in await _board_ids(app_client) + await recipient.patch(f"/api/notes/{nid}", json={"archived": False}) + + # Until they move them, a recipient sees the owner's order; after, their own. + await app_client.patch(f"/api/notes/{nid}", json={"pinned": False}) + assert await _board_ids(recipient) == [nid, older] + assert (await recipient.post("/api/notes/reorder", json={"ids": [older, nid]})).status_code == 200 + assert await _board_ids(recipient) == [older, nid] + assert await _board_ids(app_client) == [nid, older] + + async def test_unsharing_trashing_and_deleting_each_end_access(app_client, db): recipient, _, people = await _three_people(app_client) nid = await _owners_note(app_client) @@ -1420,6 +1455,45 @@ async def test_an_edit_share_pushes_text_and_nothing_else(app_client, db): assert (await app_client.get(f"/api/notes/{nid}")).status_code == 200 +async def test_a_recipients_own_state_syncs_to_their_devices_only(app_client, db): + recipient, _, people = await _three_people(app_client) + nid = await _owners_note(app_client, "first line") + await _share(app_client, nid, people["recipient"], "edit") + start = (await _feed(recipient))["cursor"] + + async def push(change: dict) -> dict: + payload = {"changes": [{"entity": "note", "id": nid, "op": "upsert", **change}]} + return (await (await recipient.post("/api/sync/push", json=payload)).get_json())["results"][0] + + # The owner edits; the recipient's phone, a version behind, pins its stale copy. + await app_client.patch(f"/api/notes/{nid}", json={"body": "first line, the owner's edit"}) + owners = (await _feed(app_client))["cursor"] + pinned = await push( + { + "edited_at": "2000-01-01T00:00:00Z", + "body": "first line", + "pinned": True, + "archived": False, + "position": 7, + "state_at": "2099-01-01T00:00:00Z", + } + ) + assert pinned["status"] == "applied", pinned + # The pin's newer time is the state's alone, so the stale text lost to the edit. + assert (await (await app_client.get(f"/api/notes/{nid}")).get_json())["body"] == "first line, the owner's edit" + + held = _feed_note(await _feed(recipient, start), nid) + assert (held["pinned"], held["position"], held["sync_revision"]) == (True, 7, pinned["sync_revision"]) + # Nothing about the note changed for its owner, so their devices aren't sent it. + assert _feed_note(await _feed(app_client, owners), nid) is None + assert _feed_note(await _feed(app_client), nid)["pinned"] is False + + # Another device's older unpin loses to the newer pin. + stale = await push({"edited_at": "2000-01-01T00:00:00Z", "pinned": False, "state_at": "2098-01-01T00:00:00Z"}) + assert stale["status"] == "kept" + assert (await (await recipient.get(f"/api/notes/{nid}")).get_json())["pinned"] is True + + async def test_deleting_a_shared_note_reaches_the_recipients_devices(app_client, db): recipient, _, people = await _three_people(app_client) nid = await _owners_note(app_client)