Phase C: private messages, with blocking and photos
An inbox between two members — conversations, per-person unread marks,
photos, blocking. Not live chat: that needs a connection held open per
signed-in member, which the sync workers cannot do.
Membership of the conversation is the whole access rule and is checked on
every hit, answering 404 rather than 403 so a member cannot tell a
conversation that is not theirs from one that does not exist. A picture
in a private message is checked the same way: on the board being signed
in is enough, here it is nowhere near.
Blocking is symmetric. One row stops both directions, and you can only
lift your own. A block that silenced only the blocked person would leave
the blocker writing freely, which is a megaphone rather than a safety
feature. Enforced in the handlers, with a test that posts from a page
held open from before the block.
Erasing a member deletes their private messages, both sides, and their
pictures off disk. A thread outlives its author because other people
replied; a two-party exchange has no remainder, and keeping half of
erased correspondence is what erasure exists to prevent. The guard added
in c9c549e did its job: it failed the moment the new tables landed and
named all four columns.
The part that needed care: schema.sql is all CREATE TABLE IF NOT EXISTS,
so it can add a table and nothing else. Every change so far happened to
be a new table. Letting an attachment belong to a message is not — and
SQLite cannot do it in place, because the table carries a CHECK
constraint and there is no DROP CONSTRAINT. Verified before building on
it: ALTER TABLE ADD COLUMN succeeds and the next insert is refused.
So migrations.py, numbered steps recorded in PRAGMA user_version, run
after the schema so a fresh database finds its work already done. Step 1
rebuilds attachments the documented way. Tested against a database built
in the old shape with rows in it, because a migration tested only on a
fresh database is tested against the one case it was never needed for —
including that the rebuilt CHECK is as strict as the one it replaced.
229 tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NizVpJ2dwzCbjCrTLCjeHn
This commit is contained in:
parent
c9c549ed22
commit
5ab9e3cab5
@ -91,11 +91,12 @@ def create_app(overrides: dict | None = None) -> Flask:
|
||||
# restart, which is a confusing way to find out the variable is unset.
|
||||
raise RuntimeError("BOARD_SECRET_KEY is required (generate one with `openssl rand -hex 32`).")
|
||||
|
||||
from . import auth, board, content, members, uploads
|
||||
from . import auth, board, content, members, messages, uploads
|
||||
app.register_blueprint(auth.bp, url_prefix=URL_PREFIX)
|
||||
app.register_blueprint(members.bp, url_prefix=URL_PREFIX)
|
||||
app.register_blueprint(content.bp, url_prefix=URL_PREFIX)
|
||||
app.register_blueprint(uploads.bp, url_prefix=URL_PREFIX)
|
||||
app.register_blueprint(messages.bp, url_prefix=URL_PREFIX)
|
||||
app.register_blueprint(board.bp, url_prefix=URL_PREFIX)
|
||||
|
||||
app.teardown_appcontext(close_db)
|
||||
@ -106,6 +107,12 @@ def create_app(overrides: dict | None = None) -> Flask:
|
||||
def _year():
|
||||
return {"current_year": datetime.now(timezone.utc).year}
|
||||
|
||||
@app.context_processor
|
||||
def _unread():
|
||||
# Lazily: base.html is rendered for signed-out pages and error pages
|
||||
# too, and neither should run a query to draw a badge nobody sees.
|
||||
return {"unread_private": messages.unread_count}
|
||||
|
||||
@app.before_request
|
||||
def _before():
|
||||
# Order matters: reject forged writes before any handler can act on
|
||||
|
||||
@ -13,6 +13,8 @@ from pathlib import Path
|
||||
|
||||
from flask import current_app, g
|
||||
|
||||
from . import migrations
|
||||
|
||||
SCHEMA_PATH = Path(__file__).with_name("schema.sql")
|
||||
|
||||
# Authorship of a removed member is reassigned to this row rather than deleted,
|
||||
@ -48,11 +50,20 @@ def close_db(_exception=None) -> None:
|
||||
|
||||
|
||||
def init_db(app) -> None:
|
||||
"""Apply the schema and make sure the fixed rows exist."""
|
||||
"""Apply the schema, bring old databases up to date, seed the fixed rows.
|
||||
|
||||
The order is deliberate. `schema.sql` is all CREATE TABLE IF NOT EXISTS, so
|
||||
on an empty database it builds everything in its current shape and the
|
||||
migrations below find nothing to do; on a database that has run before it
|
||||
adds only what is new and silently leaves existing tables alone — which is
|
||||
precisely why migrations have to come second and clean up after it.
|
||||
"""
|
||||
Path(app.config["DB_PATH"]).parent.mkdir(parents=True, exist_ok=True)
|
||||
db = connect(app.config["DB_PATH"])
|
||||
try:
|
||||
db.executescript(SCHEMA_PATH.read_text(encoding="utf-8"))
|
||||
for step in migrations.apply(db):
|
||||
app.logger.info("Applied migration %s", step)
|
||||
_ensure_tombstone(db)
|
||||
_seed_owner(db, app)
|
||||
finally:
|
||||
|
||||
@ -21,7 +21,7 @@ import sqlite3
|
||||
from flask import (Blueprint, Response, abort, current_app, flash, g, redirect,
|
||||
render_template, request, url_for)
|
||||
|
||||
from . import auth, gitea, invites, mail
|
||||
from . import auth, gitea, invites, mail, uploads
|
||||
from .db import TOMBSTONE_LOGIN, get_db
|
||||
from .security import admin_required, login_required, owner_required
|
||||
|
||||
@ -91,17 +91,62 @@ REASSIGNED_ON_ERASE = {
|
||||
}
|
||||
CLEARED_ON_ERASE = {("members", "created_by")}
|
||||
|
||||
# Private correspondence is not reassigned to the tombstone, it goes. A thread
|
||||
# outlives its author because other people replied and the conversation would
|
||||
# otherwise lose its shape; a two-party exchange has no such remainder, and
|
||||
# keeping half of somebody's erased correspondence is close to the thing
|
||||
# erasure exists to prevent. The other person's copy goes with it, which is the
|
||||
# uncomfortable half of that choice and is meant to be.
|
||||
REMOVED_ON_ERASE = {
|
||||
("conversation_members", "member_id"),
|
||||
("messages", "author_id"),
|
||||
("blocks", "blocker_id"),
|
||||
("blocks", "blocked_id"),
|
||||
}
|
||||
|
||||
def erase_member(db: sqlite3.Connection, member_id: int) -> None:
|
||||
"""Remove a member and their personal data, keeping the conversation intact.
|
||||
|
||||
def _conversations_of(member_id: int) -> str:
|
||||
return "SELECT conversation_id FROM conversation_members WHERE member_id = ?"
|
||||
|
||||
|
||||
def erase_member(db: sqlite3.Connection, member_id: int) -> list[str]:
|
||||
"""Remove a member and their personal data, keeping the board intact.
|
||||
|
||||
GDPR erasure means the name, login and address go. It does not mean the
|
||||
threads other people replied to should vanish, so authorship moves to the
|
||||
tombstone row instead of cascading or dangling.
|
||||
threads other people replied to should vanish, so public authorship moves
|
||||
to the tombstone row instead of cascading or dangling. Private messages are
|
||||
the exception and are deleted outright — see REMOVED_ON_ERASE.
|
||||
|
||||
Returns the stored names of pictures whose rows have gone, so the caller can
|
||||
take them off disk. Deleting files is not done here because this function
|
||||
is a transaction: a file removed inside one cannot be put back if the
|
||||
transaction rolls away underneath it.
|
||||
"""
|
||||
ghost = tombstone_id(db)
|
||||
db.execute("BEGIN IMMEDIATE")
|
||||
try:
|
||||
# Read before deleting: once the conversations are gone there is
|
||||
# nothing left to work out which files they carried.
|
||||
orphaned = [row["stored_name"] for row in db.execute(
|
||||
f"""SELECT a.stored_name FROM attachments a
|
||||
JOIN messages m ON m.id = a.message_id
|
||||
WHERE m.conversation_id IN ({_conversations_of(member_id)})""",
|
||||
(member_id,),
|
||||
)]
|
||||
# Attachments first: they point at messages without cascading, so
|
||||
# deleting the conversation while they exist is refused.
|
||||
db.execute(
|
||||
f"""DELETE FROM attachments WHERE message_id IN (
|
||||
SELECT id FROM messages
|
||||
WHERE conversation_id IN ({_conversations_of(member_id)}))""",
|
||||
(member_id,),
|
||||
)
|
||||
db.execute(
|
||||
f"DELETE FROM conversations WHERE id IN ({_conversations_of(member_id)})",
|
||||
(member_id,),
|
||||
)
|
||||
db.execute("DELETE FROM blocks WHERE blocker_id = ? OR blocked_id = ?",
|
||||
(member_id, 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.
|
||||
@ -116,6 +161,7 @@ def erase_member(db: sqlite3.Connection, member_id: int) -> None:
|
||||
except Exception:
|
||||
db.execute("ROLLBACK")
|
||||
raise
|
||||
return orphaned
|
||||
|
||||
|
||||
def _load(member_id: int):
|
||||
@ -306,7 +352,10 @@ def erase(member_id: int):
|
||||
target = _load(member_id)
|
||||
if target["role"] == "owner":
|
||||
abort(403)
|
||||
erase_member(get_db(), member_id)
|
||||
# Files only after the transaction has committed: a rollback can put the
|
||||
# rows back, and nothing can put the pictures back.
|
||||
for stored_name in erase_member(get_db(), member_id):
|
||||
uploads.remove(stored_name)
|
||||
flash(f"{target['display_name']} eliminado. Sus mensajes quedan como «Miembro eliminado».", "ok")
|
||||
return redirect(url_for("members.index"))
|
||||
|
||||
|
||||
260
apps/board/messages.py
Normal file
260
apps/board/messages.py
Normal file
@ -0,0 +1,260 @@
|
||||
"""Private messages between two members.
|
||||
|
||||
An inbox, not a chat. Each conversation has exactly two people, each keeps
|
||||
their own unread mark, and either can block the other — after which neither can
|
||||
write. That symmetry is deliberate: a block that silences only one side is a
|
||||
one-way megaphone, which is worse than having no block at all.
|
||||
|
||||
Every rule here is enforced in the handler, not only in the template. A hidden
|
||||
button is a courtesy to somebody using the site normally; it is not a rule, and
|
||||
the difference matters most for exactly the person a block exists to stop.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from flask import (Blueprint, abort, flash, g, redirect, render_template,
|
||||
request, url_for)
|
||||
|
||||
from . import uploads
|
||||
from .db import TOMBSTONE_LOGIN, get_db
|
||||
from .render import to_html
|
||||
from .security import login_required
|
||||
|
||||
bp = Blueprint("messages", __name__)
|
||||
|
||||
BODY_MAX = 20_000
|
||||
|
||||
|
||||
def _other_party(conversation_id: int):
|
||||
"""The member on the far side, or None if they have been erased."""
|
||||
return get_db().execute(
|
||||
"""SELECT m.* FROM conversation_members cm
|
||||
JOIN members m ON m.id = cm.member_id
|
||||
WHERE cm.conversation_id = ? AND cm.member_id != ?""",
|
||||
(conversation_id, g.member["id"]),
|
||||
).fetchone()
|
||||
|
||||
|
||||
def _mine_or_404(conversation_id: int):
|
||||
"""Membership is the entire access rule, and it is checked on every hit.
|
||||
|
||||
Not "does this conversation exist" — a member who is not in it must not be
|
||||
able to tell the difference between a conversation that is not theirs and
|
||||
one that does not exist.
|
||||
"""
|
||||
row = get_db().execute(
|
||||
"""SELECT c.* FROM conversations c
|
||||
JOIN conversation_members cm ON cm.conversation_id = c.id
|
||||
WHERE c.id = ? AND cm.member_id = ?""",
|
||||
(conversation_id, g.member["id"]),
|
||||
).fetchone()
|
||||
if row is None:
|
||||
abort(404)
|
||||
return row
|
||||
|
||||
|
||||
def blocked_between(a: int, b: int) -> bool:
|
||||
"""A block in either direction stops both directions."""
|
||||
return get_db().execute(
|
||||
"""SELECT 1 FROM blocks
|
||||
WHERE (blocker_id = ? AND blocked_id = ?)
|
||||
OR (blocker_id = ? AND blocked_id = ?)""",
|
||||
(a, b, b, a),
|
||||
).fetchone() is not None
|
||||
|
||||
|
||||
def conversation_with(other_id: int) -> int:
|
||||
"""The conversation between the signed-in member and `other_id`, made if
|
||||
it does not exist yet."""
|
||||
db = get_db()
|
||||
row = db.execute(
|
||||
"""SELECT a.conversation_id AS id
|
||||
FROM conversation_members a
|
||||
JOIN conversation_members b ON b.conversation_id = a.conversation_id
|
||||
WHERE a.member_id = ? AND b.member_id = ?""",
|
||||
(g.member["id"], other_id),
|
||||
).fetchone()
|
||||
if row:
|
||||
return row["id"]
|
||||
|
||||
conversation_id = db.execute(
|
||||
"INSERT INTO conversations DEFAULT VALUES").lastrowid
|
||||
for member_id in (g.member["id"], other_id):
|
||||
db.execute(
|
||||
"INSERT INTO conversation_members (conversation_id, member_id) VALUES (?, ?)",
|
||||
(conversation_id, member_id),
|
||||
)
|
||||
return conversation_id
|
||||
|
||||
|
||||
def unread_count() -> int:
|
||||
"""For the badge in the navigation. One query, not one per conversation."""
|
||||
if g.member is None:
|
||||
return 0
|
||||
return get_db().execute(
|
||||
"""SELECT COUNT(*) AS n
|
||||
FROM messages m
|
||||
JOIN conversation_members cm
|
||||
ON cm.conversation_id = m.conversation_id AND cm.member_id = ?
|
||||
WHERE m.author_id != ?
|
||||
AND m.deleted_at IS NULL
|
||||
AND (cm.last_read_at IS NULL OR m.created_at > cm.last_read_at)""",
|
||||
(g.member["id"], g.member["id"]),
|
||||
).fetchone()["n"]
|
||||
|
||||
|
||||
# --- routes ---------------------------------------------------------------
|
||||
|
||||
@bp.route("/privados")
|
||||
@login_required
|
||||
def inbox():
|
||||
rows = get_db().execute(
|
||||
"""SELECT c.id,
|
||||
other.display_name AS other_name,
|
||||
other.id AS other_id,
|
||||
(SELECT body_md FROM messages
|
||||
WHERE conversation_id = c.id AND deleted_at IS NULL
|
||||
ORDER BY created_at DESC LIMIT 1) AS last_body,
|
||||
(SELECT created_at FROM messages
|
||||
WHERE conversation_id = c.id AND deleted_at IS NULL
|
||||
ORDER BY created_at DESC LIMIT 1) AS last_at,
|
||||
(SELECT COUNT(*) FROM messages m
|
||||
WHERE m.conversation_id = c.id AND m.author_id != mine.member_id
|
||||
AND m.deleted_at IS NULL
|
||||
AND (mine.last_read_at IS NULL OR m.created_at > mine.last_read_at)
|
||||
) AS unread
|
||||
FROM conversations c
|
||||
JOIN conversation_members mine
|
||||
ON mine.conversation_id = c.id AND mine.member_id = :me
|
||||
JOIN conversation_members theirs
|
||||
ON theirs.conversation_id = c.id AND theirs.member_id != :me
|
||||
JOIN members other ON other.id = theirs.member_id
|
||||
ORDER BY last_at DESC NULLS LAST""",
|
||||
{"me": g.member["id"]},
|
||||
).fetchall()
|
||||
|
||||
# Anyone you could start writing to: every active member but yourself, the
|
||||
# tombstone, and anyone either of you has blocked.
|
||||
people = get_db().execute(
|
||||
"""SELECT m.id, m.display_name FROM members m
|
||||
WHERE m.id != :me AND m.active = 1
|
||||
AND m.role IN ('owner', 'admin', 'user')
|
||||
AND m.gitea_login != :ghost
|
||||
AND NOT EXISTS (SELECT 1 FROM blocks
|
||||
WHERE (blocker_id = :me AND blocked_id = m.id)
|
||||
OR (blocker_id = m.id AND blocked_id = :me))
|
||||
ORDER BY m.display_name COLLATE NOCASE""",
|
||||
{"me": g.member["id"], "ghost": TOMBSTONE_LOGIN},
|
||||
).fetchall()
|
||||
|
||||
return render_template("inbox.html", conversations=rows, people=people,
|
||||
excerpt=lambda text: (text or "")[:120])
|
||||
|
||||
|
||||
@bp.route("/privados/nueva", methods=["POST"])
|
||||
@login_required
|
||||
def start():
|
||||
"""The member comes from the form, not the path, so the select can post
|
||||
straight here. A path parameter would need JavaScript to rewrite the
|
||||
action, and the Content-Security-Policy makes inline handlers silently
|
||||
inert — a class of bug this codebase has already paid for once."""
|
||||
member_id = request.form.get("member_id", type=int)
|
||||
if member_id is None or member_id == g.member["id"]:
|
||||
abort(400)
|
||||
target = get_db().execute(
|
||||
"""SELECT * FROM members
|
||||
WHERE id = ? AND active = 1 AND role IN ('owner', 'admin', 'user')""",
|
||||
(member_id,),
|
||||
).fetchone()
|
||||
if target is None:
|
||||
abort(404)
|
||||
if blocked_between(g.member["id"], member_id):
|
||||
abort(403)
|
||||
return redirect(url_for("messages.conversation",
|
||||
conversation_id=conversation_with(member_id)))
|
||||
|
||||
|
||||
@bp.route("/privados/<int:conversation_id>")
|
||||
@login_required
|
||||
def conversation(conversation_id: int):
|
||||
_mine_or_404(conversation_id)
|
||||
db = get_db()
|
||||
rows = db.execute(
|
||||
"""SELECT m.*, a.display_name AS author
|
||||
FROM messages m JOIN members a ON a.id = m.author_id
|
||||
WHERE m.conversation_id = ? AND m.deleted_at IS NULL
|
||||
ORDER BY m.created_at""",
|
||||
(conversation_id,),
|
||||
).fetchall()
|
||||
|
||||
# Marked read on opening, which is what the member just did. Written before
|
||||
# rendering so a slow page does not leave the badge stale.
|
||||
db.execute(
|
||||
"""UPDATE conversation_members SET last_read_at = datetime('now')
|
||||
WHERE conversation_id = ? AND member_id = ?""",
|
||||
(conversation_id, g.member["id"]),
|
||||
)
|
||||
|
||||
other = _other_party(conversation_id)
|
||||
return render_template(
|
||||
"conversation.html", conversation_id=conversation_id, messages=rows,
|
||||
other=other, to_html=to_html,
|
||||
images=uploads.for_messages([row["id"] for row in rows]),
|
||||
blocked=other is not None and blocked_between(g.member["id"], other["id"]),
|
||||
)
|
||||
|
||||
|
||||
@bp.route("/privados/<int:conversation_id>/enviar", methods=["POST"])
|
||||
@login_required
|
||||
def send(conversation_id: int):
|
||||
_mine_or_404(conversation_id)
|
||||
other = _other_party(conversation_id)
|
||||
if other is None:
|
||||
flash("Esa persona ya no está en la comunidad.", "error")
|
||||
return redirect(url_for("messages.conversation", conversation_id=conversation_id))
|
||||
# Checked here and not only hidden in the template: the template is a
|
||||
# courtesy, this is the rule.
|
||||
if blocked_between(g.member["id"], other["id"]):
|
||||
abort(403)
|
||||
|
||||
body = request.form.get("body", "").strip()[:BODY_MAX]
|
||||
try:
|
||||
staged = uploads.stage(request.files.getlist("pictures"))
|
||||
except uploads.RejectedUpload as exc:
|
||||
flash(str(exc), "error")
|
||||
return redirect(url_for("messages.conversation", conversation_id=conversation_id))
|
||||
if not body and not staged:
|
||||
flash("Escribe algo o adjunta una imagen.", "error")
|
||||
return redirect(url_for("messages.conversation", conversation_id=conversation_id))
|
||||
|
||||
cursor = get_db().execute(
|
||||
"INSERT INTO messages (conversation_id, author_id, body_md) VALUES (?, ?, ?)",
|
||||
(conversation_id, g.member["id"], body),
|
||||
)
|
||||
uploads.save(staged, g.member["id"], message_id=cursor.lastrowid)
|
||||
return redirect(url_for("messages.conversation", conversation_id=conversation_id) + "#final")
|
||||
|
||||
|
||||
@bp.route("/privados/bloquear/<int:member_id>", methods=["POST"])
|
||||
@login_required
|
||||
def block(member_id: int):
|
||||
if member_id == g.member["id"]:
|
||||
abort(400)
|
||||
get_db().execute(
|
||||
"""INSERT INTO blocks (blocker_id, blocked_id) VALUES (?, ?)
|
||||
ON CONFLICT DO NOTHING""",
|
||||
(g.member["id"], member_id),
|
||||
)
|
||||
flash("Bloqueado. Ninguno de los dos puede escribir al otro.", "ok")
|
||||
return redirect(url_for("messages.inbox"))
|
||||
|
||||
|
||||
@bp.route("/privados/desbloquear/<int:member_id>", methods=["POST"])
|
||||
@login_required
|
||||
def unblock(member_id: int):
|
||||
"""Only your own block. Removing somebody else's would let the blocked
|
||||
person undo the thing that was done to protect against them."""
|
||||
get_db().execute("DELETE FROM blocks WHERE blocker_id = ? AND blocked_id = ?",
|
||||
(g.member["id"], member_id))
|
||||
flash("Desbloqueado.", "ok")
|
||||
return redirect(url_for("messages.inbox"))
|
||||
115
apps/board/migrations.py
Normal file
115
apps/board/migrations.py
Normal file
@ -0,0 +1,115 @@
|
||||
"""Changes to tables that already exist.
|
||||
|
||||
`schema.sql` is all `CREATE TABLE IF NOT EXISTS`, which handles exactly one
|
||||
kind of change: a brand-new table. It silently does nothing to a table that is
|
||||
already there, so every alteration to an existing one has to happen here.
|
||||
|
||||
That was fine until now because every change so far had been a new table. The
|
||||
first change that is not — giving `attachments` a third possible parent — also
|
||||
happens to be one SQLite cannot do in place, because the old row carries a
|
||||
CHECK constraint and SQLite has no `DROP CONSTRAINT`. Verified rather than
|
||||
assumed: `ALTER TABLE ... ADD COLUMN` succeeds, and the next insert is refused
|
||||
by a constraint that can no longer be removed.
|
||||
|
||||
**The order matters.** `init_db` applies `schema.sql` first and then these. On
|
||||
an empty database the schema creates everything in its current shape and each
|
||||
step below finds its work already done, so every step must be written to check
|
||||
before it acts and return quietly.
|
||||
|
||||
Steps are numbered, applied once, in order, each in its own transaction, and
|
||||
recorded in SQLite's own `PRAGMA user_version`. Never renumber one and never
|
||||
edit one that has shipped: a server that has already run it will not run it
|
||||
again, so a correction is a new step.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sqlite3
|
||||
|
||||
|
||||
def _columns(db: sqlite3.Connection, table: str) -> set[str]:
|
||||
return {row[1] for row in db.execute(f"PRAGMA table_info({table})")}
|
||||
|
||||
|
||||
def _attachments_accept_messages(db: sqlite3.Connection) -> None:
|
||||
"""Let an attachment hang off a private message.
|
||||
|
||||
The table is rebuilt rather than altered because of its CHECK constraint:
|
||||
`(thread_id IS NULL) <> (comment_id IS NULL)` insists that exactly one of
|
||||
those two is set, so a row belonging to a message — with both of them null
|
||||
— is refused. Adding the column is allowed; using it is not.
|
||||
|
||||
This is SQLite's documented procedure for changing a constraint: build the
|
||||
new table beside the old one, copy the rows, drop the old, rename. The
|
||||
foreign keys are switched off around it because dropping a table with them
|
||||
on can cascade, and switched back on after, with a check that nothing was
|
||||
broken in between.
|
||||
"""
|
||||
if "message_id" in _columns(db, "attachments"):
|
||||
return # a new database: schema.sql already created it this way
|
||||
|
||||
db.execute("PRAGMA foreign_keys = OFF")
|
||||
try:
|
||||
db.execute("""
|
||||
CREATE TABLE attachments_new (
|
||||
id INTEGER PRIMARY KEY,
|
||||
thread_id INTEGER REFERENCES threads(id),
|
||||
comment_id INTEGER REFERENCES comments(id),
|
||||
message_id INTEGER REFERENCES messages(id),
|
||||
stored_name TEXT NOT NULL UNIQUE,
|
||||
original_name TEXT NOT NULL,
|
||||
content_type TEXT NOT NULL,
|
||||
bytes INTEGER NOT NULL,
|
||||
uploaded_by INTEGER NOT NULL REFERENCES members(id),
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
CHECK ((thread_id IS NOT NULL) + (comment_id IS NOT NULL)
|
||||
+ (message_id IS NOT NULL) = 1)
|
||||
)""")
|
||||
# Named columns, not SELECT *: the order has to survive somebody adding
|
||||
# a column to one of the two tables later.
|
||||
db.execute("""
|
||||
INSERT INTO attachments_new
|
||||
(id, thread_id, comment_id, stored_name, original_name,
|
||||
content_type, bytes, uploaded_by, created_at)
|
||||
SELECT id, thread_id, comment_id, stored_name, original_name,
|
||||
content_type, bytes, uploaded_by, created_at
|
||||
FROM attachments""")
|
||||
db.execute("DROP TABLE attachments")
|
||||
db.execute("ALTER TABLE attachments_new RENAME TO attachments")
|
||||
db.execute("CREATE INDEX IF NOT EXISTS attachments_thread ON attachments(thread_id)")
|
||||
db.execute("CREATE INDEX IF NOT EXISTS attachments_comment ON attachments(comment_id)")
|
||||
db.execute("CREATE INDEX IF NOT EXISTS attachments_message ON attachments(message_id)")
|
||||
|
||||
broken = db.execute("PRAGMA foreign_key_check").fetchall()
|
||||
if broken:
|
||||
raise RuntimeError(f"migration left dangling references: {broken}")
|
||||
finally:
|
||||
db.execute("PRAGMA foreign_keys = ON")
|
||||
|
||||
|
||||
# (number, description, function). The number is the value written to
|
||||
# user_version once the step succeeds.
|
||||
STEPS = [
|
||||
(1, "attachments can belong to a private message", _attachments_accept_messages),
|
||||
]
|
||||
|
||||
|
||||
def apply(db: sqlite3.Connection) -> list[str]:
|
||||
"""Run whatever this database has not run yet. Returns what was applied."""
|
||||
version = db.execute("PRAGMA user_version").fetchone()[0]
|
||||
done = []
|
||||
for number, description, step in sorted(STEPS):
|
||||
if number <= version:
|
||||
continue
|
||||
db.execute("BEGIN IMMEDIATE")
|
||||
try:
|
||||
step(db)
|
||||
# Not a parameter: PRAGMA does not take them. The value is an int
|
||||
# from the list above, never from anything a request can reach.
|
||||
db.execute(f"PRAGMA user_version = {int(number)}")
|
||||
db.execute("COMMIT")
|
||||
except Exception:
|
||||
db.execute("ROLLBACK")
|
||||
raise
|
||||
done.append(f"{number}: {description}")
|
||||
return done
|
||||
@ -55,7 +55,62 @@ CREATE TABLE IF NOT EXISTS comments (
|
||||
CREATE INDEX IF NOT EXISTS comments_thread
|
||||
ON comments(thread_id, created_at) WHERE deleted_at IS NULL;
|
||||
|
||||
-- Pictures attached to a thread or a comment.
|
||||
-- Private messages between two members.
|
||||
--
|
||||
-- An inbox, not live chat: gunicorn's sync workers cannot hold a connection
|
||||
-- open per signed-in member, and that would be the first thing on this box
|
||||
-- with a real scaling limit.
|
||||
--
|
||||
-- Membership is its own table rather than two columns on `conversations`
|
||||
-- because the unread mark is per person: each side keeps its own
|
||||
-- `last_read_at`, and the badge counts messages newer than it that somebody
|
||||
-- else wrote. Two columns would need two last-read fields and a rule about
|
||||
-- which is which.
|
||||
CREATE TABLE IF NOT EXISTS conversations (
|
||||
id INTEGER PRIMARY KEY,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now'))
|
||||
);
|
||||
|
||||
CREATE TABLE IF NOT EXISTS conversation_members (
|
||||
conversation_id INTEGER NOT NULL REFERENCES conversations(id) ON DELETE CASCADE,
|
||||
member_id INTEGER NOT NULL REFERENCES members(id),
|
||||
last_read_at TEXT,
|
||||
PRIMARY KEY (conversation_id, member_id)
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS conversation_members_member
|
||||
ON conversation_members(member_id);
|
||||
|
||||
CREATE TABLE IF NOT EXISTS messages (
|
||||
id INTEGER PRIMARY KEY,
|
||||
conversation_id INTEGER NOT NULL REFERENCES conversations(id) ON DELETE CASCADE,
|
||||
author_id INTEGER NOT NULL REFERENCES members(id),
|
||||
body_md TEXT NOT NULL,
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
deleted_at TEXT
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS messages_conversation
|
||||
ON messages(conversation_id, created_at) WHERE deleted_at IS NULL;
|
||||
|
||||
-- Blocking is symmetric: one row stops messages in both directions.
|
||||
--
|
||||
-- The alternative — the blocker may still write, the blocked may not reply —
|
||||
-- turns a safety feature into a one-way megaphone, which is worse than not
|
||||
-- having one. Somebody who blocks a person and then wants to talk to them can
|
||||
-- unblock. The CHECK is there because blocking yourself is meaningless and
|
||||
-- would quietly disable your own inbox.
|
||||
CREATE TABLE IF NOT EXISTS blocks (
|
||||
blocker_id INTEGER NOT NULL REFERENCES members(id),
|
||||
blocked_id INTEGER NOT NULL REFERENCES members(id),
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
PRIMARY KEY (blocker_id, blocked_id),
|
||||
CHECK (blocker_id <> blocked_id)
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS blocks_blocked ON blocks(blocked_id);
|
||||
|
||||
-- Pictures attached to a thread, a comment or a private message.
|
||||
--
|
||||
-- The file itself lives in /data/uploads; this is the record of what it is and
|
||||
-- what it belongs to. `stored_name` is generated, never the name the browser
|
||||
@ -69,17 +124,20 @@ CREATE TABLE IF NOT EXISTS attachments (
|
||||
id INTEGER PRIMARY KEY,
|
||||
thread_id INTEGER REFERENCES threads(id),
|
||||
comment_id INTEGER REFERENCES comments(id),
|
||||
message_id INTEGER REFERENCES messages(id),
|
||||
stored_name TEXT NOT NULL UNIQUE,
|
||||
original_name TEXT NOT NULL,
|
||||
content_type TEXT NOT NULL,
|
||||
bytes INTEGER NOT NULL,
|
||||
uploaded_by INTEGER NOT NULL REFERENCES members(id),
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
CHECK ((thread_id IS NULL) <> (comment_id IS NULL))
|
||||
CHECK ((thread_id IS NOT NULL) + (comment_id IS NOT NULL)
|
||||
+ (message_id IS NOT NULL) = 1)
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS attachments_thread ON attachments(thread_id);
|
||||
CREATE INDEX IF NOT EXISTS attachments_comment ON attachments(comment_id);
|
||||
CREATE INDEX IF NOT EXISTS attachments_message ON attachments(message_id);
|
||||
|
||||
-- Gitea access tokens for the editor.
|
||||
--
|
||||
|
||||
@ -31,6 +31,11 @@
|
||||
<ul class="page-nav">
|
||||
<li><a href="{{ url_for('board.threads') }}">Mensajes</a></li>
|
||||
{% if g.member.role in ('owner', 'admin') %}
|
||||
<li>
|
||||
{# "Mensajes" is the public board, so this cannot be called that.
|
||||
The badge is only drawn when there is something to see. #}
|
||||
<a href="{{ url_for('messages.inbox') }}">Privados{% set pending = unread_private() %}{% if pending %} <span class="tag">{{ pending }}</span>{% endif %}</a>
|
||||
</li>
|
||||
<li><a href="{{ url_for('content.index') }}">Contenido</a></li>
|
||||
{% endif %}
|
||||
<li><a href="{{ url_for('members.index') }}">Miembros</a></li>
|
||||
|
||||
50
apps/board/templates/conversation.html
Normal file
50
apps/board/templates/conversation.html
Normal file
@ -0,0 +1,50 @@
|
||||
{% extends "base.html" %}
|
||||
{% from "_attachments.html" import attachments %}
|
||||
{% block title %}{{ other.display_name if other else 'Conversación' }}{% endblock %}
|
||||
|
||||
{% block main %}
|
||||
<div class="feed-head">
|
||||
<h1>{{ other.display_name if other else 'Miembro eliminado' }}</h1>
|
||||
<a class="linkish" href="{{ url_for('messages.inbox') }}">Volver</a>
|
||||
</div>
|
||||
|
||||
{% for m in messages %}
|
||||
<article class="card card--comment">
|
||||
<div class="card__meta">
|
||||
<span>{{ m.author }}</span> · <time>{{ m.created_at }}</time>
|
||||
</div>
|
||||
<div class="prose">{{ to_html(m.body_md) }}</div>
|
||||
{{ attachments(images.get(m.id, [])) }}
|
||||
</article>
|
||||
{% endfor %}
|
||||
|
||||
<span id="final"></span>
|
||||
|
||||
{% if other is none %}
|
||||
<p class="muted">Esta persona ya no está en la comunidad.</p>
|
||||
{% elif blocked %}
|
||||
<p class="muted">
|
||||
Hay un bloqueo entre vosotros, así que ninguno de los dos puede escribir.
|
||||
</p>
|
||||
<form method="post" action="{{ url_for('messages.unblock', member_id=other.id) }}">
|
||||
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
|
||||
<button class="linkish" type="submit">Quitar mi bloqueo</button>
|
||||
</form>
|
||||
<p class="muted small">
|
||||
Si el bloqueo lo puso la otra persona, esto no lo quita.
|
||||
</p>
|
||||
{% else %}
|
||||
<form class="card" method="post" enctype="multipart/form-data"
|
||||
action="{{ url_for('messages.send', conversation_id=conversation_id) }}">
|
||||
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
|
||||
<label for="body">Mensaje</label>
|
||||
<textarea id="body" name="body" rows="4"
|
||||
placeholder="Se puede usar Markdown: **negrita**, listas, enlaces."></textarea>
|
||||
|
||||
<label for="pictures">Imágenes <span class="muted small">(opcional)</span></label>
|
||||
<input id="pictures" name="pictures" type="file" multiple accept="image/*">
|
||||
|
||||
<button class="btn" type="submit">Enviar</button>
|
||||
</form>
|
||||
{% endif %}
|
||||
{% endblock %}
|
||||
43
apps/board/templates/inbox.html
Normal file
43
apps/board/templates/inbox.html
Normal file
@ -0,0 +1,43 @@
|
||||
{% extends "base.html" %}
|
||||
{% block title %}Privados{% endblock %}
|
||||
|
||||
{% block main %}
|
||||
<div class="feed-head">
|
||||
<h1>Mensajes privados</h1>
|
||||
</div>
|
||||
|
||||
{% if people %}
|
||||
<form class="card" method="post" action="{{ url_for('messages.start') }}">
|
||||
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
|
||||
<label for="para">Escribir a</label>
|
||||
<select id="para" name="member_id">
|
||||
{% for person in people %}
|
||||
<option value="{{ person.id }}">{{ person.display_name }}</option>
|
||||
{% endfor %}
|
||||
</select>
|
||||
<button class="btn" type="submit">Abrir conversación</button>
|
||||
</form>
|
||||
{% endif %}
|
||||
|
||||
{% if not conversations %}
|
||||
<p class="muted">Todavía no tienes conversaciones privadas.</p>
|
||||
{% endif %}
|
||||
|
||||
{% for c in conversations %}
|
||||
<article class="card">
|
||||
<a class="card__title" href="{{ url_for('messages.conversation', conversation_id=c.id) }}">
|
||||
{{ c.other_name }}
|
||||
{% if c.unread %}<span class="tag">{{ c.unread }}</span>{% endif %}
|
||||
</a>
|
||||
<p class="muted small">{{ excerpt(c.last_body) or 'Sin mensajes todavía.' }}</p>
|
||||
<div class="card__meta">
|
||||
{% if c.last_at %}<time>{{ c.last_at }}</time>{% endif %}
|
||||
<form method="post" action="{{ url_for('messages.block', member_id=c.other_id) }}"
|
||||
data-confirm="Al bloquear a {{ c.other_name }} ninguno de los dos podrá escribir al otro. ¿Seguro?">
|
||||
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
|
||||
<button class="linkish linkish--danger" type="submit">Bloquear</button>
|
||||
</form>
|
||||
</div>
|
||||
</article>
|
||||
{% endfor %}
|
||||
{% endblock %}
|
||||
@ -268,8 +268,9 @@ def test_every_table_pointing_at_members_is_accounted_for(db):
|
||||
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
|
||||
from apps.board.members import (CLEARED_ON_ERASE, REASSIGNED_ON_ERASE,
|
||||
REMOVED_ON_ERASE)
|
||||
handled = REASSIGNED_ON_ERASE | CLEARED_ON_ERASE | REMOVED_ON_ERASE
|
||||
|
||||
tables = [row["name"] for row in db.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT LIKE 'sqlite_%'")]
|
||||
|
||||
209
apps/board/tests/test_messages.py
Normal file
209
apps/board/tests/test_messages.py
Normal file
@ -0,0 +1,209 @@
|
||||
"""Private messages, unread marks and blocking.
|
||||
|
||||
The access rule is membership of the conversation, and it is worth testing
|
||||
from the outside rather than trusting the query: the failure mode is not an
|
||||
error, it is somebody quietly reading correspondence that is not theirs.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
PNG = b"\x89PNG\r\n\x1a\n" + b"\x00" * 64
|
||||
|
||||
|
||||
def conversation_between(client, post, a, b, sign_in):
|
||||
sign_in(a)
|
||||
response = post("/comunidad/privados/nueva", {"member_id": str(b)})
|
||||
return int(response.headers["Location"].rstrip("/").rsplit("/", 1)[1])
|
||||
|
||||
|
||||
# --- who may read what ----------------------------------------------------
|
||||
|
||||
def test_a_stranger_cannot_read_a_conversation(client, post, make_member, sign_in):
|
||||
"""404 rather than 403, deliberately: a member who is not in a conversation
|
||||
should not be able to tell it apart from one that does not exist."""
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
conversation = conversation_between(client, post, maria, jose, sign_in)
|
||||
|
||||
sign_in(make_member("curiosa"))
|
||||
assert client.get(f"/comunidad/privados/{conversation}").status_code == 404
|
||||
|
||||
|
||||
def test_a_stranger_cannot_send_into_one_either(client, post, make_member, sign_in):
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
conversation = conversation_between(client, post, maria, jose, sign_in)
|
||||
|
||||
sign_in(make_member("curiosa"))
|
||||
response = post(f"/comunidad/privados/{conversation}/enviar", {"body": "Hola"})
|
||||
assert response.status_code == 404
|
||||
|
||||
|
||||
def test_the_two_parties_can(client, db, post, make_member, sign_in):
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
conversation = conversation_between(client, post, maria, jose, sign_in)
|
||||
post(f"/comunidad/privados/{conversation}/enviar", {"body": "Hola José"})
|
||||
|
||||
sign_in(jose)
|
||||
body = client.get(f"/comunidad/privados/{conversation}").get_data(as_text=True)
|
||||
assert "Hola José" in body
|
||||
|
||||
|
||||
def test_opening_the_same_person_twice_reuses_the_conversation(
|
||||
client, db, post, make_member, sign_in):
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
first = conversation_between(client, post, maria, jose, sign_in)
|
||||
second = conversation_between(client, post, maria, jose, sign_in)
|
||||
|
||||
assert first == second
|
||||
assert db.execute("SELECT COUNT(*) AS n FROM conversations").fetchone()["n"] == 1
|
||||
|
||||
|
||||
# --- unread ---------------------------------------------------------------
|
||||
|
||||
def test_unread_is_per_member_and_clears_on_reading(client, post, make_member, sign_in):
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
conversation = conversation_between(client, post, maria, jose, sign_in)
|
||||
post(f"/comunidad/privados/{conversation}/enviar", {"body": "¿Vienes?"})
|
||||
|
||||
badge = '<span class="tag">1</span>'
|
||||
|
||||
# The sender has nothing unread — their own message does not count.
|
||||
assert badge not in client.get("/comunidad/privados").get_data(as_text=True)
|
||||
|
||||
sign_in(jose)
|
||||
assert badge in client.get("/comunidad/privados").get_data(as_text=True)
|
||||
|
||||
client.get(f"/comunidad/privados/{conversation}") # opening marks it read
|
||||
assert badge not in client.get("/comunidad/privados").get_data(as_text=True)
|
||||
|
||||
|
||||
# --- blocking -------------------------------------------------------------
|
||||
|
||||
def test_a_block_stops_both_directions(client, post, make_member, sign_in):
|
||||
"""Symmetric on purpose. A block that silences only the blocked person
|
||||
leaves the blocker able to keep writing, which is a megaphone, not a
|
||||
safety feature."""
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
conversation = conversation_between(client, post, maria, jose, sign_in)
|
||||
|
||||
post(f"/comunidad/privados/bloquear/{jose}") # maria blocks jose
|
||||
|
||||
assert post(f"/comunidad/privados/{conversation}/enviar",
|
||||
{"body": "Otra cosa"}).status_code == 403 # …and maria too
|
||||
sign_in(jose)
|
||||
assert post(f"/comunidad/privados/{conversation}/enviar",
|
||||
{"body": "¿Hola?"}).status_code == 403
|
||||
|
||||
|
||||
def test_a_block_is_enforced_in_the_handler_not_the_template(
|
||||
client, post, make_member, sign_in):
|
||||
"""The form is hidden once blocked, but hiding is a courtesy. Somebody who
|
||||
keeps the old page open, or crafts the request, meets the same refusal."""
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
conversation = conversation_between(client, post, maria, jose, sign_in)
|
||||
sign_in(jose)
|
||||
post(f"/comunidad/privados/bloquear/{maria}")
|
||||
|
||||
sign_in(maria) # never reloaded the page, still has the form
|
||||
assert post(f"/comunidad/privados/{conversation}/enviar",
|
||||
{"body": "Hola"}).status_code == 403
|
||||
|
||||
|
||||
def test_a_blocked_person_cannot_start_a_new_conversation(
|
||||
client, post, make_member, sign_in):
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
sign_in(maria)
|
||||
post(f"/comunidad/privados/bloquear/{jose}")
|
||||
|
||||
sign_in(jose)
|
||||
assert post("/comunidad/privados/nueva", {"member_id": str(maria)}).status_code == 403
|
||||
|
||||
|
||||
def test_unblocking_only_removes_your_own(client, db, post, make_member, sign_in):
|
||||
"""Otherwise the blocked person could lift the block that exists because
|
||||
of them."""
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
sign_in(maria)
|
||||
post(f"/comunidad/privados/bloquear/{jose}")
|
||||
|
||||
sign_in(jose)
|
||||
post(f"/comunidad/privados/desbloquear/{maria}")
|
||||
|
||||
assert db.execute("SELECT COUNT(*) AS n FROM blocks").fetchone()["n"] == 1
|
||||
|
||||
|
||||
def test_a_blocked_person_is_not_offered_in_the_list(client, post, make_member, sign_in):
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
sign_in(maria)
|
||||
post(f"/comunidad/privados/bloquear/{jose}")
|
||||
|
||||
assert "Jose" not in client.get("/comunidad/privados").get_data(as_text=True)
|
||||
|
||||
|
||||
# --- pictures -------------------------------------------------------------
|
||||
|
||||
def test_a_picture_in_a_message_is_private_to_the_two_of_them(
|
||||
app, client, db, post, make_member, sign_in):
|
||||
"""The board's pictures are for every member. These are not, and being
|
||||
signed in is nowhere near enough of a check."""
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
conversation = conversation_between(client, post, maria, jose, sign_in)
|
||||
post(f"/comunidad/privados/{conversation}/enviar",
|
||||
{"body": "Mira", "pictures": (io.BytesIO(PNG), "foto.png")},
|
||||
content_type="multipart/form-data")
|
||||
|
||||
name = db.execute("SELECT stored_name FROM attachments").fetchone()["stored_name"]
|
||||
assert client.get(f"/comunidad/media/{name}").status_code == 200
|
||||
|
||||
sign_in(jose)
|
||||
assert client.get(f"/comunidad/media/{name}").status_code == 200
|
||||
|
||||
sign_in(make_member("curiosa"))
|
||||
assert client.get(f"/comunidad/media/{name}").status_code == 404
|
||||
|
||||
|
||||
def test_an_empty_message_with_no_picture_is_refused(client, db, post, make_member, sign_in):
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
conversation = conversation_between(client, post, maria, jose, sign_in)
|
||||
post(f"/comunidad/privados/{conversation}/enviar", {"body": " "})
|
||||
|
||||
assert db.execute("SELECT COUNT(*) AS n FROM messages").fetchone()["n"] == 0
|
||||
|
||||
|
||||
# --- erasure --------------------------------------------------------------
|
||||
|
||||
def test_erasing_a_member_takes_their_private_messages(
|
||||
app, client, db, post, owner_id, make_member, sign_in):
|
||||
"""Unlike a thread, which survives its author as "Miembro eliminado". A
|
||||
two-party exchange has no remainder to preserve, and keeping half of
|
||||
somebody's erased correspondence is what erasure exists to prevent."""
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
conversation = conversation_between(client, post, maria, jose, sign_in)
|
||||
post(f"/comunidad/privados/{conversation}/enviar",
|
||||
{"body": "Privado", "pictures": (io.BytesIO(PNG), "f.png")},
|
||||
content_type="multipart/form-data")
|
||||
name = db.execute("SELECT stored_name FROM attachments").fetchone()["stored_name"]
|
||||
|
||||
sign_in(owner_id)
|
||||
post(f"/comunidad/miembros/{maria}/eliminar")
|
||||
|
||||
for table in ("conversations", "conversation_members", "messages", "attachments"):
|
||||
assert db.execute(f"SELECT COUNT(*) AS n FROM {table}").fetchone()["n"] == 0, table
|
||||
assert not (Path(app.config["UPLOAD_DIR"]) / name).exists()
|
||||
|
||||
|
||||
def test_erasing_a_member_removes_blocks_either_way(
|
||||
client, db, post, owner_id, make_member, sign_in):
|
||||
maria, jose = make_member("maria"), make_member("jose")
|
||||
sign_in(maria)
|
||||
post(f"/comunidad/privados/bloquear/{jose}")
|
||||
sign_in(jose)
|
||||
post(f"/comunidad/privados/bloquear/{maria}")
|
||||
|
||||
sign_in(owner_id)
|
||||
post(f"/comunidad/miembros/{maria}/eliminar")
|
||||
|
||||
assert db.execute("SELECT COUNT(*) AS n FROM blocks").fetchone()["n"] == 0
|
||||
128
apps/board/tests/test_migrations.py
Normal file
128
apps/board/tests/test_migrations.py
Normal file
@ -0,0 +1,128 @@
|
||||
"""Changing a table that already has rows in it.
|
||||
|
||||
The one thing `schema.sql` cannot do. Every test here builds a database in the
|
||||
*old* shape first and then runs the migration over it, because a migration
|
||||
tested only against a fresh database is tested against the one case it was
|
||||
never needed for.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sqlite3
|
||||
|
||||
import pytest
|
||||
|
||||
from apps.board import migrations
|
||||
|
||||
# The attachments table exactly as it shipped in Phase B, CHECK and all. Kept
|
||||
# here as a literal rather than imported: the point is to reproduce what is on
|
||||
# the server, and schema.sql has moved on.
|
||||
OLD_SCHEMA = """
|
||||
CREATE TABLE members (id INTEGER PRIMARY KEY, gitea_login TEXT);
|
||||
CREATE TABLE threads (id INTEGER PRIMARY KEY, deleted_at TEXT);
|
||||
CREATE TABLE comments (id INTEGER PRIMARY KEY, deleted_at TEXT);
|
||||
CREATE TABLE messages (id INTEGER PRIMARY KEY, deleted_at TEXT);
|
||||
CREATE TABLE attachments (
|
||||
id INTEGER PRIMARY KEY,
|
||||
thread_id INTEGER REFERENCES threads(id),
|
||||
comment_id INTEGER REFERENCES comments(id),
|
||||
stored_name TEXT NOT NULL UNIQUE,
|
||||
original_name TEXT NOT NULL,
|
||||
content_type TEXT NOT NULL,
|
||||
bytes INTEGER NOT NULL,
|
||||
uploaded_by INTEGER NOT NULL REFERENCES members(id),
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
CHECK ((thread_id IS NULL) <> (comment_id IS NULL))
|
||||
);
|
||||
"""
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def old_db(tmp_path):
|
||||
"""A database as it existed before this migration, with real rows in it."""
|
||||
db = sqlite3.connect(tmp_path / "old.db", isolation_level=None)
|
||||
db.row_factory = sqlite3.Row
|
||||
db.executescript(OLD_SCHEMA)
|
||||
db.execute("INSERT INTO members (id, gitea_login) VALUES (1, 'salvador')")
|
||||
db.execute("INSERT INTO threads (id) VALUES (7)")
|
||||
db.execute("INSERT INTO comments (id) VALUES (9)")
|
||||
db.execute(
|
||||
"""INSERT INTO attachments
|
||||
(thread_id, stored_name, original_name, content_type, bytes, uploaded_by)
|
||||
VALUES (7, 'foto-abc123abc123.jpg', 'foto.jpg', 'image/jpeg', 2048, 1)""")
|
||||
db.execute(
|
||||
"""INSERT INTO attachments
|
||||
(comment_id, stored_name, original_name, content_type, bytes, uploaded_by)
|
||||
VALUES (9, 'otra-def456def456.png', 'otra.png', 'image/png', 512, 1)""")
|
||||
yield db
|
||||
db.close()
|
||||
|
||||
|
||||
def test_the_old_check_really_does_block_this(old_db):
|
||||
"""The premise the whole migration rests on, asserted rather than assumed.
|
||||
|
||||
If SQLite ever lets this insert through, the rebuild is unnecessary and
|
||||
this file should shrink to nothing."""
|
||||
old_db.execute("ALTER TABLE attachments ADD COLUMN message_id INTEGER")
|
||||
with pytest.raises(sqlite3.IntegrityError):
|
||||
old_db.execute(
|
||||
"""INSERT INTO attachments
|
||||
(message_id, stored_name, original_name, content_type, bytes, uploaded_by)
|
||||
VALUES (1, 'x-000000000000.png', 'x.png', 'image/png', 1, 1)""")
|
||||
|
||||
|
||||
def test_the_rows_survive(old_db):
|
||||
"""The one that would hurt: Salvador's photo is a real file on a real
|
||||
server, and a migration that drops it loses something nobody can rebuild."""
|
||||
before = old_db.execute(
|
||||
"SELECT stored_name, bytes, uploaded_by FROM attachments ORDER BY id").fetchall()
|
||||
|
||||
migrations.apply(old_db)
|
||||
|
||||
after = old_db.execute(
|
||||
"SELECT stored_name, bytes, uploaded_by FROM attachments ORDER BY id").fetchall()
|
||||
assert [tuple(row) for row in after] == [tuple(row) for row in before]
|
||||
|
||||
|
||||
def test_a_message_can_carry_a_picture_afterwards(old_db):
|
||||
migrations.apply(old_db)
|
||||
old_db.execute("INSERT INTO messages (id) VALUES (3)")
|
||||
old_db.execute(
|
||||
"""INSERT INTO attachments
|
||||
(message_id, stored_name, original_name, content_type, bytes, uploaded_by)
|
||||
VALUES (3, 'nueva-111111111111.png', 'n.png', 'image/png', 10, 1)""")
|
||||
assert old_db.execute(
|
||||
"SELECT message_id FROM attachments WHERE stored_name LIKE 'nueva%'"
|
||||
).fetchone()["message_id"] == 3
|
||||
|
||||
|
||||
def test_an_attachment_still_needs_exactly_one_parent(old_db):
|
||||
"""The rebuilt CHECK has to be as strict as the one it replaced, in three
|
||||
directions instead of two — otherwise the migration quietly removes a
|
||||
constraint while appearing to widen it."""
|
||||
migrations.apply(old_db)
|
||||
for columns, values in [("", ""), ("thread_id, comment_id", "7, 9")]:
|
||||
with pytest.raises(sqlite3.IntegrityError):
|
||||
old_db.execute(
|
||||
f"""INSERT INTO attachments
|
||||
({columns + ', ' if columns else ''}stored_name,
|
||||
original_name, content_type, bytes, uploaded_by)
|
||||
VALUES ({values + ', ' if values else ''}
|
||||
'bad-{len(columns)}00000000000.png', 'b.png',
|
||||
'image/png', 1, 1)""")
|
||||
|
||||
|
||||
def test_running_it_twice_changes_nothing(old_db):
|
||||
first = migrations.apply(old_db)
|
||||
second = migrations.apply(old_db)
|
||||
|
||||
assert first and not second # applied once, then nothing to do
|
||||
assert old_db.execute("PRAGMA user_version").fetchone()[0] == max(
|
||||
number for number, _, _ in migrations.STEPS)
|
||||
|
||||
|
||||
def test_a_fresh_database_skips_it(app, db):
|
||||
"""schema.sql already builds the new shape, so the step must find its work
|
||||
done and return quietly rather than rebuilding a table it just created."""
|
||||
assert "message_id" in {row[1] for row in db.execute("PRAGMA table_info(attachments)")}
|
||||
assert migrations.apply(db) == []
|
||||
@ -28,7 +28,7 @@ import re
|
||||
import secrets
|
||||
from pathlib import Path
|
||||
|
||||
from flask import Blueprint, abort, current_app, send_from_directory
|
||||
from flask import Blueprint, abort, current_app, g, send_from_directory
|
||||
|
||||
from .db import get_db
|
||||
from .security import login_required
|
||||
@ -142,8 +142,20 @@ def stage(files) -> list[dict]:
|
||||
return staged
|
||||
|
||||
|
||||
def remove(stored_name: str) -> None:
|
||||
"""Take a picture off disk. Missing is not an error.
|
||||
|
||||
Called after a row has already gone, so the file being absent means an
|
||||
earlier attempt got this far — which is the state we wanted anyway.
|
||||
"""
|
||||
if not STORED_NAME.match(stored_name):
|
||||
return
|
||||
directory().joinpath(stored_name).unlink(missing_ok=True)
|
||||
|
||||
|
||||
def save(staged: list[dict], member_id: int,
|
||||
thread_id: int | None = None, comment_id: int | None = None) -> None:
|
||||
thread_id: int | None = None, comment_id: int | None = None,
|
||||
message_id: int | None = None) -> None:
|
||||
"""Write the files, then record them. In that order.
|
||||
|
||||
A row pointing at a file that does not exist renders as a broken image on
|
||||
@ -155,11 +167,12 @@ def save(staged: list[dict], member_id: int,
|
||||
(folder / item["stored_name"]).write_bytes(item["data"])
|
||||
get_db().execute(
|
||||
"""INSERT INTO attachments
|
||||
(thread_id, comment_id, stored_name, original_name,
|
||||
content_type, bytes, uploaded_by)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)""",
|
||||
(thread_id, comment_id, item["stored_name"], item["original_name"],
|
||||
item["content_type"], len(item["data"]), member_id),
|
||||
(thread_id, comment_id, message_id, stored_name,
|
||||
original_name, content_type, bytes, uploaded_by)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?)""",
|
||||
(thread_id, comment_id, message_id, item["stored_name"],
|
||||
item["original_name"], item["content_type"], len(item["data"]),
|
||||
member_id),
|
||||
)
|
||||
|
||||
|
||||
@ -171,6 +184,10 @@ def for_comments(comment_ids: list[int]) -> dict[int, list]:
|
||||
return _grouped("comment_id", comment_ids)
|
||||
|
||||
|
||||
def for_messages(message_ids: list[int]) -> dict[int, list]:
|
||||
return _grouped("message_id", message_ids)
|
||||
|
||||
|
||||
def _grouped(column: str, ids: list[int]) -> dict[int, list]:
|
||||
"""One query for a whole page rather than one per comment."""
|
||||
if not ids:
|
||||
@ -187,6 +204,14 @@ def _grouped(column: str, ids: list[int]) -> dict[int, list]:
|
||||
return grouped
|
||||
|
||||
|
||||
def _in_conversation(conversation_id: int) -> bool:
|
||||
return get_db().execute(
|
||||
"""SELECT 1 FROM conversation_members
|
||||
WHERE conversation_id = ? AND member_id = ?""",
|
||||
(conversation_id, g.member["id"]),
|
||||
).fetchone() is not None
|
||||
|
||||
|
||||
@bp.route("/media/<name>")
|
||||
@login_required
|
||||
def serve(name: str):
|
||||
@ -199,16 +224,22 @@ def serve(name: str):
|
||||
if not STORED_NAME.match(name):
|
||||
abort(404)
|
||||
row = get_db().execute(
|
||||
"""SELECT a.content_type
|
||||
"""SELECT a.content_type, m.conversation_id
|
||||
FROM attachments a
|
||||
LEFT JOIN threads t ON t.id = a.thread_id
|
||||
LEFT JOIN comments c ON c.id = a.comment_id
|
||||
LEFT JOIN threads t ON t.id = a.thread_id
|
||||
LEFT JOIN comments c ON c.id = a.comment_id
|
||||
LEFT JOIN messages m ON m.id = a.message_id
|
||||
WHERE a.stored_name = ?
|
||||
AND COALESCE(t.deleted_at, c.deleted_at) IS NULL""",
|
||||
AND COALESCE(t.deleted_at, c.deleted_at, m.deleted_at) IS NULL""",
|
||||
(name,),
|
||||
).fetchone()
|
||||
if row is None:
|
||||
abort(404)
|
||||
# A picture on the board is for every member; one in a private message is
|
||||
# for the two people in that conversation and nobody else. Being signed in
|
||||
# is the whole check for the first and not nearly enough for the second.
|
||||
if row["conversation_id"] is not None and not _in_conversation(row["conversation_id"]):
|
||||
abort(404)
|
||||
# mimetype from our own column, never guessed from the name on disk, and
|
||||
# paired with the X-Content-Type-Options: nosniff set in app.py.
|
||||
return send_from_directory(directory(), name, mimetype=row["content_type"])
|
||||
|
||||
@ -613,6 +613,51 @@ Limits: 4 images per message, and `BOARD_UPLOAD_MAX_BYTES` (8MB by default)
|
||||
each. Adding a picture to a post *after* publishing it means posting a reply —
|
||||
editing changes the words, and leaves the pictures alone.
|
||||
|
||||
### 11.10 Private messages (`/comunidad/privados/`)
|
||||
|
||||
An inbox between two members: conversations, unread badges, photos, blocking.
|
||||
Deliberately not live chat — that needs a connection held open per signed-in
|
||||
member, which gunicorn's sync workers cannot do, and it would be the first
|
||||
thing on this box with a real scaling limit.
|
||||
|
||||
**Blocking is symmetric.** One block stops messages in both directions, and
|
||||
either person can only remove their own. A block that silenced just the blocked
|
||||
person would leave the blocker able to keep writing, which is a megaphone
|
||||
rather than a safety feature.
|
||||
|
||||
**Erasing a member deletes their private messages, both sides.** A thread
|
||||
outlives its author as *Miembro eliminado*, because other people replied and
|
||||
the conversation would lose its shape; a two-party exchange has no such
|
||||
remainder. This does destroy the other person's copy — the uncomfortable half
|
||||
of the choice, and deliberate.
|
||||
|
||||
### 11.11 Schema changes: `PRAGMA user_version`
|
||||
|
||||
`schema.sql` is all `CREATE TABLE IF NOT EXISTS`, which handles exactly one
|
||||
kind of change — a brand-new table — and silently ignores every other. Until
|
||||
private messages, every change happened to be a new table, so nothing noticed.
|
||||
|
||||
`apps/board/migrations.py` holds numbered steps applied once each, in order,
|
||||
recorded in SQLite's own `user_version`. `init_db` runs the schema first and
|
||||
the migrations second: on an empty database the schema builds the current
|
||||
shape and each step finds its work done; on an existing one the schema adds
|
||||
what is new and the steps fix up what it could not touch.
|
||||
|
||||
**Never edit a step that has shipped**, and never renumber one. A server that
|
||||
has run it will not run it again, so a correction is a new step.
|
||||
|
||||
The first step rebuilds `attachments` so a picture can belong to a private
|
||||
message. It has to be a rebuild rather than an `ALTER`, because the table
|
||||
carries a CHECK constraint and SQLite has no `DROP CONSTRAINT` — adding the
|
||||
column works and the next insert is refused by a constraint that can no longer
|
||||
be removed. That is tested against a database built in the old shape, with rows
|
||||
in it, because a migration tested only on a fresh database is tested against
|
||||
the one case it was never needed for.
|
||||
|
||||
**After deploying this, check that an existing photo still renders.** That is
|
||||
the proof the rebuild kept real rows. Back up first — `scripts/backup-board.sh`
|
||||
— as with any migration.
|
||||
|
||||
## 12. Make Gitea look like the site
|
||||
|
||||
Members sign in to `/comunidad/` through Gitea, so Gitea's sign-in form and its
|
||||
|
||||
Loading…
Reference in New Issue
Block a user