[Koha-bugs] [Bug 9532] reserve rules with itemtype on biblio

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9532

Galen Charlton <gmcharlt at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Passed QA                   |Failed QA

--- Comment #13 from Galen Charlton <gmcharlt at gmail.com> ---
Upon review, I strongly suspect that the change to GetItem() will damage
statistics for stats-only patrons for libraries where item-level_itypes is off.

Current behavior for such libraries is:

1. GetItem() retrieves the item.
2. items.itype is null, so it will will grab bibliotitems.itemtype to set
$item->{itype}
3. CanBookBeIssued sets     $item->{'itemtype'}=$item->{'itype'};
4. For a stats-only checkout, the call to UpdateStats refers to
$item->{'itemtype'}.  This contains the bib-level item type, as expected.

With the patch series, the behavior becomes:

1. GetItem() retrieves the item.
2. items.itype is null, but since item-level_itypes is off, $item->{itype} will
remain undef.
3. CanBookBeIssued sets     $item->{'itemtype'}=$item->{'itype'};
4. For a stats-only checkout, the call to UpdateStats refers to
$item->{'itemtype'}.  This will be undef.

Consequently, there's a regression, and I don't think that the patch you've
proposed for 9351 will help.  I'm setting to failed QA for this reason.

Patches to core routines, *especially* ones that touch holds, items, and
circulation, cry out for automated regression tests to accompany the main
patch.  I will be looking for test cases in any follow-ups you provide.

One suggestion I have to improve the API: make GetItem() and routines that
fetch item data add a calculated field called effective_item_type or the like. 
This would contain the biblio-level or item-level item type based on the system
preference, but making a separate field with a name like that should make code
that references it more readable.

-- 
You are receiving this mail because:
You are watching all bug changes.