Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding saved_only, liked_only, and disliked_only filters to search. #5034

Merged
merged 6 commits into from
Sep 23, 2024
Merged

Adding saved_only, liked_only, and disliked_only filters to search. #5034

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
601a0da
Adding saved_only, liked_only, and disliked_only filters to search.
dessalines Sep 19, 2024
d2995d4
Removing duplicate Url return type for search (was actually post).
dessalines Sep 19, 2024
57045bb
Merge remote-tracking branch 'origin/main' into more_search_filters
dessalines Sep 20, 2024
7e07ea9
Address PR comments.
dessalines Sep 20, 2024
5ee6893
Merge remote-tracking branch 'origin/main' into more_search_filters
dessalines Sep 23, 2024
67f4d1c
Add saved_only post_view test.
dessalines Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/api_common/src/site.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ pub struct Search {
pub page: Option<i64>,
pub limit: Option<i64>,
pub post_title_only: Option<bool>,
pub post_url_only: Option<bool>,
pub saved_only: Option<bool>,
pub liked_only: Option<bool>,
pub disliked_only: Option<bool>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disliked_only wasnt requested and I doubt that anyone would use it.

}

#[derive(Debug, Serialize, Deserialize, Clone)]
Expand Down
12 changes: 12 additions & 0 deletions crates/api_common/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,18 @@ fn limit_expire_time(expires: DateTime<Utc>) -> LemmyResult<Option<DateTime<Utc>
}
}

#[tracing::instrument(skip_all)]
pub fn check_conflicting_like_filters(
liked_only: Option<bool>,
disliked_only: Option<bool>,
) -> LemmyResult<()> {
if liked_only.unwrap_or_default() && disliked_only.unwrap_or_default() {
Err(LemmyErrorType::ContradictingFilters)?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the question mark needed since we're returning an Err directly here?

Copy link
Member Author

@dessalines dessalines Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked, and its necessary to convert from the LemmyErrorType to a LemmyError.

.into() also works, but its not a big deal.

} else {
Ok(())
}
}

