jpmschweitzerandClaude Sonnet 5
7c67a4a46b
The user wants a way to get rid of a message that is no longer useful. A
message is stored once and shared by every recipient; `history` is how
agents rebuild context (D-66), so deleting the message itself would take
it from every other reader. Deletion is therefore per mailbox entry
(D-70): `archive` (kept, out of the default view) and `trash` (out of the
mailbox, restorable). True erasure of content stays `redact` — not this
round.
Migration 009 (down_revision="008") adds
scribe_mailbox_entries.archived_at and trashed_at — independent nullable
timestamps, mirroring book_records.archived_at/trashed_at (migration
005). No data step: every existing entry is neither.
PATCH /scribe/mailbox/{entry_id} takes `archived` and `trashed` booleans
beside `read` (true sets the timestamp if not already set, false clears
it); the response carries both. GET /scribe/mailbox takes `view`: `inbox`
(default, neither archived nor trashed), `archive` (archived and not
trashed), `trash` (trashed, regardless of archived_at — trashing hides an
entry from the archive view too), `all` (no filter on either column).
`find` now excludes trashed entries (archived ones still match).
T-9: GET /scribe/mailbox?message_id=N is a direct lookup, not a filtered
list — the unique constraint on (address, message_id) guarantees zero or
one, in any view, ignoring unread/tag/before_id entirely. That is the
point: a client that already knows the message number never pages the
unread-first/id-descending listing to find it.
Judgement call: the listing's own cursor (`before_id`, a single `id`
comparison) still does not match its two-key sort (unread-first, then id
descending) — the pre-existing gap the dossier flagged as optional to
fix. Left unchanged: T-9's message_id lookup is the actual answer to "a
client pages to find one," and reworking the general cursor is a larger,
separate concern (which field(s) a keyset cursor needs) that risks the
already-passing test_mailbox_lists_unread_first for no requirement this
round asked for.
Judgement call: archived_at and trashed_at are independent timestamps,
never cross-clearing each other. Trashing an already-archived entry does
not clear archived_at, so restoring it (trashed=false) returns it to the
archive view, not the inbox — the view filter's own trash-takes-priority
rule is what keeps it out of view=archive while trashed, not a write-time
side effect.
Tests: 12 new (6 archive/trash flow + 4 message_id lookup + 2 migration
round-trip). Full suite 554 -> 566 passed.
Mutation checks (Edit, reverted after each):
- view=inbox dropped its trashed_at filter ->
test_trash_hides_from_inbox_archive_and_find_shows_in_trash failed
(assert 1 == 0).
- message_id lookup dropped the caller-address filter ->
test_message_id_answers_nothing_for_a_message_not_in_their_mailbox
failed (assert 1 == 0).
- trash updated every mailbox entry for the message instead of the
caller's own row ->
test_trashing_one_recipients_entry_leaves_the_others_untouched failed
(assert 0 == 1).
All three reverted; full suite re-verified green (566 passed).
Migration 008 -> 009 -> 008 -> 009 round-tripped against core_api_test,
both manually (alembic CLI) and in
TestMigration009RoundTrip. Production stays at 008 — every alembic
invocation derived POSTGRES_* from TEST_DATABASE_URL.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
111 lines
4.8 KiB
Python
111 lines
4.8 KiB
Python
"""
|
|
Tests for Scribe migrations 006 and 009.
|
|
|
|
Runs against `core_api_test` only (see conftest.py — skip when
|
|
TEST_DATABASE_URL is unset, hard failure if it resolves to `core_api`).
|
|
Self-contained: starts and ends at head regardless of what ran before it.
|
|
"""
|
|
import pytest
|
|
from sqlalchemy import text
|
|
from sqlalchemy.ext.asyncio import create_async_engine
|
|
from sqlalchemy.pool import NullPool
|
|
|
|
from tests.conftest import run_alembic
|
|
|
|
|
|
async def _table_exists(url: str, table: str) -> bool:
|
|
asyncpg_url = url.replace("postgresql://", "postgresql+asyncpg://", 1)
|
|
engine = create_async_engine(asyncpg_url, poolclass=NullPool)
|
|
try:
|
|
async with engine.connect() as conn:
|
|
result = await conn.execute(
|
|
text("SELECT to_regclass(:name) IS NOT NULL AS present"),
|
|
{"name": table},
|
|
)
|
|
return bool(result.scalar())
|
|
finally:
|
|
await engine.dispose()
|
|
|
|
|
|
async def _columns(url: str, table: str) -> set:
|
|
asyncpg_url = url.replace("postgresql://", "postgresql+asyncpg://", 1)
|
|
engine = create_async_engine(asyncpg_url, poolclass=NullPool)
|
|
try:
|
|
async with engine.connect() as conn:
|
|
result = await conn.execute(
|
|
text(
|
|
"SELECT column_name FROM information_schema.columns "
|
|
"WHERE table_name = :table"
|
|
),
|
|
{"table": table},
|
|
)
|
|
return {row[0] for row in result.all()}
|
|
finally:
|
|
await engine.dispose()
|
|
|
|
|
|
class TestMigration006RoundTrip:
|
|
"""Migration 006 must upgrade and downgrade cleanly (dossier verification #1)."""
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_upgrade_creates_scribe_tables(self, test_database_url):
|
|
result = run_alembic(test_database_url, "upgrade", "head")
|
|
assert result.returncode == 0, result.stderr
|
|
|
|
for table in ("scribe_addresses", "scribe_messages", "scribe_mailbox_entries"):
|
|
assert await _table_exists(test_database_url, table), f"{table} missing after upgrade"
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_downgrade_then_upgrade_round_trips_cleanly(self, test_database_url):
|
|
baseline = run_alembic(test_database_url, "upgrade", "head")
|
|
assert baseline.returncode == 0, baseline.stderr
|
|
|
|
down = run_alembic(test_database_url, "downgrade", "005")
|
|
assert down.returncode == 0, down.stderr
|
|
assert not await _table_exists(test_database_url, "scribe_addresses")
|
|
assert not await _table_exists(test_database_url, "scribe_messages")
|
|
assert not await _table_exists(test_database_url, "scribe_mailbox_entries")
|
|
# Book tables, created by the migration below this one, must survive
|
|
# a downgrade of 006 — proves 006 only touches its own tables.
|
|
assert await _table_exists(test_database_url, "book_records")
|
|
|
|
# Leave the schema at head for the rest of the suite.
|
|
up = run_alembic(test_database_url, "upgrade", "006")
|
|
assert up.returncode == 0, up.stderr
|
|
assert await _table_exists(test_database_url, "scribe_addresses")
|
|
assert await _table_exists(test_database_url, "scribe_messages")
|
|
assert await _table_exists(test_database_url, "scribe_mailbox_entries")
|
|
|
|
|
|
class TestMigration009RoundTrip:
|
|
"""Migration 009 (core-api T-13) must upgrade and downgrade cleanly —
|
|
dossier verification #2: 008 -> 009 -> 008 -> 009."""
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_upgrade_adds_archived_and_trashed_at(self, test_database_url):
|
|
result = run_alembic(test_database_url, "upgrade", "head")
|
|
assert result.returncode == 0, result.stderr
|
|
|
|
columns = await _columns(test_database_url, "scribe_mailbox_entries")
|
|
assert {"archived_at", "trashed_at"} <= columns
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_downgrade_then_upgrade_round_trips_cleanly(self, test_database_url):
|
|
baseline = run_alembic(test_database_url, "upgrade", "head")
|
|
assert baseline.returncode == 0, baseline.stderr
|
|
|
|
down = run_alembic(test_database_url, "downgrade", "008")
|
|
assert down.returncode == 0, down.stderr
|
|
columns_at_008 = await _columns(test_database_url, "scribe_mailbox_entries")
|
|
assert "archived_at" not in columns_at_008
|
|
assert "trashed_at" not in columns_at_008
|
|
# The table itself, created by 006, must survive a downgrade of
|
|
# 009 — proves 009 only adds/drops its own two columns.
|
|
assert await _table_exists(test_database_url, "scribe_mailbox_entries")
|
|
|
|
# Leave the schema at head (009 is head) for the rest of the suite.
|
|
up = run_alembic(test_database_url, "upgrade", "head")
|
|
assert up.returncode == 0, up.stderr
|
|
columns_at_head = await _columns(test_database_url, "scribe_mailbox_entries")
|
|
assert {"archived_at", "trashed_at"} <= columns_at_head
|