Description
Fridolin Somers
2013-07-05 14:34:43 UTC
Created attachment 19417 [details] [review] Proposed patch See commit message Created attachment 19443 [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> 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? Created attachment 19924 [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. > 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 ?
Created attachment 19925 [details] [review] Bug 10544 - add Number-local-acquisition in QueryParser config This patch adds the Number-local-acquisition in QueryParser config to behave like in ccl.properties. See commit message. Two last patches need signoff. Patch applied cleanly, go forth and signoff Created attachment 22344 [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> Created attachment 22345 [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> Created attachment 22346 [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> Created attachment 22347 [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> 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? Created attachment 22353 [details] [review] [SIGNED-OFF] Bug 10544 - stocknumber index is an alias of Number-local-acquisition No change Created attachment 22354 [details] [review] [SIGNED-OFF] Bug 10544 - add Number-local-acquisition in known indexes No change Created attachment 22355 [details] [review] Bug 10544 - add Number-local-acquisition in QueryParser config I revised QP config. I changed 'number-local-acq' to 'number-local-acquisition' It appears all indexes are lowercase in this config. Please re-sign. (In reply to Fridolin SOMERS from comment #16) > Please re-sign. Changing status to reflect that need. (In reply to M. de Rooy from comment #17) > Changing status to reflect that need. Can you signoff 3rd patch ? (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.. 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. Created attachment 24737 [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> Created attachment 24738 [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> Created attachment 24739 [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. 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. (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. Still applies on top of 11619. 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> 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> 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> Pushed to a master, along with a follow-up that updates the MARC21 DOM configuration. Thanks, Fridolin! Pushed to 3.14.x, will be in 3.14.07 |