refactor: LOW+MEDIUM cleanups — CQRS DiaryQuery/GoalCommand+GoalQuery,

test-helpers out of production, decompose get_user_profile_html, dedup secure_flag, remove vestigial social_query, drop PersistedImportSession, LoginCommand rename, DeleteAccountDeps, PersonId macro, ReviewSortBy rename, TUI Command for fs::read, generic PaginatedResponse<T>, noop ports for production fallbacks
2026-07-10 05:29:02 +02:00
parent 5e0dde656c
commit 6a9b4e5c00
52 changed files with 405 additions and 298 deletions
--- a/crates/presentation/Cargo.toml
+++ b/crates/presentation/Cargo.toml
@@ -45,7 +45,7 @@ async-trait = { workspace = true }
 futures = { workspace = true }

 api-types = { workspace = true }
-domain = { workspace = true, features = ["test-helpers"] }
+domain = { workspace = true }
 application = { workspace = true }
 auth = { workspace = true }
 metadata = { workspace = true }
--- a/crates/presentation/src/context.rs
+++ b/crates/presentation/src/context.rs
@@ -1,8 +1,8 @@
 use std::sync::Arc;

 use domain::ports::{
-    AuthService, DiaryExporter, DiaryRepository, DocumentParser, EventPublisher,
-    FederatedProfileQuery, GoalRepository, ImportProfileRepository, ImportSessionRepository,
+    AuthService, DiaryExporter, DiaryQuery, DocumentParser, EventPublisher,
+    FederatedProfileQuery, GoalCommand, GoalQuery, ImportProfileRepository, ImportSessionRepository,
    MetadataClient, MovieCommand, MovieProfileRepository, MovieQuery, ObjectStorage,
    PasswordHasher, PersonCommand, PersonEnrichmentClient, PersonQuery, PosterFetcherClient,
    RefreshSessionRepository, RemoteGoalRepository, RemoteWatchlistRepository, ReviewRepository,
@@ -19,7 +19,7 @@ pub struct Repositories {
    pub movie_command: Arc<dyn MovieCommand>,
    pub movie_query: Arc<dyn MovieQuery>,
    pub review: Arc<dyn ReviewRepository>,
-    pub diary: Arc<dyn DiaryRepository>,
+    pub diary: Arc<dyn DiaryQuery>,
    pub stats: Arc<dyn StatsRepository>,
    pub user: Arc<dyn UserRepository>,
    pub import_session: Arc<dyn ImportSessionRepository>,
@@ -38,7 +38,8 @@ pub struct Repositories {
    pub social_query: Arc<dyn SocialQueryPort>,
    pub wrapup_stats: Arc<dyn WrapUpStatsQuery>,
    pub wrapup_repo: Arc<dyn WrapUpRepository>,
-    pub goal: Arc<dyn GoalRepository>,
+    pub goal_command: Arc<dyn GoalCommand>,
+    pub goal_query: Arc<dyn GoalQuery>,
    pub user_settings: Arc<dyn UserSettingsRepository>,
    pub remote_goal: Arc<dyn RemoteGoalRepository>,
    pub refresh_session: Arc<dyn RefreshSessionRepository>,
--- a/crates/presentation/src/factory.rs
+++ b/crates/presentation/src/factory.rs
@@ -13,7 +13,7 @@ pub struct DatabaseOutput {
    pub movie_command: Arc<dyn domain::ports::MovieCommand>,
    pub movie_query: Arc<dyn domain::ports::MovieQuery>,
    pub review: Arc<dyn domain::ports::ReviewRepository>,
-    pub diary: Arc<dyn domain::ports::DiaryRepository>,
+    pub diary: Arc<dyn domain::ports::DiaryQuery>,
    pub stats: Arc<dyn domain::ports::StatsRepository>,
    pub user: Arc<dyn domain::ports::UserRepository>,
    pub import_session: Arc<dyn domain::ports::ImportSessionRepository>,
@@ -31,7 +31,8 @@ pub struct DatabaseOutput {
    pub ap_content: Arc<dyn LocalApContentQuery>,
    pub wrapup_stats: Arc<dyn domain::ports::WrapUpStatsQuery>,
    pub wrapup_repo: Arc<dyn domain::ports::WrapUpRepository>,
-    pub goal: Arc<dyn domain::ports::GoalRepository>,
+    pub goal_command: Arc<dyn domain::ports::GoalCommand>,
+    pub goal_query: Arc<dyn domain::ports::GoalQuery>,
    pub user_settings: Arc<dyn domain::ports::UserSettingsRepository>,
    pub federation_settings: std::sync::Arc<dyn domain::ports::UserFederationSettingsQuery>,
    pub remote_goal: Arc<dyn domain::ports::RemoteGoalRepository>,
@@ -75,7 +76,8 @@ pub async fn build_database_adapters(backend: &str, url: &str) -> anyhow::Result
                ap_content: w.ap_content,
                wrapup_stats: w.wrapup_stats,
                wrapup_repo: w.wrapup_repo,
-                goal: w.goal,
+                goal_command: w.goal_command,
+                goal_query: w.goal_query,
                user_settings: w.user_settings,
                federation_settings: w.federation_settings,
                remote_goal: w.remote_goal,
@@ -118,7 +120,8 @@ pub async fn build_database_adapters(backend: &str, url: &str) -> anyhow::Result
                ap_content: w.ap_content,
                wrapup_stats: w.wrapup_stats,
                wrapup_repo: w.wrapup_repo,
-                goal: w.goal,
+                goal_command: w.goal_command,
+                goal_query: w.goal_query,
                user_settings: w.user_settings,
                federation_settings: w.federation_settings,
                remote_goal: w.remote_goal,
--- a/crates/presentation/src/handlers/goals.rs
+++ b/crates/presentation/src/handlers/goals.rs
@@ -40,7 +40,7 @@ pub async fn list_goals(
    user: AuthenticatedUser,
 ) -> Result<Json<GoalsResponse>, ApiError> {
    let deps = GoalQueryDeps {
-        goal: state.app_ctx.repos.goal.clone(),
+        goal_query: state.app_ctx.repos.goal_query.clone(),
        stats: state.app_ctx.repos.stats.clone(),
    };
    let goals = application::goals::list::execute(
@@ -70,7 +70,8 @@ pub async fn create_goal(
    Json(req): Json<CreateGoalRequest>,
 ) -> Result<Json<GoalDto>, ApiError> {
    let deps = GoalCommandDeps {
-        goal: state.app_ctx.repos.goal.clone(),
+        goal_command: state.app_ctx.repos.goal_command.clone(),
+        goal_query: state.app_ctx.repos.goal_query.clone(),
        stats: state.app_ctx.repos.stats.clone(),
        event_publisher: state.app_ctx.services.event_publisher.clone(),
    };
@@ -103,7 +104,8 @@ pub async fn update_goal(
    Json(req): Json<UpdateGoalRequest>,
 ) -> Result<Json<GoalDto>, ApiError> {
    let deps = GoalCommandDeps {
-        goal: state.app_ctx.repos.goal.clone(),
+        goal_command: state.app_ctx.repos.goal_command.clone(),
+        goal_query: state.app_ctx.repos.goal_query.clone(),
        stats: state.app_ctx.repos.stats.clone(),
        event_publisher: state.app_ctx.services.event_publisher.clone(),
    };
@@ -134,7 +136,8 @@ pub async fn delete_goal(
    Path(year): Path<u16>,
 ) -> Result<StatusCode, ApiError> {
    let deps = GoalCommandDeps {
-        goal: state.app_ctx.repos.goal.clone(),
+        goal_command: state.app_ctx.repos.goal_command.clone(),
+        goal_query: state.app_ctx.repos.goal_query.clone(),
        stats: state.app_ctx.repos.stats.clone(),
        event_publisher: state.app_ctx.services.event_publisher.clone(),
    };
@@ -163,7 +166,7 @@ pub async fn get_user_goals(
    Path(user_id): Path<Uuid>,
 ) -> Result<Json<GoalsResponse>, ApiError> {
    let deps = GoalQueryDeps {
-        goal: state.app_ctx.repos.goal.clone(),
+        goal_query: state.app_ctx.repos.goal_query.clone(),
        stats: state.app_ctx.repos.stats.clone(),
    };
    let goals = application::goals::list::execute(
--- a/crates/presentation/src/handlers/users.rs
+++ b/crates/presentation/src/handlers/users.rs
@@ -351,7 +351,7 @@ pub async fn get_user_profile(
        goals: {
            let goals_list = application::goals::list::execute(
                &application::goals::deps::GoalQueryDeps {
-                    goal: state.app_ctx.repos.goal.clone(),
+                    goal_query: state.app_ctx.repos.goal_query.clone(),
                    stats: state.app_ctx.repos.stats.clone(),
                },
                application::goals::queries::ListGoalsQuery { user_id },
@@ -531,6 +531,107 @@ pub async fn get_user_by_username(
    }
 }

+// ── Profile helpers (private) ───────────────────────────────────────────────
+
+struct PaginationInfo {
+    offset: u32,
+    has_more: bool,
+    limit: u32,
+    page_items: Vec<template_askama::PageItem>,
+}
+
+fn compute_pagination(
+    entries: Option<&domain::models::collections::Paginated<domain::models::DiaryEntry>>,
+) -> PaginationInfo {
+    let (offset, has_more, limit) = entries
+        .map(|e| {
+            let has_more = (e.offset as u64).saturating_add(e.limit as u64) < e.total_count;
+            (e.offset, has_more, e.limit)
+        })
+        .unwrap_or((0, false, super::DEFAULT_PAGE_LIMIT));
+    let total = entries.map(|e| e.total_count as u32).unwrap_or(0);
+    let total_pages = total
+        .saturating_add(limit.saturating_sub(1))
+        .checked_div(limit)
+        .unwrap_or(1);
+    let current_page = offset.checked_div(limit).unwrap_or(0);
+    let page_items = build_page_items(total_pages, current_page);
+    PaginationInfo {
+        offset,
+        has_more,
+        limit,
+        page_items,
+    }
+}
+
+struct StatsDisplay {
+    avg_rating: String,
+    favorite_director: String,
+    most_active_month: String,
+}
+
+fn build_stats_display(stats: &domain::models::UserStats) -> StatsDisplay {
+    StatsDisplay {
+        avg_rating: stats
+            .avg_rating
+            .map(|r| format!("{:.1}", r))
+            .unwrap_or_else(|| "\u{2014}".to_string()),
+        favorite_director: stats
+            .favorite_director
+            .clone()
+            .unwrap_or_else(|| "\u{2014}".to_string()),
+        most_active_month: stats
+            .most_active_month
+            .clone()
+            .unwrap_or_else(|| "\u{2014}".to_string()),
+    }
+}
+
+fn build_monthly_rating_rows<'a>(
+    trends: Option<&'a domain::models::UserTrends>,
+) -> Vec<MonthlyRatingRow<'a>> {
+    trends
+        .map(|t| {
+            t.monthly_ratings
+                .iter()
+                .map(|r| MonthlyRatingRow {
+                    rating: r,
+                    bar_height_px: bar_height_px(r.avg_rating),
+                })
+                .collect()
+        })
+        .unwrap_or_default()
+}
+
+async fn fetch_profile_goals(
+    state: &AppState,
+    user_id: Uuid,
+) -> Vec<template_askama::GoalViewData> {
+    let goals_list = application::goals::list::execute(
+        &application::goals::deps::GoalQueryDeps {
+            goal_query: state.app_ctx.repos.goal_query.clone(),
+            stats: state.app_ctx.repos.stats.clone(),
+        },
+        application::goals::queries::ListGoalsQuery {
+            user_id,
+        },
+    )
+    .await
+    .unwrap_or_default();
+    goals_list
+        .iter()
+        .map(|g| template_askama::GoalViewData {
+            year: g.goal.year(),
+            target_count: g.goal.target_count(),
+            current_count: g.current_count,
+            percentage: g.percentage().round(),
+            is_complete: g.is_complete(),
+        })
+        .collect()
+}
+
+// ── Handler ─────────────────────────────────────────────────────────────────
+
 pub async fn get_user_profile_html(
    OptionalCookieUser(user_id): OptionalCookieUser,
    State(state): State<AppState>,
@@ -634,60 +735,16 @@ pub async fn get_user_profile_html(
    };
    match application::users::get_profile::execute(&html_profile_deps, query).await {
        Ok(profile) => {
-            let (offset, has_more, limit) = profile
-                .entries
-                .as_ref()
-                .map(|e| {
-                    let has_more = (e.offset as u64).saturating_add(e.limit as u64) < e.total_count;
-                    (e.offset, has_more, e.limit)
-                })
-                .unwrap_or((0, false, super::DEFAULT_PAGE_LIMIT));
+            let pag = compute_pagination(profile.entries.as_ref());
            if !is_own_profile {
                ctx.page_rss_url = Some(format!("/users/{}/feed.rss", profile_user_uuid));
            }
            let email = profile_user.email().value().to_string();
            let display_name = email.split('@').next().unwrap_or("?").to_string();
-            let avg_rating_display = profile
-                .stats
-                .avg_rating
-                .map(|r| format!("{:.1}", r))
-                .unwrap_or_else(|| "\u{2014}".to_string());
-            let favorite_director_display = profile
-                .stats
-                .favorite_director
-                .clone()
-                .unwrap_or_else(|| "\u{2014}".to_string());
-            let most_active_month_display = profile
-                .stats
-                .most_active_month
-                .clone()
-                .unwrap_or_else(|| "\u{2014}".to_string());
+            let stats_disp = build_stats_display(&profile.stats);
            let history = profile.history.map(application::users::group_by_month);
            let heatmap = history.as_deref().map(build_heatmap).unwrap_or_default();
-            let monthly_rating_rows: Vec<MonthlyRatingRow<'_>> = profile
-                .trends
-                .as_ref()
-                .map(|t| {
-                    t.monthly_ratings
-                        .iter()
-                        .map(|r| MonthlyRatingRow {
-                            rating: r,
-                            bar_height_px: bar_height_px(r.avg_rating),
-                        })
-                        .collect()
-                })
-                .unwrap_or_default();
-            let total = profile
-                .entries
-                .as_ref()
-                .map(|e| e.total_count as u32)
-                .unwrap_or(0);
-            let total_pages = total
-                .saturating_add(limit.saturating_sub(1))
-                .checked_div(limit)
-                .unwrap_or(1);
-            let current_page = offset.checked_div(limit).unwrap_or(0);
-            let page_items = build_page_items(total_pages, current_page);
+            let monthly_rating_rows = build_monthly_rating_rows(profile.trends.as_ref());
            let pending_followers: Vec<RemoteActorData> = profile
                .pending_followers
                .iter()
@@ -703,43 +760,44 @@ pub async fn get_user_profile_html(
                    profile_user_id: profile_user_uuid,
                    profile_url,
                    stats: &profile.stats,
-                    avg_rating_display,
-                    favorite_director_display,
-                    most_active_month_display,
+                    avg_rating_display: stats_disp.avg_rating,
+                    favorite_director_display: stats_disp.favorite_director,
+                    most_active_month_display: stats_disp.most_active_month,
                    view: profile_view.as_str(),
                    entries: profile.entries.as_ref(),
-                    current_offset: offset,
-                    has_more,
-                    limit,
+                    current_offset: pag.offset,
+                    has_more: pag.has_more,
+                    limit: pag.limit,
                    history: history.as_ref(),
                    trends: profile.trends.as_ref(),
                    monthly_rating_rows,
                    heatmap,
-                    page_items,
+                    page_items: pag.page_items,
                    sort_by: sort_by_str.to_string(),
                });
                let mut resp = response.into_response();
                resp.headers_mut().remove("x-frame-options");
                resp
            } else {
+                let goals = fetch_profile_goals(&state, profile_user_uuid).await;
                render_page(ProfileTemplate {
                    ctx: &ctx,
                    profile_display_name: display_name,
                    profile_user_id: profile_user_uuid,
                    stats: &profile.stats,
-                    avg_rating_display,
-                    favorite_director_display,
-                    most_active_month_display,
+                    avg_rating_display: stats_disp.avg_rating,
+                    favorite_director_display: stats_disp.favorite_director,
+                    most_active_month_display: stats_disp.most_active_month,
                    view: profile_view.as_str(),
                    entries: profile.entries.as_ref(),
-                    current_offset: offset,
-                    has_more,
-                    limit,
+                    current_offset: pag.offset,
+                    has_more: pag.has_more,
+                    limit: pag.limit,
                    history: history.as_ref(),
                    trends: profile.trends.as_ref(),
                    monthly_rating_rows,
                    heatmap,
-                    page_items,
+                    page_items: pag.page_items,
                    is_own_profile,
                    error: params.error,
                    following_count: profile.following_count,
@@ -747,29 +805,7 @@ pub async fn get_user_profile_html(
                    pending_followers,
                    sort_by: sort_by_str.to_string(),
                    search: params.search.clone(),
-                    goals: {
-                        let goals_list = application::goals::list::execute(
-                            &application::goals::deps::GoalQueryDeps {
-                                goal: state.app_ctx.repos.goal.clone(),
-                                stats: state.app_ctx.repos.stats.clone(),
-                            },
-                            application::goals::queries::ListGoalsQuery {
-                                user_id: profile_user_uuid,
-                            },
-                        )
-                        .await
-                        .unwrap_or_default();
-                        goals_list
-                            .iter()
-                            .map(|g| template_askama::GoalViewData {
-                                year: g.goal.year(),
-                                target_count: g.goal.target_count(),
-                                current_count: g.current_count,
-                                percentage: g.percentage().round(),
-                                is_complete: g.is_complete(),
-                            })
-                            .collect()
-                    },
+                    goals,
                })
                .into_response()
            }
--- a/crates/presentation/src/main.rs
+++ b/crates/presentation/src/main.rs
@@ -100,7 +100,7 @@ async fn wire_dependencies() -> anyhow::Result<(AppState, axum::Router)> {
            movie_repo: Arc::clone(&db.movie_query),
            review_repo: Arc::clone(&db.review),
            diary_repo: Arc::clone(&db.diary),
-            goal_repo: Arc::clone(&db.goal),
+            goal_repo: Arc::clone(&db.goal_query),
            stats_repo: Arc::clone(&db.stats),
            user_repo: Arc::clone(&db.user),
            federation_settings: std::sync::Arc::clone(&db.federation_settings),
@@ -158,14 +158,15 @@ async fn wire_dependencies() -> anyhow::Result<(AppState, axum::Router)> {
            #[cfg(feature = "federation")]
            remote_watchlist: remote_watchlist_repo,
            #[cfg(not(feature = "federation"))]
-            remote_watchlist: Arc::new(domain::testing::NoopRemoteWatchlistRepository),
+            remote_watchlist: Arc::new(domain::ports::noop::NoopRemoteWatchlistRepository),
            #[cfg(feature = "federation")]
            social_query: social_query.clone(),
            #[cfg(not(feature = "federation"))]
-            social_query: Arc::new(domain::testing::NoopSocialQueryPort),
+            social_query: Arc::new(domain::ports::noop::NoopSocialQueryPort),
            wrapup_stats: db.wrapup_stats,
            wrapup_repo: db.wrapup_repo,
-            goal: db.goal,
+            goal_command: db.goal_command,
+            goal_query: db.goal_query,
            user_settings: db.user_settings,
            remote_goal: db.remote_goal,
            refresh_session: db.refresh_session,
--- a/crates/presentation/src/tests/extractors.rs
+++ b/crates/presentation/src/tests/extractors.rs
@@ -17,7 +17,7 @@ use domain::{
        collections::{PageParams, Paginated},
    },
    ports::{
-        AuthService, DiaryRepository, EventPublisher, MetadataClient, MovieCommand, MovieQuery,
+        AuthService, DiaryQuery, EventPublisher, MetadataClient, MovieCommand, MovieQuery,
        ObjectStorage, PasswordHasher, PersonCommand, PersonQuery, PosterFetcherClient,
        ReviewRepository, SearchCommand, SearchPort, StatsRepository, UserRepository,
        WatchlistRepository,
@@ -106,7 +106,7 @@ impl ReviewRepository for Panic {
    }
 }
 #[async_trait::async_trait]
-impl DiaryRepository for Panic {
+impl DiaryQuery for Panic {
    async fn query_diary(&self, _: &DiaryFilter) -> Result<Paginated<DiaryEntry>, DomainError> {
        panic!()
    }
@@ -682,7 +682,7 @@ impl domain::ports::WrapUpRepository for Panic {
 }

 #[async_trait::async_trait]
-impl domain::ports::GoalRepository for Panic {
+impl domain::ports::GoalCommand for Panic {
    async fn save(&self, _: &domain::models::Goal) -> Result<(), DomainError> {
        panic!()
    }
@@ -696,6 +696,10 @@ impl domain::ports::GoalRepository for Panic {
    ) -> Result<(), DomainError> {
        panic!()
    }
+}
+
+#[async_trait::async_trait]
+impl domain::ports::GoalQuery for Panic {
    async fn find_by_user_and_year(
        &self,
        _: &domain::value_objects::UserId,
@@ -810,7 +814,8 @@ pub fn make_test_state(auth_service: Arc<dyn AuthService>) -> crate::state::AppS
                social_query: Arc::clone(&repo) as _,
                wrapup_stats: Arc::clone(&repo) as _,
                wrapup_repo: Arc::clone(&repo) as _,
-                goal: Arc::clone(&repo) as _,
+                goal_command: Arc::clone(&repo) as _,
+                goal_query: Arc::clone(&repo) as _,
                user_settings: Arc::clone(&repo) as _,
                remote_goal: Arc::clone(&repo) as _,
                refresh_session: Arc::clone(&repo) as _,
--- a/crates/presentation/tests/api_test.rs
+++ b/crates/presentation/tests/api_test.rs
@@ -467,7 +467,8 @@ async fn test_app() -> Router {
                social_query: Arc::new(PanicSocialQuery),
                wrapup_stats: Arc::new(domain::testing::PanicWrapUpStatsQuery) as _,
                wrapup_repo: Arc::new(domain::testing::PanicWrapUpRepository) as _,
-                goal: Arc::new(domain::testing::NoopGoalRepository),
+                goal_command: Arc::new(domain::testing::NoopGoalCommand),
+                goal_query: Arc::new(domain::testing::NoopGoalQuery),
                user_settings: Arc::new(domain::testing::NoopUserSettingsRepository),
                remote_goal: Arc::new(domain::testing::NoopRemoteGoalRepository),
                refresh_session: Arc::new(domain::testing::PanicRefreshSessionRepository),