Evergreen

TPAC: Preserve session prefs across URLs via consistent mkurl() usage

Bug #909578 reported by Dan Scott
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Undecided
Bill Erickson
Evergreen 2.2.0alpha2

Bug Description

* Evergreen master

Currently, CGI params like "loc" are dropped when a user follows a link from one part of the TPAC to another. This is rather disorienting. The use of mkurl() to generate URLs preserves the current CGI params in the generated URL, helping to keep session prefs & behaviour consistent. In addition, we can use mkurl() to generate new CGI params in a consistent way. If need be, we can also use mkurl() to clear particular CGI params, but thus far manual testing suggests that that is not generally necessary; with the exception of some actions like "Logout" or "Another Search".

Please see user/dbs/tpac_mkurl_madness in the working repo.

Tags: pullrequest
Revision history for this message
Kathy Lussier (klussier) wrote :

Thanks Dan!

There are a few places still where the catalog is still forgetting its loc parameter:

* When clicking "Another Search"
* When clicking to view your book list from the "search results" page
* When clicking on any of the subtabs in account preferences (personal information, notification preferences, search preferences.)

That's all I see for now.

Revision history for this message
Dan Scott (denials) wrote :

Hmm. I left out "Another Search" deliberately because you can always launch another search via the omni-present search bar that retains your location preference; the purpose of "Another Search" (at least as I envision it) is to initiate a new search session that resets things back to their default values.

I'll check into the other paths you mentioned. Thanks Kathy!

Revision history for this message
Dan Scott (denials) wrote :

I pushed another commit onto the branch to address these remaining issues and a number of other issues I found in further testing. These were primarily to do with actions submitted via forms, such as creating/editing/updating bookbags, although redirects to login when placing holds also needed fixing.

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

Thanks, Dan! I'll load it on my dev machine and I'm sure Kathy will have some more feedback before too long. :)

Revision history for this message
Dan Scott (denials) wrote :

Added another commit to prevent the "expand" and "cnoffset" parameters from propagating in most places from record summary links, as you probably don't want to display the MARC source of a record if you launch a new subject search.

Also fixed the double-encoding of subjects that would lead to broken searches, for example "Concert violin" would become "Concert%2520violin" instead of "Concert%20violin".

Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (erickson-esilibrary)
status: New → In Progress
Revision history for this message
Bill Erickson (berick) wrote :

Tested. Looks great. Merged to master. Thanks!

As a follow up patch (discussed in IRC), we should decide if the advanced search "pane" param should be propagated with searches from the advanced search page or remove "pane" from other advanced search page links for consistent behavior in remembering the advanced search pane.

Changed in evergreen:
status: In Progress → Fix Committed
Jason Stephenson (jstephenson)
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

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