Bug 8293

Summary: Software error when clicking on first or last result of each page on OPAC
Product: Koha Reporter: Koha Team University Lyon 3 <koha>
Component: OPACAssignee: Fridolin Somers <fridolin.somers>
Status: CLOSED FIXED QA Contact: Mason James <mtj>
Severity: critical    
Priority: P1 - high CC: chris, jonathan.druart+koha, mtj, paul.poulain
Version: 3.8   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7894
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Proposed patch
Bug 8293: Software error when clicking on first or last result of each page on OPAC
Bug 8293: Replace IFNULL with COALESCE
Bug 8293 : Fixing POD to match actuality.
Bug 8293 Fixing POD to match actuality

Description Koha Team University Lyon 3 2012-06-22 13:39:10 UTC
When you click on the last result of first page or first/last result of following pages of results you get this :

Unsupported UNIMARC character encoding [] for XML output for UNIMARC; 100$a ->  at /usr/share/perl5/MARC/File/XML.pm line 505.

This happens because of the following patch  :
"bug_6488 followup - fixes issue with bibs with no items and serials being suppressed".

+	if ($items_count > 0) {
         next if $is_opac       && $hideatopac_count >= $items_count;
         next if $hidelostitems && $itemlost_count   >= $items_count;
+	}

When commented the 2 conditions lines that have been added, the records display correctly again.
Strangely, this patch seems not to have been validated (cf comments thread in Bug 6488). But may be, I missed something...
Comment 1 Fridolin Somers 2012-08-08 14:45:38 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2012-08-08 23:44:51 UTC
I can't reproduce this error

Here's what I did

Search for fish
Click on the 20th result .. no error
Click on to page 2
Click on the first result ...  no error

What am I doing wrong?
Comment 3 Jared Camins-Esakov 2012-08-09 00:45:41 UTC
The fix for this was pushed several months ago.
Comment 4 Chris Cormack 2012-08-09 00:49:47 UTC
No wonder I can't recreate this, were you testing on master Fridolyn ?
Comment 5 Fridolin Somers 2012-08-09 07:53:42 UTC
I'm up to date on master : v3.09.00.030.
I forgot to say you must activate 'OpacBrowseResults' syspref.
I user Koha sandbox n°2 database.

@Jared Camins-Esakov : 
Do you know which bug/commit ?
Comment 6 Fridolin Somers 2012-08-20 10:33:32 UTC
Seems linked to Bug 7894 / commit d13f5780dfeb062a9c677043b89f82d7d7a60b3a
Comment 7 Fridolin Somers 2012-08-20 12:59:33 UTC
In my opinion, the bug can be explained by looking at the actual code (v3.09.00.030):

In opac-details.pl, the searchAgain method contains :
  my @records = $results_hashref->{$server}->{"RECORDS"};
  @newresults = searchResults('opac', '', $hits, $results_per_page, $offset, $arrParamsBusc->{'scan'}, \@records,, C4::Context->preference('hidelostitems'));

But, in opac-search.pl there is :
  @newresults = searchResults('opac', $query_desc, $hits, $results_per_page, $offset, $scan, $results_hashref->{$server}->{"RECORDS"});

In both, $results_hashref comes from C4::Search::getRecords.
In this method, search results are added to the results of a server by : 
  $results_hash->{'RECORDS'}[$j] = $record;

So the hash entry $results_hash->{'RECORDS'} contains a ref to an array of records.

Therefore, my patch proposes to use in opac-details.pl/searchAgain the same syntax as in opac-search.pl where it works fine.

Test :

You can reproduce the bug by using Sandbox n°2 (check that OpacBrowseResults syspref is on) : http://wiki.koha-community.org/wiki/Sandboxes.
Perform a search with more than 20 results and click on first result of second page.

I think it corrects the bug 7894 which is not fully corrected : see http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7894#c7
Comment 8 Jonathan Druart 2012-08-23 14:00:23 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2012-08-23 14:04:42 UTC
The proposed patch fixes the problem for me.
- Enable the OpacBrowseResults
- launch a search with more than 20 results
- click on page 2
- click on the first result
- boum ...

SearchResult wants 7 arguments, 8 are given with the current code, so the last one is useless.
Comment 10 Jonathan Druart 2012-08-23 14:37:22 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2012-08-23 14:38:12 UTC
Comment on attachment 11790 [details] [review]
Bug 8293: Replace IFNULL with COALESCE

Oups, wrong bug !
Comment 12 Mason James 2012-08-30 20:51:51 UTC
(In reply to comment #8)
> Created attachment 11787 [details] [review]
> Bug 8293: Software error when clicking on first or last result of each page
> on OPAC
> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

passing QA on this...

$ koha-qa.pl -v 2 
testing 1 commit(s) (applied to commit ba98cb9)
 * ec365ff Bug 8293: Software error when clicking on first or last result of each page on OPAC

- perlcritic-progressive tests...                 OK
- perl -c syntax tests...                         OK
- xt/tt_valid.t tests...                          OK
- xt/author/valid-template.t tests...             OK
- t/00-valid-xml.t tests...                       OK
Comment 13 Paul Poulain 2012-08-31 14:02:57 UTC
This bug fixes an obvious API call and also fixes the problem. Although the message can seem to be UNIMARC related, it is not (and I haven't the same message on my setup).

Patch pushed
Comment 14 Chris Cormack 2012-09-01 03:03:34 UTC
Pushed to 3.8.x, will be in 3.8.5

I also pushed a follow up to update the POD so people won't read that and be confused. 

I will attach that patch here for sign off and inclusion in master.
Comment 15 Chris Cormack 2012-09-01 03:04:48 UTC Comment hidden (obsolete)
Comment 16 Fridolin Somers 2012-09-03 11:46:48 UTC
Created attachment 11946 [details] [review]
Bug 8293 Fixing POD to match actuality

Signed off
(Just a comment change)
Comment 17 Jonathan Druart 2012-09-03 13:33:59 UTC
Last patch passes QA (FIX POD)
Comment 18 Paul Poulain 2012-09-05 12:39:10 UTC
Follow-up pushed, everything in master & stable if i'm not mistaking, setting status accordingly