TPAC Expert search results do not display after 100th record

Bug #1084269 reported by Diane Cowen
16
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Medium
Unassigned
2.6
Fix Released
Medium
Unassigned

Bug Description

When doing a TPAC expert search of MARC tags it is not possible to view any results beyond the first 100. Initial search results indicate a number over 100. When you get past the 100th record, it says "sorry no results".

Revision history for this message
Jessie M. Bunker-Maxwell (jessb) wrote :

We wanted to add that this behavior of the expert search occurs in version 2.3 and that we are reporting this on behalf of the Santa Cruz Public Library.

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

Confirmed in 2.2 and in master.

Changed in evergreen:
status: New → Confirmed
Revision history for this message
Ben Shum (bshum) wrote :

Assigning series targets.

Changed in evergreen:
importance: Undecided → Medium
Changed in evergreen:
milestone: none → 2.4.0-alpha
Ben Shum (bshum)
Changed in evergreen:
milestone: 2.4.0-alpha1 → 2.4.0-beta
Ben Shum (bshum)
Changed in evergreen:
milestone: 2.4.0-beta → 2.4.0-rc
Ben Shum (bshum)
Changed in evergreen:
milestone: 2.4.0-rc → none
Revision history for this message
Bill Ott (bott) wrote :

I've tracked this down to:
Storage/Publisher/metabib.pm

specifically: sub biblio_multi_search_full_rec

The following is always 100, because there's no $arg{limit} being supplied by the call from sub marc_search in Biblio/Search.pm
    my $limit = $args{limit} || 100;

The calling sub has a couple FIXME notes:
    $limit ||= 10; # FIXME: what about $args->{limit} ?
    $offset ||= 0; # FIXME: what about $args->{offset} ?

Unfortunately, I don't believe $limit in biblio_multi_search_full_rec means what I think it means. It seems that if I simply pass $limit as $args->{limit}, that becomes the maximum number of results returned, and I get 1 page of results containing the $limit number of records, and no other pages.

Revision history for this message
Bill Ott (bott) wrote :

...of course that "Biblio/Search.pm" should be "Search/Biblio.pm"

Ben Shum (bshum)
no longer affects: evergreen/2.2
tags: added: expertsearch opac tpac
Revision history for this message
Liam Whalen (whalen-ld) wrote :

I have a fix for this and other expert search functionality. It needs to be tested, but as soon as it is, I will add it to my user branch.

Bill, the args are passed from Biblio/Search.pm to Publisher/metabib.pm and it is there that offset and limit are needed as args. However, in Biblio.pm it limits the results returned by metatbib.pm according to the offset and limit as well, so by passing offset and limit as args into metabib.pm the search returns only a subset of the results. Then Biblio.pm tries to use the same offset and limit on that subset of results, which will not work.

For example, if your offset is 10 and your limit is 10, you will get records 10 - 19 back from metabib.pm, which is only 10 records. Biblio.pm then tries to find records 10 - 19 in the results passed back from metabib.pm. However, because metabib limited the results to records 10 - 19 the results passed back to Biblio.pm only have records 0 - 9 in them, so you get no results when search passed the first page. The first page works because metabib returns records 0 - 9 and Biblio.pm limits to records 0 - 9.

Revision history for this message
Liam Whalen (whalen-ld) wrote :

Here is my commit for this issue:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commit;h=d904d340f45ddcb7a79970d0600aea497e06e0fe

I have tested a similar fix on Sitka's system. However, I do not have a stock version of 2.4 working, so I could not test this commit. If someone could test this on 2.4 that would be great. If it works, I'll add a pullrequest tag to it. I did spend a fair bit of time diffing and eyeballing the code, so I feel confident that all the necessary parts are there.

At some point I will get a stock 2.4 system running, so I will test this eventually if no one else does.

As well, I have not tried to work these changes into master yet, but I will do that too.

Revision history for this message
Liam Whalen (whalen-ld) wrote :

I have found an error with my commit. Once an Expert search is performed, the other search features will not longer work because the Expert search terms are now included in the form. These terms force the search to use Expert search. I have a fix, and I am testing it.

I will install a stock 2.4 system and test this throughly this weekend.

Revision history for this message
Ben Shum (bshum) wrote :

Hi Liam, I was just taking a look at the proposed bug fixes for expert search here and wondered if we had any new progress to try in our testing.

While the last comment notes an issue with params, I wondered if that had been resolved with work in bug 1037171.

Also, when we dust this off, I'd really suggest not putting code changes in alongside whitespace/tab changes. It makes it harder for those of us reviewing since we have to check each line to know whether what changed. Additionally, I can see that a lot of the changes were from towards tabs. The coding convention I think is that we are not supposed to use tabs anymore but use 4 spaces for any equivalent tabbing. I think your patch will need reworking to accommodate that practice before we can merge it to master.

no longer affects: evergreen/2.3
Revision history for this message
Liam Whalen (whalen-ld) wrote :

Hi Ben,

Thank you for the suggestions. The issue with params was fixed in 1037171.

I have recreated this commit and tested it on:

Evergreen 2.6.1
Opensrf 2.3.0
Ubuntu 12.04
PostgreSQL 9.1

The commit for 2.6 is here:

 http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/ldw/rel_2_6_LP1084269_LP1050043_expert_search_fixes

The commit for 2_next is here: (I did not test this one)

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/ldw/rel_2_next_LP1084269_LP1050043_expert_search_fixes

Liam

Revision history for this message
Liam Whalen (whalen-ld) wrote :

In order to test that MARC expert search can go past 100 records I used MARC field 901 $b = AUTOGEN

Liam

tags: added: pullrequest
Revision history for this message
Sarah Childs (sarahc) wrote :

I have tested this code and consent to signing off on it with my email address, [<email address hidden>], and name, [Sarah Childs]

I tested it at https://mlnc1.mvlcstaff.org/eg/opac/home based on Kathy's email.

I used both Liam's suggested search above of MARC field 901 $b = AUTOGEN as well as MARC field 300 $a = p.

I got good results with both searches, could access the results all the way to the end of the list in both cases, more than 300 for one and 137 for the other. I didn't notice any other issues.

Revision history for this message
Kathy Lussier (klussier) wrote :
Kathy Lussier (klussier)
tags: added: signedoff
Revision history for this message
Ben Shum (bshum) wrote :

Thanks for testing Sarah! Added signoff line to the branch Kathy linked to and pushed to master and rel_2_6. There was a conflict with rel_2_5

Changed in evergreen:
milestone: none → 2.7.0-beta2
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
no longer affects: evergreen/2.3
no longer affects: evergreen/2.4
no longer affects: evergreen/2.5
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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