In the build method of DefaultFacetsSummaryManager, all facets are built and results for all facets are retrieved instead of just building and getting results for the facets which were selected for the current facet summary.
If I understand correctly, this should only be the case for the facets which were actually selected for that facets summary.
So
foreach ($facets as $facet) {
// For clarity, process facets is called each build.
// The first facet therefor will trigger the processing. Note that
// processing is done only once, so repeatedly calling this method will
// not trigger the processing more than once.
$this->facetManager->processFacets($facetsource_id);
$this->facetManager->build($facet);
}
should become
$facets_config = $facets_summary->getFacets();
foreach ($facets as $facet) {
// Do not build facets which are not part of this facet summary.
if (!isset($facets_config[$facet->id()])) {
continue;
}
// For clarity, process facets is called each build.
// The first facet therefor will trigger the processing. Note that
// processing is done only once, so repeatedly calling this method will
// not trigger the processing more than once.
$this->facetManager->processFacets($facetsource_id);
$this->facetManager->build($facet);
}and
foreach ($facets as $facet) {
$facet_results = $facet->getResults();
$show_count = $facets_config[$facet->id()]['show_count'];
if (!$show_count) {
foreach ($facet_results as $facet_result_id => $facet_result) {
$facet_results[$facet_result_id]->setCount(NULL);
}
}
$results = array_merge($facet_results, $results);
}
should become
foreach ($facets as $facet) {
// Do not get results for facets which are not part of this facet summary.
if (!isset($facets_config[$facet->id()])) {
continue;
}
$facet_results = $facet->getResults();
$show_count = $facets_config[$facet->id()]['show_count'];
if (!$show_count) {
foreach ($facet_results as $facet_result_id => $facet_result) {
$facet_results[$facet_result_id]->setCount(NULL);
}
}
$results = array_merge($facet_results, $results);
}
See attached patch.
(I marked this as bug report but this could be considered a feature rather than bug.)
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | facet_summary_manager-2841357-5.patch | 5.38 KB | borisson_ |
Comments
Comment #2
borisson_I'd probably wouldn't do it like that but instead do something like this, personal preference though but this reduces nesting and makes sure that if we ever add another loop that we wouldn't have to do the same thing again.
In any case this will also need a test.
Comment #3
jummonk commentedYou're right. See modified patch in attachment. Had to call method id() on $item though as you cannot access private class properties directly in anonymous functions.
(The test will be for when I get Webtests running on my local machine which unfortunately is not the case currently.)
Comment #4
borisson_Adds Integration test.
Comment #5
borisson_Comment #7
borisson_Committed, thanks @jummonk!