Search loses advanced search limiters after changing sort method or further limiting the search

Bug #788629 reported by Kathy Lussier
36
This bug affects 8 people
Affects Status Importance Assigned to Milestone
Evergreen
Won't Fix
Undecided
Unassigned

Bug Description

After applying limiters to a search from the advanced search page, if a user then tries to sort the results, tries to limit to available items, or tries to limit by applying a facet, the previously-applied limiters are no longer retained in the search. For example, in a case where a user searches for "solar system" and limits the search to juvenile books, if the user then tries to sort by pubdate on the search results page, the limiters for juvenile and non-fiction are no longer applied. Instead, the catalog will retrieve all "solar system" material and sort it by pubdate.

This was tested in a 2.1 system, but I saw the same behavior in other 2.0 catalogs.

Revision history for this message
Liam Whalen (lwhalen) wrote :

I can confirm that this is occurring in the following system.

Evergreen 2.0.8
OpenSRF 2.0.1
PostgreSQL 9.0.4
Ubuntu 10.04

I have attached a patch that fixes this problem in 2.0.8 system. Most of the code was already in existence to fix this issue. When sorting, the function called to execute the sort is searchBarSubmit. When searchBarSubmit is called by the sorting UI, the UI passes a value of isFilterSort = true.

Inside searchBarSubmit there is a call to opac_utils.js clearSearchParams function. By wraping the call to clearSearchParams in check to make sure isFilterSort is not true, we can preserve the serach options when performing a sort operation.

tags: added: pullrequest
Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
status: New → In Progress
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Liam, thank you for this contribution to Evergreen.

I do have a couple of suggestions for you and this patch:

1. There is a typo in the code. You have the variable named isFileterSort when it should be isFilterSort, so you might want to correct that on your end.

2. You are obviously using Git to generate this patch and manage your own code. If you can put your changes in a publicly accessible repository in the future, you can just add the information needed to checkout your branch and you won't need to post patches. For instance, we'd want the URL for the repository and the name of the branch to checkout/merge.

It would be a good idea if you contact Galen Charlton or Thomas Berezansky to find out what you need to do to get access to the public, Evergreen working repository. I believe you only need send them a public ssh key for access. If you don't know how to get in touch with either them, you can ask on the dev mailing list or in IRC.

If you post code in the working repository, you can then just specify the path (or whatever git calls it), such as working/user/lwhalen/lp788629 or whatever you named your branch.

The above being said, I have made a branch in my private repository to test your code and that fixes the typo. I'll post it to the public working repository next week after I've seen whether or not it works or if it needs any changes.

Thanks again, and I'll update this bug with the results of testing.

Jason Stephenson
Chief Bug Wrangler, Evergreen ILS

Revision history for this message
Jason Stephenson (jstephenson) wrote :

With this branch applied, and the typo fixed, I do not see what I think is the desired behavior on my development system.

If I do an advanced search for title: dreamweaver and limit to electronic resources, I get the electronic resources.

If I subsequently change the sort to be something other than the default, I will now get all media types.

There is obviously something more going on here.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Think I've found a "solution." It works in my limited testing anyway. I basically modified searchBarInit() to set the form_selector with the item type from advanced search properly in the search bar. I also left in Liam's change to searchBarSubmit() because I didn't test without it, and it looks like it will only help the situation.

I've pushed code to a collab branch in the working repo:

working/collab/dyrcona/lp788629

If anyone comes up with any additional changes that are needed feel free to push additional commits to that branch, update this bug, and I'll have a look.

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Meant to add that the above branch has two commits:

1. For Liam's code, plus typo fix.
2. For my change to searchBarInit().

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Also, these were trivially backportable to 2.0 and 2.1:

working/collab/dyrcona/lp788629_r21
working/collab/dyrcona/lp788629_r20

I'll target those series, too.

Changed in evergreen:
importance: Undecided → Medium
Changed in evergreen:
status: In Progress → New
Changed in evergreen:
milestone: none → 2.2.0beta1
Revision history for this message
Mike Rylander (mrylander) wrote :

Pulled into master, 2.1 and 2.0.

Changed in evergreen:
status: New → Fix Committed
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Reopen this bug after the fix was reverted because it caused problems with basic search. See https://launchpad.net/bugs/979158

Changed in evergreen:
status: Fix Committed → Confirmed
Changed in evergreen:
milestone: 2.2.0beta1 → 2.2.0rc1
Changed in evergreen:
milestone: 2.2.0rc1 → 2.2.0
Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Sorry guys, but after rereading this bug, and the linked bug #979158, I'm pretty confused. What the action that still needs taken here?

Changed in evergreen:
milestone: 2.2.0 → 2.2.1
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Lebbeous,

bug #979158 basically rolled back the proposed fix for this bug. This bug is still open.

Part of the problem seems to be that the same cgi/form parameter is overloaded with two different meanings. I forget which on that is off the top of my head.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

TPAC seems to suffer from this as well, but that probably warrants a separate bug.

Revision history for this message
Lebbeous Fogle-Weekley (lebbeous) wrote :

Not sure about the future of this bug, but pullrequest should come off for now unless I'm mistaken, so doing that.

tags: removed: pullrequest
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I am inclined to say "Won't fix."

Unless this bug is fixed by the time I get back from vacation in two and a half weeks, I'll do so then.

Changed in evergreen:
milestone: 2.2.1 → 2.2.2
Ben Shum (bshum)
Changed in evergreen:
milestone: 2.2.2 → 2.2.3
Revision history for this message
Jason Stephenson (jstephenson) wrote :

This is primarily a JSPAC bug, and we're no longer interested in JSPAC.

Changed in evergreen:
status: Confirmed → Won't Fix
no longer affects: evergreen/2.1
no longer affects: evergreen/2.0
Changed in evergreen:
milestone: 2.2.3 → none
importance: Medium → Undecided
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.