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 filter names for v.30.0 #597

Merged
merged 1 commit into from
Nov 16, 2022

Conversation

alallema
Copy link
Contributor

  • Rename:
    • indexUid renamed to indexUids
    • type renamed to types
    • status renamed to statuses
    • uid renamed to uids
  • Tests:
    • Fix tests
  • Fix comment when return a TaskInfo

Copy link
Contributor

@bidoubiwa bidoubiwa left a comment

Choose a reason for hiding this comment

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

Tests are missing on:

  • statuses
  • types
  • uids

Change of naming is missing in the .code-samples line 624

get_all_tasks_filtering_2: |-
  client.get_tasks({
    'status': ['succeeded', 'failed'],
    'type': ['documentAdditionOrUpdate']
  })

meilisearch/index.py Show resolved Hide resolved
meilisearch/index.py Show resolved Hide resolved
@alallema alallema force-pushed the update_filter_names_for_v.30.0 branch from bbe5cbf to 12c2214 Compare November 16, 2022 11:09
@alallema
Copy link
Contributor Author

alallema commented Nov 16, 2022

@bidoubiwa

Change of naming is missing in the .code-samples line 624

Done!

Tests are missing on:
statuses
types
uids

Yeah, I wondered if it was worth it and in reality, these tests would only be there to test the API the actual test is already well-testing if the function/request is working correctly. For me, it just makes the tests longer to test the API, WDYT?

@bidoubiwa
Copy link
Contributor

bidoubiwa commented Nov 16, 2022

Yeah, I wondered if it was worth it and in reality, these tests would only be there to test the API the actual test is already well-testing if the function/request is working correctly. For me, it just makes the tests longer to test the API, WDYT?

You could do a test using all the new query parameters and then check in task.details.originalFilters if they were added correctly. See this example:

https://github.com/meilisearch/meilisearch-rust/blob/05f1d5ed2e187435d8115cbf4c7e5f3365f912f6/src/tasks.rs#L758

or

https://github.com/meilisearch/meilisearch-rust/blob/05f1d5ed2e187435d8115cbf4c7e5f3365f912f6/src/tasks.rs#L821

Its only one test!

meilisearch/client.py Outdated Show resolved Hide resolved
@alallema alallema force-pushed the update_filter_names_for_v.30.0 branch 3 times, most recently from 5a9776d to 8525470 Compare November 16, 2022 12:25
@alallema
Copy link
Contributor Author

@bidoubiwa,

You could do a test using all the new query parameters and then check in task.details.originalFilters if they were added correctly.

I add tests for filters but I can test task.details.originalFilters field before the creation of cancel_tasks() or delete_tasks() method

@alallema alallema force-pushed the update_filter_names_for_v.30.0 branch from 8525470 to a3495d3 Compare November 16, 2022 12:30
Copy link
Contributor

@bidoubiwa bidoubiwa left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

@alallema alallema merged commit dee4333 into bump-meilisearch-v0.30.0 Nov 16, 2022
@alallema alallema deleted the update_filter_names_for_v.30.0 branch November 16, 2022 13:26
@alallema alallema added the breaking-change The related changes are breaking for the users label Nov 28, 2022
bors bot added a commit that referenced this pull request Nov 28, 2022
588: Changes related to the next Meilisearch release (v0.30.0) r=alallema a=meili-bot

Related to this issue: meilisearch/integration-guides#221

This PR:
- gathers the changes related to the next Meilisearch release (v0.30.0) so that this package is ready when the official release is out.
- should pass the tests against the [latest pre-release of Meilisearch](https://github.com/meilisearch/meilisearch/releases).
- might eventually contain test failures until the Meilisearch v0.30.0 is out.

⚠️ This PR should NOT be merged until the next release of Meilisearch (v0.30.0) is out.

_This PR is auto-generated for the [pre-release week](https://github.com/meilisearch/integration-guides/blob/main/resources/pre-release-week.md) purpose._

- #597 
- #595 
- #596 
- #598
- #603

Co-authored-by: meili-bot <[email protected]>
Co-authored-by: alallema <[email protected]>
Co-authored-by: Amélie <[email protected]>
bors bot added a commit that referenced this pull request Nov 28, 2022
588: Changes related to the next Meilisearch release (v0.30.0) r=alallema a=meili-bot

Related to this issue: meilisearch/integration-guides#221

This PR:
- gathers the changes related to the next Meilisearch release (v0.30.0) so that this package is ready when the official release is out.
- should pass the tests against the [latest pre-release of Meilisearch](https://github.com/meilisearch/meilisearch/releases).
- might eventually contain test failures until the Meilisearch v0.30.0 is out.

⚠️ This PR should NOT be merged until the next release of Meilisearch (v0.30.0) is out.

_This PR is auto-generated for the [pre-release week](https://github.com/meilisearch/integration-guides/blob/main/resources/pre-release-week.md) purpose._

- #597 
- #595 
- #596 
- #598
- #603

Co-authored-by: meili-bot <[email protected]>
Co-authored-by: alallema <[email protected]>
Co-authored-by: Amélie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change The related changes are breaking for the users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants