From 9d26ac2c6ff4e7e21178ba27cd04f1e558c8d99b Mon Sep 17 00:00:00 2001 From: Nutomic Date: Wed, 12 Apr 2023 16:40:59 +0200 Subject: [PATCH] Fix listing type default value (#2796) * Fix listing type default value The listing type parameter is only meant for the frontpage, but is also applied inside of communities. The result is that this call returns nothing, because it defaults to ListingType::Local: https://fedibb.ml/api/v3/post/list?community_id=3 It needs to be called like this to get any posts: https://fedibb.ml/api/v3/post/list?community_id=3&type_=All This is clearly not expected behaviour, when a community is specified, the listing type should default to All. * fix clippy --- crates/api_common/src/utils.rs | 11 ----------- crates/apub/src/api/list_comments.rs | 20 +++++++------------- crates/apub/src/api/list_posts.rs | 20 +++++++------------- crates/apub/src/api/mod.rs | 20 ++++++++++++++++++++ crates/apub/src/api/search.rs | 14 +++----------- crates/db_views/src/comment_view.rs | 7 +------ crates/db_views/src/post_view.rs | 9 ++------- 7 files changed, 40 insertions(+), 61 deletions(-) diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 27f34c02f..c6bb9ddc4 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -20,7 +20,6 @@ use lemmy_db_schema::{ }, traits::{Crud, Readable}, utils::DbPool, - ListingType, }; use lemmy_db_views::{comment_view::CommentQuery, structs::LocalUserView}; use lemmy_db_views_actor::structs::{ @@ -41,7 +40,6 @@ use lemmy_utils::{ use regex::Regex; use reqwest_middleware::ClientWithMiddleware; use rosetta_i18n::{Language, LanguageId}; -use std::str::FromStr; use tracing::warn; use url::{ParseError, Url}; @@ -781,15 +779,6 @@ pub async fn delete_user_account( Ok(()) } -pub fn listing_type_with_site_default( - listing_type: Option, - local_site: &LocalSite, -) -> Result { - Ok(listing_type.unwrap_or(ListingType::from_str( - &local_site.default_post_listing_type, - )?)) -} - #[cfg(test)] mod tests { use crate::utils::{honeypot_check, password_length_check}; diff --git a/crates/apub/src/api/list_comments.rs b/crates/apub/src/api/list_comments.rs index 48ba0ffe9..bc0010ab5 100644 --- a/crates/apub/src/api/list_comments.rs +++ b/crates/apub/src/api/list_comments.rs @@ -1,5 +1,5 @@ use crate::{ - api::PerformApub, + api::{listing_type_with_default, PerformApub}, fetcher::resolve_actor_identifier, objects::community::ApubCommunity, }; @@ -7,11 +7,7 @@ use activitypub_federation::config::Data; use lemmy_api_common::{ comment::{GetComments, GetCommentsResponse}, context::LemmyContext, - utils::{ - check_private_instance, - get_local_user_view_from_jwt_opt, - listing_type_with_site_default, - }, + utils::{check_private_instance, get_local_user_view_from_jwt_opt}, }; use lemmy_db_schema::{ source::{comment::Comment, community::Community, local_site::LocalSite}, @@ -37,16 +33,13 @@ impl PerformApub for GetComments { let local_site = LocalSite::read(context.pool()).await?; check_private_instance(&local_user_view, &local_site)?; - let community_id = data.community_id; - let listing_type = listing_type_with_site_default(data.type_, &local_site)?; - - let community_actor_id = if let Some(name) = &data.community_name { + let community_id = if let Some(name) = &data.community_name { resolve_actor_identifier::(name, context, &None, true) .await .ok() - .map(|c| c.actor_id.clone()) + .map(|c| c.id) } else { - None + data.community_id }; let sort = data.sort; let max_depth = data.max_depth; @@ -55,6 +48,8 @@ impl PerformApub for GetComments { let limit = data.limit; let parent_id = data.parent_id; + let listing_type = listing_type_with_default(data.type_, &local_site, community_id)?; + // If a parent_id is given, fetch the comment to get the path let parent_path = if let Some(parent_id) = parent_id { Some(Comment::read(context.pool(), parent_id).await?.path) @@ -72,7 +67,6 @@ impl PerformApub for GetComments { .max_depth(max_depth) .saved_only(saved_only) .community_id(community_id) - .community_actor_id(community_actor_id) .parent_path(parent_path_cloned) .post_id(post_id) .local_user(local_user.as_ref()) diff --git a/crates/apub/src/api/list_posts.rs b/crates/apub/src/api/list_posts.rs index b1cfa14d4..7859f7547 100644 --- a/crates/apub/src/api/list_posts.rs +++ b/crates/apub/src/api/list_posts.rs @@ -1,5 +1,5 @@ use crate::{ - api::PerformApub, + api::{listing_type_with_default, PerformApub}, fetcher::resolve_actor_identifier, objects::community::ApubCommunity, }; @@ -7,12 +7,7 @@ use activitypub_federation::config::Data; use lemmy_api_common::{ context::LemmyContext, post::{GetPosts, GetPostsResponse}, - utils::{ - check_private_instance, - get_local_user_view_from_jwt_opt, - is_mod_or_admin_opt, - listing_type_with_site_default, - }, + utils::{check_private_instance, get_local_user_view_from_jwt_opt, is_mod_or_admin_opt}, }; use lemmy_db_schema::source::{community::Community, local_site::LocalSite}; use lemmy_db_views::post_view::PostQuery; @@ -37,21 +32,21 @@ impl PerformApub for GetPosts { check_private_instance(&local_user_view, &local_site)?; let sort = data.sort; - let listing_type = listing_type_with_site_default(data.type_, &local_site)?; let page = data.page; let limit = data.limit; - let community_id = data.community_id; - let community_actor_id = if let Some(name) = &data.community_name { + let community_id = if let Some(name) = &data.community_name { resolve_actor_identifier::(name, context, &None, true) .await .ok() - .map(|c| c.actor_id.clone()) + .map(|c| c.id) } else { - None + data.community_id }; let saved_only = data.saved_only; + let listing_type = listing_type_with_default(data.type_, &local_site, community_id)?; + let is_mod_or_admin = is_mod_or_admin_opt(context.pool(), local_user_view.as_ref(), community_id) .await @@ -63,7 +58,6 @@ impl PerformApub for GetPosts { .listing_type(Some(listing_type)) .sort(sort) .community_id(community_id) - .community_actor_id(community_actor_id) .saved_only(saved_only) .page(page) .limit(limit) diff --git a/crates/apub/src/api/mod.rs b/crates/apub/src/api/mod.rs index 8906f3247..1b2217a8a 100644 --- a/crates/apub/src/api/mod.rs +++ b/crates/apub/src/api/mod.rs @@ -1,6 +1,8 @@ use activitypub_federation::config::Data; use lemmy_api_common::context::LemmyContext; +use lemmy_db_schema::{newtypes::CommunityId, source::local_site::LocalSite, ListingType}; use lemmy_utils::{error::LemmyError, ConnectionId}; +use std::str::FromStr; mod list_comments; mod list_posts; @@ -19,3 +21,21 @@ pub trait PerformApub { websocket_id: Option, ) -> Result; } + +/// Returns default listing type, depending if the query is for frontpage or community. +fn listing_type_with_default( + type_: Option, + local_site: &LocalSite, + community_id: Option, +) -> Result { + // On frontpage use listing type from param or admin configured default + let listing_type = if community_id.is_none() { + type_.unwrap_or(ListingType::from_str( + &local_site.default_post_listing_type, + )?) + } else { + // inside of community show everything + ListingType::All + }; + Ok(listing_type) +} diff --git a/crates/apub/src/api/search.rs b/crates/apub/src/api/search.rs index f02017ad7..99e32eb17 100644 --- a/crates/apub/src/api/search.rs +++ b/crates/apub/src/api/search.rs @@ -52,14 +52,13 @@ impl PerformApub for Search { let sort = data.sort; let listing_type = data.listing_type; let search_type = data.type_.unwrap_or(SearchType::All); - let community_id = data.community_id; - let community_actor_id = if let Some(name) = &data.community_name { + let community_id = if let Some(name) = &data.community_name { resolve_actor_identifier::(name, context, &local_user_view, false) .await .ok() - .map(|c| c.actor_id.clone()) + .map(|c| c.id) } else { - None + data.community_id }; let creator_id = data.creator_id; let local_user = local_user_view.map(|l| l.local_user); @@ -70,7 +69,6 @@ impl PerformApub for Search { .sort(sort) .listing_type(listing_type) .community_id(community_id) - .community_actor_id(community_actor_id) .creator_id(creator_id) .local_user(local_user.as_ref()) .search_term(Some(q)) @@ -88,7 +86,6 @@ impl PerformApub for Search { .listing_type(listing_type) .search_term(Some(q)) .community_id(community_id) - .community_actor_id(community_actor_id) .creator_id(creator_id) .local_user(local_user.as_ref()) .page(page) @@ -126,7 +123,6 @@ impl PerformApub for Search { // If the community or creator is included, dont search communities or users let community_or_creator_included = data.community_id.is_some() || data.community_name.is_some() || data.creator_id.is_some(); - let community_actor_id_2 = community_actor_id.clone(); let local_user_ = local_user.clone(); posts = PostQuery::builder() @@ -134,7 +130,6 @@ impl PerformApub for Search { .sort(sort) .listing_type(listing_type) .community_id(community_id) - .community_actor_id(community_actor_id_2) .creator_id(creator_id) .local_user(local_user_.as_ref()) .search_term(Some(q)) @@ -146,7 +141,6 @@ impl PerformApub for Search { .await?; let q = data.q.clone(); - let community_actor_id = community_actor_id.clone(); let local_user_ = local_user.clone(); comments = CommentQuery::builder() @@ -155,7 +149,6 @@ impl PerformApub for Search { .listing_type(listing_type) .search_term(Some(q)) .community_id(community_id) - .community_actor_id(community_actor_id) .creator_id(creator_id) .local_user(local_user_.as_ref()) .page(page) @@ -205,7 +198,6 @@ impl PerformApub for Search { .sort(sort) .listing_type(listing_type) .community_id(community_id) - .community_actor_id(community_actor_id) .creator_id(creator_id) .url_search(Some(q)) .is_mod_or_admin(is_admin) diff --git a/crates/db_views/src/comment_view.rs b/crates/db_views/src/comment_view.rs index 159ccaf11..ea967fdff 100644 --- a/crates/db_views/src/comment_view.rs +++ b/crates/db_views/src/comment_view.rs @@ -13,7 +13,7 @@ use diesel_async::RunQueryDsl; use diesel_ltree::{nlevel, subpath, Ltree, LtreeExtensions}; use lemmy_db_schema::{ aggregates::structs::CommentAggregates, - newtypes::{CommentId, CommunityId, DbUrl, LocalUserId, PersonId, PostId}, + newtypes::{CommentId, CommunityId, LocalUserId, PersonId, PostId}, schema::{ comment, comment_aggregates, @@ -170,7 +170,6 @@ pub struct CommentQuery<'a> { listing_type: Option, sort: Option, community_id: Option, - community_actor_id: Option, post_id: Option, parent_path: Option, creator_id: Option, @@ -306,10 +305,6 @@ impl<'a> CommentQuery<'a> { query = query.filter(post::community_id.eq(community_id)); } - if let Some(community_actor_id) = self.community_actor_id { - query = query.filter(community::actor_id.eq(community_actor_id)) - } - if self.saved_only.unwrap_or(false) { query = query.filter(comment_saved::comment_id.is_not_null()); } diff --git a/crates/db_views/src/post_view.rs b/crates/db_views/src/post_view.rs index 449da8b84..c87fbf0ab 100644 --- a/crates/db_views/src/post_view.rs +++ b/crates/db_views/src/post_view.rs @@ -16,7 +16,7 @@ use diesel::{ use diesel_async::RunQueryDsl; use lemmy_db_schema::{ aggregates::structs::PostAggregates, - newtypes::{CommunityId, DbUrl, LocalUserId, PersonId, PostId}, + newtypes::{CommunityId, LocalUserId, PersonId, PostId}, schema::{ community, community_block, @@ -224,7 +224,6 @@ pub struct PostQuery<'a> { sort: Option, creator_id: Option, community_id: Option, - community_actor_id: Option, local_user: Option<&'a LocalUser>, search_term: Option, url_search: Option, @@ -380,16 +379,12 @@ impl<'a> PostQuery<'a> { } } } - if self.community_id.is_none() && self.community_actor_id.is_none() { + if self.community_id.is_none() { query = query.then_order_by(post_aggregates::featured_local.desc()); } else if let Some(community_id) = self.community_id { query = query .filter(post::community_id.eq(community_id)) .then_order_by(post_aggregates::featured_community.desc()); - } else if let Some(community_actor_id) = self.community_actor_id { - query = query - .filter(community::actor_id.eq(community_actor_id)) - .then_order_by(post_aggregates::featured_community.desc()); } if let Some(url_search) = self.url_search {