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

Update documents by function #613

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

NoodleSamaChan
Copy link
Contributor

@NoodleSamaChan NoodleSamaChan commented Aug 30, 2024

Pull Request

Related issue

Fixes #610

What does this PR do?

-I've started working on the implementation of the update documents by function feature, but I'm struggling with writing the tests to check if the code I wrote is viable. @brunoocasali , I tried to have a look around the other tests to get some inspo but I don't seem able to find any that would match.

So far I've written this :

    /// Add a new capability to update documents based on a function.
    ///
    ///
    /// # Example
    ///
    /// # use serde::{Serialize, Deserialize};
    /// # use meilisearch_sdk::{client::*, indexes::*};
    /// #
    /// # let MEILISEARCH_URL = option_env!("MEILISEARCH_URL").unwrap_or("http://localhost:7700");
    /// # let MEILISEARCH_API_KEY = option_env!("MEILISEARCH_API_KEY").unwrap_or("masterKey");
    /// #
    /// #[derive(Serialize, Deserialize, Debug, PartialEq)]
    /// struct Movie {
    ///     name: String,
    ///     description: String
    /// }
    /// let function =  DocumentEditionByFunction {
    ///     filter: None,
    ///     context: None,
    ///     function: "if doc[\"id\"] % 2 == 0 { doc[\"title\"] = \"The best movie\" }".to_string(),
    /// };
    ///
    /// # tokio::runtime::Builder::new_current_thread().enable_all().build().unwrap().block_on(async {
    /// # let client = Client::new(MEILISEARCH_URL, Some(MEILISEARCH_API_KEY)).unwrap();
    /// let movies = client.index("update_documents_by_function");
    /// let interstellar = movies.update_documents_by_function(function).await.expect("Error during test");
    ///
    /// assert_eq!(interstellar.status.len(), 2);
    /// # movies.delete().await.unwrap().wait_for_completion(&client, None, None).await.unwrap();
    /// # });

I know that, as is, the test fail at this line /// let interstellar = movies.update_documents_by_function(function).await.expect("Error during test");

Meilisearch(MeilisearchError { error_message: "Using the documents edit route requires enabling the edit documents by function experimental feature. See https://github.com/orgs/meilisearch/discussions/762", error_code: Unknown, error_type: InvalidRequest, error_link: "https://docs.meilisearch.com/errors#feature_not_enabled" })

I am struggling a bit as I'm not confident on how to use experimental features, I don't know if you'd have any recommandations?

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

@sanders41
Copy link
Contributor

@NoodleSamaChan I haven't looked closely at this so I could be off of this, but 2 suggestions. Make sure you have enabled the editDocumentsByFunction experimental feature and add id to filterable attributes.

@NoodleSamaChan
Copy link
Contributor Author

@sanders41 thanks so much for the feedback, I'll definitely give it a try! : )
I'm sorry for the basic question, but to enable experimental features (according to the doc: https://www.meilisearch.com/docs/learn/experimental/overview ) I must write ./meilisearch --editDocumentsByFunction or something similar? Or did I misunderstood? Apologies again for the basic question

@sanders41
Copy link
Contributor

I'm sorry for the basic question, but to enable experimental features (according to the doc: https://www.meilisearch.com/docs/learn/experimental/overview ) I must write ./meilisearch --editDocumentsByFunction or something similar? Or did I misunderstood? Apologies again for the basic question

That will work for starting an instance with the feature enabled, but since this is in a test it would be better to enable it for the test. It's been a while since I worked with the code in this repo, but I'm thinking the best thing to do would be to add edit documents by function to the features here then enable it in the test. For anyone else that has worked in this repo more recently then me feel free to correct this if it is wrong.

@sanders41
Copy link
Contributor

sanders41 commented Aug 30, 2024

OK, I had some curiosity and some free time so I figured out what is going on.

  • You need to enable the feature as we discussed
  • filter in DocumentEditionByFunction should be Option<String>, though this doesn't have anything to do with your current failure.
  • In tasks.rs:
    • DocumentEdition needs to be added to TaskType
    • A new DocumentEdition struct is needed that captures edited_documents, deleted_documents, original_filter, context, function
  • In the tests:
    • Add some documents to the index
    • Make sure the function references fields in the documents

Hopefully this all makes sense 😄. If you need any help or want me to give some code examples let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v1.10.0] Update documents by function
2 participants