CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / Python tests (push) Successful in 9s
CI & Build / integration (push) Successful in 14s
CI & Build / Build & push image (push) Successful in 37s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m6s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m16s
Desktop (Tauri) / Update manifest (push) Successful in 6s
Operator: *"I'd like for URLs to unfurl. To be the whole note when the note is a single URL, and to be a compact slot on the bottom of the note when the URL is inline. We also need to support multiple URLs in a single note."* Less new machinery than it sounds: `unfurl.py` already fetched and parsed OG tags, SSRF-hardened, and `note_link_previews` was already `UNIQUE(note_id, url)` — so several URLs per note has worked at the storage layer all along. What was missing was that it needed a button, had one size, and drew that size in the wrong place. **Automatic, and never in the way.** New `unfurl_queue.py` detects a body's URLs and fetches them on a background task AFTER the note is committed. Capture speed is the product: an unfurl is a five-second timeout against a host nobody controls, and a note has to persist the instant someone stops typing. Scheduled from create, from a body edit, and from a synced push — so a linked desktop or Android client gets previews too, on its next pull. An unlinked one has no server to ask and simply has none, which is the honest consequence of being offline. Safe to call on every save: it re-reads what's cached and does nothing when nothing is new. Capped at five URLs per note, silent on every failure (a link that won't fetch isn't an error the person needs — the note is fine, the link is still there), and it re-checks before storing, so a slow fetch can't resurrect a preview for a URL that was deleted while it was in flight. **Two presentations.** A note whose body is nothing but a URL renders as its preview and nothing else — printing the raw URL under a card that already says where it goes is saying the same thing twice, badly. Until the fetch lands, or if it never does, the URL stands in, so the card is never blank. Anything else gets a compact strip. **And the strip moved.** Previews were rendered ABOVE the body, which put a stranger's headline where the note's own first line should be — worse now that the first line IS the note's name. They sit at the foot of the card now, under the note's own words. The editor's "Preview example.com" button is gone with the manual path; removing an unwanted preview stays, and stays editor-only. Nine tests: three on detection (order, dedupe, sentence-punctuation trimming, non-http rejection) in the unit lane, and three in the integration lane for what only a real database shows — the upsert landing on the right row, a second pass fetching nothing, and a preview NOT being stored for a URL that left the body.
284 lines
11 KiB
Python
284 lines
11 KiB
Python
"""The real-Postgres lane (family rule 6).
|
|
|
|
Everything else in this suite is deliberately DB-free, which means the schema the
|
|
migrations build has never been checked against the models that read it. That gap is
|
|
what this file closes, and it is not theoretical: M13 dropped three columns and
|
|
rebuilt a generated column, and until now `alembic upgrade head` ran for the first
|
|
time when the operator's container started.
|
|
|
|
Marked `integration` and excluded from the unit lane by `-m "not integration"`, so a
|
|
workstation without Postgres runs the rest of the suite unchanged.
|
|
|
|
The schema comes from real migrations, never `metadata.create_all` (rule 82) — the
|
|
point is to test what actually ships, and `create_all` would build a schema no
|
|
deployment has ever seen.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import uuid
|
|
|
|
import pytest
|
|
import pytest_asyncio
|
|
from sqlalchemy import select, text
|
|
|
|
from thoughtsync.db import dispose_engine, session_scope
|
|
from thoughtsync.models.note import Note
|
|
from thoughtsync.models.note_item import NoteItem
|
|
from thoughtsync.models.user import User
|
|
from thoughtsync.notes.helpers import derive_display_title
|
|
from thoughtsync.models.note_link_preview import NoteLinkPreview
|
|
from thoughtsync.sync import _apply_note_items
|
|
from thoughtsync.unfurl_queue import _unfurl_new_urls, detect_urls
|
|
|
|
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_items, note_revisions, note_labels, note_link_previews, labels, users"
|
|
|
|
|
|
@pytest_asyncio.fixture
|
|
async def db():
|
|
"""A session against the migrated database, wiped before each test.
|
|
|
|
Wiped BEFORE rather than after so a failed test leaves its rows behind to look at.
|
|
"""
|
|
async with session_scope() as session:
|
|
await session.execute(text(f"TRUNCATE {_TABLES} RESTART IDENTITY CASCADE"))
|
|
await session.commit()
|
|
yield session
|
|
await dispose_engine()
|
|
|
|
|
|
@pytest_asyncio.fixture
|
|
async def owner(db):
|
|
"""A user to hang notes off — `notes.owner_id` is a real foreign key."""
|
|
user = User(email=f"{uuid.uuid4().hex}@example.test", display_name="Integration")
|
|
db.add(user)
|
|
await db.commit()
|
|
await db.refresh(user)
|
|
return user
|
|
|
|
|
|
async def test_the_migrated_schema_matches_the_models(db, owner):
|
|
"""The check that has never run: insert through the ORM, read it back.
|
|
|
|
A column the models expect and the migrations never created — or the reverse —
|
|
fails right here, instead of when a container starts.
|
|
"""
|
|
note = Note(owner_id=owner.id, body="a thought", display_title="a thought")
|
|
db.add(note)
|
|
await db.commit()
|
|
await db.refresh(note)
|
|
|
|
found = await db.scalar(select(Note).where(Note.id == note.id))
|
|
assert found is not None
|
|
assert found.body == "a thought"
|
|
assert found.display_title == "a thought"
|
|
|
|
|
|
async def test_the_dropped_columns_are_actually_gone(db):
|
|
"""M13 dropped three. If a migration silently no-opped, this is where it shows."""
|
|
cols = set(
|
|
(
|
|
await db.execute(
|
|
text("SELECT column_name FROM information_schema.columns WHERE table_name = 'notes'")
|
|
)
|
|
)
|
|
.scalars()
|
|
.all()
|
|
)
|
|
assert "title" not in cols, "notes.title should have gone in 0026"
|
|
assert "kind" not in cols, "notes.kind should have gone in 0025"
|
|
assert "display_title" in cols and "body" in cols
|
|
|
|
rev_cols = set(
|
|
(
|
|
await db.execute(
|
|
text("SELECT column_name FROM information_schema.columns WHERE table_name = 'note_revisions'")
|
|
)
|
|
)
|
|
.scalars()
|
|
.all()
|
|
)
|
|
assert "title" not in rev_cols, "note_revisions.title should have gone in 0026"
|
|
|
|
tables = set(
|
|
(await db.execute(text("SELECT table_name FROM information_schema.tables WHERE table_schema = 'public'")))
|
|
.scalars()
|
|
.all()
|
|
)
|
|
assert "note_links" not in tables, "note_links should have gone in 0024"
|
|
|
|
|
|
async def test_the_search_vector_was_rebuilt_over_the_name(db, owner):
|
|
"""0026 had to drop and recreate a STORED GENERATED column.
|
|
|
|
Postgres refuses to drop a column another generated column depends on, so getting
|
|
this wrong doesn't produce a subtly wrong ranking — it produces a migration that
|
|
won't run at all. Worth proving the replacement actually indexes something.
|
|
"""
|
|
note = Note(owner_id=owner.id, body="ferry tickets\nbook before friday", display_title="ferry tickets")
|
|
db.add(note)
|
|
await db.commit()
|
|
|
|
hit = await db.scalar(
|
|
text(
|
|
"SELECT count(*) FROM notes "
|
|
"WHERE search_vector @@ websearch_to_tsquery('english', :q)"
|
|
).bindparams(q="ferry")
|
|
)
|
|
assert hit == 1
|
|
|
|
# The NAME is weight A and the body weight B, which is what makes a name match
|
|
# rank above a body-only one. Both must be in the vector at all.
|
|
body_only = await db.scalar(
|
|
text(
|
|
"SELECT count(*) FROM notes "
|
|
"WHERE search_vector @@ websearch_to_tsquery('english', :q)"
|
|
).bindparams(q="friday")
|
|
)
|
|
assert body_only == 1
|
|
|
|
|
|
async def test_a_note_keeps_both_its_body_and_its_items(db, owner):
|
|
"""The shape M13 step 2 made normal: a note HAS a checklist, it isn't one."""
|
|
note = Note(owner_id=owner.id, body="weekend shop", display_title="weekend shop")
|
|
db.add(note)
|
|
await db.flush()
|
|
db.add_all(
|
|
[
|
|
NoteItem(note_id=note.id, text="milk", position=0),
|
|
NoteItem(note_id=note.id, text="eggs", position=1),
|
|
]
|
|
)
|
|
await db.commit()
|
|
|
|
items = (
|
|
await db.scalars(select(NoteItem).where(NoteItem.note_id == note.id).order_by(NoteItem.position))
|
|
).all()
|
|
assert [i.text for i in items] == ["milk", "eggs"]
|
|
assert (await db.scalar(select(Note.body).where(Note.id == note.id))) == "weekend shop"
|
|
|
|
|
|
async def test_sync_no_longer_deletes_items_from_a_note_with_a_body(db, owner):
|
|
"""The data-loss path step 2 removed, pinned against a real database.
|
|
|
|
`_apply_note_items` used to delete every item when the note wasn't `kind = "list"`.
|
|
Nothing can produce that state any more, but this is the regression that would
|
|
have silently eaten a checklist, and it deserves a test that would catch its
|
|
return.
|
|
"""
|
|
note = Note(owner_id=owner.id, body="packing", display_title="packing")
|
|
db.add(note)
|
|
await db.flush()
|
|
db.add(NoteItem(note_id=note.id, text="socks", position=0))
|
|
await db.commit()
|
|
|
|
# A change that says nothing about items must LEAVE them alone — absent means
|
|
# "not telling us", not "empty".
|
|
await _apply_note_items(db, note, {"body": "packing"})
|
|
await db.commit()
|
|
assert (await db.scalar(select(NoteItem.text).where(NoteItem.note_id == note.id))) == "socks"
|
|
|
|
# An explicit list replaces them.
|
|
await _apply_note_items(db, note, {"items": [{"text": "charger", "checked": True}]})
|
|
await db.commit()
|
|
rows = (await db.scalars(select(NoteItem).where(NoteItem.note_id == note.id))).all()
|
|
assert [(r.text, r.checked) for r in rows] == [("charger", True)]
|
|
|
|
|
|
async def test_a_note_with_only_items_still_has_a_name(db, owner):
|
|
"""The hole that made removing the title unsafe until step 2 closed it."""
|
|
note = Note(owner_id=owner.id, body="", display_title="")
|
|
db.add(note)
|
|
await db.flush()
|
|
db.add(NoteItem(note_id=note.id, text="milk", position=0))
|
|
await db.commit()
|
|
|
|
first = await db.scalar(
|
|
select(NoteItem.text).where(NoteItem.note_id == note.id).order_by(NoteItem.position).limit(1)
|
|
)
|
|
note.display_title = derive_display_title(note.body, first)
|
|
await db.commit()
|
|
|
|
assert (await db.scalar(select(Note.display_title).where(Note.id == note.id))) == "milk"
|
|
|
|
|
|
async def test_auto_unfurl_stores_a_preview_and_skips_what_is_cached(db, owner, monkeypatch):
|
|
"""The background pass, run inline so the assertions are deterministic.
|
|
|
|
The network is stubbed — this is about what reaches the DATABASE, not about
|
|
parsing someone's OpenGraph tags (unfurl.py's own tests cover that). What matters
|
|
here is the part only a real database can show: the unique constraint holding, the
|
|
upsert going to the right row, and a second pass not re-fetching.
|
|
"""
|
|
note = Note(
|
|
owner_id=owner.id,
|
|
body="read https://example.com/a and https://example.com/b",
|
|
display_title="read https://example.com/a and https://example.com/b",
|
|
)
|
|
db.add(note)
|
|
await db.commit()
|
|
|
|
calls: list[str] = []
|
|
|
|
async def fake_unfurl(url):
|
|
calls.append(url)
|
|
return {"url": url, "title": f"T {url}", "description": None, "image_url": None, "site_name": "example.com"}
|
|
|
|
monkeypatch.setattr("thoughtsync.unfurl_queue.unfurl", fake_unfurl)
|
|
|
|
await _unfurl_new_urls(note.id, note.body)
|
|
assert sorted(calls) == ["https://example.com/a", "https://example.com/b"]
|
|
|
|
rows = (await db.scalars(select(NoteLinkPreview).where(NoteLinkPreview.note_id == note.id))).all()
|
|
assert {r.url for r in rows} == {"https://example.com/a", "https://example.com/b"}
|
|
assert all(r.title.startswith("T ") for r in rows)
|
|
|
|
# A second pass over an unchanged body fetches nothing — the whole reason
|
|
# `schedule` is safe to call on every save.
|
|
calls.clear()
|
|
await _unfurl_new_urls(note.id, note.body)
|
|
assert calls == []
|
|
|
|
|
|
async def test_auto_unfurl_drops_a_preview_whose_url_left_the_body(db, owner, monkeypatch):
|
|
"""A slow fetch must not resurrect a link the person deleted mid-flight."""
|
|
note = Note(owner_id=owner.id, body="https://example.com/gone", display_title="x")
|
|
db.add(note)
|
|
await db.commit()
|
|
|
|
async def fake_unfurl(url):
|
|
# Simulate the body changing while the request was in the air.
|
|
return {"url": url, "title": "T", "description": None, "image_url": None, "site_name": None}
|
|
|
|
monkeypatch.setattr("thoughtsync.unfurl_queue.unfurl", fake_unfurl)
|
|
note.body = "changed my mind"
|
|
await db.commit()
|
|
|
|
await _unfurl_new_urls(note.id, "https://example.com/gone")
|
|
rows = (await db.scalars(select(NoteLinkPreview).where(NoteLinkPreview.note_id == note.id))).all()
|
|
assert rows == [], "a preview was stored for a URL the note no longer contains"
|
|
|
|
|
|
async def test_detection_agrees_with_what_gets_stored(db, owner, monkeypatch):
|
|
"""The detector and the storage path read the same body the same way."""
|
|
body = "one https://example.com/x. two (https://example.com/y) three"
|
|
assert detect_urls(body) == ["https://example.com/x", "https://example.com/y"]
|
|
|
|
note = Note(owner_id=owner.id, body=body, display_title="one")
|
|
db.add(note)
|
|
await db.commit()
|
|
|
|
async def fake_unfurl(url):
|
|
return {"url": url, "title": "T", "description": None, "image_url": None, "site_name": None}
|
|
|
|
monkeypatch.setattr("thoughtsync.unfurl_queue.unfurl", fake_unfurl)
|
|
await _unfurl_new_urls(note.id, body)
|
|
|
|
stored = {
|
|
r for r in (await db.scalars(select(NoteLinkPreview.url).where(NoteLinkPreview.note_id == note.id))).all()
|
|
}
|
|
assert stored == set(detect_urls(body))
|