pub async fn process_markdown(
text: &str,
slur_regex: &Option<Regex>,
Expand Down
3 changes: 2 additions & 1 deletion crates/api_crud/src/post/read.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ pub async fn get_post(
// Fetch the cross_posts
let cross_posts = if let Some(url) = &post_view.post.url {
let mut x_posts = PostQuery {
url_search: Some(url.inner().as_str().into()),
url_only: Some(true),
search_term: Some(url.inner().as_str().into()),
local_user: local_user.as_ref(),
..Default::default()
}
Expand Down
8 changes: 3 additions & 5 deletions crates/apub/src/api/list_posts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ use actix_web::web::{Json, Query};
use lemmy_api_common::{
context::LemmyContext,
post::{GetPosts, GetPostsResponse},
utils::check_private_instance,
utils::{check_conflicting_like_filters, check_private_instance},
};
use lemmy_db_schema::source::community::Community;
use lemmy_db_views::{
post_view::PostQuery,
structs::{LocalUserView, PaginationCursor, SiteView},
};
use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult};
use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult};

#[tracing::instrument(skip(context))]
pub async fn list_posts(
Expand Down Expand Up @@ -45,9 +45,7 @@ pub async fn list_posts(

let liked_only = data.liked_only;
let disliked_only = data.disliked_only;
if liked_only.unwrap_or_default() && disliked_only.unwrap_or_default() {
return Err(LemmyError::from(LemmyErrorType::ContradictingFilters));
}
check_conflicting_like_filters(liked_only, disliked_only)?;

let local_user = local_user_view.as_ref().map(|u| &u.local_user);
let listing_type = Some(listing_type_with_default(
Expand Down
80 changes: 39 additions & 41 deletions crates/apub/src/api/search.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use actix_web::web::{Json, Query};
use lemmy_api_common::{
context::LemmyContext,
site::{Search, SearchResponse},
utils::{check_private_instance, is_admin},
utils::{check_conflicting_like_filters, check_private_instance, is_admin},
};
use lemmy_db_schema::{source::community::Community, utils::post_to_comment_sort_type, SearchType};
use lemmy_db_views::{
Expand Down Expand Up @@ -55,49 +55,62 @@ pub async fn search(
let creator_id = data.creator_id;
let local_user = local_user_view.as_ref().map(|l| &l.local_user);
let post_title_only = data.post_title_only;
let post_url_only = data.post_url_only;
let saved_only = data.saved_only;

let liked_only = data.liked_only;
let disliked_only = data.disliked_only;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With how many variables that are just let variable = data.variable (13 by my count, we're using every single field of Search), this might be one of those cases where a destructure is less complicatied. e.g. on line 39:

  let Query(Search {
    q,
    community_id,
    community_name,
    creator_id,
    type_,
    sort,
    listing_type,
    page,
    limit,
    post_title_only,
    post_url_only,
    saved_only,
    liked_only,
    disliked_only
  }) = data;

Then, everywhere that follows the pattern I mentioned above will become redundant. Places like line 40 will just be like let q = q.clone(); instead of let q = data.q.clone();.

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, I can do this, at least for the ones that don't need any manipulation.

check_conflicting_like_filters(liked_only, disliked_only)?;

let posts_query = PostQuery {
sort: (sort),
listing_type: (listing_type),
community_id: (community_id),
creator_id: (creator_id),
sort,
listing_type,
community_id,
creator_id,
local_user,
search_term: (Some(q.clone())),
page: (page),
limit: (limit),
title_only: (post_title_only),
search_term: Some(q.clone()),
page,
limit,
title_only: post_title_only,
url_only: post_url_only,
liked_only,
disliked_only,
saved_only,
..Default::default()
};

let comment_query = CommentQuery {
sort: (sort.map(post_to_comment_sort_type)),
listing_type: (listing_type),
search_term: (Some(q.clone())),
community_id: (community_id),
creator_id: (creator_id),
sort: sort.map(post_to_comment_sort_type),
listing_type,
search_term: Some(q.clone()),
community_id,
creator_id,
local_user,
page: (page),
limit: (limit),
page,
limit,
liked_only,
disliked_only,
saved_only,
..Default::default()
};

let community_query = CommunityQuery {
sort: (sort),
listing_type: (listing_type),
search_term: (Some(q.clone())),
sort,
listing_type,
search_term: Some(q.clone()),
local_user,
is_mod_or_admin: (is_admin),
page: (page),
limit: (limit),
is_mod_or_admin: is_admin,
page,
limit,
..Default::default()
};

let person_query = PersonQuery {
sort,
search_term: (Some(q.clone())),
listing_type: (listing_type),
page: (page),
limit: (limit),
search_term: Some(q.clone()),
listing_type,
page,
limit,
};

match search_type {
Expand Down Expand Up @@ -142,21 +155,6 @@ pub async fn search(
person_query.list(&mut context.pool()).await?
};
}
SearchType::Url => {
posts = PostQuery {
sort: (sort),
listing_type: (listing_type),
community_id: (community_id),
creator_id: (creator_id),
url_search: (Some(q)),
local_user,
page: (page),
limit: (limit),
..Default::default()
}
.list(&local_site.site, &mut context.pool())
.await?;
}
};

// Return the jwt
Expand Down
1 change: 0 additions & 1 deletion crates/db_schema/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ pub enum SearchType {
Posts,
Communities,
Users,
Url,
}

#[derive(EnumString, Display, Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Copy, Hash)]
Expand Down
28 changes: 14 additions & 14 deletions crates/db_views/src/post_view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,22 +382,22 @@ fn queries<'a>() -> Queries<
query = query.filter(community::hidden.eq(false));
}

if let Some(url_search) = &options.url_search {
query = query.filter(post::url.eq(url_search));
}

if let Some(search_term) = &options.search_term {
let searcher = fuzzy_search(search_term);
query = if options.title_only.unwrap_or_default() {
query.filter(post::name.ilike(searcher))
if options.url_only.unwrap_or_default() {
query = query.filter(post::url.eq(search_term));
} else {
query.filter(
post::name
.ilike(searcher.clone())
.or(post::body.ilike(searcher)),
)
let searcher = fuzzy_search(search_term);
query = if options.title_only.unwrap_or_default() {
query.filter(post::name.ilike(searcher))
} else {
query.filter(
post::name
.ilike(searcher.clone())
.or(post::body.ilike(searcher)),
)
}
.filter(not(post::removed.or(post::deleted)));
}
.filter(not(post::removed.or(post::deleted)));
}

if !options
Expand Down Expand Up @@ -617,7 +617,7 @@ pub struct PostQuery<'a> {
pub community_id_just_for_prefetch: bool,
pub local_user: Option<&'a LocalUser>,
pub search_term: Option<String>,
pub url_search: Option<String>,
pub url_only: Option<bool>,
pub saved_only: Option<bool>,
pub liked_only: Option<bool>,
pub disliked_only: Option<bool>,
Expand Down