Compare commits

..

2 Commits

Author SHA1 Message Date
3fc022335c Merge branch 'claude/relaxed-faraday-h4zd09' of https://github.com/pablovolenski/vienalatina
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
2026-09-28 15:52:12 +00:00
Claude
c9c549ed22
Let a member who posted a picture be erased
Deleting a member returned 500. The cause is mine and hours old:
attachments.uploaded_by is NOT NULL and does not cascade, so once
somebody has posted a photo SQLite refuses to delete their row.
erase_member knew about threads, comments and created_by — every table
that existed when it was written — and Phase B added a fourth without
telling it.

Reproduced before fixing: a member with a thread erases cleanly, the
same member with an attachment raises FOREIGN KEY constraint failed.

The picture is reassigned to the tombstone rather than deleted, the same
rule the words around it already follow: the thread survives as "Miembro
eliminado" and keeps its shape. Removing the picture means deleting the
post it hangs off.

The lists of columns to reassign and to clear are now module constants
that erase_member iterates, and a test reads the live schema with PRAGMA
foreign_key_list and fails if any table points at members(id), does not
cascade, and is not in them. Checked against the pre-fix lists: it names
attachments.uploaded_by. The next table to be added will fail a test
instead of a button — and this one failed at the moment somebody
exercised a right they are entitled to, which is the worst time to
find out.

201 tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NizVpJ2dwzCbjCrTLCjeHn
2026-09-28 15:51:14 +00:00
2 changed files with 85 additions and 3 deletions

View File

@ -74,6 +74,24 @@ def transfer_ownership(db: sqlite3.Connection, owner_id: int, target_id: int) ->
raise
# Every column pointing at members(id) that does not cascade has to be dealt
# with before the row can go, or SQLite refuses the delete and the admin gets a
# 500 with nothing to read. These two sets are the list, and
# test_members.py checks them against the schema's actual foreign keys — so a
# table added later fails a test here rather than a button in production. That
# is not hypothetical: `attachments` was added and forgotten, and the first
# person who tried to remove a member who had posted a photo got the 500.
REASSIGNED_ON_ERASE = {
("threads", "author_id"),
("comments", "author_id"),
# The picture belongs to the thread, which survives as "Miembro eliminado",
# so it is reassigned rather than deleted, exactly like the words around it.
# Removing the picture itself means deleting the post it is attached to.
("attachments", "uploaded_by"),
}
CLEARED_ON_ERASE = {("members", "created_by")}
def erase_member(db: sqlite3.Connection, member_id: int) -> None:
"""Remove a member and their personal data, keeping the conversation intact.
@ -84,9 +102,15 @@ def erase_member(db: sqlite3.Connection, member_id: int) -> None:
ghost = tombstone_id(db)
db.execute("BEGIN IMMEDIATE")
try:
db.execute("UPDATE threads SET author_id = ? WHERE author_id = ?", (ghost, member_id))
db.execute("UPDATE comments SET author_id = ? WHERE author_id = ?", (ghost, member_id))
db.execute("UPDATE members SET created_by = NULL WHERE created_by = ?", (member_id,))
# Table and column names come from the module constants above, never
# from a request, so the interpolation is not a place user input can
# reach.
for table, column in sorted(REASSIGNED_ON_ERASE):
db.execute(f"UPDATE {table} SET {column} = ? WHERE {column} = ?",
(ghost, member_id))
for table, column in sorted(CLEARED_ON_ERASE):
db.execute(f"UPDATE {table} SET {column} = NULL WHERE {column} = ?",
(member_id,))
db.execute("DELETE FROM members WHERE id = ? AND role != 'owner'", (member_id,))
db.execute("COMMIT")
except Exception:

View File

@ -227,3 +227,61 @@ def test_ticking_it_anyway_still_creates_nothing(app, client, db, post, owner_id
assert "GITEA_ADMIN_TOKEN" in response.get_data(as_text=True)
assert db.execute("SELECT 1 FROM members WHERE gitea_login = 'maria'").fetchone() is None
# --- erasing somebody who has left traces --------------------------------
def test_erasing_a_member_who_posted_a_picture(app, db, post, owner_id, make_member, sign_in):
"""This failed in production with a 500 the first time it was tried.
`attachments.uploaded_by` is NOT NULL and does not cascade, so with a
picture in the database SQLite refuses to delete the member — and the admin
sees "Internal Server Error" with nothing to act on."""
author = make_member("saliente")
db.execute("INSERT INTO threads (author_id, title, body_md) VALUES (?, 'Hola', 'Texto')",
(author,))
db.execute(
"""INSERT INTO attachments
(thread_id, stored_name, original_name, content_type, bytes, uploaded_by)
VALUES (1, 'foto-abc123abc123.jpg', 'foto.jpg', 'image/jpeg', 10, ?)""",
(author,),
)
sign_in(owner_id)
response = post(f"/comunidad/miembros/{author}/eliminar")
assert response.status_code == 302
assert db.execute("SELECT 1 FROM members WHERE id = ?", (author,)).fetchone() is None
# The picture stays with the thread, which survives as "Miembro eliminado" —
# the same rule the words follow. Deleting the post removes the picture.
row = db.execute(
"""SELECT m.display_name FROM attachments a JOIN members m ON m.id = a.uploaded_by"""
).fetchone()
assert row["display_name"] == "Miembro eliminado"
def test_every_table_pointing_at_members_is_accounted_for(db):
"""The guard that makes the bug above unrepeatable.
Read out of the live schema rather than written down twice: any future
table with a foreign key to members(id) either cascades, or is named in
members.py's erase lists. Otherwise erasure breaks — and it breaks at the
moment somebody exercises a right they are entitled to, which is the worst
possible time to find out."""
from apps.board.members import CLEARED_ON_ERASE, REASSIGNED_ON_ERASE
handled = REASSIGNED_ON_ERASE | CLEARED_ON_ERASE
tables = [row["name"] for row in db.execute(
"SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT LIKE 'sqlite_%'")]
unhandled = [
f"{table}.{fk['from']}"
for table in tables
for fk in db.execute(f"PRAGMA foreign_key_list('{table}')").fetchall()
if fk["table"] == "members"
and (fk["on_delete"] or "").upper() != "CASCADE"
and (table, fk["from"]) not in handled
]
assert not unhandled, (
"these point at members(id), do not cascade, and erase_member does not "
f"touch them, so erasing a member will fail: {unhandled}"
)