Skip to content
This repository has been archived by the owner on May 13, 2021. It is now read-only.

fix MeiliSearch\Search\SearchResult as array #86

Closed
wants to merge 1 commit into from
Closed

fix MeiliSearch\Search\SearchResult as array #86

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 2 additions & 2 deletions src/Engines/MeilisearchEngine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,11 @@ public function mapIds($results)
*/
public function map(Builder $builder, $results, $model)
{
if (is_null($results) || 0 === count($results['hits'])) {
if (is_null($results) || 0 === $results->getHitsCount()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest to check if the $results parameter is a SearchResult object before and then casting the object to an array.

e.g.

if ($results instanceof SearchResult) {
    $results = $results->toArray();
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but if in the custom search we use rawSearch() instead of search() we can keep count($results['hits']) instead of ->getHitsCount() ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@shokme correct. but I think as you mentioned in #87 (comment) that the PHP SDK (I didn't check on this) changed, it may be that other integrations are broken or will break if updated to the newer versions.

With my change we could also fix older integrations that copied the example from the docs with ->search() in their custom query.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@shokme the only change needed to work with the new behaviour would be my suggestion from #86 (comment)

Copy link
Collaborator

@shokme shokme Jan 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I don't remember everything about the PHP SDK changes, but it safe as long as it allow us to work with arrays and as I remember it will.

if we do this change, we should do it for every case (I didn't check if there is more than just that one) that can throw the object as array error.
We should find a away to have the ->toArray() conversion earlier than in each methods ?

Anyway, it is hard to me to find time at the moment. So I can't work on making everything perfect right now.
If you have free time and want to do it feel free to open another PR that will apply your change on every case that could throw an object as array error.(if this is the only one let me now and I will merge it of course)

Thank you for your time !

return $model->newCollection();
}

$objectIds = collect($results['hits'])->pluck($model->getKeyName())->values()->all();
$objectIds = collect($results->getHits())->pluck($model->getKeyName())->values()->all();
$objectIdPositions = array_flip($objectIds);

return $model->getScoutModelsByIds(
Expand Down