EZP-31779: Add missing language filter parameter in LocationSearchHitAdapter->getNbResult() #3053
Conversation
There was a problem hiding this comment.
+1, but you can use more expressive PR titles (and optionally commit messages), in this case maybe EZP-31779: Add missing language filtering in LocationSearchHitAdapter->getNbResult() breaking paging
Also should add test coverage for this, but for that maybe someone in Engineering can give you some hints, or maybe some in Support can help you on it. You can find some hints in #2940, but it's huge so search for adapter to see what is relevant, and in your case you should look for test class for location hit adapter to adapt instead.
lionelakpagni
changed the title
There was a problem hiding this comment.
+1 but as @andrerom said we should add test coverage for this.
In \eZ\Publish\Core\Pagination\Tests\LocationSearchHitAdapterTest we need to check if \eZ\Publish\API\Repository\SearchService::findLocations has been invoked with $langaugeFilter. Exaclly the same as it was done in \eZ\Publish\Core\Pagination\Tests\ContentSearchHitAdapterTest.
|
What is more, It also seams that those failing test on solr are related to this change: it seems like the results are now in reversed order. |
There was a problem hiding this comment.
it seems like the results are now in reversed order.
(not sure if somehow connected with this, #3029, @alongosz )
It uses stable 1.5.8, while it should use 1.5.x-dev. I'm not sure why. Branch tests are passing, so somehow this PR affects that, but it doesn't make sense. Anyway, w/o passing tests and w/o mentioned additional test coverage I can't accept the PR. I can look into Solr version requirement myself, but this will take some time.
Not reversed, but ordered lexicographically, so somehow connected to ID sort clause change. Probably the tests are not updated for this release. |
See Jira: EZP-31779 Adding language filter parameter. Forgotten here: 494b588
alongosz
force-pushed
the
EZP-31779-patch-1
branch
from
5b8c64b
to
1888bfb
Compare
Adding language filter parameter.
Forgotten here: 494b588
6.x/7.xfor bug fixes or improvements (on existing features),masterfor featuresTODO:
$ composer fix-cs).