Bug 6539

Summary: Z39.50 Search ISBN Field Populated with title
Product: Koha Reporter: Peter Lorimer <peter.lorimer>
Component: SearchingAssignee: Peter Lorimer <peter.lorimer>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: minor    
Priority: P3 CC: katrin.fischer, paul.poulain, wizzyrea
Version: 3.6   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Proposed Patch for this bug
Updated Patch File Z39.50 ISBN Field Populate
Updated Patch File Z39.50 ISBN Field Populate
Updated Patch File Z39.50 ISBN Field Populate
[SIGNED-OFF] When searching the catalogue, if I get no results then hit the Z39.50 search the title field in the pop up window is populated with what I searched for.
[Signed Off] Bug 6539 - When searching the catalogue, if I get no results then hit the Z39.50 search the title field in the pop up window is populated with what I searched for.

Description Peter Lorimer 2011-06-29 10:24:57 UTC
When searching the catalogue, if I get no results then hit the Z39.50 search the title field in the pop up window is populated with what I searched for.

If I search for a valid ISBN number and hit the Z39.50 search, the title field is populated with the ISBN number I searched for. This number should populate the ISBN field and not the title field.
Comment 1 Peter Lorimer 2011-06-29 13:51:43 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2011-06-29 14:03:00 UTC
I wondered if this paragraph was deleted by accicdent, because it seems not related to the patch description:

[...]
-                    
-                    # can place hold on item ?
-                    if ((!$item->{damaged} || C4::Context->preference('AllowHoldsOnDamagedItems'))
-                      && !$item->{itemlost}
-                      && !$item->{withdrawn}
-                    ) {
-                        $can_place_holds = 1;
-                    }
-                    
                     $other_count++;
[...]
Comment 3 Peter Lorimer 2011-06-29 14:50:45 UTC
Ah yes it was deleted by accident. Well spotted. I'll revert it back in a new patch I'm releasing shortly, uses Business::ISBN rather than re-inventing the wheel.

(In reply to comment #2)
> I wondered if this paragraph was deleted by accicdent, because it seems not
> related to the patch description:
> 
> [...]
> -                    
> -                    # can place hold on item ?
> -                    if ((!$item->{damaged} ||
> C4::Context->preference('AllowHoldsOnDamagedItems'))
> -                      && !$item->{itemlost}
> -                      && !$item->{withdrawn}
> -                    ) {
> -                        $can_place_holds = 1;
> -                    }
> -                    
>                      $other_count++;
> [...]
Comment 4 Peter Lorimer 2011-06-29 15:01:36 UTC Comment hidden (obsolete)
Comment 5 Peter Lorimer 2011-06-29 16:46:04 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2011-06-30 06:07:05 UTC
Hi Peter, thx for your work on this. I have one more concern: 

+    my $bsth;
+              if ( C4::Context->preference("searchMyLibraryOnly")  )
+           {
+            $bsth =$dbh->prepare("SELECT branchcode,branchname FROM branches WHERE branchcode = '". C4::Branch::mybranch() ."' "); # FIXME : use C4::Branch::GetBranches
+            }
+            else
+            {
+             $bsth =$dbh->prepare("SELECT branchcode,branchname FROM branches ");                
+            }

This adds a check for a new system preference 'searchMyLibraryOnly' that is currently not existing in Koha (hope I didn't miss it). 
If this is something you want to contribute to Koha it should go on a different bug with more code to add the system preference properly and with a description what it is supposed to do, so that it can be documented.
Comment 7 Peter Lorimer 2011-06-30 08:53:14 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2011-07-17 12:06:50 UTC Comment hidden (obsolete)
Comment 9 Paul Poulain 2011-10-06 11:55:37 UTC
Sorry for being long to QA, but I've to mark "failed QA" because there are many lines that are unrelated to the goal of the patch.
Those lines should not harm, but I think they should be removed from the patch:

For example :
-    my $bsth =$dbh->prepare("SELECT branchcode,branchname FROM branches"); # FIXME : use C4::Branch::GetBranches
+    my $bsth =$dbh->prepare("SELECT branchcode,branchname FROM branches ");                
-                # item is withdrawn, lost, damaged, not for loan, reserved or in transit
+                # item is withdrawn, lost or damaged

-                    
-                    # can place hold on item ?
+
+ # can place hold on item ?

An many:             
+
(empty line added)
Comment 10 Liz Rea 2012-01-24 23:12:46 UTC
Created attachment 7328 [details] [review]
[Signed Off] Bug 6539 - When searching the catalogue, if I get no results then hit the Z39.50 search the title field in the pop up window is populated with what I searched for.

If I search for a valid ISBN number and hit the Z39.50 search, the title field
is populated with the ISBN number I searched for. This number should populate
the ISBN field and not the title field.

http://bugs.koha-community.org/show_bug.cgi?id=6539
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>

To test:
- Search new ISBN using ISBN search from advanced search page
- 0 results - click on Z39.50 search
- Z30.50 form will have your ISBN in ISBN search option

I am signing off on this, because it's an improvement over the current
behaviour.

I see some  problems though that should perhaps be addressed in a separate
bug or as a follow-up:

If you use th catalog search field and search for an ISBN or
a keyword the right fields of the Z39.50 search form will be populated, but
the search index will be put in front:

ISBN: kw,wrdl: 9783492251495
or
Title: kw,wrdl: koha testing

If you search for ISBN as keyword on the advanced search page, it will
still populate the Title search.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 11 Liz Rea 2012-01-24 23:13:53 UTC
I ought to add, that I cleaned up the patch per Paul's desire. Please QA again? I didn't touch the meat of it, only the whitespace/weird comment changes.
Comment 12 Liz Rea 2012-01-25 02:41:41 UTC
also, katrin - I think there's another bug in here - even without this patch you get the kw,wrdl thing on the z searches now. Hrmph. I thought maybe it was someting to do with this patch but when I went back to check the original functionality I found that the kw,wrdl thing was showing up even on master. Hrmph.
Comment 13 Paul Poulain 2012-02-01 16:50:40 UTC
QA comment:
 * a minor mistake in indentation, i've added a follow-up before pushing, just with indentation fixed
 * small patch
passed QA
Comment 14 Liz Rea 2012-02-02 05:20:34 UTC
Also, this getting pushed means Peter gets added to the history!

YAY!
Comment 15 Paul Poulain 2012-02-02 08:09:56 UTC
sorry, I thought he was already here. Patch to add him pushed
Comment 16 Jared Camins-Esakov 2012-05-23 12:57:41 UTC
This fix was included in the 3.6.x branch prior to 3.6.4.