https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43435
Brendan Lawlor <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #8 from Brendan Lawlor <[email protected]> --- This patch works according to Jonathan's test plan, which covers the case of items with itype=null. This might be out of scope, but when I was testing I found another case when the item's itype is some string that is not a valid item type, then the opac can still crash for logged in users. Earlier in the same loop IsAvailableForItemLevelRequest is called if the patron is logged in. Then C4/Reserves does Koha::ItemTypes->find($itemtype)->notforloan without doing the same check as this patch. The only exception is if an item in the loop already set $can_holds_be_placed then IsAvailableForItemLevelRequest is not called. So I think the sub IsAvailableForItemLevelRequest needs the same check as in this patch. When I tested this change fixed the case of a bad string in an itype: @@ -1582,7 +1582,8 @@ sub IsAvailableForItemLevelRequest { my $itemtype = $item->effective_itemtype; return 0 unless defined $itemtype; - my $notforloan_per_itemtype = Koha::ItemTypes->find($itemtype)->notforloan; + my $itemtype_obj = Koha::ItemTypes->find($itemtype); + my $notforloan_per_itemtype = $itemtype_obj ? $itemtype_obj->notforloan : 0; return 0 if $notforloan_per_itemtype To replicate: UPDATE biblioitems SET itemtype='WRONG' where biblionumber=2;UPDATE items SET itype='WRONG' WHERE biblionumber=2; As a user logged into the opac go to the record: http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=2 Can't call method "notforloan" on an undefined value at /kohadevbox/koha/C4/Reserves.pm line 1585 So I think the sub IsAvailableForItemLevelRequest needs the same check as in this patch. Also maybe a test could be added for this case in Reserves.t subtest IsAvailableForItemLevelRequest Frédéric's comment "We are not supposed to handle corrupted data, misc/maintenance/search_for_data_inconsistencies.pl should be used to catch those incorrect itemtypes." makes me wonder if this is necessary. Out of scope, but possible other related bugs, it's possible to get a bad itype saved on an item through the staff interface UI in a few other ways: 1. A race condition with two tabs - Create a new TEST item type that nothing uses - Delete the item type, but stop on the confirmation page - In a second tab add an item with the item type TEST - In the first tab confirm the delete 2. Using Stage MARC records for import where a record's 952$y or 942$c use codes that don't exist (or not through the ui using bulkmarcimport.pl or the RESAT API) 3. Batch record modification / MARC modification templates that sets 952$y or 942$c to a string 4. A framework where 952$y or 942$c is free text instead of a dropdown -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list -- [email protected] To unsubscribe send an email to [email protected] website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
