From ec55454867e307d87c8d284621a050a1d6c5f12f Mon Sep 17 00:00:00 2001 From: Demian Katz Date: Thu, 20 Feb 2025 14:34:44 -0500 Subject: [PATCH] Fix applied filter editing on non-search-results pages. (#4258) --- .../VuFind/Controller/SearchController.php | 4 +- .../VuFind/View/Helper/Root/SearchMemory.php | 5 ++ .../src/VuFindTest/Mink/SearchFacetsTest.php | 55 +++++++++++++++++++ .../bootstrap3/templates/search/filters.phtml | 6 +- .../templates/search/searchbox.phtml | 25 ++++++--- .../bootstrap5/templates/search/filters.phtml | 6 +- .../templates/search/searchbox.phtml | 25 ++++++--- 7 files changed, 103 insertions(+), 23 deletions(-) diff --git a/module/VuFind/src/VuFind/Controller/SearchController.php b/module/VuFind/src/VuFind/Controller/SearchController.php index af48dbffca3..2793863e06e 100644 --- a/module/VuFind/src/VuFind/Controller/SearchController.php +++ b/module/VuFind/src/VuFind/Controller/SearchController.php @@ -111,7 +111,9 @@ public function editmemoryAction() ->rememberSearch($base . $query->getParams(false)); } - // Send the user back where they came from: + // Send the user back where they came from (but strip off the SID + // so we don't override the modified search with an older version): + $from = rtrim(preg_replace('/([?&])sid=\d+/', '$1', $from), '&?'); return $this->redirect()->toUrl($from); } diff --git a/module/VuFind/src/VuFind/View/Helper/Root/SearchMemory.php b/module/VuFind/src/VuFind/View/Helper/Root/SearchMemory.php index 7ad8007e3ff..1a0a3e75923 100644 --- a/module/VuFind/src/VuFind/View/Helper/Root/SearchMemory.php +++ b/module/VuFind/src/VuFind/View/Helper/Root/SearchMemory.php @@ -115,6 +115,11 @@ public function getLastSearchUrl(): ?string $url .= $queryHelper->getParams(false); + // Make sure the URL stored in search memory stays in sync; if the stored URL has been manipulated + // through the EditMemory action and the user goes back to a page with a sid parameter, things can + // get into a bad state. Refreshing the value here ensures consistent behavior. + $this->memory->rememberSearch($url); + return $url; } return null; diff --git a/module/VuFind/tests/integration-tests/src/VuFindTest/Mink/SearchFacetsTest.php b/module/VuFind/tests/integration-tests/src/VuFindTest/Mink/SearchFacetsTest.php index 8890d9b4966..81910df3108 100644 --- a/module/VuFind/tests/integration-tests/src/VuFindTest/Mink/SearchFacetsTest.php +++ b/module/VuFind/tests/integration-tests/src/VuFindTest/Mink/SearchFacetsTest.php @@ -1465,4 +1465,59 @@ public function testMultiSelectOnAdvancedSearch(bool $changeLanguage, bool $incl $this->assertCount(0, $page->findAll('css', '.facet.active')); } + + /** + * Test that filters applied during search show up on the record page and can be removed there. + * + * @return void + */ + public function testFilterClearingOnRecordPage(): void + { + // Start with a search with multiple filters applied: + $path = '/Search/Results' + . '?filter[]=building%3A"geo.mrc"' + . '&filter[]=format%3A"Book"' + . '&filter[]=author_facet%3A"Erickson%2C+Joette"&type=AllFields'; + $session = $this->getMinkSession(); + $session->visit($this->getVuFindUrl() . $path); + $page = $session->getPage(); + $this->assertCount(3, $page->findAll('css', '.facet.active')); + $this->clickCss($page, '#result0 a.title'); + $this->waitForPageLoad($page); + + // Make sure the active filters show up: + $filterArea = $this->findCss($page, '.active-filters'); + $filters = $filterArea->findAll('css', '.filter-value'); + $this->assertCount(3, $filters); + + // Save the current URL so we can return to it: + $urlWithSid = $session->getCurrentUrl(); + + // Remove the first filter: + $filters[0]->click(); + $this->waitForPageLoad($page); + + // There should now be fewer filters: + $filters = $filterArea->findAll('css', '.filter-value'); + $this->assertCount(2, $filters); + + // Now submit a new search: + $this->clickCss($page, '#searchForm .btn-primary'); + $this->waitForPageLoad($page); + $this->assertCount(2, $page->findAll('css', '.facet.active')); + + // Now go back to the original record page with the SID in the URL and + // confirm the return of the filters: + $session->visit($urlWithSid); + $filters = $filterArea->findAll('css', '.filter-value'); + $this->assertCount(3, $filters); + + // Remove the second filter: + $filters[1]->click(); + $this->waitForPageLoad($page); + + // There should now be fewer filters: + $filters = $filterArea->findAll('css', '.filter-value'); + $this->assertCount(2, $filters); + } } diff --git a/themes/bootstrap3/templates/search/filters.phtml b/themes/bootstrap3/templates/search/filters.phtml index 70a576a0396..b53955d2661 100644 --- a/themes/bootstrap3/templates/search/filters.phtml +++ b/themes/bootstrap3/templates/search/filters.phtml @@ -29,7 +29,7 @@ checkboxFilters as $filter): ?> removeFilter($filter['filter']) : $this->searchMemory()->getEditLink( $this->searchClassId, @@ -90,7 +90,7 @@ 0 || 'NOT' === $value['operator']) ? $join : '' ?> urlQuery) + $removeLink = isset($this->urlQuery) && !$fromSearchMemory ? $urlQuery->removeFacet($value['field'], $value['value'], $value['operator']) : $this->searchMemory()->getEditLink($this->searchClassId, 'removeFacet', $value); ?> @@ -120,7 +120,7 @@ removeAllFilters()->resetDefaultFilters() : $this->searchMemory()->getEditLink($this->searchClassId, 'removeAllFilters', 1); } diff --git a/themes/bootstrap3/templates/search/searchbox.phtml b/themes/bootstrap3/templates/search/searchbox.phtml index ae7c9763217..956d4a0cbb0 100644 --- a/themes/bootstrap3/templates/search/searchbox.phtml +++ b/themes/bootstrap3/templates/search/searchbox.phtml @@ -1,6 +1,13 @@ results ?? $this->searchMemory()->getCurrentSearch())) { + if ($this->results === null) { + $fromSearchMemory = true; + $results = $this->searchMemory()->getCurrentSearch(); + } else { + $fromSearchMemory = false; + $results = $this->results; + } + if ($results) { $params = $results->getParams(); $this->searchClassId = $params->getSearchClassId(); } else { @@ -70,6 +77,7 @@ 'checkboxFilters' => $showFilters ? $checkboxFilters : [], 'searchClassId' => $this->searchClassId, 'searchType' => $this->searchType, + 'fromSearchMemory' => $fromSearchMemory, ] ); ?> @@ -216,13 +224,14 @@ context($this)->renderInContext( 'search/filters.phtml', [ - 'params' => $params ?? null, - 'urlQuery' => isset($results) ? $results->getUrlQuery() : null, - 'filterList' => $showFilters ? $filterList : [], - 'checkboxFilters' => $showFilters ? $checkboxFilters : [], - 'searchClassId' => $this->searchClassId, - 'searchType' => $this->searchType, - ] + 'params' => $params ?? null, + 'urlQuery' => isset($results) ? $results->getUrlQuery() : null, + 'filterList' => $showFilters ? $filterList : [], + 'checkboxFilters' => $showFilters ? $checkboxFilters : [], + 'searchClassId' => $this->searchClassId, + 'searchType' => $this->searchType, + 'fromSearchMemory' => $fromSearchMemory, + ] );?> diff --git a/themes/bootstrap5/templates/search/filters.phtml b/themes/bootstrap5/templates/search/filters.phtml index 2d536e01b63..e7431f9ccf2 100644 --- a/themes/bootstrap5/templates/search/filters.phtml +++ b/themes/bootstrap5/templates/search/filters.phtml @@ -29,7 +29,7 @@ checkboxFilters as $filter): ?> removeFilter($filter['filter']) : $this->searchMemory()->getEditLink( $this->searchClassId, @@ -90,7 +90,7 @@ 0 || 'NOT' === $value['operator']) ? $join : '' ?> urlQuery) + $removeLink = isset($this->urlQuery) && !$fromSearchMemory ? $urlQuery->removeFacet($value['field'], $value['value'], $value['operator']) : $this->searchMemory()->getEditLink($this->searchClassId, 'removeFacet', $value); ?> @@ -120,7 +120,7 @@ removeAllFilters()->resetDefaultFilters() : $this->searchMemory()->getEditLink($this->searchClassId, 'removeAllFilters', 1); } diff --git a/themes/bootstrap5/templates/search/searchbox.phtml b/themes/bootstrap5/templates/search/searchbox.phtml index 4f2278a2aba..1a0789beccc 100644 --- a/themes/bootstrap5/templates/search/searchbox.phtml +++ b/themes/bootstrap5/templates/search/searchbox.phtml @@ -1,6 +1,13 @@ results ?? $this->searchMemory()->getCurrentSearch())) { + if ($this->results === null) { + $fromSearchMemory = true; + $results = $this->searchMemory()->getCurrentSearch(); + } else { + $fromSearchMemory = false; + $results = $this->results; + } + if ($results) { $params = $results->getParams(); $this->searchClassId = $params->getSearchClassId(); } else { @@ -70,6 +77,7 @@ 'checkboxFilters' => $showFilters ? $checkboxFilters : [], 'searchClassId' => $this->searchClassId, 'searchType' => $this->searchType, + 'fromSearchMemory' => $fromSearchMemory, ] ); ?> @@ -216,13 +224,14 @@ context($this)->renderInContext( 'search/filters.phtml', [ - 'params' => $params ?? null, - 'urlQuery' => isset($results) ? $results->getUrlQuery() : null, - 'filterList' => $showFilters ? $filterList : [], - 'checkboxFilters' => $showFilters ? $checkboxFilters : [], - 'searchClassId' => $this->searchClassId, - 'searchType' => $this->searchType, - ] + 'params' => $params ?? null, + 'urlQuery' => isset($results) ? $results->getUrlQuery() : null, + 'filterList' => $showFilters ? $filterList : [], + 'checkboxFilters' => $showFilters ? $checkboxFilters : [], + 'searchClassId' => $this->searchClassId, + 'searchType' => $this->searchType, + 'fromSearchMemory' => $fromSearchMemory, + ] );?>