Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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 CreditAttribution: 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!