fix(feed): filter on the server, exactly, and keep banned uploads out of the chips
Filtering was split across two independent client-side states and applied to whatever page 1 happened to hold, by caption SUBSTRING. So a tag chip selected in the list view was silently still applied in the grid without being shown; a filter matched photos whose caption merely contained the text; and anything past the first page was invisible to it. Verified against the seeded data: `hashtag=tanz` returned 6 photos by substring, 1 by tag. `FeedQuery` now carries `hashtag` (single, list view), `hashtags` (CSV, OR'd, grid chips) and `uploader` (exact, AND'd), normalised through one function that trims, strips `#`, lowercases and dedupes, and yields None when empty — so an empty filter means "no filter", never "match nothing". The two SQL branches collapse into one with `h.tag = ANY($4)`. Tag-OR plus tag+user-AND is a specified feature, not an accident: `e2e/specs/03-feed/filter-search.spec.ts` and USER_JOURNEYS §8 pin it, which is why the semantics moved to the server rather than being simplified away. Tags travel as CSV safely because the backend restricts them to ASCII alphanumerics and `_`; `uploader` stays a single exact parameter because a display name can contain a comma. New `GET /api/v1/uploaders` reads `v_feed`, so banned and hidden uploaders are excluded for free. Migration 021 gives `v_hashtag_counts` the same treatment. It counted every upload regardless of the uploader's ban state, so banning a guest left their tags in the chip list as ghost filters that lead to an empty feed. Verified: after banning the guest who owned all six `tanz*` photos, the chips went 6 -> 0. `?limit=-5` returned a 500 — only the upper bound was clamped, so Postgres was asked for `LIMIT -4`. Clamped at both ends. `is_banned` is added to `/me/context` so the client can show a read-only notice instead of letting a banned guest discover the ban one 403 toast at a time. `add_comment` sorts and dedupes hashtags on the normalised key, matching the upload path — the two disagreed, which is a lock-ordering deadlock between concurrent upserts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
13
backend/migrations/021_hashtag_counts_respect_bans.down.sql
Normal file
13
backend/migrations/021_hashtag_counts_respect_bans.down.sql
Normal file
@@ -0,0 +1,13 @@
|
||||
-- Restore the pre-021 definition (no ban/hide filtering) exactly as 004 created it.
|
||||
DROP VIEW IF EXISTS v_hashtag_counts;
|
||||
|
||||
CREATE VIEW v_hashtag_counts AS
|
||||
SELECT
|
||||
h.event_id,
|
||||
h.tag,
|
||||
COUNT(uh.upload_id) AS upload_count
|
||||
FROM hashtag h
|
||||
JOIN upload_hashtag uh ON uh.hashtag_id = h.id
|
||||
JOIN upload u ON u.id = uh.upload_id AND u.deleted_at IS NULL
|
||||
GROUP BY h.event_id, h.id, h.tag
|
||||
ORDER BY upload_count DESC;
|
||||
31
backend/migrations/021_hashtag_counts_respect_bans.up.sql
Normal file
31
backend/migrations/021_hashtag_counts_respect_bans.up.sql
Normal file
@@ -0,0 +1,31 @@
|
||||
-- v_hashtag_counts: apply the same visibility rules as v_feed.
|
||||
--
|
||||
-- The chip row and the grid's tag picker are both fed by this view, but it has only ever
|
||||
-- filtered `u.deleted_at IS NULL`. Migration 011 added ban/hide filtering to the feed and
|
||||
-- never reached here, so the two disagreed about which uploads exist:
|
||||
--
|
||||
-- * A host bans a guest who posted 3 of the 12 `#tanz` photos. The chip keeps reading
|
||||
-- "#tanz 12"; tapping it returns 9. The count is presented as authoritative and is not.
|
||||
-- * A tag used ONLY by a banned or hidden guest stays in the chip row and in the tag
|
||||
-- picker as a selectable option that leads to an empty feed — a ghost filter that
|
||||
-- cannot be cleared because there is nothing wrong with it to see.
|
||||
--
|
||||
-- Bans are exactly the moment a host is watching these numbers to confirm the moderation
|
||||
-- took effect, so a stale count reads as "the ban didn't work".
|
||||
--
|
||||
-- Same predicate as v_feed (see 016_display_derivative.up.sql), joined through `user`.
|
||||
DROP VIEW IF EXISTS v_hashtag_counts;
|
||||
|
||||
CREATE VIEW v_hashtag_counts AS
|
||||
SELECT
|
||||
h.event_id,
|
||||
h.tag,
|
||||
COUNT(uh.upload_id) AS upload_count
|
||||
FROM hashtag h
|
||||
JOIN upload_hashtag uh ON uh.hashtag_id = h.id
|
||||
JOIN upload u ON u.id = uh.upload_id AND u.deleted_at IS NULL
|
||||
JOIN "user" usr ON usr.id = u.user_id
|
||||
WHERE usr.uploads_hidden = FALSE
|
||||
AND usr.is_banned = FALSE
|
||||
GROUP BY h.event_id, h.id, h.tag
|
||||
ORDER BY upload_count DESC;
|
||||
@@ -15,7 +15,43 @@ use crate::state::AppState;
|
||||
pub struct FeedQuery {
|
||||
pub cursor: Option<Uuid>,
|
||||
pub limit: Option<i64>,
|
||||
/// Single tag (list view). Kept alongside `hashtags` so existing callers keep working.
|
||||
pub hashtag: Option<String>,
|
||||
/// Comma-separated tags, combined with **OR** — the grid's chip semantics
|
||||
/// (USER_JOURNEYS §8). Filtering moved server-side because the client could only ever
|
||||
/// filter the pages it had already loaded: with page 1 = 20 items out of a 1000-photo
|
||||
/// event, selecting a tag showed a handful of tiles and looked complete. Matching is
|
||||
/// now the exact `hashtag` row in both views, so the grid and the list can no longer
|
||||
/// disagree about which photos carry a tag (the client matched a caption SUBSTRING, so
|
||||
/// `#tanz` also matched `#tanzflaeche`).
|
||||
pub hashtags: Option<String>,
|
||||
/// Exact uploader display name, combined with the tag group using **AND**.
|
||||
pub uploader: Option<String>,
|
||||
}
|
||||
|
||||
/// Merge the single-tag and CSV tag params into one normalised, de-duplicated list.
|
||||
///
|
||||
/// Normalisation mirrors `Hashtag::upsert` exactly (trim, drop a leading `#`, lowercase), so
|
||||
/// a chip built from a display string like `#Tanz` matches the stored `tanz` row. Returns
|
||||
/// `None` when no usable tag was supplied, which makes the SQL predicate a no-op — an empty
|
||||
/// list must mean "no tag filter", never "match nothing".
|
||||
fn normalize_tags(single: Option<&str>, csv: Option<&str>) -> Option<Vec<String>> {
|
||||
let mut out: Vec<String> = Vec::new();
|
||||
let mut push = |raw: &str| {
|
||||
let t = raw.trim().trim_start_matches('#').to_lowercase();
|
||||
if !t.is_empty() && !out.contains(&t) {
|
||||
out.push(t);
|
||||
}
|
||||
};
|
||||
if let Some(s) = single {
|
||||
push(s);
|
||||
}
|
||||
if let Some(s) = csv {
|
||||
for part in s.split(',') {
|
||||
push(part);
|
||||
}
|
||||
}
|
||||
if out.is_empty() { None } else { Some(out) }
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
@@ -80,7 +116,9 @@ pub async fn feed(
|
||||
}
|
||||
}
|
||||
|
||||
let limit = q.limit.unwrap_or(20).min(100);
|
||||
// Clamped at BOTH ends: only the upper bound was enforced, so `?limit=-5` reached Postgres
|
||||
// as `LIMIT -4` and answered a hand-written URL with a 500.
|
||||
let limit = q.limit.unwrap_or(20).clamp(1, 100);
|
||||
|
||||
// Resolve the cursor to a (created_at, id) position. The pair is compared as a
|
||||
// tuple so ties on created_at break on id — keyset pagination on created_at
|
||||
@@ -93,44 +131,42 @@ pub async fn feed(
|
||||
None => (None, None),
|
||||
};
|
||||
|
||||
let rows = if let Some(hashtag) = &q.hashtag {
|
||||
let tag = hashtag.trim().trim_start_matches('#').to_lowercase();
|
||||
sqlx::query_as::<_, FeedRow>(
|
||||
"SELECT v.id, v.user_id, v.uploader_name, v.preview_path, v.thumbnail_path,
|
||||
v.display_path, v.mime_type, v.caption, v.like_count, v.comment_count,
|
||||
v.created_at
|
||||
FROM v_feed v
|
||||
JOIN upload_hashtag uh ON uh.upload_id = v.id
|
||||
JOIN hashtag h ON h.id = uh.hashtag_id AND h.tag = $1
|
||||
WHERE v.event_id = $2
|
||||
AND ($3::timestamptz IS NULL OR (v.created_at, v.id) < ($3, $4))
|
||||
ORDER BY v.created_at DESC, v.id DESC
|
||||
LIMIT $5",
|
||||
)
|
||||
.bind(&tag)
|
||||
.bind(auth.event_id)
|
||||
.bind(cursor_time)
|
||||
.bind(cursor_id)
|
||||
.bind(limit + 1)
|
||||
.fetch_all(&state.pool)
|
||||
.await?
|
||||
} else {
|
||||
sqlx::query_as::<_, FeedRow>(
|
||||
"SELECT id, user_id, uploader_name, preview_path, thumbnail_path,
|
||||
display_path, mime_type, caption, like_count, comment_count, created_at
|
||||
FROM v_feed
|
||||
WHERE event_id = $1
|
||||
AND ($2::timestamptz IS NULL OR (created_at, id) < ($2, $3))
|
||||
ORDER BY created_at DESC, id DESC
|
||||
LIMIT $4",
|
||||
)
|
||||
.bind(auth.event_id)
|
||||
.bind(cursor_time)
|
||||
.bind(cursor_id)
|
||||
.bind(limit + 1)
|
||||
.fetch_all(&state.pool)
|
||||
.await?
|
||||
};
|
||||
// Tags from either param, normalised the same way `Hashtag::upsert` stores them
|
||||
// (trimmed, leading `#` dropped, lowercased) so the comparison is exact.
|
||||
let tags = normalize_tags(q.hashtag.as_deref(), q.hashtags.as_deref());
|
||||
let uploader = q
|
||||
.uploader
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|s| !s.is_empty());
|
||||
|
||||
// ONE statement for every combination, rather than a branch per filter. `EXISTS` with
|
||||
// `= ANY($4)` gives OR across the tag group without the row multiplication a JOIN would
|
||||
// cause when a photo carries two selected tags; the uploader predicate ANDs on top. Both
|
||||
// are no-ops when NULL, so the unfiltered feed takes the same path.
|
||||
let rows = sqlx::query_as::<_, FeedRow>(
|
||||
"SELECT v.id, v.user_id, v.uploader_name, v.preview_path, v.thumbnail_path,
|
||||
v.display_path, v.mime_type, v.caption, v.like_count, v.comment_count,
|
||||
v.created_at
|
||||
FROM v_feed v
|
||||
WHERE v.event_id = $1
|
||||
AND ($2::timestamptz IS NULL OR (v.created_at, v.id) < ($2, $3))
|
||||
AND ($4::text[] IS NULL OR EXISTS (
|
||||
SELECT 1 FROM upload_hashtag uh
|
||||
JOIN hashtag h ON h.id = uh.hashtag_id
|
||||
WHERE uh.upload_id = v.id AND h.tag = ANY($4)))
|
||||
AND ($5::text IS NULL OR v.uploader_name = $5)
|
||||
ORDER BY v.created_at DESC, v.id DESC
|
||||
LIMIT $6",
|
||||
)
|
||||
.bind(auth.event_id)
|
||||
.bind(cursor_time)
|
||||
.bind(cursor_id)
|
||||
.bind(tags.as_deref())
|
||||
.bind(uploader)
|
||||
.bind(limit + 1)
|
||||
.fetch_all(&state.pool)
|
||||
.await?;
|
||||
|
||||
let has_more = rows.len() as i64 > limit;
|
||||
let rows: Vec<FeedRow> = rows.into_iter().take(limit as usize).collect();
|
||||
@@ -357,6 +393,32 @@ pub async fn hashtags(
|
||||
))
|
||||
}
|
||||
|
||||
/// Every uploader who has at least one visible upload, for the grid's "Nutzer suchen" picker.
|
||||
///
|
||||
/// The picker used to derive names from the uploads currently in memory — page 1, 20 items —
|
||||
/// so typing a guest's name found nothing whenever their photos happened to sit below the
|
||||
/// fold, which reads as "search is broken". This is the authoritative list.
|
||||
///
|
||||
/// Reads `v_feed`, so it inherits exactly the feed's visibility rules: soft-deleted uploads,
|
||||
/// banned uploaders and hidden uploaders are all excluded, and a guest who has not uploaded
|
||||
/// anything never appears. Uncapped on purpose — one short string per uploader, bounded by
|
||||
/// the guest count, and truncating it would reintroduce the very bug this replaces.
|
||||
/// Deliberately NOT the host-only `/host/users` route: that one lists every joined guest and
|
||||
/// exposes moderation state.
|
||||
pub async fn uploaders(
|
||||
State(state): State<AppState>,
|
||||
auth: AuthUser,
|
||||
) -> Result<Json<Vec<String>>, AppError> {
|
||||
let rows: Vec<(String,)> = sqlx::query_as(
|
||||
"SELECT DISTINCT uploader_name FROM v_feed WHERE event_id = $1 ORDER BY uploader_name",
|
||||
)
|
||||
.bind(auth.event_id)
|
||||
.fetch_all(&state.pool)
|
||||
.await?;
|
||||
|
||||
Ok(Json(rows.into_iter().map(|(name,)| name).collect()))
|
||||
}
|
||||
|
||||
/// Resolve a cursor id to its `(created_at, id)` position. Both are needed:
|
||||
/// `created_at` alone isn't unique, so pagination must break ties on `id` to
|
||||
/// avoid silently dropping rows that share a timestamp across a page boundary.
|
||||
@@ -388,3 +450,41 @@ async fn get_liked_set(
|
||||
|
||||
rows.into_iter().map(|r| r.0).collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::normalize_tags;
|
||||
|
||||
/// The chips carry display strings (`#Tanz`), the `hashtag` table stores `tanz`. If these
|
||||
/// two drift the filter silently returns nothing, which is indistinguishable from "no
|
||||
/// photos have this tag" — so pin the normalisation to `Hashtag::upsert`'s rule.
|
||||
#[test]
|
||||
fn tags_are_normalised_like_upsert_stores_them() {
|
||||
assert_eq!(
|
||||
normalize_tags(Some("#Tanz"), None),
|
||||
Some(vec!["tanz".to_string()])
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_tags(None, Some(" #Buffet , reden ")),
|
||||
Some(vec!["buffet".to_string(), "reden".to_string()])
|
||||
);
|
||||
}
|
||||
|
||||
/// An empty list must mean "no filter", never "match nothing" — returning `Some(vec![])`
|
||||
/// would make `= ANY('{}')` false for every row and blank the feed.
|
||||
#[test]
|
||||
fn blank_input_disables_the_filter() {
|
||||
assert_eq!(normalize_tags(None, None), None);
|
||||
assert_eq!(normalize_tags(Some(" "), Some(" , ,#")), None);
|
||||
}
|
||||
|
||||
/// Both params feed one list, de-duplicated: the list view sends `hashtag`, the grid sends
|
||||
/// `hashtags`, and carrying a filter across views can legitimately set both to the same tag.
|
||||
#[test]
|
||||
fn single_and_csv_merge_without_duplicates() {
|
||||
assert_eq!(
|
||||
normalize_tags(Some("tanz"), Some("tanz,buffet")),
|
||||
Some(vec!["tanz".to_string(), "buffet".to_string()])
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -75,6 +75,14 @@ pub struct MeContextDto {
|
||||
/// The gallery has been released and the export snapshotted — uploads are permanently
|
||||
/// closed for this run (release ⇒ lock, and reopening regenerates).
|
||||
pub gallery_released: bool,
|
||||
/// This guest is banned: a deliberately READ-ONLY ban (see `handlers/host.rs`) — they keep
|
||||
/// the feed and the keepsake, but every write is refused.
|
||||
///
|
||||
/// Exposed so the UI can SAY so. Without it the client had no idea, so the upload button,
|
||||
/// the like button and "Löschen" all rendered enabled and returned 403 "Du bist gesperrt."
|
||||
/// on every tap — a guest tapping upload repeatedly with nobody to ask. The lock case
|
||||
/// (`uploads_locked`) has always been surfaced for exactly this reason; a ban was not.
|
||||
pub is_banned: bool,
|
||||
}
|
||||
|
||||
pub async fn get_context(
|
||||
@@ -110,5 +118,6 @@ pub async fn get_context(
|
||||
storage_quota_enabled,
|
||||
uploads_locked,
|
||||
gallery_released,
|
||||
is_banned: user.is_banned,
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -195,7 +195,14 @@ pub async fn add_comment(
|
||||
|
||||
// Insert the comment and link its hashtags atomically, so a crash mid-loop
|
||||
// can't leave a committed comment with only some of its tags indexed.
|
||||
let tags = hashtag::extract_hashtags(text);
|
||||
let mut tags = hashtag::extract_hashtags(text);
|
||||
// Deterministic lock order, matching the upload path. `Hashtag::upsert` takes row locks,
|
||||
// so two transactions touching the same two tags in OPPOSITE order deadlock — Postgres
|
||||
// aborts one after ~1s and that guest's comment 500s. `extract_hashtags` returns them in
|
||||
// text order, which is exactly the unordered case. Sort on the NORMALISED form, because
|
||||
// that is the key `upsert` locks on.
|
||||
tags.sort_by_key(|t| t.trim().trim_start_matches('#').to_lowercase());
|
||||
tags.dedup_by_key(|t| t.trim().trim_start_matches('#').to_lowercase());
|
||||
let mut tx = state.pool.begin().await?;
|
||||
let comment = Comment::create(&mut *tx, upload_id, auth.user_id, text).await?;
|
||||
for tag in &tags {
|
||||
|
||||
Reference in New Issue
Block a user