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.

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

Search post by title only. #4677

Closed
5 tasks done
Max-dc opened this issue Apr 30, 2024 · 16 comments
Closed
5 tasks done

Search post by title only. #4677

Max-dc opened this issue Apr 30, 2024 · 16 comments
Labels
area: search enhancement New feature or request extra: good first issue Good for newcomers

Comments

@Max-dc
Copy link

Max-dc commented Apr 30, 2024

Requirements

  • Is this a feature request? For questions or discussions use https://lemmy.ml/c/lemmy_support
  • Did you check to see if this issue already exists?
  • Is this only a feature request? Do not put multiple feature requests in one issue.
  • Is this a backend issue? Use the lemmy-ui repo for UI / frontend issues.
  • Do you agree to follow the rules in our Code of Conduct?

Is your proposal related to a problem?

In order to avoid a duplicate post on an already existing topic. I always perform a search within lemmy /search
type: post, All instances, an order. all community, all creator

But the search actually dig in the body and titles... so in order to search for a specific topics it's become cumbersome..

additionally a search by Title only would make the search quicker and lighter for the system...

Describe the solution you'd like.

May-be a checkbox search in title only ?

Describe alternatives you've considered.

There is none I believe.

Additional context

No response

@Max-dc Max-dc added the enhancement New feature or request label Apr 30, 2024
@dessalines
Copy link
Member

I could see this being useful.

@leoseg
Copy link
Contributor

leoseg commented Aug 7, 2024

I would like to take up this issue :)

@Nutomic
Copy link
Member

Nutomic commented Aug 20, 2024

@leoseg Do you need any help to get started? Feel free to ask here or in the dev chat.

@Carlos-Cabello
Copy link
Contributor

If no one is working on this I can take it.

@leoseg
Copy link
Contributor

leoseg commented Sep 12, 2024

Hi I actually already started working on that issue and implemented the backend part in rust but still have to do the testing I only recently came back from vacation. But there is also the frontend part to implement the checkbox and include the parameter in the query if you want to pick up that? I named my branch "4677-Search-post-by-title-only", and the query parameter is a boolean "search_title_only" in the postquery

@leoseg
Copy link
Contributor

leoseg commented Sep 12, 2024

Hi @Nutomic after merging the current main into my branch I get multiple times the same error: "error[E0658]: use of unstable library feature 'lazy_cell'" during building when running the ./test.sh script . Before the merge all tests (including my new added test) run trough without errors ? Am I missing something here?

@Nutomic
Copy link
Member

Nutomic commented Sep 12, 2024

You probably need to use a newer Rust version.

@leoseg
Copy link
Contributor

leoseg commented Sep 12, 2024

Yes that worked, as this is only a small feature I assume a new test for the api-tests is not needed? I added a test in the src/post_view file.

@Carlos-Cabello
Copy link
Contributor

@leoseg will you continue working on this issue?

@Nutomic
Copy link
Member

Nutomic commented Sep 13, 2024

Yes that worked, as this is only a small feature I assume a new test for the api-tests is not needed? I added a test in the src/post_view file.

I dont see any new test in your PR, did you forget to push? As the change is quite small I dont mind merging it without test, but of course better testing is always good.

@Carlos-Cabello
Copy link
Contributor

Yes that worked, as this is only a small feature I assume a new test for the api-tests is not needed? I added a test in the src/post_view file.

I dont see any new test in your PR, did you forget to push? As the change is quite small I dont mind merging it without test, but of course better testing is always good.

Are you referring to my PR? I see it is already approved. I did not add any tests since I did not add any actual query logic.

@leoseg
Copy link
Contributor

leoseg commented Sep 14, 2024

I didnt open a pull request yet because it wasnt ready but as I already see @Carlos-Cabello already worked this out. So I guess he continue with the issue, but I think it would be good to at least add a small unit test in post_view , which checks if the query returns less results if only searching for title.

@leoseg
Copy link
Contributor

leoseg commented Sep 14, 2024

My test looked like this maybe you can do something similar @Carlos-Cabello

const POST_WITH_ANOTHER_TITLE: &str  = "Another title";
#[tokio::test]
  #[serial]
  async fn post_listing_title_only() -> LemmyResult<()> {
    let pool = &build_db_pool().await?;
    let pool = &mut pool.into();
    let data = init_data(pool).await?;


    // A post which contains the search them 'Post' not in the title (but in the body)
    let new_post = PostInsertForm::builder()
    .name( POST_WITH_ANOTHER_TITLE.to_string())
    .creator_id(data.local_user_view.person.id)
    .community_id(data.inserted_community.id)
    .language_id(Some(LanguageId(47)))
        .body(Some("Post".to_string()))
    .build();

    let inserted_post = Post::create(pool, &new_post).await?;


    let read_post_listing_by_title_only = PostQuery {
      community_id: Some(data.inserted_community.id),
      local_user: None,
      search_term: Some("Post".to_string()),
      search_title_only: Some(true),
      ..data.default_post_query()
    }
    .list(&data.site, pool)
    .await?;

    let read_post_listing  = PostQuery {
      community_id: Some(data.inserted_community.id),
      local_user: None,
      search_term: Some("Post".to_string()),
      ..data.default_post_query()
    }
    .list(&data.site, pool)
    .await?;

    // Should be 4 posts when we do not search for title only
    assert_eq!(
      vec![POST_WITH_ANOTHER_TITLE, POST_BY_BOT, POST, POST_BY_BLOCKED_PERSON],
      names(&read_post_listing)
    );

    // Should be 3 posts when we search for title only
   assert_eq!(
      vec![POST_BY_BOT, POST, POST_BY_BLOCKED_PERSON],
      names(&read_post_listing_by_title_only)
    );
    Post::delete(pool, inserted_post.id).await?;
    cleanup(data, pool).await
  }

@dessalines
Copy link
Member

@leoseg you can also add that as a PR to @Carlos-Cabello 's branch / PR.

@Carlos-Cabello
Copy link
Contributor

This issue has been implemented, we can go ahead and close it.

@Nutomic Nutomic closed this as completed Sep 16, 2024
@leoseg
Copy link
Contributor

leoseg commented Sep 19, 2024

Hi, sorry for the delay I was busy working, the issue is already merged but I implemented the unittest in post_view.rs and created a pull request: #5033

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: search enhancement New feature or request extra: good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants