Skip to content
Snippets Groups Projects
Commit 3526bce8 authored by Markus Kalkbrenner's avatar Markus Kalkbrenner Committed by Markus Kalkbrenner
Browse files

Issue #3295536 by smokris, mkalkbrenner, aaronpinero, Nuuou: Some facets not...

Issue #3295536 by smokris, mkalkbrenner, aaronpinero, Nuuou: Some facets not working properly after update to 2.0.5
parent de5f9c0f
No related branches found
No related tags found
No related merge requests found
......@@ -3,7 +3,6 @@
namespace Drupal\facets\Plugin\facets\widget;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Url;
use Drupal\facets\FacetInterface;
use Drupal\facets\Result\Result;
use Drupal\facets\Widget\WidgetPluginBase;
......
......@@ -66,7 +66,7 @@ class FacetsUrlGenerator {
*
* @throws \InvalidArgumentException
*/
public function getUrl(array $active_filters, $keep_active = TRUE) {
public function getUrl(array $active_filters, $keep_active = TRUE): ?Url {
// We use the first defined facet to load the url processor. As all facets
// should be from the same facet source, this is fine.
// This is because we don't support generating a url over multiple facet
......@@ -117,7 +117,9 @@ class FacetsUrlGenerator {
*
* This method statically caches the URL object for a request based on the
* facet source path. This reduces subsequent calls to the processor from
* having to regenerate the URL object.
* having to regenerate the URL object. But every time you call this function
* you'll get a fresh clone of the URL object that could be manipulated
* individually.
*
* @param \Symfony\Component\HttpFoundation\Request $request
* The request.
......@@ -132,49 +134,45 @@ class FacetsUrlGenerator {
$requestUrlsByPath = &drupal_static(__CLASS__ . __FUNCTION__, []);
$request_uri = $request->getRequestUri();
if (array_key_exists($request_uri, $requestUrlsByPath)) {
return $requestUrlsByPath[$request_uri];
}
// Try to grab any route params from the original request.
// In case of request path not having a matching route, Url generator will
// fail with.
try {
$requestUrl = Url::createFromRequest($request);
}
catch (ResourceNotFoundException $e) {
// Bypass exception if no path available.
// Should be unreachable in default FacetSource implementations,
// but you never know.
if ($facet_source_path) {
$requestUrl = Url::fromUserInput($facet_source_path, [
'query' => [
'_format' => \Drupal::request()->get('_format'),
],
]);
}
else {
if ('system.404' === $request->attributes->get('_route')) {
// It seems that a facet that is configured to be rendered without its
// facet source is currently rendered on a dedicated "page not found"
// page. If the facet source has a valid path we would not land here
// but in the condition above. So the facet source must be view block
// display or something similar. In this case we could assume that
// such a facet takes care about its link target itself and doesn't
// depend on the current path or the facet source path. Let's provide
// the front page as valid fallback to let the facet do its job.
$requestUrl = Url::fromRoute('<front>');
if (!array_key_exists($request_uri, $requestUrlsByPath)) {
// Try to grab any route params from the original request. In case of
// request path not having a matching route, Url generator will fail with.
try {
$requestUrl = Url::createFromRequest($request);
} catch (ResourceNotFoundException $e) {
// Bypass exception if no path available. Should be unreachable in
// default FacetSource implementations, but you never know.
if ($facet_source_path) {
$requestUrl = Url::fromUserInput($facet_source_path, [
'query' => [
'_format' => \Drupal::request()->get('_format'),
],
]);
}
else {
throw $e;
if ('system.404' === $request->attributes->get('_route')) {
// It seems that a facet that is configured to be rendered without
// its facet source is currently rendered on a dedicated "page not
// found" page. If the facet source has a valid path we would not
// land here but in the condition above. So the facet source must be
// view block display or something similar. In this case we could
// assume that such a facet takes care about its link target itself
// and doesn't depend on the current path or the facet source path.
// Let's provide the front page as valid fallback to let the facet
// do its job.
$requestUrl = Url::fromRoute('<front>');
}
else {
throw $e;
}
}
}
}
$requestUrl->setOption('attributes', ['rel' => 'nofollow']);
$requestUrlsByPath[$request_uri] = $requestUrl;
$requestUrl->setOption('attributes', ['rel' => 'nofollow']);
$requestUrlsByPath[$request_uri] = $requestUrl;
}
return $requestUrl;
return clone $requestUrlsByPath[$request_uri];
}
}
......@@ -249,4 +249,39 @@ class WidgetIntegrationTest extends FacetsTestBase {
$this->checkFacetIsNotActive('article');
}
/**
* Tests that, when there are multiple facets, the "Show all" link's `is-active` CSS class doesn't leak into subsequent inactive facet links.
* https://www.drupal.org/project/facets/issues/3295536
*/
public function testMultilpleResetLinks() {
$firstId = 'first_facet';
$this->createFacet('First Facet', $firstId);
$firstEditUrl = 'admin/config/search/facets/' . $firstId . '/edit';
$this->drupalGet($firstEditUrl);
$this->submitForm(['widget' => 'links', 'widget_config[show_reset_link]' => TRUE], 'Save');
$secondId = 'second_facet';
$this->createFacet('Second Facet', $secondId);
$secondEditUrl = 'admin/config/search/facets/' . $secondId . '/edit';
$this->drupalGet($secondEditUrl);
$this->submitForm(['widget' => 'links', 'widget_config[show_reset_link]' => TRUE], 'Save');
$this->drupalGet('search-api-test-fulltext');
$showAllLinks = $this->findFacetLink('Show all');
$this->assertCount(2, $showAllLinks);
for ($i = 0; $i < 2; ++$i)
$this->assertTrue($showAllLinks[$i]->getParent()->hasClass('is-active'), 'The "Show all" link should be active.');
$itemLinks = $this->findFacetLink('item');
$this->assertCount(2, $itemLinks);
for ($i = 0; $i < 2; ++$i)
$this->assertTrue(! $itemLinks[$i]->getParent()->hasClass('is-active'), 'The "item" link should not be active.');
$articleLinks = $this->findFacetLink('article');
$this->assertCount(2, $articleLinks);
for ($i = 0; $i < 2; ++$i)
$this->assertTrue(! $articleLinks[$i]->getParent()->hasClass('is-active'), 'The "article" link should not be active.');
}
}
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment