This is a real bug, in Koha::Filter::MARC::EmbedItemsAvailability::_processrecord, $biblionumber_field should be corrected to allow controlfield like 001.
Upgrading severity to critical, this prevents reindexing zebra: $ misc/migration_tools/rebuild_zebra.pl -r -b -v Zebra configuration information ================================ Zebra biblio directory = /home/koha/env/master/var/lib/zebradb/biblios Zebra authorities directory = /home/koha/env/master/var/lib/zebradb/authorities Koha directory = /home/koha/env/master/src Lockfile = /home/koha/env/master/var/lock/zebradb/rebuild/rebuild..LCK BIBLIONUMBER in : 001$@ BIBLIOITEMNUMBER in : 090$a ================================ skipping authorities ==================== exporting biblio ==================== 1Control fields (generally, just tags below 010) do not have subfields, use data() at /home/koha/env/master/src/Koha/Filter/MARC/EmbedItemsAvailability.pm line 75.
(In reply to Koha Team Lyon 3 from comment #0) > This is a real bug, in > Koha::Filter::MARC::EmbedItemsAvailability::_processrecord, > $biblionumber_field > should be corrected to allow controlfield like 001. Good catch! Should use logic as in Koha/SearchEngine/Search.pm (extract_biblionumber). Ping Tomas
Created attachment 63389 [details] [review] Bug 18579: Regression tests This patch makes sure the tests have the biblio.biblionumber mapping mocked so we test the case where the mapping is to a control field instead of just regular data fields (in the case of UNIMARC). To test: - Apply the patch - Run: $ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t => FAIL: Tests fail due to an attemp to access a subfield on a control field. Sponsored-by: ByWater Solutions
Created attachment 63390 [details] [review] Bug 18579: Make EmbedItemsAvailability handle control fields correctly This patch makes the EmbedItemsAvailability filter handle the biblio.biblionumber > control field mapping gracefully. Right now, it was assuming a regular data field was used, and such is not the case for UNIMARC (001). To test: - Apply the tests patch - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t => FAIL: Tests fail to run - Apply this patch - Run: k$ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions
Hi, please test on your UNIMARC test environment! I provided a fix for the tests so they actually cover the control field case, and the patch correctly makes it work as expected (thanks marcelr for the pointer).
Created attachment 63394 [details] [review] [SIGNED OFF] Bug 18579: Regression tests This patch makes sure the tests have the biblio.biblionumber mapping mocked so we test the case where the mapping is to a control field instead of just regular data fields (in the case of UNIMARC). To test: - Apply the patch - Run: $ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t => FAIL: Tests fail due to an attemp to access a subfield on a control field. Sponsored-by: ByWater Solutions Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 63395 [details] [review] [SIGNED OFF] Bug 18579: Make EmbedItemsAvailability handle control fields correctly This patch makes the EmbedItemsAvailability filter handle the biblio.biblionumber > control field mapping gracefully. Right now, it was assuming a regular data field was used, and such is not the case for UNIMARC (001). To test: - Apply the tests patch - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t => FAIL: Tests fail to run - Apply this patch - Run: k$ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 63396 [details] [review] Bug 18579: Regression tests This patch makes sure the tests have the biblio.biblionumber mapping mocked so we test the case where the mapping is to a control field instead of just regular data fields (in the case of UNIMARC). To test: - Apply the patch - Run: $ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t => FAIL: Tests fail due to an attemp to access a subfield on a control field. Sponsored-by: ByWater Solutions Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 63397 [details] [review] Bug 18579: Make EmbedItemsAvailability handle control fields correctly This patch makes the EmbedItemsAvailability filter handle the biblio.biblionumber > control field mapping gracefully. Right now, it was assuming a regular data field was used, and such is not the case for UNIMARC (001). To test: - Apply the tests patch - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t => FAIL: Tests fail to run - Apply this patch - Run: k$ prove t/db_dependent/Koha/Filter/EmbedItemsAvailability.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
There are still problems in a UNIMARC setup: # Failed test 'EmbedItemsAvailability tests' # at t/db_dependent/Koha/Filter/EmbedItemsAvailability.t line 151. Can't call method "subfield" on an undefined value at t/db_dependent/Koha/Filter/EmbedItemsAvailability.t line 145. and rebuild_zebra.pl die with: Arguments must be MARC::Field objects at /home/koha/env/master/src/Koha/Filter/MARC/EmbedItemsAvailability.pm line 93 I'm working on a patch
Good catch Julian. (Poor Tomas ;)
Created attachment 63414 [details] [review] Bug 18579: Fix call to append_fields, fix tests MARC::Record::append_fields takes a list of MARC::Field (not an arrayref) Use $record->subfield() instead of $record->field()->subfield() to avoid errors when field doesn't exist
Back to Signed off
Created attachment 63415 [details] [review] Bug 18579: Fix call to append_fields, fix tests MARC::Record::append_fields takes a list of MARC::Field (not an arrayref) Use $record->subfield() instead of $record->field()->subfield() to avoid errors when field doesn't exist Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Marcel de Rooy from comment #11) > Good catch Julian. (Poor Tomas ;) Hahaha
Pushed to master for 17.05, thanks Tomas, Julian!
Patches don't apply to 16.11.x - please provide patches for 16.11.x if it's needed there too!
(In reply to Katrin Fischer from comment #17) > Patches don't apply to 16.11.x - please provide patches for 16.11.x if it's > needed there too! Not needed!