From: Nutomic <me@nutomic.com>
Date: Wed, 12 Apr 2023 14:40:59 +0000 (+0200)
Subject: Fix listing type default value (#2796)
X-Git-Url: http://these/git/%7B%60%24%7BwebArchiveUrl%7D/%22%7B%7D/%7Bthis.state.thumbnail%7D?a=commitdiff_plain;h=9d26ac2c6ff4e7e21178ba27cd04f1e558c8d99b;p=lemmy.git

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
---

diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs
index 27f34c02..c6bb9ddc 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<ListingType>,
-  local_site: &LocalSite,
-) -> Result<ListingType, LemmyError> {
-  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 48ba0ffe..bc0010ab 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::<ApubCommunity, Community>(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 b1cfa14d..7859f754 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::<ApubCommunity, Community>(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 8906f324..1b2217a8 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<ConnectionId>,
   ) -> Result<Self::Response, LemmyError>;
 }
+
+/// Returns default listing type, depending if the query is for frontpage or community.
+fn listing_type_with_default(
+  type_: Option<ListingType>,
+  local_site: &LocalSite,
+  community_id: Option<CommunityId>,
+) -> Result<ListingType, LemmyError> {
+  // 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 f02017ad..99e32eb1 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::<ApubCommunity, Community>(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 159ccaf1..ea967fdf 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<ListingType>,
   sort: Option<CommentSortType>,
   community_id: Option<CommunityId>,
-  community_actor_id: Option<DbUrl>,
   post_id: Option<PostId>,
   parent_path: Option<Ltree>,
   creator_id: Option<PersonId>,
@@ -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 449da8b8..c87fbf0a 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<SortType>,
   creator_id: Option<PersonId>,
   community_id: Option<CommunityId>,
-  community_actor_id: Option<DbUrl>,
   local_user: Option<&'a LocalUser>,
   search_term: Option<String>,
   url_search: Option<String>,
@@ -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 {