Files
FabledScribe/tests/test_shared_write_access.py
T
bvandeusen cca40affe4
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 12s
CI & Build / integration (push) Successful in 25s
CI & Build / Python tests (push) Failing after 29s
CI & Build / Build & push image (push) Has been skipped
feat(snippets): add notes.data JSONB — the indexed mirror of snippet fields
Milestone #232 step 1 (task #2081). Takes the enabler first rather than the
write-path trigger: reverse lookup, drift checks and the duplicate finder all
need to QUERY structured fields, and building them on body-regex first means
writing them twice.

#227 deferred this bag "unless body-convention ergonomics prove insufficient" —
answering "which snippets live in this file?" by scanning every snippet and
regexing its body is that condition being met.

Migration 0070 adds `notes.data` (nullable JSONB) + a GIN index. The body is
UNCHANGED and still what gets embedded and read by humans; `data` mirrors the
same facts in a shape Postgres can index. Code is deliberately not copied into
it — the body holds it, and duplicating a blob into the column we index around
would be waste.

- compose_data() builds the mirror, omitting empties so the column stays sparse
- snippet_fields() prefers `data`, falling back to parsing the body. Rows written
  before 0070 have no `data` and are never backfilled, so a hand-edited body
  stays authoritative for them with no conversion deadline
- create / update / merge all write body and mirror from the same merged field
  set, so the two can't drift; merge in particular has to grow the mirror with
  the survivor's location set or a merged snippet would be unfindable at the very
  call sites the merge just recorded

Named `data`, not `metadata`, because that collides with SQLAlchemy's declarative
Base.metadata — which is why the pre-0069 model had to map an awkward
`entity_metadata` attribute. Not a revival of the column 0069 dropped: different
name, different purpose, nothing reads the old shape.

Two test fakes needed an explicit `data = None`: snippet_fields prefers `data`
when truthy and an auto-MagicMock attribute is truthy, so every parsed field
would have come back a MagicMock. Checked every fake reaching snippet code this
time rather than waiting for CI (note 2109).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLwAaV4DQEmVyn496HnEvt
2026-07-26 18:25:55 -04:00

115 lines
5.0 KiB
Python

"""Editing a shared record: an editor grant is enough, a viewer grant is not.
The agent and web paths used to disagree — and worse, within the agent path
`delete_snippet` honoured editor shares while `update_snippet` did not, so a
colleague's snippet could be destroyed through an agent but not improved. Both
now resolve the read scope and then require WRITE, which is what the sharing UI
already promises: viewer / editor / admin are distinct grants.
A record readable-but-not-writable raises PermissionError rather than reporting
"not found" — the caller can plainly open it, so not-found would be a lie that
sends them hunting for a missing id.
"""
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
def _snippet(id=1, owner=9):
n = MagicMock()
n.id = id
n.user_id = owner
n.title = "formatDuration — humanize a millisecond count"
n.body = "```ts\nexport const f = 1\n```\n"
n.tags = ["ts", "snippet"]
n.note_type = "snippet"
n.deleted_at = None
# Explicitly None — snippet_fields prefers `data` when truthy, and an
# auto-MagicMock attribute is truthy (see note 2109).
n.data = None
return n
@pytest.mark.asyncio
async def test_editor_share_can_update_and_writes_as_the_owner():
"""The underlying note update is owner-scoped, so a shared editor's own id
would match nothing — the authorised write has to be performed as the owner."""
from scribe.services import snippets as svc
note = _snippet(owner=9)
updated = _snippet(owner=9)
with patch.object(svc, "get_snippet", AsyncMock(return_value=note)), \
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \
patch.object(svc.notes_svc, "update_note",
AsyncMock(return_value=updated)) as mock_update, \
patch.object(svc, "_embed_snippet", MagicMock()):
got = await svc.update_snippet(7, 1, name="formatDuration")
assert got is updated
# Positional user_id is the OWNER (9), not the caller (7).
assert mock_update.await_args.args[0] == 9
@pytest.mark.asyncio
async def test_viewer_share_cannot_update():
from scribe.services import snippets as svc
with patch.object(svc, "get_snippet", AsyncMock(return_value=_snippet())), \
patch("scribe.services.access.can_write_note", AsyncMock(return_value=False)), \
patch.object(svc.notes_svc, "update_note", AsyncMock()) as mock_update:
with pytest.raises(PermissionError, match="read-only"):
await svc.update_snippet(7, 1, name="nope")
mock_update.assert_not_awaited()
@pytest.mark.asyncio
async def test_viewer_share_cannot_delete():
"""delete_snippet returns False rather than raising — its contract is boolean
— but a viewer must not be able to bin someone else's record."""
from scribe.services import snippets as svc
with patch.object(svc, "get_snippet", AsyncMock(return_value=_snippet())), \
patch("scribe.services.access.can_write_note", AsyncMock(return_value=False)), \
patch("scribe.services.trash.delete", AsyncMock()) as mock_trash:
assert await svc.delete_snippet(7, 1) is False
mock_trash.assert_not_awaited()
@pytest.mark.asyncio
async def test_unreadable_record_is_not_found_not_forbidden():
"""A record the caller can't see at all must NOT be distinguishable from one
that doesn't exist — otherwise the error itself confirms it exists."""
from scribe.services import snippets as svc
with patch.object(svc, "get_snippet", AsyncMock(return_value=None)):
assert await svc.update_snippet(7, 404, name="x") is None
assert await svc.delete_snippet(7, 404) is False
@pytest.mark.asyncio
async def test_merge_requires_write_on_target():
from scribe.services import snippets as svc
with patch.object(svc, "get_snippet", AsyncMock(return_value=_snippet())), \
patch("scribe.services.access.can_write_note", AsyncMock(return_value=False)):
with pytest.raises(PermissionError, match="read-only"):
await svc.merge_snippets(7, 1, [2])
@pytest.mark.asyncio
async def test_merge_skips_sources_owned_by_someone_else():
"""Cross-owner merge stays out of scope (#231): a source belonging to a
different owner than the target is skipped, not silently folded in and
trashed."""
from scribe.services import snippets as svc
target = _snippet(id=1, owner=9)
same_owner = _snippet(id=2, owner=9)
other_owner = _snippet(id=3, owner=42)
async def fake_get(_uid, sid):
return {1: target, 2: same_owner, 3: other_owner}.get(sid)
with patch.object(svc, "get_snippet", AsyncMock(side_effect=fake_get)), \
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), \
patch.object(svc.notes_svc, "update_note", AsyncMock(return_value=target)), \
patch("scribe.services.trash.delete", AsyncMock(return_value=object())), \
patch.object(svc, "_embed_snippet", MagicMock()):
_note, merged_ids = await svc.merge_snippets(7, 1, [2, 3])
assert merged_ids == [2]