Bug 10544

Summary: stocknumber index is an alias of Number-local-acquisition
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: SearchingAssignee: Fridolin Somers <fridolin.somers>
Status: CLOSED FIXED QA Contact:
Severity: minor    
Priority: P5 - low CC: gitbot, gmcharlt, kyle, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Proposed patch
Bug 10544 - stocknumber index is an alias of Number-local-acquisition
Bug 10544 - add Number-local-acquisition in known indexes
Bug 10544 - add Number-local-acquisition in QueryParser config
Bug 10544 - add Number-local-acquisition in QueryParser config
Bug 10544 - stocknumber index is an alias of Number-local-acquisition
Bug 10544 - add Number-local-acquisition in known indexes
Bug 10544 - add Number-local-acquisition in QueryParser config
[SIGNED-OFF] Bug 10544 - stocknumber index is an alias of Number-local-acquisition
[SIGNED-OFF] Bug 10544 - add Number-local-acquisition in known indexes
Bug 10544 - add Number-local-acquisition in QueryParser config
Bug 10544 - stocknumber index is an alias of Number-local-acquisition
Bug 10544 - add Number-local-acquisition in known indexes
Bug 10544 - add Number-local-acquisition in QueryParser config
Bug 10544 - stocknumber index is an alias of Number-local-acquisition
Bug 10544 - add Number-local-acquisition in known indexes
Bug 10544 - add Number-local-acquisition in QueryParser config

Description Fridolin Somers 2013-07-05 14:34:43 UTC
Bug 6256 replaced in bib1.att stocknumber by Number-local-acquisition for number 1062.
In this case, Number-local-acquisition must be used in record.abs and stocknumber can be an alias of it in ccl.properties.
Comment 1 Fridolin Somers 2013-07-05 14:40:54 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2013-07-07 03:35:25 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2013-07-14 11:17:11 UTC
I have copied the files into the right places in my koha-dev directory and reindexed with -r. Also I cataloged a new item with a unique inventorynumber and reindexed with -z.

I can find my inventorynumber using a simple keyword search and using inv=inventorynumber. 
The search examples given in the patch don't work for me. Also stocknumber= and Number-local-acquisition= don't work.

I have turned off the QueryParser for testing this. 

Fridolyn, can you explain?
Comment 4 Fridolin Somers 2013-07-24 10:59:18 UTC Comment hidden (obsolete)
Comment 5 Fridolin Somers 2013-07-24 11:04:09 UTC
> Adding Number-local-acquisition in C4::Search known indexes allows to search without using "ccl=" prefix.
@Katrin : I think this solves your problem.
If not, can you give your ccl.properties and record.abs ?
Comment 6 Fridolin Somers 2013-07-24 11:06:07 UTC Comment hidden (obsolete)
Comment 7 Fridolin Somers 2013-07-31 13:06:29 UTC
Two last patches need signoff.
Comment 8 I'm just a bot 2013-09-29 05:33:09 UTC
Patch applied cleanly, go forth and signoff
Comment 9 Marcel de Rooy 2013-10-24 08:05:10 UTC Comment hidden (obsolete)
Comment 10 Marcel de Rooy 2013-10-24 08:05:39 UTC Comment hidden (obsolete)
Comment 11 Marcel de Rooy 2013-10-24 08:05:43 UTC Comment hidden (obsolete)
Comment 12 Marcel de Rooy 2013-10-24 08:05:48 UTC Comment hidden (obsolete)
Comment 13 Marcel de Rooy 2013-10-24 08:15:13 UTC
Just some comments for QAer:

I tested the Zebra changes with MARC21/GRS1. It works as advertised.

MARC21 field 037 is mapped to Stock-number. It may be somewhat confusing to have Stocknumber and Stock-number indexing different MARC fields. Can they be merged into the same index?

If I search with catalogue/search.pl?idx=number-local-acquisition&q=stock*
I have the error: Can't call method "sort" on an undefined value at /usr/share/koha/testclone/C4/Search.pm line 426. 
This is outside the scope of this report, but I would personally like to receive the same search results (just interpret the idx parameter case insensitive) instead of this error.

