Skip to content

Commit

Permalink
Bug 19004: Adjust AddReturn for retrieving item type
Browse files Browse the repository at this point in the history
In the regular situation, you can get itemtype via biblio and then
biblioitem as well as via biblioitem (at least when item-level_itypes
is set to biblio).

But since Koha unfortunately defined two relations in item, one for
biblioitemnumber (the good one) and one for biblionumber (redundant),
TestBuilder (correctly) builds one biblioitem and two biblios.

If you item-level_itypes to biblio record, this will result in failing tests
when calling AddReturn (in this case Koha/Patrons.t).
It will crash on:
    Can't call method "itemtype" on an undefined value at C4/Circulation.pm line 1826.
Cause: AddReturn goes via the biblionumber to biblio and than to
biblioitems, and it does not find a biblioitem. (Not a fault from TestBuilder
but a database design problem.)

This patch makes a small change in AddReturn to retrieve the itemtype via
biblioitem. It actually is a shorter road than items->biblio->biblioitems.

Note: I do not test the Biblioitems->find call, since we already checked
the GetItem call before and we have a foreign key constraint.
I did not call $item->effective_itemtype since we still use GetItem; this
could be done later.

Adjusted Circulation/Returns.t too: If we add an item with TestBuilder and
we called AddBiblio before, we should link biblioitemnumber as well.

Test plan:
[1] Do not apply this patch yet.
[2] Set item-level_itypes to biblio record.
[3] Run t/db_dependent/Koha/Patrons.t. (It should fail.)
[4] Apply this patch.
[5] Run t/db_dependent/Koha/Patrons.t again.
[6] Run t/db_dependent/Circulation/Returns.t
[7] Git grep on AddReturn and run a few other tests calling it.
    Note: Bugs 19070/19071 address three tests that call AddReturn too.
[8] In the interface, check in a book.

Signed-off-by: Marcel de Rooy <[email protected]>

Note: Bugs 19070 and 19071 are already pushed. The command in comment #4
      has all the tests successful.

Signed-off-by: Mark Tompsett <[email protected]>

Signed-off-by: Kyle M Hall <[email protected]>

Signed-off-by: Jonathan Druart <[email protected]>
(cherry picked from commit 0fc3f19)
(cherry picked from commit fba3295)
Signed-off-by: Katrin Fischer <[email protected]>
  • Loading branch information
mderooy authored and kfischer committed Oct 6, 2017
1 parent 3435f41 commit bd2602a
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
1 change: 1 addition & 0 deletions C4/Circulation.pm
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use Algorithm::CheckDigits;
use Data::Dumper;
use Koha::Account;
use Koha::AuthorisedValues;
use Koha::Biblioitems;
use Koha::DateUtils;
use Koha::Calendar;
use Koha::IssuingRules;
Expand Down
4 changes: 3 additions & 1 deletion t/db_dependent/Circulation/Returns.t
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub {
t::lib::Mocks::mock_preference( "IssueLog", 1 );
t::lib::Mocks::mock_preference( "ReturnLog", 1 );

# Set item-level item types
# Set biblio level item types
t::lib::Mocks::mock_preference( "item-level_itypes", 0 );

# Create an itemtype for biblio-level item type
Expand Down Expand Up @@ -222,6 +222,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub {
source => 'Item',
value => {
biblionumber => $biblionumber,
biblioitemnumber => $biblioitemnumber,
homebranch => $branch,
holdingbranch => $branch,
itype => $ilevel_itemtype
Expand All @@ -231,6 +232,7 @@ subtest "AddReturn logging on statistics table (item-level_itypes=0)" => sub {
source => 'Item',
value => {
biblionumber => $biblionumber,
biblioitemnumber => $biblioitemnumber,
homebranch => $branch,
holdingbranch => $branch,
itype => undef
Expand Down

0 comments on commit bd2602a

Please sign in to comment.