Place hold success page changes search scope

Bug #1671635 reported by Kathy Lussier on 2017-03-09
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Medium
Unassigned
2.12
Medium
Unassigned

Bug Description

Affects all supported versions

When navigating in tpac, the catalog typically retains whatever search scope was set by the user during the search session and does not change until the user explicitly sets a new search scope.

However, on the place hold success page, the search scope will change to the user's preferred library, despite the fact that the locg parameter in the URL will correctly identify the OU that was the previous search scope.

To replicate:

1. Log into Evergreen and change the search scope to something that is not the user's preferred library (I used the consortium).

2. Search for a title and place a hold on that title.

3. After submitting the hold, you will see that the search scope reverts to the user's preferred library.

4. If the user hits Continue on this page, the search scope will correctly return to the user's previous search scope.

In my test, the URL for the search success page was:
https://mlnc1.noblenet.org/eg/opac/place_hold?qtype=keyword;locg=1;detail_record_view=0;hold_source_page=%2Feg%2Fopac%2Fresults%3Fquery%3Dharry%2520potter%3Bqtype%3Dkeyword%3Blocg%3D1%3Bdetail_record_view%3D0;hold_type=T;hold_target=468

Since the locg is set to 1 in this URL, I'm unclear as to why the search scope isn't being set to the consortium.

Kathy Lussier (klussier) wrote :

Adding a note that search type and search formats are also reset to default values. In addition, no previous search terms appear in the search box.

Kathy Lussier (klussier) wrote :

Adding hidden fields to the place hold form seems to do the trick. Working branch is forthcoming.

Changed in evergreen:
assignee: nobody → Kathy Lussier (klussier)
status: New → In Progress
Kathy Lussier (klussier) wrote :

Working branch available at:
http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/lp1671635-maintain-query-parameters-throughout-hold-process

In addition to adding the hidden fields, I changed the Place Holds link on the search results page so that it no longer removes the query variable from the URL. I don't know if the query variable was being removed to solve another problem that I just reintroduced. That change was made in commit 6ec8bcea with a comment that the query parameter was added to the hold_source_page URL variable, but the commit doesn't make clear why it was removed as a URL variable. The Place hold link on the record page does not remove this variable, so I am assuming it's safe to make this change.

Test Plan:
Log into the public catalog and perform a search that is limited to a library
that differs from the user's preferred search library and using a search type
that is not keyword (e.g. title or author). Place a hold on any title from
either the search results or record page. After submitting the form, look
at the search parameters in the search bar on the hold confirmation page.

Pre-patch, the query terms will not appear, the search type will revert back
to the default of keyword, and the search library will be the user's preferred
search library.

Post-patch, the query parameters will be maintained in the search bar.

Changed in evergreen:
assignee: Kathy Lussier (klussier) → nobody
status: In Progress → Triaged
tags: added: pullrequest
Changed in evergreen:
milestone: none → 3.0-rc
Galen Charlton (gmc) on 2017-09-27
Changed in evergreen:
milestone: 3.0-rc → 3.0.1
Changed in evergreen:
milestone: 3.0.1 → 3.0.2
Galen Charlton (gmc) on 2017-11-07
Changed in evergreen:
status: Triaged → Confirmed
assignee: nobody → Galen Charlton (gmc)
Galen Charlton (gmc) wrote :

Pushed to master, rel_3_0, and rel_2_12, along with a follow-up that closes off an avenue for an XSS attack. Thanks, Kathy and Cesar!

Changed in evergreen:
status: Confirmed → Fix Committed
assignee: Galen Charlton (gmc) → nobody
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers