Skip to content

Conversation

@brunoocasali
Copy link
Member

This PR changes all the places that currently are handling the summarized task version from uid to taskUid. And replace the tasks route index namespace with the global one + add the ability to infer the index from it.

disclaimer: The work in the tasks is not done yet, this is the first step.

@brunoocasali brunoocasali added the breaking-change The related changes are breaking for the users label Jun 29, 2022
@brunoocasali brunoocasali requested a review from alallema June 29, 2022 20:56
{
$this->expectException(ApiException::class);
$this->index->getTask(9999999999);
$this->index->getTask(99999999);
Copy link
Member Author

Choose a reason for hiding this comment

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


$this->assertIsArray($promise);
$response = $this->index->getTasks();
$this->assertCount($preCount + 1, $response['results']);
Copy link
Member Author

Choose a reason for hiding this comment

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

The test is basically the same, but now we cannot trust the size of the list anymore, so the idea is to just compare the most recent item if they changed, we can ensure that we are filtering by index. And in the other similar test the opposite is what we look for.

Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

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

Well done ✨ !!!
Just one comment about the refactor of the index.getTask(uid)

public function getTask($uid): array
{
return $this->http->get(self::PATH.'/'.$this->uid.'/tasks'.'/'.$uid);
return $this->http->get('/tasks/'.$uid, ['indexUid' => $this->uid]);
Copy link
Contributor

Choose a reason for hiding this comment

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

On the general issue it specified the following point:

Remove GET /indexes/:indexUid/tasks/:taskUid. Use GET /tasks/:taskUid instead.

So shouldn't it be something like

Suggested change
return $this->http->get('/tasks/'.$uid, ['indexUid' => $this->uid]);
return $this->http->get('/tasks'.'/'.$uid);

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right! :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Even better you can use the one from the client maybe

Copy link
Member Author

Choose a reason for hiding this comment

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

@alallema is there any particular reason to suggest:

('/tasks'.'/'.$uid);

instead of:

('/tasks/'.$uid);

Base automatically changed from remove-dumps to bump-meilisearch-v0.28.0 June 30, 2022 12:26
@brunoocasali
Copy link
Member Author

@alallema I'll apply your suggestion in a next PR :)

@brunoocasali brunoocasali merged commit 0ff220a into bump-meilisearch-v0.28.0 Jun 30, 2022
@brunoocasali brunoocasali deleted the tasks-refactor branch June 30, 2022 14:59
bors bot added a commit that referenced this pull request Jul 11, 2022
350: Update version for the next release (v0.24.0) r=brunoocasali a=brunoocasali

This version makes this package compatible with Meilisearch v0.28.0 🎉
Check out the changelog of [Meilisearch v0.28.0](https://github.com/meilisearch/meilisearch/releases/tag/v0.28.0) for more information on the changes.

## 💥  Breaking changes

:warning: Small disclaimer: The `rawSearch` (and other `raw*` functions) are a direct connection between your PHP application and Meilisearch, you may find changes that are not present in this section.

- `MeiliSearch\Client->getDumpStatus` method was removed. (#336) `@brunoocasali`
- `MeiliSearch\Client->getIndexes` method now return a object type `IndexesResults`. (#341), (#345) `@brunoocasali`
- `MeiliSearch\Client->generateTenantToken` now require a `String apiKeyUid` which is the `uid` of the `Key` instance used to sign the token.  (#343) `@brunoocasali`
- `MeiliSearch\Client->createDump` now responds with `Task` object. (#336, #337) `@brunoocasali`
- `MeiliSearch\Client->getKeys` method now return a object type `KeysResults`. (#343), (#338) `@brunoocasali`
- `MeiliSearch\Client->updateKey` now can just update a `description` and/or `name`, if there are other key/value will be silently ignored. (#343), (#338) `@brunoocasali`
- `MeiliSearch\Client->getTasks` method now return a object type `TasksResults`. (#337), (#346) `@brunoocasali`
- `MeiliSearch\Index->getTasks` method now return a object type `TasksResults`. (#337), (#346) `@brunoocasali`
- `MeiliSearch\Index->search` `facetsDistribution` is now `facets` (#332) `@curquiza`
- `MeiliSearch\Index->search` `matches` is now `showMatchesPosition` (#332) `@curquiza`
- `MeiliSearch\Index->getDocuments` method now return a object type `DocumentsResults`.
- `MeiliSearch\Index->getDocuments` method now accepts a object as a parameter and `offset`, `limit`, `attributesToRetrieve` were not longer accepted.
- `exhaustiveNbHits`, `facetsDistribution`, `exhaustiveFacetsCount` were removed from `SearchResult`. (#332) `@curquiza`


## 🚀  Enhancements
- `MeiliSearch\Client->getIndexes` accepts a object `IndexesQuery` to filter and paginate the results.
- `MeiliSearch\Client->getKeys` accepts a object `KeysQuery` to filter and paginate the results. (#343), (#338) `@brunoocasali`
- `MeiliSearch\Client->getKey` accepts both a `Key#uid` or `Key#key` value. (#343), (#338) `@brunoocasali`
- `MeiliSearch\Client->getTasks` accepts a object `TasksQuery` to filter and paginate the results. (#337), (#346) `@brunoocasali`
- `MeiliSearch\Index->getTasks` accepts a object `TasksQuery` to filter and paginate the results. (#337), (#346) `@brunoocasali`
- `MeiliSearch\Client->createKey` can specify a `uid` (optionally) to create a new `Key`. (#343), (#338) `@brunoocasali`
- `MeiliSearch\Index->getDocument` accepts a `fields` list to compact the remap the response. (#340), (#344) `@brunoocasali`
- `MeiliSearch\Index->getDocuments` accepts a object `DocumentsQuery` to filter and paginate the results. (#340), (#344) `@brunoocasali`
- `Key` has now a `name` and `uid` string fields. (#343), (#338) `@brunoocasali`
- `estimatedTotalHits`, `facetDistribution` were added to `SearchResult` (#332) `@curquiza`
  - `nbHits` is still defined and will contain the same value as `estimatedTotalHits`.
- Sending a invalid `uid` or `apiKey` will raise `InvalidApiKeyException`. (#343) `@brunoocasali`

Thanks again to `@brunoocasali,` `@curquiza!` 🎉

Co-authored-by: Bruno Casali <[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.

3 participants