search

algolia / scout-extended

Scout Extended: The Full Power of Algolia in Laravel
https://www.algolia.com/doc/framework-integration/laravel/getting-started/introduction-to-scout-extended/
MIT License
397 stars 84 forks source link

paginate()->total() always 0 #279

Closed tobz-nz closed 2 years ago

tobz-nz commented 3 years ago

Description

The count() method when paginating results seems to always be 0 when also using the query() callback method.

Steps To Reproduce

See in this example the items property has 1 result, but the total property is 0.


dd(User::search('Toby')->query(function (Builder $query) {
    return $query->with('company');
})->paginate());

Illuminate\Pagination\LengthAwarePaginator {#1977 ▼
  #total: 0
  #lastPage: 1
  #items: Illuminate\Database\Eloquent\Collection {#1929 ▼
    #items: array:1 [▶]
  }
  #perPage: 20
  #currentPage: 1
  ...
}
``
DevinCodes commented 3 years ago

Hi @tobz-nz ,

Could you please check which version of Laravel Scout gets included through the composer.lock file?

This has been a known issue, and it seems to be fixed in version 9 of Laravel Scout. Could you try to see if updating to version 9 solves the issue?

Thank you in advance!

tobz-nz commented 3 years ago

@DevinCodes I've got Scout v9.1.0 installed. So nah, doesn't seem like its fixed unfortunately..

DevinCodes commented 3 years ago

Thank you for the update, I'll take some time to dig deeper first thing tomorrow!

DevinCodes commented 3 years ago

Quick update: I believe I've found the issue. It's in the way the AlgoliaEngine in Laravel Scout handles its mapIds method; it returns the objectIDs where the new implementation of paginate in expects the model primary keys. I'll try to see if there's a way to fix this without breaking backwards compatibility.

rodrigolinsr commented 3 years ago

Just today I've bumped into this issue and after couple of hours debugging, I ended up finding the same root cause as you @DevinCodes . I've applied the changes of your pull request locally and can confirm this solves our current issue. Thanks a lot for putting this pull request up!

What's needed to make your code get merged into master and a new version generated? I consider this being quite critical.

Would be good to have a test in place for this fix. I might be able to help with this by trying to write a test for it.

DevinCodes commented 3 years ago

Hi @rodrigolinsr ,

If you could help out by writing a test for it, that would be amazing! Other than that I'll ask internally for a review of the PR, and then we need to decide when to release a new major version as this is a breaking change. I'll keep you posted.

rodrigolinsr commented 3 years ago

hey @DevinCodes , I've created this PR with a simple test for the pagination. Checked without your change and the test testPaginationWithCallback breaks, while with your changes it passes fine.

https://github.com/algolia/scout-extended/pull/288

Let me know what you think. Cheers!

cloudeweb commented 2 years ago

Hi, I updated my scout version to laravel/scout v9.3.2, but have same problem.

how can I fix it? thanks!

DevinCodes commented 2 years ago

Hi @cloudeweb ,

There's a PR open that fixes this issue. The problem is that it's BC-breaking, so we need to release a new major version before we can merge this. I can't give you an estimation of when this will be, but my guess would be somewhere in January since we'll have a new version of Laravel coming out at that time as well.