Attachment 66038 Details for Bug 19004 – Bug 19004: Adjust AddReturn for retrieving item type

[patch] Bug 19004: Adjust AddReturn for retrieving item type

SIGNED-OFF-SIGNED-OFF-Bug-19004-Adjust-AddReturn-f.patch (text/plain), 4.57 KB, created by Mark Tompsett on 2017-08-15 18:38:46 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Tompsett

Created: 2017-08-15 18:38:46 UTC

Size: 4.57 KB

patch

obsolete

>From 528ce76f644617993ed5eb7d6b72e162191c7d4b Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Wed, 9 Aug 2017 16:30:03 +0200
>Subject: [PATCH] [SIGNED-OFF] [SIGNED-OFF] Bug 19004: Adjust AddReturn for
> retrieving item type
>
>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 <m.de.rooy@rijksmuseum.nl>
>
>Note: Bugs 19070 and 19071 are already pushed. The command in comment #4
>      has all the tests successful.
>
>Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
>---
> C4/Circulation.pm                    | 8 ++++----
> t/db_dependent/Circulation/Returns.t | 4 +++-
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index fba723f..b0e26b9 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -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::Checkouts;
>@@ -1820,10 +1821,9 @@ sub AddReturn {
>     }
> 
>     my $itemnumber = $item->{ itemnumber };
>-
>-    my $item_level_itypes = C4::Context->preference("item-level_itypes");
>-    my $biblio   = $item_level_itypes ? undef : Koha::Biblios->find( $item->{ biblionumber } ); # don't get bib data unless we need it
>-    my $itemtype = $item_level_itypes ? $item->{itype} : $biblio->biblioitem->itemtype;
>+    my $itemtype = C4::Context->preference("item-level_itypes")
>+        ? $item->{itype}
>+        : Koha::Biblioitems->find( $item->{biblioitemnumber} )->itemtype;
> 
>     my $issue  = Koha::Checkouts->find( { itemnumber => $itemnumber } );
>     if ( $issue ) {
>diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t
>index 4135d66..a90f623 100644
>--- a/t/db_dependent/Circulation/Returns.t
>+++ b/t/db_dependent/Circulation/Returns.t
>@@ -202,7 +202,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
>@@ -234,6 +234,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
>@@ -243,6 +244,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
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 19004: 65789 | 65790 | 66037 | 66038 | 66486 | 66546 | 66721 | 66722 | 66723