Like Bug 19194 some other code calls $itemtype->value without testing the object is defined. Itemtype should always be defined but its better not fall into error page for that.
I've looked with : $ git grep --name-only '$itemtype->description' acqui/basketgroup.pl acqui/orderreceive.pl admin/itemtypes.pl catalogue/getitem-ajax.pl circ/transferstoreceive.pl labels/label-item-search.pl members/summary-print.pl opac/opac-shelves.pl virtualshelves/shelves.pl No problem '$itemtype->imageurl' and '$itemtype->notforloan' calls.
Created attachment 82354 [details] [review] Bug 21842: test Koha::ItemType in defined in patron summary print Like Bug 19194 calls on $itemtype->value must test the object is defined. This patch fixes members/summary-print.pl Test plan : 1) Define system preference item-level_itypes on "specific item" 2) Define in framework a subfield of items field : mapped on items.itype and not mandatory 3) Create an item without value on this subfield 4) Go on a patron page 5) Checkout this item 6) Click on "Print" > "Print summary" 7) Without patch you get the error : Can't call method "description" on an undefined value 8) With patch you see the itemtype description
Created attachment 82355 [details] [review] Bug 21842: test Koha::ItemType in defined in transfers to receive Like Bug 19194 calls on $itemtype->value must test the object is defined. This patch fixes circ/transferstoreceive.pl Test plan : 1) Define system preference item-level_itypes on "specific item" 2) Define in framework a subfield of items field : mapped on items.itype and not mandatory 3) Create an item without value on this subfield 4) Create a transfers to receive on this item 5) Go to Circulation > Transfers to receive 6) Without patch you get the error : Can't call method "description" on an undefined value 7) With patch you see the itemtype description
Created attachment 82356 [details] [review] Bug 21842: test Koha::ItemType in defined in order recieve items table Like Bug 19194 calls on $itemtype->value must test the object is defined. This patch catalogue/getitem-ajax.pl Test plan : 1) Define system preference item-level_itypes on "specific item" 2) Define in framework a subfield of items field : mapped on items.itype and not mandatory 3) Create an item without value on this subfield 4) Go to Acquisitions -> Find a vendor or make a new one -> create a new basket 5) Add the record from Step 1 to your basket 6) Close the basket 7) Go back to the vendor and click "Receive shipments" 8) Put in an invoice number, click Next 9) Click the "Receive" link for your item 10) Click on "Edit" in items table 11) You see item editor in a new tab 12) Change something and save 13) Without patch your change is not update The ajax call is with HTTP 500 error containing : Can't call method "description" on an undefined value 14) With patch items table is updated
Shouldn't the values be set to SOMETHING even if itemtype is undefined? $foo->{key} = defined $bar ? $bar->baz : q{}; and $foo{key} = defined $bar ? $bar->baz : q{}; Feel free to switch back to needs sign off after answering/fixing this.
Created attachment 82656 [details] [review] Bug 21842: test Koha::ItemType in defined in patron summary print Like Bug 19194 calls on $itemtype->value must test the object is defined. This patch fixes members/summary-print.pl Test plan : 1) Define system preference item-level_itypes on "specific item" 2) Define in framework a subfield of items field : mapped on items.itype and not mandatory 3) Create an item without value on this subfield 4) Go on a patron page 5) Checkout this item 6) Click on "Print" > "Print summary" 7) Without patch you get the error : Can't call method "description" on an undefined value 8) With patch you see the itemtype description
Created attachment 82657 [details] [review] Bug 21842: test Koha::ItemType in defined in transfers to receive Like Bug 19194 calls on $itemtype->value must test the object is defined. This patch fixes circ/transferstoreceive.pl Test plan : 1) Define system preference item-level_itypes on "specific item" 2) Define in framework a subfield of items field : mapped on items.itype and not mandatory 3) Create an item without value on this subfield 4) Create a transfers to receive on this item 5) Go to Circulation > Transfers to receive 6) Without patch you get the error : Can't call method "description" on an undefined value 7) With patch you see the itemtype description
Created attachment 82658 [details] [review] Bug 21842: test Koha::ItemType in defined in order recieve items table Like Bug 19194 calls on $itemtype->value must test the object is defined. This patch catalogue/getitem-ajax.pl Test plan : 1) Define system preference item-level_itypes on "specific item" 2) Define in framework a subfield of items field : mapped on items.itype and not mandatory 3) Create an item without value on this subfield 4) Go to Acquisitions -> Find a vendor or make a new one -> create a new basket 5) Add the record from Step 1 to your basket 6) Close the basket 7) Go back to the vendor and click "Receive shipments" 8) Put in an invoice number, click Next 9) Click the "Receive" link for your item 10) Click on "Edit" in items table 11) You see item editor in a new tab 12) Change something and save 13) Without patch your change is not update The ajax call is with HTTP 500 error containing : Can't call method "description" on an undefined value 14) With patch items table is updated
(In reply to M. Tompsett from comment #5) > Shouldn't the values be set to SOMETHING even if itemtype is undefined? > > $foo->{key} = defined $bar ? $bar->baz : q{}; > and > $foo{key} = defined $bar ? $bar->baz : q{}; > > Feel free to switch back to needs sign off after answering/fixing this. Very good point. I have changed the patches.
(In reply to Fridolin SOMERS from comment #6) > 2) Define in framework a subfield of items field : mapped on items.itype and > not mandatory Isn't this 952$y by default when you create a new framework from Default? (See the Advanced Constraints) or is this that mapped thing elsewhere?
(In reply to M. Tompsett from comment #10) > (In reply to Fridolin SOMERS from comment #6) > > 2) Define in framework a subfield of items field : mapped on items.itype and > > not mandatory > > Isn't this 952$y by default when you create a new framework from Default? > (See the Advanced Constraints) or is this that mapped thing elsewhere? Indeed it is 952$y : y Koha item type Tab:10, | Koha field: items.itype, Not repeatable, Not mandatory, | Auth value:itemtypes,
As I have explained on the list in a previous thread (http://lists.koha-community.org/pipermail/koha-devel/2017-July/043841.html) I am against the idea of testing for data inconsistencies at code level. I have worked on providing a script to catch such inconsistencies in the DB (see bug 21150), the idea is more to not hide them and compel people to run the script to make sure they have correct data, and that Koha will behave correctly.
Comment on attachment 82656 [details] [review] Bug 21842: test Koha::ItemType in defined in patron summary print Patch for patron summary print is obsolete. members/summary-print.pl does not contain $checkout->{itemtype_description} anymore
Marking as won't fix regarding to comment 12.