Your editing of the query parser config file now shows another index variant: number-local-acq. Why not just Number-local-acquisition? Is it really needed to have four names for this index?
Comment 14 Fridolin Somers 2013-10-24 11:27:25 UTC Comment hidden (obsolete)
Comment 15 Fridolin Somers 2013-10-24 11:28:06 UTC Comment hidden (obsolete)
Comment 16 Fridolin Somers 2013-10-24 11:30:53 UTC Comment hidden (obsolete)
Comment 17 Marcel de Rooy 2013-12-06 08:35:52 UTC
(In reply to Fridolin SOMERS from comment #16)

> Please re-sign.

Changing status to reflect that need.
Comment 18 Fridolin Somers 2013-12-06 13:25:11 UTC
(In reply to M. de Rooy from comment #17)
> Changing status to reflect that need.
Can you signoff 3rd patch ?
Comment 19 Marcel de Rooy 2014-01-23 13:33:22 UTC
(In reply to Fridolin SOMERS from comment #18)
> Can you signoff 3rd patch ?

I still have a question. We had Number-local-acquisition with a capital letter. If I disable QueryParser, I can search with Number-local-acquisition:testmr and find a record.
If I enable QueryParser, this will no longer work. The URLs from your commit message do all work, but note that now it is number-local-acquisition with a lower case letter.

I am not yet so familiar with the QueryParser configuration, but is it possible to make these indexes case insensitive or so? 
I am hesitating to sign a patch that makes this index with the capital letter disappear..
I have asked jcamins this same question in an email..
Comment 20 Marcel de Rooy 2014-01-27 09:06:52 UTC
Got the following response from Jared:
Upper-case indexes seem odd to me, but it would be easy to add them. For example, the following change in Koha/QueryParser/Driver/PQF.pm:
97    $self->add_search_field_alias( $class => $field => uc $field );

Note btw that I did not mean an uppercase index but actually an index starting with one uppercase letter (in this case: Number-local-acquisition).
I have spent some time scrolling through QueryParser and found that we could change its behavior around lines 856-857 in the regexes that parse the query for fields and aliases, but that is certainly outside the scope of this report.

I do not think that it is necessary or even appropriate to add the first mixed-case variant just for this field.
So in short, I will just sign off the current version.
Comment 21 Marcel de Rooy 2014-01-27 09:17:35 UTC Comment hidden (obsolete)
Comment 22 Marcel de Rooy 2014-01-27 09:17:48 UTC Comment hidden (obsolete)
Comment 23 Marcel de Rooy 2014-01-27 09:18:01 UTC Comment hidden (obsolete)
Comment 24 Marcel de Rooy 2014-01-27 09:22:06 UTC
Another note for the QAer:
The last patch with changes to the YAML file make koha-qa warn about duplicates. I will send Jonathan a message about that. Seems a false warning; the changes look good to me.
Comment 25 Marcel de Rooy 2014-01-27 12:59:30 UTC
(In reply to M. de Rooy from comment #24)
> Another note for the QAer:
> The last patch with changes to the YAML file make koha-qa warn about
> duplicates. I will send Jonathan a message about that. Seems a false
> warning; the changes look good to me.

Jonathan found that the language is duplicated. This problem is solved under report 11619.
Comment 26 Marcel de Rooy 2014-02-05 11:29:50 UTC
Still applies on top of 11619.
Comment 27 Kyle M Hall (khall) 2014-02-14 15:30:25 UTC
Created attachment 25276 [details] [review]
Bug 10544 - stocknumber index is an alias of Number-local-acquisition

Bug 6256 replaced in bib1.att stocknumber by Number-local-acquisition for number 1062.
In this case, Number-local-acquisition must be used in record.abs and stocknumber can be an alias of it in ccl.properties.

Test plan :
- drop zebra database (rebuild_zebra.pl -r ...)
- reindex
- test in simple search : ccl=Number-local-acquisition,alwaysmatches=''
=> you get all records with a stocknumber
- test in simple search : ccl=stocknumber,alwaysmatches=''
=> you get the same results

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 28 Kyle M Hall (khall) 2014-02-14 15:30:40 UTC
Created attachment 25277 [details] [review]
Bug 10544 - add Number-local-acquisition in known indexes

Adding Number-local-acquisition in C4::Search known indexes allows to search without using "ccl=" prefix.
Also corrects in ccl.properties : inv must be an alias of Number-local-acquisition.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 29 Kyle M Hall (khall) 2014-02-14 15:30:49 UTC
Created attachment 25278 [details] [review]
Bug 10544 - add Number-local-acquisition in QueryParser config

This patch adds the Number-local-acquisition into QueryParser configuration file.
Like in ccl.properties, "Number-local-acquisition" is the main index name and "stocknumber" and "inv" are aliases.

Test plan :
Enable QueryParser :
- Enable UseQueryParser syspref
- Edit your koha-conf.xml
- Add to "config" node : <queryparser_config>[your path]/etc/searchengine/queryparser.yaml</queryparser_config>, adapt [your patch] to your install configuration folder
- If needed copy from sources "etc/searchengine/queryparser.yaml" into your install configuration folder
Test search :
- Add Number-local-acquisition on an existing subfield in records.abs. For example on item barcode field
- Reindex Zebra database
- Choose a value of this field that will match some results. For example : "0*" will match all barcodes beginning with zero
- In intranet, enter this URL : <your server>/cgi-bin/koha/catalogue/search.pl?idx=stocknumber&q=0*&sort_by=relevance
=> You get some results
- In intranet, enter this URL : <your server>/cgi-bin/koha/catalogue/search.pl?idx=inv&q=0*&sort_by=relevance
=> You get the same results
- In intranet, enter this URL : <your server>/cgi-bin/koha/catalogue/search.pl?idx=number-local-acquisition&q=0*&sort_by=relevance
=> You get the same results

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comments on case sensitivity of index names in QueryParser, see Bugzilla.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 30 Galen Charlton 2014-02-19 20:56:12 UTC
Pushed to a master, along with a follow-up that updates the MARC21 DOM configuration.

Thanks, Fridolin!
Comment 31 Fridolin Somers 2014-05-19 15:20:55 UTC
Pushed to 3.14.x, will be in 3.14.07