It seems we expect MARC::Record objects, but get MarcXML for local searches - since we can double process the easiest method seem to be attempting conversion and using the updated record if successful
Created attachment 64431 [details] [review] Bug 18822 - Advanced editor - Rancor - searching broken This patch attempt to convert records before processing as we sometimes get XML instead of record objects. To test: 1 - Perform an advanced search from the advanced editor, make sure to select 'Local catalog' and a remote source 2 - Error 'Internal search error [Object object]' 3 - Apply patch 4 - Repeat search, success! 5 - Select and deselect various servers, search should still work
Created attachment 64432 [details] [review] Bug 18822 - Better error response Apply this patch first to see a change in the error feedback - instead of [Object object] you should get a text readable error
I don't get the error indicated in the test plan on a fresh kohadevbox. I did the following steps and did not get an error: (in Kohadevbox) 1. reset_all 2. Login to Intranet 3. Set EnableAdvancedCatalogingEditor to "Enable" under SysPrefs -> Labs 4. Navigate to the Cataloging module 5. Click Advanced Editor 6. Under Search, clicking "Advanced >>" 7. Select LIBRARY OF CONGRESS and Local catalog as servers 8. Type "history" in the Title field and click Search 9. Search results are listed and no error appears
As with Lee I also do not get the error specified in step 2 of the test plan of comment 1. I have tested the patches and they work the same as the current default behaviour on an up to date branch
Ah, looks like I didn't realize I had Elasticsearch set when encountering this issue, that seems to be the trigger
Wanted to test, but can't get Elasticsearch working in kohadevbox :(
Still trying to get Elasticsearch to work, when reindexing from koha-shell I get: [NoNodes] ** No nodes are available: [http://localhost:9200], called from sub Search::Elasticsearch::Role::Client::Direct::__ANON__ at /usr/share/perl5/Catmandu/Store/ElasticSearch.pm line 27.kohadev-koha@kohadevbox:/home/vagrant/kohaclone$
Created attachment 68803 [details] [review] Bug 18822 - Advanced editor - Rancor - searching broken This patch attempt to convert records before processing as we sometimes get XML instead of record objects. To test: 1 - Perform an advanced search from the advanced editor, make sure to select 'Local catalog' and a remote source 2 - Error 'Internal search error [Object object]' 3 - Apply patch 4 - Repeat search, success! 5 - Select and deselect various servers, search should still work Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 68804 [details] [review] Bug 18822 - Better error response Apply this patch first to see a change in the error feedback - instead of [Object object] you should get a text readable error Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
+ my $fixed_rec; + eval { $fixed_rec = MARC::Record::new_from_xml( $set->{hits}->[$j], 'UTF-8' ) }; + $set->{hits}->[$j] = $fixed_rec unless $@; No way to know what to expect ? Is it from internal or external ? Do we have properties of these external ones ? And can we check ref($set->{hits}->[$j]) to see if it is already MARC::Record ?
Comment10 did not receive an answer. Changing the status sometimes helps :)
(In reply to Marcel de Rooy from comment #10) > + my $fixed_rec; > + eval { $fixed_rec = MARC::Record::new_from_xml( $set->{hits}->[$j], > 'UTF-8' ) }; > + $set->{hits}->[$j] = $fixed_rec unless $@; > > No way to know what to expect ? Is it from internal or external ? Do we have > properties of these external ones ? > And can we check ref($set->{hits}->[$j]) to see if it is already > MARC::Record ? We have a slightly larger issue here of sometimes returning MARC::Records and sometimes returning marcxml. I suppose we could test the type, or assume in this case always marcxml, this was the simplest band-aid, we will need to address the larger problem on another report.
(In reply to Marcel de Rooy from comment #10) > + my $fixed_rec; > + eval { $fixed_rec = MARC::Record::new_from_xml( $set->{hits}->[$j], > 'UTF-8' ) }; > + $set->{hits}->[$j] = $fixed_rec unless $@; > > No way to know what to expect ? Is it from internal or external ? Do we have > properties of these external ones ? > And can we check ref($set->{hits}->[$j]) to see if it is already > MARC::Record ? Aha, so... We are searching the local database via z39.50 (not ES) We pass the results to new_record_from_zebra That sub assumes that if ES is enabled it will only be getting ES results (not true) I see two solutions: 1 - Add a param to new_record_from_zebra to tell it to convert records or not 2 - Move the test for ES outside the sub, always convert the records in the sub and just don't call it if using ES Opinion Marcel? or other suggestion
Created attachment 75561 [details] [review] Bug 18822: Check if we have MARC::Record and convert if not when using ES The new_record_from_zebra subroutine assumes that when using ES we always get MARC::Record objects when using ES, but sometimes we get them as xml via Z39 or internally. This adds a test to new_from_zebra to confirm we have a record object and to convert it if not To test: 1 - Perform an advanced search from the advanced editor, make sure to select 'Local catalog' and a remote source 2 - Error 'Internal search error [Object object]' 3 - Apply patch 4 - Repeat search, success! 5 - Select and deselect various servers, search should still work
*** Bug 17739 has been marked as a duplicate of this bug. ***
This will only explode when there is a record found locally.
Created attachment 76764 [details] [review] Bug 18822 - Better error response Apply this patch first to see a change in the error feedback - instead of [Object object] you should get a text readable error Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76765 [details] [review] Bug 18822: Check if we have MARC::Record and convert if not when using ES The new_record_from_zebra subroutine assumes that when using ES we always get MARC::Record objects when using ES, but sometimes we get them as xml via Z39 or internally. This adds a test to new_from_zebra to confirm we have a record object and to convert it if not To test: 1 - Perform an advanced search from the advanced editor, make sure to select 'Local catalog' and a remote source 2 - Error 'Internal search error [Object object]' 3 - Apply patch 4 - Repeat search, success! 5 - Select and deselect various servers, search should still work Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76937 [details] [review] Bug 18822 - Better error response Apply this patch first to see a change in the error feedback - instead of [Object object] you should get a text readable error Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 76938 [details] [review] Bug 18822: Check if we have MARC::Record and convert if not when using ES The new_record_from_zebra subroutine assumes that when using ES we always get MARC::Record objects when using ES, but sometimes we get them as xml via Z39 or internally. This adds a test to new_from_zebra to confirm we have a record object and to convert it if not To test: 1 - Perform an advanced search from the advanced editor, make sure to select 'Local catalog' and a remote source 2 - Error 'Internal search error [Object object]' 3 - Apply patch 4 - Repeat search, success! 5 - Select and deselect various servers, search should still work Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Second patch: no eval around new_from_xml as in the Zebra counterpart. No blocker.
Awesome work all! Pushed to master for 18.11
Pushed to 18.05.x for 18.05.02
Pushed to 17.11.x for 17.11.09
*** Bug 18193 has been marked as a duplicate of this bug. ***