Fix listing type default value #2796

Nutomic · 2023-04-04T21:12:08Z

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.

Also simplified the defaults for db view forms in second commit.

dessalines · 2023-04-05T20:41:03Z

crates/apub/src/api/list_comments.rs

    let max_depth = data.max_depth;
    let saved_only = data.saved_only;
    let page = data.page;
    let limit = data.limit;
    let parent_id = data.parent_id;

+    // On frontpage use listing type from param or admin configured default


This is copy pasted below, might as well use a helper function.

True, I didnt do that before because with community_id and community_actor_id, there would have been too many parameters and it would be awkward. Added helper now.

dessalines · 2023-04-05T20:41:32Z

crates/apub/src/api/list_posts.rs

+    } else {
+      // inside of community show everything
+      ListingType::All
+    };


Here's the other place its pasted.

dessalines · 2023-04-05T20:43:39Z

crates/db_views/src/comment_view.rs

@@ -167,10 +167,9 @@ impl CommentView {
 pub struct CommentQuery<'a> {
  #[builder(!default)]
  pool: &'a DbPool,
-  listing_type: Option<ListingType>,
-  sort: Option<CommentSortType>,
+  listing_type: ListingType,


Since these aren't optional anymore, you might need to add that builder !default line to these two.

edit:

To me it makes sense to leave these optional, and move the unwrap_or_default into the query impls.

dessalines · 2023-04-05T20:44:09Z

crates/db_views/src/post_view.rs

@@ -220,11 +220,10 @@ impl PostView {
 pub struct PostQuery<'a> {
  #[builder(!default)]
  pool: &'a DbPool,
-  listing_type: Option<ListingType>,
-  sort: Option<SortType>,
+  listing_type: ListingType,


dessalines · 2023-04-05T20:44:33Z

crates/db_views_actor/src/community_view.rs

@@ -121,8 +121,8 @@ impl CommunityView {
 pub struct CommunityQuery<'a> {
  #[builder(!default)]
  pool: &'a DbPool,


dessalines · 2023-04-05T20:46:12Z

crates/db_views_actor/src/community_view.rs

@@ -176,7 +176,7 @@ impl<'a> CommunityQuery<'a> {
        .filter(community::deleted.eq(false));
    }

-    match self.sort.unwrap_or(SortType::Hot) {
+    match self.sort {


Makes more sense to me have these optional in the builder, and move the unwrap_or_default to here.

dessalines · 2023-04-05T20:46:39Z

crates/db_views_actor/src/person_view.rs

@@ -73,7 +73,7 @@ impl PersonView {
 pub struct PersonQuery<'a> {
  #[builder(!default)]
  pool: &'a DbPool,
-  sort: Option<SortType>,
+  sort: SortType,
  search_term: Option<String>,


Same issue.

Nutomic · 2023-04-06T20:22:49Z

I removed the second commit, apparently its not as easy as I thought.

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.

dessalines · 2023-04-10T19:51:46Z

Feel free to merge once you get to the clippy issues.

dessalines · 2023-04-12T14:41:14Z

I'll see what I can do to fix this woodpecker issue on another PR.

* 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

Nutomic requested a review from dessalines as a code owner April 4, 2023 21:12

dessalines reviewed Apr 5, 2023

View reviewed changes

Nutomic force-pushed the fix-listing-type branch from 16053bf to e9dcd84 Compare April 6, 2023 20:22

Nutomic force-pushed the fix-listing-type branch from e9dcd84 to 3bff89b Compare April 6, 2023 20:23

dessalines approved these changes Apr 10, 2023

View reviewed changes

fix clippy

3dc8c6f

Nutomic enabled auto-merge (squash) April 11, 2023 20:15

dessalines disabled auto-merge April 12, 2023 14:40

dessalines merged commit 9d26ac2 into main Apr 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix listing type default value #2796

Fix listing type default value #2796

Nutomic commented Apr 4, 2023

dessalines Apr 5, 2023

Nutomic Apr 6, 2023

dessalines Apr 5, 2023

dessalines Apr 5, 2023

dessalines Apr 5, 2023

dessalines Apr 5, 2023

dessalines Apr 5, 2023

dessalines Apr 5, 2023

Nutomic commented Apr 6, 2023

dessalines commented Apr 10, 2023

dessalines commented Apr 12, 2023

Fix listing type default value #2796

Fix listing type default value #2796

Conversation

Nutomic commented Apr 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nutomic commented Apr 6, 2023

dessalines commented Apr 10, 2023

dessalines commented Apr 12, 2023