From 1fb318f6a1d9f54ed112fcd238a2152d02d7a22b Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Tue, 16 Jun 2026 19:14:47 +0200 Subject: [PATCH] fixed deletion --- backend-rs/src/db.rs | 21 + backend-rs/src/entities/user.rs | 6 + backend-rs/src/routes/users.rs | 78 ++- community-redesign.md | 95 ++++ frontend/src/api/queries/useCommunityFeed.ts | 10 + frontend/src/i18n.ts | 12 + frontend/src/pages/CommunityFeed.tsx | 483 +++++++++++-------- 7 files changed, 493 insertions(+), 212 deletions(-) create mode 100644 community-redesign.md diff --git a/backend-rs/src/db.rs b/backend-rs/src/db.rs index 7ad6033..99c3cfb 100644 --- a/backend-rs/src/db.rs +++ b/backend-rs/src/db.rs @@ -52,6 +52,27 @@ async fn ensure_schema(conn: &DatabaseConnection) -> anyhow::Result<()> { "ALTER TABLE api_flavor ADD COLUMN aliases TEXT", ) .await?; + // Account-deletion (GDPR "right to erasure") flow gets its own dedicated, + // DB-persisted code + expiry. Previously it reused `email_confirmation_code` + // and an in-process TTL, so a concurrent email-confirm/password-reset flow + // could silently clobber the deletion code (the column ended up empty) and a + // backend restart dropped the expiry — both could block a user from deleting + // their account. Dedicated, persisted columns make the flow restart-safe, + // multi-instance-safe, and immune to cross-flow collisions. + add_column_if_missing( + conn, + "api_user", + "deletion_code", + "ALTER TABLE api_user ADD COLUMN deletion_code TEXT", + ) + .await?; + add_column_if_missing( + conn, + "api_user", + "deletion_code_expires", + "ALTER TABLE api_user ADD COLUMN deletion_code_expires DATETIME", + ) + .await?; Ok(()) } diff --git a/backend-rs/src/entities/user.rs b/backend-rs/src/entities/user.rs index 5770bb8..8d625af 100644 --- a/backend-rs/src/entities/user.rs +++ b/backend-rs/src/entities/user.rs @@ -26,6 +26,12 @@ pub struct Model { pub language: String, pub selected_banner_id: Option, pub drawer_anchor: String, + /// Dedicated account-deletion confirmation code + its expiry. Kept separate + /// from `email_confirmation_code` so other code-driven flows (email confirm, + /// password reset) can never clobber an in-flight deletion code, and + /// persisted so the 15-min TTL survives restarts / spans instances. + pub deletion_code: Option, + pub deletion_code_expires: Option, } #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] diff --git a/backend-rs/src/routes/users.rs b/backend-rs/src/routes/users.rs index b8bc8fe..04d6ec6 100644 --- a/backend-rs/src/routes/users.rs +++ b/backend-rs/src/routes/users.rs @@ -16,7 +16,7 @@ use axum::extract::{ConnectInfo, Multipart, Path, Query, State}; use axum::http::{HeaderMap, StatusCode}; use axum::routing::{get, patch, post}; use axum::{Json, Router}; -use chrono::Datelike; +use chrono::{Datelike, Duration}; use sea_orm::ActiveValue::Set; use sea_orm::{ ActiveModelTrait, ColumnTrait, ConnectionTrait, DbBackend, EntityTrait, PaginatorTrait, @@ -147,7 +147,11 @@ fn err_json(status: StatusCode, body: Value) -> ApiError { /// to be constant-time to avoid leaking a correct prefix. fn code_matches(stored: Option<&str>, provided: Option<&str>) -> bool { match (stored, provided) { - (Some(s), Some(p)) if s.len() == p.len() => s.as_bytes().ct_eq(p.as_bytes()).into(), + // An empty stored code is never a valid secret: it means the column was + // cleared/never set, so it must not be satisfiable by an empty input. + (Some(s), Some(p)) if !s.is_empty() && s.len() == p.len() => { + s.as_bytes().ct_eq(p.as_bytes()).into() + } _ => false, } } @@ -995,10 +999,15 @@ async fn request_account_deletion( let code = gen_code(); let uname = user.username.clone(); let email = user.email.clone(); + let expires = crate::datetime::now_micros() + + Duration::from_std(throttle::CODE_TTL).expect("CODE_TTL fits in chrono::Duration"); let mut active: user::ActiveModel = user.into(); - active.email_confirmation_code = Set(Some(code.clone())); + // Dedicated, persisted columns — never touch `email_confirmation_code`, so a + // concurrent email/password flow can't clobber the in-flight deletion code, + // and the TTL survives restarts instead of living only in process memory. + active.deletion_code = Set(Some(code.clone())); + active.deletion_code_expires = Set(Some(expires)); active.update(&state.db).await?; - state.security.record_code(&del_code_key(uid)); crate::email::spawn_mail( state.config.email.clone(), @@ -1018,26 +1027,33 @@ async fn confirm_account_deletion( AuthUser(uid): AuthUser, body: Bytes, ) -> ApiResult> { - let code_key = del_code_key(uid); - state - .security - .check_rate(&format!("attempt:{code_key}"), throttle::CODE_ATTEMPTS)?; - if state.security.code_expired(&code_key) { + // Per-process attempt throttle (defence-in-depth; nginx limits sit in front). + state.security.check_rate( + &format!("attempt:{}", del_code_key(uid)), + throttle::CODE_ATTEMPTS, + )?; + let user = load_user(&state, uid).await?; + let data = parse_body(&body); + let code = data.get("code").and_then(|v| v.as_str()); + + // Validate the dedicated, DB-persisted deletion code + expiry. Both checks + // read from the DB so they're restart-safe and consistent across instances. + let expired = user + .deletion_code_expires + .is_none_or(|exp| exp < crate::datetime::now_micros()); + if expired { return Err(err_json( StatusCode::BAD_REQUEST, json!({ "error": "Deletion code has expired. Please request a new one." }), )); } - let user = load_user(&state, uid).await?; - let data = parse_body(&body); - let code = data.get("code").and_then(|v| v.as_str()); - if !code_matches(user.email_confirmation_code.as_deref(), code) { + if !code_matches(user.deletion_code.as_deref(), code) { return Err(err_json( StatusCode::BAD_REQUEST, json!({ "error": "Invalid or expired code" }), )); } - state.security.clear_code(&code_key); + // delete_user_cascade removes the user row (and the code with it) atomically. delete_user_cascade(&state, uid).await?; Ok(Json(json!({ "status": "Account deleted" }))) } @@ -1548,3 +1564,37 @@ mod dashboard_stats_tests { assert_eq!(fav.slug, "apple"); } } + +#[cfg(test)] +mod deletion_code_tests { + use super::*; + + // The account-deletion confirm endpoint gates on `code_matches`. Guard the + // exact failure that once blocked the GDPR erasure flow: an empty stored + // code (column was clobbered/never set) must never satisfy any input. + #[test] + fn empty_or_missing_stored_code_never_matches() { + assert!(!code_matches(None, Some("123456"))); + assert!(!code_matches(Some(""), Some(""))); + assert!(!code_matches(Some("123456"), None)); + assert!(!code_matches(None, None)); + } + + #[test] + fn only_exact_equal_length_code_matches() { + assert!(code_matches(Some("123456"), Some("123456"))); + assert!(!code_matches(Some("123456"), Some("123450"))); + assert!(!code_matches(Some("123456"), Some("12345"))); // length mismatch + } + + // Mirror the inline expiry guard in `confirm_account_deletion`: a missing or + // past expiry is treated as expired; a future expiry is valid. + #[test] + fn expiry_guard_treats_missing_or_past_as_expired() { + let now = crate::datetime::now_micros(); + let is_expired = |exp: Option| exp.is_none_or(|e| e < now); + assert!(is_expired(None)); + assert!(is_expired(Some(now - Duration::minutes(1)))); + assert!(!is_expired(Some(now + Duration::minutes(1)))); + } +} diff --git a/community-redesign.md b/community-redesign.md new file mode 100644 index 0000000..009bd57 --- /dev/null +++ b/community-redesign.md @@ -0,0 +1,95 @@ +# Community Tab Redesign + +The Community tab is the oldest part of the site and doesn't fit the robust, +professional design of the rest. This document tracks the redesign. + +## Starting state + +`frontend/src/pages/CommunityFeed.tsx` — a paginated feed of ratings **from +followed users only**, with a single sidebar card ("Circle's Top Rated"). Replies +are hidden behind a per-card collapse. Loading is a bare center spinner. + +### Core problems + +- **Cold start is dead.** No follows → empty feed → no reason to return. The + backend `ratings/feed/` is gated on the follow graph and returns nothing for + users who follow no one. +- **Single content type.** Only ratings. No new-flavor drops, profile comments, + follow events, or milestones. +- **No engagement loop.** No reactions/likes. Replies buried in a collapse. The + feed is a read-only wall. +- **No filtering/sort.** Can't sort recent/top or filter by category, even though + the payload already carries `category_name`/`category_slug`. +- **Sparse sidebar.** One card wastes the 4-column rail. + +--- + +## Phase 1 — frontend-only (DONE) + +No backend changes. Reuses the existing `GET /api/ratings/recent/` endpoint +(global, AllowAny, 10 newest ratings with a comment). + +- **Following | Discover tabs.** Discover surfaces the whole community's newest + ratings, so the page is useful even when you follow nobody — fixes the + cold-start hole. Discover is fetched lazily (`enabled: tab === 'discover'`). +- **Skeleton loader** (`FeedSkeleton`) replaces the center spinner — matches the + professional look of the rest of the app. +- **Inline reply preview.** The latest reply shows compact under each card; click + expands the full thread + compose box. Replies feel like conversation, not + archive. +- **Smarter empty states.** Empty Following nudges to Discover (button switches + tab); empty Discover nudges to explore flavors. +- **i18n** EN + DE keys added under `community.*`. + +### Files touched + +- `frontend/src/api/queries/useCommunityFeed.ts` — added `useRecentRatings`. +- `frontend/src/pages/CommunityFeed.tsx` — tabs, skeleton, inline reply preview, + empty-state logic. +- `frontend/src/i18n.ts` — `tabFollowing`, `tabDiscover`, `discoverSubtitle`, + `discoverEmpty`, `discoverEmptyHint`, `browseDiscover` (EN + DE). + +--- + +## Phase 2 — needs backend + +### 1. Reactions (highest engagement lever) + +Lightweight 👍 / emoji on ratings. Social proof, drives return visits. + +- New `rating_reaction` table (`user_id`, `rating_id`, `kind`, `created_at`; + unique on `user_id + rating_id + kind`). +- Endpoints: `POST /api/ratings/{id}/react/`, `DELETE /api/ratings/{id}/react/`. +- Include reaction counts + the caller's own reaction in `RatingOut`. +- Frontend: reaction bar on each feed card; optimistic toggle. + +### 2. Discover feed with sort + pagination + +`recent/` is fixed at 10 and unpaginated. Promote Discover to a real feed. + +- Extend `ratings/feed/` (or a new `ratings/discover/`) to accept + `?sort=recent|top` and `?category=` and `?page=`. +- Frontend: sticky filter bar with `Recent | Top | `. Reuse the + `category_slug` already in the payload. + +### 3. Who-to-follow + +Sidebar card suggesting active raters the user doesn't follow yet. + +- `GET /api/users/suggested/` — most-active raters excluding self + already-followed. +- Frontend: avatar + name + follow button; optimistic follow. + +### 4. Richer activity types + +Make the feed alive when friends are quiet. + +- New-flavor drops (from the `sync_flavors` job) and milestones + ("X hit 100 ratings") as feed items. +- Likely a unified activity payload or a separate `activity/` endpoint. + +--- + +## Suggested order + +Phase 2.1 (reactions) and 2.2 (Discover sort/filter) give the most UX lift per +unit of backend work. 2.3 and 2.4 follow. diff --git a/frontend/src/api/queries/useCommunityFeed.ts b/frontend/src/api/queries/useCommunityFeed.ts index 19bb798..223f902 100644 --- a/frontend/src/api/queries/useCommunityFeed.ts +++ b/frontend/src/api/queries/useCommunityFeed.ts @@ -60,6 +60,16 @@ export const useCommunityFeed = (page: number) => placeholderData: (prev) => prev, }); +export const useRecentRatings = (enabled = true) => + useQuery({ + queryKey: ['recentRatings'], + queryFn: async (): Promise => { + const res = await api.get('ratings/recent/'); + return Array.isArray(res.data) ? res.data : (res.data.results ?? []); + }, + enabled, + }); + export const useFollowingList = () => useQuery({ queryKey: ['followingList'], diff --git a/frontend/src/i18n.ts b/frontend/src/i18n.ts index fc48d54..27da4c5 100644 --- a/frontend/src/i18n.ts +++ b/frontend/src/i18n.ts @@ -293,6 +293,12 @@ const resources = { searchFriends: 'Search friends...', writeReply: 'Add a reply...', rated: 'rated', + tabFollowing: 'Following', + tabDiscover: 'Discover', + discoverSubtitle: 'See what the whole community is rating right now.', + discoverEmpty: 'No recent activity yet.', + discoverEmptyHint: 'Be the first to rate a flavor!', + browseDiscover: 'Browse Discover', }, support: { title: 'Help & Support', @@ -830,6 +836,12 @@ const resources = { searchFriends: 'Freunde suchen...', writeReply: 'Eine Antwort hinzufügen...', rated: 'bewertete', + tabFollowing: 'Folge ich', + tabDiscover: 'Entdecken', + discoverSubtitle: 'Sieh, was die ganze Community gerade bewertet.', + discoverEmpty: 'Noch keine Aktivität.', + discoverEmptyHint: 'Sei der Erste, der einen Geschmack bewertet!', + browseDiscover: 'Entdecken durchstöbern', }, support: { title: 'Hilfe & Support', diff --git a/frontend/src/pages/CommunityFeed.tsx b/frontend/src/pages/CommunityFeed.tsx index 15a3d72..2d3b409 100644 --- a/frontend/src/pages/CommunityFeed.tsx +++ b/frontend/src/pages/CommunityFeed.tsx @@ -4,7 +4,6 @@ import { Box, CardContent, Avatar, - CircularProgress, Divider, Button, Grid, @@ -15,14 +14,20 @@ import { IconButton, Collapse, Link as MuiLink, + Tabs, + Tab, + Skeleton, } from '@mui/material'; import CommentIcon from '@mui/icons-material/Comment'; import SendIcon from '@mui/icons-material/Send'; import WhatshotIcon from '@mui/icons-material/Whatshot'; +import PeopleAltIcon from '@mui/icons-material/PeopleAlt'; +import ExploreIcon from '@mui/icons-material/Explore'; import { Link, useNavigate } from 'react-router-dom'; import { useCommunityFeed, useFollowedTopFlavors, + useRecentRatings, type FeedRating, } from '../api/queries/useCommunityFeed'; import { useCreateReply } from '../api/mutations/useRatingMutations'; @@ -44,6 +49,8 @@ import { } from '../components/ui'; import { useToast } from '../hooks/useToast'; +type FeedTab = 'following' | 'discover'; + const SidebarCard: React.FC<{ icon: React.ReactNode; title: React.ReactNode; @@ -69,11 +76,41 @@ const SidebarCard: React.FC<{ ); +const FeedSkeleton: React.FC = () => ( + + {[0, 1, 2].map((i) => ( + + + + + + + + + + + + + + + + + + + + + + + ))} + +); + const CommunityFeed: React.FC = () => { const { t } = useTranslation(); useTitle(t('community.title')); const navigate = useNavigate(); const { notify } = useToast(); + const [tab, setTab] = useState('following'); const [page, setPage] = useState(1); const [replyInputs, setReplyInputs] = useState>({}); const [expandedReplies, setExpandedReplies] = useState>({}); @@ -81,21 +118,29 @@ const CommunityFeed: React.FC = () => { const { user } = useAuth(); const isAuthed = !!user; const { data: feed, isLoading: feedLoading, error: feedError } = useCommunityFeed(page); + const { + data: discover, + isLoading: discoverLoading, + error: discoverError, + } = useRecentRatings(tab === 'discover'); const { data: topFollowed = [] } = useFollowedTopFlavors(); const createReply = useCreateReply(); - const ratings: FeedRating[] = feed?.ratings ?? []; + const followingRatings: FeedRating[] = feed?.ratings ?? []; + const discoverRatings: FeedRating[] = discover ?? []; + const ratings = tab === 'following' ? followingRatings : discoverRatings; const totalPages = feed?.totalPages ?? 1; - const loading = feedLoading; + const loading = tab === 'following' ? feedLoading : discoverLoading; useEffect(() => { if (!isAuthed) navigate('/login'); }, [isAuthed, navigate]); useEffect(() => { - const status = (feedError as { response?: { status?: number } } | null)?.response?.status; + const err = tab === 'following' ? feedError : discoverError; + const status = (err as { response?: { status?: number } } | null)?.response?.status; if (status === 401) navigate('/login'); - }, [feedError, navigate]); + }, [feedError, discoverError, tab, navigate]); const handlePageChange = (_: React.ChangeEvent, value: number) => { setPage(value); @@ -117,211 +162,253 @@ const CommunityFeed: React.FC = () => { } }; - if (loading && ratings.length === 0) + const renderCard = (rating: FeedRating) => { + const expanded = !!expandedReplies[rating.id]; + const latestReply = rating.replies[rating.replies.length - 1]; return ( - - - - - + + + + + + + {!rating.user_avatar && rating.user.charAt(0).toUpperCase()} + + + + + + {rating.user} + + + + {formatDate(rating.created_at)} + + + + + + + {user?.is_superuser && ( + + )} + + + + + {t('community.rated')}{' '} + + {rating.flavor_name} + + + {rating.comment ? ( + + + + ) : ( + + {t('common.noComment')} + + )} + + + + + + + + + + + {!expanded && latestReply && ( + handleReplyToggle(rating.id)} + sx={{ + mt: 1, + px: 1.5, + py: 1, + borderRadius: 1.5, + bgcolor: 'action.hover', + cursor: 'pointer', + display: 'flex', + gap: 1, + alignItems: 'baseline', + minWidth: 0, + }} + > + + {latestReply.user} + + + + + + )} + + + + {rating.replies.map((reply) => ( + + + + {reply.user} + + + {formatDate(reply.created_at)} + + + + + + + {user?.is_superuser && ( + + )} + + + ))} + + setReplyInputs((prev) => ({ ...prev, [rating.id]: val }))} + onKeyDown={(e) => { + if ((e.metaKey || e.ctrlKey) && e.key === 'Enter') { + handleReplySubmit(rating.id); + } + }} + /> + handleReplySubmit(rating.id)} + disabled={!replyInputs[rating.id]} + > + + + + + + + ); + }; + + const subtitle = + tab === 'discover' ? t('community.discoverSubtitle') : t('community.subtitle'); return ( }> - + + + setTab(v)} + sx={{ mb: 3, '& .MuiTab-root': { textTransform: 'none', fontWeight: 700 } }} + > + } + iconPosition="start" + label={t('community.tabFollowing')} + /> + } + iconPosition="start" + label={t('community.tabDiscover')} + /> + - {ratings.length === 0 ? ( - - {t('home.exploreFlavors')} - - } - /> + {loading && ratings.length === 0 ? ( + + ) : ratings.length === 0 ? ( + tab === 'following' ? ( + } + onClick={() => setTab('discover')} + sx={{ borderRadius: 2 }} + > + {t('community.browseDiscover')} + + } + /> + ) : ( + + {t('home.exploreFlavors')} + + } + /> + ) ) : ( - {ratings.map((rating) => ( - - - - - - - {!rating.user_avatar && rating.user.charAt(0).toUpperCase()} - - - - - - {rating.user} - - - - {formatDate(rating.created_at)} - - - - - - - {user?.is_superuser && ( - - )} - - - - - {t('community.rated')}{' '} - - {rating.flavor_name} - - - {rating.comment ? ( - - - - ) : ( - - {t('common.noComment')} - - )} - - - - - - - - - - - - - {rating.replies.map((reply) => ( - - - - {reply.user} - - - {formatDate(reply.created_at)} - - - - - - - {user?.is_superuser && ( - - )} - - - ))} - - - setReplyInputs((prev) => ({ ...prev, [rating.id]: val })) - } - onKeyDown={(e) => { - if ((e.metaKey || e.ctrlKey) && e.key === 'Enter') { - handleReplySubmit(rating.id); - } - }} - /> - handleReplySubmit(rating.id)} - disabled={!replyInputs[rating.id]} - > - - - - - - - - ))} - {totalPages > 1 && ( + {ratings.map(renderCard)} + {tab === 'following' && totalPages > 1 && (