On my search page, I don't get the retain current filters box with 7.x-1.7. This appears to be a regression from #2288867: Search results built twice when using search pages with a search box and this commit: http://cgit.drupalcode.org/apachesolr/commit/apachesolr_search.pages.inc...
When I undo that commit, I get the retain current filters box back. It's happening because the checkbox element is checked with the following code:
<?php
if (apachesolr_has_searched($search_page['env_id'])) {
$query = apachesolr_current_query($search_page['env_id']);
// We use the presence of filter query params as a flag for the retain filters checkbox.
$fq = $query->getParam('fq');
}
if ($fq || isset($form_state['input']['retain-filters'])) {
$form['basic']['retain-filters'] = array(
'#type' => 'checkbox',
'#title' => t('Retain current filters'),
'#default_value' => (int) !empty($_GET['retain-filters']),
);
}
?>
The commit above moves the actual search to after the form is built. So, it's too early to check for apachesolr_has_searched().
Not sure what the best solution is. Obviously, just moving the search to before form build reintroduces the original issue, but I don't understand that issue really well. When I use the search box from the search page itself, it doesn't appear to load the results twice. This commit was backported to 6.x-3.x so the issue may exist there too.
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | 6x-3x-retain_filters_checkbox-2343001-19.patch | 2.1 KB | jlandfried |
| #11 | 2343001_retain_filters.patch | 2.11 KB | mkalkbrenner |
| #9 | apachesolr_retainfilters.patch | 1.15 KB | nemanja |
Comments
Comment #1
zengenuity commentedFollow-up: I am able to recreate the original issue and get the search results lookup to fire twice when I click the search button. I think I must have been refreshing the page yesterday when testing. But, this issue remains. Unless the search results lookup happens before the form build, you can't get the retain filters checkbox to display.
Comment #2
kmcculloch commentedI ran into this issue today. I had zeroed in on the exact same code as zengenuity before I found this bug report. He described the problem precisely--undoing the commit he referenced also solved the issue for me. I don't have anything to add except to confirm that this is a real bug.
Comment #3
vangelisp commentedI also verify that the checkbox "Retain filters" is missing with this release.
As with kmcculloch, I replaced the commit and it seems to be working ok and get the results correctly (no double results).
Another issue though is that it doesn't work at all with FacetAPI Pretty Paths .. of course, this is another issue on its own.
Comment #4
jordanmagnuson commentedSame issue.
Comment #5
nemanja commentedYep, I see the same, and patch fixes it. My only concern is if this will break somewhere else. Can maintainer confirm and accept it into next release?
Comment #6
nick_vhWithout a patch I can't confirm anything :) But I would love for someone to give it a try and see if they can fix it?
Comment #7
nemanja commentedI mean the solution that you provided, not the patch. Can we simply add that line for getting search results before webform building? Can that make any problem with something else?
Comment #8
nick_vhExactly what I am curious about also. So that is what I am asking you also. Have you tried it? Are the simpletests passing? We can start iteratively, so if you upload that patch we can let the testsbots do their work.
Even though I maintain the module, I don't have all the answers. I rely on the community to tell me ;-)
Comment #9
nemanja commentedSorry, here is the patch.
And also, I have tested it on works on our site.
Comment #10
mkalkbrennerI think the proposed solution to apply the reverse patch of issue #2288867: Search results built twice when using search pages with a search box is an acceptable workaround at the moment.
But it isn't a solution for the next release. We should work on a patch that fixes both issues.
Comment #11
mkalkbrennerI think I solved this issue without reintroducing the other one.
The new code handles filters provided via Facet API as well. This might solve the facet issues mentioned here, too.
Can you please test the patch.
If some people second my solution I will push it to git and start the 6.x backport.
Comment #12
mkalkbrennerForgot status change
Comment #13
cspitzlay#11 works for me. I tried with different combinations of facet conditions, fulltext keyword, our custom filter options and the "retain filters" checkbox in question.
Comment #15
mkalkbrennerComment #16
star-szrThere is a slight regression here, before #2288867: Search results built twice when using search pages with a search box the "Retain current filters" checkbox wouldn't show on the initial search page (when no search has been performed yet). With the patch, "Retain current filters" is always displayed.
@mkalkbrenner or any other maintainers, would you prefer that be fixed here or in a separate issue?
Comment #17
mkalkbrennerIn the sites I maintain I don't see that regression. But maybe I missed something.
Feel free to reset this issue to 7.x-1.x-dev and to submit a patch based on the already committed patch from #11.
Comment #19
jlandfried commentedI also haven't run into the regression mentioned in #16.
Thanks for the 7.x-1.x-dev fix in #11! Here's a 6.x-3.x-dev backport for review.
Comment #20
MickL commented#11 should go into dev please.
I updated apachesolr module cause of security update. And now the patch doesnt apply anymore.
Comment #21
rooby commented@MickL:
The patch in 11 was committed a year ago and is part of the new version, which is why the patch won't apply now.
This issue is now for the D6 version of the patch in #19.
[EDIT]
Oops sorry I just looked at the dates and didn't initally realise that the latest release was security fix only.
This patch (#11) has been committed to the dev version but isn't in 1.8, however the patch in #11 still applies cleanly to 1.8.